Handle error conditions in CheckMissingResultReferences

Before this change, it was possible for a PipelineRun to exist which would cause the controller to crashloop.
tektoncd · Jul 9, 2024 · 969f5d3 · 969f5d3
1 parent c2dc17a
commit 969f5d3
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 1 deletion.
diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -797,14 +797,23 @@ func isCustomRunCancelledByPipelineRunTimeout(cr *v1beta1.CustomRun) bool {
 func CheckMissingResultReferences(pipelineRunState PipelineRunState, targets PipelineRunState) error {
 	for _, target := range targets {
 		for _, resultRef := range v1.PipelineTaskResultRefs(target.PipelineTask) {
-			referencedPipelineTask := pipelineRunState.ToMap()[resultRef.PipelineTask]
+			referencedPipelineTask, ok := pipelineRunState.ToMap()[resultRef.PipelineTask]
+			if !ok {
+				return fmt.Errorf("Result reference error: Could not find ref \"%s\" in internal pipelineRunState", resultRef.PipelineTask)
+			}
 			if referencedPipelineTask.IsCustomTask() {
+				if len(referencedPipelineTask.CustomRuns) == 0 {
+					return fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length CustomRuns", resultRef.PipelineTask)
+				}
 				customRun := referencedPipelineTask.CustomRuns[0]
 				_, err := findRunResultForParam(customRun, resultRef)
 				if err != nil {
 					return err
 				}
 			} else {
+				if len(referencedPipelineTask.TaskRuns) == 0 {
+					return fmt.Errorf("Result reference error: Internal result ref \"%s\" has zero-length TaskRuns", resultRef.PipelineTask)
+				}
 				taskRun := referencedPipelineTask.TaskRuns[0]
 				_, err := findTaskResultForParam(taskRun, resultRef)
 				if err != nil {

diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go
@@ -341,6 +341,28 @@ var pipelineRunState = PipelineRunState{{
 			Value: *v1.NewStructuredValues("$(tasks.kTask.results.I-DO-NOT-EXIST)[*]"),
 		}},
 	},
+}, {
+	TaskRunNames: []string{"lTaskRun"},
+	TaskRuns:     []*v1.TaskRun{},
+	PipelineTask: &v1.PipelineTask{
+		Name:    "lTask",
+		TaskRef: &v1.TaskRef{Name: "lTask"},
+		Params: []v1.Param{{
+			Name:  "jParam",
+			Value: *v1.NewStructuredValues("$(tasks.does-not-exist.results.some-result)"),
+		}},
+	},
+}, {
+	TaskRunNames: []string{"mTaskRun"},
+	TaskRuns:     []*v1.TaskRun{},
+	PipelineTask: &v1.PipelineTask{
+		Name:    "mTask",
+		TaskRef: &v1.TaskRef{Name: "mTask"},
+		Params: []v1.Param{{
+			Name:  "mParam",
+			Value: *v1.NewStructuredValues("$(tasks.lTask.results.aResult)"),
+		}},
+	},
 }}
 
 func TestResolveResultRefs(t *testing.T) {
@@ -724,6 +746,20 @@ func TestCheckMissingResultReferences(t *testing.T) {
 			pipelineRunState[14],
 		},
 		wantErr: "Invalid task result reference: Could not find result with name iDoNotExist for task aCustomPipelineTask",
+	}, {
+		name:             "Invalid: Test result references where ref does not exist in pipelineRunState map",
+		pipelineRunState: pipelineRunState,
+		targets: PipelineRunState{
+			pipelineRunState[18],
+		},
+		wantErr: "Result reference error: Could not find ref \"does-not-exist\" in internal pipelineRunState",
+	}, {
+		name:             "Invalid: Test result references where referencedPipelineTask has no TaskRuns",
+		pipelineRunState: pipelineRunState,
+		targets: PipelineRunState{
+			pipelineRunState[19],
+		},
+		wantErr: "Result reference error: Internal result ref \"lTask\" has zero-length TaskRuns",
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
 			err := CheckMissingResultReferences(tt.pipelineRunState, tt.targets)