[TEP-0142] Add VolumeMounts to StepAction

This commit adds VolumeMounts to StepAction, the VolumeMount.Name should use string param reference, and cannot be set both with Step from Task/TaskRun. Signed-off-by: Yongxuan Zhang [email protected]
tektoncd · Nov 8, 2023 · d6f8600 · d6f8600
1 parent a106110
commit d6f8600
Show file tree

Hide file tree

Showing 18 changed files with 435 additions and 37 deletions.
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -6612,6 +6612,21 @@ Params must be supplied as inputs in Steps unless they declare a defaultvalue.</
 <p>Results are values that this StepAction can output</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>volumeMounts</code><br/>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#volumemount-v1-core">
+[]Kubernetes core/v1.VolumeMount
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Volumes to mount into the Step&rsquo;s filesystem.
+Cannot be updated.</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -7471,6 +7486,21 @@ Params must be supplied as inputs in Steps unless they declare a defaultvalue.</
 <p>Results are values that this StepAction can output</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>volumeMounts</code><br/>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#volumemount-v1-core">
+[]Kubernetes core/v1.VolumeMount
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Volumes to mount into the Step&rsquo;s filesystem.
+Cannot be updated.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="tekton.dev/v1alpha1.VerificationPolicySpec">VerificationPolicySpec

