Merge place-tools and step-init together

cmd/entrypoint/subcommands/init.go

@@ -0,0 +1,13 @@
+package subcommands
+
+// InitCommand is the name of main initialization command
+const InitCommand = "init"
+
+// init copies the entrypoint to the right place and sets up /tekton/steps directory for the pod.
+// This expects  the list of steps (in order matching the Task spec).
+func entrypointInit(src, dst string, steps []string) error {
+	if err := cp(src, dst); err != nil {
+		return err
+	}
+	return stepInit(steps)
+}

cmd/entrypoint/subcommands/init_test.go

@@ -0,0 +1,82 @@
+package subcommands
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestEntrypointInit(t *testing.T) {
+	tmp := t.TempDir()
+	src := filepath.Join(tmp, "foo.txt")
+	dst := filepath.Join(tmp, "bar.txt")
+	if err := ioutil.WriteFile(src, []byte("hello world"), 0700); err != nil {
+		t.Fatalf("error writing source file: %v", err)
+	}
+
+	// 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 := entrypointInit(src, dst, steps); err != nil {
+		t.Fatalf("stepInit: %v", err)
+	}
+
+	info, err := os.Lstat(dst)
+	if err != nil {
+		t.Fatalf("error statting destination file: %v", err)
+	}
+
+	// os.OpenFile is subject to umasks, so the created permissions of the
+	// created dst file might be more restrictive than dstPermissions.
+	// excludePerm represents the value of permissions we do not want in the
+	// resulting file - e.g. if dstPermissions is 0311, excludePerm should be
+	// 0466.
+	// This is done instead of trying to look up the system umask, since this
+	// relies on syscalls that we are not sure will be portable across
+	// environments.
+	excludePerm := os.ModePerm ^ dstPermissions
+	if p := info.Mode().Perm(); p&excludePerm != 0 {
+		t.Errorf("expected permissions <= %#o for destination file but found %#o", dstPermissions, p)
+	}
+
+	// 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

@@ -50,6 +50,18 @@ func Process(args []string) error {
 		return nil
 	}
 	switch args[0] {
+	case InitCommand:
+		// If invoked in "init mode" (`entrypoint init <src> <dst> [<step-name>]`),
+		// it will copy the src path to the dst path (like CopyCommand), and initialize
+		// the /tekton/steps folder (like StepInitCommand)
+		if len(args) >= 3 {
+			src, dst := args[1], args[2]
+			steps := args[3:]
+			if err := entrypointInit(src, dst, steps); err != nil {
+				return SubcommandError{subcommand: InitCommand, message: err.Error()}
+			}
+			return SubcommandSuccessful{message: "Entrypoint initialization"}
+		}
 	case CopyCommand:
 		// If invoked in "cp mode" (`entrypoint cp <src> <dst>`), simply copy
 		// the src path to the dst path. This is used to place the entrypoint

examples/v1beta1/pipelineruns/4808-regression.yaml

@@ -0,0 +1,95 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: print-result
+spec:
+  description: >-
+    Prints a result from another task
+  params:
+    - name: TO_PRINT
+      type: string
+  steps:
+    - name: print-result
+      image: bash:latest
+      env:
+        - name: PARAM_TO_PRINT
+          value: $(params.TO_PRINT)
+      script: |
+        #!/usr/bin/env bash
+        set -e
+        echo $PARAM_TO_PRINT
+---
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: generate-result
+spec:
+  description: >-
+    Creates strings of length based on parameters and puts them into results fields
+  params:
+    - name: STRING_LENGTH
+      description: Length of the string to create
+    - name: STRING_CHAR
+      description: Char to use when creating string
+      type: string
+      default: '.'
+  results:
+    - name: RESULT_STRING
+      description: A result string
+  steps:
+    - name: gen-result
+      image: bash:latest
+      env:
+        - name: PARAM_STRING_LENGTH
+          value: $(params.STRING_LENGTH)
+        - name: PARAM_STRING_CHAR
+          value: $(params.STRING_CHAR)
+      script: |
+        #!/usr/bin/env bash
+        set -e
+        len=$PARAM_STRING_LENGTH
+        ch=$PARAM_STRING_CHAR
+        printf '%*s' "$len" | tr ' ' "$ch" >>  $(results.RESULT_STRING.path)
+---
+apiVersion: tekton.dev/v1beta1
+kind: Pipeline
+metadata:
+  name: result-test
+spec:
+  description: >-
+    Generate a result of a certain length in a task and print the result in another task
+  params:
+  - name: RESULT_STRING_LENGTH
+    description: Length of string to generate for generate-result task
+  - name: RESULT_STRING_CHAR
+    description: Char to repeat in result string
+    default: '.'
+  tasks:
+  - name: generate-result
+    params:
+    - name: STRING_LENGTH
+      value: $(params.RESULT_STRING_LENGTH)
+    - name: STRING_CHAR
+      value: $(params.RESULT_STRING_CHAR)
+    taskRef:
+      kind: Task
+      name: generate-result
+  - name: print-result
+    params:
+    - name: TO_PRINT
+      value: $(tasks.generate-result.results.RESULT_STRING)
+    taskRef:
+      kind: Task
+      name: print-result
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: result-test-run
+spec:
+  pipelineRef:
+    name: result-test
+  params:
+  - name: RESULT_STRING_LENGTH
+    value: "3000"

pkg/pod/entrypoint.go

@@ -26,6 +26,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"gomodules.xyz/jsonpatch/v2"
 	corev1 "k8s.io/api/core/v1"
@@ -71,6 +72,10 @@ var (
 		Name:         binVolumeName,
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}
+	internalStepsMount = corev1.VolumeMount{
+		Name:      "tekton-internal-steps",
+		MountPath: pipeline.StepsDir,
+	}
 
 	// TODO(#1605): Signal sidecar readiness by injecting entrypoint,
 	// remove dependency on Downward API.

pkg/pod/pod.go

@@ -23,8 +23,6 @@ import (
 	"path/filepath"
 	"strconv"
 
-	"knative.dev/pkg/kmeta"
-
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
@@ -35,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"knative.dev/pkg/changeset"
+	"knative.dev/pkg/kmeta"
 )
 
 const (
@@ -147,22 +146,25 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		return nil, err
 	}
 
+	initContainers = []corev1.Container{
+		entrypointInitContainer(b.Images.EntrypointImage, steps),
+	}
+
 	// Convert any steps with Script to command+args.
 	// If any are found, append an init container to initialize scripts.
 	if alphaAPIEnabled {
 		scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, b.Images.ShellImageWin, steps, sidecars, taskRun.Spec.Debug)
 	} else {
 		scriptsInit, stepContainers, sidecarContainers = convertScripts(b.Images.ShellImage, "", steps, sidecars, nil)
 	}
