Add preserve-permission flag to push command

Signed-off-by: Joao Pereira <joaod@vmware.com>

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

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)

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.

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")
 }

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

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

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
+		})
+	})
 }

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

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

pkg/imgpkg/plainimage/contents.go

@@ -18,8 +18,9 @@ import (
 
 // Contents of the OCI Image
 type Contents struct {
-	paths         []string
-	excludedPaths []string
+	paths               []string
+	excludedPaths       []string
+	preservePermissions bool
 }
 
 // ImagesWriter defines the needed functions to write to the registry
@@ -29,8 +30,8 @@ type ImagesWriter interface {
 }
 
 // NewContents creates the struct that represent an OCI Image based on the provided paths
-func NewContents(paths []string, excludedPaths []string) Contents {
-	return Contents{paths: paths, excludedPaths: excludedPaths}
+func NewContents(paths []string, excludedPaths []string, preservePermissions bool) Contents {
+	return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions}
 }
 
 // Push the OCI Image to the registry
@@ -40,7 +41,7 @@ func (i Contents) Push(uploadRef regname.Tag, labels map[string]string, writer I
 		return "", err
 	}
 
-	tarImg := ctlimg.NewTarImage(i.paths, i.excludedPaths, logger)
+	tarImg := ctlimg.NewTarImage(i.paths, i.excludedPaths, logger, i.preservePermissions)
 
 	img, err := tarImg.AsFileImage(labels)
 	if err != nil {

test/e2e/pull_test_unix_only_test.go

@@ -69,6 +69,50 @@ func TestPull(t *testing.T) {
 		assert.Equal(t, os.FileMode(0006).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
 	})
 
+	t.Run("Image - when --preserve-permissions flag is provided it keeps the original permissions on the files", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("Skipping test as this is a known issue: https://github.com/vmware-tanzu/carvel-imgpkg/issues/270")
+		}
+
+		folder := env.Assets.CreateTempFolder("simple-image")
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(folder, "all-on-user-only"), "some text", 0755)
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(folder, "read-on-user-only"), "some text", 0455)
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(folder, "read-write-on-user-only"), "some text", 0655)
+
+		out := imgpkg.Run([]string{"push", "--tty", "-i", env.Image, "-f", folder, "--preserve-permissions=true"})
+		imgDigest := fmt.Sprintf("@%s", helpers.ExtractDigest(t, out))
+
+		pullDir := filepath.Join(env.Assets.CreateTempFolder("pull-dir-simple-image"), "pull-dir")
+		imageRef := fmt.Sprintf("%s%s", env.Image, imgDigest)
+
+		oldMask := unix.Umask(0)
+		defer unix.Umask(oldMask)
+
+		imgpkg.Run([]string{"pull", "-i", imageRef, "-o", pullDir})
+
+		logger.Section("ensures that pull folder is created with 0777 permissions", func() {
+			info, err := os.Stat(pullDir)
+			require.NoError(t, err)
+			assert.Equal(t, os.FileMode(0777).String(), (info.Mode() & 0777).String(), "outer folder permission should be set to 0777")
+		})
+
+		info, err := os.Stat(filepath.Join(pullDir, "all-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0700).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+		info, err = os.Stat(filepath.Join(pullDir, "read-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0400).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+		info, err = os.Stat(filepath.Join(pullDir, "read-write-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0600).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+	})
+
 	t.Run("Image - copies the User Permission to group and other but skips execution because umask is set to 0111", func(t *testing.T) {
 		if runtime.GOOS == "windows" {
 			t.Skip("Skipping test as this is a known issue: https://github.com/vmware-tanzu/carvel-imgpkg/issues/270")
@@ -165,4 +209,42 @@ func TestPull(t *testing.T) {
 		assert.Equal(t, os.FileMode(0060).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
 		assert.Equal(t, os.FileMode(0006).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
 	})
+
+	t.Run("Bundle - when --preserve-permissions flag is provided it keeps the original permissions on the files", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("Skipping test as this is a known issue: https://github.com/vmware-tanzu/carvel-imgpkg/issues/270")
+		}
+
+		bundleDir := env.BundleFactory.CreateBundleDir(helpers.BundleYAML, helpers.ImagesYAML)
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(bundleDir, "all-on-user-only"), "some text", 0755)
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(bundleDir, "read-on-user-only"), "some text", 0455)
+		env.Assets.AddFileToFolderWithPermissions(filepath.Join(bundleDir, "read-write-on-user-only"), "some text", 0655)
+
+		out := imgpkg.Run([]string{"push", "--tty", "-b", env.Image, "-f", bundleDir, "--preserve-permissions=true"})
+		imgDigest := fmt.Sprintf("@%s", helpers.ExtractDigest(t, out))
+
+		pullDir := env.Assets.CreateTempFolder("pull-dir-simple-image")
+		imageRef := fmt.Sprintf("%s%s", env.Image, imgDigest)
+
+		oldMask := unix.Umask(0)
+		defer unix.Umask(oldMask)
+
+		imgpkg.Run([]string{"pull", "-b", imageRef, "-o", pullDir})
+
+		info, err := os.Stat(filepath.Join(pullDir, "all-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0700).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+		info, err = os.Stat(filepath.Join(pullDir, "read-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0400).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+		info, err = os.Stat(filepath.Join(pullDir, "read-write-on-user-only"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0600).String(), (info.Mode() & 0700).String(), "user permission doesnt match")
+		assert.Equal(t, os.FileMode(0050).String(), (info.Mode() & 0070).String(), "group permission doesnt match")
+		assert.Equal(t, os.FileMode(0005).String(), (info.Mode() & 0007).String(), "other permission doesnt match")
+	})
 }