From 47343d4e5fd4f082028f028a9e278495b4980c34 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Thu, 16 Feb 2023 02:58:00 +0000 Subject: [PATCH] change param array indexing validation functions to member functions This commit refactors param array indexing validation to extract all references first, then validate if references are out of bound. Previously we traverse all references and collect invalid references there. The benefit of this refactoring is that it would be convenient for other validation to reuse these references. This commit also moves validation functions into api and change them to member functions. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- docs/pipeline-api.md | 102 +++-- go.mod | 24 +- pkg/apis/pipeline/v1/param_types.go | 214 ++++++++++ pkg/apis/pipeline/v1/pipeline_types.go | 8 +- pkg/apis/pipeline/v1/pipeline_validation.go | 48 +++ .../pipeline/v1/pipeline_validation_test.go | 383 +++++++++++++++++ pkg/apis/pipeline/v1/task_types.go | 2 +- pkg/apis/pipeline/v1/task_validation.go | 36 ++ pkg/apis/pipeline/v1/task_validation_test.go | 196 +++++++++ pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 54 ++- pkg/apis/pipeline/v1beta1/param_types.go | 214 ++++++++++ pkg/apis/pipeline/v1beta1/pipeline_types.go | 8 +- .../pipeline/v1beta1/pipeline_validation.go | 48 +++ .../v1beta1/pipeline_validation_test.go | 383 +++++++++++++++++ pkg/apis/pipeline/v1beta1/task_types.go | 2 +- pkg/apis/pipeline/v1beta1/task_validation.go | 36 ++ .../pipeline/v1beta1/task_validation_test.go | 196 +++++++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 54 ++- pkg/reconciler/pipelinerun/pipelinerun.go | 2 +- .../pipelinerun/resources/validate_params.go | 113 +---- .../resources/validate_params_test.go | 403 +----------------- pkg/reconciler/taskrun/taskrun.go | 2 +- pkg/reconciler/taskrun/validate_resources.go | 202 +-------- .../taskrun/validate_resources_test.go | 203 +-------- 24 files changed, 1947 insertions(+), 986 deletions(-) diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 4d9a1c5797e..6bbb60e8b6c 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -561,8 +561,8 @@ used to populate a UI.

params
- -[]ParamSpec + +ParamSpecs @@ -881,8 +881,8 @@ TaskSpec params
- -[]ParamSpec + +ParamSpecs @@ -1488,8 +1488,8 @@ TaskSpec params
- -[]Param + +Params @@ -1549,8 +1549,8 @@ string params
- -[]Param + +Params @@ -1587,7 +1587,7 @@ The names of the params must match the names of the paramsParam

-(Appears on:Matrix, MatrixInclude, PipelineRunSpec, PipelineTask, ResolverRef, TaskRunInputs, TaskRunSpec) +(Appears on:PipelineRunSpec, ResolverRef, TaskRunInputs, TaskRunSpec)

Param declares an ParamValues to use for the parameter called name.

@@ -1626,9 +1626,6 @@ ParamValue

ParamSpec

-

-(Appears on:PipelineSpec, TaskSpec) -

ParamSpec defines arbitrary parameters needed beyond typed inputs (such as resources). Parameter values are provided by users as inputs on a TaskRun @@ -1713,6 +1710,14 @@ parameter.

+

ParamSpecs +([]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec alias)

+

+(Appears on:PipelineSpec, TaskSpec) +

+
+

ParamSpecs is a list of ParamSpec

+

ParamType (string alias)

@@ -1798,6 +1803,14 @@ map[string]string +

Params +([]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Param alias)

+

+(Appears on:Matrix, MatrixInclude, PipelineTask) +

+
+

Params is a list of Param

+

PipelineRef

@@ -2489,8 +2502,8 @@ used to populate a UI.

params
- -[]ParamSpec + +ParamSpecs @@ -2646,8 +2659,8 @@ this Task executes. (Used to force a specific ordering in graph execution.)

params
- -[]Param + +Params @@ -5254,8 +5267,8 @@ Kubernetes core/v1.ResourceRequirements params
- -[]ParamSpec + +ParamSpecs @@ -7236,8 +7249,8 @@ PipelineResources.

params
- -[]ParamSpec + +ParamSpecs @@ -7650,8 +7663,8 @@ Pipeline’s tasks as inputs and outputs.

params
- -[]ParamSpec + +ParamSpecs @@ -8028,8 +8041,8 @@ PipelineResources.

params
- -[]ParamSpec + +ParamSpecs @@ -9046,8 +9059,8 @@ TaskSpec params
- -[]Param + +Params @@ -9107,8 +9120,8 @@ string params
- -[]Param + +Params @@ -9130,7 +9143,7 @@ The names of the params must match the names of the paramsParam

-(Appears on:RunSpec, CustomRunSpec, Matrix, MatrixInclude, PipelineRunSpec, PipelineTask, ResolverRef, TaskRunInputs, TaskRunSpec, ResolutionRequestSpec) +(Appears on:RunSpec, CustomRunSpec, PipelineRunSpec, ResolverRef, TaskRunInputs, TaskRunSpec, ResolutionRequestSpec)

Param declares an ParamValues to use for the parameter called name.

@@ -9169,9 +9182,6 @@ ParamValue

ParamSpec

-

-(Appears on:PipelineSpec, TaskSpec) -

ParamSpec defines arbitrary parameters needed beyond typed inputs (such as resources). Parameter values are provided by users as inputs on a TaskRun @@ -9256,6 +9266,14 @@ parameter.

+

ParamSpecs +([]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamSpec alias)

+

+(Appears on:PipelineSpec, TaskSpec) +

+
+

ParamSpecs is a list of ParamSpec

+

ParamType (string alias)

@@ -9326,6 +9344,14 @@ map[string]string +

Params +([]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param alias)

+

+(Appears on:Matrix, MatrixInclude, PipelineTask) +

+
+

Params is a list of Param

+

PipelineDeclaredResource

@@ -10279,8 +10305,8 @@ Pipeline’s tasks as inputs and outputs.

params
- -[]ParamSpec + +ParamSpecs @@ -10451,8 +10477,8 @@ outputs.

params
- -[]Param + +Params @@ -13722,8 +13748,8 @@ PipelineResources.

