diff --git a/docs/conditions.md b/docs/conditions.md index aa1e14a9b1c..a6145a36a57 100644 --- a/docs/conditions.md +++ b/docs/conditions.md @@ -6,6 +6,8 @@ weight: 11 --> # Conditions +**Note:** `Conditions` are deprecated, use [WhenExpressions](pipelines.md#guard-task-execution-using-whenexpressions) instead. + - [Overview](#overview) - [Configuring a `Condition`](#configuring-a-condition) - [Specifying the condition `check`](#specifying-the-condition-check) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 46d14d602c0..3c8f4f92702 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -457,6 +457,26 @@ False|PipelineRunTimeout|Yes|The `PipelineRun` timed out. When a `PipelineRun` changes status, [events](events.md#pipelineruns) are triggered accordingly. +When a `PipelineRun` has `Tasks` with [WhenExpressions](pipelines.md#guard-task-execution-using-whenexpressions): +- If the `WhenExpressions` evaluate to `true`, the `Task` is executed and the `TaskRun` will be listed in the `Task Runs` section of the `status` of the `PipelineRun`. +- If the `WhenExpressions` evaluate to `false`, the `Task` is skipped and it is listed in the `Skipped Tasks` section of the `status` of the `PipelineRun`. + +```yaml +Conditions: + Last Transition Time: 2020-08-27T15:07:34Z + Message: Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 1 + Reason: Completed + Status: True + Type: Succeeded +Skipped Tasks: + Name: skip-this-task +Task Runs: + pipelinerun-to-skip-task-run-this-task-r2djj: + Pipeline Task Name: run-this-task + Status: + ... +``` + ## Cancelling a `PipelineRun` To cancel a `PipelineRun` that's currently executing, update its definition diff --git a/docs/pipelines.md b/docs/pipelines.md index d4b32bce308..2c31b59dd67 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -15,6 +15,7 @@ weight: 3 - [Using the `from` parameter](#using-the-from-parameter) - [Using the `runAfter` parameter](#using-the-runafter-parameter) - [Using the `retries` parameter](#using-the-retries-parameter) + - [Guard `Task` execution using `When Expressions`](#guard-task-execution-using-whenexpressions) - [Guard `Task` execution using `Conditions`](#guard-task-execution-using-conditions) - [Configuring the failure timeout](#configuring-the-failure-timeout) - [Using `Results`](#using-results) @@ -316,8 +317,56 @@ tasks: name: build-push ``` +### Guard `Task` execution using `WhenExpressions` + +To run a `Task` only when certain conditions are met, it is possible to _guard_ task execution using the `when` field. The `when` field allows you to list a series of references to `WhenExpressions`. + +The components of `WhenExpressions` are `Input`, `Operator` and `Values`: +- `Input` is the input for the `WhenExpression` which can be static inputs or variables ([`Parameters`](#specifying-parameters) or [`Results`](#using-results)). If the `Input` is not provided, it defaults to an empty string. +- `Operator` represents an `Input`'s relationship to a set of `Values`. A valid `Operator` must be provided, which can be either `in` or `notin`. +- `Values` is an array of string values. The `Values` array must be provided and be non-empty. It can contain static values or variables ([`Parameters`](#specifying-parameters) or [`Results`](#using-results)). + +The [`Parameters`](#specifying-parameters) are read from the `Pipeline` and [`Results`](#using-results) are read directly from previous [`Tasks`](#adding-tasks-to-the-pipeline). Using [`Results`](#using-results) in a `WhenExpression` in a guarded `Task` introduces a resource dependency on the previous `Task` that produced the `Result`. + +The declared `WhenExpressions` are evaluated before the `Task` is run. If all the `WhenExpressions` evaluate to `True`, the `Task` is run. If any of the `WhenExpressions` evaluate to `False`, the `Task` is not run and the `Task` is listed in the [`Skipped Tasks` section of the `PipelineRunStatus`](pipelineruns.md#monitoring-execution-status). + +In these examples, `create-readme-file` task will only be executed if the `path` parameter is `README.md` and `echo-file-exists` task will only be executed if the `status` result from `check-file` task is `exists`. + +```yaml +tasks: + - name: first-create-file + when: + - input: "$(params.path)" + operator: in + values: ["README.md"] + taskRef: + name: create-readme-file +--- +tasks: + - name: echo-file-exists + when: + - input: "$(tasks.check-file.results.status)" + operator: in + values: ["exists"] + taskRef: + name: echo-file-exists +``` + +For an end-to-end example, see [PipelineRun with WhenExpressions](../examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml). + +When `WhenExpressions` are specified in a `Task`, [`Conditions`](#guard-task-execution-using-conditions) should not be specified in the same `Task`. The `Pipeline` will be rejected as invalid if both `WhenExpressions` and `Conditions` are included. + +There are a lot of scenarios where `WhenExpressions` can be really useful. Some of these are: +- Checking if the name of a git branch matches +- Checking if the `Result` of a previous `Task` is as expected +- Checking if a git file has changed in the previous commits +- Checking if an image exists in the registry +- Checking if the name of a CI job matches + ### Guard `Task` execution using `Conditions` +**Note:** `Conditions` are deprecated, use [`WhenExpressions`](#guard-task-execution-using-whenexpressions) instead. + To run a `Task` only when certain conditions are met, it is possible to _guard_ task execution using the `conditions` field. The `conditions` field allows you to list a series of references to [`Condition`](./conditions.md) resources. The declared `Conditions` are run before the `Task` is run. diff --git a/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-optional-resources.yaml b/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-optional-resources.yaml index 48b3f855f6a..dac461f658b 100644 --- a/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-optional-resources.yaml +++ b/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-optional-resources.yaml @@ -1,3 +1,5 @@ +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-same-condition-refer.yaml b/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-same-condition-refer.yaml index 153e884c493..053f5454cdf 100644 --- a/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-same-condition-refer.yaml +++ b/examples/v1beta1/pipelineruns/conditional-pipelinerun-with-same-condition-refer.yaml @@ -1,3 +1,5 @@ +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/conditional-pipelinerun.yaml b/examples/v1beta1/pipelineruns/conditional-pipelinerun.yaml index 03b16e029f8..271998f3dd0 100644 --- a/examples/v1beta1/pipelineruns/conditional-pipelinerun.yaml +++ b/examples/v1beta1/pipelineruns/conditional-pipelinerun.yaml @@ -1,3 +1,5 @@ +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/demo-optional-resources.yaml b/examples/v1beta1/pipelineruns/demo-optional-resources.yaml index ea26fde0c9f..1710af07433 100644 --- a/examples/v1beta1/pipelineruns/demo-optional-resources.yaml +++ b/examples/v1beta1/pipelineruns/demo-optional-resources.yaml @@ -1,3 +1,5 @@ +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: @@ -13,7 +15,8 @@ spec: image: alpine script: 'test -f $(resources.git-repo.path)/$(params.path)' --- - +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/pipeline-result-conditions.yaml b/examples/v1beta1/pipelineruns/pipeline-result-conditions.yaml index f065964b493..650e1723802 100644 --- a/examples/v1beta1/pipelineruns/pipeline-result-conditions.yaml +++ b/examples/v1beta1/pipelineruns/pipeline-result-conditions.yaml @@ -1,3 +1,5 @@ +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-resourcespec.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-resourcespec.yaml index 85f4c7a7af4..93c5e78b20a 100644 --- a/examples/v1beta1/pipelineruns/pipelinerun-with-resourcespec.yaml +++ b/examples/v1beta1/pipelineruns/pipelinerun-with-resourcespec.yaml @@ -20,7 +20,8 @@ spec: - | ls -al $(resources.inputs.pipeline-git.path) --- - +# `Conditions` are deprecated, use `WhenExpressions` instead +# https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#guard-task-execution-using-whenexpressions apiVersion: tekton.dev/v1alpha1 kind: Condition metadata: diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml new file mode 100644 index 00000000000..a2a399f15c0 --- /dev/null +++ b/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml @@ -0,0 +1,99 @@ +apiVersion: tekton.dev/v1beta1 +kind: Pipeline +metadata: + name: guarded-pipeline +spec: + params: + - name: path + type: string + description: The path of the file to be created + workspaces: + - name: source + description: | + This workspace will receive the cloned git repo and be passed + to the next Task to create a file + tasks: + - name: create-file + when: + - input: "$(params.path)" + operator: in + values: ["README.md"] + workspaces: + - name: source + workspace: source + taskSpec: + workspaces: + - name: source + description: The workspace to create the readme file in + steps: + - name: write-new-stuff + image: ubuntu + script: 'touch $(workspaces.source.path)/README.md' + - name: check-file + params: + - name: path + value: "$(params.path)" + workspaces: + - name: source + workspace: source + runAfter: + - create-file + taskSpec: + params: + - name: path + workspaces: + - name: source + description: The workspace to check for the file + results: + - name: status + description: indicates whether the file exists or is missing + steps: + - name: check-file + image: alpine + script: | + if test -f $(workspaces.source.path)/$(params.path); then + printf exists | tee /tekton/results/status + else + printf missing | tee /tekton/results/status + fi + - name: echo-file-exists + when: + - input: "$(tasks.check-file.results.status)" + operator: in + values: ["exists"] + taskSpec: + steps: + - name: echo + image: ubuntu + script: 'echo file exists' + - name: task-should-be-skipped + when: + - input: "$(params.path)" + operator: notin + values: ["README.md"] + taskSpec: + steps: + - name: echo + image: ubuntu + script: exit 1 +--- +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + name: guarded-pr +spec: + serviceAccountName: 'default' + pipelineRef: + name: guarded-pipeline + params: + - name: path + value: README.md + workspaces: + - name: source + volumeClaimTemplate: + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi diff --git a/internal/builder/v1beta1/pipeline.go b/internal/builder/v1beta1/pipeline.go index 68a764a120c..4cfe6bde60c 100644 --- a/internal/builder/v1beta1/pipeline.go +++ b/internal/builder/v1beta1/pipeline.go @@ -24,6 +24,7 @@ import ( resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" ) @@ -332,6 +333,18 @@ func PipelineTaskConditionResource(name, resource string, from ...string) Pipeli } } +// PipelineTaskWhenExpression adds a WhenExpression with the specified input, operator and values +// which are used to determine whether the PipelineTask should be executed or skipped. +func PipelineTaskWhenExpression(input string, operator selection.Operator, values []string) PipelineTaskOp { + return func(pt *v1beta1.PipelineTask) { + pt.WhenExpressions = append(pt.WhenExpressions, v1beta1.WhenExpression{ + Input: input, + Operator: operator, + Values: values, + }) + } +} + // PipelineTaskWorkspaceBinding adds a workspace with the specified name, workspace and subpath on a PipelineTask. func PipelineTaskWorkspaceBinding(name, workspace, subPath string) PipelineTaskOp { return func(pt *v1beta1.PipelineTask) { diff --git a/internal/builder/v1beta1/pipeline_test.go b/internal/builder/v1beta1/pipeline_test.go index cc40e365b5b..3647ec36ad6 100644 --- a/internal/builder/v1beta1/pipeline_test.go +++ b/internal/builder/v1beta1/pipeline_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" @@ -54,6 +55,7 @@ func TestPipeline(t *testing.T) { tb.PipelineTaskOutputResource("some-image", "my-only-image-resource"), ), tb.PipelineTask("never-gonna", "give-you-up", + tb.PipelineTaskWhenExpression("foo", selection.In, []string{"foo", "bar"}), tb.RunAfter("foo"), tb.PipelineTaskTimeout(5*time.Second), ), @@ -133,10 +135,11 @@ func TestPipeline(t *testing.T) { }}, }, }, { - Name: "never-gonna", - TaskRef: &v1beta1.TaskRef{Name: "give-you-up"}, - RunAfter: []string{"foo"}, - Timeout: &metav1.Duration{Duration: 5 * time.Second}, + Name: "never-gonna", + TaskRef: &v1beta1.TaskRef{Name: "give-you-up"}, + WhenExpressions: []v1beta1.WhenExpression{{Input: "foo", Operator: selection.In, Values: []string{"foo", "bar"}}}, + RunAfter: []string{"foo"}, + Timeout: &metav1.Duration{Duration: 5 * time.Second}, }, { Name: "foo", TaskSpec: &v1beta1.EmbeddedTask{ diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 002e8b45896..97e010d7cbe 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -124,9 +124,14 @@ type PipelineTask struct { TaskSpec *EmbeddedTask `json:"taskSpec,inline,omitempty"` // Conditions is a list of conditions that need to be true for the task to run + // Conditions are deprecated, use WhenExpressions instead // +optional Conditions []PipelineTaskCondition `json:"conditions,omitempty"` + // WhenExpressions is a list of when expressions that need to be true for the task to run + // +optional + WhenExpressions WhenExpressions `json:"when,omitempty"` + // Retries represents how many times this task should be retried in case of task failure: ConditionSucceeded set to False // +optional Retries int `json:"retries,omitempty"` @@ -197,6 +202,16 @@ func (pt PipelineTask) Deps() []string { } } } + // Add any dependents from when expressions + for _, whenExpression := range pt.WhenExpressions { + expressions, ok := whenExpression.GetVarSubstitutionExpressions() + if ok { + resultRefs := NewResultRefs(expressions) + for _, resultRef := range resultRefs { + deps = append(deps, resultRef.PipelineTask) + } + } + } return deps } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index ee52521f21d..fac8294c2c5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -212,6 +212,10 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError { return err } + if err := validateWhenExpressions(ps.Tasks); err != nil { + return err + } + return nil } @@ -364,6 +368,9 @@ func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, pa if err := validatePipelineParametersVariablesInTaskParameters(task.Params, prefix, paramNames, arrayParamNames); err != nil { return err } + if err := task.WhenExpressions.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames); err != nil { + return err + } } return nil } @@ -460,6 +467,9 @@ func validateFinalTasks(finalTasks []PipelineTask) *apis.FieldError { if len(f.Conditions) != 0 { return apis.ErrInvalidValue(fmt.Sprintf("no conditions allowed under spec.finally, final task %s has conditions specified", f.Name), "spec.finally") } + if len(f.WhenExpressions) != 0 { + return apis.ErrInvalidValue(fmt.Sprintf("no when expressions allowed under spec.finally, final task %s has when expressions specified", f.Name), "spec.finally") + } } if err := validateTaskResultReferenceNotUsed(finalTasks); err != nil { @@ -503,3 +513,23 @@ func validateTasksInputFrom(tasks []PipelineTask) *apis.FieldError { } return nil } + +func validateWhenExpressions(tasks []PipelineTask) *apis.FieldError { + for i, t := range tasks { + if err := validateOneOfWhenExpressionsOrConditions(i, t); err != nil { + return err + } + if err := t.WhenExpressions.validate(); err != nil { + return err + } + } + return nil +} + +func validateOneOfWhenExpressionsOrConditions(i int, t PipelineTask) *apis.FieldError { + prefix := "spec.tasks" + if t.WhenExpressions != nil && t.Conditions != nil { + return apis.ErrMultipleOneOf(fmt.Sprintf(fmt.Sprintf(prefix+"[%d].when", i), fmt.Sprintf(prefix+"[%d].conditions", i))) + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 9615a56a07a..30c5c34d1eb 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" ) func TestPipeline_Validate_Success(t *testing.T) { @@ -157,6 +158,107 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }, + }, { + name: "invalid pipeline with one pipeline task having both conditions and when expressions", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"bar"}, + }}, + Conditions: []PipelineTaskCondition{{ + ConditionRef: "some-condition", + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having when expression with invalid operator (not In/NotIn)", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.Exists, + Values: []string{"foo"}, + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having when expression with invalid values (empty)", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{}, + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having when expression with invalid operator (missing)", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Values: []string{"foo"}, + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having when expression with invalid values (missing)", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having when expression with misconfigured result reference", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(tasks.a-task.resultTypo.bResult)", + Operator: selection.In, + Values: []string{"bar"}, + }}, + }}, + }, + }, { + name: "invalid pipeline with one pipeline task having blank when expression", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline with invalid pipeline task", + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{}}, + }}, + }, }, { name: "invalid pipeline with pipeline task having reference to resources which does not exist", ps: &PipelineSpec{ @@ -738,6 +840,26 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param", Value: ArrayOrString{StringVal: "$(baz) and $(foo-is-baz)"}, }}, }}, + }, { + name: "valid string parameter variables in when expression", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, + }, { + Name: "foo-is-baz", Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(params.baz)", + Operator: selection.In, + Values: []string{"foo"}, + }, { + Input: "baz", + Operator: selection.In, + Values: []string{"$(params.foo-is-baz)"}, + }}, + }}, }, { name: "valid array parameter variables", params: []ParamSpec{{ @@ -803,6 +925,56 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Name: "a-param", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.does-not-exist)"}, }}, }}, + }, { + name: "invalid string parameter variables in when expression, missing input param from the param declarations", + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(params.baz)", + Operator: selection.In, + Values: []string{"foo"}, + }}, + }}, + }, { + name: "invalid string parameter variables in when expression, missing values param from the param declarations", + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "bax", + Operator: selection.In, + Values: []string{"$(params.foo-is-baz)"}, + }}, + }}, + }, { + name: "invalid string parameter variables in when expression, array reference in input", + params: []ParamSpec{{ + Name: "foo", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(params.foo)", + Operator: selection.In, + Values: []string{"foo"}, + }}, + }}, + }, { + name: "invalid string parameter variables in when expression, array reference in values", + params: []ParamSpec{{ + Name: "foo", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "bax", + Operator: selection.In, + Values: []string{"$(params.foo)"}, + }}, + }}, }, { name: "invalid pipeline task with a parameter combined with missing param from the param declarations", params: []ParamSpec{{ @@ -1361,6 +1533,17 @@ func TestValidateFinalTasks_Failure(t *testing.T) { Name: "param1", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.a-task.results.output)"}, }}, }}, + }, { + name: "invalid pipeline with final task specifying when expressions", + finalTasks: []PipelineTask{{ + Name: "final-task", + TaskRef: &TaskRef{Name: "final-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }}, + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index 267f5e19491..c49a3f9ec38 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -331,6 +331,18 @@ type PipelineRunStatusFields struct { // PipelineRunSpec contains the exact spec used to instantiate the run PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"` + + // list of tasks that were skipped due to when expressions evaluating to false + // +optional + SkippedTasks []SkippedTask `json:"skippedTasks,omitempty"` +} + +// SkippedTask is used to describe the Tasks that were skipped due to their When Expressions +// evaluating to False. This is a struct because we are looking into including more details +// about the When Expressions that caused this Task to be skipped. +type SkippedTask struct { + // Name is the Pipeline Task name + Name string `json:"name"` } // PipelineRunResult used to describe the results of a pipeline diff --git a/pkg/apis/pipeline/v1beta1/resultref_test.go b/pkg/apis/pipeline/v1beta1/resultref_test.go index a01a3b1f6b4..ba279188c02 100644 --- a/pkg/apis/pipeline/v1beta1/resultref_test.go +++ b/pkg/apis/pipeline/v1beta1/resultref_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" + "k8s.io/apimachinery/pkg/selection" ) func TestNewResultReference(t *testing.T) { @@ -433,3 +434,315 @@ func TestLooksLikeResultRef(t *testing.T) { }) } } + +func TestNewResultReferenceWhenExpressions(t *testing.T) { + type args struct { + whenExpression v1beta1.WhenExpression + } + tests := []struct { + name string + args args + want []*v1beta1.ResultRef + }{ + { + name: "Test valid expression", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(tasks.sumTask.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask", + Result: "sumResult", + }, + }, + }, { + name: "substitution within string", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "sum-will-go-here -> $(tasks.sumTask.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask", + Result: "sumResult", + }, + }, + }, { + name: "multiple substitution", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask1", + Result: "sumResult", + }, { + PipelineTask: "sumTask2", + Result: "sumResult", + }, + }, + }, { + name: "multiple substitution with param", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(params.param) $(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask1", + Result: "sumResult", + }, { + PipelineTask: "sumTask2", + Result: "sumResult", + }, + }, + }, { + name: "first separator typo", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(task.sumTasks.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: nil, + }, { + name: "third separator typo", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(tasks.sumTasks.result.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: nil, + }, { + name: "param substitution shouldn't be considered result ref", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "$(params.paramName)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: nil, + }, { + name: "One bad and good result substitution", + args: args{ + whenExpression: v1beta1.WhenExpression{ + Input: "good -> $(tasks.sumTask1.results.sumResult) bad-> $(task.sumTask2.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + want: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask1", + Result: "sumResult", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expressions, ok := tt.args.whenExpression.GetVarSubstitutionExpressions() + if !ok { + t.Fatalf("expected to find expressions but didn't find any") + } else { + got := v1beta1.NewResultRefs(expressions) + if d := cmp.Diff(tt.want, got); d != "" { + t.Errorf("TestNewResultReference/%s %s", tt.name, diff.PrintWantGot(d)) + } + } + }) + } +} + +func TestHasResultReferenceWhenExpression(t *testing.T) { + tests := []struct { + name string + we v1beta1.WhenExpression + wantRef []*v1beta1.ResultRef + }{ + { + name: "Test valid expression", + we: v1beta1.WhenExpression{ + Input: "sumResult", + Operator: selection.In, + Values: []string{"$(tasks.sumTask.results.sumResult)"}, + }, + wantRef: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask", + Result: "sumResult", + }, + }, + }, { + name: "Test valid expression with dashes", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sum-task.results.sum-result)", + Operator: selection.In, + Values: []string{"sum-result"}, + }, + wantRef: []*v1beta1.ResultRef{ + { + PipelineTask: "sum-task", + Result: "sum-result", + }, + }, + }, { + name: "Test valid expression with underscores", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sum-task.results.sum_result)", + Operator: selection.In, + Values: []string{"sum-result"}, + }, + wantRef: []*v1beta1.ResultRef{ + { + PipelineTask: "sum-task", + Result: "sum_result", + }, + }, + }, { + name: "Test invalid expression: param substitution shouldn't be considered result ref", + we: v1beta1.WhenExpression{ + Input: "$(params.paramName)", + Operator: selection.In, + Values: []string{"sum-result"}, + }, + wantRef: nil, + }, { + name: "Test valid expression in array", + we: v1beta1.WhenExpression{ + Input: "$sumResult", + Operator: selection.In, + Values: []string{"$(tasks.sumTask.results.sumResult)", "$(tasks.sumTask2.results.sumResult2)"}, + }, + wantRef: []*v1beta1.ResultRef{ + { + PipelineTask: "sumTask", + Result: "sumResult", + }, + { + PipelineTask: "sumTask2", + Result: "sumResult2", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expressions, ok := tt.we.GetVarSubstitutionExpressions() + if !ok { + t.Fatalf("expected to find expressions but didn't find any") + } + got := v1beta1.NewResultRefs(expressions) + if d := cmp.Diff(tt.wantRef, got); d != "" { + t.Errorf("TestHasResultReference/%s %s", tt.name, diff.PrintWantGot(d)) + } + }) + } +} + +func TestLooksLikeResultRefWhenExpressionTrue(t *testing.T) { + tests := []struct { + name string + we v1beta1.WhenExpression + }{ + { + name: "test expression that is a result ref", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sumTasks.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, { + name: "test expression: looks like result ref, but typo in 'task' separator", + we: v1beta1.WhenExpression{ + Input: "$(task.sumTasks.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, { + name: "test expression: looks like result ref, but typo in 'results' separator", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sumTasks.result.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, { + name: "test expression: one good ref, one bad one should return true", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sumTasks.results.sumResult) $(task.sumTasks.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expressions, ok := tt.we.GetVarSubstitutionExpressions() + if !ok { + t.Fatalf("expected to find expressions but didn't find any") + } + if !v1beta1.LooksLikeContainsResultRefs(expressions) { + t.Errorf("expected expressions to look like they contain result refs") + } + }) + } +} + +func TestLooksLikeResultRefWhenExpressionFalse(t *testing.T) { + tests := []struct { + name string + we v1beta1.WhenExpression + }{ + { + name: "test expression: missing 'task' separator", + we: v1beta1.WhenExpression{ + Input: "$(sumTasks.results.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, { + name: "test expression: missing 'results' separator", + we: v1beta1.WhenExpression{ + Input: "$(tasks.sumTasks.sumResult)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, { + name: "test expression: param substitution shouldn't be considered result ref", + we: v1beta1.WhenExpression{ + Input: "$(params.someParam)", + Operator: selection.In, + Values: []string{"foo"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expressions, ok := tt.we.GetVarSubstitutionExpressions() + if !ok { + t.Fatalf("expected to find expressions but didn't find any") + } + if v1beta1.LooksLikeContainsResultRefs(expressions) { + t.Errorf("expected expressions to not look like they contain results refs") + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go new file mode 100644 index 00000000000..7adc6d43dc1 --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/when_types.go @@ -0,0 +1,115 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "k8s.io/apimachinery/pkg/selection" +) + +// WhenExpression allows a PipelineTask to declare expressions to be evaluated before the Task is run +// to determine whether the Task should be executed or skipped +type WhenExpression struct { + // Input is the string for guard checking which can be a static input or an output from a parent Task + Input string + // Operator that represents an Input's relationship to the values + Operator selection.Operator + // Values is an array of strings, which is compared against the input, for guard checking + // It must be non-empty + Values []string +} + +func (we *WhenExpression) isInputInValues() bool { + for i := range we.Values { + if we.Values[i] == we.Input { + return true + } + } + return false +} + +func (we *WhenExpression) isTrue() bool { + if we.Operator == selection.In { + return we.isInputInValues() + } + // selection.NotIn + return !we.isInputInValues() +} + +func (we *WhenExpression) hasVariable() bool { + if _, hasVariable := we.GetVarSubstitutionExpressions(); hasVariable { + return true + } + return false +} + +func (we *WhenExpression) applyReplacements(replacements map[string]string) WhenExpression { + replacedInput := ApplyReplacements(we.Input, replacements) + + var replacedValues []string + for _, val := range we.Values { + replacedValues = append(replacedValues, ApplyReplacements(val, replacements)) + } + + return WhenExpression{Input: replacedInput, Operator: we.Operator, Values: replacedValues} +} + +// GetVarSubstitutionExpressions extracts all the values between "$(" and ")" in a When Expression +func (we *WhenExpression) GetVarSubstitutionExpressions() ([]string, bool) { + var allExpressions []string + allExpressions = append(allExpressions, validateString(we.Input)...) + for _, value := range we.Values { + allExpressions = append(allExpressions, validateString(value)...) + } + return allExpressions, len(allExpressions) != 0 +} + +// WhenExpressions are used to specify whether a Task should be executed or skipped +// All of them need to evaluate to True for a guarded Task to be executed. +type WhenExpressions []WhenExpression + +// AllowsExecution evaluates an Input's relationship to an array of Values, based on the Operator, +// to determine whether all the When Expressions are True. If they are all True, the guarded Task is +// executed, otherwise it is skipped. +func (wes WhenExpressions) AllowsExecution() bool { + for _, we := range wes { + if !we.isTrue() { + return false + } + } + return true +} + +// HaveVariables indicates whether When Expressions contains variables, such as Parameters +// or Results in the Inputs or Values. +func (wes WhenExpressions) HaveVariables() bool { + for _, we := range wes { + if we.hasVariable() { + return true + } + } + return false +} + +// ReplaceWhenExpressionsVariables interpolates variables, such as Parameters and Results, in +// the Input and Values. Returns a new instance of WhenExpressions. +func (wes WhenExpressions) ReplaceWhenExpressionsVariables(replacements map[string]string) WhenExpressions { + var replaced []WhenExpression + for _, we := range wes { + replaced = append(replaced, we.applyReplacements(replacements)) + } + return replaced +} diff --git a/pkg/apis/pipeline/v1beta1/when_types_test.go b/pkg/apis/pipeline/v1beta1/when_types_test.go new file mode 100644 index 00000000000..a12ac8ab0fb --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/when_types_test.go @@ -0,0 +1,319 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/test/diff" + "k8s.io/apimachinery/pkg/selection" +) + +func TestAllowsExecution(t *testing.T) { + tests := []struct { + name string + whenExpressions WhenExpressions + expected bool + }{{ + name: "in expression", + whenExpressions: WhenExpressions{ + { + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }, + }, + expected: true, + }, { + name: "notin expression", + whenExpressions: WhenExpressions{ + { + Input: "foobar", + Operator: selection.NotIn, + Values: []string{"foobar"}, + }, + }, + expected: false, + }, { + name: "multiple expressions - false", + whenExpressions: WhenExpressions{ + { + Input: "foobar", + Operator: selection.In, + Values: []string{"foobar"}, + }, { + Input: "foo", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + expected: false, + }, { + name: "multiple expressions - true", + whenExpressions: WhenExpressions{ + { + Input: "foobar", + Operator: selection.In, + Values: []string{"foobar"}, + }, { + Input: "foo", + Operator: selection.NotIn, + Values: []string{"bar"}, + }, + }, + expected: true, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.whenExpressions.AllowsExecution() + if d := cmp.Diff(tc.expected, got); d != "" { + t.Errorf("Error evaluating AllowsExecution() for When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} + +func TestHaveVariables(t *testing.T) { + tests := []struct { + name string + whenExpressions WhenExpressions + expected bool + }{{ + name: "doesn't have variable", + whenExpressions: WhenExpressions{ + { + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }, + }, + expected: false, + }, { + name: "have variable - input", + whenExpressions: WhenExpressions{ + { + Input: "$(params.foobar)", + Operator: selection.NotIn, + Values: []string{"foobar"}, + }, + }, + expected: true, + }, { + name: "have variable - values", + whenExpressions: WhenExpressions{ + { + Input: "foobar", + Operator: selection.In, + Values: []string{"$(params.foobar)"}, + }, + }, + expected: true, + }, { + name: "have variable - multiple when expressions", + whenExpressions: WhenExpressions{ + { + Input: "foo", + Operator: selection.NotIn, + Values: []string{"bar"}, + }, { + Input: "foobar", + Operator: selection.In, + Values: []string{"$(params.foobar)"}, + }, + }, + expected: true, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.whenExpressions.HaveVariables() + if d := cmp.Diff(tc.expected, got); d != "" { + t.Errorf("Error evaluating HaveVariables() for When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} + +func TestReplaceWhenExpressionsVariables(t *testing.T) { + tests := []struct { + name string + whenExpressions WhenExpressions + replacements map[string]string + expected WhenExpressions + }{{ + name: "params replacement in input", + whenExpressions: WhenExpressions{ + { + Input: "$(params.foo)", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + replacements: map[string]string{ + "params.foo": "bar", + }, + expected: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + }, { + name: "params replacement in values", + whenExpressions: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"$(params.foo)"}, + }, + }, + replacements: map[string]string{ + "params.foo": "bar", + }, + expected: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + }, { + name: "results replacement in input", + whenExpressions: WhenExpressions{ + { + Input: "$(tasks.aTask.results.foo)", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + replacements: map[string]string{ + "tasks.aTask.results.foo": "bar", + }, + expected: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + }, { + name: "results replacement in values", + whenExpressions: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.foo)"}, + }, + }, + replacements: map[string]string{ + "tasks.aTask.results.foo": "bar", + }, + expected: WhenExpressions{ + { + Input: "bar", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + }, { + name: "replacements in multiple when expressions", + whenExpressions: WhenExpressions{ + { + Input: "foo", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.foo)"}, + }, { + Input: "$(params.bar)", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + replacements: map[string]string{ + "tasks.aTask.results.foo": "foo", + "params.bar": "bar", + }, + expected: WhenExpressions{ + { + Input: "foo", + Operator: selection.In, + Values: []string{"foo"}, + }, { + Input: "bar", + Operator: selection.In, + Values: []string{"bar"}, + }, + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.whenExpressions.ReplaceWhenExpressionsVariables(tc.replacements) + if d := cmp.Diff(tc.expected, got); d != "" { + t.Errorf("Error evaluating When Expressions in test case %s: %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} + +func TestApplyReplacements(t *testing.T) { + tests := []struct { + name string + original *WhenExpression + replacements map[string]string + expected *WhenExpression + }{{ + name: "replace parameters variables", + original: &WhenExpression{ + Input: "$(params.path)", + Operator: selection.In, + Values: []string{"$(params.branch)"}, + }, + replacements: map[string]string{ + "params.path": "readme.md", + "params.branch": "staging", + }, + expected: &WhenExpression{ + Input: "readme.md", + Operator: selection.In, + Values: []string{"staging"}, + }, + }, { + name: "replace results variables", + original: &WhenExpression{ + Input: "$(tasks.foo.results.bar)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult)"}, + }, + replacements: map[string]string{ + "tasks.foo.results.bar": "foobar", + "tasks.aTask.results.aResult": "barfoo", + }, + expected: &WhenExpression{ + Input: "foobar", + Operator: selection.In, + Values: []string{"barfoo"}, + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.original.applyReplacements(tc.replacements) + if d := cmp.Diff(tc.expected, &got); d != "" { + t.Errorf("Error applying replacements for When Expressions: %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/when_validation.go b/pkg/apis/pipeline/v1beta1/when_validation.go new file mode 100644 index 00000000000..ff7a49e37df --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/when_validation.go @@ -0,0 +1,106 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "fmt" + "strings" + + "github.com/tektoncd/pipeline/pkg/substitution" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/util/sets" + "knative.dev/pkg/apis" +) + +var validWhenOperators = []string{ + string(selection.In), + string(selection.NotIn), +} + +func (wes WhenExpressions) validate() *apis.FieldError { + if err := wes.validateWhenExpressionsFields(); err != nil { + return err + } + if err := wes.validateTaskResultsVariables(); err != nil { + return err + } + return nil +} + +func (wes WhenExpressions) validateWhenExpressionsFields() *apis.FieldError { + for _, we := range wes { + if err := we.validateWhenExpressionFields(); err != nil { + return err + } + } + return nil +} + +func (we *WhenExpression) validateWhenExpressionFields() *apis.FieldError { + if equality.Semantic.DeepEqual(we, &WhenExpression{}) || we == nil { + return apis.ErrMissingField(apis.CurrentField) + } + if !sets.NewString(validWhenOperators...).Has(string(we.Operator)) { + message := fmt.Sprintf("operator %q is not recognized. valid operators: %s", we.Operator, strings.Join(validWhenOperators, ",")) + return apis.ErrInvalidValue(message, "spec.task.when") + } + if len(we.Values) == 0 { + return apis.ErrInvalidValue("expecting non-empty values field", "spec.task.when") + } + return nil +} + +func (wes WhenExpressions) validateTaskResultsVariables() *apis.FieldError { + for _, we := range wes { + expressions, ok := we.GetVarSubstitutionExpressions() + if ok { + if LooksLikeContainsResultRefs(expressions) { + expressions = filter(expressions, looksLikeResultRef) + resultRefs := NewResultRefs(expressions) + if len(expressions) != len(resultRefs) { + message := fmt.Sprintf("expected all of the expressions %v to be result expressions but only %v were", expressions, resultRefs) + return apis.ErrInvalidValue(message, "spec.tasks.when") + } + } + } + } + return nil +} + +func (wes WhenExpressions) validatePipelineParametersVariables(prefix string, paramNames sets.String, arrayParamNames sets.String) *apis.FieldError { + for _, we := range wes { + if err := validateStringVariable(fmt.Sprintf("input[%s]", we.Input), we.Input, prefix, paramNames, arrayParamNames); err != nil { + return err + } + for _, val := range we.Values { + if err := validateStringVariable(fmt.Sprintf("values[%s]", val), val, prefix, paramNames, arrayParamNames); err != nil { + return err + } + } + } + return nil +} +func validateStringVariable(name, value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError { + if err := substitution.ValidateVariable(name, value, prefix, "task when expression", "pipelinespec.when", stringVars); err != nil { + return err + } + if err := substitution.ValidateVariableProhibited(name, value, prefix, "task when expression", "pipelinespec.when", arrayVars); err != nil { + return err + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/when_validation_test.go b/pkg/apis/pipeline/v1beta1/when_validation_test.go new file mode 100644 index 00000000000..b63e7847b06 --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/when_validation_test.go @@ -0,0 +1,112 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "testing" + + "k8s.io/apimachinery/pkg/selection" +) + +func TestWhenExpressions_Valid(t *testing.T) { + tests := []struct { + name string + wes WhenExpressions + }{{ + name: "valid operator - In - and values", + wes: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo"}, + }}, + }, { + name: "valid operator - NotIn - and values", + wes: []WhenExpression{{ + Input: "foo", + Operator: selection.NotIn, + Values: []string{"bar"}, + }}, + }, { + wes: []WhenExpression{{ + Input: "$(tasks.a-task.results.output)", + Operator: selection.In, + Values: []string{"bar"}, + }}, + }, { + wes: []WhenExpression{{ // missing Input defaults to empty string + Operator: selection.In, + Values: []string{""}, + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.wes.validate(); err != nil { + t.Errorf("WhenExpressions.validate() returned an error for valid when expressions: %s, %s", tt.name, tt.wes) + } + }) + } +} + +func TestWhenExpressions_Invalid(t *testing.T) { + tests := []struct { + name string + wes WhenExpressions + }{{ + name: "invalid operator - exists", + wes: []WhenExpression{{ + Input: "foo", + Operator: selection.Exists, + Values: []string{"foo"}, + }}, + }, { + name: "invalid values - empty", + wes: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{}, + }}, + }, { + name: "missing Operator", + wes: []WhenExpression{{ + Input: "foo", + Values: []string{"foo"}, + }}, + }, { + name: "missing Values", + wes: []WhenExpression{{ + Input: "foo", + Operator: selection.NotIn, + }}, + }, { + name: "invalid variable", + wes: []WhenExpression{{ + Input: "$(tasks.a-task.resultsTypo.output)", + Operator: selection.In, + Values: []string{"bar"}, + }}, + }, { + name: "missing when expression", + wes: []WhenExpression{{}}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.wes.validate(); err == nil { + t.Errorf("WhenExpressions.validate() did not return error for invalid when expressions: %s, %s, %s", tt.name, tt.wes, err) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 37733e74976..97cf2670876 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -726,6 +726,11 @@ func (in *PipelineRunStatusFields) DeepCopyInto(out *PipelineRunStatusFields) { *out = new(PipelineSpec) (*in).DeepCopyInto(*out) } + if in.SkippedTasks != nil { + in, out := &in.SkippedTasks, &out.SkippedTasks + *out = make([]SkippedTask, len(*in)) + copy(*out, *in) + } return } @@ -847,6 +852,13 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.WhenExpressions != nil { + in, out := &in.WhenExpressions, &out.WhenExpressions + *out = make(WhenExpressions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.RunAfter != nil { in, out := &in.RunAfter, &out.RunAfter *out = make([]string, len(*in)) @@ -1153,6 +1165,22 @@ func (in *SidecarState) DeepCopy() *SidecarState { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SkippedTask) DeepCopyInto(out *SkippedTask) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SkippedTask. +func (in *SkippedTask) DeepCopy() *SkippedTask { + if in == nil { + return nil + } + out := new(SkippedTask) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Step) DeepCopyInto(out *Step) { *out = *in @@ -1708,6 +1736,49 @@ func (in *TaskSpec) DeepCopy() *TaskSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WhenExpression) DeepCopyInto(out *WhenExpression) { + *out = *in + if in.Values != nil { + in, out := &in.Values, &out.Values + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WhenExpression. +func (in *WhenExpression) DeepCopy() *WhenExpression { + if in == nil { + return nil + } + out := new(WhenExpression) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in WhenExpressions) DeepCopyInto(out *WhenExpressions) { + { + in := &in + *out = make(WhenExpressions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WhenExpressions. +func (in WhenExpressions) DeepCopy() WhenExpressions { + if in == nil { + return nil + } + out := new(WhenExpressions) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkspaceBinding) DeepCopyInto(out *WorkspaceBinding) { *out = *in diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 47d14c68273..da5b466a192 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -481,6 +481,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err // Read the condition the way it was set by the Mark* helpers after = pr.Status.GetCondition(apis.ConditionSucceeded) pr.Status.TaskRuns = getTaskRunsStatus(pr, pipelineState) + pr.Status.SkippedTasks = getSkippedTasks(pr, pipelineState, d) logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) return nil } @@ -613,6 +614,16 @@ func getTaskRunsStatus(pr *v1beta1.PipelineRun, state []*resources.ResolvedPipel return status } +func getSkippedTasks(pr *v1beta1.PipelineRun, state []*resources.ResolvedPipelineRunTask, d *dag.Graph) []v1beta1.SkippedTask { + skipped := []v1beta1.SkippedTask{} + for _, rprt := range state { + if rprt.Skip(state, d) { + skipped = append(skipped, v1beta1.SkippedTask{Name: rprt.PipelineTask.Name}) + } + } + return skipped +} + func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1beta1.PipelineRun) error { for taskRunName := range pr.Status.TaskRuns { // TODO(dibyom): Add conditionCheck statuses here diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 512eea8c8d2..25ccb7e1a54 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -47,6 +47,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/types" ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" @@ -2004,6 +2005,192 @@ func ensurePVCCreated(t *testing.T, clients test.Clients, name, namespace string } } +func TestReconcileWithWhenExpressionsWithParameters(t *testing.T) { + names.TestingSeed() + prName := "test-pipeline-run" + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineParamSpec("run", v1beta1.ParamTypeString), + tb.PipelineTask("hello-world-1", "hello-world-1", + tb.PipelineTaskWhenExpression("foo", selection.NotIn, []string{"bar"}), + tb.PipelineTaskWhenExpression("$(params.run)", selection.In, []string{"yes"})), + tb.PipelineTask("hello-world-2", "hello-world-2", + tb.PipelineTaskWhenExpression("$(params.run)", selection.NotIn, []string{"yes"})), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun(prName, tb.PipelineRunNamespace("foo"), + tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa"), + tb.PipelineRunParam("run", "yes"), + ), + )} + ts := []*v1beta1.Task{ + tb.Task("hello-world-1", tb.TaskNamespace("foo")), + tb.Task("hello-world-2", tb.TaskNamespace("foo")), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := NewPipelineRunTest(d, t) + defer prt.Cancel() + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 0 \\(Failed: 0, Cancelled 0\\), Incomplete: 1, Skipped: 1", + } + pipelineRun, clients := prt.reconcileRun("foo", prName, wantEvents, false) + + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{ + LabelSelector: "tekton.dev/pipelineTask=hello-world-1,tekton.dev/pipelineRun=test-pipeline-run", + Limit: 1, + }) + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(actual.Items) != 1 { + t.Fatalf("Expected 1 TaskRun got %d", len(actual.Items)) + } + expectedTaskRunName := "test-pipeline-run-hello-world-1-9l9zj" + expectedTaskRun := tb.TaskRun(expectedTaskRunName, + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineLabelKey, "test-pipeline"), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineTaskLabelKey, "hello-world-1"), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineRunLabelKey, "test-pipeline-run"), + tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-1"), + tb.TaskRunServiceAccountName("test-sa"), + ), + ) + actualTaskRun := actual.Items[0] + if d := cmp.Diff(&actualTaskRun, expectedTaskRun, ignoreResourceVersion); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRunName, diff.PrintWantGot(d)) + } + + actualSkippedTasks := pipelineRun.Status.SkippedTasks + expectedSkippedTasks := []v1beta1.SkippedTask{{ + Name: "hello-world-2", + }} + if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" { + t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d)) + } +} + +func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) { + names.TestingSeed() + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask("a-task", "a-task"), + tb.PipelineTask("b-task", "b-task", + tb.PipelineTaskWhenExpression("$(tasks.a-task.results.aResult)", selection.In, []string{"aResultValue"}), + tb.PipelineTaskWhenExpression("aResultValue", selection.In, []string{"$(tasks.a-task.results.aResult)"}), + ), + tb.PipelineTask("c-task", "c-task", + tb.PipelineTaskWhenExpression("$(tasks.a-task.results.aResult)", selection.In, []string{"missing"}), + ), + tb.PipelineTask("d-task", "d-task", tb.RunAfter("c-task")), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-different-service-accs", tb.PipelineRunNamespace("foo"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa-0"), + ), + )} + ts := []*v1beta1.Task{ + tb.Task("a-task", tb.TaskNamespace("foo")), + tb.Task("b-task", tb.TaskNamespace("foo")), + tb.Task("c-task", tb.TaskNamespace("foo")), + tb.Task("d-task", tb.TaskNamespace("foo")), + } + trs := []*v1beta1.TaskRun{ + tb.TaskRun("test-pipeline-run-different-service-accs-a-task-xxyyy", + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "a-task"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world"), + tb.TaskRunServiceAccountName("test-sa"), + ), + tb.TaskRunStatus( + tb.StatusCondition( + apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + ), + tb.TaskRunResult("aResult", "aResultValue"), + ), + ), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + TaskRuns: trs, + } + prt := NewPipelineRunTest(d, t) + defer prt.Cancel() + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 1 \\(Failed: 0, Cancelled 0\\), Incomplete: 1, Skipped: 2", + } + pipelineRun, clients := prt.reconcileRun("foo", "test-pipeline-run-different-service-accs", wantEvents, false) + + expectedTaskRunName := "test-pipeline-run-different-service-accs-b-task-9l9zj" + expectedTaskRun := tb.TaskRun(expectedTaskRunName, + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "b-task"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("b-task"), + tb.TaskRunServiceAccountName("test-sa-0"), + ), + ) + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{ + LabelSelector: "tekton.dev/pipelineTask=b-task,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs", + Limit: 1, + }) + + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(actual.Items) != 1 { + t.Fatalf("Expected 1 TaskRuns got %d", len(actual.Items)) + } + actualTaskRun := actual.Items[0] + if d := cmp.Diff(&actualTaskRun, expectedTaskRun, ignoreResourceVersion); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRunName, diff.PrintWantGot(d)) + } + + actualSkippedTasks := pipelineRun.Status.SkippedTasks + expectedSkippedTasks := []v1beta1.SkippedTask{{ + Name: "c-task", + }, { + Name: "d-task", + }} + if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" { + t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d)) + } +} + // TestReconcileWithAffinityAssistantStatefulSet tests that given a pipelineRun with workspaces, // an Affinity Assistant StatefulSet is created for each PVC workspace and // that the Affinity Assistant names is propagated to TaskRuns. diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index d12a2a33b4d..3c34275a0b8 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -65,7 +65,7 @@ func ApplyContexts(spec *v1beta1.PipelineSpec, pipelineName string, pr *v1beta1. return ApplyReplacements(spec, replacements, map[string][]string{}) } -// ApplyTaskResults applies the ResolvedResultRef to each PipelineTask.Params in targets +// ApplyTaskResults applies the ResolvedResultRef to each PipelineTask.Params and Pipeline.WhenExpressions in targets func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResultRefs) { stringReplacements := resolvedResultRefs.getStringReplacements() for _, resolvedPipelineRunTask := range targets { @@ -78,6 +78,7 @@ func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResul if resolvedPipelineRunTask.PipelineTask != nil { pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, nil) + pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements) resolvedPipelineRunTask.PipelineTask = pipelineTask } } @@ -96,6 +97,7 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string, c := tasks[i].Conditions[j] c.Params = replaceParamValues(c.Params, replacements, arrayReplacements) } + tasks[i].WhenExpressions = tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements) } return p diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 3be1f59fa05..eb1a6a36226 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -59,6 +60,25 @@ func TestApplyParameters(t *testing.T) { tb.PipelineTaskParam("first-task-second-param", "second-value"), tb.PipelineTaskParam("first-task-third-param", "static value"), ))), + }, { + name: "single parameter with when expression", + original: tb.Pipeline("test-pipeline", + tb.PipelineSpec( + tb.PipelineParamSpec("first-param", v1beta1.ParamTypeString, tb.ParamSpecDefault("default-value")), + tb.PipelineParamSpec("second-param", v1beta1.ParamTypeString), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskWhenExpression("$(params.first-param)", selection.In, []string{"$(params.second-param)"}), + ))), + run: tb.PipelineRun("test-pipeline-run", + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("second-param", "second-value"))), + expected: tb.Pipeline("test-pipeline", + tb.PipelineSpec( + tb.PipelineParamSpec("first-param", v1beta1.ParamTypeString, tb.ParamSpecDefault("default-value")), + tb.PipelineParamSpec("second-param", v1beta1.ParamTypeString), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskWhenExpression("default-value", selection.In, []string{"second-value"}), + ))), }, { name: "pipeline parameter nested inside task parameter", original: tb.Pipeline("test-pipeline", @@ -177,7 +197,7 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) { want PipelineRunState }{ { - name: "Test result substitution on minimal variable substitution expression", + name: "Test result substitution on minimal variable substitution expression - params", args: args{ resolvedResultRefs: ResolvedResultRefs{ { @@ -227,6 +247,53 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) { }, }, }, + }, { + name: "Test result substitution on minimal variable substitution expression - when expressions", + args: args{ + resolvedResultRefs: ResolvedResultRefs{ + { + Value: v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeString, + StringVal: "aResultValue", + }, + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aTask", + Result: "aResult", + }, + FromTaskRun: "aTaskRun", + }, + }, + targets: PipelineRunState{ + { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "$(tasks.aTask.results.aResult)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult)"}, + }, + }, + }, + }, + }, + }, + want: PipelineRunState{ + { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "aResultValue", + Operator: selection.In, + Values: []string{"aResultValue"}, + }, + }, + }, + }, + }, }, } for _, tt := range tests { @@ -250,7 +317,7 @@ func TestApplyTaskResults_EmbeddedExpression(t *testing.T) { want PipelineRunState }{ { - name: "Test result substitution on embedded variable substitution expression", + name: "Test result substitution on embedded variable substitution expression - params", args: args{ resolvedResultRefs: ResolvedResultRefs{ { @@ -300,6 +367,53 @@ func TestApplyTaskResults_EmbeddedExpression(t *testing.T) { }, }, }, + }, { + name: "Test result substitution on embedded variable substitution expression - when expressions", + args: args{ + resolvedResultRefs: ResolvedResultRefs{ + { + Value: v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeString, + StringVal: "aResultValue", + }, + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aTask", + Result: "aResult", + }, + FromTaskRun: "aTaskRun", + }, + }, + targets: PipelineRunState{ + { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "Result value --> $(tasks.aTask.results.aResult)", + Operator: selection.In, + Values: []string{"Result value --> $(tasks.aTask.results.aResult)"}, + }, + }, + }, + }, + }, + }, + want: PipelineRunState{ + { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "Result value --> aResultValue", + Operator: selection.In, + Values: []string{"Result value --> aResultValue"}, + }, + }, + }, + }, + }, }, } for _, tt := range tests { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index f528d05103e..e8d56045846 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -141,12 +141,13 @@ func (t ResolvedPipelineRunTask) IsStarted() bool { return true } -// IsSkipped returns true if a PipelineTask will not be run because -// (1) its Condition Checks failed or -// (2) one of the parent task's conditions failed or -// (3) Pipeline is in stopping state (one of the PipelineTasks failed) -// Note that this means IsSkipped returns false if a conditionCheck is in progress -func (t ResolvedPipelineRunTask) IsSkipped(state PipelineRunState, d *dag.Graph) bool { +// Skip returns true if a PipelineTask will not be run because +// (1) its When Expressions evaluated to false +// (2) its Condition Checks failed +// (3) its parent task was skipped +// (4) Pipeline is in stopping state (one of the PipelineTasks failed) +// Note that this means Skip returns false if a conditionCheck is in progress +func (t *ResolvedPipelineRunTask) Skip(state PipelineRunState, d *dag.Graph) bool { // it already has TaskRun associated with it - PipelineTask not skipped if t.IsStarted() { return false @@ -159,6 +160,15 @@ func (t ResolvedPipelineRunTask) IsSkipped(state PipelineRunState, d *dag.Graph) } } + // Check if the when expressions are false, based on the input's relationship to the values + if len(t.PipelineTask.WhenExpressions) > 0 { + if !t.PipelineTask.WhenExpressions.HaveVariables() { + if !t.PipelineTask.WhenExpressions.AllowsExecution() { + return true + } + } + } + // Skip the PipelineTask if pipeline is in stopping state if isTaskInGraph(t.PipelineTask.Name, d) && state.IsStopping(d) { return true @@ -170,7 +180,7 @@ func (t ResolvedPipelineRunTask) IsSkipped(state PipelineRunState, d *dag.Graph) node := d.Nodes[t.PipelineTask.Name] if isTaskInGraph(t.PipelineTask.Name, d) { for _, p := range node.Prev { - if stateMap[p.Task.HashKey()].IsSkipped(state, d) { + if stateMap[p.Task.HashKey()].Skip(state, d) { return true } } @@ -253,7 +263,7 @@ func (state PipelineRunState) SuccessfulOrSkippedDAGTasks(d *dag.Graph) []string tasks := []string{} for _, t := range state { if isTaskInGraph(t.PipelineTask.Name, d) { - if t.IsSuccessful() || t.IsSkipped(state, d) { + if t.IsSuccessful() || t.Skip(state, d) { tasks = append(tasks, t.PipelineTask.Name) } } @@ -270,7 +280,7 @@ func (state PipelineRunState) checkTasksDone(d *dag.Graph) bool { // this task might have skipped if taskRun is nil // continue and ignore if this task was skipped // skipped task is considered part of done - if t.IsSkipped(state, d) { + if t.Skip(state, d) { continue } return false @@ -554,7 +564,7 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, switch { case rprt.IsSuccessful(): withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) - case rprt.IsSkipped(state, dag): + case rprt.Skip(state, dag): skipTasks++ withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) // At least one is skipped and no failure yet, mark as completed diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index a1b4dbc0fea..588351a5482 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -38,6 +38,7 @@ import ( corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" @@ -78,6 +79,22 @@ var pts = []v1beta1.PipelineTask{{ Name: "mytask9", TaskRef: &v1beta1.TaskRef{Name: "taskHasParentWithRunAfter"}, RunAfter: []string{"mytask8"}, +}, { + Name: "mytask10", + TaskRef: &v1beta1.TaskRef{Name: "taskWithWhenExpressions"}, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }}, +}, { + Name: "mytask11", + TaskRef: &v1beta1.TaskRef{Name: "taskWithWhenExpressions"}, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.NotIn, + Values: []string{"foo", "bar"}, + }}, }} var p = &v1beta1.Pipeline{ @@ -1134,6 +1151,30 @@ func TestIsSkipped(t *testing.T) { }, }}, expected: true, + }, { + name: "tasks-when-expressions-passed", + taskName: "mytask10", + state: PipelineRunState{{ + PipelineTask: &pts[9], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: false, + }, { + name: "tasks-when-expression-failed", + taskName: "mytask11", + state: PipelineRunState{{ + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: true, }} for _, tc := range tcs { @@ -1147,7 +1188,7 @@ func TestIsSkipped(t *testing.T) { if rprt == nil { t.Fatalf("Could not get task %s from the state: %v", tc.taskName, tc.state) } - isSkipped := rprt.IsSkipped(tc.state, dag) + isSkipped := rprt.Skip(tc.state, dag) if d := cmp.Diff(isSkipped, tc.expected); d != "" { t.Errorf("Didn't get expected isSkipped %s", diff.PrintWantGot(d)) } @@ -2660,3 +2701,54 @@ func TestPipelineRunState_GetFinalTasks(t *testing.T) { }) } } + +func TestResolvePipeline_WhenExpressions(t *testing.T) { + names.TestingSeed() + tName1 := "pipelinerun-mytask1-9l9zj-always-true-mz4c7" + + t1 := &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: tName1, + }, + } + + ptwe1 := v1beta1.WhenExpression{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo"}, + } + + pts := []v1beta1.PipelineTask{{ + Name: "mytask1", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + WhenExpressions: []v1beta1.WhenExpression{ptwe1}, + }} + + providedResources := map[string]*resourcev1alpha1.PipelineResource{} + + getTask := func(name string) (v1beta1.TaskInterface, error) { return task, nil } + getClusterTask := func(name string) (v1beta1.TaskInterface, error) { return nil, errors.New("should not get called") } + getCondition := func(name string) (*v1alpha1.Condition, error) { return &condition, nil } + pr := v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinerun", + }, + } + + getTaskRun := func(name string) (*v1beta1.TaskRun, error) { + switch name { + case "pipelinerun-mytask1-9l9zj-always-true-0-mz4c7": + return t1, nil + case "pipelinerun-mytask1-9l9zj": + return &trs[0], nil + } + return nil, fmt.Errorf("getTaskRun called with unexpected name %s", name) + } + + t.Run("When Expressions exist", func(t *testing.T) { + _, err := ResolvePipelineRun(context.Background(), pr, getTask, getTaskRun, getClusterTask, getCondition, pts, providedResources) + if err != nil { + t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) + } + }) +} diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index dfe46f06a86..c86a2214849 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -40,7 +40,7 @@ type ResolvedResultRef struct { func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState) (ResolvedResultRefs, error) { var allResolvedResultRefs ResolvedResultRefs for _, target := range targets { - resolvedResultRefs, err := convertParamsToResultRefs(pipelineRunState, target) + resolvedResultRefs, err := convertToResultRefs(pipelineRunState, target) if err != nil { return nil, err } @@ -139,24 +139,30 @@ func removeDup(refs ResolvedResultRefs) ResolvedResultRefs { return deduped } -// convertParamsToResultRefs converts all params of the resolved pipeline run task -func convertParamsToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipelineRunTask) (ResolvedResultRefs, error) { - var resolvedParams ResolvedResultRefs +// convertToResultRefs replaces result references for all params and when expressions of the resolved pipeline run task +func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipelineRunTask) (ResolvedResultRefs, error) { + var resolvedResultRefs ResolvedResultRefs for _, condition := range target.PipelineTask.Conditions { condRefs, err := convertParams(condition.Params, pipelineRunState, condition.ConditionRef) if err != nil { return nil, err } - resolvedParams = append(resolvedParams, condRefs...) + resolvedResultRefs = append(resolvedResultRefs, condRefs...) } taskParamsRefs, err := convertParams(target.PipelineTask.Params, pipelineRunState, target.PipelineTask.Name) if err != nil { return nil, err } - resolvedParams = append(resolvedParams, taskParamsRefs...) + resolvedResultRefs = append(resolvedResultRefs, taskParamsRefs...) - return resolvedParams, nil + taskWhenExpressionsRefs, err := convertWhenExpressions(target.PipelineTask.WhenExpressions, pipelineRunState, target.PipelineTask.Name) + if err != nil { + return nil, err + } + resolvedResultRefs = append(resolvedResultRefs, taskWhenExpressionsRefs...) + + return resolvedResultRefs, nil } func convertParams(params []v1beta1.Param, pipelineRunState PipelineRunState, name string) (ResolvedResultRefs, error) { @@ -173,6 +179,23 @@ func convertParams(params []v1beta1.Param, pipelineRunState PipelineRunState, na return resolvedParams, nil } +func convertWhenExpressions(whenExpressions []v1beta1.WhenExpression, pipelineRunState PipelineRunState, name string) (ResolvedResultRefs, error) { + var resolvedWhenExpressions ResolvedResultRefs + for _, whenExpression := range whenExpressions { + expressions, ok := whenExpression.GetVarSubstitutionExpressions() + if ok { + resolvedResultRefs, err := extractResultRefs(expressions, pipelineRunState) + if err != nil { + return nil, fmt.Errorf("unable to find result referenced by when expression with input %q in task %q: %w", whenExpression.Input, name, err) + } + if resolvedResultRefs != nil { + resolvedWhenExpressions = append(resolvedWhenExpressions, resolvedResultRefs...) + } + } + } + return resolvedWhenExpressions, nil +} + // convertPipelineResultToResultRefs converts all params of the resolved pipeline run task func convertPipelineResultToResultRefs(pipelineStatus v1beta1.PipelineRunStatus, pipelineResult v1beta1.PipelineResult) ResolvedResultRefs { resolvedResultRefs, err := extractResultRefsForPipelineResult(pipelineStatus, pipelineResult) diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index 518379d9962..c06ea87b1f7 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -11,6 +11,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) @@ -323,6 +324,44 @@ func TestResolveResultRefs(t *testing.T) { }, }, }, + }, { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "$(tasks.aTask.results.aResult)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult)"}, + }, + }, + }, + }, { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{ + { + Input: "$(tasks.aTask.results.missingResult)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.missingResult)"}, + }, + }, + }, + }, { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + Params: []v1beta1.Param{ + { + Name: "bParam", + Value: v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeString, + StringVal: "$(tasks.aTask.results.missingResult)", + }, + }, + }, + }, }, } @@ -333,7 +372,7 @@ func TestResolveResultRefs(t *testing.T) { wantErr bool }{ { - name: "Test successful result references resolution", + name: "Test successful result references resolution - params", args: args{ pipelineRunState: pipelineRunState, targets: PipelineRunState{ @@ -354,8 +393,29 @@ func TestResolveResultRefs(t *testing.T) { }, }, wantErr: false, - }, - { + }, { + name: "Test successful result references resolution - when expressions", + args: args{ + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[2], + }, + }, + want: ResolvedResultRefs{ + { + Value: v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeString, + StringVal: "aResultValue", + }, + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aTask", + Result: "aResult", + }, + FromTaskRun: "aTaskRun", + }, + }, + wantErr: false, + }, { name: "Test successful result references resolution non result references", args: args{ pipelineRunState: pipelineRunState, @@ -365,6 +425,26 @@ func TestResolveResultRefs(t *testing.T) { }, want: nil, wantErr: false, + }, { + name: "Test unsuccessful result references resolution - when expression", + args: args{ + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[3], + }, + }, + want: nil, + wantErr: true, + }, { + name: "Test unsuccessful result references resolution - params", + args: args{ + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[4], + }, + }, + want: nil, + wantErr: true, }, } for _, tt := range tests {