diff --git a/docs/stepactions.md b/docs/stepactions.md
@@ -40,6 +40,7 @@ A `StepAction` definition supports the following fields:
   - `env`
   - [`params`](#declaring-params)
   - [`results`](#declaring-results)
+  - [`volumeMounts`](#declaring-volumemounts)
 
 The non-functional example below demonstrates the use of most of the above-mentioned fields:
 
@@ -113,6 +114,30 @@ spec:
     date | tee $(results.current-date-human-readable.path)
 ```
 
+### Declaring VolumeMounts
+
+You can define `VolumeMounts` in `StepAction`, note that the `name` of the `VolumeMount` MUST be a **single param reference** to a string param ( for example, `$(params.registryConfig)` is valid but `$(params.registryConfig)-foo` or `"unparametrized-name"` is invalid).
+
+```yaml
+apiVersion: tekton.dev/v1alpha1
+kind: StepAction
+metadata:
+  name: myStep
+spec:
+  params:
+    - name: registryConfig
+    - name: otherConfig
+  volumeMounts:
+    - name: $(params.registryConfig)
+      mountPath: /registry-config
+  volumeMounts:
+    - name: $(params.otherConfig)
+      mountPath: /other-config
+  image: ...
+  script: ...
+```
+
+
 ## Referencing a StepAction
 
 `StepActions` can be referenced from the `Step` using the `ref` field, as follows:
@@ -170,6 +195,7 @@ If a `Step` is referencing a `StepAction`, it cannot contain the fields supporte
 - `args`
 - `script`
 - `env`
+- `volumeMounts`
 
 Using any of the above fields and referencing a `StepAction` in the same `Step` is not allowed and will cause an validation error.
 

diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go
@@ -329,7 +329,7 @@ func (ps ParamSpecs) ExtractDefaultParamArrayLengths() map[string]int {
 // it would return ["$(params.array-param[1])", "$(params.other-array-param[2])"].
 func extractArrayIndexingParamRefs(paramReference string) []string {
 	l := []string{}
-	list := substitution.ExtractParamsExpressions(paramReference)
+	list := substitution.ExtractArrayIndexingParamsExpressions(paramReference)
 	for _, val := range list {
 		indexString := substitution.ExtractIndexString(val)
 		if indexString != "" {

diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go
@@ -299,6 +299,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 				Paths:   []string{"env"},
 			})
 		}
+		if len(s.VolumeMounts) > 0 {
+			errs = errs.Also(&apis.FieldError{
+				Message: "volumeMounts cannot be used with Ref",
+				Paths:   []string{"volumeMounts"},
+			})
+		}
 	} else {
 		if len(s.Params) > 0 {
 			errs = errs.Also(&apis.FieldError{

diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go
@@ -1513,7 +1513,24 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) {
 			Message: "params cannot be used without Ref",
 			Paths:   []string{"steps[0].params"},
 		},
-	}}
+	}, {
+		name: "Cannot use volumeMounts with Ref",
+		Steps: []v1.Step{{
+			Ref: &v1.Ref{
+				Name: "stepAction",
+			},
+			VolumeMounts: []corev1.VolumeMount{{
+				Name:      "$(params.foo)",
+				MountPath: "/registry-config",
+			}},
+		}},
+		enableStepActions: true,
+		expectedError: apis.FieldError{
+			Message: "volumeMounts cannot be used with Ref",
+			Paths:   []string{"steps[0].volumeMounts"},
+		},
+	},
+	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ts := v1.TaskSpec{

diff --git a/pkg/apis/pipeline/v1alpha1/openapi_generated.go b/pkg/apis/pipeline/v1alpha1/openapi_generated.go
diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_types.go b/pkg/apis/pipeline/v1alpha1/stepaction_types.go
@@ -121,6 +121,13 @@ type StepActionSpec struct {
 	// +optional
 	// +listType=atomic
 	Results []StepActionResult `json:"results,omitempty"`
+	// Volumes to mount into the Step's filesystem.
+	// Cannot be updated.
+	// +optional
+	// +patchMergeKey=mountPath
+	// +patchStrategy=merge
+	// +listType=atomic
+	VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty" patchStrategy:"merge" patchMergeKey:"mountPath" protobuf:"bytes,9,rep,name=volumeMounts"`
 }
 
 // StepActionObject is implemented by StepAction

diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go
@@ -23,6 +23,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/webhook/resourcesemantics"
@@ -67,6 +68,7 @@ func (ss *StepActionSpec) Validate(ctx context.Context) (errs *apis.FieldError)
 	errs = errs.Also(validateParameterVariables(ctx, *ss, ss.Params))
 	errs = errs.Also(validateStepActionResultsVariables(ctx, *ss))
 	errs = errs.Also(validateResults(ctx, ss.Results).ViaField("results"))
+	errs = errs.Also(validateVolumeMounts(ss.VolumeMounts, ss.Params).ViaField("volumeMounts"))
 	return errs
 }
 
@@ -89,6 +91,28 @@ func validateResults(ctx context.Context, results []StepActionResult) (errs *api
 	return errs
 }
 
+func validateVolumeMounts(volumeMounts []corev1.VolumeMount, params v1.ParamSpecs) (errs *apis.FieldError) {
+	if len(volumeMounts) == 0 {
+		return
+	}
+	paramNames := sets.String{}
+	for _, p := range params {
+		paramNames.Insert(p.Name)
+	}
+	for idx, v := range volumeMounts {
+		matches, _ := substitution.ExtractVariableExpressions(v.Name, "params")
+		if len(matches) != 1 {
+			errs = errs.Also(apis.ErrInvalidValue(v.Name, "name", "expect the Name to be a single param reference").ViaIndex(idx))
+			return errs
+		} else if matches[0] != v.Name {
+			errs = errs.Also(apis.ErrInvalidValue(v.Name, "name", "expect the Name to be a single param reference").ViaIndex(idx))
+			return errs
+		}
+		errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(v.Name, "params", paramNames).ViaIndex(idx))
+	}
+	return errs
+}
+
 // validateParameterVariables validates all variables within a slice of ParamSpecs against a StepAction
 func validateParameterVariables(ctx context.Context, sas StepActionSpec, params v1.ParamSpecs) *apis.FieldError {
 	var errs *apis.FieldError
@@ -133,6 +157,9 @@ func validateStepActionObjectUsageAsWhole(sas StepActionSpec, prefix string, var
 	for _, env := range sas.Env {
 		errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(env.Value, prefix, vars).ViaFieldKey("env", env.Name))
 	}
+	for i, vm := range sas.VolumeMounts {
+		errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(vm.Name, prefix, vars).ViaFieldIndex("volumeMounts", i))
+	}
 	return errs
 }
 
@@ -149,6 +176,9 @@ func validateStepActionArrayUsage(sas StepActionSpec, prefix string, arrayParamN
 	for _, env := range sas.Env {
 		errs = errs.Also(substitution.ValidateNoReferencesToProhibitedVariables(env.Value, prefix, arrayParamNames).ViaFieldKey("env", env.Name))
 	}
+	for i, vm := range sas.VolumeMounts {
+		errs = errs.Also(substitution.ValidateNoReferencesToProhibitedVariables(vm.Name, prefix, arrayParamNames).ViaFieldIndex("volumeMounts", i))
+	}
 	return errs
 }
 
@@ -165,6 +195,9 @@ func validateStepActionVariables(ctx context.Context, sas StepActionSpec, prefix
 	for _, env := range sas.Env {
 		errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(env.Value, prefix, vars).ViaFieldKey("env", env.Name))
 	}
+	for i, vm := range sas.VolumeMounts {
+		errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(vm.Name, prefix, vars).ViaFieldIndex("volumeMounts", i))
+	}
 	return errs
 }