From fddbbbe6ce58b243dd1f3a6fa83bfecd12e820a6 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 27 Jun 2024 18:37:26 -0400 Subject: [PATCH] Handle error conditions in CheckMissingResultReferences Before this change, it was possible for a PipelineRun to exist which would cause the controller to crashloop. --- .../resources/pipelinerunresolution.go | 11 +++++- .../resources/resultrefresolution_test.go | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index dde3dd6bc0e..381424b19bb 100644 --- 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 index ef18e506b13..ca2f54be49f 100644 --- 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)