From 5993c980b3bc8bef8dc81a35043feb8956a247b6 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 | 78 ++++++++++++++++++++++++++++++--- pkg/pod/pod_test.go | 1 - pkg/pod/workingdir_init.go | 46 +++++++++++-------- pkg/pod/workingdir_init_test.go | 29 ++++++------ 4 files changed, 114 insertions(+), 40 deletions(-) diff --git a/cmd/workingdirinit/main.go b/cmd/workingdirinit/main.go index a131ff49b69..5b8fd75f44f 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,20 +26,82 @@ 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)) + parent := path + i := 0 + // If we reach the file system root then we could not find any existing Dir. + // In this case, workingDir Init cannot handle it. Let k8s handle the creation instead. + for parent != "/" { + parent = filepath.Dir(parent) + if ex, err := exists(parent); err != nil { + return fmt.Errorf("error accessing path %s: %w", parent, err) + } else if ex { + // We need to get its permissions and apply it to the child directories. + perm, err := getPermissions(parent) + if err != nil { + return fmt.Errorf("error accessing path %s: %w", parent, 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 := parent + idx := len(parts) - i - 1 + for j := idx; 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 + } + i += 1 + } + return nil +} + func main() { for i, d := range os.Args { // os.Args[0] is the path to this executable, so we should skip it if i == 0 { continue } - - 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) - } + 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..cfeff217038 100644 --- a/pkg/pod/workingdir_init.go +++ b/pkg/pod/workingdir_init.go @@ -36,27 +36,38 @@ import ( // If the init container will run on windows, `windows` should be set to `true`, // 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() + 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) + 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 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) + // Gather all unique workingDirs that are on VolumeMounts. + workingDirs := sets.New[string]() + for _, step := range stepContainers { + if step.WorkingDir != "" { + // Only handles relative path inside `/workspace` for backwards compatibility. + if !filepath.IsAbs(step.WorkingDir) { + step.WorkingDir = filepath.Join(pipeline.WorkspaceDir, step.WorkingDir) + } + for _, vm := range vms { + // only append to workingDirs if the step's workingDir is a subdir of a VolumeMount. + // Otherwise, the path will be unique to the init container and won't actually affect + // the step's workingDir. In that case, we want to let k8s handle the creation. + if strings.HasPrefix(step.WorkingDir, vm.MountPath) { + workingDirs.Insert(filepath.Clean(step.WorkingDir)) + } + } } } - if len(relativeDirs) == 0 { + if workingDirs.Len() == 0 { // There are no workingDirs to initialize. return nil } @@ -69,9 +80,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.UnsortedList(), + 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..a15a7a8045c 100644 --- a/pkg/pod/workingdir_init_test.go +++ b/pkg/pod/workingdir_init_test.go @@ -17,10 +17,10 @@ limitations under the License. package pod import ( + "sort" "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 +41,7 @@ func TestWorkingDirInit(t *testing.T) { }}, want: nil, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored", + desc: "workingDirs are unique", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -61,12 +61,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{"/workspace/zzz", "/workspace/aaa", "/workspace", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, + securitycontext", + desc: "workingDirs are unique + securitycontext", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -87,13 +86,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{"/workspace/zzz", "/workspace/aaa", "/workspace", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, SecurityContext: linuxSecurityContext, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, uses windows", + desc: "workingDirs are unique, uses windows", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -114,12 +112,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{"/workspace/zzz", "/workspace/aaa", "/workspace", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, }, }, { - desc: "workingDirs are unique and sorted, absolute dirs are ignored, uses windows, + securityContext", + desc: "workingDirs are unique, uses windows, + securityContext", stepContainers: []corev1.Container{{ WorkingDir: "zzz", }, { @@ -141,15 +138,19 @@ 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{"/workspace/zzz", "/workspace/aaa", "/workspace", "/workspace/bbb"}, VolumeMounts: implicitVolumeMounts, SecurityContext: windowsSecurityContext, }, }} { t.Run(c.desc, func(t *testing.T) { got := workingDirInit(images.WorkingDirInitImage, c.stepContainers, c.setSecurityContext, c.windows) - if d := cmp.Diff(c.want, got); d != "" { + trans := cmp.Transformer("Sort", func(inp corev1.Container) []string { + out := append([]string(nil), inp.Args...) // Copy input to avoid mutating it + sort.Strings(out) + return out + }) + if d := cmp.Diff(c.want, got, trans); d != "" { t.Fatalf("Diff %s", diff.PrintWantGot(d)) } })