Skip to content

Commit

Permalink
fix(buildah): add support for git owner/group settings
Browse files Browse the repository at this point in the history
Enable e2e test check for owner/group.

Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
  • Loading branch information
distorhead committed Sep 23, 2022
1 parent 414dd38 commit 623ef86
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 30 deletions.
10 changes: 8 additions & 2 deletions pkg/build/stage/git_mapping.go
Expand Up @@ -636,7 +636,10 @@ func (gm *GitMapping) PreparePatchForImage(ctx context.Context, c Conveyor, cb c
}()

logboek.Context(ctx).Debug().LogF("Adding git patch data archive with included paths: %v\n", includePaths)
stageImage.Builder.StapelStageBuilder().AddDataArchive(patchArchiveReader, archiveType, gm.To)
stageImage.Builder.StapelStageBuilder().AddDataArchive(patchArchiveReader, archiveType, gm.To, container_backend.AddDataArchiveOptions{
Owner: gm.Owner,
Group: gm.Group,
})

logboek.Context(ctx).Debug().LogF("Adding git paths to remove: %v\n", patch.GetPathsToRemove())
var pathsToRemove []string
Expand Down Expand Up @@ -699,7 +702,10 @@ func (gm *GitMapping) PrepareArchiveForImage(ctx context.Context, c Conveyor, cb
return fmt.Errorf("unable to open archive file %q: %w", archive.GetFilePath(), err)
}

stageImage.Builder.StapelStageBuilder().AddDataArchive(f, archiveType, gm.To)
stageImage.Builder.StapelStageBuilder().AddDataArchive(f, archiveType, gm.To, container_backend.AddDataArchiveOptions{
Owner: gm.Owner,
Group: gm.Group,
})
}

gm.AddGitCommitToImageLabels(ctx, c, cb, stageImage, commitInfo)
Expand Down
2 changes: 1 addition & 1 deletion pkg/buildah/base.go
Expand Up @@ -93,7 +93,7 @@ func (b *BaseBuildah) prepareBuildFromDockerfile(dockerfile []byte, contextTar i
}

if contextTar != nil {
if err := util.ExtractTar(contextTar, contextTmpDir); err != nil {
if err := util.ExtractTar(contextTar, contextTmpDir, util.ExtractTarOptions{}); err != nil {
return "", "", "", fmt.Errorf("unable to extract context tar to tmp context dir: %w", err)
}
}
Expand Down
17 changes: 12 additions & 5 deletions pkg/container_backend/build_stapel_stage_options.go
Expand Up @@ -5,6 +5,10 @@ import (
"io"
)

type AddDataArchiveOptions struct {
Owner, Group string
}

type BuildStapelStageOptionsInterface interface {
SetBaseImage(baseImage string) BuildStapelStageOptionsInterface

Expand All @@ -21,7 +25,7 @@ type BuildStapelStageOptionsInterface interface {
AddBuildVolumes(volumes ...string) BuildStapelStageOptionsInterface
AddCommands(commands ...string) BuildStapelStageOptionsInterface

AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string) BuildStapelStageOptionsInterface
AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string, o AddDataArchiveOptions) BuildStapelStageOptionsInterface
RemoveData(removeType RemoveType, paths, keepParentDirs []string) BuildStapelStageOptionsInterface
AddDependencyImport(imageName, fromPath, toPath string, includePaths, excludePaths []string, owner, group string) BuildStapelStageOptionsInterface
}
Expand Down Expand Up @@ -56,9 +60,10 @@ const (
)

type DataArchiveSpec struct {
Archive io.ReadCloser
Type ArchiveType
To string
Archive io.ReadCloser
Type ArchiveType
To string
Owner, Group string
}

type RemoveType int
Expand Down Expand Up @@ -155,11 +160,13 @@ func (opts *BuildStapelStageOptions) AddCommands(commands ...string) BuildStapel
return opts
}

func (opts *BuildStapelStageOptions) AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string) BuildStapelStageOptionsInterface {
func (opts *BuildStapelStageOptions) AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string, o AddDataArchiveOptions) BuildStapelStageOptionsInterface {
opts.DataArchiveSpecs = append(opts.DataArchiveSpecs, DataArchiveSpec{
Archive: archive,
Type: archiveType,
To: to,
Owner: o.Owner,
Group: o.Group,
})
return opts
}
Expand Down
11 changes: 9 additions & 2 deletions pkg/container_backend/buildah_backend.go
Expand Up @@ -278,12 +278,19 @@ func (runtime *BuildahBackend) applyDataArchives(ctx context.Context, container
return fmt.Errorf("unknown archive type %q", archive.Type)
}

