Skip to content

Commit 623ef86

Browse files
committed
fix(buildah): add support for git owner/group settings
Enable e2e test check for owner/group. Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
1 parent 414dd38 commit 623ef86

File tree

8 files changed

+140
-30
lines changed

8 files changed

+140
-30
lines changed

pkg/build/stage/git_mapping.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,10 @@ func (gm *GitMapping) PreparePatchForImage(ctx context.Context, c Conveyor, cb c
636636
}()
637637

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

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

702-
stageImage.Builder.StapelStageBuilder().AddDataArchive(f, archiveType, gm.To)
705+
stageImage.Builder.StapelStageBuilder().AddDataArchive(f, archiveType, gm.To, container_backend.AddDataArchiveOptions{
706+
Owner: gm.Owner,
707+
Group: gm.Group,
708+
})
703709
}
704710

705711
gm.AddGitCommitToImageLabels(ctx, c, cb, stageImage, commitInfo)

pkg/buildah/base.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (b *BaseBuildah) prepareBuildFromDockerfile(dockerfile []byte, contextTar i
9393
}
9494

9595
if contextTar != nil {
96-
if err := util.ExtractTar(contextTar, contextTmpDir); err != nil {
96+
if err := util.ExtractTar(contextTar, contextTmpDir, util.ExtractTarOptions{}); err != nil {
9797
return "", "", "", fmt.Errorf("unable to extract context tar to tmp context dir: %w", err)
9898
}
9999
}

pkg/container_backend/build_stapel_stage_options.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import (
55
"io"
66
)
77

8+
type AddDataArchiveOptions struct {
9+
Owner, Group string
10+
}
11+
812
type BuildStapelStageOptionsInterface interface {
913
SetBaseImage(baseImage string) BuildStapelStageOptionsInterface
1014

@@ -21,7 +25,7 @@ type BuildStapelStageOptionsInterface interface {
2125
AddBuildVolumes(volumes ...string) BuildStapelStageOptionsInterface
2226
AddCommands(commands ...string) BuildStapelStageOptionsInterface
2327

24-
AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string) BuildStapelStageOptionsInterface
28+
AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string, o AddDataArchiveOptions) BuildStapelStageOptionsInterface
2529
RemoveData(removeType RemoveType, paths, keepParentDirs []string) BuildStapelStageOptionsInterface
2630
AddDependencyImport(imageName, fromPath, toPath string, includePaths, excludePaths []string, owner, group string) BuildStapelStageOptionsInterface
2731
}
@@ -56,9 +60,10 @@ const (
5660
)
5761

5862
type DataArchiveSpec struct {
59-
Archive io.ReadCloser
60-
Type ArchiveType
61-
To string
63+
Archive io.ReadCloser
64+
Type ArchiveType
65+
To string
66+
Owner, Group string
6267
}
6368

6469
type RemoveType int
@@ -155,11 +160,13 @@ func (opts *BuildStapelStageOptions) AddCommands(commands ...string) BuildStapel
155160
return opts
156161
}
157162

