fix: reference params in default values, allow chained references in stepactions

docs/stepactions.md

@@ -141,14 +141,15 @@ spec:
 When applying parameters to a StepAction, the substitutions are applied in the following order:
 
 1. TaskRun parameter values in step parameters
-2. Parameters from StepAction defaults
-3. Parameters from the step (overwriting any defaults)
-4. Step result replacements
+2. Step-provided parameter values 
+3. Default values that reference other parameters
+4. Simple default values
+5. Step result references
 
 This order ensures that:
-- Step parameters can reference TaskRun parameters
-- StepAction defaults provide fallback values
-- Step-specific parameters take precedence over defaults
+- TaskRun parameters are available for step parameter substitution
+- Step-provided values take precedence over defaults
+- Parameter references in defaults are resolved before simple defaults
 - Step result references are resolved last to allow referencing results from previous steps
 
 ### Emitting Results

pkg/apis/pipeline/v1alpha1/stepaction_validation.go

@@ -15,6 +15,7 @@ package v1alpha1
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
@@ -128,7 +129,80 @@ func validateParameterVariables(ctx context.Context, sas StepActionSpec, params
 	stringParameterNames := sets.NewString(stringParams.GetNames()...)
 	arrayParameterNames := sets.NewString(arrayParams.GetNames()...)
 	errs = errs.Also(v1.ValidateNameFormat(stringParameterNames.Insert(arrayParameterNames.List()...), objectParams))
-	return errs.Also(validateStepActionArrayUsage(sas, "params", arrayParameterNames))
+	errs = errs.Also(validateStepActionArrayUsage(sas, "params", arrayParameterNames))
+	return errs.Also(validateDefaultParameterReferences(params))
+}
+
+// validateDefaultParameterReferences ensures that parameters referenced in default values are defined
+func validateDefaultParameterReferences(params v1.ParamSpecs) *apis.FieldError {
+	var errs *apis.FieldError
+	allParams := sets.NewString(params.GetNames()...)
+
+	// First pass: collect all parameters that have no references in their default values
+	resolvedParams := sets.NewString()
+	paramsNeedingResolution := make(map[string][]string)
+
+	for _, p := range params {
+		if p.Default != nil {
+			matches, _ := substitution.ExtractVariableExpressions(p.Default.StringVal, "params")
+			if len(matches) == 0 {
+				resolvedParams.Insert(p.Name)
+			} else {
+				// Track which parameters this parameter depends on
+				referencedParams := make([]string, 0, len(matches))
+				hasUndefinedParam := false
+				for _, match := range matches {
+					paramName := strings.TrimSuffix(strings.TrimPrefix(match, "$(params."), ")")
+					if !allParams.Has(paramName) {
+						hasUndefinedParam = true
+						errs = errs.Also(&apis.FieldError{
+							Message: fmt.Sprintf("param %q default value references param %q which is not defined", p.Name, paramName),
+							Paths:   []string{"params"},
+						})
+					}
+					referencedParams = append(referencedParams, paramName)
+				}
+				// Only track dependencies if all referenced parameters exist
+				if !hasUndefinedParam {
+					paramsNeedingResolution[p.Name] = referencedParams
+				}
+			}
+		} else {
+			resolvedParams.Insert(p.Name)
+		}
+	}
+
+	// Second pass: iteratively resolve parameters whose dependencies are satisfied
+	for len(paramsNeedingResolution) > 0 {
+		paramWasResolved := false
+		for paramName, referencedParams := range paramsNeedingResolution {
+			canResolveParam := true
+			for _, referencedParam := range referencedParams {
+				if !resolvedParams.Has(referencedParam) {
+					canResolveParam = false
+					break
+				}
+			}
+			if canResolveParam {
+				resolvedParams.Insert(paramName)
+				delete(paramsNeedingResolution, paramName)
+				paramWasResolved = true
+			}
+		}
+		if !paramWasResolved {
+			// If we couldn't resolve any parameters in this iteration,
+			// we have a circular dependency
+			for paramName := range paramsNeedingResolution {
+				errs = errs.Also(&apis.FieldError{
+					Message: fmt.Sprintf("param %q default value has a circular dependency", paramName),
+					Paths:   []string{"params"},
+				})
+			}
+			break
+		}
+	}
+
+	return errs
 }
 
 // validateObjectUsage validates the usage of individual attributes of an object param and the usage of the entire object

pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go

@@ -80,6 +80,38 @@ func TestStepActionSpecValidate(t *testing.T) {
 			Command: []string{"ls"},
 			Args:    []string{"-lh"},
 		},
+	}, {
+		name: "valid param default value references defined param",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param1",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("hello"),
+			}, {
+				Name:    "param2",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1) world"),
+			}},
+		},
+	}, {
+		name: "multiple param references in default value",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param1",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("hello"),
+			}, {
+				Name:    "param2",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("hi"),
+			}, {
+				Name:    "param3",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1) $(params.param2)"),
+			}},
+		},
 	}, {
 		name: "step action with script",
 		fields: fields{
@@ -540,6 +572,69 @@ func TestStepActionValidateError(t *testing.T) {
 			Message: `non-existent variable in "$(params.gitrepo.foo)"`,
 			Paths:   []string{"spec.volumeMounts[0]"},
 		},
+	}, {
+		name: "circular dependency in param default values",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param1",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param2)"),
+			}, {
+				Name:    "param2",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1)"),
+			}},
+		},
+		expectedError: *((&apis.FieldError{
+			Message: `param "param1" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		}).Also(&apis.FieldError{
+			Message: `param "param2" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		})),
+	}, {
+		name: "complex circular dependency in param default values",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param1",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param2)"),
+			}, {
+				Name:    "param2",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param3)"),
+			}, {
+				Name:    "param3",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1)"),
+			}},
+		},
+		expectedError: *((&apis.FieldError{
+			Message: `param "param1" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		}).Also(&apis.FieldError{
+			Message: `param "param2" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		}).Also(&apis.FieldError{
+			Message: `param "param3" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		})),
+	}, {
+		name: "self-referential param default value",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param1",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1)"),
+			}},
+		},
+		expectedError: apis.FieldError{
+			Message: `param "param1" default value has a circular dependency`,
+			Paths:   []string{"spec.params"},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -562,6 +657,7 @@ func TestStepActionValidateError(t *testing.T) {
 			if err == nil {
 				t.Fatalf("Expected an error, got nothing for %v", sa)
 			}
+			t.Logf("Actual error: %v", err)
 			if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
 				t.Errorf("StepActionSpec.Validate() errors diff %s", diff.PrintWantGot(d))
 			}
@@ -592,6 +688,20 @@ func TestStepActionSpecValidateError(t *testing.T) {
 			Message: `missing field(s)`,
 			Paths:   []string{"Image"},
 		},
+	}, {
+		name: "param default value references undefined param",
+		fields: fields{
+			Image: "myimage",
+			Params: []v1.ParamSpec{{
+				Name:    "param2",
+				Type:    v1.ParamTypeString,
+				Default: v1.NewStructuredValues("$(params.param1)"),
+			}},
+		},
+		expectedError: apis.FieldError{
+			Message: `param "param2" default value references param "param1" which is not defined`,
+			Paths:   []string{"params"},
+		},
 	}, {
 		name: "command and script both used.",
 		fields: fields{
@@ -967,6 +1077,7 @@ func TestStepActionSpecValidateError(t *testing.T) {
 			if err == nil {
 				t.Fatalf("Expected an error, got nothing for %v", sa)
 			}
+			t.Logf("Actual error: %v", err)
 			if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
 				t.Errorf("StepActionSpec.Validate() errors diff %s", diff.PrintWantGot(d))
 			}