From 764baa96a4d2593dd8a428df626880caa1fa0034 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Fri, 24 May 2024 12:23:46 -0400 Subject: [PATCH] WorkingDir inherit permissions of parent Prior to this PR, workingDirInit did not deal handle permissions of nested child directories in workingDirs. This PR provides the same permissions as that of the existing parent directory. This allows non-root users to also create relative sub diectories in a workspace if used as a workingDir. Fixes https://github.com/tektoncd/pipeline/issues/7804. --- cmd/workingdirinit/main.go | 85 +++++++++++++++++++++++++++++++-- pkg/pod/pod_test.go | 1 - pkg/pod/workingdir_init.go | 34 ++++++------- pkg/pod/workingdir_init_test.go | 21 ++++---- 4 files changed, 103 insertions(+), 38 deletions(-) diff --git a/cmd/workingdirinit/main.go b/cmd/workingdirinit/main.go index a131ff49b69..5c85281c54d 100644 --- a/cmd/workingdirinit/main.go +++ b/cmd/workingdirinit/main.go @@ -17,6 +17,8 @@ limitations under the License. package main import ( + "fmt" + "io/fs" "log" "os" "path/filepath" @@ -24,6 +26,77 @@ import ( "strings" ) +// exists returns whether the given file or directory exists +func exists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return false, err +} + +func getPermissions(path string) (fs.FileMode, error) { + fileInfo, err := os.Stat(path) + if err != nil { + return fs.FileMode(0o666), err + } + return fileInfo.Mode(), nil +} + +func createPath(path string) error { + // Check if the path already exists. + ex, err := exists(path) + if err != nil { + return fmt.Errorf("error accessing path %s: %w", path, err) + } + // Path already exists, so no subdirectory to create. Skip the rest. + if ex { + return nil + } + // Find the innermost parent directory to get the permissions to apply to the child directories. + parts := strings.Split(path, string(filepath.Separator)) + + // Walk in reverse order until we find the path of the innermost parent directory that exists. + for i := len(parts) - 1; i > 0; i-- { + dir := filepath.Join(string(filepath.Separator), filepath.Join(parts[:i]...)) + + // backtracked and reached the root "/". Means that this was not a mount. Let kubernetes handle it. + if dir == string(filepath.Separator) { + return nil + } + // Check if the immediate parent path exists + ex, err := exists(dir) + if err != nil { + return fmt.Errorf("error accessing path %s: %w", dir, err) + } + // Found the innermost parent directory that exists. + if ex { + // We need to get its permissions and apply it to the child directories. + perm, err := getPermissions(dir) + if err != nil { + return fmt.Errorf("error accessing path %s: %w", dir, err) + } + // Create the path + if err := os.MkdirAll(path, perm); err != nil { + return fmt.Errorf("failed to mkdir %q: %w", path, err) + } + // walk up again and set the permissions of the parent to the child directories + d := dir + for j := i; j < len(parts); j++ { + d = filepath.Join(d, parts[j]) + if err := os.Chmod(d, perm); err != nil { + return fmt.Errorf("failed to chmod %q: %w", d, err) + } + } + return nil + } + } + return nil +} + func main() { for i, d := range os.Args { // os.Args[0] is the path to this executable, so we should skip it @@ -34,10 +107,14 @@ func main() { ws := cleanPath("/workspace/") p := cleanPath(d) - if !filepath.IsAbs(p) || strings.HasPrefix(p, ws+string(filepath.Separator)) { - if err := os.MkdirAll(p, 0755); err != nil { - log.Fatalf("Failed to mkdir %q: %v", p, err) - } + // Only handles relative path inside `/workspace` for backwards compatibility. + if !filepath.IsAbs(p) { + p = filepath.Join(ws, p) + } + + err := createPath(p) + if err != nil { + log.Fatalf("Failed to create path %s: %v", p, err) } } } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 2917333a4f1..87554e4cea6 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -473,7 +473,6 @@ func TestPodBuild(t *testing.T) { Image: images.WorkingDirInitImage, Command: []string{"/ko-app/workingdirinit"}, Args: []string{filepath.Join(pipeline.WorkspaceDir, "test")}, - WorkingDir: pipeline.WorkspaceDir, VolumeMounts: implicitVolumeMounts, }, }, diff --git a/pkg/pod/workingdir_init.go b/pkg/pod/workingdir_init.go index af001dc104d..0065d412ccb 100644 --- a/pkg/pod/workingdir_init.go +++ b/pkg/pod/workingdir_init.go @@ -18,11 +18,8 @@ package pod import ( "path/filepath" - "strings" - "github.com/tektoncd/pipeline/pkg/apis/pipeline" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" ) // workingDirInit returns a Container that should be run as an init @@ -37,26 +34,24 @@ import ( // so that the correct security context can be applied. func workingDirInit(workingdirinitImage string, stepContainers []corev1.Container, setSecurityContext, windows bool) *corev1.Container { // Gather all unique workingDirs. - workingDirs := sets.NewString() + workingDirs := []string{} + vms := implicitVolumeMounts + // To ensure that we don't duplicate volume mounts + requestedVolumeMounts := map[string]bool{} for _, step := range stepContainers { if step.WorkingDir != "" { - workingDirs.Insert(step.WorkingDir) + workingDirs = append(workingDirs, filepath.Clean(step.WorkingDir)) } - } - if workingDirs.Len() == 0 { - return nil - } - - // Clean and append each relative workingDir. - var relativeDirs []string - for _, wd := range workingDirs.List() { - p := filepath.Clean(wd) - if !filepath.IsAbs(p) || strings.HasPrefix(p, "/workspace/") { - relativeDirs = append(relativeDirs, p) + for _, vm := range step.VolumeMounts { + // Only add unique volume mounts from Steps + if !requestedVolumeMounts[filepath.Clean(vm.MountPath)] { + vms = append(vms, vm) + requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true + } } } - if len(relativeDirs) == 0 { + if len(workingDirs) == 0 { // There are no workingDirs to initialize. return nil } @@ -69,9 +64,8 @@ func workingDirInit(workingdirinitImage string, stepContainers []corev1.Containe Name: "working-dir-initializer", Image: workingdirinitImage, Command: []string{"/ko-app/workingdirinit"}, - Args: relativeDirs, - WorkingDir: pipeline.WorkspaceDir, - VolumeMounts: implicitVolumeMounts, + Args: workingDirs, + VolumeMounts: vms, } if setSecurityContext { c.SecurityContext = securityContext diff --git a/pkg/pod/workingdir_init_test.go b/pkg/pod/workingdir_init_test.go index 05f1f65f133..0df90337f07 100644 --- a/pkg/pod/workingdir_init_test.go +++ b/pkg/pod/workingdir_init_test.go @@ -20,7 +20,6 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" @@ -41,7 +40,7 @@ func TestWorkingDirInit(t *testing.T) { }}, want: nil, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored", + desc: "workingDirs are unique and sorted", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -61,12 +60,11 @@ func TestWorkingDirInit(t *testing.T) { Name: "working-dir-initializer", Image: images.WorkingDirInitImage, Command: []string{"/ko-app/workingdirinit"}, - Args: []string{"/workspace/bbb", "aaa", "zzz"}, - WorkingDir: pipeline.WorkspaceDir, + Args: []string{"zzz", "aaa", "/ignored", "/workspace", "zzz", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, + securitycontext", + desc: "workingDirs are unique and sorted + securitycontext", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -87,13 +85,12 @@ func TestWorkingDirInit(t *testing.T) { Name: "working-dir-initializer", Image: images.WorkingDirInitImage, Command: []string{"/ko-app/workingdirinit"}, - Args: []string{"/workspace/bbb", "aaa", "zzz"}, - WorkingDir: pipeline.WorkspaceDir, + Args: []string{"zzz", "aaa", "/ignored", "/workspace", "zzz", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, SecurityContext: linuxSecurityContext, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, uses windows", + desc: "workingDirs are unique and sorted, uses windows", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -114,12 +111,11 @@ func TestWorkingDirInit(t *testing.T) { Name: "working-dir-initializer", Image: images.WorkingDirInitImage, Command: []string{"/ko-app/workingdirinit"}, - Args: []string{"/workspace/bbb", "aaa", "zzz"}, - WorkingDir: pipeline.WorkspaceDir, + Args: []string{"zzz", "aaa", "/ignored", "/workspace", "zzz", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, uses windows, + securityContext", + desc: "workingDirs are unique and sorted, uses windows, + securityContext", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -141,8 +137,7 @@ func TestWorkingDirInit(t *testing.T) { Name: "working-dir-initializer", Image: images.WorkingDirInitImage, Command: []string{"/ko-app/workingdirinit"}, - Args: []string{"/workspace/bbb", "aaa", "zzz"}, - WorkingDir: pipeline.WorkspaceDir, + Args: []string{"zzz", "aaa", "/ignored", "/workspace", "zzz", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, SecurityContext: windowsSecurityContext, },