Skip to content

Commit

Permalink
cleanup,test: differenciate between unresolved parameters and circula…
Browse files Browse the repository at this point in the history
…r dependencies
  • Loading branch information
waveywaves committed Jan 28, 2025
1 parent 08aaf78 commit 3127dbe
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
25 changes: 23 additions & 2 deletions pkg/reconciler/taskrun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun,
// process parameters until no more can be resolved
for len(paramsNeedingResolution) > 0 {
paramWasResolved := false
// track unresolved params and their references
unresolvedParams := make(map[string][]string)

for paramName := range paramsNeedingResolution {
canResolveParam := true
for _, referencedParam := range paramReferenceMap[paramName] {
Expand All @@ -144,6 +147,7 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun,
}
if !isReferenceResolved {
canResolveParam = false
unresolvedParams[paramName] = append(unresolvedParams[paramName], referencedParam)
break
}
}
Expand Down Expand Up @@ -180,9 +184,26 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun,
}
}

// circular dependency detected
// unresolvable parameters or circular dependencies
if !paramWasResolved {
return nil, fmt.Errorf("circular dependency detected in parameter references")
// check parameter references to a non-existent parameter
for param, unresolvedRefs := range unresolvedParams {
// check referenced parameters in defaults
for _, ref := range unresolvedRefs {
exists := false
for _, p := range defaults {
if p.Name == ref {
exists = true
break
}
}
if !exists {
return nil, fmt.Errorf("parameter %q references non-existent parameter %q", param, ref)
}
}
// parameters exist but can't be resolved hence it's a circular dependency
return nil, fmt.Errorf("circular dependency detected in parameter references")

Check failure on line 205 in pkg/reconciler/taskrun/resources/apply.go

View workflow job for this annotation

GitHub Actions / lint

fmt.Errorf can be replaced with errors.New (perfsprint)
}
}
}

Expand Down
36 changes: 36 additions & 0 deletions pkg/reconciler/taskrun/resources/taskspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,42 @@ func TestGetStepActionsData_Error(t *testing.T) {
stepAction *v1beta1.StepAction
expectedError error
}{{
name: "unresolvable parameter reference",
tr: &v1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "mytaskrun",
Namespace: "default",
},
Spec: v1.TaskRunSpec{
TaskSpec: &v1.TaskSpec{
Steps: []v1.Step{{
Ref: &v1.Ref{
Name: "stepAction",
},
}},
},
},
},
stepAction: &v1beta1.StepAction{
ObjectMeta: metav1.ObjectMeta{
Name: "stepAction",
Namespace: "default",
},
Spec: v1beta1.StepActionSpec{
Image: "myimage",
Args: []string{"$(params.param1)"},
Params: v1.ParamSpecs{{
Name: "param1",
Type: v1.ParamTypeString,
Default: &v1.ParamValue{
Type: v1.ParamTypeString,
StringVal: "$(params.nonexistent)",
},
}},
},
},
expectedError: errors.New(`parameter "param1" references non-existent parameter "nonexistent"`),
}, {
name: "circular dependency in params",
tr: &v1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit 3127dbe

Please sign in to comment.