From 623ef86dfc8c689a81087088b10b4dde9865455f Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Thu, 22 Sep 2022 13:56:01 +0300 Subject: [PATCH] fix(buildah): add support for git owner/group settings Enable e2e test check for owner/group. Signed-off-by: Timofey Kirillov --- pkg/build/stage/git_mapping.go | 10 +++- pkg/buildah/base.go | 2 +- .../build_stapel_stage_options.go | 17 +++++-- pkg/container_backend/buildah_backend.go | 11 ++++- pkg/util/archive.go | 45 +++++++++++++++--- pkg/util/path.go | 26 +++++++++++ pkg/util/path_test.go | 46 ++++++++++++++++--- test/e2e/build/complex_test.go | 13 +++--- 8 files changed, 140 insertions(+), 30 deletions(-) diff --git a/pkg/build/stage/git_mapping.go b/pkg/build/stage/git_mapping.go index 003ad62b02..0bbed8b821 100644 --- a/pkg/build/stage/git_mapping.go +++ b/pkg/build/stage/git_mapping.go @@ -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 @@ -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) diff --git a/pkg/buildah/base.go b/pkg/buildah/base.go index 0d3cec46d6..d865ab05ee 100644 --- a/pkg/buildah/base.go +++ b/pkg/buildah/base.go @@ -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) } } diff --git a/pkg/container_backend/build_stapel_stage_options.go b/pkg/container_backend/build_stapel_stage_options.go index 2187c05ed4..2cd4da0944 100644 --- a/pkg/container_backend/build_stapel_stage_options.go +++ b/pkg/container_backend/build_stapel_stage_options.go @@ -5,6 +5,10 @@ import ( "io" ) +type AddDataArchiveOptions struct { + Owner, Group string +} + type BuildStapelStageOptionsInterface interface { SetBaseImage(baseImage string) BuildStapelStageOptionsInterface @@ -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 } @@ -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 @@ -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 } diff --git a/pkg/container_backend/buildah_backend.go b/pkg/container_backend/buildah_backend.go index 7e945c25c1..396f933955 100644 --- a/pkg/container_backend/buildah_backend.go +++ b/pkg/container_backend/buildah_backend.go @@ -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) } diff --git a/pkg/util/archive.go b/pkg/util/archive.go index 6bab49d81c..9bf7eac446 100644 --- a/pkg/util/archive.go +++ b/pkg/util/archive.go @@ -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() @@ -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) @@ -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 diff --git a/pkg/util/path.go b/pkg/util/path.go index b6cd7d544c..61d668d70c 100644 --- a/pkg/util/path.go +++ b/pkg/util/path.go @@ -1,6 +1,7 @@ package util import ( + "fmt" "os" "os/user" "path/filepath" @@ -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... @@ -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 +} diff --git a/pkg/util/path_test.go b/pkg/util/path_test.go index 282e1299fd..ca7088d583 100644 --- a/pkg/util/path_test.go +++ b/pkg/util/path_test.go @@ -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()) @@ -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"}, + }), +) diff --git a/test/e2e/build/complex_test.go b/test/e2e/build/complex_test.go index 30a88bd890..b58d2b548e 100644 --- a/test/e2e/build/complex_test.go +++ b/test/e2e/build/complex_test.go @@ -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 'Hello, world' /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", @@ -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 'Hello, world' /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",