From be9cc3eed7ce304d00c44cb3c8f8c4efa554b35b Mon Sep 17 00:00:00 2001
From: Dibyo Mukherjee <dibyo@google.com>
Date: Tue, 3 Sep 2019 22:27:02 -0400
Subject: [PATCH] Support Condition only resources in PipelineTask

This commit fixes the Pipeline validation logic to account
for resources that are declared in a `PipelineTaskCondition` but
not in a `PipelineTask`. Previously, each resource used in a
`PipelineTaskCondition` also had to be declared in the `PipelineTask`.

Fixes #1263

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
---
 .../pipeline/v1alpha1/pipeline_validation.go  |  6 ++++++
 .../v1alpha1/pipeline_validation_test.go      | 21 ++++++++++++++++++-
 test/builder/pipeline.go                      | 15 ++++++++++++-
 test/builder/pipeline_test.go                 |  5 +++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go
index a7d2b3ad434..7725fdc6cbd 100644
--- a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go
+++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go
@@ -46,6 +46,12 @@ func validateDeclaredResources(ps *PipelineSpec) error {
 				required = append(required, output.Resource)
 			}
 		}
+
+		for _, condition := range t.Conditions {
+			for _, cr := range condition.Resources {
+				required = append(required, cr.Resource)
+			}
+		}
 	}
 
 	provided := make([]string, 0, len(ps.Resources))
diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
index ad85667ea78..0b8421fd94e 100644
--- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
+++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
@@ -110,11 +110,22 @@ func TestPipelineSpec_Validate(t *testing.T) {
 			tb.PipelineDeclaredResource("wonderful-resource", v1alpha1.PipelineResourceTypeImage),
 			tb.PipelineTask("bar", "bar-task",
 				tb.PipelineTaskInputResource("some-workspace", "great-resource"),
-				tb.PipelineTaskOutputResource("some-image", "wonderful-resource")),
+				tb.PipelineTaskOutputResource("some-image", "wonderful-resource"),
+				tb.PipelineTaskCondition("some-condition",
+					tb.PipelineTaskConditionResource("some-workspace", "great-resource"))),
 			tb.PipelineTask("foo", "foo-task",
 				tb.PipelineTaskInputResource("wow-image", "wonderful-resource", tb.From("bar"))),
 		)),
 		failureExpected: false,
+	}, {
+		name: "valid condition only resource",
+		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
+			tb.PipelineDeclaredResource("great-resource", v1alpha1.PipelineResourceTypeGit),
+			tb.PipelineTask("bar", "bar-task",
+				tb.PipelineTaskCondition("some-condition",
+					tb.PipelineTaskConditionResource("some-workspace", "great-resource"))),
+		)),
+		failureExpected: false,
 	}, {
 		name: "valid parameter variables",
 		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
@@ -221,6 +232,14 @@ func TestPipelineSpec_Validate(t *testing.T) {
 				tb.PipelineTaskOutputResource("the-magic-resource", "great-resource")),
 		)),
 		failureExpected: true,
+	}, {
+		name: "invalid condition only resource",
+		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
+			tb.PipelineTask("bar", "bar-task",
+				tb.PipelineTaskCondition("some-condition",
+					tb.PipelineTaskConditionResource("some-workspace", "missing-resource"))),
+		)),
+		failureExpected: true,
 	}, {
 		name: "from resource isn't output by task",
 		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go
index 1209ca70723..9a3750409eb 100644
--- a/test/builder/pipeline.go
+++ b/test/builder/pipeline.go
@@ -232,7 +232,7 @@ func PipelineTaskCondition(conditionRef string, ops ...PipelineTaskConditionOp)
 	}
 }
 
-// PipelineTaskCondition adds a parameter to a PipelineTaskCondition
+// PipelineTaskConditionParam adds a parameter to a PipelineTaskCondition
 func PipelineTaskConditionParam(name, val string) PipelineTaskConditionOp {
 	return func(condition *v1alpha1.PipelineTaskCondition) {
 		if condition.Params == nil {
@@ -245,6 +245,19 @@ func PipelineTaskConditionParam(name, val string) PipelineTaskConditionOp {
 	}
 }
 
+// PipelineTaskConditionResource adds a resource to a PipelineTaskCondition
+func PipelineTaskConditionResource(name, resource string) PipelineTaskConditionOp {
+	return func(condition *v1alpha1.PipelineTaskCondition) {
+		if condition.Resources == nil {
+			condition.Resources = []v1alpha1.PipelineConditionResource{}
+		}
+		condition.Resources = append(condition.Resources, v1alpha1.PipelineConditionResource{
+			Name:     name,
+			Resource: resource,
+		})
+	}
+}
+
 // PipelineRun creates a PipelineRun with default values.
 // Any number of PipelineRun modifier can be passed to transform it.
 func PipelineRun(name, namespace string, ops ...PipelineRunOp) *v1alpha1.PipelineRun {
diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go
index 434487ef107..e57de4f656c 100644
--- a/test/builder/pipeline_test.go
+++ b/test/builder/pipeline_test.go
@@ -41,6 +41,7 @@ func TestPipeline(t *testing.T) {
 			tb.PipelineTaskParam("arrayparam", "array", "value"),
 			tb.PipelineTaskCondition("some-condition-ref",
 				tb.PipelineTaskConditionParam("param-name", "param-value"),
+				tb.PipelineTaskConditionResource("some-resource", "my-only-git-resource"),
 			),
 		),
 		tb.PipelineTask("bar", "chocolate",
@@ -92,6 +93,10 @@ func TestPipeline(t *testing.T) {
 							StringVal: "param-value",
 						},
 					}},
+					Resources: []v1alpha1.PipelineConditionResource{{
+						Name: "some-resource",
+						Resource: "my-only-git-resource",
+					}},
 				}},
 			}, {
 				Name:    "bar",