From 10d3aa3b9424041dd7081ca9cd30d6b7ba23df96 Mon Sep 17 00:00:00 2001 From: Joao Pereira Date: Tue, 18 Apr 2023 15:04:27 -0500 Subject: [PATCH] Add preserve-permission flag to push command Signed-off-by: Joao Pereira --- pkg/imgpkg/bundle/contents.go | 12 +-- pkg/imgpkg/bundle/contents_test.go | 4 +- pkg/imgpkg/bundle/locations_configs.go | 2 +- pkg/imgpkg/cmd/file_flags.go | 5 +- pkg/imgpkg/cmd/push.go | 6 +- pkg/imgpkg/image/dir_image.go | 5 ++ pkg/imgpkg/image/dir_image_test.go | 36 ++++++++ pkg/imgpkg/image/tar_image.go | 36 +++++--- pkg/imgpkg/image/tar_image_test.go | 15 +++- .../test_assets/img_tar_with_permissions.tar | Bin 0 -> 4096 bytes pkg/imgpkg/plainimage/contents.go | 11 +-- test/e2e/pull_test_unix_only_test.go | 82 ++++++++++++++++++ 12 files changed, 185 insertions(+), 29 deletions(-) create mode 100644 pkg/imgpkg/image/test_assets/img_tar_with_permissions.tar diff --git a/pkg/imgpkg/bundle/contents.go b/pkg/imgpkg/bundle/contents.go index 35497b46d..42df385fc 100644 --- a/pkg/imgpkg/bundle/contents.go +++ b/pkg/imgpkg/bundle/contents.go @@ -24,8 +24,9 @@ const ( ) type Contents struct { - paths []string - excludedPaths []string + paths []string + excludedPaths []string + preservePermissions bool } //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ImagesMetadataWriter @@ -36,8 +37,9 @@ type ImagesMetadataWriter interface { CloneWithLogger(logger util.ProgressLogger) registry.Registry } -func NewContents(paths []string, excludedPaths []string) Contents { - return Contents{paths: paths, excludedPaths: excludedPaths} +// NewContents creates Contents struct +func NewContents(paths []string, excludedPaths []string, preservePermissions bool) Contents { + return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions} } // Push the contents of the bundle to the registry as an OCI Image @@ -48,7 +50,7 @@ func (b Contents) Push(uploadRef regname.Tag, registry ImagesMetadataWriter, log } labels := map[string]string{BundleConfigLabel: "true"} - return plainimage.NewContents(b.paths, b.excludedPaths).Push(uploadRef, labels, registry, logger) + return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRef, labels, registry, logger) } // PresentsAsBundle checks if the provided folders have the needed structure to be a bundle diff --git a/pkg/imgpkg/bundle/contents_test.go b/pkg/imgpkg/bundle/contents_test.go index 3251a1762..3a5577c3a 100644 --- a/pkg/imgpkg/bundle/contents_test.go +++ b/pkg/imgpkg/bundle/contents_test.go @@ -38,7 +38,7 @@ images: fakeRegistry.ImageReturns(bundleImg, nil) t.Run("push is successful", func(t *testing.T) { - subject := bundle.NewContents([]string{bundleDir}, nil) + subject := bundle.NewContents([]string{bundleDir}, nil, false) imgTag, err := name.NewTag("my.registry.io/new-bundle:tag") if err != nil { t.Fatalf("failed to read tag: %s", err) @@ -72,7 +72,7 @@ images: fakeRegistry.ImageReturns(bundleImg, nil) t.Run("push is successful", func(t *testing.T) { - subject := bundle.NewContents([]string{bundleDir}, nil) + subject := bundle.NewContents([]string{bundleDir}, nil, false) imgTag, err := name.NewTag("my.registry.io/new-bundle:tag") if err != nil { t.Fatalf("failed to read tag: %s", err) diff --git a/pkg/imgpkg/bundle/locations_configs.go b/pkg/imgpkg/bundle/locations_configs.go index 94af2fa84..19da35157 100644 --- a/pkg/imgpkg/bundle/locations_configs.go +++ b/pkg/imgpkg/bundle/locations_configs.go @@ -133,7 +133,7 @@ func (r LocationsConfigs) Save(reg ImagesMetadataWriter, bundleRef name.Digest, r.ui.Tracef("Pushing image\n") - _, err = plainimage.NewContents([]string{tmpDir}, nil).Push(locRef, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger) + _, err = plainimage.NewContents([]string{tmpDir}, nil, false).Push(locRef, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger) if err != nil { // Immutable tag errors within registries are not standardized. // Assume word "immutable" would be present in most cases. diff --git a/pkg/imgpkg/cmd/file_flags.go b/pkg/imgpkg/cmd/file_flags.go index 30ae2769a..f5e1a2077 100644 --- a/pkg/imgpkg/cmd/file_flags.go +++ b/pkg/imgpkg/cmd/file_flags.go @@ -10,7 +10,8 @@ import ( type FileFlags struct { Files []string - ExcludedFilePaths []string + ExcludedFilePaths []string + PreservePermissions bool } func (f *FileFlags) Set(cmd *cobra.Command) { @@ -20,4 +21,6 @@ func (f *FileFlags) Set(cmd *cobra.Command) { cmd.Flags().MarkDeprecated("file-exclude-defaults", "use '--file-exclusion' instead") cmd.Flags().StringSliceVar(&f.ExcludedFilePaths, "file-exclusion", []string{".git"}, "Exclude file whose path, relative to the bundle root, matches (format: bar.yaml, nested-dir/baz.txt) (can be specified multiple times)") + + cmd.Flags().BoolVar(&f.PreservePermissions, "preserve-permissions", false, "Preserve the group and all permissions of all the files and folders") } diff --git a/pkg/imgpkg/cmd/push.go b/pkg/imgpkg/cmd/push.go index da1d5b268..96b381025 100644 --- a/pkg/imgpkg/cmd/push.go +++ b/pkg/imgpkg/cmd/push.go @@ -96,7 +96,7 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) { } logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths).Push(uploadRef, registry, logger) + imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, registry, logger) if err != nil { return "", err } @@ -132,7 +132,7 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { return "", fmt.Errorf("Parsing '%s': %s", po.ImageFlags.Image, err) } - isBundle, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths).PresentsAsBundle() + isBundle, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).PresentsAsBundle() if err != nil { return "", err } @@ -141,5 +141,5 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) { } logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui)) - return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths).Push(uploadRef, nil, registry, logger) + return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, nil, registry, logger) } diff --git a/pkg/imgpkg/image/dir_image.go b/pkg/imgpkg/image/dir_image.go index 955b94606..76d79019e 100644 --- a/pkg/imgpkg/image/dir_image.go +++ b/pkg/imgpkg/image/dir_image.go @@ -135,6 +135,11 @@ func (i *DirImage) extractTarEntry(header *tar.Header, input io.Reader) error { userPermission := int64(mode & 0700) permMode := os.FileMode(userPermission | userPermission>>3 | userPermission>>6) + // if group/all permission is set assume that the image was created to keep the full original permission set + if mode&0077 > 0 { + permMode = mode + } + err := os.MkdirAll(filepath.Dir(path), 0777) if err != nil { return err diff --git a/pkg/imgpkg/image/dir_image_test.go b/pkg/imgpkg/image/dir_image_test.go index 26f3804b3..ef7b957ed 100644 --- a/pkg/imgpkg/image/dir_image_test.go +++ b/pkg/imgpkg/image/dir_image_test.go @@ -5,9 +5,13 @@ package image_test import ( "fmt" + "io/fs" "os" + "path/filepath" + "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/image" "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util" @@ -61,4 +65,36 @@ func TestDirImage(t *testing.T) { }) } } + + t.Run("When extracting tar that contains files and folders with permissions", func(t *testing.T) { + img, err := image.NewFileImage(filepath.Join("test_assets", "img_tar_with_permissions.tar"), nil) + require.NoError(t, err) + folder, err := os.MkdirTemp("", "test") + require.NoError(t, err) + defer os.RemoveAll(folder) + + imgDir := image.NewDirImage(folder, img, util.NewNoopLogger()) + require.NoError(t, imgDir.AsDirectory()) + + // this is true using umask 022 that will remove the write bit from the permissions + expectedPermissions := map[string]string{ + "folder_all": "drwxr-xr-x", + "folder_all/exec_perm_all.sh": "-rwxr-xr-x", + "folder_all/some_file.txt": "-rw-r--r--", + "folder_group": "drwxr-xr-x", + "folder_group/exec_perm_group.sh": "-rwxr-xr-x", + "folder_group/some_other.txt": "-rw-r--r--", + } + filepath.WalkDir(folder, func(path string, d fs.DirEntry, _ error) error { + if path == folder { + return nil + } + key := strings.ReplaceAll(path, folder+string(filepath.Separator), "") + key = strings.ReplaceAll(key, "/", string(filepath.Separator)) + fInfo, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, expectedPermissions[key], fInfo.Mode().String(), fmt.Sprintf("validating file %s", key)) + return nil + }) + }) } diff --git a/pkg/imgpkg/image/tar_image.go b/pkg/imgpkg/image/tar_image.go index 8e1857cf8..94aa825b0 100644 --- a/pkg/imgpkg/image/tar_image.go +++ b/pkg/imgpkg/image/tar_image.go @@ -16,14 +16,15 @@ import ( ) type TarImage struct { - files []string - excludePaths []string - logger Logger + files []string + excludePaths []string + logger Logger + keepPermissions bool } // NewTarImage creates a struct that will allow users to create a representation of a set of paths as an OCI Image -func NewTarImage(files []string, excludePaths []string, logger Logger) *TarImage { - return &TarImage{files, excludePaths, logger} +func NewTarImage(files []string, excludePaths []string, logger Logger, keepPermissions bool) *TarImage { + return &TarImage{files, excludePaths, logger, keepPermissions} } // AsFileImage Creates an OCI Image representation of the provided folders @@ -80,7 +81,7 @@ func (i *TarImage) createTarball(file *os.File, filePaths []string) error { if i.isExcluded(relPath) { return filepath.SkipDir } - return i.addDirToTar(relPath, tarWriter) + return i.addDirToTar(path, relPath, tarWriter) } if (info.Mode() & os.ModeType) != 0 { return fmt.Errorf("Expected file '%s' to be a regular file", walkedPath) @@ -101,7 +102,7 @@ func (i *TarImage) createTarball(file *os.File, filePaths []string) error { return nil } -func (i *TarImage) addDirToTar(relPath string, tarWriter *tar.Writer) error { +func (i *TarImage) addDirToTar(fullPath string, relPath string, tarWriter *tar.Writer) error { if i.isExcluded(relPath) { panic("Unreachable") // directories excluded above } @@ -113,10 +114,19 @@ func (i *TarImage) addDirToTar(relPath string, tarWriter *tar.Writer) error { relPath = strings.ReplaceAll(relPath, "\\", "/") } + folderPermission := int64(0700) + if i.keepPermissions { + fInfo, err := os.Stat(fullPath) + if err != nil { + return fmt.Errorf("Unable to stat the folder '%s': %s", fullPath, err) + } + folderPermission = int64(fInfo.Mode()) + } + header := &tar.Header{ Name: relPath, - Mode: 0700, // static - ModTime: time.Time{}, // static + Mode: folderPermission, // static + ModTime: time.Time{}, // static Typeflag: tar.TypeDir, } @@ -141,12 +151,16 @@ func (i *TarImage) addFileToTar(fullPath, relPath string, info os.FileInfo, tarW if runtime.GOOS == "windows" { relPath = strings.ReplaceAll(relPath, "\\", "/") } + filePermission := int64(info.Mode() & 0700) + if i.keepPermissions { + filePermission = int64(info.Mode()) + } header := &tar.Header{ Name: relPath, Size: info.Size(), - Mode: int64(info.Mode() & 0700), // static - ModTime: time.Time{}, // static + Mode: filePermission, // static + ModTime: time.Time{}, // static Typeflag: tar.TypeReg, } diff --git a/pkg/imgpkg/image/tar_image_test.go b/pkg/imgpkg/image/tar_image_test.go index 394be4529..dedb0595c 100644 --- a/pkg/imgpkg/image/tar_image_test.go +++ b/pkg/imgpkg/image/tar_image_test.go @@ -14,7 +14,7 @@ import ( func TestTarImage(t *testing.T) { logger := testLogger{} t.Run("Ensure image tar as the same SHA", func(t *testing.T) { - tarImage := image.NewTarImage([]string{"test_assets/tar_folder"}, nil, logger) + tarImage := image.NewTarImage([]string{"test_assets/tar_folder"}, nil, logger, false) img, err := tarImage.AsFileImage(nil) require.NoError(t, err) d, err := img.Digest() @@ -25,6 +25,19 @@ func TestTarImage(t *testing.T) { require.Equal(t, "sha256:895251d345c46b3f9b6c2adb1443f39755187e5f314b23b78443a1bf0fa0cad2", d.String()) } }) + + t.Run("When keeping the files and folder permissions ensure image tar as the same SHA", func(t *testing.T) { + tarImage := image.NewTarImage([]string{"test_assets/tar_folder"}, nil, logger, true) + img, err := tarImage.AsFileImage(nil) + require.NoError(t, err) + d, err := img.Digest() + require.NoError(t, err) + if runtime.GOOS != "windows" { + require.Equal(t, "sha256:96bec7bb1b5585626f70f0084e942b8546cf4cf7412fc36dbd1633590d040f5b", d.String()) + } else { + require.Equal(t, "sha256:895251d345c46b3f9b6c2adb1443f39755187e5f314b23b78443a1bf0fa0cad2", d.String()) + } + }) } type testLogger struct{} diff --git a/pkg/imgpkg/image/test_assets/img_tar_with_permissions.tar b/pkg/imgpkg/image/test_assets/img_tar_with_permissions.tar new file mode 100644 index 0000000000000000000000000000000000000000..84df4a456eb08f4e4d19fe0540a9ffb31c091a4a GIT binary patch literal 4096 zcmeHJZEnLL49!tEL1z=d$BCj1)=sIYNGkRCGwHV~bsprTr1^^I|s(7WjX|n4I%^f6aRw@7*9z$I-eW(si9`E|Z9Mqocd32c zzRmeq2)D1-$>kmSkv!Dvf&aM@?)>|c|7;k7ef}FZ{dTc{84qGw3-~8;%s(s^!JCVO z|EvD{xc5!ib$&CPhBt^6Z!!PQob~>v{;fvVKh