diff --git a/config/300-crds/300-pipelinerun.yaml b/config/300-crds/300-pipelinerun.yaml index 6fb4a179738..dd12373f76c 100644 --- a/config/300-crds/300-pipelinerun.yaml +++ b/config/300-crds/300-pipelinerun.yaml @@ -2360,6 +2360,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -2740,6 +2742,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -3013,6 +3017,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -5459,6 +5465,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: diff --git a/config/300-crds/300-task.yaml b/config/300-crds/300-task.yaml index 17dc802c5f2..08888637a0a 100644 --- a/config/300-crds/300-task.yaml +++ b/config/300-crds/300-task.yaml @@ -192,6 +192,9 @@ spec: required: - name properties: + default: + description: Default + x-kubernetes-preserve-unknown-fields: true description: description: Description type: string @@ -4357,6 +4360,12 @@ spec: required: - name properties: + default: + description: |- + Default is the value a result takes if no actual value is produced by the task. + If default is set, a task may be executed without producing the result and + the default value will be used instead. + x-kubernetes-preserve-unknown-fields: true description: description: Description is a human-readable description of the result type: string diff --git a/config/300-crds/300-taskrun.yaml b/config/300-crds/300-taskrun.yaml index 1aa0dc32834..3de15dbd70b 100644 --- a/config/300-crds/300-taskrun.yaml +++ b/config/300-crds/300-taskrun.yaml @@ -1875,6 +1875,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -2148,6 +2150,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -4029,6 +4033,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -4294,6 +4300,8 @@ spec: type: boolean enableConciseResolverSyntax: type: boolean + enableDefaultResults: + type: boolean enableKeepPodOnCancel: type: boolean enableKubernetesSidecar: @@ -4512,6 +4520,12 @@ spec: required: - name properties: + default: + description: |- + Default is the value a result takes if no actual value is produced by the task. + If default is set, a task may be executed without producing the result and + the default value will be used instead. + x-kubernetes-preserve-unknown-fields: true description: description: Description is a human-readable description of the result type: string diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 0faa1e55702..ca7aa516af5 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -120,6 +120,10 @@ data: enable-artifacts: "false" # Setting this flag to "true" will enable the built-in param input validation via param enum. enable-param-enum: "false" + # Setting this flag to "true" will enable default values for task results. + # When enabled, tasks can define default values for results that will be used + # if the task doesn't produce the result. This is an alpha feature. + enable-default-results: "false" # Setting this flag to "pipeline,pipelinerun,taskrun" will prevent users from creating # embedded spec Taskruns or Pipelineruns for Pipeline, Pipelinerun and taskrun # respectively. We can specify "pipeline" to disable for Pipeline resource only. diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 2444f3e737c..701ec83db89 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -1412,6 +1412,7 @@ _Appears in:_ | `type` _[ResultsType](#resultstype)_ | Type is the user-specified type of the result. The possible type
is currently "string" and will support "array" in following work. | | Optional: \{\}
| | `properties` _object (keys:string, values:[PropertySpec](#propertyspec))_ | Properties is the JSON Schema properties to support key-value pairs results. | | Optional: \{\}
| | `description` _string_ | Description is a human-readable description of the result | | Optional: \{\}
| +| `default` _[ResultValue](#resultvalue)_ | Default is the value a result takes if no actual value is produced by the task.
If default is set, a task may be executed without producing the result and
the default value will be used instead. | | Schemaless: \{\}
Optional: \{\}
| | `value` _[ResultValue](#resultvalue)_ | Value the expression used to retrieve the value of the result from an underlying Step. | | Schemaless: \{\}
Optional: \{\}
| @@ -3871,6 +3872,7 @@ _Appears in:_ | `type` _[ResultsType](#resultstype)_ | Type is the user-specified type of the result. The possible type
is currently "string" and will support "array" in following work. | | Optional: \{\}
| | `properties` _object (keys:string, values:[PropertySpec](#propertyspec))_ | Properties is the JSON Schema properties to support key-value pairs results. | | Optional: \{\}
| | `description` _string_ | Description is a human-readable description of the result | | Optional: \{\}
| +| `default` _[ResultValue](#resultvalue)_ | Default is the value a result takes if no actual value is produced by the task.
If default is set, a task may be executed without producing the result and
the default value will be used instead. | | Schemaless: \{\}
Optional: \{\}
| | `value` _[ResultValue](#resultvalue)_ | Value the expression used to retrieve the value of the result from an underlying Step. | | Schemaless: \{\}
Optional: \{\}
| diff --git a/docs/tasks.md b/docs/tasks.md index c87aed1e885..9211f72492e 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -25,6 +25,7 @@ weight: 201 - [Specifying `Workspaces`](#specifying-workspaces) - [Emitting `Results`](#emitting-results) - [Larger `Results` using sidecar logs](#larger-results-using-sidecar-logs) + - [Default Value](#default-value) - [Specifying `Volumes`](#specifying-volumes) - [Specifying a `Step` template](#specifying-a-step-template) - [Specifying `Sidecars`](#specifying-sidecars) @@ -1059,6 +1060,91 @@ leading to bad user experience. Refer to the detailed instructions listed in [additional config](additional-configs.md#enabling-larger-results-using-sidecar-logs) to learn how to enable this feature. +#### Default Value + +This is an alpha feature guarded behind its own feature flag. The `enable-default-results` feature flag must be set to +`true` to use default values for results. + +Result declarations (within Tasks and Pipelines) can include default values that are used if the Result is not produced. +This allows you to specify defaults for `string`, `array`, and `object` results. + +##### `string` type + +```yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-with-default-string-result +spec: + results: + - name: branch + type: string + default: main +``` + +##### `array` type + +```yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-with-default-array-result +spec: + results: + - name: tags + type: array + default: + - latest + - v2.0 + - v1.0 +``` + +##### `object` type + +```yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-with-default-object-result +spec: + results: + - name: image + type: object + properties: + url: + type: string + digest: + type: string + default: + url: cr.io/foo/bar + digest: sha256:abc123 +``` + +For a more detailed example showing how default results are defined, see the [pipelinerun-with-default-results.yaml](../examples/v1/pipelineruns/alpha/pipelinerun-with-default-results.yaml). + +**Note:** If a Task does not specify a default value for a particular `Result` and fails to emit that `Result`, the +associated `TaskRun` will be marked as Failed, as described in +[TEP-0048](https://github.com/tektoncd/community/blob/main/teps/0048-task-results-without-results.md). +Below is an example where the `taskRun` fails because one of the results was not produced: + +```yaml +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + generateName: test-tr- +spec: + taskSpec: + results: + - name: result1 + description: will be produced + - name: result2 + description: will not be produced + steps: + - name: failing-step + image: busybox + script: "echo -n 123 | tee $(results.result1.path)" +``` + ### Specifying Volumes Specifies one or more [`Volumes`](https://kubernetes.io/docs/concepts/storage/volumes/) that the `Steps` in your diff --git a/examples/v1/pipelineruns/alpha/pipelinerun-emitting-default-results.yaml b/examples/v1/pipelineruns/alpha/pipelinerun-emitting-default-results.yaml new file mode 100644 index 00000000000..adc2b0203cc --- /dev/null +++ b/examples/v1/pipelineruns/alpha/pipelinerun-emitting-default-results.yaml @@ -0,0 +1,96 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + generateName: pipelinerun-emit-default-results- +spec: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: array-results + type: array + description: The array results + default: + - "1" + - "2" + - "3" + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "this script won't emit array result" + - name: task2 + taskSpec: + results: + - name: object-results + type: object + description: The object results + properties: + foo: {type: string} + hello: {type: string} + default: + foo: bar + hello: world + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "this script won't emit object result" + - name: task3 + # task3 is skipped because the when expression is never satisfied. + # The string-result declared below is never written by the step, so the + # pipeline result that references it is populated with the declared default. + when: + - input: "foo" + operator: in + values: ["bar"] + taskSpec: + results: + - name: string-result + type: string + description: The string result + default: "default-string-value" + steps: + - name: write-string + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "this step is skipped and won't emit string result" + results: + - name: array-results + type: array + description: whole array + value: $(tasks.task1.results.array-results[*]) + - name: array-results-from-array-indexing-and-object-elements + type: array + description: whole array + value: + [ + "$(tasks.task1.results.array-results[0])", + "$(tasks.task2.results.object-results.foo)", + ] + - name: array-indexing-results + type: string + description: array element + value: $(tasks.task1.results.array-results[1]) + - name: object-results + type: object + description: whole object + value: $(tasks.task2.results.object-results[*]) + - name: object-results-from-array-indexing-and-object-elements + type: object + description: whole object + value: + key1: $(tasks.task1.results.array-results[1]) + key2: $(tasks.task2.results.object-results.hello) + - name: object-element + type: string + description: object element + value: $(tasks.task2.results.object-results.foo) + - name: string-result-from-skipped-task + type: string + description: string result from skipped task3, populated with the declared default + value: $(tasks.task3.results.string-result) \ No newline at end of file diff --git a/examples/v1/pipelineruns/alpha/pipelinerun-with-default-results.yaml b/examples/v1/pipelineruns/alpha/pipelinerun-with-default-results.yaml new file mode 100644 index 00000000000..9d20907a44d --- /dev/null +++ b/examples/v1/pipelineruns/alpha/pipelinerun-with-default-results.yaml @@ -0,0 +1,117 @@ +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + generateName: pipeline-default-results- +spec: + pipelineSpec: + results: + - name: string-result + description: String result from task1 (default since task1 fails to produce it) + value: $(tasks.task1.results.string-result) + - name: array-results + type: array + description: Array results from task2 (default since task2 fails to produce it) + value: $(tasks.task2.results.array-results[*]) + tasks: + - name: task1 + taskSpec: + results: + - name: string-result + description: String result with default value + default: "default-string-from-task1" + steps: + - name: step-that-fails + image: bash:latest + onError: continue + script: | + #!/usr/bin/env bash + echo "This step fails, so the default string result will be used" + exit 1 + # This won't execute; default result "default-string-from-task1" is used instead + echo -n "actual-string-value" | tee $(results.string-result.path) + - name: task2 + runAfter: + - task1 + params: + - name: input-string + value: $(tasks.task1.results.string-result) + taskSpec: + params: + - name: input-string + description: String result consumed from task1 + results: + - name: array-results + type: array + description: Array result with default value + default: + - "default-1" + - "default-2" + - "default-3" + steps: + - name: step-that-fails + image: bash:latest + onError: continue + script: | + #!/usr/bin/env bash + echo "Consumed task1 result: $(params.input-string)" + echo "This step fails, so the default array result will be used" + exit 1 + # This won't execute; default result ["default-1","default-2","default-3"] is used instead + echo -n '["actual-1","actual-2"]' | tee $(results.array-results.path) + # task3 is skipped because its when expression evaluates to false. + # Its result has a default value that the finally task can use. + - name: task3 + when: + - input: "false" + operator: in + values: ["true"] + taskSpec: + results: + - name: skipped-task-result + description: Result with default, produced only when task3 runs + default: "default-from-skipped-task3" + steps: + - name: produce-result + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "actual-value-from-task3" | tee $(results.skipped-task-result.path) + finally: + # finally-task-1 consumes a result from task1 which fails to produce it. + # The default result value "default-string-from-task1" is used instead. + - name: finally-task-1 + params: + - name: task1-result + value: $(tasks.task1.results.string-result) + taskSpec: + params: + - name: task1-result + description: Result from task1, falls back to default when task1 fails + steps: + - name: use-default-result + image: bash:latest + script: | + #!/usr/bin/env bash + echo "finally-task-1 received task1 result (or its default): $(params.task1-result)" + if [ $(params.task1-result) != "default-string-from-task1" ]; then + exit 1 + fi + # finally-task-2 consumes a result from task3 which is skipped (when expression is false). + # The default result value "default-from-skipped-task3" is used instead. + - name: finally-task-2 + params: + - name: task3-result + value: $(tasks.task3.results.skipped-task-result) + taskSpec: + params: + - name: task3-result + description: Result from task3, falls back to default when task3 is skipped + steps: + - name: use-default-result + image: bash:latest + script: | + #!/usr/bin/env bash + echo "finally-task-2 received task3 result (or its default): $(params.task3-result)" + if [ $(params.task3-result) != "default-from-skipped-task3" ]; then + exit 1 + fi \ No newline at end of file diff --git a/examples/v1/pipelineruns/beta/pipelinerun-with-matrix-context-variables.yaml b/examples/v1/pipelineruns/beta/pipelinerun-with-matrix-context-variables.yaml index 5ae1b232047..08d1559f263 100644 --- a/examples/v1/pipelineruns/beta/pipelinerun-with-matrix-context-variables.yaml +++ b/examples/v1/pipelineruns/beta/pipelinerun-with-matrix-context-variables.yaml @@ -41,8 +41,8 @@ spec: echo "Error: expected matrix results to have the length 3 but has length $(params.matrixresultslength-1)" exit 1 fi - if [ "$(params.matrixresultslength-2)" != 1 ]; then - echo "Error: expected matrix results to have the length 1 but has length $(params.matrixresultslength-2)" + if [ "$(params.matrixresultslength-2)" != 3 ]; then + echo "Error: expected matrix results to have the length 3 but has length $(params.matrixresultslength-2)" exit 1 fi echo "Done validating the length of the matrix context variable" @@ -66,9 +66,7 @@ spec: #!/usr/bin/env bash echo "Building image for $(params.IMAGE)" echo -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-DIGEST.path) - if [ -z $(params.DIGEST) ]; then - echo -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-URL.path) - fi + echo -n "$(params.IMAGE)" | sha256sum | tee $(results.IMAGE-URL.path) --- apiVersion: tekton.dev/v1 kind: PipelineRun diff --git a/examples/v1/taskruns/alpha/default-results.yaml b/examples/v1/taskruns/alpha/default-results.yaml new file mode 100644 index 00000000000..69ae20b1ae3 --- /dev/null +++ b/examples/v1/taskruns/alpha/default-results.yaml @@ -0,0 +1,46 @@ +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + generateName: default-results- +spec: + taskSpec: + results: + - name: result1 + description: A result that will be produced + - name: string-result + description: A string result with default value + default: "default-string-value" + - name: array-result + type: array + description: An array result with default value + default: + - "foo" + - "bar" + - "baz" + - name: object-result + type: object + description: An object result with default value + properties: + url: + type: string + digest: + type: string + default: + url: "https://default.example.com" + digest: "sha256:default" + steps: + - name: produce-result1 + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "actual-value" | tee $(results.result1.path) + - name: step-with-error + onError: continue + image: bash:latest + script: | + #!/usr/bin/env bash + # This step fails intentionally, so string-result, array-result, + # and object-result will use their default values + echo "This step fails, but default results will be used" + exit 1 + echo -n "this-wont-be-written" | tee $(results.string-result.path) \ No newline at end of file diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index c2de63f06f8..19aa05f889f 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -103,6 +103,8 @@ const ( EnableArtifacts = "enable-artifacts" // EnableParamEnum is the flag to enabled enum in params EnableParamEnum = "enable-param-enum" + // EnableDefaultResults is the flag to enable default values for task results + EnableDefaultResults = "enable-default-results" // EnableConciseResolverSyntax is the flag to enable concise resolver syntax EnableConciseResolverSyntax = "enable-concise-resolver-syntax" // EnableKubernetesSidecar is the flag to enable kubernetes sidecar support @@ -179,6 +181,13 @@ var ( Enabled: DefaultAlphaFeatureEnabled, } + // DefaultEnableDefaultResults is the default PerFeatureFlag value for EnableDefaultResults + DefaultEnableDefaultResults = PerFeatureFlag{ + Name: EnableDefaultResults, + Stability: AlphaAPIFields, + Enabled: DefaultAlphaFeatureEnabled, + } + // DefaultEnableConciseResolverSyntax is the default PerFeatureFlag value for EnableConciseResolverSyntax DefaultEnableConciseResolverSyntax = PerFeatureFlag{ Name: EnableConciseResolverSyntax, @@ -229,6 +238,7 @@ type FeatureFlags struct { // EnableStepActions is a no-op flag since StepActions are stable EnableStepActions bool `json:"enableStepActions,omitempty"` EnableParamEnum bool `json:"enableParamEnum,omitempty"` + EnableDefaultResults bool `json:"enableDefaultResults,omitempty"` EnableArtifacts bool `json:"enableArtifacts,omitempty"` DisableInlineSpec string `json:"disableInlineSpec,omitempty"` EnableConciseResolverSyntax bool `json:"enableConciseResolverSyntax,omitempty"` @@ -332,6 +342,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setPerFeatureFlag(EnableParamEnum, DefaultEnableParamEnum, &tc.EnableParamEnum); err != nil { return nil, err } + if err := setPerFeatureFlag(EnableDefaultResults, DefaultEnableDefaultResults, &tc.EnableDefaultResults); err != nil { + return nil, err + } if err := setPerFeatureFlag(EnableArtifacts, DefaultEnableArtifacts, &tc.EnableArtifacts); err != nil { return nil, err } diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 6cbcb68b5e7..6eeb19c0eac 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -3718,6 +3718,12 @@ func schema_pkg_apis_pipeline_v1_TaskResult(ref common.ReferenceCallback) common Format: "", }, }, + "default": { + SchemaProps: spec.SchemaProps{ + Description: "Default is the value a result takes if no actual value is produced by the task. If default is set, a task may be executed without producing the result and the default value will be used instead.", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamValue"), + }, + }, "value": { SchemaProps: spec.SchemaProps{ Description: "Value the expression used to retrieve the value of the result from an underlying Step.", diff --git a/pkg/apis/pipeline/v1/result_types.go b/pkg/apis/pipeline/v1/result_types.go index 0e6d28d5fee..cb888d20e8e 100644 --- a/pkg/apis/pipeline/v1/result_types.go +++ b/pkg/apis/pipeline/v1/result_types.go @@ -33,6 +33,14 @@ type TaskResult struct { // +optional Description string `json:"description,omitempty"` + // Default is the value a result takes if no actual value is produced by the task. + // If default is set, a task may be executed without producing the result and + // the default value will be used instead. + // +optional + // +kubebuilder:pruning:PreserveUnknownFields + // +kubebuilder:validation:Schemaless + Default *ResultValue `json:"default,omitempty"` + // Value the expression used to retrieve the value of the result from an underlying Step. // +optional // +kubebuilder:pruning:PreserveUnknownFields diff --git a/pkg/apis/pipeline/v1/result_validation.go b/pkg/apis/pipeline/v1/result_validation.go index 6f4298daf84..40e9e5637e6 100644 --- a/pkg/apis/pipeline/v1/result_validation.go +++ b/pkg/apis/pipeline/v1/result_validation.go @@ -18,6 +18,7 @@ import ( "fmt" "regexp" + "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -39,7 +40,8 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { case tr.Type != ResultsTypeString: errs = errs.Also(apis.ErrInvalidValue(tr.Type, "type", "type must be string")) } - return errs.Also(tr.validateValue(ctx)) + errs = errs.Also(tr.validateValue(ctx)) + return errs.Also(tr.validateDefault(ctx)) } // validateObjectResult validates the object result and check if the Properties is missing @@ -107,6 +109,52 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError) return errs } +// validateDefault validates the default value of the TaskResult. +// It ensures that the default value type matches the result type. +func (tr TaskResult) validateDefault(ctx context.Context) (errs *apis.FieldError) { + if tr.Default == nil { + return nil + } + + // Check if the feature flag is enabled + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults { + return apis.ErrGeneric(fmt.Sprintf("feature flag %q must be set to true to use default values for results", config.EnableDefaultResults), tr.Name+".default") + } + + // Determine the expected type based on the result type + expectedType := ParamTypeString + if tr.Type == ResultsTypeArray { + expectedType = ParamTypeArray + } else if tr.Type == ResultsTypeObject { + expectedType = ParamTypeObject + } + + // Validate that the default type matches the result type + if tr.Default.Type != expectedType { + return &apis.FieldError{ + Message: fmt.Sprintf( + "Invalid default type. Result type is %q but default type is %q", tr.Type, tr.Default.Type), + Paths: []string{ + tr.Name + ".default", + }, + } + } + + // For object results, validate that default object keys match the properties schema + if tr.Type == ResultsTypeObject && tr.Properties != nil { + for key := range tr.Default.ObjectVal { + if _, ok := tr.Properties[key]; !ok { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("default object key %q is not defined in properties", key), + Paths: []string{tr.Name + ".default"}, + }) + } + } + } + + return errs +} + // Validate implements apis.Validatable func (sr StepResult) Validate(ctx context.Context) (errs *apis.FieldError) { if !resultNameFormatRegex.MatchString(sr.Name) { diff --git a/pkg/apis/pipeline/v1/result_validation_test.go b/pkg/apis/pipeline/v1/result_validation_test.go index 10c36f783be..dbab5cd0733 100644 --- a/pkg/apis/pipeline/v1/result_validation_test.go +++ b/pkg/apis/pipeline/v1/result_validation_test.go @@ -19,10 +19,12 @@ package v1_test import ( "errors" "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" "knative.dev/pkg/apis" @@ -409,3 +411,122 @@ func TestStepResultsValidateError(t *testing.T) { }) } } + +func TestTaskResultValidateDefault(t *testing.T) { + tests := []struct { + name string + result v1.TaskResult + featureFlag bool + expectedError string + }{{ + name: "valid default value with feature flag enabled - string", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeString, + Description: "my great result", + Default: v1.NewStructuredValues("defaultValue"), + }, + featureFlag: true, + expectedError: "", + }, { + name: "valid default value with feature flag enabled - array", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeArray, + Description: "my great result", + Default: v1.NewStructuredValues("value1", "value2"), + }, + featureFlag: true, + expectedError: "", + }, { + name: "valid default value with feature flag enabled - object", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}, "key2": {Type: v1.ParamTypeString}}, + Default: v1.NewObject(map[string]string{"key1": "value1", "key2": "value2"}), + }, + featureFlag: true, + expectedError: "", + }, { + name: "default value rejected when feature flag disabled", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeString, + Description: "my great result", + Default: v1.NewStructuredValues("defaultValue"), + }, + featureFlag: false, + expectedError: "feature flag \"enable-default-results\" must be set to true to use default values for results", + }, { + name: "invalid default type - string result with array default", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeString, + Description: "my great result", + Default: v1.NewStructuredValues("value1", "value2"), + }, + featureFlag: true, + expectedError: "Invalid default type. Result type is \"string\" but default type is \"array\"", + }, { + name: "invalid default type - array result with string default", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeArray, + Description: "my great result", + Default: v1.NewStructuredValues("singleValue"), + }, + featureFlag: true, + expectedError: "Invalid default type. Result type is \"array\" but default type is \"string\"", + }, { + name: "invalid default type - object result with string default", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, + Default: v1.NewStructuredValues("stringValue"), + }, + featureFlag: true, + expectedError: "Invalid default type. Result type is \"object\" but default type is \"string\"", + }, { + name: "invalid default object key not in properties", + result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, + Default: v1.NewObject(map[string]string{"key1": "value1", "invalidKey": "value2"}), + }, + featureFlag: true, + expectedError: "default object key \"invalidKey\" is not defined in properties", + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := t.Context() + if tt.featureFlag { + ctx = cfgtesting.SetFeatureFlags(ctx, t, map[string]string{ + "enable-default-results": "true", + }) + } else { + ctx = cfgtesting.SetFeatureFlags(ctx, t, map[string]string{ + "enable-default-results": "false", + }) + } + err := tt.result.Validate(ctx) + if tt.expectedError == "" { + if err != nil { + t.Errorf("TaskResult.Validate() = %v, expected no error", err) + } + } else { + if err == nil { + t.Fatalf("Expected an error containing %q, got nothing", tt.expectedError) + } + if !strings.Contains(err.Error(), tt.expectedError) { + t.Errorf("TaskResult.Validate() error = %v, expected to contain %q", err.Error(), tt.expectedError) + } + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index a0964b1d49b..409e066e977 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1910,6 +1910,10 @@ "name" ], "properties": { + "default": { + "description": "Default is the value a result takes if no actual value is produced by the task. If default is set, a task may be executed without producing the result and the default value will be used instead.", + "$ref": "#/definitions/v1.ParamValue" + }, "description": { "description": "Description is a human-readable description of the result", "type": "string" diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 3cddb19a5dc..3ab72d551aa 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1734,6 +1734,11 @@ func (in *TaskResult) DeepCopyInto(out *TaskResult) { (*out)[key] = val } } + if in.Default != nil { + in, out := &in.Default, &out.Default + *out = new(ParamValue) + (*in).DeepCopyInto(*out) + } if in.Value != nil { in, out := &in.Value, &out.Value *out = new(ParamValue) diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index df40de7089d..d0472580c44 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -5011,6 +5011,12 @@ func schema_pkg_apis_pipeline_v1beta1_TaskResult(ref common.ReferenceCallback) c Format: "", }, }, + "default": { + SchemaProps: spec.SchemaProps{ + Description: "Default is the value a result takes if no actual value is produced by the task. If default is set, a task may be executed without producing the result and the default value will be used instead.", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamValue"), + }, + }, "value": { SchemaProps: spec.SchemaProps{ Description: "Value the expression used to retrieve the value of the result from an underlying Step.", diff --git a/pkg/apis/pipeline/v1beta1/result_conversion.go b/pkg/apis/pipeline/v1beta1/result_conversion.go index 8854a283cb7..53faf2c9214 100644 --- a/pkg/apis/pipeline/v1beta1/result_conversion.go +++ b/pkg/apis/pipeline/v1beta1/result_conversion.go @@ -37,6 +37,10 @@ func (r TaskResult) convertTo(ctx context.Context, sink *v1.TaskResult) { sink.Value = &v1.ParamValue{} r.Value.convertTo(ctx, sink.Value) } + if r.Default != nil { + sink.Default = &v1.ParamValue{} + r.Default.convertTo(ctx, sink.Default) + } } func (r *TaskResult) convertFrom(ctx context.Context, source v1.TaskResult) { @@ -54,4 +58,8 @@ func (r *TaskResult) convertFrom(ctx context.Context, source v1.TaskResult) { r.Value = &ParamValue{} r.Value.convertFrom(ctx, *source.Value) } + if source.Default != nil { + r.Default = &ParamValue{} + r.Default.convertFrom(ctx, *source.Default) + } } diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index 302fa1b70ec..66b2149ea1e 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -33,6 +33,14 @@ type TaskResult struct { // +optional Description string `json:"description,omitempty"` + // Default is the value a result takes if no actual value is produced by the task. + // If default is set, a task may be executed without producing the result and + // the default value will be used instead. + // +optional + // +kubebuilder:pruning:PreserveUnknownFields + // +kubebuilder:validation:Schemaless + Default *ResultValue `json:"default,omitempty"` + // Value the expression used to retrieve the value of the result from an underlying Step. // +optional // +kubebuilder:pruning:PreserveUnknownFields diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index 4bdf1a7138c..5d89999226c 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -17,6 +17,7 @@ import ( "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" @@ -39,7 +40,8 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { case tr.Type != ResultsTypeString: errs = errs.Also(apis.ErrInvalidValue(tr.Type, "type", "type must be string")) } - return errs.Also(tr.validateValue(ctx)) + errs = errs.Also(tr.validateValue(ctx)) + return errs.Also(tr.validateDefault(ctx)) } // validateObjectResult validates the object result and check if the Properties is missing @@ -106,3 +108,49 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError) } return errs } + +// validateDefault validates the default value of the TaskResult. +// It ensures that the default value type matches the result type. +func (tr TaskResult) validateDefault(ctx context.Context) (errs *apis.FieldError) { + if tr.Default == nil { + return nil + } + + // Check if the feature flag is enabled + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults { + return apis.ErrGeneric(fmt.Sprintf("feature flag %q must be set to true to use default values for results", config.EnableDefaultResults), tr.Name+".default") + } + + // Determine the expected type based on the result type + expectedType := ParamTypeString + if tr.Type == ResultsTypeArray { + expectedType = ParamTypeArray + } else if tr.Type == ResultsTypeObject { + expectedType = ParamTypeObject + } + + // Validate that the default type matches the result type + if tr.Default.Type != expectedType { + return &apis.FieldError{ + Message: fmt.Sprintf( + "Invalid default type. Result type is %q but default type is %q", tr.Type, tr.Default.Type), + Paths: []string{ + tr.Name + ".default", + }, + } + } + + // For object results, validate that default object keys match the properties schema + if tr.Type == ResultsTypeObject && tr.Properties != nil { + for key := range tr.Default.ObjectVal { + if _, ok := tr.Properties[key]; !ok { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("default object key %q is not defined in properties", key), + Paths: []string{tr.Name + ".default"}, + }) + } + } + } + + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index cd95f64e142..c99701a06cf 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2755,6 +2755,10 @@ "name" ], "properties": { + "default": { + "description": "Default is the value a result takes if no actual value is produced by the task. If default is set, a task may be executed without producing the result and the default value will be used instead.", + "$ref": "#/definitions/v1beta1.ParamValue" + }, "description": { "description": "Description is a human-readable description of the result", "type": "string" diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 5360103be4b..456cd382989 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -355,6 +355,23 @@ spec: stepActionTaskResultV1beta1 := parse.MustParseV1beta1Task(t, stepActionTaskResultYAML) stepActionTaskResultV1 := parse.MustParseV1Task(t, stepActionTaskResultYAML) + taskResultWithDefaultYAML := ` +metadata: + name: foo + namespace: bar +spec: + results: + - name: my-result + type: string + default: "fallback-value" + steps: + - name: produce + image: bash:latest + script: echo -n "hello" | tee $(results.my-result.path) +` + taskResultWithDefaultV1beta1 := parse.MustParseV1beta1Task(t, taskResultWithDefaultYAML) + taskResultWithDefaultV1 := parse.MustParseV1Task(t, taskResultWithDefaultYAML) + remoteStepActionTaskV1beta1 := parse.MustParseV1beta1Task(t, remoteStepActionTaskYAML) remoteStepActionTaskV1 := parse.MustParseV1Task(t, remoteStepActionTaskYAML) @@ -406,6 +423,10 @@ spec: name: "value in task result", v1beta1Task: stepActionTaskResultV1beta1, v1Task: stepActionTaskResultV1, + }, { + name: "default value in task result", + v1beta1Task: taskResultWithDefaultV1beta1, + v1Task: taskResultWithDefaultV1, }, { name: "remote step action in task", v1beta1Task: remoteStepActionTaskV1beta1, diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 61de6093a8f..ba2ee6ed58f 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -2309,6 +2309,11 @@ func (in *TaskResult) DeepCopyInto(out *TaskResult) { (*out)[key] = val } } + if in.Default != nil { + in, out := &in.Default, &out.Default + *out = new(ParamValue) + (*in).DeepCopyInto(*out) + } if in.Value != nil { in, out := &in.Value, &out.Value *out = new(ParamValue) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 38667abd57a..033ec115675 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -773,10 +773,11 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel // Build PipelineRunFacts with a list of resolved pipeline tasks, // dag tasks graph and final tasks graph pipelineRunFacts := &resources.PipelineRunFacts{ - State: pipelineRunState, - SpecStatus: pr.Spec.Status, - TasksGraph: d, - FinalTasksGraph: dfinally, + State: pipelineRunState, + SpecStatus: pr.Spec.Status, + TasksGraph: d, + FinalTasksGraph: dfinally, + EnableDefaultResults: config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults, TimeoutsState: resources.PipelineRunTimeoutsState{ Clock: c.Clock, }, @@ -956,7 +957,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel after = pr.Status.GetCondition(apis.ConditionSucceeded) pr.Status.StartTime = pipelineRunFacts.State.AdjustStartTime(pr.Status.StartTime) - pr.Status.ChildReferences = pipelineRunFacts.GetChildReferences() + pr.Status.ChildReferences = pipelineRunFacts.GetChildReferences(ctx) pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() pipelineTaskStatus := pipelineRunFacts.GetPipelineTaskStatus() @@ -965,10 +966,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel if after.Status == corev1.ConditionTrue || after.Status == corev1.ConditionFalse { pr.Status.Results, err = resources.ApplyTaskResultsToPipelineResults( + ctx, pipelineSpec.Results, pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults(), pipelineTaskStatus, + pipelineRunFacts.State.GetTaskResultDefaults(), ) if err != nil { pr.Status.MarkFailed(v1.PipelineRunReasonCouldntGetPipelineResult.String(), @@ -1003,7 +1006,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline // Check for Missing Result References // if error found, present rpt will be // added to the validationFailedTask list - err := resources.CheckMissingResultReferences(pipelineRunFacts.State, rpt) + err := resources.CheckMissingResultReferences(pipelineRunFacts.EnableDefaultResults, pipelineRunFacts.State, rpt) if err != nil { // Use Errorf when a task succeeded but didn't emit a result (surprising), // Infof when the referenced task failed (expected that results are missing). @@ -1037,7 +1040,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline // Before creating TaskRun for scheduled final task, check if it's consuming a task result // Resolve and apply task result wherever applicable, report warning in case resolution fails for _, rpt := range fNextRpts { - resolvedResultRefs, _, err := resources.ResolveResultRef(pipelineRunFacts.State, rpt) + resolvedResultRefs, _, err := resources.ResolveResultRef(pipelineRunFacts.EnableDefaultResults, pipelineRunFacts.State, rpt) if err != nil { logger.Infof("Final task %q is not executed as it could not resolve task params for %q: %v", rpt.PipelineTask.Name, pr.Name, err) continue diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 3e72c511850..def04f3be39 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -17,11 +17,13 @@ limitations under the License. package resources import ( + "context" "encoding/json" "fmt" "strconv" "strings" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" @@ -719,11 +721,15 @@ func PropagateArtifacts(rpt *ResolvedPipelineTask, runStates PipelineRunState) e // non-existent TaskResults or failed TaskRuns or Runs result in a PipelineResult being considered invalid // and omitted from the returned slice. A nil slice is returned if no results are passed in or all // results are invalid. +// If taskResultDefaults is provided and the enable-default-results feature flag is enabled, default values +// will be used for task results that are not initialized. func ApplyTaskResultsToPipelineResults( + ctx context.Context, results []v1.PipelineResult, taskRunResults map[string][]v1.TaskRunResult, customTaskResults map[string][]v1beta1.CustomRunResult, taskstatus map[string]string, + taskResultDefaults map[string]map[string]*v1.ResultValue, ) ([]v1.PipelineRunResult, error) { var runResults []v1.PipelineRunResult var invalidPipelineResults []string @@ -783,6 +789,27 @@ func ApplyTaskResultsToPipelineResults( } } else if resultValue := runResultValue(taskName, resultName, customTaskResults); resultValue != nil { stringReplacements[variable] = *resultValue + } else if defaultValue := taskResultDefaultValue(ctx, taskName, resultName, taskResultDefaults); defaultValue != nil { + // Use default value if the result is not found but a default is defined + switch defaultValue.Type { + case v1.ParamTypeString: + stringReplacements[variable] = defaultValue.StringVal + case v1.ParamTypeArray: + if stringIdx != "*" { + intIdx, _ := strconv.Atoi(stringIdx) + if intIdx < len(defaultValue.ArrayVal) { + stringReplacements[variable] = defaultValue.ArrayVal[intIdx] + } else { + // referred array index out of bound + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) + validPipelineResult = false + } + } else { + arrayReplacements[substitution.StripStarVarSubExpression(variable)] = defaultValue.ArrayVal + } + case v1.ParamTypeObject: + objectReplacements[substitution.StripStarVarSubExpression(variable)] = defaultValue.ObjectVal + } } else { // if the task is not successful (e.g. skipped or failed) and the results is missing, don't return error if status, ok := taskstatus[PipelineTaskStatusPrefix+taskName+PipelineTaskStatusSuffix]; ok { @@ -807,6 +834,21 @@ func ApplyTaskResultsToPipelineResults( invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) validPipelineResult = false } + } else if defaultValue := taskResultDefaultValue(ctx, taskName, resultName, taskResultDefaults); defaultValue != nil { + // Use default value if the result is not found but a default is defined + if defaultValue.Type == v1.ParamTypeObject { + if _, ok := defaultValue.ObjectVal[objectKey]; ok { + stringReplacements[variable] = defaultValue.ObjectVal[objectKey] + } else { + // referred object key is not existent in default + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) + validPipelineResult = false + } + } else { + // default type doesn't match expected object type + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) + validPipelineResult = false + } } else { // if the task is not successful (e.g. skipped or failed) and the results is missing, don't return error if status, ok := taskstatus[PipelineTaskStatusPrefix+taskName+PipelineTaskStatusSuffix]; ok { @@ -865,6 +907,24 @@ func runResultValue(taskName string, resultName string, runResults map[string][] return nil } +// taskResultDefaultValue returns the default value for a given task result if the enable-default-results feature flag +// is enabled and a default is defined in the task spec. It returns nil if the feature flag is disabled, the task +// doesn't have defaults defined, or the specific result doesn't have a default value. +func taskResultDefaultValue(ctx context.Context, taskName string, resultName string, taskResultDefaults map[string]map[string]*v1.ResultValue) *v1.ResultValue { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults { + return nil + } + if taskResultDefaults == nil { + return nil + } + if taskDefaults, ok := taskResultDefaults[taskName]; ok { + if defaultValue, ok := taskDefaults[resultName]; ok { + return defaultValue + } + } + return nil +} + // ApplyParametersToWorkspaceBindings applies parameters from PipelineSpec and PipelineRun to the WorkspaceBindings in a PipelineRun. It replaces // placeholders in various binding types with values from provided parameters. func ApplyParametersToWorkspaceBindings(pr *v1.PipelineRun) { diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 2753a2002b7..e8718b205fb 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -17,11 +17,13 @@ limitations under the License. package resources_test import ( + "context" "errors" "strings" "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" @@ -4244,7 +4246,7 @@ func TestApplyFinallyResultsToPipelineResults(t *testing.T) { }, } { t.Run(tc.description, func(t *testing.T) { - received, _ := resources.ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults, nil /* skippedTasks */) + received, _ := resources.ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, nil /* skippedTasks */, nil /* taskResultDefaults */) if d := cmp.Diff(tc.expected, received); d != "" { t.Error(diff.PrintWantGot(d)) } @@ -4577,7 +4579,7 @@ func TestApplyTaskResultsToPipelineResults_Success(t *testing.T) { }}, }} { t.Run(tc.description, func(t *testing.T) { - received, err := resources.ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults, tc.taskstatus) + received, err := resources.ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, tc.taskstatus, nil /* taskResultDefaults */) if err != nil { t.Errorf("Got unexpected error:%v", err) } @@ -4793,7 +4795,7 @@ func TestApplyTaskResultsToPipelineResults_Error(t *testing.T) { expectedError: errors.New("invalid pipelineresults [foo], the referenced results don't exist"), }} { t.Run(tc.description, func(t *testing.T) { - received, err := resources.ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults, nil /*skipped tasks*/) + received, err := resources.ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, nil /*skipped tasks*/, nil /* taskResultDefaults */) if err == nil { t.Errorf("Expect error but got nil") return @@ -4810,6 +4812,143 @@ func TestApplyTaskResultsToPipelineResults_Error(t *testing.T) { } } +func TestApplyTaskResultsToPipelineResults_DefaultResults(t *testing.T) { + for _, tc := range []struct { + description string + results []v1.PipelineResult + taskResults map[string][]v1.TaskRunResult + taskResultDefaults map[string]map[string]*v1.ResultValue + enableDefaults bool + expectedResults []v1.PipelineRunResult + expectedError error + }{{ + description: "use default when result not produced and feature enabled", + results: []v1.PipelineResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("$(tasks.pt1.results.foo)"), + }}, + taskResults: map[string][]v1.TaskRunResult{ + "pt1": {}, // task succeeded but didn't produce the result + }, + taskResultDefaults: map[string]map[string]*v1.ResultValue{ + "pt1": { + "foo": v1.NewStructuredValues("default-value"), + }, + }, + enableDefaults: true, + expectedResults: []v1.PipelineRunResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("default-value"), + }}, + }, { + description: "use actual result when available even with default defined", + results: []v1.PipelineResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("$(tasks.pt1.results.foo)"), + }}, + taskResults: map[string][]v1.TaskRunResult{ + "pt1": {{ + Name: "foo", + Value: *v1.NewStructuredValues("actual-value"), + }}, + }, + taskResultDefaults: map[string]map[string]*v1.ResultValue{ + "pt1": { + "foo": v1.NewStructuredValues("default-value"), + }, + }, + enableDefaults: true, + expectedResults: []v1.PipelineRunResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("actual-value"), + }}, + }, { + description: "use default for array result", + results: []v1.PipelineResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("$(tasks.pt1.results.array-result[*])"), + }}, + taskResults: map[string][]v1.TaskRunResult{ + "pt1": {}, + }, + taskResultDefaults: map[string]map[string]*v1.ResultValue{ + "pt1": { + "array-result": v1.NewStructuredValues("val1", "val2", "val3"), + }, + }, + enableDefaults: true, + expectedResults: []v1.PipelineRunResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("val1", "val2", "val3"), + }}, + }, { + description: "use default for object result element", + results: []v1.PipelineResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("$(tasks.pt1.results.obj-result.key1)"), + }}, + taskResults: map[string][]v1.TaskRunResult{ + "pt1": {}, + }, + taskResultDefaults: map[string]map[string]*v1.ResultValue{ + "pt1": { + "obj-result": &v1.ResultValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key1": "value1", "key2": "value2"}, + }, + }, + }, + enableDefaults: true, + expectedResults: []v1.PipelineRunResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("value1"), + }}, + }, { + description: "don't use default when feature flag disabled", + results: []v1.PipelineResult{{ + Name: "pipeline-result", + Value: *v1.NewStructuredValues("$(tasks.pt1.results.foo)"), + }}, + taskResults: map[string][]v1.TaskRunResult{ + "pt1": {}, + }, + taskResultDefaults: map[string]map[string]*v1.ResultValue{ + "pt1": { + "foo": v1.NewStructuredValues("default-value"), + }, + }, + enableDefaults: false, + expectedResults: nil, + expectedError: errors.New("invalid pipelineresults [pipeline-result], the referenced results don't exist"), + }} { + t.Run(tc.description, func(t *testing.T) { + ctx := context.Background() + if tc.enableDefaults { + ctx = config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableDefaultResults: true, + }, + }) + } + received, err := resources.ApplyTaskResultsToPipelineResults(ctx, tc.results, tc.taskResults, nil /* customTaskResults */, nil /* taskstatus */, tc.taskResultDefaults) + if tc.expectedError != nil { + if err == nil { + t.Errorf("Expected error but got nil") + return + } + if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { + t.Errorf("ApplyTaskResultsToPipelineResults() errors diff %s", diff.PrintWantGot(d)) + } + } else if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + if d := cmp.Diff(tc.expectedResults, received); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} + func TestApplyTaskRunContext(t *testing.T) { r := map[string]string{ "tasks.task1.status": "succeeded", diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 3b6863d33f1..6c5835b9ae0 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -544,7 +544,7 @@ func (t *ResolvedPipelineTask) skipBecauseParentTaskWasSkipped(facts *PipelineRu // reason for skipping the task, and applies result references if found func (t *ResolvedPipelineTask) skipBecauseResultReferencesAreMissing(facts *PipelineRunFacts) bool { if t.checkParentsDone(facts) && t.hasResultReferences() { - resolvedResultRefs, pt, err := ResolveResultRefs(facts.State, PipelineRunState{t}) + resolvedResultRefs, pt, err := ResolveResultRefs(facts.EnableDefaultResults, facts.State, PipelineRunState{t}) rpt := facts.State.ToMap()[pt] if rpt != nil { if err != nil && @@ -720,7 +720,7 @@ func ResolvePipelineTask( // We want to resolve all of the result references and ignore any errors at this point since there could be // instances where result references are missing here, but will be later skipped and resolved in // skipBecauseResultReferencesAreMissing. The final validation is handled in CheckMissingResultReferences. - resolvedResultRefs, _, _ := ResolveResultRefs(pst, PipelineRunState{&rpt}) + resolvedResultRefs, _, _ := ResolveResultRefs(config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults, pst, PipelineRunState{&rpt}) if err := validateArrayResultsIndex(resolvedResultRefs); err != nil { return nil, err } @@ -765,6 +765,17 @@ func ResolvePipelineTask( return nil, err } } + // If no TaskRun exists for this task (e.g. it is skipped due to a when expression or has + // not been scheduled yet), ResolvedTask is still nil because setTaskRunsAndResolvedTask was + // never called. Resolve the task spec without a TaskRun so that declared result defaults are + // available for pipeline result population regardless of whether the task actually ran. + if rpt.ResolvedTask == nil { + rt, err := resolveTask(ctx, nil, getTask, pipelineTask) + if err != nil { + return nil, err + } + rpt.ResolvedTask = rt + } } return &rpt, nil @@ -1039,7 +1050,7 @@ func isCustomRunCancelledByPipelineRunTimeout(cr *v1beta1.CustomRun) bool { // CheckMissingResultReferences returns an error if it is missing any result references. // Missing result references can occur if task fails to produce a result but has // OnError: continue (ie TestMissingResultWhenStepErrorIsIgnored) -func CheckMissingResultReferences(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) error { +func CheckMissingResultReferences(enableDefaultResults bool, pipelineRunState PipelineRunState, target *ResolvedPipelineTask) error { for _, resultRef := range v1.PipelineTaskResultRefs(target.PipelineTask) { referencedPipelineTask, ok := pipelineRunState.ToMap()[resultRef.PipelineTask] if !ok { @@ -1066,7 +1077,7 @@ func CheckMissingResultReferences(pipelineRunState PipelineRunState, target *Res return fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length TaskRuns", resultRef.PipelineTask) } taskRun := referencedPipelineTask.TaskRuns[0] - _, err := findTaskResultForParam(taskRun, resultRef) + _, err := findTaskResultForParam(enableDefaultResults, taskRun, resultRef, referencedPipelineTask.ResolvedTask) if err != nil { if referencedPipelineTask.isSuccessful() { return &MissingResultFromCompletedTaskError{ diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 993e43a963a..d6e5b60a731 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -3538,6 +3538,143 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) { } } +func TestResolvedPipelineRunTask_IsFinallySkipped_WithDefaultResults(t *testing.T) { + makeFacts := func() (*ResolvedPipelineTask, *PipelineRunFacts) { + failedTaskRun := &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "failed-dag-task"}, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}, + }, + }, + } + dagTask := &ResolvedPipelineTask{ + TaskRunNames: []string{"failed-dag-task"}, + TaskRuns: []*v1.TaskRun{failedTaskRun}, + PipelineTask: &v1.PipelineTask{ + Name: "dag-task", + TaskRef: &v1.TaskRef{Name: "task"}, + }, + ResolvedTask: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "my-result", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("default-value"), + }}, + }, + }, + } + finallyTask := &ResolvedPipelineTask{ + PipelineTask: &v1.PipelineTask{ + Name: "finally-task", + TaskRef: &v1.TaskRef{Name: "task"}, + Params: v1.Params{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.dag-task.results.my-result)"), + }}, + }, + } + state := PipelineRunState{dagTask, finallyTask} + d, _ := dag.Build(v1.PipelineTaskList([]v1.PipelineTask{*dagTask.PipelineTask}), map[string][]string{}) + dfinally, _ := dag.Build(v1.PipelineTaskList([]v1.PipelineTask{*finallyTask.PipelineTask}), map[string][]string{}) + facts := &PipelineRunFacts{ + State: state, + TasksGraph: d, + FinalTasksGraph: dfinally, + TimeoutsState: PipelineRunTimeoutsState{Clock: testClock}, + } + return finallyTask, facts + } + + t.Run("finally task NOT skipped when dag task fails but result has default and feature flag enabled", func(t *testing.T) { + finallyTask, facts := makeFacts() + facts.EnableDefaultResults = true + skipStatus := finallyTask.IsFinallySkipped(facts) + if skipStatus.IsSkipped { + t.Errorf("expected finally task to NOT be skipped when default result is available, got skippingReason: %v", skipStatus.SkippingReason) + } + }) + + t.Run("finally task IS skipped when dag task fails and result has default but feature flag disabled", func(t *testing.T) { + finallyTask, facts := makeFacts() + skipStatus := finallyTask.IsFinallySkipped(facts) + if !skipStatus.IsSkipped { + t.Errorf("expected finally task to be skipped when default results feature flag is disabled") + } + if skipStatus.SkippingReason != v1.MissingResultsSkip { + t.Errorf("expected skipping reason MissingResultsSkip, got: %v", skipStatus.SkippingReason) + } + }) + + makeFactsSkippedDAGTask := func() (*ResolvedPipelineTask, *PipelineRunFacts) { + // dagTask has no TaskRuns - it was skipped due to a when expression evaluating to false + dagTask := &ResolvedPipelineTask{ + PipelineTask: &v1.PipelineTask{ + Name: "dag-task", + TaskRef: &v1.TaskRef{Name: "task"}, + When: v1.WhenExpressions{{ + Input: "false", + Operator: selection.In, + Values: []string{"true"}, + }}, + }, + ResolvedTask: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "my-result", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("default-from-skipped"), + }}, + }, + }, + } + finallyTask := &ResolvedPipelineTask{ + PipelineTask: &v1.PipelineTask{ + Name: "finally-task", + TaskRef: &v1.TaskRef{Name: "task"}, + Params: v1.Params{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.dag-task.results.my-result)"), + }}, + }, + } + state := PipelineRunState{dagTask, finallyTask} + d, _ := dag.Build(v1.PipelineTaskList([]v1.PipelineTask{*dagTask.PipelineTask}), map[string][]string{}) + dfinally, _ := dag.Build(v1.PipelineTaskList([]v1.PipelineTask{*finallyTask.PipelineTask}), map[string][]string{}) + facts := &PipelineRunFacts{ + State: state, + TasksGraph: d, + FinalTasksGraph: dfinally, + TimeoutsState: PipelineRunTimeoutsState{Clock: testClock}, + } + return finallyTask, facts + } + + t.Run("finally task NOT skipped when dag task is skipped but result has default and feature flag enabled", func(t *testing.T) { + finallyTask, facts := makeFactsSkippedDAGTask() + facts.EnableDefaultResults = true + skipStatus := finallyTask.IsFinallySkipped(facts) + if skipStatus.IsSkipped { + t.Errorf("expected finally task to NOT be skipped when referenced task is skipped but default is available, got skippingReason: %v", skipStatus.SkippingReason) + } + }) + + t.Run("finally task IS skipped when dag task is skipped and feature flag disabled", func(t *testing.T) { + finallyTask, facts := makeFactsSkippedDAGTask() + skipStatus := finallyTask.IsFinallySkipped(facts) + if !skipStatus.IsSkipped { + t.Errorf("expected finally task to be skipped when referenced task is skipped and feature flag is disabled") + } + if skipStatus.SkippingReason != v1.MissingResultsSkip { + t.Errorf("expected skipping reason MissingResultsSkip, got: %v", skipStatus.SkippingReason) + } + }) +} + func TestResolvedPipelineRunTask_IsFinallySkippedByCondition(t *testing.T) { task := &ResolvedPipelineTask{ TaskRunNames: []string{"dag-task"}, diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index cc2c9068ec6..472e552fa09 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -83,6 +83,10 @@ type PipelineRunFacts struct { // condition to help users understand why specific tasks were not executed // (e.g. missing result references). ValidationFailedErrors map[string]string + + // EnableDefaultResults indicates whether the enable-default-results feature flag is on. + // When true, tasks that do not produce a declared result use the result's declared Default value. + EnableDefaultResults bool } // PipelineRunTimeoutsState records information about start times and timeouts for the PipelineRun, so that the PipelineRunFacts @@ -231,6 +235,34 @@ func (state PipelineRunState) GetTaskRunsArtifacts() map[string]*v1.Artifacts { return results } +// GetTaskResultDefaults returns a map of task result defaults from the resolved tasks. +// The map is keyed by pipeline task name, and the value is a map of result name to default value. +// This is used to populate pipeline results when a task result is not initialized but has a default defined. +func (state PipelineRunState) GetTaskResultDefaults() map[string]map[string]*v1.ResultValue { + defaults := make(map[string]map[string]*v1.ResultValue) + for _, rpt := range state { + if rpt.IsChildPipeline() { + continue + } + if rpt.IsCustomTask() { + continue + } + if rpt.ResolvedTask == nil || rpt.ResolvedTask.TaskSpec == nil { + continue + } + taskDefaults := make(map[string]*v1.ResultValue) + for _, result := range rpt.ResolvedTask.TaskSpec.Results { + if result.Default != nil { + taskDefaults[result.Name] = result.Default + } + } + if len(taskDefaults) > 0 { + defaults[rpt.PipelineTask.Name] = taskDefaults + } + } + return defaults +} + // ConvertResultsMapToTaskRunResults converts the map of results from Matrixed PipelineTasks to a list // of TaskRunResults to standard the format func ConvertResultsMapToTaskRunResults(resultsMap map[string][]string) []v1.TaskRunResult { @@ -273,13 +305,13 @@ func (state PipelineRunState) GetRunsResults() map[string][]v1beta1.CustomRunRes // GetChildReferences returns a slice of references, including version, kind, name, and pipeline task name, for all // child PipelineRuns, TaskRuns and Runs in the state. -func (facts *PipelineRunFacts) GetChildReferences() []v1.ChildStatusReference { +func (facts *PipelineRunFacts) GetChildReferences(ctx context.Context) []v1.ChildStatusReference { var childRefs []v1.ChildStatusReference for _, rpt := range facts.State { // try to replace the parameters of the reference result of when expression in the TaskRun that has ended if rpt.isDone(facts) { - resolvedResultRefs, _, err := ResolveResultRefs(facts.State, PipelineRunState{rpt}) + resolvedResultRefs, _, err := ResolveResultRefs(facts.EnableDefaultResults, facts.State, PipelineRunState{rpt}) if err == nil { ApplyTaskResults(facts.State, resolvedResultRefs) } @@ -675,21 +707,22 @@ func (facts *PipelineRunFacts) GetSkippedTasks() []v1.SkippedTask { var skipped []v1.SkippedTask for _, rpt := range facts.State { if rpt.Skip(facts).IsSkipped { + skipStatus := rpt.Skip(facts) skippedTask := v1.SkippedTask{ Name: rpt.PipelineTask.Name, - Reason: rpt.Skip(facts).SkippingReason, + Reason: skipStatus.SkippingReason, WhenExpressions: rpt.PipelineTask.When, } skipped = append(skipped, skippedTask) } - if rpt.IsFinallySkipped(facts).IsSkipped { + if finallySkipStatus := rpt.IsFinallySkipped(facts); finallySkipStatus.IsSkipped { skippedTask := v1.SkippedTask{ Name: rpt.PipelineTask.Name, - Reason: rpt.IsFinallySkipped(facts).SkippingReason, + Reason: finallySkipStatus.SkippingReason, } // include the when expressions only when the finally task was skipped because // its when expressions evaluated to false (not because results variables were missing) - if rpt.IsFinallySkipped(facts).SkippingReason == v1.WhenExpressionsSkip { + if finallySkipStatus.SkippingReason == v1.WhenExpressionsSkip { skippedTask.WhenExpressions = rpt.PipelineTask.When } skipped = append(skipped, skippedTask) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index c6fe3a8dbfe..aed3cbdb40f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "context" "fmt" "testing" "time" @@ -3747,6 +3748,68 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { } } +func TestPipelineRunState_GetTaskResultDefaults(t *testing.T) { + state := PipelineRunState{{ + TaskRunNames: []string{"task-with-defaults"}, + PipelineTask: &v1.PipelineTask{ + Name: "task-with-defaults-1", + }, + ResolvedTask: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "result-with-default", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("default-value"), + }, { + Name: "result-without-default", + Type: v1.ResultsTypeString, + }, { + Name: "array-result-with-default", + Type: v1.ResultsTypeArray, + Default: v1.NewStructuredValues("val1", "val2"), + }}, + }, + }, + }, { + TaskRunNames: []string{"task-without-defaults"}, + PipelineTask: &v1.PipelineTask{ + Name: "task-without-defaults-1", + }, + ResolvedTask: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "result-no-default", + Type: v1.ResultsTypeString, + }}, + }, + }, + }, { + TaskRunNames: []string{"task-nil-resolved-task"}, + PipelineTask: &v1.PipelineTask{ + Name: "task-nil-resolved-task-1", + }, + ResolvedTask: nil, + }, { + CustomTask: true, + CustomRunNames: []string{"custom-task-run"}, + PipelineTask: &v1.PipelineTask{ + Name: "custom-task-1", + }, + }} + + expectedDefaults := map[string]map[string]*v1.ResultValue{ + "task-with-defaults-1": { + "result-with-default": v1.NewStructuredValues("default-value"), + "array-result-with-default": v1.NewStructuredValues("val1", "val2"), + }, + } + + actualDefaults := state.GetTaskResultDefaults() + if d := cmp.Diff(expectedDefaults, actualDefaults); d != "" { + t.Errorf("Didn't get expected task result defaults map: %s", diff.PrintWantGot(d)) + } +} + func TestPipelineRunState_GetTaskRunsArtifacts(t *testing.T) { testCases := []struct { name string @@ -5089,7 +5152,7 @@ func TestPipelineRunState_GetChildReferences(t *testing.T) { TimeoutsState: PipelineRunTimeoutsState{ Clock: testClock, }, - }).GetChildReferences() + }).GetChildReferences(context.Background()) if d := cmp.Diff(tc.childRefs, childRefs); d != "" { t.Errorf("Didn't get expected child references for %s: %s", tc.name, diff.PrintWantGot(d)) } diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index 32a8ddf5232..d73d496e706 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -25,6 +25,7 @@ import ( pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" ) // ErrInvalidTaskResultReference indicates that the reason for the failure status is that there @@ -45,8 +46,8 @@ type ResolvedResultRef struct { } // ResolveResultRef resolves any ResultReference that are found in the target ResolvedPipelineTask -func ResolveResultRef(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (ResolvedResultRefs, string, error) { - resolvedResultRefs, pt, err := convertToResultRefs(pipelineRunState, target) +func ResolveResultRef(enableDefaultResults bool, pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (ResolvedResultRefs, string, error) { + resolvedResultRefs, pt, err := convertToResultRefs(enableDefaultResults, pipelineRunState, target) if err != nil { return nil, pt, err } @@ -54,10 +55,10 @@ func ResolveResultRef(pipelineRunState PipelineRunState, target *ResolvedPipelin } // ResolveResultRefs resolves any ResultReference that are found in the target ResolvedPipelineTask -func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunState) (ResolvedResultRefs, string, error) { +func ResolveResultRefs(enableDefaultResults bool, pipelineRunState PipelineRunState, targets PipelineRunState) (ResolvedResultRefs, string, error) { var allResolvedResultRefs ResolvedResultRefs for _, target := range targets { - resolvedResultRefs, pt, err := convertToResultRefs(pipelineRunState, target) + resolvedResultRefs, pt, err := convertToResultRefs(enableDefaultResults, pipelineRunState, target) if err != nil { return nil, pt, err } @@ -113,7 +114,7 @@ func removeDup(refs ResolvedResultRefs) ResolvedResultRefs { // found they are resolved to a value by searching pipelineRunState. The list of resolved // references are returned. If an error is encountered due to an invalid result reference // then a nil list and error is returned instead. -func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (ResolvedResultRefs, string, error) { +func convertToResultRefs(enableDefaultResults bool, pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (ResolvedResultRefs, string, error) { var resolvedResultRefs ResolvedResultRefs for _, resultRef := range v1.PipelineTaskResultRefs(target.PipelineTask) { referencedPipelineTask := pipelineRunState.ToMap()[resultRef.PipelineTask] @@ -122,6 +123,13 @@ func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipe } if !referencedPipelineTask.isSuccessful() && !referencedPipelineTask.isFailure() { + // Task did not run (e.g. skipped due to when expressions). If the feature flag is + // enabled, try to use the declared default value for the referenced result so that + // downstream tasks (both DAG and finally) can still execute with the default. + if resolved, err := resolveDefaultForUnfinishedTask(enableDefaultResults, referencedPipelineTask, resultRef); err == nil { + resolvedResultRefs = append(resolvedResultRefs, resolved...) + continue + } return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not finished", referencedPipelineTask.PipelineTask.Name) } // Custom Task @@ -135,7 +143,7 @@ func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipe default: // Matrixed referenced Pipeline Task if referencedPipelineTask.PipelineTask.IsMatrixed() { - arrayValues, err := findResultValuesForMatrix(referencedPipelineTask, resultRef) + arrayValues, err := findResultValuesForMatrix(enableDefaultResults, referencedPipelineTask, resultRef) if err != nil { return nil, resultRef.PipelineTask, err } @@ -145,7 +153,7 @@ func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipe } } else { // Regular PipelineTask - resolved, err := resolveResultRef(referencedPipelineTask.TaskRuns, resultRef) + resolved, err := resolveResultRef(enableDefaultResults, referencedPipelineTask.TaskRuns, resultRef, referencedPipelineTask.ResolvedTask) if err != nil { return nil, resultRef.PipelineTask, err } @@ -186,10 +194,10 @@ func paramValueFromCustomRunResult(result string) *v1.ParamValue { return v1.NewStructuredValues(result) } -func resolveResultRef(taskRuns []*v1.TaskRun, resultRef *v1.ResultRef) (*ResolvedResultRef, error) { +func resolveResultRef(enableDefaultResults bool, taskRuns []*v1.TaskRun, resultRef *v1.ResultRef, resolvedTask *resources.ResolvedTask) (*ResolvedResultRef, error) { taskRun := taskRuns[0] taskRunName := taskRun.Name - resultValue, err := findTaskResultForParam(taskRun, resultRef) + resultValue, err := findTaskResultForParam(enableDefaultResults, taskRun, resultRef, resolvedTask) if err != nil { return nil, err } @@ -211,20 +219,88 @@ func findRunResultForParam(customRun *v1beta1.CustomRun, reference *v1.ResultRef return "", err } -func findTaskResultForParam(taskRun *v1.TaskRun, reference *v1.ResultRef) (v1.ResultValue, error) { +// resolveDefaultForUnfinishedTask attempts to resolve a default result value for a task that +// did not produce a result because it did not run (e.g. skipped due to when expressions). +// Returns one or more ResolvedResultRefs using the declared default, or an error if no default +// is available or the feature flag is disabled. +func resolveDefaultForUnfinishedTask(enableDefaultResults bool, referencedPipelineTask *ResolvedPipelineTask, resultRef *v1.ResultRef) (ResolvedResultRefs, error) { + var err error + if !enableDefaultResults { + return nil, errors.New("enable-default-results feature flag is disabled") + } + if referencedPipelineTask.ResolvedTask == nil || referencedPipelineTask.ResolvedTask.TaskSpec == nil { + return nil, fmt.Errorf("no task spec available for task %q", referencedPipelineTask.PipelineTask.Name) + } + for _, taskResult := range referencedPipelineTask.ResolvedTask.TaskSpec.Results { + if taskResult.Name != resultRef.Result || taskResult.Default == nil { + continue + } + if referencedPipelineTask.PipelineTask.IsMatrixed() { + // For a matrix task that didn't run, wrap the default into array format. + var arrayVal v1.ParamValue + arrayVal, err = matrixDefaultValue(taskResult.Name, taskResult.Default) + if err != nil { + return nil, err + } + return ResolvedResultRefs{{ + Value: arrayVal, + FromTaskRun: "", + ResultReference: *resultRef, + }}, nil + } + return ResolvedResultRefs{{ + Value: *taskResult.Default, + FromTaskRun: "", + ResultReference: *resultRef, + }}, nil + } + return nil, fmt.Errorf("%w: no default value for result %q in task %q", ErrInvalidTaskResultReference, resultRef.Result, resultRef.PipelineTask) +} + +func findTaskResultForParam(enableDefaultResults bool, taskRun *v1.TaskRun, reference *v1.ResultRef, resolvedTask *resources.ResolvedTask) (v1.ResultValue, error) { results := taskRun.Status.TaskRunStatusFields.Results for _, result := range results { if result.Name == reference.Result { return result.Value, nil } } + // If result not found, check for default value in the Task spec (only if feature flag is enabled) + if resolvedTask != nil && resolvedTask.TaskSpec != nil { + if enableDefaultResults { + for _, taskResult := range resolvedTask.TaskSpec.Results { + if taskResult.Name == reference.Result && taskResult.Default != nil { + return *taskResult.Default, nil + } + } + } + } err := fmt.Errorf("%w: Could not find result with name %s for pipeline task %s", ErrInvalidTaskResultReference, reference.Result, reference.PipelineTask) return v1.ResultValue{}, err } +// matrixDefaultValue converts a task result default value into the array format required +// for matrixed tasks. Matrix result references are always represented as arrays (one entry +// per matrix iteration), so scalar string defaults are wrapped in a single-element array. +// Object defaults are not supported for matrixed tasks because matrix results cannot be +// of object type. +func matrixDefaultValue(resultName string, def *v1.ResultValue) (v1.ParamValue, error) { + switch def.Type { + case v1.ParamTypeArray: + // Array defaults are already in the expected format; return as-is. + return *def, nil + case v1.ParamTypeString: + // String defaults are wrapped in a single-element array to satisfy the + // array representation required for matrix result references. + return v1.ParamValue{Type: v1.ParamTypeArray, ArrayVal: []string{def.StringVal}}, nil + case v1.ParamTypeObject: + return v1.ParamValue{}, fmt.Errorf("object default for result %q is not supported for matrixed tasks", resultName) + } + return v1.ParamValue{}, fmt.Errorf("unsupported default type %q for result %q", def.Type, resultName) +} + // findResultValuesForMatrix checks the resultsCache of the referenced Matrixed TaskRun to retrieve the resultValues and aggregate them into // arrayValues. If the resultCache is empty, it will create the ResultCache so that the results can be accessed in subsequent tasks. -func findResultValuesForMatrix(referencedPipelineTask *ResolvedPipelineTask, resultRef *v1.ResultRef) (v1.ParamValue, error) { +func findResultValuesForMatrix(enableDefaultResults bool, referencedPipelineTask *ResolvedPipelineTask, resultRef *v1.ResultRef) (v1.ParamValue, error) { var resultsCache *map[string][]string if len(referencedPipelineTask.ResultsCache) == 0 { cache := createResultsCacheMatrixedTaskRuns(referencedPipelineTask) @@ -237,6 +313,16 @@ func findResultValuesForMatrix(referencedPipelineTask *ResolvedPipelineTask, res ArrayVal: arrayValues, }, nil } + // If result not found in cache, check for default value in the Task spec (only if feature flag is enabled) + if referencedPipelineTask.ResolvedTask != nil && referencedPipelineTask.ResolvedTask.TaskSpec != nil { + if enableDefaultResults { + for _, taskResult := range referencedPipelineTask.ResolvedTask.TaskSpec.Results { + if taskResult.Name == resultRef.Result && taskResult.Default != nil { + return matrixDefaultValue(taskResult.Name, taskResult.Default) + } + } + } + } err := fmt.Errorf("%w: Could not find result with name %s for task %s", ErrInvalidTaskResultReference, resultRef.Result, resultRef.PipelineTask) return v1.ParamValue{}, err } diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index 7aab4651926..9de1ec376eb 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + taskresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -403,12 +404,13 @@ var pipelineRunState = PipelineRunState{{ func TestResolveResultRefs(t *testing.T) { for _, tt := range []struct { - name string - pipelineRunState PipelineRunState - targets PipelineRunState - want ResolvedResultRefs - wantErr bool - wantPt string + name string + pipelineRunState PipelineRunState + targets PipelineRunState + want ResolvedResultRefs + wantErr bool + wantPt string + enableDefaultResults bool }{{ name: "Test successful result references resolution - params", pipelineRunState: pipelineRunState, @@ -604,9 +606,69 @@ func TestResolveResultRefs(t *testing.T) { }, FromTaskRun: "nTaskRun", }}, + }, { + name: "Test successful result references resolution with default value", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"taskWithDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskWithDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + // Note: no results in TaskRun status - should use default + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "taskWithDefault", + TaskRef: &v1.TaskRef{Name: "taskWithDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "defaultResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("defaultValue"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("defaultValue"), + ResultReference: v1.ResultRef{ + PipelineTask: "taskWithDefault", + Result: "defaultResult", + }, + FromTaskRun: "taskWithDefault", + }}, + wantErr: false, + enableDefaultResults: true, }} { t.Run(tt.name, func(t *testing.T) { - got, pt, err := ResolveResultRefs(tt.pipelineRunState, tt.targets) + got, pt, err := ResolveResultRefs(tt.enableDefaultResults, tt.pipelineRunState, tt.targets) if (err != nil) != tt.wantErr { t.Errorf("ResolveResultRefs() error = %v, wantErr %v", err, tt.wantErr) return @@ -690,7 +752,7 @@ func TestResolveResultRef(t *testing.T) { wantErr: false, }} { t.Run(tt.name, func(t *testing.T) { - got, pt, err := ResolveResultRef(tt.pipelineRunState, tt.target) + got, pt, err := ResolveResultRef(false, tt.pipelineRunState, tt.target) if (err != nil) != tt.wantErr { t.Errorf("ResolveResultRefs() error = %v, wantErr %v", err, tt.wantErr) return @@ -817,7 +879,7 @@ func TestCheckMissingResultReferences(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var err error for _, target := range tt.targets { - tmpErr := CheckMissingResultReferences(tt.pipelineRunState, target) + tmpErr := CheckMissingResultReferences(false, tt.pipelineRunState, target) if tmpErr != nil { err = tmpErr } @@ -1033,7 +1095,7 @@ func TestCheckMissingResultReferences_UninitializedResults(t *testing.T) { wantErr: `Result reference error: Internal result ref "producer" has zero-length TaskRuns`, }} { t.Run(tt.name, func(t *testing.T) { - err := CheckMissingResultReferences(tt.pipelineRunState, tt.target) + err := CheckMissingResultReferences(true, tt.pipelineRunState, tt.target) if tt.wantErr == "" { if err != nil { t.Errorf("CheckMissingResultReferences() unexpected error: %v", err) @@ -1203,3 +1265,461 @@ func TestParamValueFromCustomRunResult(t *testing.T) { }) } } + +func TestResolveResultRefsWithDefaultValues(t *testing.T) { + tests := []struct { + name string + pipelineRunState PipelineRunState + targets PipelineRunState + featureFlag bool + want ResolvedResultRefs + wantErr bool + wantErrMsg string + }{{ + name: "default value used when feature flag enabled and result missing", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"taskWithDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskWithDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + // Note: no results in TaskRun status - should use default + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "taskWithDefault", + TaskRef: &v1.TaskRef{Name: "taskWithDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "defaultResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("defaultValue"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + featureFlag: true, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("defaultValue"), + ResultReference: v1.ResultRef{ + PipelineTask: "taskWithDefault", + Result: "defaultResult", + }, + FromTaskRun: "taskWithDefault", + }}, + wantErr: false, + }, { + name: "default value not used when feature flag disabled", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"taskWithDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskWithDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + // Note: no results in TaskRun status + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "taskWithDefault", + TaskRef: &v1.TaskRef{Name: "taskWithDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "defaultResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("defaultValue"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + featureFlag: false, + wantErr: true, + wantErrMsg: "Could not find result with name defaultResult", + }, { + name: "actual result takes precedence over default when both exist", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"taskWithDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskWithDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "defaultResult", + Value: *v1.NewStructuredValues("actualValue"), + }}, + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "taskWithDefault", + TaskRef: &v1.TaskRef{Name: "taskWithDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "defaultResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("defaultValue"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithDefault.results.defaultResult)"), + }}, + }, + }}, + featureFlag: true, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("actualValue"), + ResultReference: v1.ResultRef{ + PipelineTask: "taskWithDefault", + Result: "defaultResult", + }, + FromTaskRun: "taskWithDefault", + }}, + wantErr: false, + }, { + name: "default value used for array result", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"taskWithArrayDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskWithArrayDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + // Note: no results in TaskRun status + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "taskWithArrayDefault", + TaskRef: &v1.TaskRef{Name: "taskWithArrayDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "arrayResult", + Type: v1.ResultsTypeArray, + Default: v1.NewStructuredValues("value1", "value2", "value3"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithArrayDefault.results.arrayResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "consumerParam", + Value: *v1.NewStructuredValues("$(tasks.taskWithArrayDefault.results.arrayResult)"), + }}, + }, + }}, + featureFlag: true, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("value1", "value2", "value3"), + ResultReference: v1.ResultRef{ + PipelineTask: "taskWithArrayDefault", + Result: "arrayResult", + }, + FromTaskRun: "taskWithArrayDefault", + }}, + wantErr: false, + }, { + name: "default value used for matrix parameter when result missing", + pipelineRunState: PipelineRunState{{ + TaskRunNames: []string{"matrixTaskWithDefault"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "matrixTaskWithDefault", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + // Note: no results in TaskRun status + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "matrixTaskWithDefault", + TaskRef: &v1.TaskRef{Name: "matrixTaskWithDefault"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "matrixResult", + Type: v1.ResultsTypeArray, + Default: v1.NewStructuredValues("default1", "default2"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerMatrixTask", + TaskRef: &v1.TaskRef{Name: "consumerMatrixTask"}, + Matrix: &v1.Matrix{ + Params: v1.Params{{ + Name: "matrixParam", + Value: *v1.NewStructuredValues("$(tasks.matrixTaskWithDefault.results.matrixResult[*])"), + }}, + }, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerMatrixTask", + TaskRef: &v1.TaskRef{Name: "consumerMatrixTask"}, + Matrix: &v1.Matrix{ + Params: v1.Params{{ + Name: "matrixParam", + Value: *v1.NewStructuredValues("$(tasks.matrixTaskWithDefault.results.matrixResult[*])"), + }}, + }, + }, + }}, + featureFlag: true, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("default1", "default2"), + ResultReference: v1.ResultRef{ + PipelineTask: "matrixTaskWithDefault", + Result: "matrixResult", + }, + FromTaskRun: "matrixTaskWithDefault", + }}, + wantErr: false, + }, { + name: "default value used when referenced task is skipped (no TaskRun) and feature flag enabled", + pipelineRunState: PipelineRunState{{ + // skippedTask has no TaskRuns - it was skipped (e.g. when expression evaluated to false) + PipelineTask: &v1.PipelineTask{ + Name: "skippedTask", + TaskRef: &v1.TaskRef{Name: "skippedTask"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "skippedResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("default-from-skipped"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + featureFlag: true, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("default-from-skipped"), + ResultReference: v1.ResultRef{ + PipelineTask: "skippedTask", + Result: "skippedResult", + }, + FromTaskRun: "", + }}, + wantErr: false, + }, { + name: "error when referenced task is skipped and feature flag disabled", + pipelineRunState: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "skippedTask", + TaskRef: &v1.TaskRef{Name: "skippedTask"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "skippedResult", + Type: v1.ResultsTypeString, + Default: v1.NewStructuredValues("default-from-skipped"), + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + featureFlag: false, + wantErr: true, + wantErrMsg: "was not finished", + }, { + name: "error when referenced task is skipped and result has no default", + pipelineRunState: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "skippedTask", + TaskRef: &v1.TaskRef{Name: "skippedTask"}, + }, + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "skippedResult", + Type: v1.ResultsTypeString, + // no Default + }}, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + targets: PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "consumerTask", + TaskRef: &v1.TaskRef{Name: "consumerTask"}, + Params: []v1.Param{{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.skippedTask.results.skippedResult)"), + }}, + }, + }}, + featureFlag: true, + wantErr: true, + wantErrMsg: "was not finished", + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, pt, err := ResolveResultRefs(tt.featureFlag, tt.pipelineRunState, tt.targets) + if (err != nil) != tt.wantErr { + t.Errorf("ResolveResultRefs() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + if err != nil && !strings.Contains(err.Error(), tt.wantErrMsg) { + t.Errorf("ResolveResultRefs() error = %v, expected to contain %q", err.Error(), tt.wantErrMsg) + } + return + } + if d := cmp.Diff(tt.want, got, cmpopts.SortSlices(lessResolvedResultRefs)); d != "" { + t.Fatalf("ResolveResultRef %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff("", pt); d != "" { + t.Fatalf("ResolvedPipelineTask %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index db18f0f9070..e179d1952b0 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -683,6 +683,18 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, return nil, nil, controller.NewPermanentError(err) } + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults && len(tr.Status.Results) == 0 { + for _, tR := range rtr.TaskSpec.Results { + if tR.Default != nil { + tr.Status.Results = append(tr.Status.Results, v1.TaskRunResult{ + Name: tR.Name, + Type: tR.Type, + Value: *tR.Default, + }) + } + } + } + return taskSpec, rtr, nil } @@ -801,6 +813,11 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1.TaskRun, rtr *resourc return err } + if err := validateResultsProduced(ctx, tr, rtr.TaskSpec); err != nil { + tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) + return err + } + logger.Infof("Successfully reconciled taskrun %s/%s with status: %#v", tr.Name, tr.Namespace, tr.Status.GetCondition(apis.ConditionSucceeded)) return nil } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index ff6c1bca65b..edfb9e3b26c 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -8521,3 +8521,121 @@ func TestAppendPreviousConditionContext(t *testing.T) { }) } } + +func TestReconcile_DefaultResults(t *testing.T) { + // Task with defaults — valid only when enable-default-results=true. + taskWithDefaults := parse.MustParseV1Task(t, ` +metadata: + name: test-task-with-default-results + namespace: foo +spec: + results: + - name: string-result + default: "default-string-value" + - name: array-result + type: array + default: + - "default-1" + - "default-2" + - name: no-default-result + steps: + - name: step1 + image: foo + command: ["/mycmd"] +`) + + // Task without defaults — usable regardless of the feature flag. + taskNoDefaults := parse.MustParseV1Task(t, ` +metadata: + name: test-task-no-defaults + namespace: foo +spec: + results: + - name: string-result + steps: + - name: step1 + image: foo + command: ["/mycmd"] +`) + + for _, tc := range []struct { + name string + task *v1.Task + featureFlag string + existingResults []v1.TaskRunResult + wantResults []v1.TaskRunResult + wantRequeue bool + }{{ + name: "default results populated when feature flag enabled and no results yet", + task: taskWithDefaults, + featureFlag: "true", + wantRequeue: true, + wantResults: []v1.TaskRunResult{ + {Name: "string-result", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("default-string-value")}, + {Name: "array-result", Type: v1.ResultsTypeArray, Value: *v1.NewStructuredValues("default-1", "default-2")}, + }, + }, { + name: "default results NOT populated when feature flag disabled", + task: taskNoDefaults, + featureFlag: "false", + wantRequeue: true, + wantResults: nil, + }, { + name: "default results NOT re-initialized when results already present", + task: taskWithDefaults, + featureFlag: "true", + existingResults: []v1.TaskRunResult{ + {Name: "string-result", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("step-produced-value")}, + }, + wantRequeue: true, + wantResults: []v1.TaskRunResult{ + {Name: "string-result", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("step-produced-value")}, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + tr := parse.MustParseV1TaskRun(t, ` +metadata: + name: test-taskrun-default-results + namespace: foo +spec: + taskRef: + name: `+tc.task.Name+` +`) + tr.Status.Results = tc.existingResults + + d := test.Data{ + TaskRuns: []*v1.TaskRun{tr}, + Tasks: []*v1.Task{tc.task}, + ConfigMaps: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-default-results": tc.featureFlag, + }, + }}, + } + + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, tr.Spec.ServiceAccountName, tr.Namespace) + + err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(tr)) + if tc.wantRequeue { + if err == nil { + t.Error("Wanted a wrapped requeue error, but got nil.") + } else if ok, _ := controller.IsRequeueKey(err); !ok { + t.Errorf("Error reconciling TaskRun: %v", err) + } + } else if err != nil { + t.Errorf("Unexpected reconcile error: %v", err) + } + + reconciledTR, err := testAssets.Clients.Pipeline.TektonV1().TaskRuns(tr.Namespace).Get(testAssets.Ctx, tr.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting updated taskrun: %v", err) + } + if d := cmp.Diff(tc.wantResults, reconciledTR.Status.Results); d != "" { + t.Errorf("unexpected results %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/validate_taskrun.go b/pkg/reconciler/taskrun/validate_taskrun.go index 58ecb47d2a5..f97633f2f65 100644 --- a/pkg/reconciler/taskrun/validate_taskrun.go +++ b/pkg/reconciler/taskrun/validate_taskrun.go @@ -18,12 +18,12 @@ package taskrun import ( "context" + "errors" "fmt" "sort" "strings" - "errors" - + "github.com/tektoncd/pipeline/pkg/apis/config" pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/list" @@ -263,16 +263,21 @@ func validateSidecarOverrides(ts *v1.TaskSpec, trs *v1.TaskRunSpec) error { return errors.Join(errs...) } -// validateResults checks the emitted results type and object properties against the ones defined in spec. -func validateTaskRunResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error { - specResults := []v1.TaskResult{} +// collectAllTaskResults combines results from the inline TaskSpec and the resolved TaskSpec. +func collectAllTaskResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) []v1.TaskResult { + var specResults []v1.TaskResult if tr.Spec.TaskSpec != nil { specResults = append(specResults, tr.Spec.TaskSpec.Results...) } - if resolvedTaskSpec != nil { specResults = append(specResults, resolvedTaskSpec.Results...) } + return specResults +} + +// validateTaskRunResults checks the emitted results type and object properties against the ones defined in spec. +func validateTaskRunResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error { + specResults := collectAllTaskResults(tr, resolvedTaskSpec) // When get the results, check if the type of result is the expected one if missmatchedTypes := mismatchedTypesResults(tr, specResults); len(missmatchedTypes) != 0 { @@ -291,6 +296,40 @@ func validateTaskRunResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error return nil } +// validateResultsProduced checks that every declared result without a default was produced by the TaskRun. +// Results that have a default are pre-populated in prepare(), so they will always be present in +// tr.Status.Results even when the step did not write them. Results with no default must be explicitly +// written by a step; if they are absent the TaskRun has violated its declared contract. +// This validation only runs on terminal TaskRuns and only when the enable-default-results feature flag is set. +func validateResultsProduced(ctx context.Context, tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableDefaultResults { + return nil + } + if !tr.IsSuccessful() { + return nil + } + + specResults := collectAllTaskResults(tr, resolvedTaskSpec) + + producedResults := sets.NewString() + for _, r := range tr.Status.Results { + producedResults.Insert(r.Name) + } + + var missing []string + for _, r := range specResults { + if r.Default == nil && !producedResults.Has(r.Name) { + missing = append(missing, r.Name) + } + } + + if len(missing) > 0 { + sort.Strings(missing) + return pipelineErrors.WrapUserError(fmt.Errorf("missing results: %s", strings.Join(missing, ", "))) + } + return nil +} + // mismatchedTypesResults checks and returns all the mismatched types of emitted results against specified results. func mismatchedTypesResults(tr *v1.TaskRun, specResults []v1.TaskResult) map[string]string { neededTypes := make(map[string]string) diff --git a/pkg/reconciler/taskrun/validate_taskrun_test.go b/pkg/reconciler/taskrun/validate_taskrun_test.go index 40293cf91b3..4f2542ca7d0 100644 --- a/pkg/reconciler/taskrun/validate_taskrun_test.go +++ b/pkg/reconciler/taskrun/validate_taskrun_test.go @@ -20,12 +20,15 @@ import ( "errors" "testing" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" ) func TestValidateResolvedTask_ValidParams(t *testing.T) { @@ -958,3 +961,139 @@ func TestEnumValidation_Failure(t *testing.T) { } } } + +func doneTaskRun(tr *v1.TaskRun) *v1.TaskRun { + tr.Status.Conditions = duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }} + return tr +} + +func TestValidateResultsProduced(t *testing.T) { + taskSpec := &v1.TaskSpec{ + Results: []v1.TaskResult{ + {Name: "with-default", Default: &v1.ParamValue{Type: v1.ParamTypeString, StringVal: "fallback"}}, + {Name: "without-default"}, + }, + } + + for _, tc := range []struct { + name string + tr *v1.TaskRun + taskSpec *v1.TaskSpec + featureFlag bool + wantErr error + }{{ + name: "feature flag disabled — no error even if results are missing", + featureFlag: false, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + }), + taskSpec: taskSpec, + wantErr: nil, + }, { + name: "taskrun not done — no error", + featureFlag: true, + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }}}, + }, + }, + taskSpec: taskSpec, + wantErr: nil, + }, { + name: "all results produced — no error", + featureFlag: true, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + {Name: "with-default", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("produced")}, + {Name: "without-default", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("produced")}, + }, + }, + }, + }), + taskSpec: taskSpec, + wantErr: nil, + }, { + name: "result with default not produced — no error (default was pre-populated)", + featureFlag: true, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + {Name: "with-default", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("fallback")}, + {Name: "without-default", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("produced")}, + }, + }, + }, + }), + taskSpec: taskSpec, + wantErr: nil, + }, { + name: "result without default not produced — error", + featureFlag: true, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + {Name: "with-default", Type: v1.ResultsTypeString, Value: *v1.NewStructuredValues("fallback")}, + }, + }, + }, + }), + taskSpec: taskSpec, + wantErr: errors.New("missing results: without-default"), + }, { + name: "multiple results without defaults not produced — all listed in error", + featureFlag: true, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + }), + taskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{ + {Name: "result-a"}, + {Name: "result-b"}, + }, + }, + wantErr: errors.New("missing results: result-a, result-b"), + }, { + name: "no declared results — no error", + featureFlag: true, + tr: doneTaskRun(&v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + }), + taskSpec: &v1.TaskSpec{}, + wantErr: nil, + }} { + t.Run(tc.name, func(t *testing.T) { + ctx := config.ToContext(t.Context(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableDefaultResults: tc.featureFlag, + }, + }) + err := validateResultsProduced(ctx, tc.tr, tc.taskSpec) + if tc.wantErr == nil { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error %q but got none", tc.wantErr) + } + if d := cmp.Diff(tc.wantErr.Error(), err.Error()); d != "" { + t.Errorf("error mismatch: %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/test/default_results_test.go b/test/default_results_test.go new file mode 100644 index 00000000000..da8d46c0c3c --- /dev/null +++ b/test/default_results_test.go @@ -0,0 +1,241 @@ +//go:build e2e + +/* +Copyright 2026 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 test + +import ( + "context" + "fmt" + "testing" + + "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/test/parse" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/system" + knativetest "knative.dev/pkg/test" + "knative.dev/pkg/test/helpers" +) + +// TestFinallyTaskUsesDefaultResultWhenTaskFails verifies that when a DAG task +// declares a result with a default value but fails before writing that +// result, a finally task consuming the result falls back to the declared +// default instead of leaving the PipelineRun unresolvable. +// +// @test:execution=serial +// @test:reason=modifies enable-default-results field in feature-flags ConfigMap +func TestFinallyTaskUsesDefaultResultWhenTaskFails(t *testing.T) { + ctx := t.Context() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) + defer tearDown(ctx, t, c, namespace) + + previous := setDefaultResultsFeatureFlag(ctx, t, c, "true") + knativetest.CleanupOnInterrupt(func() { setDefaultResultsFeatureFlag(ctx, t, c, previous) }, t.Logf) + defer setDefaultResultsFeatureFlag(ctx, t, c, previous) + + pipelineRun := parse.MustParseV1PipelineRun(t, fmt.Sprintf(` +metadata: + name: %s +spec: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: greeting + default: "default-greeting" + steps: + - name: fail-before-writing-result + image: mirror.gcr.io/busybox + script: | + echo "failing before the result is written" + exit 1 + finally: + - name: finaltask1 + params: + - name: greeting + value: $(tasks.task1.results.greeting) + taskSpec: + params: + - name: greeting + steps: + - name: check-default-is-used + image: mirror.gcr.io/busybox + script: | + echo "finaltask1 received: $(params.greeting)" + if [ "$(params.greeting)" != "default-greeting" ]; then + echo "expected default-greeting but got $(params.greeting)" + exit 1 + fi +`, helpers.ObjectNameForTest(t))) + + if _, err := c.V1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err) + } + + t.Logf("Waiting for PipelineRun %s in namespace %s to fail", pipelineRun.Name, namespace) + if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, PipelineRunFailed(pipelineRun.Name), "PipelineRunFailed", v1Version); err != nil { + t.Fatalf("Error waiting for PipelineRun to fail: %s", err) + } + + taskrunList, err := c.V1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name}) + if err != nil { + t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err) + } + if len(taskrunList.Items) != 2 { + t.Fatalf("The PipelineRun %q should have exactly 2 TaskRuns, one for task1 and one for finaltask1, instead it has %d", pipelineRun.Name, len(taskrunList.Items)) + } + + for _, taskrunItem := range taskrunList.Items { + switch taskrunItem.Labels["tekton.dev/pipelineTask"] { + case "task1": + if !isFailed(t, taskrunItem.Name, taskrunItem.Status.Conditions) { + t.Fatalf("task1 should have failed") + } + if len(taskrunItem.Status.Results) != 1 { + t.Fatalf("task1 should have exactly one result populated from its default, got %d", len(taskrunItem.Status.Results)) + } + if r := taskrunItem.Status.Results[0]; r.Name != "greeting" || r.Value.StringVal != "default-greeting" { + t.Fatalf("task1 result should be \"greeting\"=\"default-greeting\", got %q=%q", r.Name, r.Value.StringVal) + } + case "finaltask1": + if !isSuccessful(t, taskrunItem.Name, taskrunItem.Status.Conditions) { + t.Fatalf("finaltask1 should have succeeded using the default result value") + } + default: + t.Fatalf("Unexpected TaskRun %q for PipelineRun %q", taskrunItem.Name, pipelineRun.Name) + } + } +} + +// TestFinallyTaskUsesProducedResultOverDefault verifies that when a DAG task +// declares a result with a default value and produces its own value for that +// result before later failing, a finally task consuming the result observes +// the produced value rather than the declared default. +// +// @test:execution=serial +// @test:reason=modifies enable-default-results field in feature-flags ConfigMap +func TestFinallyTaskUsesProducedResultOverDefault(t *testing.T) { + ctx := t.Context() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t) + knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) + defer tearDown(ctx, t, c, namespace) + + previous := setDefaultResultsFeatureFlag(ctx, t, c, "true") + knativetest.CleanupOnInterrupt(func() { setDefaultResultsFeatureFlag(ctx, t, c, previous) }, t.Logf) + defer setDefaultResultsFeatureFlag(ctx, t, c, previous) + + pipelineRun := parse.MustParseV1PipelineRun(t, fmt.Sprintf(` +metadata: + name: %s +spec: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: greeting + default: "default-greeting" + steps: + - name: produce-result-then-fail + image: mirror.gcr.io/busybox + script: | + echo -n "actual-greeting" | tee $(results.greeting.path) + exit 1 + finally: + - name: finaltask1 + params: + - name: greeting + value: $(tasks.task1.results.greeting) + taskSpec: + params: + - name: greeting + steps: + - name: check-produced-value-is-used + image: mirror.gcr.io/busybox + script: | + echo "finaltask1 received: $(params.greeting)" + if [ "$(params.greeting)" != "actual-greeting" ]; then + echo "expected actual-greeting but got $(params.greeting)" + exit 1 + fi +`, helpers.ObjectNameForTest(t))) + + if _, err := c.V1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err) + } + + t.Logf("Waiting for PipelineRun %s in namespace %s to fail", pipelineRun.Name, namespace) + if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, PipelineRunFailed(pipelineRun.Name), "PipelineRunFailed", v1Version); err != nil { + t.Fatalf("Error waiting for PipelineRun to fail: %s", err) + } + + taskrunList, err := c.V1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name}) + if err != nil { + t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err) + } + if len(taskrunList.Items) != 2 { + t.Fatalf("The PipelineRun %q should have exactly 2 TaskRuns, one for task1 and one for finaltask1, instead it has %d", pipelineRun.Name, len(taskrunList.Items)) + } + + for _, taskrunItem := range taskrunList.Items { + switch taskrunItem.Labels["tekton.dev/pipelineTask"] { + case "task1": + if !isFailed(t, taskrunItem.Name, taskrunItem.Status.Conditions) { + t.Fatalf("task1 should have failed") + } + if len(taskrunItem.Status.Results) != 1 { + t.Fatalf("task1 should have exactly one result populated from the step, got %d", len(taskrunItem.Status.Results)) + } + if r := taskrunItem.Status.Results[0]; r.Name != "greeting" || r.Value.StringVal != "actual-greeting" { + t.Fatalf("task1 result should be \"greeting\"=\"actual-greeting\", got %q=%q", r.Name, r.Value.StringVal) + } + case "finaltask1": + if !isSuccessful(t, taskrunItem.Name, taskrunItem.Status.Conditions) { + t.Fatalf("finaltask1 should have succeeded using the produced result value") + } + default: + t.Fatalf("Unexpected TaskRun %q for PipelineRun %q", taskrunItem.Name, pipelineRun.Name) + } + } +} + +// setDefaultResultsFeatureFlag sets the "enable-default-results" field in the +// feature-flags ConfigMap to value, returning the previous value so callers +// can restore it. +func setDefaultResultsFeatureFlag(ctx context.Context, t *testing.T, c *clients, value string) string { + t.Helper() + featureFlagsCM, err := c.KubeClient.CoreV1().ConfigMaps(system.Namespace()).Get(ctx, config.GetFeatureFlagsConfigName(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get ConfigMap `%s`: %s", config.GetFeatureFlagsConfigName(), err) + } + previous, ok := featureFlagsCM.Data[config.EnableDefaultResults] + if !ok { + previous = "false" + } + if err := updateConfigMap(ctx, c.KubeClient, system.Namespace(), config.GetFeatureFlagsConfigName(), map[string]string{ + config.EnableDefaultResults: value, + }); err != nil { + t.Fatalf("Failed to set %q on the feature-flags ConfigMap: %s", config.EnableDefaultResults, err) + } + return previous +} diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 4cb109ed171..7ea98f60bc5 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -35,6 +35,7 @@ ENABLE_ARTIFACTS=${ENABLE_ARTIFACTS:="false"} ENABLE_CONCISE_RESOLVER_SYNTAX=${ENABLE_CONCISE_RESOLVER_SYNTAX:="false"} ENABLE_KUBERNETES_SIDECAR=${ENABLE_KUBERNETES_SIDECAR:="false"} ENABLE_TERMINATION_MESSAGE_COMPRESSION=${ENABLE_TERMINATION_MESSAGE_COMPRESSION:="false"} +ENABLE_DEFAULT_RESULTS=${ENABLE_DEFAULT_RESULTS:="false"} # Script entry point. @@ -175,6 +176,18 @@ function set_default_sidecar_log_polling_interval() { with_retries kubectl patch configmap config-defaults -n tekton-pipelines -p "$jsonpatch" } +function set_enable_default_results() { + local method="$1" + if [ "$method" != "false" ] && [ "$method" != "true" ]; then + printf "Invalid value for enable-default-results %s\n" ${method} + exit 255 + fi + printf "Setting enable-default-results to %s\n" ${method} + jsonpatch=$(printf "{\"data\": {\"enable-default-results\": \"%s\"}}" $1) + echo "feature-flags ConfigMap patch: ${jsonpatch}" + with_retries kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" +} + function run_e2e() { # Run the integration tests header "Running Go e2e tests" @@ -187,7 +200,17 @@ function run_e2e() { # and they cause a lot of noise in the logs, making it harder to debug integration # test failures. if [ "${RUN_YAML_TESTS}" == "true" ]; then + # enable-default-results is an alpha feature; enable it for alpha example runs + # so that examples under examples/v1/pipelineruns/alpha/ and examples/v1/taskruns/alpha/ + # that rely on default results are actually executed. + if [ "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then + set_enable_default_results "true" + fi go_test_e2e -mod=readonly -parallel=2 -tags=examples -timeout=${E2E_GO_TEST_TIMEOUT} ./test/ + # restore enable-default-results to its configured value after example runs + if [ "${PIPELINE_FEATURE_GATE}" == "alpha" ]; then + set_enable_default_results "${ENABLE_DEFAULT_RESULTS}" + fi fi if [ "${RUN_FEATUREFLAG_TESTS}" == "true" ]; then @@ -209,6 +232,7 @@ set_enable_concise_resolver_syntax "$ENABLE_CONCISE_RESOLVER_SYNTAX" set_enable_kubernetes_sidecar "$ENABLE_KUBERNETES_SIDECAR" set_enable_termination_message_compression "$ENABLE_TERMINATION_MESSAGE_COMPRESSION" set_default_sidecar_log_polling_interval +set_enable_default_results "$ENABLE_DEFAULT_RESULTS" run_e2e success