diff --git a/docs/pipelines.md b/docs/pipelines.md index 1d401077bb0..91c12c1b2fc 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -1041,6 +1041,7 @@ This will cause an `InvalidTaskResultReference` validation error during `Pipelin **Note:** Since a `Pipeline Result` can contain references to multiple `Task Results`, if any of those `Task Result` references are invalid the entire `Pipeline Result` is not emitted. +**Note:** If a `PipelineTask` referenced by the `Pipeline Result` was skipped, the `Pipeline Result` will not be emitted and the `PipelineRun` will not fail due to a missing result. ## Configuring the `Task` execution order @@ -1284,7 +1285,7 @@ results: value: $(finally.check-count.results.comment-count-validate) finally: - name: check-count - taskRef: + taskRef: name: example-task-name ``` @@ -1772,7 +1773,7 @@ Consult the documentation of the custom task that you are using to determine whe Pipelines do not support the following items with custom tasks: * Pipeline Resources -### Known Custom Tasks +### Known Custom Tasks We try to list as many known Custom Tasks as possible here so that users can easily find what they want. Please feel free to share the Custom Task you implemented in this table. @@ -1790,7 +1791,7 @@ We try to list as many known Custom Tasks as possible here so that users can eas | [Common Expression Language][cel]| Provides Common Expression Language support in Tekton Pipelines. | [Wait][wait]| Waits a given amount of time, specified by a `Parameter` named "duration", before succeeding. | [Approvals][approvals]| Pauses the execution of `PipelineRuns` and waits for manual approvals. -| [Pipelines in Pipelines][pipelines-in-pipelines]| Defines and executes a `Pipeline` in a `Pipeline`. +| [Pipelines in Pipelines][pipelines-in-pipelines]| Defines and executes a `Pipeline` in a `Pipeline`. | [Task Group][task-group]| Groups `Tasks` together as a `Task`. | [Pipeline in a Pod][pipeline-in-pod]| Runs `Pipeline` in a `Pod`. diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index a6423a9f39a..d53c8e3141a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -736,8 +736,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() if after.Status == corev1.ConditionTrue || after.Status == corev1.ConditionFalse { - pr.Status.PipelineResults, err = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, - pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults()) + pr.Status.PipelineResults, err = resources.ApplyTaskResultsToPipelineResults(ctx, pipelineSpec.Results, + pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults(), pr.Status.SkippedTasks) if err != nil { pr.Status.MarkFailed(ReasonFailedValidation, err.Error()) return err diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index c43ec9e5dbf..a09e6daa335 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -316,11 +316,18 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st // and omitted from the returned slice. A nil slice is returned if no results are passed in or all // results are invalid. func ApplyTaskResultsToPipelineResults( + ctx context.Context, results []v1beta1.PipelineResult, taskRunResults map[string][]v1beta1.TaskRunResult, - customTaskResults map[string][]v1beta1.CustomRunResult) ([]v1beta1.PipelineRunResult, error) { + customTaskResults map[string][]v1beta1.CustomRunResult, + skippedTasks []v1beta1.SkippedTask) ([]v1beta1.PipelineRunResult, error) { var runResults []v1beta1.PipelineRunResult var invalidPipelineResults []string + skippedTaskNames := map[string]bool{} + for _, t := range skippedTasks { + skippedTaskNames[t.Name] = true + } + stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} objectReplacements := map[string]map[string]string{} @@ -341,6 +348,11 @@ func ApplyTaskResultsToPipelineResults( continue } variableParts := strings.Split(variable, ".") + // if the referenced task is skipped, we should also skip the results replacements + if _, ok := skippedTaskNames[variableParts[1]]; ok { + validPipelineResult = false + continue + } if (variableParts[0] != v1beta1.ResultTaskPart && variableParts[0] != v1beta1.ResultFinallyPart) || variableParts[2] != v1beta1.ResultResultPart { validPipelineResult = false invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index af3c0503318..3348543dd63 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -3131,7 +3131,7 @@ func TestApplyFinallyResultsToPipelineResults(t *testing.T) { }, } { t.Run(tc.description, func(t *testing.T) { - received, _ := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults) + received, _ := ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, nil /* skippedTasks */) if d := cmp.Diff(tc.expected, received); d != "" { t.Errorf(diff.PrintWantGot(d)) } @@ -3145,6 +3145,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { results []v1beta1.PipelineResult taskResults map[string][]v1beta1.TaskRunResult runResults map[string][]v1beta1.CustomRunResult + skippedTasks []v1beta1.SkippedTask expectedResults []v1beta1.PipelineRunResult expectedError error }{{ @@ -3599,9 +3600,28 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "pipeline-result-2", Value: *v1beta1.NewStructuredValues("do, rae, mi, rae, do"), }}, + }, { + description: "multiple-results-skipped-and-normal-tasks", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo)"), + }, { + Name: "pipeline-result-2", + Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo), $(tasks.normaltask.results.baz)"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "normaltask": {{ + Name: "baz", + Value: *v1beta1.NewStructuredValues("rae"), + }}, + }, + skippedTasks: []v1beta1.SkippedTask{{ + Name: "skippedTask", + }}, + expectedResults: nil, }} { t.Run(tc.description, func(t *testing.T) { - received, err := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults) + received, err := ApplyTaskResultsToPipelineResults(context.Background(), tc.results, tc.taskResults, tc.runResults, tc.skippedTasks) if tc.expectedError != nil { if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { t.Errorf("ApplyTaskResultsToPipelineResults() errors diff %s", diff.PrintWantGot(d))