Make ResourceRef in PipelineResourceBinding consistent with TaskRun

In TaskRun, TaskRef can be nil, which was not the case for PipelineResourceBinding (and ResourceRef). This fixes that by making the two consistent. Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
tektoncd · Oct 30, 2019 · 7ba0cd7 · 7ba0cd7
1 parent ee806d2
commit 7ba0cd7
Show file tree

Hide file tree

Showing 18 changed files with 125 additions and 90 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go
@@ -149,7 +149,6 @@ type PipelineResource struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	// Spec holds the desired state of the PipelineResource from the client
-	// +optional
 	Spec PipelineResourceSpec `json:"spec,omitempty"`
 	// Status communicates the observed state of the PipelineResource from the controller
 	// +optional
@@ -163,10 +162,11 @@ type PipelineResourceBinding struct {
 	Name string `json:"name,omitempty"`
 	// ResourceRef is a reference to the instance of the actual PipelineResource
 	// that should be used
-	ResourceRef PipelineResourceRef `json:"resourceRef,omitempty"`
 	// +optional
+	ResourceRef *PipelineResourceRef `json:"resourceRef,omitempty"`
 	// ResourceSpec is specification of a resource that should be created and
 	// consumed by the task
+	// +optional
 	ResourceSpec *PipelineResourceSpec `json:"resourceSpec,omitempty"`
 }
 

diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go
@@ -101,11 +101,11 @@ func validatePipelineResources(ctx context.Context, resources []TaskResourceBind
 		}
 		encountered[name] = struct{}{}
 		// Check that both resource ref and resource Spec are not present