158-
func (opts *BuildStapelStageOptions) AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string) BuildStapelStageOptionsInterface {
163+
func (opts *BuildStapelStageOptions) AddDataArchive(archive io.ReadCloser, archiveType ArchiveType, to string, o AddDataArchiveOptions) BuildStapelStageOptionsInterface {
159164
opts.DataArchiveSpecs = append(opts.DataArchiveSpecs, DataArchiveSpec{
160165
Archive: archive,
161166
Type: archiveType,
162167
To: to,
168+
Owner: o.Owner,
169+
Group: o.Group,
163170
})
164171
return opts
165172
}

pkg/container_backend/buildah_backend.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,12 +278,19 @@ func (runtime *BuildahBackend) applyDataArchives(ctx context.Context, container
278278
return fmt.Errorf("unknown archive type %q", archive.Type)
279279
}
280280

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

283288
logboek.Context(ctx).Debug().LogF("Extracting archive into container path %s\n", archive.To)
284-
if err := util.ExtractTar(archive.Archive, extractDestPath); err != nil {
289+
290+
if err := util.ExtractTar(archive.Archive, extractDestPath, util.ExtractTarOptions{UID: uid, GID: gid}); err != nil {
285291
return fmt.Errorf("unable to extract data archive into %s: %w", archive.To, err)
286292
}
293+
287294
if err := archive.Archive.Close(); err != nil {
288295
return fmt.Errorf("error closing archive data stream: %w", err)
289296
}

pkg/util/archive.go

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,18 @@ ArchiveCopying:
208208
return nil
209209
}
210210

211-
func ExtractTar(tarFileReader io.Reader, dstDir string) error {
211+
type ExtractTarOptions struct {
212+
UID, GID *uint32
213+
}
214+
215+
func ExtractTar(tarFileReader io.Reader, dstDir string, opts ExtractTarOptions) error {
216+
if err := os.MkdirAll(dstDir, os.ModePerm); err != nil {
217+
return fmt.Errorf("unable to create dir %q: %w", dstDir, err)
218+
}
219+
if err := Chown(dstDir, opts.UID, opts.GID); err != nil {
220+
return fmt.Errorf("unable to chown dir %q: %w", dstDir, err)
221+
}
222+
212223
tarReader := tar.NewReader(tarFileReader)
213224
for {
214225
tarEntryHeader, err := tarReader.Next()
@@ -227,6 +238,7 @@ func ExtractTar(tarFileReader io.Reader, dstDir string) error {
227238
if err = os.MkdirAll(tarEntryPath, tarEntryFileInfo.Mode()); err != nil {
228239
return fmt.Errorf("unable to create new dir %q while extracting tar: %w", tarEntryPath, err)
229240
}
241+
230242
case tar.TypeReg, tar.TypeSymlink, tar.TypeLink, tar.TypeGNULongName, tar.TypeGNULongLink:
231243
if err := os.MkdirAll(filepath.Dir(tarEntryPath), os.ModePerm); err != nil {
232244
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 {
247259
return fmt.Errorf("unable to close file %q while extracting tar: %w", tarEntryPath, err)
248260
}
249261

250-
// TODO: set uid/gid from function options
251-
// if err := os.Chown(tarEntryPath, tarEntryHeader.Uid, tarEntryHeader.Gid); err != nil {
252-
// return fmt.Errorf("unable to set owner and group to %d:%d for file %q: %w", tarEntryHeader.Uid, tarEntryHeader.Gid, tarEntryPath, err)
253-
// }
254-
255262
default:
256263
return fmt.Errorf("tar entry %q of unexpected type: %b", tarEntryHeader.Name, tarEntryHeader.Typeflag)
257264
}
265+
266+
for _, p := range FilepathsWithParents(tarEntryHeader.Name) {
267+
if err := Chown(filepath.Join(dstDir, p), opts.UID, opts.GID); err != nil {
268+
return fmt.Errorf("unable to chown file %q: %w", p, err)
269+
}
270+
}
271+
}
272+
273+
return nil
274+
}
275+
276+
func Chown(path string, uid, gid *uint32) error {
277+
if uid != nil || gid != nil {
278+
osUid := -1
279+
osGid := -1
280+
281+
if uid != nil {
282+
osUid = int(*uid)
283+
}
284+
if gid != nil {
285+
osGid = int(*gid)
286+
}
287+
288+
if err := os.Chown(path, osUid, osGid); err != nil {
289+
return fmt.Errorf("unable to set owner and group to %d:%d for file %q: %w", osUid, osGid, path, err)
290+
}
258291
}
259292

260293
return nil

pkg/util/path.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package util
22

33
import (
4+
"fmt"
45
"os"
56
"os/user"
67
"path/filepath"
@@ -37,8 +38,22 @@ func ExpandPath(path string) string {
3738

3839
func SplitFilepath(path string) (result []string) {
3940
path = filepath.FromSlash(path)
41+
path = filepath.Clean(path)
42+
43+
if filepath.IsAbs(path) {
44+
p, err := filepath.Rel(string(os.PathSeparator), path)
45+
if err != nil {
46+
panic(fmt.Sprintf("unable to get relative path for %q", path))
47+
}
48+
path = p
49+
}
50+
4051
separator := os.PathSeparator
4152

53+
if path == "." || path == string(separator) {
54+
return nil
55+
}
56+
4257
idx := 0
4358
if separator == '\\' {
4459
// if the separator is '\\', then we can just split...
@@ -117,3 +132,14 @@ func GlobPrefixWithoutPatterns(glob string) (string, string) {
117132

118133
return prefix, glob
119134
}
135+
136+
func FilepathsWithParents(path string) []string {
137+
var res []string
138+
base := ""
139+
for _, part := range SplitFilepath(path) {
140+
base = filepath.Join(base, part)
141+
res = append(res, base)
142+
}
143+
144+
return res
145+
}

pkg/util/path_test.go

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ import (
1212
"github.com/werf/werf/pkg/util"
1313
)
1414

15-
type entry struct {
15+
type expandPathTest struct {
1616
path string
1717
expectedPathFormat string
1818
}
1919

2020
var _ = DescribeTable("expand path",
21-
func(e entry) {
21+
func(e expandPathTest) {
2222
usr, err := user.Current()
2323
Ω(err).ShouldNot(HaveOccurred())
2424

@@ -28,24 +28,56 @@ var _ = DescribeTable("expand path",
2828
expectedPath := fmt.Sprintf(e.expectedPathFormat, usr.HomeDir, wd)
2929
Ω(util.ExpandPath(filepath.FromSlash(e.path))).Should(Equal(filepath.FromSlash(expectedPath)))
3030
},
31-
Entry("~", entry{
31+
Entry("~", expandPathTest{
3232
path: "~",
3333
expectedPathFormat: "%[1]s",
3434
}),
35-
Entry("~/", entry{
35+
Entry("~/", expandPathTest{
3636
path: "~/",
3737
expectedPathFormat: "%[1]s",
3838
}),
39-
Entry("~/path", entry{
39+
Entry("~/path", expandPathTest{
4040
path: "~/path",
4141
expectedPathFormat: "%[1]s/path",
4242
}),
43-
Entry("path", entry{
43+
Entry("path", expandPathTest{
4444
path: "path",
4545
expectedPathFormat: "%[2]s/path",
4646
}),
47-
Entry("path1/../path2", entry{
47+
Entry("path1/../path2", expandPathTest{
4848
path: "path1/../path2",
4949
expectedPathFormat: "%[2]s/path2",
5050
}),
5151
)
52+
53+
type splitPathTest struct {
54+
path string
55+
expectedPathParts []string
56+
}
57+
58+
var _ = DescribeTable("split path",
59+
func(t splitPathTest) {
60+
parts := util.SplitFilepath(t.path)
61+
Expect(parts).To(Equal(t.expectedPathParts))
62+
},
63+
Entry("root path", splitPathTest{
64+
path: "/",
65+
expectedPathParts: nil,
66+
}),
67+
Entry("unnormalized root path", splitPathTest{
68+
path: "////",
69+
expectedPathParts: nil,
70+
}),
71+
Entry("empty path", splitPathTest{
72+
path: "",
73+
expectedPathParts: nil,
74+
}),
75+
Entry("absolute path", splitPathTest{
76+
path: "/path/to/dir/or/file",
77+
expectedPathParts: []string{"path", "to", "dir", "or", "file"},
78+
}),
79+
Entry("relative path", splitPathTest{
80+
path: "path/to/dir/or/file",
81+
expectedPathParts: []string{"path", "to", "dir", "or", "file"},
82+
}),
83+
)

test/e2e/build/complex_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,15 @@ var _ = Describe("Complex build", Label("e2e", "build", "complex"), func() {
9898
contRuntime.ExpectCmdsToSucceed(
9999
buildReport.Images["stapel-shell"].DockerImageName,
100100
"test -f /app/README.md",
101-
// TODO: fix buildah: support setting git.owner/group for git-archives
102-
// "stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
101+
"stat -c %u:%g /app/README.md | diff <(echo 1050:1051) -",
103102
"grep -qF 'https://cloud.google.com/appengine/docs/go/#Go_tools' /app/README.md",
104103

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

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

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

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

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

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

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

0 commit comments

Comments
 (0)