Add parameter variable substitution support for conditionals

A Condition's `spec.Check` can refer to its declared parameters using
the ${params.param-name} syntax just like in a Task. Values can be
provided via the pipelinerun's parameters or via default values in the
condition or in the PipelineTaskCondition

Signed-off-by: Dibyo Mukherjee <[email protected]>

docs/conditions.md

@@ -8,6 +8,7 @@ This document defines `Conditions` and their capabilities.
 
 - [Syntax](#syntax)
   - [Check](#check)
+  - [Parameters](#parameters)
 - [Examples](#examples)
 
 ## Syntax
@@ -32,11 +33,34 @@ following fields:
 ### Check
 
 The `check` field is required. You define a single check to define the body of a `Condition`. The 
-check must specify a container image that adheres to the [container contract](./container-contract.md). The container image 
-runs till completion. The container must exit successfully i.e. with an exit code 0 for the 
-condition evaluation to be successful. All other exit codes are considered to be a condition check
+check must specify a container image that adheres to the [container contract](./container-contract.md). 
+The container image runs till completion. The container must exit successfully i.e. with an exit code 0 
+for the condition evaluation to be successful. All other exit codes are considered to be a condition check
 failure.
 
+### Parameters
+
+A Condition can declare parameters that must be supplied to it during a PipelineRun. Sub-fields
+within the check field can access the parameter values using the templating syntax:
+
+```yaml
+spec:
+  parameters:
+    - name: image
+      default: ubuntu
+  check:
+    image: ${params.image}
+```
+
+Parameters name are limited to alpha-numeric characters, `-` and `_` and can
+only start with alpha characters and `_`. For example, `fooIs-Bar_` is a valid
+parameter name, `barIsBa$` or `0banana` are not.
+
+Each declared parameter has a type field, assumed to be string if not provided by the user. 
+The other possible type is array — useful,checking a pushed branch name doesn't match any of 
+multiple protected branch names. When the actual parameter value is supplied, its parsed type 
+is validated against the type field.
+
 ## Examples
 
 For complete examples, see

examples/pipelineruns/conditional-pipelinerun.yaml

@@ -1,12 +1,17 @@
 apiVersion: tekton.dev/v1alpha1
 kind: Condition
 metadata:
-  name: always-true
+  name: strings-equal
 spec:
+ params:
+   - name: "x"
+     type: string
+   - name: "y"
+     type: string
  check:
     image: alpine
     command: ["/bin/sh"]
-    args: ['-c', 'exit 0']
+    args: ['-c', 'echo "Comparing ${params.x} and ${params.y}" && [ "${params.x}" == "${params.y}" ]']
 ---
 apiVersion: tekton.dev/v1alpha1
 kind: PipelineResource
@@ -43,12 +48,22 @@ spec:
   resources:
     - name: source-repo
       type: git
+  params:
+    - name: "x"
+      default: "abc"
+    - name: "y"
+      default: "abc"
   tasks:
     - name: list-files-1
       taskRef:
         name: list-files
       conditions:
-        - conditionRef: "always-true"
+        - conditionRef: "strings-equal"
+          params:
+            - name: "x"
+              value: "${params.x}"
+            - name: "y"
+              value: "${params.y}"
       resources:
         inputs:
           - name: workspace

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go

@@ -633,10 +633,13 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
 		Spec: v1alpha1.TaskRunSpec{
 			TaskSpec:       rcc.ConditionToTaskSpec(),
 			ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name),
-			Timeout:        getTaskRunTimeout(pr),
-			NodeSelector:   pr.Spec.NodeSelector,
-			Tolerations:    pr.Spec.Tolerations,
-			Affinity:       pr.Spec.Affinity,
+			Inputs: v1alpha1.TaskRunInputs{
+				Params: rcc.PipelineTaskCondition.Params,
+			},
+			Timeout:      getTaskRunTimeout(pr),
+			NodeSelector: pr.Spec.NodeSelector,
+			Tolerations:  pr.Spec.Tolerations,
+			Affinity:     pr.Spec.Affinity,
 		}}
 
 	cctr, err := c.PipelineClientSet.TektonV1alpha1().TaskRuns(pr.Namespace).Create(tr)

pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go

@@ -60,14 +60,19 @@ func ApplyReplacements(p *v1alpha1.Pipeline, replacements map[string]string, arr
 	tasks := p.Spec.Tasks
 
 	for i := range tasks {
-		params := tasks[i].Params
-
-		for j := range params {
-			params[j].Value.ApplyReplacements(replacements, arrayReplacements)
+		tasks[i].Params = replaceParamValues(tasks[i].Params, replacements, arrayReplacements)
+		for j := range tasks[i].Conditions {
+			c := tasks[i].Conditions[j]
+			c.Params = replaceParamValues(c.Params, replacements, arrayReplacements)
 		}
-
-		tasks[i].Params = params
 	}
 
 	return p
 }
+
+func replaceParamValues(params []v1alpha1.Param, stringReplacements map[string]string, arrayReplacements map[string][]string) []v1alpha1.Param {
+	for i := range params {
+		params[i].Value.ApplyReplacements(stringReplacements, arrayReplacements)
+	}
+	return params
+}

pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	tb "github.com/tektoncd/pipeline/test/builder"
 )
@@ -69,6 +70,49 @@ func TestApplyParameters(t *testing.T) {
 				tb.PipelineTask("first-task-1", "first-task",
 					tb.PipelineTaskParam("first-task-first-param", "${input.workspace.default-value}"),
 				))),
+	}, {
+		name: "single parameter in task condition",
+		original: tb.Pipeline("test-pipeline", "foo",
+			tb.PipelineSpec(
+				tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
+				tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString),
+				tb.PipelineTask("first-task-1", "first-task",
+					tb.PipelineTaskCondition("task-condition",
+						tb.PipelineTaskConditionParam("cond-first-param", "${params.first-param}"),
+						tb.PipelineTaskConditionParam("cond-second-param", "${params.second-param}"),
+					),
+				))),
+		run: tb.PipelineRun("test-pipeline-run", "foo",
+			tb.PipelineRunSpec("test-pipeline",
+				tb.PipelineRunParam("second-param", "second-value"))),
+		expected: tb.Pipeline("test-pipeline", "foo",
+			tb.PipelineSpec(
+				tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
+				tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString),
+				tb.PipelineTask("first-task-1", "first-task",
+					tb.PipelineTaskCondition("task-condition",
+						tb.PipelineTaskConditionParam("cond-first-param", "default-value"),
+						tb.PipelineTaskConditionParam("cond-second-param", "second-value"),
+					),
+				))),
+	}, {
+		name: "pipeline parameter nested inside condition parameter",
+		original: tb.Pipeline("test-pipeline", "foo",
+			tb.PipelineSpec(
+				tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
+				tb.PipelineTask("first-task-1", "first-task",
+					tb.PipelineTaskCondition("cond",
+						tb.PipelineTaskConditionParam("cond-first-param", "${params.first-param}")),
+				))),
+		run: tb.PipelineRun("test-pipeline-run", "foo",
+			tb.PipelineRunSpec("test-pipeline")),
+		expected: tb.Pipeline("test-pipeline", "foo",
+			tb.PipelineSpec(
+				tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
+				tb.PipelineTask("first-task-1", "first-task",
+					tb.PipelineTaskCondition("cond",
+						tb.PipelineTaskConditionParam("cond-first-param", "default-value")),
+				))),
 	}, {
 		name: "single array parameter",
 		original: tb.Pipeline("test-pipeline", "foo",

pkg/reconciler/v1alpha1/pipelinerun/resources/conditionresolution.go

@@ -18,9 +18,12 @@
 package resources
 
 import (
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
+	"github.com/tektoncd/pipeline/pkg/templating"
 )
 
 const (
@@ -36,9 +39,10 @@ type GetCondition func(string) (*v1alpha1.Condition, error)
 // exists. ConditionCheck can be nil to represent there being no ConditionCheck (i.e the condition
 // has not been evaluated).
 type ResolvedConditionCheck struct {
-	ConditionCheckName string
-	Condition          *v1alpha1.Condition
-	ConditionCheck     *v1alpha1.ConditionCheck
+	ConditionCheckName    string
+	Condition             *v1alpha1.Condition
+	ConditionCheck        *v1alpha1.ConditionCheck
+	PipelineTaskCondition *v1alpha1.PipelineTaskCondition
 }
 
 // TaskConditionCheckState is a slice of ResolvedConditionCheck the represents the current execution
@@ -95,11 +99,22 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() *v1alpha1.TaskSpec {
 		t.Inputs = &v1alpha1.Inputs{
 			Params: rcc.Condition.Spec.Params,
 		}
+		// convert param strings of type ${params.x} to ${inputs.params.x}
+		// in order to apply taskrun substitution
+		convertParamTemplates(&t.Steps[0], rcc.Condition.Spec.Params)
 	}
-
 	return t
 }
 
+// Replaces all instances of ${params.x} in the container to ${inputs.params.x} for each param name
+func convertParamTemplates(container *corev1.Container, params []v1alpha1.ParamSpec) {
+	replacements := make(map[string]string)
+	for _, p := range params {
+		replacements[fmt.Sprintf("params.%s", p.Name)] = fmt.Sprintf("${inputs.params.%s}", p.Name)
+		templating.ApplyContainerReplacements(container, replacements, map[string][]string{})
+	}
+}
+
 // NewConditionCheck status creates a ConditionCheckStatus from a ConditionCheck
 func (rcc *ResolvedConditionCheck) NewConditionCheckStatus() *v1alpha1.ConditionCheckStatus {
 	var checkStep corev1.ContainerState

pkg/reconciler/v1alpha1/pipelinerun/resources/conditionresolution_test.go

@@ -18,6 +18,8 @@
 package resources
 
 import (
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -26,8 +28,6 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	tb "github.com/tektoncd/pipeline/test/builder"
-
-	"testing"
 )
 
 var c = tb.Condition("conditionname", "foo")
@@ -214,19 +214,25 @@ func TestResolvedConditionCheck_ConditionToTaskSpec(t *testing.T) {
 	}, {
 		name: "with-input-params",
 		cond: tb.Condition("bar", "foo", tb.ConditionSpec(
-			tb.ConditionSpecCheck("", "ubuntu"),
-			tb.ConditionParamSpec("abc", v1alpha1.ParamTypeString),
+			tb.ConditionSpecCheck("${params.name}", "${params.img}",
+				tb.WorkingDir("${params.not.replaced}")),
+			tb.ConditionParamSpec("name", v1alpha1.ParamTypeString),
+			tb.ConditionParamSpec("img", v1alpha1.ParamTypeString),
 		)),
 		want: v1alpha1.TaskSpec{
 			Inputs: &v1alpha1.Inputs{
 				Params: []v1alpha1.ParamSpec{{
-					Name: "abc",
+					Name: "name",
+					Type: "string",
+				}, {
+					Name: "img",
 					Type: "string",
 				}},
 			},
 			Steps: []corev1.Container{{
-				Name:  "condition-check-bar",
-				Image: "ubuntu",
+				Name:       "${inputs.params.name}",
+				Image:      "${inputs.params.img}",
+				WorkingDir: "${params.not.replaced}",
 			}},
 		},
 	}}

pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go

@@ -450,8 +450,8 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask,
 	taskRunStatus map[string]*v1alpha1.PipelineRunTaskRunStatus,
 	taskRunName string, getTaskRun resources.GetTaskRun, getCondition GetCondition) ([]*ResolvedConditionCheck, error) {
 	rcc := []*ResolvedConditionCheck{}
-	for j := range pt.Conditions {
-		cName := pt.Conditions[j].ConditionRef
+	for _, ptc := range pt.Conditions {
+		cName := ptc.ConditionRef
 		c, err := getCondition(cName)
 		if err != nil {
 			return nil, &ConditionNotFoundError{
@@ -468,9 +468,10 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask,
 		}
 
 		rcc = append(rcc, &ResolvedConditionCheck{
-			Condition:          c,
-			ConditionCheckName: conditionCheckName,
-			ConditionCheck:     v1alpha1.NewConditionCheck(cctr),
+			Condition:             c,
+			ConditionCheckName:    conditionCheckName,
+			ConditionCheck:        v1alpha1.NewConditionCheck(cctr),
+			PipelineTaskCondition: &ptc,
 		})
 	}
 	return rcc, nil

pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go

@@ -1492,12 +1492,15 @@ func TestResolveConditionChecks(t *testing.T) {
 		},
 		Spec: v1alpha1.TaskRunSpec{},
 	}
+
+	ptc := v1alpha1.PipelineTaskCondition{
+		ConditionRef: "always-true",
+	}
+
 	pts := []v1alpha1.PipelineTask{{
-		Name:    "mytask1",
-		TaskRef: v1alpha1.TaskRef{Name: "task"},
-		Conditions: []v1alpha1.PipelineTaskCondition{{
-			ConditionRef: "always-true",
-		}},
+		Name:       "mytask1",
+		TaskRef:    v1alpha1.TaskRef{Name: "task"},
+		Conditions: []v1alpha1.PipelineTaskCondition{ptc},
 	}}
 	providedResources := map[string]v1alpha1.PipelineResourceRef{}
 
@@ -1529,9 +1532,10 @@ func TestResolveConditionChecks(t *testing.T) {
 				return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name)
 			},
 			expectedConditionCheck: TaskConditionCheckState{{
-				ConditionCheckName: "pipelinerun-mytask1-9l9zj-always-true-mz4c7",
-				Condition:          &condition,
-				ConditionCheck:     v1alpha1.NewConditionCheck(cc),
+				ConditionCheckName:    "pipelinerun-mytask1-9l9zj-always-true-mz4c7",
+				Condition:             &condition,
+				ConditionCheck:        v1alpha1.NewConditionCheck(cc),
+				PipelineTaskCondition: &ptc,
 			}},
 		},
 		{
@@ -1545,8 +1549,9 @@ func TestResolveConditionChecks(t *testing.T) {
 				return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name)
 			},
 			expectedConditionCheck: TaskConditionCheckState{{
-				ConditionCheckName: "pipelinerun-mytask1-mssqb-always-true-78c5n",
-				Condition:          &condition,
+				ConditionCheckName:    "pipelinerun-mytask1-mssqb-always-true-78c5n",
+				Condition:             &condition,
+				PipelineTaskCondition: &ptc,
 			}},
 		},
 	}
