don't validate skipped task results for pipeline results

This commit skip the validation for skipped task results in pipeline results. This fixes tektoncd#6139. The skipped task results are not emitted by design thus we shouldn't validate if they are emitted. Signed-off-by: Yongxuan Zhang [email protected]
Yongxuanzhang · Feb 13, 2023 · 5847110 · 5847110
1 parent 8407669
commit 5847110
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 4 deletions.
diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -720,7 +720,7 @@ 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())
+			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
@@ -318,9 +318,15 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st
 func ApplyTaskResultsToPipelineResults(
 	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 +347,10 @@ 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 {
+				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
@@ -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(tc.results, tc.taskResults, tc.runResults, nil)
 			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,34 @@ 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: []v1beta1.PipelineRunResult{{
+			Name:  "pipeline-result-1",
+			Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo)"),
+		}, {
+			Name:  "pipeline-result-2",
+			Value: *v1beta1.NewStructuredValues("$(tasks.skippedTask.results.foo), rae"),
+		}},
 	}} {
 		t.Run(tc.description, func(t *testing.T) {
-			received, err := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults)
+			received, err := ApplyTaskResultsToPipelineResults(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))