Split /tekton/run directories into separate volumes.

This changes splits each `/tekton/run` volume into separate volumes
so that steps can only mutate their own runtime information. This
prevents steps from unexpectedly interfering with other step execution.

To do this, we repurpose the `/tekton/run/#` path into a
step-specific directory. Since this was previously used by the
entrypoint for the post/wait files, we now use `/tekton/run/#/out` as
the post/wait filepath instead.

This does not change behavior of the directory, it enforces expected
behavior of steps.

`/tekton/run` is considered an internal implementation detail and is not
covered by the API compatibility policy, so it is safe to make changes
to the behavior of these files/paths.

This does not stop user execution from writing the step's own `/tekton/run/#`
folder. This needs more discussion/design - additional changes (if
needed) will be made in another commit.

This change is only focused on `/tekton/run` to reduce PR complexity. We will
likely want to make a similar change to /tekton/steps in another commit.
We may also look to consolidate all per-step volumes into a single
source (i.e. creds-init does something similar as well).

AFAICT, Ephemeral Volumes (i.e. EmptyDir) are exempt from Node Volume
limits (https://kubernetes.io/docs/concepts/storage/storage-limits/) -
spot checked this with a TaskRun with 100+ steps on both kind and GKE.

cmd/entrypoint/README.md

@@ -29,14 +29,14 @@ Any extra positional arguments are passed to the original entrypoint command.
 ## Example
 
 The following example of usage for `entrypoint` waits for
-`/tekton/run/3` file to exist and executes the command `bash` with args
-`echo` and `hello`, then writes the file `/tekton/run/4`, or
-`/tekton/run/4.err` in case the command fails.
+`/tekton/run/3/out` file to exist and executes the command `bash` with args
+`echo` and `hello`, then writes the file `/tekton/run/4/out`, or
+`/tekton/run/4/out.err` in case the command fails.
 
 ```shell
 entrypoint \
-  -wait_file /tekton/run/3 \
-  -post_file /tekton/run/4 \
+  -wait_file /tekton/run/3/out \
+  -post_file /tekton/run/4/out \
   -entrypoint bash -- \
   echo hello
 ```
@@ -64,14 +64,14 @@ to contain contents before proceeding.
 The following example of usage for `entrypoint` waits for
 `/tekton/downward/ready` file to exist and contain actual contents
 (`-wait_file_contents`), and executes the command `bash` with args
-`echo` and `hello`, then writes the file `/tekton/run/1`, or
-`/tekton/run/1.err` in case the command fails.
+`echo` and `hello`, then writes the file `/tekton/run/1/out`, or
+`/tekton/run/1/out.err` in case the command fails.
 
 ```shell
 entrypoint \
   -wait_file /tekton/downward/ready \
   -wait_file_contents \
-  -post_file /tekton/run/1 \
+  -post_file /tekton/run/1/out \
   -entrypoint bash -- \
   echo hello
 ```

docs/developers/README.md

@@ -166,6 +166,7 @@ Here is an example of a directory layout for a simple Task with 2 script steps:
 |-- results
 |-- run
     `-- 0
+        `-- out
 |-- scripts
 |   |-- script-0-t4jd8
 |   `-- script-1-4pjwp

examples/v1beta1/taskruns/readonly-internal-dir.yaml

@@ -0,0 +1,21 @@
+# This file is primarily used for test validation of internal Tekton
+# directories. This is not a useful example of typical user config.
+apiVersion: tekton.dev/v1beta1
+kind: TaskRun
+metadata:
+  generateName: readonly-internal-dir-
+spec:
+  taskSpec:
+    steps:
+    - image: ubuntu
+      script: exit 0
+    - image: ubuntu
+      script: |
+        set +e # dont fail the script on error
+
+        # Steps should not be able to write to other step's run directories.
+        echo "hello world" > /tekton/run/0/out
+        if [ $? -eq 0 ] ; then
+          echo "able to write to run directory of non-current step"
+          exit 1
+        fi

pkg/pod/entrypoint.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"log"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
@@ -72,15 +73,6 @@ var (
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}
 
-	runMount = corev1.VolumeMount{
-		Name:      runVolumeName,
-		MountPath: runDir,
-	}
-	runVolume = corev1.Volume{
-		Name:         runVolumeName,
-		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
-	}
-
 	// TODO(#1605): Signal sidecar readiness by injecting entrypoint,
 	// remove dependency on Downward API.
 	downwardVolume = corev1.Volume{
@@ -127,16 +119,16 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe
 				"-wait_file", filepath.Join(downwardMountPoint, downwardMountReadyFile),
 				"-wait_file_content", // Wait for file contents, not just an empty file.
 				// Start next step.
-				"-post_file", filepath.Join(runDir, fmt.Sprintf("%d", i)),
+				"-post_file", filepath.Join(runDir, strconv.Itoa(i), "out"),
 				"-termination_path", terminationPath,
 				"-step_metadata_dir", filepath.Join(pipeline.StepsDir, name),
 				"-step_metadata_dir_link", filepath.Join(pipeline.StepsDir, fmt.Sprintf("%d", i)),
 			}
 		default:
 			// All other steps wait for previous file, write next file.
 			argsForEntrypoint = []string{
-				"-wait_file", filepath.Join(runDir, fmt.Sprintf("%d", i-1)),
-				"-post_file", filepath.Join(runDir, fmt.Sprintf("%d", i)),
+				"-wait_file", filepath.Join(runDir, strconv.Itoa(i-1), "out"),
+				"-post_file", filepath.Join(runDir, strconv.Itoa(i), "out"),
 				"-termination_path", terminationPath,
 				"-step_metadata_dir", filepath.Join(pipeline.StepsDir, name),
 				"-step_metadata_dir_link", filepath.Join(pipeline.StepsDir, fmt.Sprintf("%d", i)),
@@ -179,7 +171,6 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe
 
 		steps[i].Command = []string{entrypointBinary}
 		steps[i].Args = argsForEntrypoint
-		steps[i].VolumeMounts = append(steps[i].VolumeMounts, binROMount, runMount)
 		steps[i].TerminationMessagePath = terminationPath
 	}
 	// Mount the Downward volume into the first step container.

pkg/pod/entrypoint_test.go

@@ -56,43 +56,42 @@ func TestOrderContainers(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-0",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}, {
 		Image:   "step-2",
 		Command: []string{entrypointBinary},
 		Args: []string{
-			"-wait_file", "/tekton/run/0",
-			"-post_file", "/tekton/run/1",
+			"-wait_file", "/tekton/run/0/out",
+			"-post_file", "/tekton/run/1/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-1",
 			"-step_metadata_dir_link", "/tekton/steps/1",
 			"-entrypoint", "cmd1", "--",
 			"cmd2", "cmd3",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{volumeMount, binROMount, runMount},
+		VolumeMounts:           []corev1.VolumeMount{volumeMount},
 		TerminationMessagePath: "/tekton/termination",
 	}, {
 		Image:   "step-3",
 		Command: []string{entrypointBinary},
 		Args: []string{
-			"-wait_file", "/tekton/run/1",
-			"-post_file", "/tekton/run/2",
+			"-wait_file", "/tekton/run/1/out",
+			"-post_file", "/tekton/run/2/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-2",
 			"-step_metadata_dir_link", "/tekton/steps/2",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	got, err := orderContainers([]string{}, steps, nil, nil)
@@ -116,15 +115,15 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-0",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-breakpoint_on_failure",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	taskRunDebugConfig := &v1beta1.TaskRunDebug{
@@ -170,22 +169,22 @@ func TestEntryPointResults(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-0",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-results", "sum,sub",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}, {
 		Image:   "step-2",
 		Command: []string{entrypointBinary},
 		Args: []string{
-			"-wait_file", "/tekton/run/0",
-			"-post_file", "/tekton/run/1",
+			"-wait_file", "/tekton/run/0/out",
+			"-post_file", "/tekton/run/1/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-1",
 			"-step_metadata_dir_link", "/tekton/steps/1",
@@ -194,22 +193,21 @@ func TestEntryPointResults(t *testing.T) {
 			"cmd2", "cmd3",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{volumeMount, binROMount, runMount},
+		VolumeMounts:           []corev1.VolumeMount{volumeMount},
 		TerminationMessagePath: "/tekton/termination",
 	}, {
 		Image:   "step-3",
 		Command: []string{entrypointBinary},
 		Args: []string{
-			"-wait_file", "/tekton/run/1",
-			"-post_file", "/tekton/run/2",
+			"-wait_file", "/tekton/run/1/out",
+			"-post_file", "/tekton/run/2/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-2",
 			"-step_metadata_dir_link", "/tekton/steps/2",
 			"-results", "sum,sub",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
@@ -243,15 +241,15 @@ func TestEntryPointResultsSingleStep(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-0",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-results", "sum,sub",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
@@ -281,15 +279,15 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-unnamed-0",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-results", "sum",
 			"-entrypoint", "cmd", "--",
 			"arg1", "arg2",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
@@ -327,29 +325,28 @@ func TestEntryPointOnError(t *testing.T) {
 		Args: []string{
 			"-wait_file", "/tekton/downward/ready",
 			"-wait_file_content",
-			"-post_file", "/tekton/run/0",
+			"-post_file", "/tekton/run/0/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-failing-step",
 			"-step_metadata_dir_link", "/tekton/steps/0",
 			"-on_error", "continue",
 			"-entrypoint", "cmd", "--",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
+		VolumeMounts:           []corev1.VolumeMount{downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}, {
 		Name:    "passing-step",
 		Image:   "step-2",
 		Command: []string{entrypointBinary},
 		Args: []string{
-			"-wait_file", "/tekton/run/0",
-			"-post_file", "/tekton/run/1",
+			"-wait_file", "/tekton/run/0/out",
+			"-post_file", "/tekton/run/1/out",
 			"-termination_path", "/tekton/termination",
 			"-step_metadata_dir", "/tekton/steps/step-passing-step",
 			"-step_metadata_dir_link", "/tekton/steps/1",
 			"-on_error", "stopAndFail",
 			"-entrypoint", "cmd", "--",
 		},
-		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
 	got, err := orderContainers([]string{}, steps, &taskSpec, nil)

pkg/pod/pod.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"strconv"
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
@@ -105,8 +106,8 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		scriptsInit                                       *corev1.Container
 		initContainers, stepContainers, sidecarContainers []corev1.Container
 		volumes                                           []corev1.Volume
-		volumeMounts                                      []corev1.VolumeMount
 	)
+	volumeMounts := []corev1.VolumeMount{binROMount}
 	implicitEnvVars := []corev1.EnvVar{}
 	alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields
 
@@ -185,7 +186,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	// place the entrypoint first in case other init containers rely on its
 	// features (e.g. decode-script).
 	initContainers = append([]corev1.Container{entrypointInit}, initContainers...)
-	volumes = append(volumes, binVolume, runVolume, downwardVolume)
+	volumes = append(volumes, binVolume, downwardVolume)
 
 	// Add implicit env vars.
 	// They're prepended to the list, so that if the user specified any
@@ -220,6 +221,14 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 			s.VolumeMounts = append(s.VolumeMounts, *vm)
 		}
 
+		// Add /tekton/run state volumes.
+		// Each step should only mount their own volume as RW,
+		// all other steps should be mounted RO.
+		volumes = append(volumes, runVolume(i))
+		for j := 0; j < len(stepContainers); j++ {
+			s.VolumeMounts = append(s.VolumeMounts, runMount(j, i != j))
+		}
+
 		requestedVolumeMounts := map[string]bool{}
 		for _, vm := range s.VolumeMounts {
 			requestedVolumeMounts[filepath.Clean(vm.MountPath)] = true
@@ -369,3 +378,18 @@ func shouldAddReadyAnnotationOnPodCreate(ctx context.Context, sidecars []v1beta1
 	cfg := config.FromContextOrDefaults(ctx)
 	return !cfg.FeatureFlags.RunningInEnvWithInjectedSidecars
 }
+
+func runMount(i int, ro bool) corev1.VolumeMount {
+	return corev1.VolumeMount{
+		Name:      fmt.Sprintf("%s-%d", runVolumeName, i),
+		MountPath: filepath.Join(runDir, strconv.Itoa(i)),
+		ReadOnly:  ro,
+	}
+}
+
+func runVolume(i int) corev1.Volume {
+	return corev1.Volume{
+		Name:         fmt.Sprintf("%s-%d", runVolumeName, i),
+		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
+	}
+}