storage: set `0o744` for files with exec mode set

This commit ensures that files with exec permissions set continue to be
executable by the user extracting the archive.

This is not of use to any of Flux itself, but does help downstream
dependents making use of the controller to facilitate artifact
acquisitions for their (CI/CD) software suite.

Co-authored-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Rashed Kamal <krashed@vmware.com>
This commit is contained in:
Rashed Kamal 2023-05-11 16:33:59 -04:00 committed by Hidde Beydals
parent 8d9b0f4645
commit 2736b748e6
No known key found for this signature in database
GPG Key ID: 979F380FC2341744
2 changed files with 113 additions and 50 deletions

View File

@ -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
}
}

View File

@ -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) {