params
- -[]ParamSpec + +ParamSpecs diff --git a/go.mod b/go.mod index 1d822afca7c..c86b499bae5 100644 --- a/go.mod +++ b/go.mod @@ -41,19 +41,9 @@ require github.com/benbjohnson/clock v1.1.0 // indirect require ( code.gitea.io/sdk/gitea v0.15.1 - github.com/cenkalti/backoff/v3 v3.2.2 github.com/goccy/kpoward v0.1.0 github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20221030203717-1711cefd7eec - github.com/hashicorp/go-cleanhttp v0.5.2 - github.com/hashicorp/go-retryablehttp v0.7.1 - github.com/hashicorp/go-rootcerts v1.0.2 - github.com/hashicorp/go-secure-stdlib/parseutil v0.1.7 - github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 - github.com/hashicorp/go-sockaddr v1.0.2 - github.com/hashicorp/hcl v1.0.0 github.com/letsencrypt/boulder v0.0.0-20221109233200-85aa52084eaf - github.com/mitchellh/mapstructure v1.5.0 - github.com/ryanuber/go-glob v1.0.0 github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 go.opentelemetry.io/otel v1.14.0 go.opentelemetry.io/otel/exporters/jaeger v1.14.0 @@ -83,6 +73,7 @@ require ( github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect github.com/aws/aws-sdk-go-v2/service/kms v1.20.4 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.14.3 // indirect + github.com/cenkalti/backoff/v3 v3.2.2 // indirect github.com/cloudflare/circl v1.1.0 // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect github.com/fatih/color v1.13.0 // indirect @@ -92,12 +83,21 @@ require ( github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect github.com/googleapis/gax-go/v2 v2.7.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.11.3 // indirect + github.com/hashicorp/go-cleanhttp v0.5.2 // indirect + github.com/hashicorp/go-retryablehttp v0.7.1 // indirect + github.com/hashicorp/go-rootcerts v1.0.2 // indirect + github.com/hashicorp/go-secure-stdlib/parseutil v0.1.7 // indirect + github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect + github.com/hashicorp/go-sockaddr v1.0.2 // indirect + github.com/hashicorp/hcl v1.0.0 // indirect github.com/hashicorp/vault/api v1.9.0 // indirect github.com/jellydator/ttlcache/v2 v2.11.1 // indirect github.com/kr/pretty v0.3.0 // indirect + github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pjbgf/sha1cd v0.2.3 // indirect github.com/rogpeppe/go-internal v1.8.0 // indirect + github.com/ryanuber/go-glob v1.0.0 // indirect github.com/skeema/knownhosts v1.1.0 // indirect github.com/theupdateframework/go-tuf v0.5.2-0.20220930112810-3890c1e7ace4 // indirect github.com/zeebo/errs v1.3.0 // indirect @@ -193,12 +193,12 @@ require ( go.uber.org/multierr v1.8.0 // indirect golang.org/x/crypto v0.6.0 golang.org/x/mod v0.7.0 // indirect - golang.org/x/net v0.7.0 + golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.1.0 golang.org/x/sys v0.5.0 // indirect golang.org/x/term v0.5.0 // indirect golang.org/x/text v0.7.0 // indirect - golang.org/x/time v0.2.0 + golang.org/x/time v0.2.0 // indirect golang.org/x/tools v0.5.0 // indirect google.golang.org/api v0.110.0 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index add766d37cd..9a4666aeb29 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -25,6 +25,7 @@ import ( resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/substitution" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" ) @@ -62,6 +63,9 @@ type ParamSpec struct { Default *ParamValue `json:"default,omitempty"` } +// ParamSpecs is a list of ParamSpec +type ParamSpecs []ParamSpec + // PropertySpec defines the struct for object keys type PropertySpec struct { Type ParamType `json:"type,omitempty"` @@ -117,6 +121,216 @@ type Param struct { Value ParamValue `json:"value"` } +// extractParamValuesFromParams get all param values from params +func (ps Params) extractParamValuesFromParams() []string { + pvs := []string{} + for i := range ps { + pvs = append(pvs, ps[i].Value.StringVal) + pvs = append(pvs, ps[i].Value.ArrayVal...) + for _, v := range ps[i].Value.ObjectVal { + pvs = append(pvs, v) + } + } + return pvs +} + +// Params is a list of Param +type Params []Param + +// extractParamArrayLengths extract and return the lengths of all array params +// Example of returned value: {"a-array-params": 2,"b-array-params": 2 } +func (ps Params) extractParamArrayLengths() map[string]int { + // Collect all array params + arrayParamsLengths := make(map[string]int) + + // Collect array params lengths from params + for _, p := range ps { + if p.Value.Type == ParamTypeArray { + arrayParamsLengths[p.Name] = len(p.Value.ArrayVal) + } + } + return arrayParamsLengths +} + +// extractParamArrayLengths extract and return the lengths of all array params +// Example of returned value: {"a-array-params": 2,"b-array-params": 2 } +func (ps ParamSpecs) extractParamArrayLengths() map[string]int { + // Collect all array params + arrayParamsLengths := make(map[string]int) + + // Collect array params lengths from defaults + for _, p := range ps { + if p.Default != nil { + if p.Default.Type == ParamTypeArray { + arrayParamsLengths[p.Name] = len(p.Default.ArrayVal) + } + } + } + return arrayParamsLengths +} + +// validateOutofBoundArrayParams validates if the array indexing params are out of bound +// example of arrayIndexingParams: ["$(params.a-array-param[1])", "$(params.b-array-param[2])"] +// example of arrayParamsLengths: {"a-array-params": 2,"b-array-params": 2 } +func validateOutofBoundArrayParams(arrayIndexingParams []string, arrayParamsLengths map[string]int) error { + outofBoundParams := sets.String{} + for _, val := range arrayIndexingParams { + indexString := substitution.ExtractIndexString(val) + idx, _ := substitution.ExtractIndex(indexString) + // this will extract the param name from reference + // e.g. $(params.a-array-param[1]) -> a-array-param + paramName, _, _ := substitution.ExtractVariablesFromString(substitution.TrimArrayIndex(val), "params") + + if paramLength, ok := arrayParamsLengths[paramName[0]]; ok { + if idx >= paramLength { + outofBoundParams.Insert(val) + } + } + } + if outofBoundParams.Len() > 0 { + return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) + } + return nil +} + +// extractArrayIndexingParamRefs takes a string of the form `foo-$(params.array-param[1])-bar` and extracts the portions of the string that reference an element in an array param. +// For example, for the string “foo-$(params.array-param[1])-bar-$(params.other-array-param[2])-$(params.string-param)`, +// it would return ["$(params.array-param[1])", "$(params.other-array-param[2])"]. +func extractArrayIndexingParamRefs(paramReference string) []string { + l := []string{} + list := substitution.ExtractParamsExpressions(paramReference) + for _, val := range list { + indexString := substitution.ExtractIndexString(val) + if indexString != "" { + l = append(l, val) + } + } + return l +} + +// extractParamRefsFromSteps get all array indexing references from steps +func extractParamRefsFromSteps(steps []Step) []string { + paramsRefs := []string{} + for _, step := range steps { + paramsRefs = append(paramsRefs, step.Script) + container := step.ToK8sContainer() + paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...) + } + return paramsRefs +} + +// extractParamRefsFromStepTemplate get all array indexing references from StepsTemplate +func extractParamRefsFromStepTemplate(stepTemplate *StepTemplate) []string { + if stepTemplate == nil { + return nil + } + container := stepTemplate.ToK8sContainer() + return extractParamRefsFromContainer(container) +} + +// extractParamRefsFromSidecars get all array indexing references from sidecars +func extractParamRefsFromSidecars(sidecars []Sidecar) []string { + paramsRefs := []string{} + for _, s := range sidecars { + paramsRefs = append(paramsRefs, s.Script) + container := s.ToK8sContainer() + paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...) + } + return paramsRefs +} + +// extractParamRefsFromVolumes get all array indexing references from volumes +func extractParamRefsFromVolumes(volumes []corev1.Volume) []string { + paramsRefs := []string{} + for i, v := range volumes { + paramsRefs = append(paramsRefs, v.Name) + if v.VolumeSource.ConfigMap != nil { + paramsRefs = append(paramsRefs, v.ConfigMap.Name) + for _, item := range v.ConfigMap.Items { + paramsRefs = append(paramsRefs, item.Key) + paramsRefs = append(paramsRefs, item.Path) + } + } + if v.VolumeSource.Secret != nil { + paramsRefs = append(paramsRefs, v.Secret.SecretName) + for _, item := range v.Secret.Items { + paramsRefs = append(paramsRefs, item.Key) + paramsRefs = append(paramsRefs, item.Path) + } + } + if v.PersistentVolumeClaim != nil { + paramsRefs = append(paramsRefs, v.PersistentVolumeClaim.ClaimName) + } + if v.Projected != nil { + for _, s := range volumes[i].Projected.Sources { + if s.ConfigMap != nil { + paramsRefs = append(paramsRefs, s.ConfigMap.Name) + } + if s.Secret != nil { + paramsRefs = append(paramsRefs, s.Secret.Name) + } + if s.ServiceAccountToken != nil { + paramsRefs = append(paramsRefs, s.ServiceAccountToken.Audience) + } + } + } + if v.CSI != nil { + if v.CSI.NodePublishSecretRef != nil { + paramsRefs = append(paramsRefs, v.CSI.NodePublishSecretRef.Name) + } + if v.CSI.VolumeAttributes != nil { + for _, value := range v.CSI.VolumeAttributes { + paramsRefs = append(paramsRefs, value) + } + } + } + } + return paramsRefs +} + +// extractParamRefsFromContainer get all array indexing references from container +func extractParamRefsFromContainer(c *corev1.Container) []string { + paramsRefs := []string{} + paramsRefs = append(paramsRefs, c.Name) + paramsRefs = append(paramsRefs, c.Image) + paramsRefs = append(paramsRefs, string(c.ImagePullPolicy)) + paramsRefs = append(paramsRefs, c.Args...) + + for ie, e := range c.Env { + paramsRefs = append(paramsRefs, e.Value) + if c.Env[ie].ValueFrom != nil { + if e.ValueFrom.SecretKeyRef != nil { + paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.LocalObjectReference.Name) + paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.Key) + } + if e.ValueFrom.ConfigMapKeyRef != nil { + paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name) + paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.Key) + } + } + } + + for _, e := range c.EnvFrom { + paramsRefs = append(paramsRefs, e.Prefix) + if e.ConfigMapRef != nil { + paramsRefs = append(paramsRefs, e.ConfigMapRef.LocalObjectReference.Name) + } + if e.SecretRef != nil { + paramsRefs = append(paramsRefs, e.SecretRef.LocalObjectReference.Name) + } + } + + paramsRefs = append(paramsRefs, c.WorkingDir) + paramsRefs = append(paramsRefs, c.Command...) + + for _, v := range c.VolumeMounts { + paramsRefs = append(paramsRefs, v.Name) + paramsRefs = append(paramsRefs, v.MountPath) + paramsRefs = append(paramsRefs, v.SubPath) + } + return paramsRefs +} + // ParamType indicates the type of an input parameter; // Used to distinguish between a single string and an array of strings. type ParamType string diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go index f4b08d65376..c5e197ca330 100644 --- a/pkg/apis/pipeline/v1/pipeline_types.go +++ b/pkg/apis/pipeline/v1/pipeline_types.go @@ -90,7 +90,7 @@ type PipelineSpec struct { // Params declares a list of input parameters that must be supplied when // this Pipeline is run. // +listType=atomic - Params []ParamSpec `json:"params,omitempty"` + Params ParamSpecs `json:"params,omitempty"` // Workspaces declares a set of named workspaces that are expected to be // provided by a PipelineRun. // +optional @@ -184,7 +184,7 @@ type PipelineTask struct { // Parameters declares parameters passed to this task. // +optional // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` // Matrix declares parameters used to fan out this task. // +optional @@ -210,7 +210,7 @@ type Matrix struct { // Each array element is supplied to the `PipelineTask` by substituting `params` of type `"string"` in the underlying `Task`. // The names of the `params` in the `Matrix` must match the names of the `params` in the underlying `Task` that they will be substituting. // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` // Include is a list of MatrixInclude which allows passing in specific combinations of Parameters into the Matrix. // Note that Include is in preview mode and not yet supported. @@ -228,7 +228,7 @@ type MatrixInclude struct { // Params takes only `Parameters` of type `"string"` // The names of the `params` must match the names of the `params` in the underlying `Task` // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` } // validateRefOrSpec validates at least one of taskRef or taskSpec is specified diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 444d6b3a76c..db7dd5edd47 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -424,3 +424,51 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f } return errs } + +// ValidateParamArrayIndex validates if the param reference to an array param is out of bound. +// error is returned when the array indexing reference is out of bound of the array param +// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. +func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { + if !config.CheckAlphaOrBetaAPIFields(ctx) { + return nil + } + + // Collect all array params lengths + arrayParamsLengths := ps.Params.extractParamArrayLengths() + for k, v := range params.extractParamArrayLengths() { + arrayParamsLengths[k] = v + } + + paramsRefs := []string{} + for i := range ps.Tasks { + paramsRefs = append(paramsRefs, ps.Tasks[i].Params.extractParamValuesFromParams()...) + if ps.Tasks[i].IsMatrixed() { + paramsRefs = append(paramsRefs, ps.Tasks[i].Matrix.Params.extractParamValuesFromParams()...) + } + for j := range ps.Tasks[i].Workspaces { + paramsRefs = append(paramsRefs, ps.Tasks[i].Workspaces[j].SubPath) + } + for _, wes := range ps.Tasks[i].When { + paramsRefs = append(paramsRefs, wes.Input) + paramsRefs = append(paramsRefs, wes.Values...) + } + } + + for i := range ps.Finally { + paramsRefs = append(paramsRefs, ps.Finally[i].Params.extractParamValuesFromParams()...) + if ps.Finally[i].IsMatrixed() { + paramsRefs = append(paramsRefs, ps.Finally[i].Matrix.Params.extractParamValuesFromParams()...) + } + for _, wes := range ps.Finally[i].When { + paramsRefs = append(paramsRefs, wes.Values...) + } + } + + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := []string{} + for _, p := range paramsRefs { + arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) + } + + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) +} diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 8b26c7b8e81..d6f6efe4eeb 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3269,3 +3269,386 @@ func getTaskSpec() TaskSpec { }}, } } + +func TestValidateParamArrayIndex_valid(t *testing.T) { + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields + ctx = config.ToContext(ctx, cfg) + for _, tt := range []struct { + name string + original PipelineSpec + params []Param + }{{ + name: "single parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, + {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "single parameter with when expression", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + When: []WhenExpression{{ + Input: "$(params.first-param[1])", + Operator: selection.In, + Values: []string{"$(params.second-param[0])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "pipeline parameter nested inside task parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, + }, + }}, + }, + params: nil, // no parameter values. + }, { + name: "array parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + }, { + name: "parameter evaluation with final tasks", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + When: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter evaluation with both tasks and final tasks", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + }}, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + When: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter references with bracket notation and special characters", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second/param", Type: ParamTypeArray}, + {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, + {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][0])`)}, + {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, + {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, + {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{ + {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + }, { + name: "single parameter in workspace subpath", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + }, + Workspaces: []WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[1])", + }, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.original.ValidateParamArrayIndex(ctx, tt.params) + if err != nil { + t.Errorf("ValidateParamArrayIndex() got err %s", err) + } + }) + } +} + +func TestValidateParamArrayIndex_invalid(t *testing.T) { + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields + ctx = config.ToContext(ctx, cfg) + for _, tt := range []struct { + name string + original PipelineSpec + params []Param + expected error + }{{ + name: "single parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "single parameter reference with when expression out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + When: []WhenExpression{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "pipeline parameter reference nested inside task parameter out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, + }, + }}, + }, + params: nil, // no parameter values. + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "array parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param[3])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[4])")}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), + }, { + name: "object parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewObject(map[string]string{ + "val1": "$(params.first-param[4])", + "val2": "$(params.second-param[4])", + })}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), + }, { + name: "parameter evaluation with final tasks reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + }, + When: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "parameter evaluation with both tasks and final tasks reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + }, + }}, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[3])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[3])")}, + }, + When: WhenExpressions{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.second-param[2]) $(params.second-param[3])]"), + }, { + name: "parameter references with bracket notation and special characters reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second/param", Type: ParamTypeArray}, + {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][2])`)}, + {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][2])`)}, + {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][2])`)}, + {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][2])`)}, + {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{ + {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), + }, { + name: "single parameter in workspace subpath reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + }, + Workspaces: []WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[3])", + }, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.original.ValidateParamArrayIndex(ctx, tt.params) + if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { + t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/task_types.go b/pkg/apis/pipeline/v1/task_types.go index eca16b7a7d2..03184ac7f95 100644 --- a/pkg/apis/pipeline/v1/task_types.go +++ b/pkg/apis/pipeline/v1/task_types.go @@ -59,7 +59,7 @@ type TaskSpec struct { // value. // +optional // +listType=atomic - Params []ParamSpec `json:"params,omitempty"` + Params ParamSpecs `json:"params,omitempty"` // Description is a user-facing description of the task that may be // used to populate a UI. diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 5982fa33c73..fdf30e52dcb 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -600,3 +600,39 @@ func validateTaskArraysIsolated(value, prefix string, arrayNames sets.String) *a func isParamRefs(s string) bool { return strings.HasPrefix(s, "$("+ParamsPrefix) } + +// ValidateParamArrayIndex validates if the param reference to an array param is out of bound. +// error is returned when the array indexing reference is out of bound of the array param +// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. +// - `trParams` are params from taskrun. +// - `taskSpec` contains params declarations. +func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { + return nil + } + + // Collect all array params lengths + arrayParamsLengths := ts.Params.extractParamArrayLengths() + for k, v := range params.extractParamArrayLengths() { + arrayParamsLengths[k] = v + } + + // collect all the possible places to use param references + paramsRefs := []string{} + paramsRefs = append(paramsRefs, extractParamRefsFromSteps(ts.Steps)...) + paramsRefs = append(paramsRefs, extractParamRefsFromStepTemplate(ts.StepTemplate)...) + paramsRefs = append(paramsRefs, extractParamRefsFromVolumes(ts.Volumes)...) + for _, v := range ts.Workspaces { + paramsRefs = append(paramsRefs, v.MountPath) + } + paramsRefs = append(paramsRefs, extractParamRefsFromSidecars(ts.Sidecars)...) + + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := []string{} + for _, p := range paramsRefs { + arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) + } + + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) +} diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 496b22fe049..5fa4d763f19 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -16,6 +16,7 @@ package v1_test import ( "context" "fmt" + "strings" "testing" "time" @@ -1729,3 +1730,198 @@ func TestSubstitutedContext(t *testing.T) { }) } } + +func TestValidateParamArrayIndex(t *testing.T) { + stepsInvalidReferences := []string{} + for i := 10; i <= 26; i++ { + stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + volumesInvalidReferences := []string{} + for i := 10; i <= 22; i++ { + volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + + tcs := []struct { + name string + params []v1.Param + taskspec *v1.TaskSpec + expectedError error + }{{ + name: "steps reference invalid", + params: []v1.Param{{ + Name: "array-params", + Value: *v1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "array-params", + Default: v1.NewStructuredValues("bar", "foo"), + }}, + Steps: []v1.Step{{ + Name: "$(params.array-params[10])", + Image: "$(params.array-params[11])", + Command: []string{"$(params.array-params[12])"}, + Args: []string{"$(params.array-params[13])"}, + Script: "echo $(params.array-params[14])", + Env: []corev1.EnvVar{{ + Value: "$(params.array-params[15])", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + Key: "$(params.array-params[16])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[17])", + }, + }, + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + Key: "$(params.array-params[18])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + }, + }}, + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(params.array-params[20])", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + }, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[22])", + }, + }, + }}, + WorkingDir: "$(params.array-params[23])", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(params.array-params[24])", + MountPath: "$(params.array-params[25])", + SubPath: "$(params.array-params[26])", + }}, + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), + }, { + name: "stepTemplate reference invalid", + params: []v1.Param{{ + Name: "array-params", + Value: *v1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "array-params", + Default: v1.NewStructuredValues("bar", "foo"), + }}, + StepTemplate: &v1.StepTemplate{ + Image: "$(params.array-params[3])", + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "volumes reference invalid", + params: []v1.Param{{ + Name: "array-params", + Value: *v1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "array-params", + Default: v1.NewStructuredValues("bar", "foo"), + }}, + Volumes: []corev1.Volume{{ + Name: "$(params.array-params[10])", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[11])", + }, + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[12])", + Path: "$(params.array-params[13])", + }, + }, + }, + Secret: &corev1.SecretVolumeSource{ + SecretName: "$(params.array-params[14])", + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[15])", + Path: "$(params.array-params[16])", + }}, + }, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "$(params.array-params[17])", + }, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + ConfigMap: &corev1.ConfigMapProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[18])", + }, + }, + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "$(params.array-params[20])", + }, + }}, + }, + CSI: &corev1.CSIVolumeSource{ + NodePublishSecretRef: &corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, + }, + }, + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), + }, { + name: "workspaces reference invalid", + params: []v1.Param{{ + Name: "array-params", + Value: *v1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "array-params", + Default: v1.NewStructuredValues("bar", "foo"), + }}, + Workspaces: []v1.WorkspaceDeclaration{{ + MountPath: "$(params.array-params[3])", + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "sidecar reference invalid", + params: []v1.Param{{ + Name: "array-params", + Value: *v1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "array-params", + Default: v1.NewStructuredValues("bar", "foo"), + }}, + Sidecars: []v1.Sidecar{{ + Script: "$(params.array-params[3])", + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + ctx := config.ToContext(context.Background(), &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) + err := tc.taskspec.ValidateParamArrayIndex(ctx, tc.params) + if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { + t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 5b300cae81e..7e7c2d9f9c6 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -102,7 +102,7 @@ func (in *Matrix) DeepCopyInto(out *Matrix) { *out = *in if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -132,7 +132,7 @@ func (in *MatrixInclude) DeepCopyInto(out *MatrixInclude) { *out = *in if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -195,6 +195,28 @@ func (in *ParamSpec) DeepCopy() *ParamSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in ParamSpecs) DeepCopyInto(out *ParamSpecs) { + { + in := &in + *out = make(ParamSpecs, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParamSpecs. +func (in ParamSpecs) DeepCopy() ParamSpecs { + if in == nil { + return nil + } + out := new(ParamSpecs) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamValue) DeepCopyInto(out *ParamValue) { *out = *in @@ -223,6 +245,28 @@ func (in *ParamValue) DeepCopy() *ParamValue { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Params) DeepCopyInto(out *Params) { + { + in := &in + *out = make(Params, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Params. +func (in Params) DeepCopy() Params { + if in == nil { + return nil + } + out := new(Params) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Pipeline) DeepCopyInto(out *Pipeline) { *out = *in @@ -593,7 +637,7 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]ParamSpec, len(*in)) + *out = make(ParamSpecs, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -657,7 +701,7 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -1649,7 +1693,7 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) { *out = *in if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]ParamSpec, len(*in)) + *out = make(ParamSpecs, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 1003fa33825..5547bbdddbb 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -24,6 +24,7 @@ import ( resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/substitution" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" ) @@ -55,6 +56,9 @@ type ParamSpec struct { Default *ParamValue `json:"default,omitempty"` } +// ParamSpecs is a list of ParamSpec +type ParamSpecs []ParamSpec + // PropertySpec defines the struct for object keys type PropertySpec struct { Type ParamType `json:"type,omitempty"` @@ -110,6 +114,216 @@ type Param struct { Value ParamValue `json:"value"` } +// Params is a list of Param +type Params []Param + +// extractParamValuesFromParams get all param values from params +func (ps Params) extractParamValuesFromParams() []string { + pvs := []string{} + for i := range ps { + pvs = append(pvs, ps[i].Value.StringVal) + pvs = append(pvs, ps[i].Value.ArrayVal...) + for _, v := range ps[i].Value.ObjectVal { + pvs = append(pvs, v) + } + } + return pvs +} + +// extractParamArrayLengths extract and return the lengths of all array params +// Example of returned value: {"a-array-params": 2,"b-array-params": 2 } +func (ps Params) extractParamArrayLengths() map[string]int { + // Collect all array params + arrayParamsLengths := make(map[string]int) + + // Collect array params lengths from params + for _, p := range ps { + if p.Value.Type == ParamTypeArray { + arrayParamsLengths[p.Name] = len(p.Value.ArrayVal) + } + } + return arrayParamsLengths +} + +// extractParamArrayLengths extract and return the lengths of all array params +// Example of returned value: {"a-array-params": 2,"b-array-params": 2 } +func (ps ParamSpecs) extractParamArrayLengths() map[string]int { + // Collect all array params + arrayParamsLengths := make(map[string]int) + + // Collect array params lengths from defaults + for _, p := range ps { + if p.Default != nil { + if p.Default.Type == ParamTypeArray { + arrayParamsLengths[p.Name] = len(p.Default.ArrayVal) + } + } + } + return arrayParamsLengths +} + +// validateOutofBoundArrayParams validates if the array indexing params are out of bound +// example of arrayIndexingParams: ["$(params.a-array-param[1])", "$(params.b-array-param[2])"] +// example of arrayParamsLengths: {"a-array-params": 2,"b-array-params": 2 } +func validateOutofBoundArrayParams(arrayIndexingParams []string, arrayParamsLengths map[string]int) error { + outofBoundParams := sets.String{} + for _, val := range arrayIndexingParams { + indexString := substitution.ExtractIndexString(val) + idx, _ := substitution.ExtractIndex(indexString) + // this will extract the param name from reference + // e.g. $(params.a-array-param[1]) -> a-array-param + paramName, _, _ := substitution.ExtractVariablesFromString(substitution.TrimArrayIndex(val), "params") + + if paramLength, ok := arrayParamsLengths[paramName[0]]; ok { + if idx >= paramLength { + outofBoundParams.Insert(val) + } + } + } + if outofBoundParams.Len() > 0 { + return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) + } + return nil +} + +// extractArrayIndexingParamRefs takes a string of the form `foo-$(params.array-param[1])-bar` and extracts the portions of the string that reference an element in an array param. +// For example, for the string “foo-$(params.array-param[1])-bar-$(params.other-array-param[2])-$(params.string-param)`, +// it would return ["$(params.array-param[1])", "$(params.other-array-param[2])"]. +func extractArrayIndexingParamRefs(paramReference string) []string { + l := []string{} + list := substitution.ExtractParamsExpressions(paramReference) + for _, val := range list { + indexString := substitution.ExtractIndexString(val) + if indexString != "" { + l = append(l, val) + } + } + return l +} + +// extractParamRefsFromSteps get all array indexing references from steps +func extractParamRefsFromSteps(steps []Step) []string { + paramsRefs := []string{} + for _, step := range steps { + paramsRefs = append(paramsRefs, step.Script) + container := step.ToK8sContainer() + paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...) + } + return paramsRefs +} + +// extractParamRefsFromStepTemplate get all array indexing references from StepsTemplate +func extractParamRefsFromStepTemplate(stepTemplate *StepTemplate) []string { + if stepTemplate == nil { + return nil + } + container := stepTemplate.ToK8sContainer() + return extractParamRefsFromContainer(container) +} + +// extractParamRefsFromSidecars get all array indexing references from sidecars +func extractParamRefsFromSidecars(sidecars []Sidecar) []string { + paramsRefs := []string{} + for _, s := range sidecars { + paramsRefs = append(paramsRefs, s.Script) + container := s.ToK8sContainer() + paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...) + } + return paramsRefs +} + +// extractParamRefsFromVolumes get all array indexing references from volumes +func extractParamRefsFromVolumes(volumes []corev1.Volume) []string { + paramsRefs := []string{} + for i, v := range volumes { + paramsRefs = append(paramsRefs, v.Name) + if v.VolumeSource.ConfigMap != nil { + paramsRefs = append(paramsRefs, v.ConfigMap.Name) + for _, item := range v.ConfigMap.Items { + paramsRefs = append(paramsRefs, item.Key) + paramsRefs = append(paramsRefs, item.Path) + } + } + if v.VolumeSource.Secret != nil { + paramsRefs = append(paramsRefs, v.Secret.SecretName) + for _, item := range v.Secret.Items { + paramsRefs = append(paramsRefs, item.Key) + paramsRefs = append(paramsRefs, item.Path) + } + } + if v.PersistentVolumeClaim != nil { + paramsRefs = append(paramsRefs, v.PersistentVolumeClaim.ClaimName) + } + if v.Projected != nil { + for _, s := range volumes[i].Projected.Sources { + if s.ConfigMap != nil { + paramsRefs = append(paramsRefs, s.ConfigMap.Name) + } + if s.Secret != nil { + paramsRefs = append(paramsRefs, s.Secret.Name) + } + if s.ServiceAccountToken != nil { + paramsRefs = append(paramsRefs, s.ServiceAccountToken.Audience) + } + } + } + if v.CSI != nil { + if v.CSI.NodePublishSecretRef != nil { + paramsRefs = append(paramsRefs, v.CSI.NodePublishSecretRef.Name) + } + if v.CSI.VolumeAttributes != nil { + for _, value := range v.CSI.VolumeAttributes { + paramsRefs = append(paramsRefs, value) + } + } + } + } + return paramsRefs +} + +// extractParamRefsFromContainer get all array indexing references from container +func extractParamRefsFromContainer(c *corev1.Container) []string { + paramsRefs := []string{} + paramsRefs = append(paramsRefs, c.Name) + paramsRefs = append(paramsRefs, c.Image) + paramsRefs = append(paramsRefs, string(c.ImagePullPolicy)) + paramsRefs = append(paramsRefs, c.Args...) + + for ie, e := range c.Env { + paramsRefs = append(paramsRefs, e.Value) + if c.Env[ie].ValueFrom != nil { + if e.ValueFrom.SecretKeyRef != nil { + paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.LocalObjectReference.Name) + paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.Key) + } + if e.ValueFrom.ConfigMapKeyRef != nil { + paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name) + paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.Key) + } + } + } + + for _, e := range c.EnvFrom { + paramsRefs = append(paramsRefs, e.Prefix) + if e.ConfigMapRef != nil { + paramsRefs = append(paramsRefs, e.ConfigMapRef.LocalObjectReference.Name) + } + if e.SecretRef != nil { + paramsRefs = append(paramsRefs, e.SecretRef.LocalObjectReference.Name) + } + } + + paramsRefs = append(paramsRefs, c.WorkingDir) + paramsRefs = append(paramsRefs, c.Command...) + + for _, v := range c.VolumeMounts { + paramsRefs = append(paramsRefs, v.Name) + paramsRefs = append(paramsRefs, v.MountPath) + paramsRefs = append(paramsRefs, v.SubPath) + } + return paramsRefs +} + // ParamType indicates the type of an input parameter; // Used to distinguish between a single string and an array of strings. type ParamType string diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index c5a11bb0893..cd43fd92cc3 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -100,7 +100,7 @@ type PipelineSpec struct { // Params declares a list of input parameters that must be supplied when // this Pipeline is run. // +listType=atomic - Params []ParamSpec `json:"params,omitempty"` + Params ParamSpecs `json:"params,omitempty"` // Workspaces declares a set of named workspaces that are expected to be // provided by a PipelineRun. // +optional @@ -168,7 +168,7 @@ type Matrix struct { // Each array element is supplied to the `PipelineTask` by substituting `params` of type `"string"` in the underlying `Task`. // The names of the `params` in the `Matrix` must match the names of the `params` in the underlying `Task` that they will be substituting. // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` // Include is a list of MatrixInclude which allows passing in specific combinations of Parameters into the Matrix. // Note that Include is in preview mode and not yet supported. @@ -186,7 +186,7 @@ type MatrixInclude struct { // Params takes only `Parameters` of type `"string"` // The names of the `params` must match the names of the `params` in the underlying `Task` // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` } // PipelineTask defines a task in a Pipeline, passing inputs from both @@ -227,7 +227,7 @@ type PipelineTask struct { // Parameters declares parameters passed to this task. // +optional // +listType=atomic - Params []Param `json:"params,omitempty"` + Params Params `json:"params,omitempty"` // Matrix declares parameters used to fan out this task. // +optional diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index fab984661e7..b1af8de0c26 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -535,3 +535,51 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f } return errs } + +// ValidateParamArrayIndex validates if the param reference to an array param is out of bound. +// error is returned when the array indexing reference is out of bound of the array param +// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. +func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { + if !config.CheckAlphaOrBetaAPIFields(ctx) { + return nil + } + + // Collect all array params lengths + arrayParamsLengths := ps.Params.extractParamArrayLengths() + for k, v := range params.extractParamArrayLengths() { + arrayParamsLengths[k] = v + } + + paramsRefs := []string{} + for i := range ps.Tasks { + paramsRefs = append(paramsRefs, ps.Tasks[i].Params.extractParamValuesFromParams()...) + if ps.Tasks[i].IsMatrixed() { + paramsRefs = append(paramsRefs, ps.Tasks[i].Matrix.Params.extractParamValuesFromParams()...) + } + for j := range ps.Tasks[i].Workspaces { + paramsRefs = append(paramsRefs, ps.Tasks[i].Workspaces[j].SubPath) + } + for _, wes := range ps.Tasks[i].WhenExpressions { + paramsRefs = append(paramsRefs, wes.Input) + paramsRefs = append(paramsRefs, wes.Values...) + } + } + + for i := range ps.Finally { + paramsRefs = append(paramsRefs, ps.Finally[i].Params.extractParamValuesFromParams()...) + if ps.Finally[i].IsMatrixed() { + paramsRefs = append(paramsRefs, ps.Finally[i].Matrix.Params.extractParamValuesFromParams()...) + } + for _, wes := range ps.Finally[i].WhenExpressions { + paramsRefs = append(paramsRefs, wes.Values...) + } + } + + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := []string{} + for _, p := range paramsRefs { + arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) + } + + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index eefc0a39bbd..1bc03563ec3 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3729,3 +3729,386 @@ func enableFeatures(t *testing.T, features []string) func(context.Context) conte return s.ToContext(ctx) } } + +func TestValidateParamArrayIndex_valid(t *testing.T) { + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields + ctx = config.ToContext(ctx, cfg) + for _, tt := range []struct { + name string + original PipelineSpec + params []Param + }{{ + name: "single parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, + {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "single parameter with when expression", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + WhenExpressions: []WhenExpression{{ + Input: "$(params.first-param[1])", + Operator: selection.In, + Values: []string{"$(params.second-param[0])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "pipeline parameter nested inside task parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, + }, + }}, + }, + params: nil, // no parameter values. + }, { + name: "array parameter", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + }, { + name: "parameter evaluation with final tasks", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + WhenExpressions: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter evaluation with both tasks and final tasks", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + }}, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, + WhenExpressions: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter references with bracket notation and special characters", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second/param", Type: ParamTypeArray}, + {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, + {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][0])`)}, + {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, + {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, + {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{ + {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + }, { + name: "single parameter in workspace subpath", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + }, + Workspaces: []WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[1])", + }, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.original.ValidateParamArrayIndex(ctx, tt.params) + if err != nil { + t.Errorf("ValidateParamArrayIndex() got err %s", err) + } + }) + } +} + +func TestValidateParamArrayIndex_invalid(t *testing.T) { + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields + ctx = config.ToContext(ctx, cfg) + for _, tt := range []struct { + name string + original PipelineSpec + params []Param + expected error + }{{ + name: "single parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "single parameter reference with when expression out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + WhenExpressions: []WhenExpression{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "pipeline parameter reference nested inside task parameter out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, + }, + }}, + }, + params: nil, // no parameter values. + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "array parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param[3])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[4])")}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), + }, { + name: "object parameter reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewObject(map[string]string{ + "val1": "$(params.first-param[4])", + "val2": "$(params.second-param[4])", + })}, + }, + }}, + }, + params: []Param{ + {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), + }, { + name: "parameter evaluation with final tasks reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + }, + WhenExpressions: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "parameter evaluation with both tasks and final tasks reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, + }, + }}, + Finally: []PipelineTask{{ + Params: []Param{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[3])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[3])")}, + }, + WhenExpressions: WhenExpressions{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.second-param[2]) $(params.second-param[3])]"), + }, { + name: "parameter references with bracket notation and special characters reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second/param", Type: ParamTypeArray}, + {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][2])`)}, + {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][2])`)}, + {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][2])`)}, + {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][2])`)}, + {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, + }, + }}, + }, + params: []Param{ + {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), + }, { + name: "single parameter in workspace subpath reference out of bound", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + }, + Workspaces: []WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[3])", + }, + }, + }}, + }, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := tt.original.ValidateParamArrayIndex(ctx, tt.params) + if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { + t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/task_types.go b/pkg/apis/pipeline/v1beta1/task_types.go index 957b0aef697..b72abf2c3ff 100644 --- a/pkg/apis/pipeline/v1beta1/task_types.go +++ b/pkg/apis/pipeline/v1beta1/task_types.go @@ -91,7 +91,7 @@ type TaskSpec struct { // value. // +optional // +listType=atomic - Params []ParamSpec `json:"params,omitempty"` + Params ParamSpecs `json:"params,omitempty"` // Description is a user-facing description of the task that may be // used to populate a UI. diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 24447097a0c..892aa1b35f3 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -621,3 +621,39 @@ func validateTaskArraysIsolated(value, prefix string, arrayNames sets.String) *a func isParamRefs(s string) bool { return strings.HasPrefix(s, "$("+ParamsPrefix) } + +// ValidateParamArrayIndex validates if the param reference to an array param is out of bound. +// error is returned when the array indexing reference is out of bound of the array param +// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. +// - `trParams` are params from taskrun. +// - `taskSpec` contains params declarations. +func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { + return nil + } + + // Collect all array params lengths + arrayParamsLengths := ts.Params.extractParamArrayLengths() + for k, v := range params.extractParamArrayLengths() { + arrayParamsLengths[k] = v + } + + // collect all the possible places to use param references + paramsRefs := []string{} + paramsRefs = append(paramsRefs, extractParamRefsFromSteps(ts.Steps)...) + paramsRefs = append(paramsRefs, extractParamRefsFromStepTemplate(ts.StepTemplate)...) + paramsRefs = append(paramsRefs, extractParamRefsFromVolumes(ts.Volumes)...) + for _, v := range ts.Workspaces { + paramsRefs = append(paramsRefs, v.MountPath) + } + paramsRefs = append(paramsRefs, extractParamRefsFromSidecars(ts.Sidecars)...) + + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := []string{} + for _, p := range paramsRefs { + arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) + } + + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) +} diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 8d091010469..0a42616e981 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -19,6 +19,7 @@ package v1beta1_test import ( "context" "fmt" + "strings" "testing" "time" @@ -1842,3 +1843,198 @@ func TestSubstitutedContext(t *testing.T) { }) } } + +func TestValidateParamArrayIndex(t *testing.T) { + stepsInvalidReferences := []string{} + for i := 10; i <= 26; i++ { + stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + volumesInvalidReferences := []string{} + for i := 10; i <= 22; i++ { + volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + + tcs := []struct { + name string + params []v1beta1.Param + taskspec *v1beta1.TaskSpec + expectedError error + }{{ + name: "steps reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Steps: []v1beta1.Step{{ + Name: "$(params.array-params[10])", + Image: "$(params.array-params[11])", + Command: []string{"$(params.array-params[12])"}, + Args: []string{"$(params.array-params[13])"}, + Script: "echo $(params.array-params[14])", + Env: []corev1.EnvVar{{ + Value: "$(params.array-params[15])", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + Key: "$(params.array-params[16])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[17])", + }, + }, + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + Key: "$(params.array-params[18])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + }, + }}, + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(params.array-params[20])", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + }, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[22])", + }, + }, + }}, + WorkingDir: "$(params.array-params[23])", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(params.array-params[24])", + MountPath: "$(params.array-params[25])", + SubPath: "$(params.array-params[26])", + }}, + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), + }, { + name: "stepTemplate reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + StepTemplate: &v1beta1.StepTemplate{ + Image: "$(params.array-params[3])", + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "volumes reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Volumes: []corev1.Volume{{ + Name: "$(params.array-params[10])", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[11])", + }, + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[12])", + Path: "$(params.array-params[13])", + }, + }, + }, + Secret: &corev1.SecretVolumeSource{ + SecretName: "$(params.array-params[14])", + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[15])", + Path: "$(params.array-params[16])", + }}, + }, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "$(params.array-params[17])", + }, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + ConfigMap: &corev1.ConfigMapProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[18])", + }, + }, + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "$(params.array-params[20])", + }, + }}, + }, + CSI: &corev1.CSIVolumeSource{ + NodePublishSecretRef: &corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, + }, + }, + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), + }, { + name: "workspaces reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Workspaces: []v1beta1.WorkspaceDeclaration{{ + MountPath: "$(params.array-params[3])", + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "sidecar reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Sidecars: []v1beta1.Sidecar{{ + Script: "$(params.array-params[3])", + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + ctx := config.ToContext(context.Background(), &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) + err := tc.taskspec.ValidateParamArrayIndex(ctx, tc.params) + if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { + t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 27b38027958..512456098c6 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -362,7 +362,7 @@ func (in *Matrix) DeepCopyInto(out *Matrix) { *out = *in if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -392,7 +392,7 @@ func (in *MatrixInclude) DeepCopyInto(out *MatrixInclude) { *out = *in if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -455,6 +455,28 @@ func (in *ParamSpec) DeepCopy() *ParamSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in ParamSpecs) DeepCopyInto(out *ParamSpecs) { + { + in := &in + *out = make(ParamSpecs, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParamSpecs. +func (in ParamSpecs) DeepCopy() ParamSpecs { + if in == nil { + return nil + } + out := new(ParamSpecs) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamValue) DeepCopyInto(out *ParamValue) { *out = *in @@ -483,6 +505,28 @@ func (in *ParamValue) DeepCopy() *ParamValue { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Params) DeepCopyInto(out *Params) { + { + in := &in + *out = make(Params, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Params. +func (in Params) DeepCopy() Params { + if in == nil { + return nil + } + out := new(Params) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Pipeline) DeepCopyInto(out *Pipeline) { *out = *in @@ -955,7 +999,7 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]ParamSpec, len(*in)) + *out = make(ParamSpecs, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -1024,7 +1068,7 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) + *out = make(Params, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -2264,7 +2308,7 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]ParamSpec, len(*in)) + *out = make(ParamSpecs, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 97317efca42..ece3aaf5256 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -510,7 +510,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } // Ensure that the array reference is not out of bound - if err := resources.ValidateParamArrayIndex(ctx, pipelineSpec, pr); err != nil { + if err := pipelineSpec.ValidateParamArrayIndex(ctx, pr.Spec.Params); err != nil { // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonParamArrayIndexingInvalid, "PipelineRun %s/%s failed validation: failed to validate Pipeline %s/%s's parameter which has an invalid index while referring to an array: %s", diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index 0b65d3683b9..a93181c3d39 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -17,15 +17,11 @@ limitations under the License. package resources import ( - "context" "fmt" - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun" - "github.com/tektoncd/pipeline/pkg/substitution" - "k8s.io/apimachinery/pkg/util/sets" ) // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type. @@ -55,7 +51,7 @@ func ValidateParamTypesMatching(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun // ValidateRequiredParametersProvided validates that all the parameters expected by the Pipeline are provided by the PipelineRun. // Extra Parameters are allowed, the Pipeline will use the Parameters it needs and ignore the other Parameters. -func ValidateRequiredParametersProvided(pipelineParameters *[]v1beta1.ParamSpec, pipelineRunParameters *[]v1beta1.Param) error { +func ValidateRequiredParametersProvided(pipelineParameters *v1beta1.ParamSpecs, pipelineRunParameters *[]v1beta1.Param) error { // Build a list of parameter names declared in pr. var providedParams []string for _, param := range *pipelineRunParameters { @@ -88,110 +84,3 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip return nil } - -// ValidateParamArrayIndex validate if the array indexing param reference target is existent -func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error { - if !config.CheckAlphaOrBetaAPIFields(ctx) { - return nil - } - - arrayParams := extractParamIndexes(p.Params, pr.Spec.Params) - - outofBoundParams := sets.String{} - - // collect all the references - for i := range p.Tasks { - findInvalidParamArrayReferences(p.Tasks[i].Params, arrayParams, &outofBoundParams) - if p.Tasks[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams) - } - for j := range p.Tasks[i].Workspaces { - findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams) - } - for _, wes := range p.Tasks[i].WhenExpressions { - findInvalidParamArrayReference(wes.Input, arrayParams, &outofBoundParams) - for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) - } - } - } - - for i := range p.Finally { - findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams, &outofBoundParams) - if p.Finally[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams) - } - for _, wes := range p.Finally[i].WhenExpressions { - for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) - } - } - } - - if outofBoundParams.Len() > 0 { - return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) - } - return nil -} - -func extractParamIndexes(defaults []v1beta1.ParamSpec, params []v1beta1.Param) map[string]int { - // Collect all array params - arrayParams := make(map[string]int) - - patterns := []string{ - "$(params.%s)", - "$(params[%q])", - "$(params['%s'])", - } - - // Collect array params lengths from defaults - for _, p := range defaults { - if p.Default != nil { - if p.Default.Type == v1beta1.ParamTypeArray { - for _, pattern := range patterns { - for i := 0; i < len(p.Default.ArrayVal); i++ { - arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal) - } - } - } - } - } - - // Collect array params lengths from pipelinerun or taskrun - for _, p := range params { - if p.Value.Type == v1beta1.ParamTypeArray { - for _, pattern := range patterns { - for i := 0; i < len(p.Value.ArrayVal); i++ { - arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal) - } - } - } - } - return arrayParams -} - -func findInvalidParamArrayReferences(params []v1beta1.Param, arrayParams map[string]int, outofBoundParams *sets.String) { - for i := range params { - findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams, outofBoundParams) - for _, v := range params[i].Value.ArrayVal { - findInvalidParamArrayReference(v, arrayParams, outofBoundParams) - } - for _, v := range params[i].Value.ObjectVal { - findInvalidParamArrayReference(v, arrayParams, outofBoundParams) - } - } -} - -func findInvalidParamArrayReference(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) { - list := substitution.ExtractParamsExpressions(paramReference) - for _, val := range list { - indexString := substitution.ExtractIndexString(paramReference) - idx, _ := substitution.ExtractIndex(indexString) - v := substitution.TrimArrayIndex(val) - if paramLength, ok := arrayParams[v]; ok { - if idx >= paramLength { - outofBoundParams.Insert(val) - } - } - } -} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index 685b06dd829..4a92ed128ff 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -17,16 +17,10 @@ limitations under the License. package resources import ( - "context" - "fmt" "testing" - "github.com/google/go-cmp/cmp" - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/test/diff" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/selection" ) func TestValidateParamTypesMatching_Valid(t *testing.T) { @@ -122,7 +116,7 @@ func TestValidateRequiredParametersProvided_Valid(t *testing.T) { for _, tc := range []struct { name string description string - pp []v1beta1.ParamSpec + pp v1beta1.ParamSpecs prp []v1beta1.Param }{{ name: "required string params provided", @@ -164,7 +158,7 @@ func TestValidateRequiredParametersProvided_Invalid(t *testing.T) { for _, tc := range []struct { name string description string - pp []v1beta1.ParamSpec + pp v1beta1.ParamSpecs prp []v1beta1.Param }{{ name: "required string param missing", @@ -322,396 +316,3 @@ func TestValidateObjectParamRequiredKeys_Valid(t *testing.T) { }) } } - -func TestValidateParamArrayIndex_valid(t *testing.T) { - ctx := context.Background() - cfg := config.FromContextOrDefaults(ctx) - cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields - ctx = config.ToContext(ctx, cfg) - for _, tt := range []struct { - name string - original v1beta1.PipelineSpec - params []v1beta1.Param - }{{ - name: "single parameter", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeString}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[1])")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[0])")}, - {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "single parameter with when expression", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeString}, - }, - Tasks: []v1beta1.PipelineTask{{ - WhenExpressions: []v1beta1.WhenExpression{{ - Input: "$(params.first-param[1])", - Operator: selection.In, - Values: []string{"$(params.second-param[0])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "pipeline parameter nested inside task parameter", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, - }, - }}, - }, - params: nil, // no parameter values. - }, { - name: "array parameter", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.first-param)")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.second-param[0])")}, - }, - }}, - }, - params: []v1beta1.Param{ - {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, - }, - }, { - name: "parameter evaluation with final tasks", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Finally: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, - }, - WhenExpressions: v1beta1.WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter evaluation with both tasks and final tasks", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, - }, - }}, - Finally: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, - }, - WhenExpressions: v1beta1.WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter references with bracket notation and special characters", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: v1beta1.ParamTypeArray}, - {Name: "third.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues(`$(params["first.param"][0])`)}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues(`$(params["second/param"][0])`)}, - {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues(`$(params['third.param'][1])`)}, - {Name: "first-task-fourth-param", Value: *v1beta1.NewStructuredValues(`$(params['fourth/param'][1])`)}, - {Name: "first-task-fifth-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - }}, - }, - params: []v1beta1.Param{ - {Name: "second/param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *v1beta1.NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - }, { - name: "single parameter in workspace subpath", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - Workspaces: []v1beta1.WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[1])", - }, - }, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - run := &v1beta1.PipelineRun{ - Spec: v1beta1.PipelineRunSpec{ - Params: tt.params, - }, - } - err := ValidateParamArrayIndex(ctx, &tt.original, run) - if err != nil { - t.Errorf("ValidateParamArrayIndex() got err %s", err) - } - }) - } -} - -func TestValidateParamArrayIndex_invalid(t *testing.T) { - ctx := context.Background() - cfg := config.FromContextOrDefaults(ctx) - cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields - ctx = config.ToContext(ctx, cfg) - for _, tt := range []struct { - name string - original v1beta1.PipelineSpec - params []v1beta1.Param - expected error - }{{ - name: "single parameter reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeString}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, - {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "single parameter reference with when expression out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeString}, - }, - Tasks: []v1beta1.PipelineTask{{ - WhenExpressions: []v1beta1.WhenExpression{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "pipeline parameter reference nested inside task parameter out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, - }, - }}, - }, - params: nil, // no parameter values. - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "array parameter reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.first-param[3])")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.second-param[4])")}, - }, - }}, - }, - params: []v1beta1.Param{ - {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), - }, { - name: "object parameter reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewObject(map[string]string{ - "val1": "$(params.first-param[4])", - "val2": "$(params.second-param[4])", - })}, - }, - }}, - }, - params: []v1beta1.Param{ - {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), - }, { - name: "parameter evaluation with final tasks reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Finally: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, - }, - WhenExpressions: v1beta1.WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "parameter evaluation with both tasks and final tasks reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, - }, - }}, - Finally: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[3])")}, - {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[3])")}, - }, - WhenExpressions: v1beta1.WhenExpressions{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.second-param[2]) $(params.second-param[3])]"), - }, { - name: "parameter references with bracket notation and special characters reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: v1beta1.ParamTypeArray}, - {Name: "third.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues(`$(params["first.param"][2])`)}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues(`$(params["second/param"][2])`)}, - {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues(`$(params['third.param'][2])`)}, - {Name: "first-task-fourth-param", Value: *v1beta1.NewStructuredValues(`$(params['fourth/param'][2])`)}, - {Name: "first-task-fifth-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - }}, - }, - params: []v1beta1.Param{ - {Name: "second/param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *v1beta1.NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), - }, { - name: "single parameter in workspace subpath reference out of bound", - original: v1beta1.PipelineSpec{ - Params: []v1beta1.ParamSpec{ - {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: v1beta1.ParamTypeArray}, - }, - Tasks: []v1beta1.PipelineTask{{ - Params: []v1beta1.Param{ - {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("static value")}, - }, - Workspaces: []v1beta1.WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[3])", - }, - }, - }}, - }, - params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - run := &v1beta1.PipelineRun{ - Spec: v1beta1.PipelineRunSpec{ - Params: tt.params, - }, - } - err := ValidateParamArrayIndex(ctx, &tt.original, run) - if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { - t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 0ec2ec2df93..870e8fabf78 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -405,7 +405,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 return nil, nil, controller.NewPermanentError(err) } - if err := validateParamArrayIndex(ctx, tr.Spec.Params, rtr.TaskSpec); err != nil { + if err := rtr.TaskSpec.ValidateParamArrayIndex(ctx, tr.Spec.Params); err != nil { logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err) tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err) diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index e1ff9e5eef0..afad7a58d1b 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -23,13 +23,11 @@ import ( "strings" "github.com/hashicorp/go-multierror" - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" - "github.com/tektoncd/pipeline/pkg/substitution" - corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" ) @@ -158,7 +156,7 @@ func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, needed return wrongTypeParamNames } -// MissingKeysObjectParamNames checks if all required keys of object type params are provided in taskrun params or taskSpec's default. +// MissingKeysObjectParamNames checks if all required keys of object type param definitions are provided in params or param definitions' defaults. func MissingKeysObjectParamNames(paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param) map[string][]string { neededKeys := make(map[string][]string) providedKeys := make(map[string][]string) @@ -364,199 +362,3 @@ func missingKeysofObjectResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskR } return findMissingKeys(neededKeys, providedKeys) } - -func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec *v1beta1.TaskSpec) error { - cfg := config.FromContextOrDefaults(ctx) - if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { - return nil - } - - var defaults []v1beta1.ParamSpec - if len(spec.Params) > 0 { - defaults = append(defaults, spec.Params...) - } - // Collect all array params - arrayParams := make(map[string]int) - - patterns := []string{ - "$(params.%s)", - "$(params[%q])", - "$(params['%s'])", - } - - // Collect array params lengths from defaults - for _, p := range defaults { - if p.Default != nil { - if p.Default.Type == v1beta1.ParamTypeArray { - for _, pattern := range patterns { - for i := 0; i < len(p.Default.ArrayVal); i++ { - arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal) - } - } - } - } - } - - // Collect array params lengths from pipeline - for _, p := range params { - if p.Value.Type == v1beta1.ParamTypeArray { - for _, pattern := range patterns { - for i := 0; i < len(p.Value.ArrayVal); i++ { - arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal) - } - } - } - } - - outofBoundParams := sets.String{} - - // Validate array param in steps fields. - validateStepsParamArrayIndexing(spec.Steps, arrayParams, &outofBoundParams) - - // Validate array param in StepTemplate fields. - validateStepsTemplateParamArrayIndexing(spec.StepTemplate, arrayParams, &outofBoundParams) - - // Validate array param in build's volumes - validateVolumesParamArrayIndexing(spec.Volumes, arrayParams, &outofBoundParams) - - for _, v := range spec.Workspaces { - extractParamIndex(v.MountPath, arrayParams, &outofBoundParams) - } - - validateSidecarsParamArrayIndexing(spec.Sidecars, arrayParams, &outofBoundParams) - - if outofBoundParams.Len() > 0 { - return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) - } - - return nil -} - -func extractParamIndex(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) { - list := substitution.ExtractParamsExpressions(paramReference) - for _, val := range list { - indexString := substitution.ExtractIndexString(paramReference) - idx, _ := substitution.ExtractIndex(indexString) - v := substitution.TrimArrayIndex(val) - if paramLength, ok := arrayParams[v]; ok { - if idx >= paramLength { - outofBoundParams.Insert(val) - } - } - } -} - -func validateStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, outofBoundParams *sets.String) { - for _, step := range steps { - extractParamIndex(step.Script, arrayParams, outofBoundParams) - container := step.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) - } -} - -func validateStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, outofBoundParams *sets.String) { - if stepTemplate == nil { - return - } - container := stepTemplate.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) -} - -func validateSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, outofBoundParams *sets.String) { - for _, s := range sidecars { - extractParamIndex(s.Script, arrayParams, outofBoundParams) - container := s.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) - } -} - -func validateVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, outofBoundParams *sets.String) { - for i, v := range volumes { - extractParamIndex(v.Name, arrayParams, outofBoundParams) - if v.VolumeSource.ConfigMap != nil { - extractParamIndex(v.ConfigMap.Name, arrayParams, outofBoundParams) - for _, item := range v.ConfigMap.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) - } - } - if v.VolumeSource.Secret != nil { - extractParamIndex(v.Secret.SecretName, arrayParams, outofBoundParams) - for _, item := range v.Secret.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) - } - } - if v.PersistentVolumeClaim != nil { - extractParamIndex(v.PersistentVolumeClaim.ClaimName, arrayParams, outofBoundParams) - } - if v.Projected != nil { - for _, s := range volumes[i].Projected.Sources { - if s.ConfigMap != nil { - extractParamIndex(s.ConfigMap.Name, arrayParams, outofBoundParams) - } - if s.Secret != nil { - extractParamIndex(s.Secret.Name, arrayParams, outofBoundParams) - } - if s.ServiceAccountToken != nil { - extractParamIndex(s.ServiceAccountToken.Audience, arrayParams, outofBoundParams) - } - } - } - if v.CSI != nil { - if v.CSI.NodePublishSecretRef != nil { - extractParamIndex(v.CSI.NodePublishSecretRef.Name, arrayParams, outofBoundParams) - } - if v.CSI.VolumeAttributes != nil { - for _, value := range v.CSI.VolumeAttributes { - extractParamIndex(value, arrayParams, outofBoundParams) - } - } - } - } -} - -func validateContainerParamArrayIndexing(c *corev1.Container, arrayParams map[string]int, outofBoundParams *sets.String) { - extractParamIndex(c.Name, arrayParams, outofBoundParams) - extractParamIndex(c.Image, arrayParams, outofBoundParams) - extractParamIndex(string(c.ImagePullPolicy), arrayParams, outofBoundParams) - - for _, a := range c.Args { - extractParamIndex(a, arrayParams, outofBoundParams) - } - - for ie, e := range c.Env { - extractParamIndex(e.Value, arrayParams, outofBoundParams) - if c.Env[ie].ValueFrom != nil { - if e.ValueFrom.SecretKeyRef != nil { - extractParamIndex(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - extractParamIndex(e.ValueFrom.SecretKeyRef.Key, arrayParams, outofBoundParams) - } - if e.ValueFrom.ConfigMapKeyRef != nil { - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, outofBoundParams) - } - } - } - - for _, e := range c.EnvFrom { - extractParamIndex(e.Prefix, arrayParams, outofBoundParams) - if e.ConfigMapRef != nil { - extractParamIndex(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - } - if e.SecretRef != nil { - extractParamIndex(e.SecretRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - } - } - - extractParamIndex(c.WorkingDir, arrayParams, outofBoundParams) - for _, cc := range c.Command { - extractParamIndex(cc, arrayParams, outofBoundParams) - } - - for _, v := range c.VolumeMounts { - extractParamIndex(v.Name, arrayParams, outofBoundParams) - extractParamIndex(v.MountPath, arrayParams, outofBoundParams) - extractParamIndex(v.SubPath, arrayParams, outofBoundParams) - } -} diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index e7a209197f5..7091a3596bb 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -18,18 +18,14 @@ package taskrun import ( "context" - "fmt" - "strings" "testing" - "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" - "github.com/tektoncd/pipeline/test/diff" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestValidateResolvedTaskResources_ValidResources(t *testing.T) { @@ -910,198 +906,3 @@ func TestValidateResult(t *testing.T) { }) } } - -func TestValidateParamArrayIndex(t *testing.T) { - stepsInvalidReferences := []string{} - for i := 10; i <= 26; i++ { - stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - volumesInvalidReferences := []string{} - for i := 10; i <= 22; i++ { - volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - - tcs := []struct { - name string - params []v1beta1.Param - taskspec *v1beta1.TaskSpec - expectedError error - }{{ - name: "steps reference invalid", - params: []v1beta1.Param{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Steps: []v1beta1.Step{{ - Name: "$(params.array-params[10])", - Image: "$(params.array-params[11])", - Command: []string{"$(params.array-params[12])"}, - Args: []string{"$(params.array-params[13])"}, - Script: "echo $(params.array-params[14])", - Env: []v1.EnvVar{{ - Value: "$(params.array-params[15])", - ValueFrom: &v1.EnvVarSource{ - SecretKeyRef: &v1.SecretKeySelector{ - Key: "$(params.array-params[16])", - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[17])", - }, - }, - ConfigMapKeyRef: &v1.ConfigMapKeySelector{ - Key: "$(params.array-params[18])", - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - }, - }}, - EnvFrom: []v1.EnvFromSource{{ - Prefix: "$(params.array-params[20])", - ConfigMapRef: &v1.ConfigMapEnvSource{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - }, - SecretRef: &v1.SecretEnvSource{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[22])", - }, - }, - }}, - WorkingDir: "$(params.array-params[23])", - VolumeMounts: []v1.VolumeMount{{ - Name: "$(params.array-params[24])", - MountPath: "$(params.array-params[25])", - SubPath: "$(params.array-params[26])", - }}, - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), - }, { - name: "stepTemplate reference invalid", - params: []v1beta1.Param{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - StepTemplate: &v1beta1.StepTemplate{ - Image: "$(params.array-params[3])", - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "volumes reference invalid", - params: []v1beta1.Param{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Volumes: []v1.Volume{{ - Name: "$(params.array-params[10])", - VolumeSource: v1.VolumeSource{ - ConfigMap: &v1.ConfigMapVolumeSource{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[11])", - }, - Items: []v1.KeyToPath{{ - Key: "$(params.array-params[12])", - Path: "$(params.array-params[13])", - }, - }, - }, - Secret: &v1.SecretVolumeSource{ - SecretName: "$(params.array-params[14])", - Items: []v1.KeyToPath{{ - Key: "$(params.array-params[15])", - Path: "$(params.array-params[16])", - }}, - }, - PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ - ClaimName: "$(params.array-params[17])", - }, - Projected: &v1.ProjectedVolumeSource{ - Sources: []v1.VolumeProjection{{ - ConfigMap: &v1.ConfigMapProjection{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[18])", - }, - }, - Secret: &v1.SecretProjection{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - ServiceAccountToken: &v1.ServiceAccountTokenProjection{ - Audience: "$(params.array-params[20])", - }, - }}, - }, - CSI: &v1.CSIVolumeSource{ - NodePublishSecretRef: &v1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, - }, - }, - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), - }, { - name: "workspaces reference invalid", - params: []v1beta1.Param{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Workspaces: []v1beta1.WorkspaceDeclaration{{ - MountPath: "$(params.array-params[3])", - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "sidecar reference invalid", - params: []v1beta1.Param{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Sidecars: []v1beta1.Sidecar{{ - Script: "$(params.array-params[3])", - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, - } - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - ctx := config.ToContext(context.Background(), &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) - err := validateParamArrayIndex(ctx, tc.params, tc.taskspec) - if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { - t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -}