Fix variable interpolation on StepTemplate

This changes the way we interpolate variables by using `StepTemplate`
directly instead of modifying a de-referenced version of it.

Dereferencing means that changes happening to it (in
`ApplyStepReplacements`) are not reflected to the original object.
Before this change, using variable interpolation on `StepTemplate`
would only work on part of the fields they were supposed to be.
For example, it works on `env` because, it is an array and the
de-referenced version of `StepTemplate` would share the same pointer
to it, making any changes on the array reflect to the initial object.

This is now fixed.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>

examples/v1beta1/taskruns/steptemplate-variable-interop.yaml

@@ -0,0 +1,17 @@
+apiVersion: tekton.dev/v1beta1
+kind: TaskRun
+metadata:
+  generateName: steptemplate-variables-
+spec:
+  params:
+  - name: image
+    value: ubuntu
+  taskSpec:
+    params:
+    - name: image
+    stepTemplate:
+      image: $(params.image)
+    steps:
+    - name: foo
+      script: |
+        echo hello from $(params.image)

pkg/apis/pipeline/v1beta1/container_types.go

@@ -421,6 +421,31 @@ type StepTemplate struct {
 	TTY bool `json:"tty,omitempty" protobuf:"varint,18,opt,name=tty"`
 }
 
+// SetContainerFields sets the fields of the Step to the values of the corresponding fields in the Container
+func (s *StepTemplate) SetContainerFields(c corev1.Container) {
+	s.Name = c.Name
+	s.Image = c.Image
+	s.Command = c.Command
+	s.Args = c.Args
+	s.WorkingDir = c.WorkingDir
+	s.Ports = c.Ports
+	s.EnvFrom = c.EnvFrom
+	s.Env = c.Env
+	s.Resources = c.Resources
+	s.VolumeMounts = c.VolumeMounts
+	s.VolumeDevices = c.VolumeDevices
+	s.LivenessProbe = c.LivenessProbe
+	s.ReadinessProbe = c.ReadinessProbe
+	s.StartupProbe = c.StartupProbe
+	s.Lifecycle = c.Lifecycle
+	s.TerminationMessagePath = c.TerminationMessagePath
+	s.ImagePullPolicy = c.ImagePullPolicy
+	s.SecurityContext = c.SecurityContext
+	s.Stdin = c.Stdin
+	s.StdinOnce = c.StdinOnce
+	s.TTY = c.TTY
+}
+
 // ToK8sContainer converts the StepTemplate to a Kubernetes Container struct
 func (s *StepTemplate) ToK8sContainer() *corev1.Container {
 	return &corev1.Container{

pkg/apis/pipeline/v1beta1/step_replacements.go

@@ -25,3 +25,10 @@ func ApplyStepReplacements(step *Step, stringReplacements map[string]string, arr
 	step.Script = substitution.ApplyReplacements(step.Script, stringReplacements)
 	applyStepReplacements(step, stringReplacements, arrayReplacements)
 }
+
+// ApplyStepTemplateReplacements applies variable interpolation on a StepTemplate (aka a container)
+func ApplyStepTemplateReplacements(stepTemplate *StepTemplate, stringReplacements map[string]string, arrayReplacements map[string][]string) {
+	container := stepTemplate.ToK8sContainer()
+	applyContainerReplacements(container, stringReplacements, arrayReplacements)
+	stepTemplate.SetContainerFields(*container)
+}

pkg/reconciler/taskrun/resources/apply.go

@@ -248,10 +248,7 @@ func ApplyReplacements(spec *v1beta1.TaskSpec, stringReplacements map[string]str
 
 	// Apply variable expansion to stepTemplate fields.
 	if spec.StepTemplate != nil {
-		c := spec.StepTemplate.ToK8sContainer()
-		step := v1beta1.Step{}
-		step.SetContainerFields(*c)
-		v1beta1.ApplyStepReplacements(&step, stringReplacements, arrayReplacements)
+		v1beta1.ApplyStepTemplateReplacements(spec.StepTemplate, stringReplacements, arrayReplacements)
 	}
 
 	// Apply variable expansion to the build's volumes

pkg/reconciler/taskrun/resources/apply_test.go

@@ -62,6 +62,7 @@ var (
 				Name:  "template-var",
 				Value: `$(params["FOO"])`,
 			}},
+			Image: "$(params.myimage)",
 		},
 		Steps: []v1beta1.Step{{
 			Name:  "foo",
@@ -526,6 +527,7 @@ func TestApplyParameters(t *testing.T) {
 	}}
 	want := applyMutation(simpleTaskSpec, func(spec *v1beta1.TaskSpec) {
 		spec.StepTemplate.Env[0].Value = "world"
+		spec.StepTemplate.Image = "bar"
 
 		spec.Steps[0].Image = "bar"
 		spec.Steps[2].Image = "mydefault"