From 02ac7759507baf35f068ee56f25640238fff6cc7 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:19:45 +0100 Subject: [PATCH 1/9] feat(storage): retain file permission and owner when adding via configMap Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/build.go | 9 ++- pkg/instance/errors.go | 2 + pkg/instance/execution.go | 1 - pkg/instance/resources.go | 12 ++-- pkg/instance/storage.go | 53 +++++++++-------- pkg/k8s/pod.go | 119 ++++++++++++++++++++++++++------------ pkg/k8s/types.go | 2 +- 7 files changed, 126 insertions(+), 72 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index aa1bb54..e47f76e 100644 --- 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 @@ -226,7 +225,11 @@ 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) + tmpDir, err := os.MkdirTemp("", "knuu-build-*") + if err != nil { + return "" + } + return filepath.Join(tmpDir, b.instance.name) } // addFileToBuilder adds a file to the builder diff --git a/pkg/instance/errors.go b/pkg/instance/errors.go index a0487ec..31b594a 100644 --- a/pkg/instance/errors.go +++ b/pkg/instance/errors.go @@ -102,7 +102,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") 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 index f105834..4351e5b 100644 --- a/pkg/instance/execution.go +++ b/pkg/instance/execution.go @@ -411,7 +411,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 index 9497b1f..bbe99e4 100644 --- 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 index 4287a4d..d53cda7 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -20,7 +20,6 @@ type storage struct { instance *Instance volumes []*k8s.Volume files []*k8s.File - fsGroup int64 } func (i *Instance) Storage() *storage { @@ -41,7 +40,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 } @@ -51,7 +50,7 @@ func (s *storage) AddFile(src string, dest string, chown string) error { s.instance.build.addFileToBuilder(src, dest, chown) return nil case StateCommitted, StateStopped: - return s.addFileToInstance(dstPath, dest, chown) + return s.addFileToInstance(buildDirPath, dest, chown) } s.instance.Logger.WithFields(logrus.Fields{ @@ -249,46 +248,55 @@ func (s *storage) copyFileToBuildDir(src, dest string) (string, error) { 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) } - file := s.instance.K8sClient.NewFile(dstPath, dest) + // get the permission of the src file + permission := fmt.Sprintf("%o", srcInfo.Mode().Perm()) + parts := strings.Split(chown, ":") if len(parts) != 2 { - return ErrInvalidFormat + return ErrInvalidFormat.WithParams(chown) } - - group, err := strconv.ParseInt(parts[1], 10, 64) - if err != nil { - return ErrFailedToConvertToInt64.Wrap(err) + for _, part := range parts { + if _, err := strconv.ParseInt(part, 10, 64); err != nil { + return ErrFailedToConvertToInt64.WithParams(part).Wrap(err) + } } + file := s.instance.K8sClient.NewFile(srcPath, dest, chown, permission) - if s.fsGroup != 0 && s.fsGroup != group { - return ErrAllFilesMustHaveSameGroup - } - s.fsGroup = group s.files = append(s.files, file) return nil } @@ -439,6 +447,5 @@ func (s *storage) clone() *storage { instance: nil, volumes: volumesCopy, files: filesCopy, - fsGroup: s.fsGroup, } } diff --git a/pkg/k8s/pod.go b/pkg/k8s/pod.go index 0127d5d..be8bd03 100644 --- a/pkg/k8s/pod.go +++ b/pkg/k8s/pod.go @@ -26,9 +26,6 @@ const ( // knuuPath is the path where the knuu volume is mounted knuuPath = "/knuu" - // 0777 is used so that the files are usable by any user in the container without needing to change permissions - defaultFileModeForVolume = 0777 - podFilesConfigmapNameSuffix = "-config" initContainerNameSuffix = "-init" @@ -71,8 +68,10 @@ type Volume struct { } type File struct { - Source string - Dest string + Source string + Dest string + Chown string + Permission string } // DeployPod creates a new pod in the namespace that k8s client is initiate with if it doesn't already exist. @@ -101,10 +100,12 @@ func (c *Client) NewVolume(path string, size resource.Quantity, owner int64) *Vo } } -func (c *Client) NewFile(source, dest string) *File { +func (c *Client) NewFile(source, dest, chown, permission string) *File { return &File{ - Source: source, - Dest: dest, + Source: source, + Dest: dest, + Chown: chown, + Permission: permission, } } @@ -381,6 +382,16 @@ func buildPodVolumes(name string, volumesAmount, filesAmount int) []v1.Volume { podVolumes = append(podVolumes, podVolume) } + if volumesAmount == 0 && filesAmount != 0 { + podVolume := v1.Volume{ + Name: name, + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + } + podVolumes = append(podVolumes, podVolume) + } + if filesAmount != 0 { podFiles := v1.Volume{ Name: name + podFilesConfigmapNameSuffix, @@ -389,7 +400,7 @@ func buildPodVolumes(name string, volumesAmount, filesAmount int) []v1.Volume { LocalObjectReference: v1.LocalObjectReference{ Name: name, }, - DefaultMode: ptr.To[int32](defaultFileModeForVolume), + DefaultMode: ptr.To[int32](0600), }, }, } @@ -414,25 +425,39 @@ func buildContainerVolumes(name string, volumes []*Volume, files []*File) []v1.V ) } - var containerFiles []v1.VolumeMount - - for n, file := range files { - shouldAddFile := true - for _, volume := range volumes { - if strings.HasPrefix(file.Dest, volume.Path) { - shouldAddFile = false - break - } + if len(volumes) == 0 && len(files) != 0 { + uniquePaths := make(map[string]bool) + for _, file := range files { + uniquePaths[filepath.Dir(file.Dest)] = true } - if shouldAddFile { - containerFiles = append(containerFiles, v1.VolumeMount{ - Name: name + podFilesConfigmapNameSuffix, - MountPath: file.Dest, - SubPath: fmt.Sprintf("%d", n), + for path := range uniquePaths { + containerVolumes = append(containerVolumes, v1.VolumeMount{ + Name: name, + MountPath: path, + SubPath: strings.TrimPrefix(path, "/"), }) } } + var containerFiles []v1.VolumeMount + + // for n, file := range files { + // shouldAddFile := true + // for _, volume := range volumes { + // if strings.HasPrefix(file.Dest, volume.Path) { + // shouldAddFile = false + // break + // } + // } + // if shouldAddFile { + // containerFiles = append(containerFiles, v1.VolumeMount{ + // Name: name + podFilesConfigmapNameSuffix, + // MountPath: file.Dest, + // SubPath: fmt.Sprintf("%d", n), + // }) + // } + // } + return append(containerVolumes, containerFiles...) } @@ -442,11 +467,25 @@ func buildInitContainerVolumes(name string, volumes []*Volume, files []*File) [] return []v1.VolumeMount{} // return empty slice if no volumes are specified } - containerVolumes := []v1.VolumeMount{ - { + var containerVolumes []v1.VolumeMount + if len(volumes) != 0 { + containerVolumes = append(containerVolumes, v1.VolumeMount{ Name: name, - MountPath: knuuPath, // set the path to "/knuu" as per the requirements - }, + MountPath: knuuPath, + }) + } + if len(volumes) == 0 && len(files) != 0 { + uniquePaths := make(map[string]bool) + for _, file := range files { + uniquePaths[filepath.Dir(file.Dest)] = true + } + for path := range uniquePaths { + containerVolumes = append(containerVolumes, v1.VolumeMount{ + Name: name, + MountPath: knuuPath + path, + SubPath: strings.TrimPrefix(path, "/"), + }) + } } var containerFiles []v1.VolumeMount @@ -483,26 +522,33 @@ func (c *Client) buildInitContainerCommand(volumes []*Volume, files []*File) []s cmds = append(cmds, parentDirCmd) dirsProcessed[folder] = true } - copyFileToKnuu := fmt.Sprintf("cp %s %s && ", file.Dest, filepath.Join(knuuPath, file.Dest)) - cmds = append(cmds, copyFileToKnuu) + chown := file.Chown + permission := file.Permission + addFileToKnuu := fmt.Sprintf("cp %s %s && ", file.Dest, filepath.Join(knuuPath, file.Dest)) + if chown != "" { + addFileToKnuu += fmt.Sprintf("chown %s %s && ", chown, filepath.Join(knuuPath, file.Dest)) + } + if permission != "" { + addFileToKnuu += fmt.Sprintf("chmod %s %s && ", permission, filepath.Join(knuuPath, file.Dest)) + } + cmds = append(cmds, addFileToKnuu) } // for each volume, copy the contents of the volume to the knuu volume - for i, volume := range volumes { + for _, volume := range volumes { knuuVolumePath := fmt.Sprintf("%s%s", knuuPath, volume.Path) cmd := fmt.Sprintf("if [ -d %s ] && [ \"$(ls -A %s)\" ]; then mkdir -p %s && cp -r %s/* %s && chown -R %d:%d %s", volume.Path, volume.Path, knuuVolumePath, volume.Path, knuuVolumePath, volume.Owner, volume.Owner, knuuVolumePath) - if i < len(volumes)-1 { - cmd += " ;fi && " - } else { - cmd += " ;fi" - } + cmd += " ;fi && " cmds = append(cmds, cmd) } fullCommand := strings.Join(cmds, "") commands = append(commands, fullCommand) + if strings.HasSuffix(fullCommand, " && ") { + commands[len(commands)-1] = strings.TrimSuffix(commands[len(commands)-1], " && ") + } c.logger.WithField("command", fullCommand).Debug("init container command") return commands @@ -541,7 +587,7 @@ func prepareContainer(config ContainerConfig) v1.Container { // prepareInitContainers creates a slice of v1.Container as init containers. func (c *Client) prepareInitContainers(config ContainerConfig, init bool) []v1.Container { - if !init || len(config.Volumes) == 0 { + if !init || (len(config.Volumes) == 0 && len(config.Files) == 0) { return nil } @@ -566,7 +612,6 @@ func preparePodVolumes(config ContainerConfig) []v1.Volume { func (c *Client) preparePodSpec(spec PodConfig, init bool) v1.PodSpec { podSpec := v1.PodSpec{ ServiceAccountName: spec.ServiceAccountName, - SecurityContext: &v1.PodSecurityContext{FSGroup: &spec.FsGroup}, InitContainers: c.prepareInitContainers(spec.ContainerConfig, init), Containers: []v1.Container{prepareContainer(spec.ContainerConfig)}, Volumes: preparePodVolumes(spec.ContainerConfig), diff --git a/pkg/k8s/types.go b/pkg/k8s/types.go index 6e9cb0a..b42baf0 100644 --- a/pkg/k8s/types.go +++ b/pkg/k8s/types.go @@ -65,7 +65,7 @@ type KubeManager interface { Namespace() string NamespaceExists(ctx context.Context, name string) (bool, error) NetworkPolicyExists(ctx context.Context, name string) bool - NewFile(source, dest string) *File + NewFile(source, dest, chown, permission string) *File NewVolume(path string, size resource.Quantity, owner int64) *Volume PatchService(ctx context.Context, name string, opts ServiceOptions) (*corev1.Service, error) PortForwardPod(ctx context.Context, podName string, localPort, remotePort int) error From a185a61225e94434acf284a1626c59c3cd3ee2c9 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:46:21 +0100 Subject: [PATCH 2/9] feat: fix file permission for sidecars Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/storage.go | 3 +++ pkg/k8s/pod.go | 5 ++++- pkg/sidecars/observability/obsy.go | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index d53cda7..e46f74c 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -130,6 +130,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(0644); err != nil { + return err + } if err := tmpfile.Close(); err != nil { return err } diff --git a/pkg/k8s/pod.go b/pkg/k8s/pod.go index be8bd03..7c145df 100644 --- a/pkg/k8s/pod.go +++ b/pkg/k8s/pod.go @@ -29,6 +29,7 @@ const ( podFilesConfigmapNameSuffix = "-config" initContainerNameSuffix = "-init" + initContainerImage = "nicolaka/netshoot" defaultContainerUser = 0 ) @@ -594,7 +595,7 @@ func (c *Client) prepareInitContainers(config ContainerConfig, init bool) []v1.C return []v1.Container{ { Name: config.Name + initContainerNameSuffix, - Image: config.Image, + Image: initContainerImage, SecurityContext: &v1.SecurityContext{ RunAsUser: ptr.To[int64](defaultContainerUser), }, @@ -619,9 +620,11 @@ func (c *Client) preparePodSpec(spec PodConfig, init bool) v1.PodSpec { // Prepare sidecar containers and append to the pod spec for _, sidecarConfig := range spec.SidecarConfigs { + sidecarInitContainer := c.prepareInitContainers(sidecarConfig, true) sidecarContainer := prepareContainer(sidecarConfig) sidecarVolumes := preparePodVolumes(sidecarConfig) + podSpec.InitContainers = append(podSpec.InitContainers, sidecarInitContainer...) podSpec.Containers = append(podSpec.Containers, sidecarContainer) podSpec.Volumes = append(podSpec.Volumes, sidecarVolumes...) } diff --git a/pkg/sidecars/observability/obsy.go b/pkg/sidecars/observability/obsy.go index 413bb86..f96f4dc 100644 --- a/pkg/sidecars/observability/obsy.go +++ b/pkg/sidecars/observability/obsy.go @@ -20,7 +20,7 @@ const ( otelAgentName = "otel-agent" // %s will be replaced with the otelCollectorVersion otelAgentConfigFile = "/etc/otel-agent.yaml" - otelAgentConfigFilePermissions = "0:0" + otelAgentConfigFilePermissions = "10001:10001" otelCollectorCommand = "/otelcol-contrib" otelCollectorConfigArg = "--config=/etc/otel-agent.yaml" From 10bedaf8ee2dc3e0d6cbd85f9130f4ba19f39b49 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:58:27 +0100 Subject: [PATCH 3/9] fix: build dir now with error and cache Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/build.go | 20 +++++++++++++++----- pkg/instance/errors.go | 1 + pkg/instance/storage.go | 24 ++++++++++++++++++++---- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index e47f76e..b608ec3 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -52,10 +52,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, @@ -85,9 +90,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, @@ -224,12 +234,12 @@ func getImageRegistry(imageName string) (string, error) { } // getBuildDir returns the build directory for the instance -func (b *build) getBuildDir() string { +func (b *build) getBuildDir() (string, error) { tmpDir, err := os.MkdirTemp("", "knuu-build-*") if err != nil { - return "" + return "", err } - return filepath.Join(tmpDir, b.instance.name) + 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 index 31b594a..978f962 100644 --- 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") diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index e46f74c..5cbab4f 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -53,11 +53,15 @@ func (s *storage) AddFile(src string, dest string, chown string) error { 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 } @@ -91,7 +95,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 @@ -105,11 +113,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 } @@ -246,7 +258,11 @@ func (s *storage) validateFileArgs(src, dest, chown string) error { } 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) } From 57302c1a40338fdbdd81dc81cf4154cced329264 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:04:39 +0100 Subject: [PATCH 4/9] feat: remove commented out code Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/build.go | 5 +++++ pkg/k8s/pod.go | 17 ----------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/pkg/instance/build.go b/pkg/instance/build.go index b608ec3..fecc13d 100644 --- a/pkg/instance/build.go +++ b/pkg/instance/build.go @@ -24,6 +24,7 @@ type build struct { args []string env map[string]string imageCache *sync.Map + buildDir string } func (i *Instance) Build() *build { @@ -235,6 +236,10 @@ func getImageRegistry(imageName string) (string, error) { // getBuildDir returns the build directory for the instance func (b *build) getBuildDir() (string, error) { + if b.buildDir != "" { + return b.buildDir, nil + } + tmpDir, err := os.MkdirTemp("", "knuu-build-*") if err != nil { return "", err diff --git a/pkg/k8s/pod.go b/pkg/k8s/pod.go index 7c145df..347ba7f 100644 --- a/pkg/k8s/pod.go +++ b/pkg/k8s/pod.go @@ -442,23 +442,6 @@ func buildContainerVolumes(name string, volumes []*Volume, files []*File) []v1.V var containerFiles []v1.VolumeMount - // for n, file := range files { - // shouldAddFile := true - // for _, volume := range volumes { - // if strings.HasPrefix(file.Dest, volume.Path) { - // shouldAddFile = false - // break - // } - // } - // if shouldAddFile { - // containerFiles = append(containerFiles, v1.VolumeMount{ - // Name: name + podFilesConfigmapNameSuffix, - // MountPath: file.Dest, - // SubPath: fmt.Sprintf("%d", n), - // }) - // } - // } - return append(containerVolumes, containerFiles...) } From 08c587ebe34f72819484a0c80df66a6165baaa90 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:08:06 +0100 Subject: [PATCH 5/9] feat: add comments to if Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/k8s/pod.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/k8s/pod.go b/pkg/k8s/pod.go index 347ba7f..46e82bb 100644 --- a/pkg/k8s/pod.go +++ b/pkg/k8s/pod.go @@ -452,12 +452,14 @@ func buildInitContainerVolumes(name string, volumes []*Volume, files []*File) [] } var containerVolumes []v1.VolumeMount + // if the user want do add volumes, we need to mount the knuu path if len(volumes) != 0 { containerVolumes = append(containerVolumes, v1.VolumeMount{ Name: name, MountPath: knuuPath, }) } + // if the user don't want to add volumes, but want to add files, we need to mount the knuu path for the init container if len(volumes) == 0 && len(files) != 0 { uniquePaths := make(map[string]bool) for _, file := range files { From 3137e58a1f75688b6d7f0bacd41cba1e4cec90c8 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:11:57 +0100 Subject: [PATCH 6/9] fix: error message Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instance/errors.go b/pkg/instance/errors.go index 978f962..14a4c6f 100644 --- a/pkg/instance/errors.go +++ b/pkg/instance/errors.go @@ -103,7 +103,7 @@ 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") + 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") From 34a7443782f68347cc7e267ec868a0ee94e8d5f5 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:18:22 +0100 Subject: [PATCH 7/9] feat: have default permission as const Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/storage.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 5cbab4f..86c5135 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -22,6 +22,8 @@ type storage struct { files []*k8s.File } +const defaultFilePermission = 0644 + func (i *Instance) Storage() *storage { return i.storage } @@ -142,7 +144,7 @@ func (s *storage) AddFileBytes(bytes []byte, dest string, chown string) error { if _, err := tmpfile.Write(bytes); err != nil { return err } - if err := tmpfile.Chmod(0644); err != nil { + if err := tmpfile.Chmod(defaultFilePermission); err != nil { return err } if err := tmpfile.Close(); err != nil { From bc44e4eee7626f30b48e25e796b0204f5889a4e4 Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 10:48:05 +0100 Subject: [PATCH 8/9] feat: move validation to proper function Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/storage.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 86c5135..bc27ed7 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -256,6 +256,13 @@ 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 } @@ -307,15 +314,6 @@ func (s *storage) addFileToInstance(srcPath, dest, chown string) error { // get the permission of the src file permission := fmt.Sprintf("%o", srcInfo.Mode().Perm()) - parts := strings.Split(chown, ":") - if len(parts) != 2 { - return ErrInvalidFormat.WithParams(chown) - } - for _, part := range parts { - if _, err := strconv.ParseInt(part, 10, 64); err != nil { - return ErrFailedToConvertToInt64.WithParams(part).Wrap(err) - } - } file := s.instance.K8sClient.NewFile(srcPath, dest, chown, permission) s.files = append(s.files, file) From 5d9bbb84944e4322cd2f0fae5f9425efae16e57f Mon Sep 17 00:00:00 2001 From: Smuu <18609909+Smuu@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:36:44 +0100 Subject: [PATCH 9/9] fix: merge conlfict Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com> --- pkg/instance/storage.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/pkg/instance/storage.go b/pkg/instance/storage.go index 930d68d..b2343ec 100644 --- a/pkg/instance/storage.go +++ b/pkg/instance/storage.go @@ -322,7 +322,7 @@ func (s *storage) addFileToInstance(srcPath, dest, chown string) error { // 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) @@ -331,19 +331,13 @@ func (s *storage) addFileToInstance(srcPath, 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) - } - - file := s.instance.K8sClient.NewFile(dstPath, dest) - parts := strings.Split(chown, ":") - if len(parts) != 2 { - return ErrInvalidFormat + return ErrTotalFilesSizeTooLarge.WithParams(srcPath) } file := s.instance.K8sClient.NewFile(srcPath, dest, chown, permission)