Repository: tektoncd/pipeline (language: go)

Issue:
Parameter substitution with default values for StepAction git resolver not working
# Expected Behavior
* Taskrun should complete successfully without requiring to specify values for git resolver parameters for StepActions.

# Actual Behavior

```
error requesting remote resource: invalid resource request "dev-release-team-tenant/git-b066d2cf02c4cb2137da7606529dd96d": invalid git repository url: $(params.stepActionGitUrl)
```
# Steps to Reproduce the Problem

1. Create both Task and TaskRun from below:

```
---
apiVersion: tekton.dev/v1
kind: Task
metadata:
  name: my-task
spec:
  params:
    - name: stepActionGitUrl
      type: string
      default: https://github.com/scoheb/my-catalog.git
    - name: stepActionGitRevision
      type: string
      default: main
  steps:
    - name: step1
      ref:
        resolver: "git"
        params:
          - name: url
            value: "$(params.stepActionGitUrl)"
          - name: revision
            value: "$(params.stepActionGitRevision)"
          - name: pathInRepo
            value: stepactions/my-stepaction.yaml
```

```
apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  generateName: step-action-run-
spec:
  taskRef:
    name: my-task
```
2. Observe the error

# Additional Info

- Kubernetes version: 

  **Output of `kubectl version`:**

```
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.0", GitCommit:"a866cbe2e5bbaa01cfd5e969aa3e033f3282a8a2", GitTreeState:"clean", BuildDate:"2022-08-23T17:44:59Z", GoVersion:"go1.19", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"28", GitVersion:"v1.28.15+ff493be", GitCommit:"4cf5291f1e18d974b97cae658aa9b2654bd9ea29", GitTreeState:"clean", BuildDate:"2024-11-23T03:11:13Z", GoVersion:"go1.20.12 X:strictfipsruntime", Compiler:"gc", Platform:"linux/amd64"}
WARNING: version difference between client (1.25) and server (1.28) exceeds the supported minor version skew of +/-1
```

- **Tekton Pipelines** version:

  **Output of `kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'`**

```
v0.69.0
```


<!-- Any other additional information -->



Discussion / hints:

Fix patch:
diff --git a/examples/v1/taskruns/default_task_params.yaml b/examples/v1/taskruns/default_task_params.yaml
new file mode 100644
index 00000000000..0a8f426e5f6
--- /dev/null
+++ b/examples/v1/taskruns/default_task_params.yaml
@@ -0,0 +1,27 @@
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  # This has to be explicit instead of `generateName`, since it will be referenced
+  # by the TaskRun
+  name: example-default-task-param
+spec:
+  params:
+    - name: input
+      default: "No input provided, but that's okay!"
+  steps:
+    - name: echo-input
+      image: mirror.gcr.io/ubuntu
+      script: |
+        echo "$(params.input)"
+---
+apiVersion: tekton.dev/v1
+kind: TaskRun
+metadata:
+  generateName: default-task-params-run-
+spec:
+  taskRef:
+    name: example-default-task-param
+    # # Uncomment this block to override the default param value!
+    # params:
+    #   - name: input
+    #     value: "You can supply the param from the TaskRun if the default not what you want"
diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go
index 8b493cbf7e1..2a1e9d8ca9b 100644
--- a/pkg/reconciler/taskrun/resources/taskref.go
+++ b/pkg/reconciler/taskrun/resources/taskref.go
@@ -138,7 +138,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
 // It also requires a kubeclient, tektonclient, requester in case it needs to find that task in
 // cluster or authorize against an external repository. It will figure out whether it needs to look in the cluster or in
 // a remote location to fetch the reference.