-		if r.ResourceRef.Name != "" && r.ResourceSpec != nil {
+		if r.ResourceRef != nil && r.ResourceSpec != nil {
 			return apis.ErrDisallowedFields(fmt.Sprintf("%s.ResourceRef", path), fmt.Sprintf("%s.ResourceSpec", path))
 		}
 		// Check that one of resource ref and resource Spec is present
-		if r.ResourceRef.Name == "" && r.ResourceSpec == nil {
+		if (r.ResourceRef == nil || r.ResourceRef.Name == "") && r.ResourceSpec == nil {
 			return apis.ErrMissingField(fmt.Sprintf("%s.ResourceRef", path), fmt.Sprintf("%s.ResourceSpec", path))
 		}
 		if r.ResourceSpec != nil && r.ResourceSpec.Validate(ctx) != nil {

diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go
@@ -173,7 +173,7 @@ func TestInput_Validate(t *testing.T) {
 		}},
 		Resources: []v1alpha1.TaskResourceBinding{{
 			PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-				ResourceRef: v1alpha1.PipelineResourceRef{
+				ResourceRef: &v1alpha1.PipelineResourceRef{
 					Name: "testresource",
 				},
 				Name: "workspace",
@@ -195,14 +195,14 @@ func TestInput_Invalidate(t *testing.T) {
 		inputs: v1alpha1.TaskRunInputs{
 			Resources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource1",
 					},
 					Name: "workspace",
 				},
 			}, {
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource2",
 					},
 					Name: "workspace",
@@ -215,7 +215,7 @@ func TestInput_Invalidate(t *testing.T) {
 		inputs: v1alpha1.TaskRunInputs{
 			Resources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource",
 					},
 					Name: "resource",
@@ -235,7 +235,7 @@ func TestInput_Invalidate(t *testing.T) {
 		inputs: v1alpha1.TaskRunInputs{
 			Resources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource",
 					},
 					ResourceSpec: &v1alpha1.PipelineResourceSpec{
@@ -284,7 +284,7 @@ func TestOutput_Validate(t *testing.T) {
 	i := v1alpha1.TaskRunOutputs{
 		Resources: []v1alpha1.TaskResourceBinding{{
 			PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-				ResourceRef: v1alpha1.PipelineResourceRef{
+				ResourceRef: &v1alpha1.PipelineResourceRef{
 					Name: "testresource",
 				},
 				Name: "someimage",
@@ -305,14 +305,14 @@ func TestOutput_Invalidate(t *testing.T) {
 		outputs: v1alpha1.TaskRunOutputs{
 			Resources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource1",
 					},
 					Name: "workspace",
 				},
 			}, {
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{
+					ResourceRef: &v1alpha1.PipelineResourceRef{
 						Name: "testresource2",
 					},
 					Name: "workspace",

diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go
@@ -180,7 +180,7 @@ func (rcc *ResolvedConditionCheck) ToTaskResourceBindings() []v1alpha1.TaskResou
 			},
 		}
 		if r.SelfLink != "" {
-			tr.ResourceRef = v1alpha1.PipelineResourceRef{
+			tr.ResourceRef = &v1alpha1.PipelineResourceRef{
 				Name:       r.Name,
 				APIVersion: r.APIVersion,
 			}

diff --git a/pkg/reconciler/pipelinerun/resources/input_output_steps.go b/pkg/reconciler/pipelinerun/resources/input_output_steps.go
@@ -36,7 +36,7 @@ func GetOutputSteps(outputs map[string]*v1alpha1.PipelineResource, taskName, sto
 		// SelfLink is being checked there to determine if this PipelineResource is an instance that
 		// exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec
 		if outputResource.SelfLink != "" {
-			taskOutputResource.ResourceRef = v1alpha1.PipelineResourceRef{
+			taskOutputResource.ResourceRef = &v1alpha1.PipelineResourceRef{
 				Name:       outputResource.Name,
 				APIVersion: outputResource.APIVersion,
 			}
@@ -66,7 +66,7 @@ func GetInputSteps(inputs map[string]*v1alpha1.PipelineResource, pt *v1alpha1.Pi
 		// SelfLink is being checked there to determine if this PipelineResource is an instance that
 		// exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec
 		if inputResource.SelfLink != "" {
-			taskInputResource.ResourceRef = v1alpha1.PipelineResourceRef{
+			taskInputResource.ResourceRef = &v1alpha1.PipelineResourceRef{
 				Name:       inputResource.Name,
 				APIVersion: inputResource.APIVersion,
 			}

diff --git a/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go b/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go
@@ -62,7 +62,7 @@ func TestGetOutputSteps(t *testing.T) {
 		expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{
 			PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 				Name:        "test-output",
-				ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+				ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 			},
 			Paths: []string{"/pvc/test-taskname/test-output"},
 		}},
@@ -76,13 +76,13 @@ func TestGetOutputSteps(t *testing.T) {
 		expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{
 			PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 				Name:        "test-output",
-				ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+				ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 			},
 			Paths: []string{"/pvc/test-multiple-outputs/test-output"},
 		}, {
 			PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 				Name:        "test-output-2",
-				ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource2"},
+				ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource2"},
 			},
 			Paths: []string{"/pvc/test-multiple-outputs/test-output-2"},
 		}},
@@ -166,7 +166,7 @@ func TestGetInputSteps(t *testing.T) {
 			},
 			expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+					ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 					Name:        "test-input",
 				},
 				Paths: []string{"/pvc/prev-task-1/test-input"},
@@ -176,7 +176,7 @@ func TestGetInputSteps(t *testing.T) {
 			inputs: map[string]*v1alpha1.PipelineResource{"test-input": r1},
 			expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+					ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 					Name:        "test-input",
 				},
 			}},
@@ -196,7 +196,7 @@ func TestGetInputSteps(t *testing.T) {
 			},
 			expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{
 				PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-					ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+					ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 					Name:        "test-input",
 				},
 				Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"},
@@ -305,13 +305,13 @@ func TestWrapSteps(t *testing.T) {
 
 	expectedtaskInputResources := []v1alpha1.TaskResourceBinding{{
 		PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-			ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+			ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 			Name:        "test-input",
 		},
 		Paths: []string{"/pvc/prev-task/test-input"},
 	}, {
 		PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-			ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+			ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 			Name:        "test-input-2",
 		},
 	}, {
@@ -322,7 +322,7 @@ func TestWrapSteps(t *testing.T) {
 	}}
 	expectedtaskOuputResources := []v1alpha1.TaskResourceBinding{{
 		PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
-			ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"},
+			ResourceRef: &v1alpha1.PipelineResourceRef{Name: "resource1"},
 			Name:        "test-output",
 		},
 		Paths: []string{"/pvc/test-task/test-output"},

diff --git a/pkg/reconciler/taskrun/resources/image_exporter_test.go b/pkg/reconciler/taskrun/resources/image_exporter_test.go
@@ -70,7 +70,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
 					Resources: []v1alpha1.TaskResourceBinding{{
 						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 							Name: "source-image",
-							ResourceRef: v1alpha1.PipelineResourceRef{
+							ResourceRef: &v1alpha1.PipelineResourceRef{
 								Name: "source-image-1",
 							},
 						},
@@ -80,7 +80,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
 					Resources: []v1alpha1.TaskResourceBinding{{
 						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 							Name: "source-image",
-							ResourceRef: v1alpha1.PipelineResourceRef{
+							ResourceRef: &v1alpha1.PipelineResourceRef{
 								Name: "source-image-1",
 							},
 						},
@@ -137,7 +137,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
 					Resources: []v1alpha1.TaskResourceBinding{{
 						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 							Name: "source-image",
-							ResourceRef: v1alpha1.PipelineResourceRef{
+							ResourceRef: &v1alpha1.PipelineResourceRef{
 								Name: "source-image-1",
 							},
 						},
@@ -147,7 +147,7 @@ func TestAddOutputImageDigestExporter(t *testing.T) {
 					Resources: []v1alpha1.TaskResourceBinding{{
 						PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
 							Name: "source-image",
-							ResourceRef: v1alpha1.PipelineResourceRef{
+							ResourceRef: &v1alpha1.PipelineResourceRef{
 								Name: "source-image-1",
 							},
 						},