Skip to content

Commit

Permalink
feedback changes
Browse files Browse the repository at this point in the history
  • Loading branch information
dbw7 committed Aug 26, 2024
1 parent 28e0c1a commit cf895fa
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 26 deletions.
4 changes: 2 additions & 2 deletions pkg/combustion/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ func copyCertificates(ctx *image.Context) error {
return fmt.Errorf("creating certificates directory '%s': %w", destDir, err)
}

if err := fileio.CopyFiles(srcDir, destDir, ".pem", false, true); err != nil {
if err := fileio.CopyFiles(srcDir, destDir, ".pem", false, nil); err != nil {
return fmt.Errorf("copying pem files: %w", err)
}

if err := fileio.CopyFiles(srcDir, destDir, ".crt", false, true); err != nil {
if err := fileio.CopyFiles(srcDir, destDir, ".crt", false, nil); err != nil {
return fmt.Errorf("copying certificates: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/combustion/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func (c *Combustion) configureManifests(ctx *image.Context) (string, error) {

if c.Registry != nil {
if c.Registry.ManifestsPath() != "" {
if err := fileio.CopyFiles(c.Registry.ManifestsPath(), manifestDestDir, "", false, false); err != nil {
if err := fileio.CopyFiles(c.Registry.ManifestsPath(), manifestDestDir, "", false, &fileio.NonExecutablePerms); err != nil {
return "", fmt.Errorf("copying manifests to combustion dir: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/combustion/osfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func copyOSFiles(ctx *image.Context) error {
return fmt.Errorf("no files found in directory %s", srcDirectory)
}

if err := fileio.CopyFiles(srcDirectory, destDirectory, "", true, true); err != nil {
if err := fileio.CopyFiles(srcDirectory, destDirectory, "", true, nil); err != nil {
return fmt.Errorf("copying os-files: %w", err)
}

Expand Down
16 changes: 9 additions & 7 deletions pkg/fileio/file_io.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import (
"go.uber.org/zap"
)

const (
var (
// ExecutablePerms are Linux permissions (rwxr--r--) for executable files (scripts, binaries, etc.)
ExecutablePerms os.FileMode = 0o744
ExecutablePerms = os.FileMode(0o744)
// NonExecutablePerms are Linux permissions (rw-r--r--) for non-executable files (configs, RPMs, etc.)
NonExecutablePerms os.FileMode = 0o644
NonExecutablePerms = os.FileMode(0o644)
)

func CopyFile(src string, dest string, perms os.FileMode) error {
Expand Down Expand Up @@ -77,7 +77,7 @@ func CopyFileN(src io.Reader, dest string, perms os.FileMode, n int64) error {
//
// If `copySubDir` is used with 'ext', iterates through all sub-directories
// and only copies files with the specified extension.
func CopyFiles(src, dest, ext string, copySubDir, maintainPerms bool) error {
func CopyFiles(src, dest, ext string, copySubDir bool, overridePerms *os.FileMode) error {
files, err := os.ReadDir(src)
if err != nil {
return fmt.Errorf("reading source dir: %w", err)
Expand All @@ -97,7 +97,7 @@ func CopyFiles(src, dest, ext string, copySubDir, maintainPerms bool) error {
continue
}

err = CopyFiles(sourcePath, destPath, ext, true, maintainPerms)
err = CopyFiles(sourcePath, destPath, ext, true, overridePerms)
if err != nil {
return fmt.Errorf("copying files from sub-directory '%s': %w", destPath, err)
}
Expand All @@ -107,8 +107,10 @@ func CopyFiles(src, dest, ext string, copySubDir, maintainPerms bool) error {
continue
}

perms := NonExecutablePerms
if maintainPerms {
var perms os.FileMode
if overridePerms != nil {
perms = *overridePerms
} else {
fileInfo, InfoErr := file.Info()
if err != nil {
return fmt.Errorf("reading file info: %w", InfoErr)
Expand Down
24 changes: 12 additions & 12 deletions pkg/fileio/file_io_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ func TestCopyFiles(t *testing.T) {
expectedRootDirFileNames []string
expectedSubDirFileNames []string
expectedPerms []os.FileMode
extentsion string
extension string
destDirPrefix string
copySubDir bool
keepPerms bool
perms *os.FileMode
}{
{
name: "Copy full directory filesystem",
Expand All @@ -164,39 +164,39 @@ func TestCopyFiles(t *testing.T) {
expectedPerms: []os.FileMode{NonExecutablePerms, 0o755},
destDirPrefix: "eib-copy-files-all-dirs-",
copySubDir: true,
keepPerms: false,
perms: &NonExecutablePerms,
},
{
name: "Copy full directory structure and files with specific extension",
expectedRootDirFileNames: []string{"rpm.rpm", "sub1-copy-files"},
expectedSubDirFileNames: []string{"rpm.rpm"},
expectedPerms: []os.FileMode{NonExecutablePerms, 0o755},
destDirPrefix: "eib-copy-files-ext-all-dirs-",
extentsion: ".rpm",
extension: ".rpm",
copySubDir: true,
keepPerms: false,
perms: &NonExecutablePerms,
},
{
name: "Copy all files only from the root directory",
expectedRootDirFileNames: []string{"gpg.gpg", "rpm.rpm", "unwritable.txt"},
expectedPerms: []os.FileMode{NonExecutablePerms},
destDirPrefix: "eib-copy-files-root-dir-only-",
keepPerms: false,
perms: &NonExecutablePerms,
},
{
name: "Copy files with specific extension only from the root directory",
expectedRootDirFileNames: []string{"rpm.rpm"},
expectedPerms: []os.FileMode{NonExecutablePerms},
destDirPrefix: "eib-copy-files-root-dir-only-",
extentsion: ".rpm",
keepPerms: false,
extension: ".rpm",
perms: &NonExecutablePerms,
},
{
name: "Copy files while maintaining their original permissions only from root directory",
expectedRootDirFileNames: []string{"gpg.gpg", "rpm.rpm", "unwritable.txt"},
expectedPerms: []os.FileMode{NonExecutablePerms, 0o444},
destDirPrefix: "eib-copy-files-keep-perms-root-dir-only-",
keepPerms: true,
perms: nil,
},
}

Expand All @@ -205,7 +205,7 @@ func TestCopyFiles(t *testing.T) {
rootDir, err := os.MkdirTemp("", test.destDirPrefix)
require.NoError(t, err)

err = CopyFiles(testDataPath, rootDir, test.extentsion, test.copySubDir, test.keepPerms)
err = CopyFiles(testDataPath, rootDir, test.extension, test.copySubDir, test.perms)
require.NoError(t, err)

if test.copySubDir {
Expand All @@ -224,7 +224,7 @@ func TestCopyFiles(t *testing.T) {
}

func TestCopyFilesMissingSource(t *testing.T) {
err := CopyFiles("", "", "", false, false)
err := CopyFiles("", "", "", false, &NonExecutablePerms)
assert.EqualError(t, err, "reading source dir: open : no such file or directory")
}

Expand All @@ -233,7 +233,7 @@ func TestCopyFilesMissingDestination(t *testing.T) {
require.NoError(t, err)
testDataPath := filepath.Join(pwd, "testdata", "copy-files")

err = CopyFiles(testDataPath, "", "", false, false)
err = CopyFiles(testDataPath, "", "", false, &NonExecutablePerms)
assert.EqualError(t, err, "creating directory '': mkdir : no such file or directory")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func storeManifests(ctx *image.Context, localManifestsDir string) (string, error
}

if _, err := os.Stat(localManifestsDir); err == nil {
if err = fileio.CopyFiles(localManifestsDir, manifestsDestDir, "", false, false); err != nil {
if err = fileio.CopyFiles(localManifestsDir, manifestsDestDir, "", false, &fileio.NonExecutablePerms); err != nil {
return "", fmt.Errorf("copying manifests: %w", err)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/rpm/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func (r *Resolver) prepare(localRPMConfig *image.LocalRPMConfig, packages *image

func (r *Resolver) prepareLocalRPMs(localRPMConfig *image.LocalRPMConfig) error {
rpmDest := r.generateRPMPathInBuildContext()
if err := fileio.CopyFiles(localRPMConfig.RPMPath, rpmDest, ".rpm", false, false); err != nil {
if err := fileio.CopyFiles(localRPMConfig.RPMPath, rpmDest, ".rpm", false, &fileio.NonExecutablePerms); err != nil {
return fmt.Errorf("copying local rpms to %s: %w", rpmDest, err)
}

Expand All @@ -163,7 +163,7 @@ func (r *Resolver) prepareLocalRPMs(localRPMConfig *image.LocalRPMConfig) error

if localRPMConfig.GPGKeysPath != "" {
gpgDest := r.generateGPGPathInBuildContext()
if err := fileio.CopyFiles(localRPMConfig.GPGKeysPath, gpgDest, "", false, false); err != nil {
if err := fileio.CopyFiles(localRPMConfig.GPGKeysPath, gpgDest, "", false, &fileio.NonExecutablePerms); err != nil {
return fmt.Errorf("copying local GPG keys to %s: %w", gpgDest, err)
}

Expand Down

0 comments on commit cf895fa

Please sign in to comment.