From 699b93cabe7be6ec7909f9e520113cfcd0d005b4 Mon Sep 17 00:00:00 2001
From: Yongxuan Zhang <yongxuanzhang@google.com>
Date: Tue, 7 Nov 2023 21:49:11 +0000
Subject: [PATCH] [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 yongxuanzhang@google.com
---
docs/pipeline-api.md | 30 +++
docs/stepactions.md | 26 +++
go.mod | 2 +-
pkg/apis/pipeline/v1/param_types.go | 2 +-
pkg/apis/pipeline/v1/task_validation.go | 6 +
pkg/apis/pipeline/v1/task_validation_test.go | 19 +-
.../pipeline/v1alpha1/openapi_generated.go | 23 +-
.../pipeline/v1alpha1/stepaction_types.go | 7 +
.../v1alpha1/stepaction_validation.go | 33 +++
.../v1alpha1/stepaction_validation_test.go | 211 +++++++++++++++---
pkg/apis/pipeline/v1alpha1/swagger.json | 11 +
.../v1alpha1/zz_generated.deepcopy.go | 7 +
pkg/apis/pipeline/v1beta1/param_types.go | 2 +-
pkg/apis/pipeline/v1beta1/task_validation.go | 6 +
.../pipeline/v1beta1/task_validation_test.go | 18 +-
pkg/reconciler/taskrun/resources/taskspec.go | 3 +
.../taskrun/resources/taskspec_test.go | 8 +
pkg/substitution/substitution.go | 18 +-
pkg/substitution/substitution_test.go | 42 +++-
19 files changed, 436 insertions(+), 38 deletions(-)
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
index fd0e5b73880..fdb95ffb338 100644
--- a/docs/pipeline-api.md
+++ b/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’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’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
index 85ac1d5e51f..978ce2cb196 100644
--- a/docs/stepactions.md
+++ b/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.
diff --git a/go.mod b/go.mod
index 6ce453422a9..06965015b16 100644
--- a/go.mod
+++ b/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
diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go
index 9a964641bc0..c5973544221 100644
--- 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
index 4f3ba830c75..7fb4c70be9d 100644
--- 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
index 16554c7e36f..5bc80e018fc 100644
--- 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
index 48b42c1663f..cb18896855e 100644
--- a/pkg/apis/pipeline/v1alpha1/openapi_generated.go
+++ b/pkg/apis/pipeline/v1alpha1/openapi_generated.go
@@ -927,11 +927,32 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref common.ReferenceCallba
Ref: ref("k8s.io/api/core/v1.SecurityContext"),
},
},
+ "volumeMounts": {
+ VendorExtensible: spec.VendorExtensible{
+ Extensions: spec.Extensions{
+ "x-kubernetes-list-type": "atomic",
+ "x-kubernetes-patch-merge-key": "mountPath",
+ "x-kubernetes-patch-strategy": "merge",
+ },
+ },
+ SchemaProps: spec.SchemaProps{
+ Description: "Volumes to mount into the Step's filesystem. Cannot be updated.",
+ Type: []string{"array"},
+ Items: &spec.SchemaOrArray{
+ Schema: &spec.Schema{
+ SchemaProps: spec.SchemaProps{
+ Default: map[string]interface{}{},
+ Ref: ref("k8s.io/api/core/v1.VolumeMount"),
+ },
+ },
+ },
+ },
+ },
},
},
},
Dependencies: []string{
- "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.SecurityContext"},
+ "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeMount"},
}
}
diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_types.go b/pkg/apis/pipeline/v1alpha1/stepaction_types.go
index 4e9efb77714..97752a2b421 100644
--- a/pkg/apis/pipeline/v1alpha1/stepaction_types.go
+++ b/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
diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go
index 47674cfc62f..f170410674a 100644
--- 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
}
diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go
index 9e371c1edba..2288ea4fdae 100644
--- a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go
+++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go
@@ -61,13 +61,14 @@ func TestStepActionValidate(t *testing.T) {
func TestStepActionSpecValidate(t *testing.T) {
type fields struct {
- Image string
- Command []string
- Args []string
- Script string
- Env []corev1.EnvVar
- Params []v1.ParamSpec
- Results []v1alpha1.StepActionResult
+ Image string
+ Command []string
+ Args []string
+ Script string
+ Env []corev1.EnvVar
+ Params []v1.ParamSpec
+ Results []v1alpha1.StepActionResult
+ VolumeMounts []corev1.VolumeMount
}
tests := []struct {
name string
@@ -250,17 +251,47 @@ func TestStepActionSpecValidate(t *testing.T) {
},
}},
},
+ }, {
+ name: "valid volumeMounts",
+ fields: fields{
+ Image: "my-image",
+ Args: []string{"arg"},
+ Params: []v1.ParamSpec{{
+ Name: "foo",
+ }, {
+ Name: "array-params",
+ Type: v1.ParamTypeArray,
+ }, {
+ Name: "object-params",
+ Type: v1.ParamTypeObject,
+ Properties: map[string]v1.PropertySpec{
+ "key": {Type: "string"},
+ },
+ },
+ },
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)",
+ MountPath: "/config",
+ }, {
+ Name: "$(params.array-params[0])",
+ MountPath: "/config",
+ }, {
+ Name: "$(params.object-params.key)",
+ MountPath: "/config",
+ }},
+ },
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sa := &v1alpha1.StepActionSpec{
- Image: tt.fields.Image,
- Command: tt.fields.Command,
- Args: tt.fields.Args,
- Script: tt.fields.Script,
- Env: tt.fields.Env,
- Params: tt.fields.Params,
- Results: tt.fields.Results,
+ Image: tt.fields.Image,
+ Command: tt.fields.Command,
+ Args: tt.fields.Args,
+ Script: tt.fields.Script,
+ Env: tt.fields.Env,
+ Params: tt.fields.Params,
+ Results: tt.fields.Results,
+ VolumeMounts: tt.fields.VolumeMounts,
}
ctx := context.Background()
sa.SetDefaults(ctx)
@@ -273,13 +304,14 @@ func TestStepActionSpecValidate(t *testing.T) {
func TestStepActionValidateError(t *testing.T) {
type fields struct {
- Image string
- Command []string
- Args []string
- Script string
- Env []corev1.EnvVar
- Params []v1.ParamSpec
- Results []v1alpha1.StepActionResult
+ Image string
+ Command []string
+ Args []string
+ Script string
+ Env []corev1.EnvVar
+ Params []v1.ParamSpec
+ Results []v1alpha1.StepActionResult
+ VolumeMounts []corev1.VolumeMount
}
tests := []struct {
name string
@@ -399,19 +431,142 @@ func TestStepActionValidateError(t *testing.T) {
Message: `non-existent variable in "$(params.foo) && $(params.inexistent)"`,
Paths: []string{"spec.args[0]"},
},
+ }, {
+ name: "invalid param reference in volumeMount.Name - not a param reference",
+ fields: fields{
+ Image: "myimage",
+ Params: []v1.ParamSpec{{
+ Name: "foo",
+ }},
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "params.foo",
+ MountPath: "/path",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `invalid value: params.foo`,
+ Paths: []string{"spec.volumeMounts[0].name"},
+ Details: `expect the Name to be a single param reference`,
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - nested reference",
+ fields: fields{
+ Image: "myimage",
+ Params: []v1.ParamSpec{{
+ Name: "foo",
+ }},
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)-foo",
+ MountPath: "/path",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `invalid value: $(params.foo)-foo`,
+ Paths: []string{"spec.volumeMounts[0].name"},
+ Details: `expect the Name to be a single param reference`,
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - multiple params references",
+ fields: fields{
+ Image: "myimage",
+ Params: []v1.ParamSpec{{
+ Name: "foo",
+ }, {
+ Name: "bar",
+ }},
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)$(params.bar)",
+ MountPath: "/path",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `invalid value: $(params.foo)$(params.bar)`,
+ Paths: []string{"spec.volumeMounts[0].name"},
+ Details: `expect the Name to be a single param reference`,
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - not defined in params",
+ fields: fields{
+ Image: "myimage",
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)",
+ MountPath: "/path",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `non-existent variable in "$(params.foo)"`,
+ Paths: []string{"spec.volumeMounts[0]"},
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - array used in a volumeMounts name field",
+ fields: fields{
+ Params: []v1.ParamSpec{{
+ Name: "gitrepo",
+ Type: v1.ParamTypeArray,
+ }},
+ Image: "image",
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.gitrepo)",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `variable type invalid in "$(params.gitrepo)"`,
+ Paths: []string{"spec.volumeMounts[0]"},
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - object used in a volumeMounts name field",
+ fields: fields{
+ Params: []v1.ParamSpec{{
+ Name: "gitrepo",
+ Type: v1.ParamTypeObject,
+ Properties: map[string]v1.PropertySpec{
+ "url": {},
+ "commit": {},
+ },
+ }},
+ Image: "image",
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.gitrepo)",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `variable type invalid in "$(params.gitrepo)"`,
+ Paths: []string{"spec.volumeMounts[0]"},
+ },
+ }, {
+ name: "invalid param reference in volumeMount.Name - object key not existent in params",
+ fields: fields{
+ Params: []v1.ParamSpec{{
+ Name: "gitrepo",
+ Type: v1.ParamTypeObject,
+ Properties: map[string]v1.PropertySpec{
+ "url": {},
+ "commit": {},
+ },
+ }},
+ Image: "image",
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.gitrepo.foo)",
+ }},
+ },
+ expectedError: apis.FieldError{
+ Message: `non-existent variable in "$(params.gitrepo.foo)"`,
+ Paths: []string{"spec.volumeMounts[0]"},
+ },
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sa := &v1alpha1.StepAction{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: v1alpha1.StepActionSpec{
- Image: tt.fields.Image,
- Command: tt.fields.Command,
- Args: tt.fields.Args,
- Script: tt.fields.Script,
- Env: tt.fields.Env,
- Params: tt.fields.Params,
- Results: tt.fields.Results,
+ Image: tt.fields.Image,
+ Command: tt.fields.Command,
+ Args: tt.fields.Args,
+ Script: tt.fields.Script,
+ Env: tt.fields.Env,
+ Params: tt.fields.Params,
+ Results: tt.fields.Results,
+ VolumeMounts: tt.fields.VolumeMounts,
},
}
ctx := context.Background()
diff --git a/pkg/apis/pipeline/v1alpha1/swagger.json b/pkg/apis/pipeline/v1alpha1/swagger.json
index aa6a859ecdc..eea6d5d9a67 100644
--- a/pkg/apis/pipeline/v1alpha1/swagger.json
+++ b/pkg/apis/pipeline/v1alpha1/swagger.json
@@ -476,6 +476,17 @@
"securityContext": {
"description": "SecurityContext defines the security options the Step should be run with. If set, the fields of SecurityContext override the equivalent fields of PodSecurityContext. More info: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ The value set in StepAction will take precedence over the value from Task.",
"$ref": "#/definitions/v1.SecurityContext"
+ },
+ "volumeMounts": {
+ "description": "Volumes to mount into the Step's filesystem. Cannot be updated.",
+ "type": "array",
+ "items": {
+ "default": {},
+ "$ref": "#/definitions/v1.VolumeMount"
+ },
+ "x-kubernetes-list-type": "atomic",
+ "x-kubernetes-patch-merge-key": "mountPath",
+ "x-kubernetes-patch-strategy": "merge"
}
}
},
diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
index 592f5961a91..4e02d12fe6d 100644
--- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
@@ -340,6 +340,13 @@ func (in *StepActionSpec) DeepCopyInto(out *StepActionSpec) {
*out = new(v1.SecurityContext)
(*in).DeepCopyInto(*out)
}
+ if in.VolumeMounts != nil {
+ in, out := &in.VolumeMounts, &out.VolumeMounts
+ *out = make([]v1.VolumeMount, len(*in))
+ for i := range *in {
+ (*in)[i].DeepCopyInto(&(*out)[i])
+ }
+ }
return
}
diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go
index 689770d663e..f0634f8c580 100644
--- a/pkg/apis/pipeline/v1beta1/param_types.go
+++ b/pkg/apis/pipeline/v1beta1/param_types.go
@@ -283,7 +283,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/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go
index e623acbe115..127d20cd788 100644
--- a/pkg/apis/pipeline/v1beta1/task_validation.go
+++ b/pkg/apis/pipeline/v1beta1/task_validation.go
@@ -305,6 +305,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/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
index 04688954224..936b32ca2bb 100644
--- a/pkg/apis/pipeline/v1beta1/task_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -1526,7 +1526,23 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) {
Message: "params cannot be used without Ref",
Paths: []string{"steps[0].params"},
},
- }}
+ }, {
+ name: "Cannot use volumeMounts with Ref",
+ Steps: []v1beta1.Step{{
+ Ref: &v1beta1.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 := v1beta1.TaskSpec{
diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go
index ac6c9845f92..cf24b19b977 100644
--- a/pkg/reconciler/taskrun/resources/taskspec.go
+++ b/pkg/reconciler/taskrun/resources/taskspec.go
@@ -128,6 +128,9 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T
if stepActionSpec.Env != nil {
s.Env = stepActionSpec.Env
}
+ if len(stepActionSpec.VolumeMounts) > 0 {
+ s.VolumeMounts = stepActionSpec.VolumeMounts
+ }
if err := validateStepHasStepActionParameters(s.Params, stepActionSpec.Params); err != nil {
return nil, err
}
diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go
index 95b82a8b377..33e11ae9e9b 100644
--- a/pkg/reconciler/taskrun/resources/taskspec_test.go
+++ b/pkg/reconciler/taskrun/resources/taskspec_test.go
@@ -336,6 +336,10 @@ func TestGetStepActionsData(t *testing.T) {
Image: "myimage",
Command: []string{"ls"},
Args: []string{"-lh"},
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)",
+ MountPath: "/path",
+ }},
},
},
want: []v1.Step{{
@@ -344,6 +348,10 @@ func TestGetStepActionsData(t *testing.T) {
Args: []string{"-lh"},
WorkingDir: "/bar",
Timeout: &metav1.Duration{Duration: time.Hour},
+ VolumeMounts: []corev1.VolumeMount{{
+ Name: "$(params.foo)",
+ MountPath: "/path",
+ }},
}},
}, {
name: "step-action-with-script",
diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go
index 69b2c0d827d..8e1acab2fe1 100644
--- a/pkg/substitution/substitution.go
+++ b/pkg/substitution/substitution.go
@@ -334,11 +334,25 @@ func TrimArrayIndex(s string) string {
return arrayIndexingRegex.ReplaceAllString(s, "")
}
-// ExtractParamsExpressions will find all `$(params.paramName[int])` expressions
-func ExtractParamsExpressions(s string) []string {
+// ExtractArrayIndexingParamsExpressions will find all `$(params.paramName[int])` expressions
+func ExtractArrayIndexingParamsExpressions(s string) []string {
return paramIndexingRegex.FindAllString(s, -1)
}
+func ExtractVariableExpressions(s, prefix string) ([]string, error) {
+ pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, parameterSubstitution, parameterSubstitution)
+ re, err := regexp.Compile(pattern)
+ if err != nil {
+ return nil, fmt.Errorf("failed to parse regex pattern: %w", err)
+ }
+
+ matches := re.FindAllString(s, -1)
+ if len(matches) == 0 {
+ return []string{}, nil
+ }
+ return matches, nil
+}
+
// ExtractIndexString will find the leftmost match of `[int]`
func ExtractIndexString(s string) string {
return intIndexRegex.FindString(s)
diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go
index ff8c236f13f..c265510d195 100644
--- a/pkg/substitution/substitution_test.go
+++ b/pkg/substitution/substitution_test.go
@@ -542,7 +542,7 @@ func TestApplyArrayReplacements(t *testing.T) {
}
}
-func TestExtractParamsExpressions(t *testing.T) {
+func TestExtractArrayParamsExpressionsExpressions(t *testing.T) {
tests := []struct {
name string
input string
@@ -567,7 +567,45 @@ func TestExtractParamsExpressions(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- got := substitution.ExtractParamsExpressions(tt.input)
+ got := substitution.ExtractArrayIndexingParamsExpressions(tt.input)
+ if d := cmp.Diff(tt.want, got); d != "" {
+ t.Error(diff.PrintWantGot(d))
+ }
+ })
+ }
+}
+
+func TestExtractVariableExpressions(t *testing.T) {
+ tests := []struct {
+ name string
+ input string
+ prefix string
+ want []string
+ }{{
+ name: "normal string",
+ input: "hello world",
+ prefix: "params",
+ want: []string{},
+ }, {
+ name: "param reference",
+ input: "$(params.paramName)",
+ prefix: "params",
+ want: []string{"$(params.paramName)"},
+ }, {
+ name: "param star reference",
+ input: "$(params.paramName[*])",
+ prefix: "params",
+ want: []string{"$(params.paramName[*])"},
+ }, {
+ name: "param index reference",
+ input: "$(params.paramName[1])",
+ prefix: "params",
+ want: []string{"$(params.paramName[1])"},
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got, _ := substitution.ExtractVariableExpressions(tt.input, tt.prefix)
if d := cmp.Diff(tt.want, got); d != "" {
t.Error(diff.PrintWantGot(d))
}