@@ -1626,12 +1631,14 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) {
 		Spec: v1alpha1.TaskRunSpec{},
 	}
 
+	ptc := v1alpha1.PipelineTaskCondition{
+		ConditionRef: "always-true",
+	}
+
 	pts := []v1alpha1.PipelineTask{{
-		Name:    "mytask1",
-		TaskRef: v1alpha1.TaskRef{Name: "task"},
-		Conditions: []v1alpha1.PipelineTaskCondition{{
-			ConditionRef: "always-true",
-		}},
+		Name:       "mytask1",
+		TaskRef:    v1alpha1.TaskRef{Name: "task"},
+		Conditions: []v1alpha1.PipelineTaskCondition{ptc},
 	}}
 	providedResources := map[string]v1alpha1.PipelineResourceRef{}
 
@@ -1674,9 +1681,10 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) {
 		t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err)
 	}
 	expectedConditionChecks := TaskConditionCheckState{{
-		ConditionCheckName: ccName,
-		Condition:          &condition,
-		ConditionCheck:     v1alpha1.NewConditionCheck(cc),
+		ConditionCheckName:    ccName,
+		Condition:             &condition,
+		ConditionCheck:        v1alpha1.NewConditionCheck(cc),
+		PipelineTaskCondition: &ptc,
 	}}
 
 	if d := cmp.Diff(pipelineState[0].ResolvedConditionChecks, expectedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" {