pkg/pod: simplify orderContainers returns

This simplifies a bit the `orderContainers` function to remove an unecessary return that can be extracted outside. It simplifies the functino a bit reduces its responsability. Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Sep 21, 2021 · 4887c02 · 4887c02
1 parent 6852852
commit 4887c02
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 40 deletions.
diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go
@@ -103,30 +103,15 @@ var (
 )
 
 // orderContainers returns the specified steps, modified so that they are
-// executed in order by overriding the entrypoint binary. It also returns the
-// init container that places the entrypoint binary pulled from the
-// entrypointImage.
+// executed in order by overriding the entrypoint binary.
 //
 // Containers must have Command specified; if the user didn't specify a
 // command, we must have fetched the image's ENTRYPOINT before calling this
 // method, using entrypoint_lookup.go.
 // Additionally, Step timeouts are added as entrypoint flag.
-func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec, breakpointConfig *v1beta1.TaskRunDebug) (corev1.Container, []corev1.Container, error) {
-	initContainer := corev1.Container{
-		Name:  "place-tools",
-		Image: 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},
-	}
-
+func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1beta1.TaskSpec, breakpointConfig *v1beta1.TaskRunDebug) ([]corev1.Container, error) {
 	if len(steps) == 0 {
-		return corev1.Container{}, nil, errors.New("No steps specified")
+		return nil, errors.New("No steps specified")
 	}
 
 	for i, s := range steps {
@@ -169,7 +154,7 @@ func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string,
 
 		cmd, args := s.Command, s.Args
 		if len(cmd) == 0 {
-			return corev1.Container{}, nil, fmt.Errorf("Step %d did not specify command", i)
+			return nil, fmt.Errorf("Step %d did not specify command", i)
 		}
 		if len(cmd) > 1 {
 			args = append(cmd[1:], args...)
@@ -197,7 +182,7 @@ func orderContainers(entrypointImage string, commonExtraEntrypointArgs []string,
 	// Mount the Downward volume into the first step container.
 	steps[0].VolumeMounts = append(steps[0].VolumeMounts, downwardMount)
 
-	return initContainer, steps, nil
+	return steps, nil
 }
 
 func resultArgument(steps []corev1.Container, results []v1beta1.TaskResult) []string {

diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go
@@ -95,24 +95,13 @@ func TestOrderContainers(t *testing.T) {
 		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
-	gotInit, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, nil)
+	got, err := orderContainers([]string{}, steps, nil, nil)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}
 	if d := cmp.Diff(want, got); d != "" {
 		t.Errorf("Diff %s", diff.PrintWantGot(d))
 	}
-
-	wantInit := corev1.Container{
-		Name:         "place-tools",
-		Image:        images.EntrypointImage,
-		WorkingDir:   "/",
-		Command:      []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", entrypointBinary},
-		VolumeMounts: []corev1.VolumeMount{binMount},
-	}
-	if d := cmp.Diff(wantInit, gotInit); d != "" {
-		t.Errorf("Init Container Diff %s", diff.PrintWantGot(d))
-	}
 }
 
 func TestOrderContainersWithDebugOnFailure(t *testing.T) {
@@ -141,7 +130,7 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) {
 	taskRunDebugConfig := &v1beta1.TaskRunDebug{
 		Breakpoint: []string{"onFailure"},
 	}
-	_, got, err := orderContainers(images.EntrypointImage, []string{}, steps, nil, taskRunDebugConfig)
+	got, err := orderContainers([]string{}, steps, nil, taskRunDebugConfig)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}
@@ -223,7 +212,7 @@ func TestEntryPointResults(t *testing.T) {
 		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
-	_, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil)
+	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}
@@ -265,7 +254,7 @@ func TestEntryPointResultsSingleStep(t *testing.T) {
 		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
-	_, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil)
+	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}
@@ -303,7 +292,7 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) {
 		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount, downwardMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
-	_, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil)
+	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}
@@ -363,7 +352,7 @@ func TestEntryPointOnError(t *testing.T) {
 		VolumeMounts:           []corev1.VolumeMount{binROMount, runMount},
 		TerminationMessagePath: "/tekton/termination",
 	}}
-	_, got, err := orderContainers(images.EntrypointImage, []string{}, steps, &taskSpec, nil)
+	got, err := orderContainers([]string{}, steps, &taskSpec, nil)
 	if err != nil {
 		t.Fatalf("orderContainers: %v", err)
 	}

diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
@@ -105,7 +105,6 @@ type Transformer func(*corev1.Pod) (*corev1.Pod, error)
 func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, transformers ...Transformer) (*corev1.Pod, error) {
 	var (
 		scriptsInit                                       *corev1.Container
-		entrypointInit                                    corev1.Container
 		initContainers, stepContainers, sidecarContainers []corev1.Container
 		volumes                                           []corev1.Volume
 		volumeMounts                                      []corev1.VolumeMount
@@ -171,10 +170,23 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	// 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 {
-		entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug)
+		stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug)
 	} else {
-		entrypointInit, stepContainers, err = orderContainers(b.Images.EntrypointImage, credEntrypointArgs, stepContainers, &taskSpec, nil)
+		stepContainers, err = orderContainers(credEntrypointArgs, stepContainers, &taskSpec, nil)
 	}
 	if err != nil {
 		return nil, err