Lockdown /tekton/step folders to their own steps.

This change symlinks /tekton/step folders to a step's corresponding /tekton/run folder.
This is an incremental change to lock down the /tekton folder to prevent tampering
of exitCode files from other steps.
Note: this does not completely protect against a step from tampering from its own
output - more work will needed in a future PR to fully lock this down, but this
is a step in the right direction (and a complete fix will likely require a more
involved design).

While /tekton/steps is now considered an implementation detail and could
potentially be removed, the folder is preserved for now to limit the scope of this PR.

- Moves `exitCode` output to `/tekton/run/<step #>/status`
- Symlinks `/tekton/steps/<step #>` and `/tekton/steps/<step name>` to
`/tekton/run/<step #>/status`.
- Creates new `tekton-init` entrypoint subcommand to initialize the
Tekton step directory.
- Removes `-step_metadata_dir_link` flag from the main entrypoint binary
(this behavior is now handled by the initcontainer).

Co-authored-by: Lee Bernick <leebernick@google.com>

Author: 2 people

cmd/entrypoint/main.go

@@ -47,8 +47,7 @@ var (
 	breakpointOnFailure = flag.Bool("breakpoint_on_failure", false, "If specified, expect steps to not skip on failure")
 	onError             = flag.String("on_error", "", "Set to \"continue\" to ignore an error and continue when a container terminates with a non-zero exit code."+
 		" Set to \"stopAndFail\" to declare a failure with a step error and stop executing the rest of the steps.")
-	stepMetadataDir     = flag.String("step_metadata_dir", "", "If specified, create directory to store the step metadata e.g. /tekton/steps/<step-name>/")
-	stepMetadataDirLink = flag.String("step_metadata_dir_link", "", "creates a symbolic link to the specified step_metadata_dir e.g. /tekton/steps/<step-index>/")
+	stepMetadataDir = flag.String("step_metadata_dir", "", "If specified, create directory to store the step metadata e.g. /tekton/steps/<step-name>/")
 )
 
 const (
@@ -133,7 +132,6 @@ func main() {
 		BreakpointOnFailure: *breakpointOnFailure,
 		OnError:             *onError,
 		StepMetadataDir:     *stepMetadataDir,
-		StepMetadataDirLink: *stepMetadataDirLink,
 	}
 
 	// Copy any creds injected by the controller into the $HOME directory of the current

cmd/entrypoint/post_writer.go

@@ -3,6 +3,7 @@ package main
 import (
 	"log"
 	"os"
+	"path/filepath"
 
 	"github.com/tektoncd/pipeline/pkg/entrypoint"
 )
@@ -13,11 +14,16 @@ type realPostWriter struct{}
 var _ entrypoint.PostWriter = (*realPostWriter)(nil)
 
 // Write creates a file and writes content to that file if content is specified
-// assumption here is the underlying directory structure already exists
 func (*realPostWriter) Write(file string, content string) {
 	if file == "" {
 		return
 	}
+
+	// Create directory if it doesn't already exist
+	if err := os.MkdirAll(filepath.Dir(file), os.ModePerm); err != nil {
+		log.Fatalf("Error creating parent directory of %q: %v", file, err)
+	}
+
 	f, err := os.Create(file)
 	if err != nil {
 		log.Fatalf("Creating %q: %v", file, err)
@@ -29,26 +35,3 @@ func (*realPostWriter) Write(file string, content string) {
 		}
 	}
 }
-
-// CreateDirWithSymlink creates the specified directory and a symbolic link to that directory
-func (*realPostWriter) CreateDirWithSymlink(source, link string) {
-	if source == "" {
-		return
-	}
-	if err := os.MkdirAll(source, 0770); err != nil {
-		log.Fatalf("Creating directory %q: %v", source, err)
-	}
-
-	if link == "" {
-		return
-	}
-	// create a symlink if it does not exist
-	if _, err := os.Stat(link); os.IsNotExist(err) {
-		// check if a source exist before creating a symbolic link
-		if _, err := os.Stat(source); err == nil {
-			if err := os.Symlink(source, link); err != nil {
-				log.Fatalf("Creating a symlink %q: %v", link, err)
-			}
-		}
-	}
-}

cmd/entrypoint/post_writer_test.go

@@ -3,10 +3,16 @@ package main
 import (
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 )
 
 func TestRealPostWriter_WriteFileContent(t *testing.T) {
+	testdir, err := ioutil.TempDir("", "post-writer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(testdir)
 	tests := []struct {
 		name, file, content string
 	}{{
@@ -18,6 +24,9 @@ func TestRealPostWriter_WriteFileContent(t *testing.T) {
 	}, {
 		name: "create an empty file",
 		file: "sample.txt",
+	}, {
+		name: "create an empty file in new subdirectory",
+		file: filepath.Join(testdir, "dir", "sample.txt"),
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -39,39 +48,3 @@ func TestRealPostWriter_WriteFileContent(t *testing.T) {
 		})
 	}
 }
-
-func TestRealPostWriter_CreateStepPath(t *testing.T) {
-	tests := []struct {
-		name, source, link string
-	}{{
-		name:   "Create a path with a file",
-		source: "sample.txt",
-		link:   "0",
-	}, {
-		name: "Create a path without specifying any path",
-	}, {
-		name:   "Create a sym link without specifying any link path",
-		source: "sample.txt",
-	}, {
-		name: "Create a sym link without specifying any source",
-		link: "0.txt",
-	}}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			rw := realPostWriter{}
-			rw.CreateDirWithSymlink(tt.source, tt.link)
-			if tt.source != "" {
-				defer os.Remove(tt.source)
-				if _, err := os.Stat(tt.source); err != nil {
-					t.Fatalf("Failed to create a file %q", tt.source)
-				}
-			}
-			if tt.source != "" && tt.link != "" {
-				defer os.Remove(tt.link)
-				if _, err := os.Stat(tt.link); err != nil {
-					t.Fatalf("Failed to create a sym link %q", tt.link)
-				}
-			}
-		})
-	}
-}

