diff --git a/internal/controller/storage.go b/internal/controller/storage.go index ef1ac7978..15fe93b36 100644 --- a/internal/controller/storage.go +++ b/internal/controller/storage.go @@ -48,10 +48,12 @@ import ( const GarbageCountLimit = 1000 const ( - // defaultFileMode is the permission mode applied to all files inside an artifact archive. + // defaultFileMode is the permission mode applied to files inside an artifact archive. defaultFileMode int64 = 0o644 // defaultDirMode is the permission mode applied to all directories inside an artifact archive. defaultDirMode int64 = 0o755 + // defaultExeFileMode is the permission mode applied to executable files inside an artifact archive. + defaultExeFileMode int64 = 0o744 ) // Storage manages artifacts @@ -424,6 +426,7 @@ func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFi if err != nil { return err } + // The name needs to be modified to maintain directory structure // as tar.FileInfoHeader only has access to the base name of the file. // Ref: https://golang.org/src/archive/tar/common.go?#L626 @@ -434,21 +437,7 @@ func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFi return err } } - header.Name = relFilePath - - // We want to remove any environment specific data as well, this - // ensures the checksum is purely content based. - header.Gid = 0 - header.Uid = 0 - header.Uname = "" - header.Gname = "" - header.ModTime = time.Time{} - header.AccessTime = time.Time{} - header.ChangeTime = time.Time{} - header.Mode = defaultFileMode - if fi.Mode().IsDir() { - header.Mode = defaultDirMode - } + sanitizeHeader(relFilePath, header) if err := tw.WriteHeader(header); err != nil { return err @@ -689,3 +678,42 @@ func (wc *writeCounter) Write(p []byte) (int, error) { wc.written += int64(n) return n, nil } + +// sanitizeHeader modifies the tar.Header to be relative to the root of the +// archive and removes any environment specific data. +func sanitizeHeader(relP string, h *tar.Header) { + // Modify the name to be relative to the root of the archive, + // this ensures we maintain the same structure when extracting. + h.Name = relP + + // We want to remove any environment specific data as well, this + // ensures the checksum is purely content based. + h.Gid = 0 + h.Uid = 0 + h.Uname = "" + h.Gname = "" + h.ModTime = time.Time{} + h.AccessTime = time.Time{} + h.ChangeTime = time.Time{} + + // Override the mode to be the default for the type of file. + setDefaultMode(h) +} + +// setDefaultMode sets the default mode for the given header. +func setDefaultMode(h *tar.Header) { + if h.FileInfo().IsDir() { + h.Mode = defaultDirMode + return + } + + if h.FileInfo().Mode().IsRegular() { + mode := h.FileInfo().Mode() + if mode&os.ModeType == 0 && mode&0o111 != 0 { + h.Mode = defaultExeFileMode + return + } + h.Mode = defaultFileMode + return + } +} diff --git a/internal/controller/storage_test.go b/internal/controller/storage_test.go index 4d624e9f5..8501093bb 100644 --- a/internal/controller/storage_test.go +++ b/internal/controller/storage_test.go @@ -109,9 +109,14 @@ func TestStorage_Archive(t *testing.T) { t.Fatalf("error while bootstrapping storage: %v", err) } - createFiles := func(files map[string][]byte) (dir string, err error) { + type dummyFile struct { + content []byte + mode int64 + } + + createFiles := func(files map[string]dummyFile) (dir string, err error) { dir = t.TempDir() - for name, b := range files { + for name, df := range files { absPath := filepath.Join(dir, name) if err = os.MkdirAll(filepath.Dir(absPath), 0o750); err != nil { return @@ -120,18 +125,24 @@ func TestStorage_Archive(t *testing.T) { if err != nil { return "", fmt.Errorf("could not create file %q: %w", absPath, err) } - if n, err := f.Write(b); err != nil { + if n, err := f.Write(df.content); err != nil { f.Close() return "", fmt.Errorf("could not write %d bytes to file %q: %w", n, f.Name(), err) } f.Close() + + if df.mode != 0 { + if err = os.Chmod(absPath, os.FileMode(df.mode)); err != nil { + return "", fmt.Errorf("could not chmod file %q: %w", absPath, err) + } + } } return } - matchFiles := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, files map[string][]byte, dirs []string) { + matchFiles := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, files map[string]dummyFile, dirs []string) { t.Helper() - for name, b := range files { + for name, df := range files { mustExist := !(name[0:1] == "!") if !mustExist { name = name[1:] @@ -140,7 +151,7 @@ func TestStorage_Archive(t *testing.T) { if err != nil { t.Fatalf("failed reading tarball: %v", err) } - if bs := int64(len(b)); s != bs { + if bs := int64(len(df.content)); s != bs { t.Fatalf("%q size %v != %v", name, s, bs) } if exist != mustExist { @@ -150,8 +161,12 @@ func TestStorage_Archive(t *testing.T) { t.Errorf("tarball contained excluded file %q", name) } } - if exist && m != defaultFileMode { - t.Fatalf("%q mode %v != %v", name, m, defaultFileMode) + expectMode := df.mode + if expectMode == 0 { + expectMode = defaultFileMode + } + if exist && m != expectMode { + t.Fatalf("%q mode %v != %v", name, m, expectMode) } } for _, name := range dirs { @@ -179,62 +194,62 @@ func TestStorage_Archive(t *testing.T) { tests := []struct { name string - files map[string][]byte + files map[string]dummyFile filter ArchiveFileFilter - want map[string][]byte + want map[string]dummyFile wantDirs []string wantErr bool }{ { name: "no filter", - files: map[string][]byte{ - ".git/config": nil, - "file.jpg": []byte(`contents`), - "manifest.yaml": nil, + files: map[string]dummyFile{ + ".git/config": {}, + "file.jpg": {content: []byte(`contents`)}, + "manifest.yaml": {}, }, filter: nil, - want: map[string][]byte{ - ".git/config": nil, - "file.jpg": []byte(`contents`), - "manifest.yaml": nil, + want: map[string]dummyFile{ + ".git/config": {}, + "file.jpg": {content: []byte(`contents`)}, + "manifest.yaml": {}, }, }, { name: "exclude VCS", - files: map[string][]byte{ - ".git/config": nil, - "manifest.yaml": nil, + files: map[string]dummyFile{ + ".git/config": {}, + "manifest.yaml": {}, }, wantDirs: []string{ "!.git", }, filter: SourceIgnoreFilter(nil, nil), - want: map[string][]byte{ - "!.git/config": nil, - "manifest.yaml": nil, + want: map[string]dummyFile{ + "!.git/config": {}, + "manifest.yaml": {}, }, }, { name: "custom", - files: map[string][]byte{ - ".git/config": nil, - "custom": nil, - "horse.jpg": nil, + files: map[string]dummyFile{ + ".git/config": {}, + "custom": {}, + "horse.jpg": {}, }, filter: SourceIgnoreFilter([]gitignore.Pattern{ gitignore.ParsePattern("custom", nil), }, nil), - want: map[string][]byte{ - "!git/config": nil, - "!custom": nil, - "horse.jpg": nil, + want: map[string]dummyFile{ + "!git/config": {}, + "!custom": {}, + "horse.jpg": {}, }, wantErr: false, }, { name: "including directories", - files: map[string][]byte{ - "test/.gitkeep": nil, + files: map[string]dummyFile{ + "test/.gitkeep": {}, }, filter: SourceIgnoreFilter([]gitignore.Pattern{ gitignore.ParsePattern("custom", nil), @@ -244,6 +259,26 @@ func TestStorage_Archive(t *testing.T) { }, wantErr: false, }, + { + name: "sets default file modes", + files: map[string]dummyFile{ + "test/file": { + mode: 0o666, + }, + "test/executable": { + mode: 0o777, + }, + }, + want: map[string]dummyFile{ + "test/file": { + mode: defaultFileMode, + }, + "test/executable": { + mode: defaultExeFileMode, + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {