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)) - } - }) - } -}