cmd/entrypoint/subcommands/step_init.go

@@ -0,0 +1,46 @@
+package subcommands
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"strconv"
+)
+
+// StepInitCommand is the name of the /tekton/steps initialization command.
+const StepInitCommand = "step-init"
+
+var (
+	// root is the location of the Tekton root directory.
+	// Included as a global variable to allow overriding for tests.
+	tektonRoot = "/tekton"
+)
+
+// stepInit sets up the /tekton/steps directory for the pod.
+// This expects the list of steps (in order matching the Task spec).
+func stepInit(steps []string) error {
+	// Setup step directory symlinks - step data is written to a /tekton/run/<step>/status
+	// folder corresponding to each step - this is only mounted RW for the matching user step
+	// (and RO for all other steps).
+	// /tekton/steps provides a convenience symlink so that Tekton utilities to reference steps
+	// by name or index.
+	// NOTE: /tekton/steps may be removed in the future. Prefer using /tekton/run directly if
+	// possible.
+
+	// Create directory if it doesn't already exist
+	stepDir := filepath.Join(tektonRoot, "steps")
+	if err := os.MkdirAll(stepDir, os.ModePerm); err != nil {
+		log.Fatalf("Error creating steps directory %q: %v", stepDir, err)
+	}
+
+	for i, s := range steps {
+		run := filepath.Join(tektonRoot, "run", strconv.Itoa(i), "status")
+		if err := os.Symlink(run, filepath.Join(stepDir, s)); err != nil {
+			return err
+		}
+		if err := os.Symlink(run, filepath.Join(stepDir, strconv.Itoa(i))); err != nil {
+			return err
+		}
+	}
+	return nil
+}

cmd/entrypoint/subcommands/step_init_test.go

@@ -0,0 +1,63 @@
+package subcommands
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestStepInit(t *testing.T) {
+	tmp, err := ioutil.TempDir("", "step-init-*")
+	if err != nil {
+		t.Fatalf("error creating temp directory: %v", err)
+	}
+	defer os.RemoveAll(tmp)
+
+	// Override tektonRoot for testing.
+	tektonRoot = tmp
+
+	// Create step directory so that symlinks can be successfully created.
+	// This is typically done by volume mounts, so it needs to be done manually
+	// in tests.
+	stepDir := filepath.Join(tmp, "steps")
+	if err := os.Mkdir(stepDir, os.ModePerm); err != nil {
+		t.Fatalf("error creating step directory: %v", err)
+	}
+
+	steps := []string{"a", "b"}
+	if err := stepInit(steps); err != nil {
+		t.Fatalf("stepInit: %v", err)
+	}
+
+	// Map of symlinks to expected /tekton/run folders.
+	// Expected format:
+	// Key: /tekton/steps/<key>
+	// Value: /tekton/run/<value>/status
+	wantLinks := map[string]string{
+		"a": "0",
+		"0": "0",
+		"b": "1",
+		"1": "1",
+	}
+
+	direntry, err := os.ReadDir(stepDir)
+	if err != nil {
+		t.Fatalf("os.ReadDir: %v", err)
+	}
+	for _, de := range direntry {
+		t.Run(de.Name(), func(t *testing.T) {
+			l, err := os.Readlink(filepath.Join(stepDir, de.Name()))
+			if err != nil {
+				t.Fatal(err)
+			}
+			want, ok := wantLinks[de.Name()]
+			if !ok {
+				t.Fatalf("unexpected symlink: %s", de.Name())
+			}
+			if wantDir := filepath.Join(tmp, "run", want, "status"); l != wantDir {
+				t.Errorf("want %s, got %s", wantDir, l)
+			}
+		})
+	}
+}

cmd/entrypoint/subcommands/subcommands.go

@@ -72,6 +72,11 @@ func Process(args []string) error {
 			}
 			return SubcommandSuccessful{message: fmt.Sprintf("Decoded script %s", src)}
 		}
+	case StepInitCommand:
+		if err := stepInit(args[1:]); err != nil {
+			return SubcommandError{subcommand: StepInitCommand, message: err.Error()}
+		}
+		return SubcommandSuccessful{message: "Setup /step directories"}
 	default:
 	}
 	return nil

cmd/entrypoint/subcommands/subcommands_test.go

@@ -38,6 +38,21 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
 	if _, ok := returnValue.(SubcommandSuccessful); !ok {
 		t.Errorf("unexpected return value from decode-script command: %v", returnValue)
 	}
+
+	t.Run(StepInitCommand, func(t *testing.T) {
+		tektonRoot = tmp
+
+		returnValue := Process([]string{StepInitCommand})
+		if _, ok := returnValue.(SubcommandSuccessful); !ok {
+			t.Errorf("unexpected return value from step-init command: %v", returnValue)
+		}
+
+		returnValue = Process([]string{StepInitCommand, "foo", "bar"})
+		if _, ok := returnValue.(SubcommandSuccessful); !ok {
+			t.Errorf("unexpected return value from step-init command w/ params: %v", returnValue)
+		}
+	})
+
 }
 
 // TestProcessIgnoresNonSubcommands checks that any input to Process which