logboek.Context(ctx).Debug().LogF("Apply data archive into %q\n", archive.To)
var err error
var uid, gid *uint32
uid, gid, err = getUIDAndGID(archive.Owner, archive.Group, container.RootMount)
if err != nil {
return fmt.Errorf("error getting UID/GID: %w", err)
}

logboek.Context(ctx).Debug().LogF("Extracting archive into container path %s\n", archive.To)
if err := util.ExtractTar(archive.Archive, extractDestPath); err != nil {

if err := util.ExtractTar(archive.Archive, extractDestPath, util.ExtractTarOptions{UID: uid, GID: gid}); err != nil {
return fmt.Errorf("unable to extract data archive into %s: %w", archive.To, err)
}

if err := archive.Archive.Close(); err != nil {
return fmt.Errorf("error closing archive data stream: %w", err)
}
Expand Down
45 changes: 39 additions & 6 deletions pkg/util/archive.go
Expand Up @@ -208,7 +208,18 @@ ArchiveCopying:
return nil
}

func ExtractTar(tarFileReader io.Reader, dstDir string) error {
type ExtractTarOptions struct {
UID, GID *uint32
}

func ExtractTar(tarFileReader io.Reader, dstDir string, opts ExtractTarOptions) error {
if err := os.MkdirAll(dstDir, os.ModePerm); err != nil {
return fmt.Errorf("unable to create dir %q: %w", dstDir, err)
}
if err := Chown(dstDir, opts.UID, opts.GID); err != nil {
return fmt.Errorf("unable to chown dir %q: %w", dstDir, err)
}

tarReader := tar.NewReader(tarFileReader)
for {
tarEntryHeader, err := tarReader.Next()
Expand All @@ -227,6 +238,7 @@ func ExtractTar(tarFileReader io.Reader, dstDir string) error {
if err = os.MkdirAll(tarEntryPath, tarEntryFileInfo.Mode()); err != nil {
return fmt.Errorf("unable to create new dir %q while extracting tar: %w", tarEntryPath, err)
}

case tar.TypeReg, tar.TypeSymlink, tar.TypeLink, tar.TypeGNULongName, tar.TypeGNULongLink:
if err := os.MkdirAll(filepath.Dir(tarEntryPath), os.ModePerm); err != nil {
return fmt.Errorf("unable to create new directory %q while extracting tar: %w", filepath.Dir(tarEntryPath), err)
Expand All @@ -247,14 +259,35 @@ func ExtractTar(tarFileReader io.Reader, dstDir string) error {
return fmt.Errorf("unable to close file %q while extracting tar: %w", tarEntryPath, err)
}

// TODO: set uid/gid from function options
// if err := os.Chown(tarEntryPath, tarEntryHeader.Uid, tarEntryHeader.Gid); err != nil {
// return fmt.Errorf("unable to set owner and group to %d:%d for file %q: %w", tarEntryHeader.Uid, tarEntryHeader.Gid, tarEntryPath, err)
// }

default:
return fmt.Errorf("tar entry %q of unexpected type: %b", tarEntryHeader.Name, tarEntryHeader.Typeflag)
}

for _, p := range FilepathsWithParents(tarEntryHeader.Name) {
if err := Chown(filepath.Join(dstDir, p), opts.UID, opts.GID); err != nil {
return fmt.Errorf("unable to chown file %q: %w", p, err)
}
}
}

return nil
}

func Chown(path string, uid, gid *uint32) error {
if uid != nil || gid != nil {
osUid := -1
osGid := -1

if uid != nil {
osUid = int(*uid)
}
if gid != nil {
osGid = int(*gid)
}

if err := os.Chown(path, osUid, osGid); err != nil {
return fmt.Errorf("unable to set owner and group to %d:%d for file %q: %w", osUid, osGid, path, err)
}
}

return nil
Expand Down
26 changes: 26 additions & 0 deletions pkg/util/path.go
@@ -1,6 +1,7 @@
package util

import (
"fmt"
"os"
"os/user"
"path/filepath"
Expand Down Expand Up @@ -37,8 +38,22 @@ func ExpandPath(path string) string {

func SplitFilepath(path string) (result []string) {
path = filepath.FromSlash(path)
path = filepath.Clean(path)

if filepath.IsAbs(path) {
p, err := filepath.Rel(string(os.PathSeparator), path)
if err != nil {
panic(fmt.Sprintf("unable to get relative path for %q", path))
}
path = p
}

separator := os.PathSeparator

if path == "." || path == string(separator) {
return nil
}

idx := 0
if separator == '\\' {
// if the separator is '\\', then we can just split...
Expand Down Expand Up @@ -117,3 +132,14 @@ func GlobPrefixWithoutPatterns(glob string) (string, string) {

return prefix, glob
}

func FilepathsWithParents(path string) []string {
var res []string
base := ""
for _, part := range SplitFilepath(path) {
base = filepath.Join(base, part)
res = append(res, base)
}

return res
}
46 changes: 39 additions & 7 deletions pkg/util/path_test.go
Expand Up @@ -12,13 +12,13 @@ import (
"github.com/werf/werf/pkg/util"
)

type entry struct {
type expandPathTest struct {
path string
expectedPathFormat string
}

var _ = DescribeTable("expand path",
func(e entry) {
func(e expandPathTest) {
usr, err := user.Current()
Ω(err).ShouldNot(HaveOccurred())

Expand All @@ -28,24 +28,56 @@ var _ = DescribeTable("expand path",
expectedPath := fmt.Sprintf(e.expectedPathFormat, usr.HomeDir, wd)
Ω(util.ExpandPath(filepath.FromSlash(e.path))).Should(Equal(filepath.FromSlash(expectedPath)))
},
Entry("~", entry{
Entry("~", expandPathTest{
path: "~",
expectedPathFormat: "%[1]s",
}),
Entry("~/", entry{
Entry("~/", expandPathTest{
path: "~/",
expectedPathFormat: "%[1]s",
}),
Entry("~/path", entry{
Entry("~/path", expandPathTest{
path: "~/path",
expectedPathFormat: "%[1]s/path",
}),
Entry("path", entry{
Entry("path", expandPathTest{
path: "path",
expectedPathFormat: "%[2]s/path",
}),
Entry("path1/../path2", entry{
Entry("path1/../path2", expandPathTest{
path: "path1/../path2",
expectedPathFormat: "%[2]s/path2",
}),
)

type splitPathTest struct {
path string
expectedPathParts []string
}

var _ = DescribeTable("split path",
func(t splitPathTest) {
parts := util.SplitFilepath(t.path)
Expect(parts).To(Equal(t.expectedPathParts))
},
Entry("root path", splitPathTest{
path: "/",
expectedPathParts: nil,
}),
Entry("unnormalized root path", splitPathTest{
path: "////",
expectedPathParts: nil,
}),
Entry("empty path", splitPathTest{
path: "",
expectedPathParts: nil,
}),
Entry("absolute path", splitPathTest{
path: "/path/to/dir/or/file",
expectedPathParts: []string{"path", "to", "dir", "or", "file"},
}),
Entry("relative path", splitPathTest{
path: "path/to/dir/or/file",
expectedPathParts: []string{"path", "to", "dir", "or", "file"},
}),
)
13 changes: 6 additions & 7 deletions test/e2e/build/complex_test.go
Expand Up @@ -98,16 +98,15 @@ var _ = Describe("Complex build", Label("e2e", "build", "complex"), func() {
contRuntime.ExpectCmdsToSucceed(
buildReport.Images["stapel-shell"].DockerImageName,
"test -f /app/README.md",
// TODO: fix buildah: support setting git.owner/group for git-archives
// "stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
"grep -qF 'https://cloud.google.com/appengine/docs/go/#Go_tools' /app/README.md",

"test -f /app/static/index.html",
// "stat -c %u:%g /app/static/index.html | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/static/index.html | diff <(echo 1050:1051) -",
"grep -qF '<title>Hello, world</title>' /app/static/index.html",

"test -f /app/static/style.css",
// "stat -c %u:%g /app/static/style.css | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/static/style.css | diff <(echo 1050:1051) -",
"grep -qF 'text-align: center;' /app/static/style.css",

"! test -f /app/app.go",
Expand Down Expand Up @@ -190,17 +189,17 @@ var _ = Describe("Complex build", Label("e2e", "build", "complex"), func() {
contRuntime.ExpectCmdsToSucceed(
buildReport.Images["stapel-shell"].DockerImageName,
"test -f /app/README.md",
// "stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
"grep -qF 'https://cloud.google.com/sdk/' /app/README.md",

"test -f /app/static/index.html",
// "stat -c %u:%g /app/static/index.html | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/static/index.html | diff <(echo 1050:1051) -",
"grep -qF '<title>Hello, world</title>' /app/static/index.html",

"! test -f /app/static/style.css",

"test -f /app/app.go",
// "stat -c %u:%g /app/app.go | diff <(echo 1050:1051) -",
"stat -c %u:%g /app/app.go | diff <(echo 1050:1051) -",
"grep -qF 'package hello' /app/app.go",

"! test -f /app/static/script.js",
Expand Down

0 comments on commit 623ef86

Please sign in to comment.