From 978c01716e34113797696de7db5b804e950b4620 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Thu, 16 Feb 2023 02:58:00 +0000 Subject: [PATCH] refactor param array indexing validation This commit refactors param array indexing validation to extract all references first, then validate if references are out of bound. Previously we traverse all references and collect invalid references there. The benefit of this refactoring is that it would be convenient for other validation to reuse these references. This commit also moves some shared functions into a separate pkg. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- pkg/apis/pipeline/v1beta1/param_types.go | 214 ++++++++++ pkg/apis/pipeline/v1beta1/pipeline_types.go | 8 +- .../pipeline/v1beta1/pipeline_validation.go | 48 +++ .../v1beta1/pipeline_validation_test.go | 383 +++++++++++++++++ pkg/apis/pipeline/v1beta1/task_types.go | 2 +- pkg/apis/pipeline/v1beta1/task_validation.go | 36 ++ .../pipeline/v1beta1/task_validation_test.go | 197 +++++++++ pkg/reconciler/pipelinerun/pipelinerun.go | 2 +- .../pipelinerun/resources/validate_params.go | 113 +---- .../resources/validate_params_test.go | 403 +----------------- pkg/reconciler/taskrun/taskrun.go | 2 +- pkg/reconciler/taskrun/validate_resources.go | 202 +-------- .../taskrun/validate_resources_test.go | 203 +-------- 13 files changed, 892 insertions(+), 921 deletions(-) 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..468660117f0 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..463fcfef8e5 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" @@ -29,6 +30,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) @@ -1842,3 +1844,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: []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 := 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/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)) - } - }) - } -}