-func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester, tr *v1.TaskRun, step *v1.Step) GetStepAction {
+func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester, tr *v1.TaskRun, taskSpec v1.TaskSpec, step *v1.Step) GetStepAction {
 	trName := tr.Name
 	namespace := tr.Namespace
 	if step.Ref != nil && step.Ref.Resolver != "" && requester != nil {
@@ -146,7 +146,7 @@ func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, req
 		// casting it to a StepAction.
 		return func(ctx context.Context, name string) (*v1beta1.StepAction, *v1.RefSource, error) {
 			// Perform params replacements for StepAction resolver params
-			ApplyParameterSubstitutionInResolverParams(tr, step)
+			ApplyParameterSubstitutionInResolverParams(tr, taskSpec, step)
 			resolverPayload := remoteresource.ResolverPayload{
 				Name:      trName,
 				Namespace: namespace,
@@ -167,14 +167,14 @@ func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, req
 }
 
 // ApplyParameterSubstitutionInResolverParams applies parameter substitutions in resolver params for Step Ref.
-func ApplyParameterSubstitutionInResolverParams(tr *v1.TaskRun, step *v1.Step) {
+func ApplyParameterSubstitutionInResolverParams(tr *v1.TaskRun, taskSpec v1.TaskSpec, step *v1.Step) {
 	stringReplacements := make(map[string]string)
 	arrayReplacements := make(map[string][]string)
 	objectReplacements := make(map[string]map[string]string)
-	if tr.Spec.TaskSpec != nil {
-		defaultSR, defaultAR, defaultOR := replacementsFromDefaultParams(tr.Spec.TaskSpec.Params)
-		stringReplacements, arrayReplacements, objectReplacements = extendReplacements(stringReplacements, arrayReplacements, objectReplacements, defaultSR, defaultAR, defaultOR)
-	}
+
+	defaultSR, defaultAR, defaultOR := replacementsFromDefaultParams(taskSpec.Params)
+	stringReplacements, arrayReplacements, objectReplacements = extendReplacements(stringReplacements, arrayReplacements, objectReplacements, defaultSR, defaultAR, defaultOR)
+
 	paramSR, paramAR, paramOR := replacementsFromParams(tr.Spec.Params)
 	stringReplacements, arrayReplacements, objectReplacements = extendReplacements(stringReplacements, arrayReplacements, objectReplacements, paramSR, paramAR, paramOR)
 	step.Ref.Params = step.Ref.Params.ReplaceVariables(stringReplacements, arrayReplacements, objectReplacements)
diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go
index ae4e89c3f2c..e9b683d7520 100644
--- a/pkg/reconciler/taskrun/resources/taskspec.go
+++ b/pkg/reconciler/taskrun/resources/taskspec.go
@@ -107,7 +107,7 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T
 	for i, step := range taskSpec.Steps {
 		s := step.DeepCopy()
 		if step.Ref != nil {
-			getStepAction := GetStepActionFunc(tekton, k8s, requester, taskRun, s)
+			getStepAction := GetStepActionFunc(tekton, k8s, requester, taskRun, taskSpec, s)
 			stepAction, source, err := getStepAction(ctx, s.Ref.Name)
 			if err != nil {
 				return nil, err


Test patch:
diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go
index 2354743e63a..526e37aa69b 100644
--- a/pkg/reconciler/taskrun/resources/taskref_test.go
+++ b/pkg/reconciler/taskrun/resources/taskref_test.go
@@ -347,6 +347,7 @@ func TestStepActionResolverParamReplacements(t *testing.T) {
 		name      string
 		namespace string
 		taskrun   *v1.TaskRun
+		taskSpec  *v1.TaskSpec
 		want      *v1.Step
 	}{{
 		name:      "default taskspec parms",
@@ -585,11 +586,53 @@ func TestStepActionResolverParamReplacements(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name:      "defaults from remote task",
+		namespace: "default",
+		taskrun: &v1.TaskRun{
+			ObjectMeta: metav1.ObjectMeta{Name: "some-tr"},
+			Spec: v1.TaskRunSpec{
+				TaskRef: &v1.TaskRef{
+					Name: "resolved-task-name",
+				},
+			},
+		},
+		taskSpec: &v1.TaskSpec{
+			Params: []v1.ParamSpec{{
+				Name:    "resolver-param",
+				Default: v1.NewStructuredValues("foo/bar"),
+			}},
+			Steps: []v1.Step{{
+				Ref: &v1.Ref{
+					ResolverRef: v1.ResolverRef{
+						Resolver: "git",
+						Params: []v1.Param{{
+							Name:  "pathInRepo",
+							Value: *v1.NewStructuredValues("$(params.resolver-param)"),
+						}},
+					},
+				},
+			}},
+		},
+		want: &v1.Step{
+			Ref: &v1.Ref{
+				ResolverRef: v1.ResolverRef{
+					Resolver: "git",
+					Params: []v1.Param{{
+						Name:  "pathInRepo",
+						Value: *v1.NewStructuredValues("foo/bar"),
+					}},
+				},
+			},
+		},
 	}}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			step := &tc.taskrun.Spec.TaskSpec.Steps[0]
-			resources.ApplyParameterSubstitutionInResolverParams(tc.taskrun, step)
+			if tc.taskSpec == nil {
+				tc.taskSpec = tc.taskrun.Spec.TaskSpec
+			}
+			step := &tc.taskSpec.Steps[0]
+			resources.ApplyParameterSubstitutionInResolverParams(tc.taskrun, *tc.taskSpec, step)
 			if d := cmp.Diff(tc.want, step); tc.want != nil && d != "" {
 				t.Error(diff.PrintWantGot(d))
 			}
@@ -861,7 +904,7 @@ func TestGetStepActionFunc_Local(t *testing.T) {
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 			tektonclient := fake.NewSimpleClientset(tc.localStepActions...)
-			fn := resources.GetStepActionFunc(tektonclient, nil, nil, tc.taskRun, &tc.taskRun.Spec.TaskSpec.Steps[0])
+			fn := resources.GetStepActionFunc(tektonclient, nil, nil, tc.taskRun, *tc.taskRun.Spec.TaskSpec, &tc.taskRun.Spec.TaskSpec.Steps[0])
 
 			stepAction, refSource, err := fn(ctx, tc.taskRun.Spec.TaskSpec.Steps[0].Ref.Name)
 			if err != nil {
@@ -922,7 +965,7 @@ func TestGetStepActionFunc_RemoteResolution_Success(t *testing.T) {
 				},
 			}
 			tektonclient := fake.NewSimpleClientset()
-			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, &tr.Spec.TaskSpec.Steps[0])
+			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, *tr.Spec.TaskSpec, &tr.Spec.TaskSpec.Steps[0])
 
 			resolvedStepAction, resolvedRefSource, err := fn(ctx, tr.Spec.TaskSpec.Steps[0].Ref.Name)
 			if tc.wantErr {
@@ -983,7 +1026,7 @@ func TestGetStepActionFunc_RemoteResolution_Error(t *testing.T) {
 				},
 			}
 			tektonclient := fake.NewSimpleClientset()
-			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, &tr.Spec.TaskSpec.Steps[0])
+			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, *tr.Spec.TaskSpec, &tr.Spec.TaskSpec.Steps[0])
 			if _, _, err := fn(ctx, tr.Spec.TaskSpec.Steps[0].Ref.Name); err == nil {
 				t.Fatalf("expected error due to invalid pipeline data but saw none")
 			}
diff --git a/test/taskrun_test.go b/test/taskrun_test.go
index f720a576d36..d8382c18331 100644
--- a/test/taskrun_test.go
+++ b/test/taskrun_test.go
@@ -604,3 +604,101 @@ spec:
 		t.Fatalf("expected 1 retry status, got %d", len(taskrun.Status.RetriesStatus))
 	}
 }
+
+func TestTaskRunResolveDefaultParameterSubstitutionOnStepAction(t *testing.T) {
+	ctx := context.Background()
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	c, namespace := setup(ctx, t, requireAllGates(requireEnableStepActionsGate))
+
+	knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf)
+	defer tearDown(ctx, t, c, namespace)
+
+	t.Logf("Creating Task and TaskRun in namespace %s", namespace)
+	task := parse.MustParseV1Task(t, fmt.Sprintf(`
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  params:
+    - name: repository
+      type: string
+      default: https://github.com/tektoncd/catalog.git
+    - name: revision
+      type: string
+      default: main
+  steps:
+    - name: clone
+      ref:
+        resolver: git
+        params:
+        - name: url
+          value: "$(params.repository)"
+        - name: pathInRepo
+          value: /stepaction/git-clone/0.1/git-clone.yaml
+        - name: revision
+          value: "$(params.revision)"
+      params:
+        - name: output-path
+          value: "/tmp"
+        - name: url
+          value: $(params.repository)
+        - name: revision
+          value: $(params.revision)
+`, helpers.ObjectNameForTest(t), namespace))
+	if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil {
+		t.Fatalf("Failed to create Task: %s", err)
+	}
+
+	taskRunName := helpers.ObjectNameForTest(t)
+	taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(`
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  taskRef:
+    name: %s
+  retries: 1
+`, taskRunName, namespace, task.Name))
+	if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil {
+		t.Fatalf("Failed to create TaskRun: %s", err)
+	}
+
+	t.Logf("Waiting for TaskRun in namespace %s to complete", namespace)
+	if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceed", v1Version); err != nil {
+		t.Errorf("Error waiting for TaskRun to finish: %s", err)
+	}
+
+	taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err)
+	}
+
+	if !isSuccessful(t, taskrun.GetName(), taskrun.Status.Conditions) {
+		t.Fatalf("task should have succeeded")
+	}
+
+	expectedReason := "Succeeded"
+	actualReason := taskrun.Status.GetCondition(apis.ConditionSucceeded).GetReason()
+	if actualReason != expectedReason {
+		t.Fatalf("expected TaskRun to have failed reason %s, got %s", expectedReason, actualReason)
+	}
+
+	expectedStepState := []v1.StepState{{
+		ContainerState: corev1.ContainerState{
+			Terminated: &corev1.ContainerStateTerminated{
+				ExitCode: 0,
+				Reason:   "Completed",
+			},
+		},
+		TerminationReason: "Completed",
+		Name:              "clone",
+		Container:         "step-clone",
+	}}
+	ignoreTerminatedFields := cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt", "ContainerID", "Message")
+	ignoreStepFields := cmpopts.IgnoreFields(v1.StepState{}, "ImageID", "Results", "Provenance")
+	if d := cmp.Diff(taskrun.Status.Steps, expectedStepState, ignoreTerminatedFields, ignoreStepFields); d != "" {
+		t.Fatalf("-got, +want: %v", d)
+	}
+}