TEP-0142: Add syntax for providing params to StepActions

chitrangpatel · tekton-robot · commit 7e3897a6353f · 2023-11-07T20:22:52.000Z
Following the previous [PR](#7317), which introduced Params to the `StepAction` CRD, this PR integrates `param` usage between `Steps` and `StepActions`. This PR adds the necessary syntax to `Steps` to pass `params` to `StepActions`. This work is part of issue #7259.
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -1802,7 +1802,7 @@ map[string]string
 <h3 id="tekton.dev/v1.Params">Params
 (<code>[]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Param</code> alias)</h3>
 <p>
-(<em>Appears on:</em><a href="#tekton.dev/v1.IncludeParams">IncludeParams</a>, <a href="#tekton.dev/v1.Matrix">Matrix</a>, <a href="#tekton.dev/v1.PipelineRunSpec">PipelineRunSpec</a>, <a href="#tekton.dev/v1.PipelineTask">PipelineTask</a>, <a href="#tekton.dev/v1.ResolverRef">ResolverRef</a>, <a href="#tekton.dev/v1.TaskRunInputs">TaskRunInputs</a>, <a href="#tekton.dev/v1.TaskRunSpec">TaskRunSpec</a>)
+(<em>Appears on:</em><a href="#tekton.dev/v1.IncludeParams">IncludeParams</a>, <a href="#tekton.dev/v1.Matrix">Matrix</a>, <a href="#tekton.dev/v1.PipelineRunSpec">PipelineRunSpec</a>, <a href="#tekton.dev/v1.PipelineTask">PipelineTask</a>, <a href="#tekton.dev/v1.ResolverRef">ResolverRef</a>, <a href="#tekton.dev/v1.Step">Step</a>, <a href="#tekton.dev/v1.TaskRunInputs">TaskRunInputs</a>, <a href="#tekton.dev/v1.TaskRunSpec">TaskRunSpec</a>)
 </p>
 <div>
 <p>Params is a list of Param</p>
@@ -4427,6 +4427,20 @@ Ref
 <p>Contains the reference to an existing StepAction.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>params</code><br/>
+<em>
+<a href="#tekton.dev/v1.Params">
+Params
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Params declares parameters passed to this step action.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="tekton.dev/v1.StepOutputConfig">StepOutputConfig
@@ -10215,7 +10229,7 @@ map[string]string
 <h3 id="tekton.dev/v1beta1.Params">Params
 (<code>[]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param</code> alias)</h3>
 <p>
-(<em>Appears on:</em><a href="#tekton.dev/v1alpha1.RunSpec">RunSpec</a>, <a href="#tekton.dev/v1beta1.CustomRunSpec">CustomRunSpec</a>, <a href="#tekton.dev/v1beta1.IncludeParams">IncludeParams</a>, <a href="#tekton.dev/v1beta1.Matrix">Matrix</a>, <a href="#tekton.dev/v1beta1.PipelineRunSpec">PipelineRunSpec</a>, <a href="#tekton.dev/v1beta1.PipelineTask">PipelineTask</a>, <a href="#tekton.dev/v1beta1.ResolverRef">ResolverRef</a>, <a href="#tekton.dev/v1beta1.TaskRunSpec">TaskRunSpec</a>)
+(<em>Appears on:</em><a href="#tekton.dev/v1alpha1.RunSpec">RunSpec</a>, <a href="#tekton.dev/v1beta1.CustomRunSpec">CustomRunSpec</a>, <a href="#tekton.dev/v1beta1.IncludeParams">IncludeParams</a>, <a href="#tekton.dev/v1beta1.Matrix">Matrix</a>, <a href="#tekton.dev/v1beta1.PipelineRunSpec">PipelineRunSpec</a>, <a href="#tekton.dev/v1beta1.PipelineTask">PipelineTask</a>, <a href="#tekton.dev/v1beta1.ResolverRef">ResolverRef</a>, <a href="#tekton.dev/v1beta1.Step">Step</a>, <a href="#tekton.dev/v1beta1.TaskRunSpec">TaskRunSpec</a>)
 </p>
 <div>
 <p>Params is a list of Param</p>
@@ -13227,6 +13241,20 @@ Ref
 <p>Contains the reference to an existing StepAction.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>params</code><br/>
+<em>
+<a href="#tekton.dev/v1beta1.Params">
+Params
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Params declares parameters passed to this step action.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="tekton.dev/v1beta1.StepOutputConfig">StepOutputConfig
diff --git a/docs/stepactions.md b/docs/stepactions.md
@@ -206,6 +206,8 @@ When a `Step` is referencing a `StepAction`, it can contain the following fields
 - `securityContext`
 - `envFrom`
 - `timeout`
+- `ref`
+- `params`
 
 Using any of the above fields and referencing a `StepAction` is allowed and will not cause an error. For example, the `TaskRun` below will execute without any errors:
 
@@ -220,10 +222,39 @@ spec:
       - name: action-runner
         ref:
           name: step-action
+        params:
+          - name: step-action-param
+            value: hello
         computeResources:
           requests:
             memory: 1Gi
             cpu: 500m
         timeout: 1h
         onError: continue
 ```
+
+### Passing Params to StepAction 
+
+A `StepAction` may require [params](#(declaring-parameters)). In this case, a `Task` needs to ensure that the `StepAction` has access to all the required `params`.
+When referencing a `StepAction`, a `Step` can also provide it with `params`, just like how a `TaskRun` provides params to the underlying `Task`.
+
+```yaml
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  name: step-action
+spec:
+  TaskSpec:
+    params:
+      - name: param-for-step-action
+        description: "this is a param that the step action needs."
+    steps:
+      - name: action-runner
+        ref:
+          name: step-action
+        params:
+          - name: step-action-param
+            value: $(params.param-for-step-action)
+```
+
+**Note:** If a `Step` declares `params` for an `inlined Step`, it will also lead to a validation error. This is because an `inlined Step` gets it's `params` from the `TaskRun`.
diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go
@@ -138,6 +138,10 @@ type Step struct {
 	// Contains the reference to an existing StepAction.
 	//+optional
 	Ref *Ref `json:"ref,omitempty"`
+	// Params declares parameters passed to this step action.
+	// +optional
+	// +listType=atomic
+	Params Params `json:"params,omitempty"`
 }
 
 // Ref can be used to refer to a specific instance of a StepAction.
diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go
diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json
@@ -1463,6 +1463,15 @@
           "description": "OnError defines the exiting behavior of a container on error can be set to [ continue | stopAndFail ]",
           "type": "string"
         },
+        "params": {
+          "description": "Params declares parameters passed to this step action.",
+          "type": "array",
+          "items": {
+            "default": {},
+            "$ref": "#/definitions/v1.Param"
+          },
+          "x-kubernetes-list-type": "atomic"
+        },
         "ref": {
           "description": "Contains the reference to an existing StepAction.",
           "$ref": "#/definitions/v1.Ref"
diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go
@@ -300,6 +300,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 			})
 		}
 	} else {
+		if len(s.Params) > 0 {
+			errs = errs.Also(&apis.FieldError{
+				Message: "params cannot be used without Ref",
+				Paths:   []string{"params"},
+			})
+		}
 		if s.Image == "" {
 			errs = errs.Also(apis.ErrMissingField("Image"))
 		}
diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go
@@ -534,6 +534,16 @@ func TestTaskSpecStepActionReferenceValidate(t *testing.T) {
 				Name: "stepAction",
 			},
 		}},
+	}, {
+		name: "valid use of params with Ref",
+		Steps: []v1.Step{{
+			Ref: &v1.Ref{
+				Name: "stepAction",
+			},
+			Params: v1.Params{{
+				Name: "param",
+			}},
+		}},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1490,6 +1500,19 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) {
 			Message: "env cannot be used with Ref",
 			Paths:   []string{"steps[0].env"},
 		},
+	}, {
+		name: "Cannot use params without Ref",
+		Steps: []v1.Step{{
+			Image: "my-image",
+			Params: v1.Params{{
+				Name: "param",
+			}},
+		}},
+		enableStepActions: true,
+		expectedError: apis.FieldError{
+			Message: "params cannot be used without Ref",
+			Paths:   []string{"steps[0].params"},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go
diff --git a/pkg/apis/pipeline/v1beta1/container_conversion.go b/pkg/apis/pipeline/v1beta1/container_conversion.go
@@ -65,6 +65,12 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) {
 		sink.Ref = &v1.Ref{}
 		s.Ref.convertTo(ctx, sink.Ref)
 	}
+	sink.Params = nil
+	for _, p := range s.Params {
+		new := v1.Param{}
+		p.convertTo(ctx, &new)
+		sink.Params = append(sink.Params, new)
+	}
 }
 
 func (s *Step) convertFrom(ctx context.Context, source v1.Step) {
@@ -97,6 +103,12 @@ func (s *Step) convertFrom(ctx context.Context, source v1.Step) {
 		newRef.convertFrom(ctx, *source.Ref)
 		s.Ref = &newRef
 	}
+	s.Params = nil
+	for _, p := range source.Params {
+		new := Param{}
+		new.ConvertFrom(ctx, p)
+		s.Params = append(s.Params, new)
+	}
 }
 
 func (s StepTemplate) convertTo(ctx context.Context, sink *v1.StepTemplate) {
diff --git a/pkg/apis/pipeline/v1beta1/container_types.go b/pkg/apis/pipeline/v1beta1/container_types.go
@@ -232,6 +232,10 @@ type Step struct {
 	// Contains the reference to an existing StepAction.
 	//+optional
 	Ref *Ref `json:"ref,omitempty"`
+	// Params declares parameters passed to this step action.
+	// +optional
+	// +listType=atomic
+	Params Params `json:"params,omitempty"`
 }
 
 // Ref can be used to refer to a specific instance of a StepAction.
diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go
diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json
@@ -2070,6 +2070,15 @@
           "description": "OnError defines the exiting behavior of a container on error can be set to [ continue | stopAndFail ]",
           "type": "string"
         },
+        "params": {
+          "description": "Params declares parameters passed to this step action.",
+          "type": "array",
+          "items": {
+            "default": {},
+            "$ref": "#/definitions/v1beta1.Param"
+          },
+          "x-kubernetes-list-type": "atomic"
+        },
         "ports": {
           "description": "List of ports to expose from the Step's container. Exposing a port here gives the system additional information about the network connections a container uses, but is primarily informational. Not specifying a port here DOES NOT prevent that port from being exposed. Any port which is listening on the default \"0.0.0.0\" address inside a container will be accessible from the network. Cannot be updated.\n\nDeprecated: This field will be removed in a future release.",
           "type": "array",
diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go
@@ -83,6 +83,9 @@ spec:
   steps:
     - ref:
         name: "step-action"
+      params:
+        - name: param1
+          value: hello
 `
 
 	remoteStepActionTaskYAML := `
diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go
@@ -306,6 +306,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 			})
 		}
 	} else {
+		if len(s.Params) > 0 {
+			errs = errs.Also(&apis.FieldError{
+				Message: "params cannot be used without Ref",
+				Paths:   []string{"params"},
+			})
+		}
 		if s.Image == "" {
 			errs = errs.Also(apis.ErrMissingField("Image"))
 		}
diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -538,6 +538,16 @@ func TestTaskSpecStepActionReferenceValidate(t *testing.T) {
 				Name: "stepAction",
 			},
 		}},
+	}, {
+		name: "valid use of params with Ref",
+		Steps: []v1beta1.Step{{
+			Ref: &v1beta1.Ref{
+				Name: "stepAction",
+			},
+			Params: v1beta1.Params{{
+				Name: "param",
+			}},
+		}},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1503,6 +1513,19 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) {
 			Message: "env cannot be used with Ref",
 			Paths:   []string{"steps[0].env"},
 		},
+	}, {
+		name: "Cannot use params without Ref",
+		Steps: []v1beta1.Step{{
+			Image: "my-image",
+			Params: v1beta1.Params{{
+				Name: "param",
+			}},
+		}},
+		enableStepActions: true,
+		expectedError: apis.FieldError{
+			Message: "params cannot be used without Ref",
+			Paths:   []string{"steps[0].params"},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go

Original file line number	Diff line number	Diff line change
`@@ -300,6 +300,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi`
`300`	`300`	`})`
`301`	`301`	`}`
`302`	`302`	`} else {`
	`303`	`+ if len(s.Params) > 0 {`
	`304`	`+ errs = errs.Also(&apis.FieldError{`
	`305`	`+ Message: "params cannot be used without Ref",`
	`306`	`+ Paths: []string{"params"},`
	`307`	`+ })`
	`308`	`+ }`
`303`	`309`	`if s.Image == "" {`
`304`	`310`	`errs = errs.Also(apis.ErrMissingField("Image"))`
`305`	`311`	`}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,12 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) {`
`65`	`65`	`sink.Ref = &v1.Ref{}`
`66`	`66`	`s.Ref.convertTo(ctx, sink.Ref)`
`67`	`67`	`}`
	`68`	`+ sink.Params = nil`
	`69`	`+ for _, p := range s.Params {`
	`70`	`+ new := v1.Param{}`
	`71`	`+ p.convertTo(ctx, &new)`
	`72`	`+ sink.Params = append(sink.Params, new)`
	`73`	`+ }`
`68`	`74`	`}`
`69`	`75`
`70`	`76`	`func (s *Step) convertFrom(ctx context.Context, source v1.Step) {`
`@@ -97,6 +103,12 @@ func (s *Step) convertFrom(ctx context.Context, source v1.Step) {`
`97`	`103`	`newRef.convertFrom(ctx, *source.Ref)`
`98`	`104`	`s.Ref = &newRef`
`99`	`105`	`}`
	`106`	`+ s.Params = nil`
	`107`	`+ for _, p := range source.Params {`
	`108`	`+ new := Param{}`
	`109`	`+ new.ConvertFrom(ctx, p)`
	`110`	`+ s.Params = append(s.Params, new)`
	`111`	`+ }`
`100`	`112`	`}`
`101`	`113`
`102`	`114`	`func (s StepTemplate) convertTo(ctx context.Context, sink *v1.StepTemplate) {`