[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]

docs/pipeline-api.md

@@ -6629,6 +6629,21 @@ More info: <a href="https://kubernetes.io/docs/tasks/configure-pod-container/sec
 The value set in StepAction will take precedence over the value from Task.</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>
@@ -7505,6 +7520,21 @@ More info: <a href="https://kubernetes.io/docs/tasks/configure-pod-container/sec
 The value set in StepAction will take precedence over the value from Task.</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

docs/stepactions.md

@@ -41,6 +41,7 @@ A `StepAction` definition supports the following fields:
   - [`params`](#declaring-params)
   - [`results`](#declaring-results)
   - [`securityContext`](#declaring-securitycontext)
+  - [`volumeMounts`](#declaring-volumemounts)
 
 The non-functional example below demonstrates the use of most of the above-mentioned fields:
 
@@ -134,6 +135,30 @@ spec:
 
 Note that the `securityContext` from `StepAction` will overwrite the `securityContext` from [`TaskRun`](./taskruns.md/#example-of-running-step-containers-as-a-non-root-user).
 
+### 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). This is to avoid the case where `VolumeMount` from 2 different `StepActions` using the same mounted volume but different names. In this case the Task Author doesn't need to provide 2 separate volume devices.
+
+```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:
@@ -191,6 +216,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.
 

go.mod

@@ -51,6 +51,7 @@ require (
 	github.com/goccy/kpoward v0.1.0
 	github.com/google/cel-go v0.18.1
 	github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20230625233257-b8504803389b
+	github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20230516205744-dbecb1de8cfa
 	github.com/sigstore/sigstore/pkg/signature/kms/aws v1.7.5
 	github.com/sigstore/sigstore/pkg/signature/kms/azure v1.7.5
 	github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.7.5
@@ -102,7 +103,6 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/golang-jwt/jwt/v5 v5.0.0 // indirect
 	github.com/google/gnostic v0.6.9 // indirect
-	github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20230516205744-dbecb1de8cfa // indirect
 	github.com/google/s2a-go v0.1.7 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.1 // indirect
 	github.com/googleapis/gax-go/v2 v2.12.0 // indirect

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 != "" {

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{

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{

pkg/apis/pipeline/v1alpha1/stepaction_types.go

@@ -127,6 +127,13 @@ type StepActionSpec struct {
 	// The value set in StepAction will take precedence over the value from Task.
 	// +optional
 	SecurityContext *corev1.SecurityContext `json:"securityContext,omitempty" protobuf:"bytes,15,opt,name=securityContext"`
+	// 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

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
 }