+
 	if scriptsInit != nil {
 		initContainers = append(initContainers, *scriptsInit)
 		volumes = append(volumes, scriptsVolume)
 	}
-
 	if alphaAPIEnabled && taskRun.Spec.Debug != nil {
 		volumes = append(volumes, debugScriptsVolume, debugInfoVolume)
 	}
-
 	// Initialize any workingDirs under /workspace.
 	if workingDirInit := workingDirInit(b.Images.WorkingDirInitImage, stepContainers); workingDirInit != nil {
 		initContainers = append(initContainers, *workingDirInit)
@@ -181,22 +183,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		return nil, err
 	}
 
-	// Rewrite steps with entrypoint binary. Append the entrypoint init
-	// container to place the entrypoint binary. Also add timeout flags
-	// to entrypoint binary.
-	entrypointInit := corev1.Container{
-		Name:  "place-tools",
-		Image: b.Images.EntrypointImage,
-		// Rewrite default WorkingDir from "/home/nonroot" to "/"
-		// as suggested at https://github.com/GoogleContainerTools/distroless/issues/718
-		// to avoid permission errors with nonroot users not equal to `65532`
-		WorkingDir: "/",
-		// Invoke the entrypoint binary in "cp mode" to copy itself
-		// into the correct location for later steps.
-		Command:      []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", entrypointBinary},
-		VolumeMounts: []corev1.VolumeMount{binMount},
-	}
-
 	if alphaAPIEnabled {
 		stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug)
 	} else {
@@ -205,12 +191,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	if err != nil {
 		return nil, err
 	}
-	// place the entrypoint first in case other init containers rely on its
-	// features (e.g. decode-script).
-	initContainers = append([]corev1.Container{
-		entrypointInit,
-		tektonDirInit(b.Images.EntrypointImage, steps),
-	}, initContainers...)
 	volumes = append(volumes, binVolume, downwardVolume)
 
 	// Add implicit env vars.
@@ -421,21 +401,29 @@ func runVolume(i int) corev1.Volume {
 	}
 }
 
-func tektonDirInit(image string, steps []v1beta1.Step) corev1.Container {
-	cmd := make([]string, 0, len(steps)+2)
-	cmd = append(cmd, "/ko-app/entrypoint", "step-init")
+// prepareInitContainers generates a few init containers based of a set of command (in images) and volumes to run
+// This should effectively merge multiple command and volumes together.
+func entrypointInitContainer(image string, steps []v1beta1.Step) corev1.Container {
+	// Invoke the entrypoint binary in "cp mode" to copy itself
+	// into the correct location for later steps and initialize steps folder
+	command := []string{"/ko-app/entrypoint", "init", "/ko-app/entrypoint", entrypointBinary}
 	for i, s := range steps {
-		cmd = append(cmd, StepName(s.Name, i))
+		command = append(command, StepName(s.Name, i))
 	}
+	volumeMounts := []corev1.VolumeMount{binMount, internalStepsMount}
 
-	return corev1.Container{
-		Name:       "step-init",
-		Image:      image,
-		WorkingDir: "/",
-		Command:    cmd,
-		VolumeMounts: []corev1.VolumeMount{{
-			Name:      "tekton-internal-steps",
-			MountPath: pipeline.StepsDir,
-		}},
+	// Rewrite steps with entrypoint binary. Append the entrypoint init
+	// container to place the entrypoint binary. Also add timeout flags
+	// to entrypoint binary.
+	prepareInitContainer := corev1.Container{
+		Name:  "prepare",
+		Image: image,
+		// Rewrite default WorkingDir from "/home/nonroot" to "/"
+		// as suggested at https://github.com/GoogleContainerTools/distroless/issues/718
+		// to avoid permission errors with nonroot users not equal to `65532`
+		WorkingDir:   "/",
+		Command:      command,
+		VolumeMounts: volumeMounts,
 	}
+	return prepareInitContainer
 }