From f0a82012c58fe037473a2bdea62243b20cc75087 Mon Sep 17 00:00:00 2001
From: Alexander Misdorp <6093240+Peaorl@users.noreply.github.com>
Date: Fri, 7 Aug 2020 19:10:08 +0200
Subject: [PATCH] separate Step from Sidecar

separate Step from Sidecar

Originally the Sidecar type is an alias of the Step type.
Both https://github.com/tektoncd/pipeline/pull/3013 and https://github.com/tektoncd/pipeline/issues/1690 will require the two types to divert from each other.
This commit separates the two types and updates code to accomodate convertListOfSteps() which originally depended on theses types to be the same.
Partially based on: https://github.com/tektoncd/pipeline/pull/3013/files
---
 pkg/apis/pipeline/v1beta1/task_types.go       | 11 +++++++++--
 .../pipeline/v1beta1/zz_generated.deepcopy.go | 19 ++++++++++++++++++-
 pkg/pod/script.go                             | 12 +++++++++++-
 pkg/pod/script_test.go                        |  8 ++++----
 4 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/pkg/apis/pipeline/v1beta1/task_types.go b/pkg/apis/pipeline/v1beta1/task_types.go
index eaadc3d79ec..c68d2fc1890 100644
--- a/pkg/apis/pipeline/v1beta1/task_types.go
+++ b/pkg/apis/pipeline/v1beta1/task_types.go
@@ -125,8 +125,15 @@ type Step struct {
 	Script string `json:"script,omitempty"`
 }
 
-// A sidecar has the same data structure as a Step, consisting of a Container, and optional Script.
-type Sidecar = Step
+// Sidecar has nearly the same data structure as Step, consisting of a Container and an optional Script, but does not have the ability to timeout.
+type Sidecar struct {
+	corev1.Container `json:",inline"`
+
+	// Script is the contents of an executable file to execute.
+	//
+	// If Script is not empty, the Step cannot have an Command or Args.
+	Script string `json:"script,omitempty"`
+}
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // TaskList contains a list of Task
diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go
index 0017ae23937..cc27813f798 100644
--- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go
+++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go
@@ -1067,6 +1067,23 @@ func (in *ResultRef) DeepCopy() *ResultRef {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *Sidecar) DeepCopyInto(out *Sidecar) {
+	*out = *in
+	in.Container.DeepCopyInto(&out.Container)
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Sidecar.
+func (in *Sidecar) DeepCopy() *Sidecar {
+	if in == nil {
+		return nil
+	}
+	out := new(Sidecar)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *SidecarState) DeepCopyInto(out *SidecarState) {
 	*out = *in
@@ -1611,7 +1628,7 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) {
 	}
 	if in.Sidecars != nil {
 		in, out := &in.Sidecars, &out.Sidecars
-		*out = make([]Step, len(*in))
+		*out = make([]Sidecar, len(*in))
 		for i := range *in {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
diff --git a/pkg/pod/script.go b/pkg/pod/script.go
index f77c68e0bac..343a7bea00d 100644
--- a/pkg/pod/script.go
+++ b/pkg/pod/script.go
@@ -62,7 +62,17 @@ func convertScripts(shellImage string, steps []v1beta1.Step, sidecars []v1beta1.
 	}
 
 	convertedStepContainers := convertListOfSteps(steps, &placeScriptsInit, &placeScripts, "script")
-	sidecarContainers := convertListOfSteps(sidecars, &placeScriptsInit, &placeScripts, "sidecar-script")
+	// As the Sidecar type will divert slightly from the Step type in https://github.com/tektoncd/pipeline/issues/1690 and https://github.com/tektoncd/pipeline/pull/3013,
+	// while convertListOfSteps operates on overlapping fields, Sidecar is converted into Step by copying the overlapping fields between the types to avoid code duplication
+	sideCarSteps := []v1beta1.Step{}
+	for _, step := range sidecars {
+		sidecarStep := v1beta1.Step{
+			step.Container,
+			step.Script,
+		}
+		sideCarSteps = append(sideCarSteps, sidecarStep)
+	}
+	sidecarContainers := convertListOfSteps(sideCarSteps, &placeScriptsInit, &placeScripts, "sidecar-script")
 
 	if placeScripts {
 		return &placeScriptsInit, convertedStepContainers, sidecarContainers
diff --git a/pkg/pod/script_test.go b/pkg/pod/script_test.go
index 065de4cfa74..54f95faa336 100644
--- a/pkg/pod/script_test.go
+++ b/pkg/pod/script_test.go
@@ -35,7 +35,7 @@ func TestConvertScripts_NothingToConvert_EmptySidecars(t *testing.T) {
 		Container: corev1.Container{
 			Image: "step-2",
 		},
-	}}, []v1alpha1.Step{})
+	}}, []v1alpha1.Sidecar{})
 	want := []corev1.Container{{
 		Image: "step-1",
 	}, {
@@ -89,7 +89,7 @@ func TestConvertScripts_NothingToConvert_WithSidecar(t *testing.T) {
 		Container: corev1.Container{
 			Image: "step-2",
 		},
-	}}, []v1alpha1.Step{{
+	}}, []v1alpha1.Sidecar{}{{
 		Container: corev1.Container{
 			Image: "sidecar-1",
 		},
@@ -153,7 +153,7 @@ script-3`,
 			VolumeMounts: preExistingVolumeMounts,
 			Args:         []string{"my", "args"},
 		},
-	}}, []v1alpha1.Step{})
+	}}, []v1alpha1.Sidecar{})
 	wantInit := &corev1.Container{
 		Name:    "place-scripts",
 		Image:   images.ShellImage,
@@ -241,7 +241,7 @@ script-3`,
 			VolumeMounts: preExistingVolumeMounts,
 			Args:         []string{"my", "args"},
 		},
-	}}, []v1alpha1.Step{{
+	}}, []v1alpha1.Sidecar{}{{
 		Script: `#!/bin/sh
 sidecar-1`,
 		Container: corev1.Container{Image: "sidecar-1"},