Make results folder writable (Fix for tektoncd#2131)

othomann · Mar 3, 2020 · 18a2618 · 18a2618
1 parent c317d64
commit 18a2618
Show file tree

Hide file tree

Showing 8 changed files with 257 additions and 60 deletions.
diff --git a/cmd/entrypoint/README.md b/cmd/entrypoint/README.md
@@ -5,6 +5,7 @@ wrapping it. In `tektoncd/pipeline` this is used to make sure `Task`'s
 steps are executed in order, or for sidecars.
 
 The following flags are available :
+
 - `-entrypoint`: "original" command to be executed (as
   entrypoint). This will be executed as a sub-process on `entrypoint`
 - `-post_file`: file path to write once the sub-process has
@@ -22,13 +23,13 @@ The following flags are available :
 The following example of usage for `entrypoint`, wait's for
 `/tekton/downward/ready` file to exists and have some content before
 executing `/ko-app/bash -- -args mkdir -p /workspace/git-resource`,
-and will write to `/tekton/tools/0` in casse of succes, or
+and will write to `/tekton/tools/0` in case of succes, or
 `/tekton/tools/0.err` in case of failure.
 
-```
+```shell
 entrypoint \
-	-wait_file /tekton/downward/ready \
-	-post_file /tekton/tools/0" \
-	-wait_file_content  \
-	-entrypoint /ko-app/bash -- -args mkdir -p /workspace/git-resource
+ -wait_file /tekton/downward/ready \
+ -post_file /tekton/tools/0" \
+ -wait_file_content  \
+ -entrypoint /ko-app/bash -- -args mkdir -p /workspace/git-resource
 ```
diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go
@@ -25,7 +25,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/entrypoint"
 )
 
@@ -54,13 +53,6 @@ func main() {
 		PostWriter:      &realPostWriter{},
 		Results:         strings.Split(*results, ","),
 	}
-	// strings.Split(..) with an empty string returns an array that contains one element, an empty string.
-	// The result folder should only be created if there are actual results to defined for the entrypoint.
-	if len(e.Results) >= 1 && e.Results[0] != "" {
-		if err := os.MkdirAll(pipeline.DefaultResultPath, 0755); err != nil {
-			log.Fatalf("Error creating the results directory: %v", err)
-		}
-	}
 	if err := e.Go(); err != nil {
 		switch t := err.(type) {
 		case skipError:

diff --git a/docs/developers/README.md b/docs/developers/README.md
@@ -281,6 +281,10 @@ status:
 
 Instead of hardcoding the path to the result file, the user can also use a variable. So `/tekton/results/current-date-unix-timestamp` can be replaced with: `$(results.current-date-unix-timestamp.path)`. This is more flexible if the path to result files ever changes.
 
+### Known issues
+
+When using a podTemplate to specify a non-root user for all containers, a task will not be able to modify the contents of the results directory and may fail.
+
 ## How task results can be used in pipeline's tasks
 
 Now that we have tasks that can return a result, the user can refer to a task result in a pipeline by using the syntax

diff --git a/examples/pipelineruns/task_results_example_user.yaml b/examples/pipelineruns/task_results_example_user.yaml
@@ -0,0 +1,97 @@
+apiVersion: tekton.dev/v1alpha1
+kind: Pipeline
+metadata:
+  name: sum-and-multiply-pipeline-user
+spec:
+  params:
+    - name: a
+      type: string
+      default: "1"
+    - name: b
+      type: string
+      default: "1"
+  tasks:
+    - name: sum-inputs
+      taskRef:
+        name: sum-user
+      params:
+        - name: a
+          value: "$(params.a)"
+        - name: b
+          value: "$(params.b)"
+    - name: multiply-inputs
+      taskRef:
+        name: multiply-user
+      params:
+        - name: a
+          value: "$(params.a)"
+        - name: b
+          value: "$(params.b)"
+    - name: sum-and-multiply
+      taskRef:
+        name: sum-user
+      params:
+        - name: a
+          value: "$(tasks.multiply-inputs.results.product)$(tasks.sum-inputs.results.sum)"
+        - name: b
+          value: "$(tasks.multiply-inputs.results.product)$(tasks.sum-inputs.results.sum)"
+---
+apiVersion: tekton.dev/v1alpha1
+kind: Task
+metadata:
+  name: sum-user
+  annotations:
+    description: |
+      A simple task that sums the two provided integers
+spec:
+  inputs:
+    params:
+      - name: a
+        type: string
+        default: "1"
+        description: The first integer
+      - name: b
+        type: string
+        default: "1"
+        description: The second integer
+  results:
+    - name: sum
+      description: The sum of the two provided integers
+  steps:
+    - name: sum
+      image: bash:latest
+      script: |
+        #!/usr/bin/env bash
+        echo -n $(( "$(inputs.params.a)" + "$(inputs.params.b)" )) | tee $(results.sum.path)
+      securityContext:
+        runAsUser: 1000
+---
+apiVersion: tekton.dev/v1alpha1
+kind: Task
+metadata:
+  name: multiply-user
+  annotations:
+    description: |
+      A simple task that multiplies the two provided integers
+spec:
+  inputs:
+    params:
+      - name: a
+        type: string
+        default: "1"
+        description: The first integer
+      - name: b
+        type: string
+        default: "1"
+        description: The second integer
+  results:
+    - name: product
+      description: The product of the two provided integers
+  steps:
+    - name: product
+      image: bash:latest
+      script: |
+        #!/usr/bin/env bash
+        echo -n $(( "$(inputs.params.a)" * "$(inputs.params.b)" )) | tee $(results.product.path)
+      securityContext:
+        runAsUser: 1000
diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
@@ -34,6 +34,9 @@ import (
 const (
 	homeDir = "/tekton/home"
 
+	// ResultsDir is the folder used by default to create the results file
+	ResultsDir = "/tekton/results"
+
 	featureFlagConfigMapName        = "feature-flags"
 	featureFlagDisableHomeEnvKey    = "disable-home-env-overwrite"
 	featureFlagDisableWorkingDirKey = "disable-working-directory-overwrite"
@@ -58,13 +61,19 @@ var (
 	}, {
 		Name:      "tekton-internal-home",
 		MountPath: homeDir,
+	}, {
+		Name:      "tekton-internal-results",
+		MountPath: ResultsDir,
 	}}
 	implicitVolumes = []corev1.Volume{{
 		Name:         "tekton-internal-workspace",
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}, {
 		Name:         "tekton-internal-home",
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
+	}, {
+		Name:         "tekton-internal-results",
+		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}}
 )
 
@@ -94,6 +103,11 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
 		volumes = append(volumes, secretsVolumes...)
 	}
 
+	// make tekton results folder writable by any user
+	if makeTektonFolderWritableInit := makeTektonResultsFolderWritable(images.ShellImage, implicitVolumeMounts); makeTektonFolderWritableInit != nil {
+		initContainers = append(initContainers, *makeTektonFolderWritableInit)
+	}
+
 	// Merge step template with steps.
 	// TODO(#1605): Move MergeSteps to pkg/pod
 	steps, err := v1alpha1.MergeStepsWithStepTemplate(taskSpec.StepTemplate, taskSpec.Steps)
@@ -253,7 +267,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
 	}, nil
 }
 
-// makeLabels constructs the labels we will propagate from TaskRuns to Pods.
+// MakeLabels constructs the labels we will propagate from TaskRuns to Pods.
 func MakeLabels(s *v1alpha1.TaskRun) map[string]string {
 	labels := make(map[string]string, len(s.ObjectMeta.Labels)+1)
 	// NB: Set this *before* passing through TaskRun labels. If the TaskRun

diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go
@@ -65,6 +65,14 @@ func TestMakePod(t *testing.T) {
 		VolumeMounts: []corev1.VolumeMount{toolsMount},
 	}
 
+	resultsInit := corev1.Container{
+		Name:         "tekton-results-folder-writable",
+		Image:        images.ShellImage,
+		Command:      []string{"sh"},
+		Args:         []string{"-c", "chmod 777 /tekton/results"},
+		VolumeMounts: implicitVolumeMounts,
+	}
+
 	runtimeClassName := "gvisor"
 	automountServiceAccountToken := false
 	dnsPolicy := corev1.DNSNone
@@ -88,7 +96,7 @@ func TestMakePod(t *testing.T) {
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-name",
 				Image:   "image",
@@ -141,6 +149,7 @@ func TestMakePod(t *testing.T) {
 				VolumeMounts: append(implicitVolumeMounts, secretsVolumeMount),
 				Env:          implicitEnvVars,
 			},
+				resultsInit,
 				placeToolsInit,
 			},
 			Containers: []corev1.Container{{
@@ -196,7 +205,7 @@ func TestMakePod(t *testing.T) {
 		},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-name",
 				Image:   "image",
@@ -246,7 +255,7 @@ func TestMakePod(t *testing.T) {
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-a-very-very-long-character-step-name-to-trigger-max-len", // step name trimmed.
 				Image:   "image",
@@ -282,7 +291,7 @@ func TestMakePod(t *testing.T) {
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-ends-with-invalid", // invalid suffix removed.
 				Image:   "image",
@@ -319,14 +328,16 @@ func TestMakePod(t *testing.T) {
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy: corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{{
-				Name:         "working-dir-initializer",
-				Image:        images.ShellImage,
-				Command:      []string{"sh"},
-				Args:         []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(pipeline.WorkspaceDir, "test"))},
-				WorkingDir:   pipeline.WorkspaceDir,
-				VolumeMounts: implicitVolumeMounts,
-			},
+			InitContainers: []corev1.Container{
+				resultsInit,
+				{
+					Name:         "working-dir-initializer",
+					Image:        images.ShellImage,
+					Command:      []string{"sh"},
+					Args:         []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(pipeline.WorkspaceDir, "test"))},
+					WorkingDir:   pipeline.WorkspaceDir,
+					VolumeMounts: implicitVolumeMounts,
+				},
 				placeToolsInit,
 			},
 			Containers: []corev1.Container{{
@@ -371,7 +382,7 @@ func TestMakePod(t *testing.T) {
 		wantAnnotations: map[string]string{},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-primary-name",
 				Image:   "primary-image",
@@ -421,20 +432,22 @@ func TestMakePod(t *testing.T) {
 		wantAnnotations: map[string]string{},
 		want: &corev1.PodSpec{
 			RestartPolicy: corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{{
-				Name:         "place-scripts",
-				Image:        "busybox",
-				Command:      []string{"sh"},
-				TTY:          true,
-				VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
-				Args: []string{"-c", `tmpfile="/tekton/scripts/sidecar-script-0-9l9zj"
+			InitContainers: []corev1.Container{
+				resultsInit,
+				{
+					Name:         "place-scripts",
+					Image:        "busybox",
+					Command:      []string{"sh"},
+					TTY:          true,
+					VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
+					Args: []string{"-c", `tmpfile="/tekton/scripts/sidecar-script-0-9l9zj"
 touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << 'sidecar-script-heredoc-randomly-generated-mz4c7'
 #!/bin/sh
 echo hello from sidecar
 sidecar-script-heredoc-randomly-generated-mz4c7
 `},
-			},
+				},
 				placeToolsInit,
 			},
 			Containers: []corev1.Container{{
@@ -494,7 +507,7 @@ sidecar-script-heredoc-randomly-generated-mz4c7
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			Containers: []corev1.Container{{
 				Name:    "step-unnamed-0",
 				Image:   "image",
@@ -582,12 +595,13 @@ print("Hello from Python")`,
 		}},
 		want: &corev1.PodSpec{
 			RestartPolicy: corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{{
-				Name:    "place-scripts",
-				Image:   images.ShellImage,
-				Command: []string{"sh"},
-				TTY:     true,
-				Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-9l9zj"
+			InitContainers: []corev1.Container{
+				resultsInit, {
+					Name:    "place-scripts",
+					Image:   images.ShellImage,
+					Command: []string{"sh"},
+					TTY:     true,
+					Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-9l9zj"
 touch ${tmpfile} && chmod +x ${tmpfile}
 cat > ${tmpfile} << 'script-heredoc-randomly-generated-mz4c7'
 #!/bin/sh
@@ -600,13 +614,14 @@ cat > ${tmpfile} << 'script-heredoc-randomly-generated-78c5n'
 print("Hello from Python")
 script-heredoc-randomly-generated-78c5n
 `},
-				VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
-			}, {
-				Name:         "place-tools",
-				Image:        images.EntrypointImage,
-				Command:      []string{"cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"},
-				VolumeMounts: []corev1.VolumeMount{toolsMount},
-			}},
+					VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
+				},
+				{
+					Name:         "place-tools",
+					Image:        images.EntrypointImage,
+					Command:      []string{"cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"},
+					VolumeMounts: []corev1.VolumeMount{toolsMount},
+				}},
 			Containers: []corev1.Container{{
 				Name:    "step-one",
 				Image:   "image",
@@ -700,7 +715,7 @@ script-heredoc-randomly-generated-78c5n
 		},
 		want: &corev1.PodSpec{
 			RestartPolicy:  corev1.RestartPolicyNever,
-			InitContainers: []corev1.Container{placeToolsInit},
+			InitContainers: []corev1.Container{resultsInit, placeToolsInit},
 			SchedulerName:  "there-scheduler",
 			Volumes:        append(implicitVolumes, toolsVolume, downwardVolume),
 			Containers: []corev1.Container{{