feat(storage): retain file permission and owner when adding via confi… (

#589) * feat(storage): retain file permission and owner when adding via configMap Signed-off-by: Smuu <[email protected]> * feat: fix file permission for sidecars Signed-off-by: Smuu <[email protected]> * fix: build dir now with error and cache Signed-off-by: Smuu <[email protected]> * feat: remove commented out code Signed-off-by: Smuu <[email protected]> * feat: add comments to if Signed-off-by: Smuu <[email protected]> * fix: error message Signed-off-by: Smuu <[email protected]> * feat: have default permission as const Signed-off-by: Smuu <[email protected]> * feat: move validation to proper function Signed-off-by: Smuu <[email protected]> * fix: merge conlfict Signed-off-by: Smuu <[email protected]> --------- Signed-off-by: Smuu <[email protected]>
celestiaorg · Dec 6, 2024 · 21d8b67 · 21d8b67
1 parent bf82cb3
commit 21d8b67
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 87 deletions.
diff --git a/pkg/instance/build.go b/pkg/instance/build.go
@@ -3,6 +3,7 @@ package instance
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"sync"
 
@@ -14,8 +15,6 @@ import (
 	"github.com/celestiaorg/knuu/pkg/container"
 )
 
-const buildDirBase = "/tmp/knuu"
-
 type build struct {
 	instance        *Instance
 	imageName       string
@@ -25,6 +24,7 @@ type build struct {
 	args            []string
 	env             map[string]string
 	imageCache      *sync.Map
+	buildDir        string
 }
 
 func (i *Instance) Build() *build {
@@ -53,10 +53,15 @@ func (b *build) SetImage(ctx context.Context, image string, args ...builder.ArgI
 		return ErrSettingImageNotAllowed.WithParams(b.instance.state.String())
 	}
 
+	buildDir, err := b.getBuildDir()
+	if err != nil {
+		return ErrGettingBuildDir.Wrap(err)
+	}
+
 	// Use the builder to build a new image
 	factory, err := container.NewBuilderFactory(container.BuilderFactoryOptions{
 		ImageName:    image,
-		BuildContext: b.getBuildDir(),
+		BuildContext: buildDir,
 		ImageBuilder: b.instance.ImageBuilder,
 		Args:         args,
 		Logger:       b.instance.Logger,
@@ -86,9 +91,14 @@ func (b *build) SetGitRepo(ctx context.Context, gitContext builder.GitContext, a
 		return ErrGettingImageName.Wrap(err)
 	}
 
+	buildDir, err := b.getBuildDir()
+	if err != nil {
+		return ErrGettingBuildDir.Wrap(err)
+	}
+
 	factory, err := container.NewBuilderFactory(container.BuilderFactoryOptions{
 		ImageName:    imageName,
-		BuildContext: b.getBuildDir(),
+		BuildContext: buildDir,
 		ImageBuilder: b.instance.ImageBuilder,
 		Args:         args,
 		Logger:       b.instance.Logger,
@@ -225,8 +235,16 @@ func getImageRegistry(imageName string) (string, error) {
 }
 
 // getBuildDir returns the build directory for the instance
-func (b *build) getBuildDir() string {
-	return filepath.Join(buildDirBase, b.instance.name)
+func (b *build) getBuildDir() (string, error) {
+	if b.buildDir != "" {
+		return b.buildDir, nil
+	}
+
+	tmpDir, err := os.MkdirTemp("", "knuu-build-*")
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(tmpDir, b.instance.name), nil
 }
 
 // addFileToBuilder adds a file to the builder

diff --git a/pkg/instance/errors.go b/pkg/instance/errors.go
@@ -83,6 +83,7 @@ var (
 	ErrSettingGitRepo                            = errors.New("SettingGitRepo", "setting git repo is only allowed in state 'None'. Current state is '%s'")
 	ErrGettingBuildContext                       = errors.New("GettingBuildContext", "error getting build context")
 	ErrGettingImageName                          = errors.New("GettingImageName", "error getting image name")
+	ErrGettingBuildDir                           = errors.New("GettingBuildDir", "error getting build directory")
 	ErrSettingImageNotAllowedForSidecars         = errors.New("SettingImageNotAllowedForSidecars", "setting image is not allowed for sidecars")
 	ErrSettingCommand                            = errors.New("SettingCommand", "setting command is only allowed in state 'Preparing' or 'Committed'. Current state is '%s")
 	ErrSettingArgsNotAllowed                     = errors.New("SettingArgsNotAllowed", "setting args is only allowed in state 'Preparing' or 'Committed'. Current state is '%s")
@@ -102,7 +103,9 @@ var (
 	ErrCreatingDirectory                         = errors.New("CreatingDirectory", "error creating directory")
 	ErrFailedToCreateDestFile                    = errors.New("FailedToCreateDestFile", "failed to create destination file '%s'")
 	ErrFailedToOpenSrcFile                       = errors.New("FailedToOpenSrcFile", "failed to open source file '%s'")
+	ErrFailedToGetSrcFileInfo                    = errors.New("FailedToGetSrcFileInfo", "failed to get source file info for %s")
 	ErrFailedToCopyFile                          = errors.New("FailedToCopyFile", "failed to copy from source '%s' to destination '%s'")
+	ErrFailedToSetPermissions                    = errors.New("FailedToSetPermissions", "failed to set permissions for destination file")
 	ErrSrcDoesNotExistOrIsDirectory              = errors.New("SrcDoesNotExistOrIsDirectory", "src '%s' does not exist or is a directory")
 	ErrInvalidFormat                             = errors.New("InvalidFormat", "invalid format")
 	ErrFailedToConvertToInt64                    = errors.New("FailedToConvertToInt64", "failed to convert to int64")

diff --git a/pkg/instance/execution.go b/pkg/instance/execution.go
@@ -415,7 +415,6 @@ func (e *execution) prepareReplicaSetConfig() k8s.ReplicaSetConfig {
 		Name:               e.instance.name,
 		Labels:             e.Labels(),
 		ServiceAccountName: e.instance.name,
-		FsGroup:            e.instance.storage.fsGroup,
 		ContainerConfig:    containerConfig,
 		SidecarConfigs:     sidecarConfigs,
 	}

diff --git a/pkg/instance/resources.go b/pkg/instance/resources.go
@@ -89,13 +89,12 @@ func (r *resources) deployStorage(ctx context.Context) error {
 			return ErrDeployingVolumeForInstance.WithParams(r.instance.name).Wrap(err)
 		}
 	}
-	if len(r.instance.storage.files) == 0 {
-		return nil
+	if len(r.instance.storage.files) != 0 {
+		if err := r.instance.storage.deployFiles(ctx); err != nil {
+			return ErrDeployingFilesForInstance.WithParams(r.instance.name).Wrap(err)
+		}
 	}
 
-	if err := r.instance.storage.deployFiles(ctx); err != nil {
-		return ErrDeployingFilesForInstance.WithParams(r.instance.name).Wrap(err)
-	}
 	return nil
 }
 
@@ -123,8 +122,7 @@ func (r *resources) destroyResources(ctx context.Context) error {
 	}
 
 	if len(r.instance.storage.files) != 0 {
-		err := r.instance.storage.destroyFiles(ctx)
-		if err != nil {
+		if err := r.instance.storage.destroyFiles(ctx); err != nil {
 			return ErrDestroyingFilesForInstance.WithParams(r.instance.name).Wrap(err)
 		}
 	}

diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go
@@ -22,9 +22,10 @@ type storage struct {
 	instance *Instance
 	volumes  []*k8s.Volume
 	files    []*k8s.File
-	fsGroup  int64
 }
 
+const defaultFilePermission = 0644
+
 func (i *Instance) Storage() *storage {
 	return i.storage
 }
@@ -43,7 +44,7 @@ func (s *storage) AddFile(src string, dest string, chown string) error {
 		return err
 	}
 
-	dstPath, err := s.copyFileToBuildDir(src, dest)
+	buildDirPath, err := s.copyFileToBuildDir(src, dest)
 	if err != nil {
 		return err
 	}
@@ -60,14 +61,18 @@ func (s *storage) AddFile(src string, dest string, chown string) error {
 		if srcInfo.Size() > maxTotalFilesBytes {
 			return ErrFileTooLargeCommitted.WithParams(src)
 		}
-		return s.addFileToInstance(dstPath, dest, chown)
+		return s.addFileToInstance(buildDirPath, dest, chown)
 	}
 
+	buildDir, err := s.instance.build.getBuildDir()
+	if err != nil {
+		return ErrGettingBuildDir.Wrap(err)
+	}
 	s.instance.Logger.WithFields(logrus.Fields{
 		"file":      dest,
 		"instance":  s.instance.name,
 		"state":     s.instance.state,
-		"build_dir": s.instance.build.getBuildDir(),
+		"build_dir": buildDir,
 	}).Debug("added file")
 	return nil
 }
@@ -101,7 +106,11 @@ func (s *storage) AddFolder(src string, dest string, chown string) error {
 			if err != nil {
 				return err
 			}
-			dstPath := filepath.Join(s.instance.build.getBuildDir(), dest, relPath)
+			buildDir, err := s.instance.build.getBuildDir()
+			if err != nil {
+				return ErrGettingBuildDir.Wrap(err)
+			}
+			dstPath := filepath.Join(buildDir, dest, relPath)
 
 			if info.IsDir() {
 				// create directory at destination path
@@ -115,11 +124,15 @@ func (s *storage) AddFolder(src string, dest string, chown string) error {
 		return ErrCopyingFolderToInstance.WithParams(src, s.instance.name).Wrap(err)
 	}
 
+	buildDir, err := s.instance.build.getBuildDir()
+	if err != nil {
+		return ErrGettingBuildDir.Wrap(err)
+	}
 	s.instance.Logger.WithFields(logrus.Fields{
 		"folder":    dest,
 		"instance":  s.instance.name,
 		"state":     s.instance.state,
-		"build_dir": s.instance.build.getBuildDir(),
+		"build_dir": buildDir,
 	}).Debug("added folder")
 	return nil
 }
@@ -140,6 +153,9 @@ func (s *storage) AddFileBytes(bytes []byte, dest string, chown string) error {
 	if _, err := tmpfile.Write(bytes); err != nil {
 		return err
 	}
+	if err := tmpfile.Chmod(defaultFilePermission); err != nil {
+		return err
+	}
 	if err := tmpfile.Close(); err != nil {
 		return err
 	}
@@ -249,40 +265,64 @@ func (s *storage) validateFileArgs(src, dest, chown string) error {
 	if !strings.Contains(chown, ":") || len(strings.Split(chown, ":")) != 2 {
 		return ErrChownMustBeInFormatUserGroup
 	}
+
+	parts := strings.Split(chown, ":")
+	for _, part := range parts {
+		if _, err := strconv.ParseInt(part, 10, 64); err != nil {
+			return ErrFailedToConvertToInt64.WithParams(part).Wrap(err)
+		}
+	}
 	return nil
 }
 
 func (s *storage) copyFileToBuildDir(src, dest string) (string, error) {
-	dstPath := filepath.Join(s.instance.build.getBuildDir(), dest)
+	buildDir, err := s.instance.build.getBuildDir()
+	if err != nil {
+		return "", ErrGettingBuildDir.Wrap(err)
+	}
+	dstPath := filepath.Join(buildDir, dest)
 	if err := os.MkdirAll(filepath.Dir(dstPath), os.ModePerm); err != nil {
 		return "", ErrCreatingDirectory.Wrap(err)
 	}
 
-	dst, err := os.Create(dstPath)
-	if err != nil {
-		return "", ErrFailedToCreateDestFile.WithParams(dstPath).Wrap(err)
-	}
-	defer dst.Close()
-
 	srcFile, err := os.Open(src)
 	if err != nil {
 		return "", ErrFailedToOpenSrcFile.WithParams(src).Wrap(err)
 	}
 	defer srcFile.Close()
 
+	srcInfo, err := srcFile.Stat()
+	if err != nil {
+		return "", ErrFailedToGetSrcFileInfo.WithParams(src).Wrap(err)
+	}
+
+	dst, err := os.OpenFile(dstPath, os.O_CREATE|os.O_WRONLY, srcInfo.Mode().Perm())
+	if err != nil {
+		return "", ErrFailedToCreateDestFile.WithParams(dstPath).Wrap(err)
+	}
+	defer dst.Close()
+
 	if _, err := io.Copy(dst, srcFile); err != nil {
 		return "", ErrFailedToCopyFile.WithParams(src, dstPath).Wrap(err)
 	}
 
+	// Ensure the destination file has the same permissions as the source file
+	if err := os.Chmod(dstPath, srcInfo.Mode().Perm()); err != nil {
+		return "", ErrFailedToSetPermissions.WithParams(dstPath).Wrap(err)
+	}
+
 	return dstPath, nil
 }
 
-func (s *storage) addFileToInstance(dstPath, dest, chown string) error {
-	srcInfo, err := os.Stat(dstPath)
+func (s *storage) addFileToInstance(srcPath, dest, chown string) error {
+	srcInfo, err := os.Stat(srcPath)
 	if os.IsNotExist(err) || srcInfo.IsDir() {
-		return ErrSrcDoesNotExistOrIsDirectory.WithParams(dstPath).Wrap(err)
+		return ErrSrcDoesNotExistOrIsDirectory.WithParams(srcPath).Wrap(err)
 	}
 
+	// get the permission of the src file
+	permission := fmt.Sprintf("%o", srcInfo.Mode().Perm())
+
 	size := int64(0)
 	for _, file := range s.files {
 		srcInfo, err := os.Stat(file.Source)
@@ -291,30 +331,17 @@ func (s *storage) addFileToInstance(dstPath, dest, chown string) error {
 		}
 		size += srcInfo.Size()
 	}
-	srcInfo, err = os.Stat(dstPath)
+	srcInfo, err = os.Stat(srcPath)
 	if err != nil {
 		return ErrFailedToGetFileSize.Wrap(err)
 	}
 	size += srcInfo.Size()
 	if size > maxTotalFilesBytes {
-		return ErrTotalFilesSizeTooLarge.WithParams(dstPath)
+		return ErrTotalFilesSizeTooLarge.WithParams(srcPath)
 	}
 
-	file := s.instance.K8sClient.NewFile(dstPath, dest)
-	parts := strings.Split(chown, ":")
-	if len(parts) != 2 {
-		return ErrInvalidFormat
-	}
+	file := s.instance.K8sClient.NewFile(srcPath, dest, chown, permission)
 
-	group, err := strconv.ParseInt(parts[1], 10, 64)
-	if err != nil {
-		return ErrFailedToConvertToInt64.Wrap(err)
-	}
-
-	if s.fsGroup != 0 && s.fsGroup != group {
-		return ErrAllFilesMustHaveSameGroup
-	}
-	s.fsGroup = group
 	s.files = append(s.files, file)
 	return nil
 }
@@ -468,6 +495,5 @@ func (s *storage) clone() *storage {
 		instance: nil,
 		volumes:  volumesCopy,
 		files:    filesCopy,
-		fsGroup:  s.fsGroup,
 	}
 }