diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 799d2abd37d..212fbef6416 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -94,6 +94,9 @@ const ( // ReasonObjectParameterMissKeys indicates that the object param value provided from PipelineRun spec // misses some keys required for the object param declared in Pipeline spec. ReasonObjectParameterMissKeys = "ObjectParameterMissKeys" + // ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate + // or the array is out of bound. + ReasonParamArrayIndexingInvalid = "ParamArrayIndexingInvalid" // ReasonCouldntGetTask indicates that the reason for the failure status is that the // associated Pipeline's Tasks couldn't all be retrieved ReasonCouldntGetTask = "CouldntGetTask" @@ -509,8 +512,8 @@ 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 { // This Run has failed, so we need to mark it as failed and stop reconciling it - pr.Status.MarkFailed(ReasonObjectParameterMissKeys, - "PipelineRun %s/%s parameters is missing object keys required by Pipeline %s/%s's parameters: %s", + pr.Status.MarkFailed(ReasonParamArrayIndexingInvalid, + "PipelineRun %s/%s array indexing params fail validation by Pipeline %s/%s's parameters: %s", pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err) return controller.NewPermanentError(err) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index e619df1aee9..40e0cd51f45 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1061,6 +1061,22 @@ spec: name: a-task-that-needs-array-params `, v1beta1.ParamTypeArray)), parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: a-pipeline-with-array-indexing-params + namespace: foo +spec: + params: + - name: some-param + type: %s + tasks: + - name: some-task + taskRef: + name: a-task-that-needs-array-params + params: + - name: param + value: "$(params.some-param[2])" +`, v1beta1.ParamTypeArray)), + parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` metadata: name: a-pipeline-with-object-params namespace: foo @@ -1225,6 +1241,27 @@ spec: "Normal Started", "Warning Failed PipelineRun foo/pipeline-missing-object-param-keys parameters is missing object keys required by Pipeline foo/a-pipeline-with-object-params's parameters: PipelineRun missing object keys for parameters", }, + }, { + name: "invalid-pipeline-array-index-out-of-bound", + pipelineRun: parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pipeline-param-array-out-of-bound + namespace: foo +spec: + pipelineRef: + name: a-pipeline-with-array-indexing-params + params: + - name: some-param + value: + - "a" + - "b" +`), + reason: ReasonParamArrayIndexingInvalid, + permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-param-array-out-of-bound array indexing params fail validation by Pipeline foo/a-pipeline-with-array-indexing-params's parameters: non-existent param references:[$(params.some-param[2]", + }, }, { name: "invalid-embedded-pipeline-resources-bot-bound-shd-stop-reconciling", pipelineRun: parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index 363d18281c9..5c2da11fe28 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -20,11 +20,9 @@ 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" ) @@ -91,40 +89,51 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip // ValidateParamArrayIndex validate if the array indexing param reference target is existent func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error { - cfg := config.FromContextOrDefaults(ctx) - if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { - 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 err := findInvalidParamArrayReferences(ctx, p.Tasks[i].Params, arrayParams, &outofBoundParams); err != nil { + return err + } if p.Tasks[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams) + if err := findInvalidParamArrayReferences(ctx, p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams); err != nil { + return err + } } for j := range p.Tasks[i].Workspaces { - findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams); err != nil { + return err + } } for _, wes := range p.Tasks[i].WhenExpressions { - findInvalidParamArrayReference(wes.Input, arrayParams, &outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, wes.Input, arrayParams, &outofBoundParams); err != nil { + return err + } for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, v, arrayParams, &outofBoundParams); err != nil { + return err + } } } } for i := range p.Finally { - findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams, &outofBoundParams) + if err := findInvalidParamArrayReferences(ctx, p.Finally[i].Params, arrayParams, &outofBoundParams); err != nil { + return err + } if p.Finally[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams) + if err := findInvalidParamArrayReferences(ctx, p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams); err != nil { + return err + } } for _, wes := range p.Finally[i].WhenExpressions { for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, v, arrayParams, &outofBoundParams); err != nil { + return err + } } } } @@ -171,28 +180,22 @@ func extractParamIndexes(defaults []v1beta1.ParamSpec, params []v1beta1.Param) m return arrayParams } -func findInvalidParamArrayReferences(params []v1beta1.Param, arrayParams map[string]int, outofBoundParams *sets.String) { +// findInvalidParamArrayReferences validates all the params' array indexing references and check if they are valid +func findInvalidParamArrayReferences(ctx context.Context, params []v1beta1.Param, arrayParams map[string]int, outofBoundParams *sets.String) error { for i := range params { - findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams, outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, params[i].Value.StringVal, arrayParams, outofBoundParams); err != nil { + return err + } for _, v := range params[i].Value.ArrayVal { - findInvalidParamArrayReference(v, arrayParams, outofBoundParams) + if err := taskrun.FindInvalidParamArrayReference(ctx, v, arrayParams, outofBoundParams); err != nil { + return err + } } 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) + if err := taskrun.FindInvalidParamArrayReference(ctx, v, arrayParams, outofBoundParams); err != nil { + return err } } } + return nil } diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index 27ae5f48479..fa8d988fe3a 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -506,15 +506,12 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { } func TestValidateParamArrayIndex_invalid(t *testing.T) { - ctx := context.Background() - cfg := config.FromContextOrDefaults(ctx) - cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields - ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original v1beta1.PipelineSpec - params []v1beta1.Param - expected error + name string + original v1beta1.PipelineSpec + params []v1beta1.Param + apifields string + expected error }{{ name: "single parameter reference out of bound", original: v1beta1.PipelineSpec{ @@ -698,11 +695,38 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { }, 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])]"), + }, { + name: "alpha gate not enabled", + 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")}}, + apifields: "stable", + expected: fmt.Errorf(`indexing into array param %s requires "enable-api-fields" feature gate to be "alpha"`, "$(params.first-param[2])"), }, } { tt := tt // capture range variable t.Run(tt.name, func(t *testing.T) { - t.Parallel() + ctx := context.Background() + if tt.apifields != "stable" { + ctx = config.ToContext(ctx, &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) + } run := &v1beta1.PipelineRun{ Spec: v1beta1.PipelineRunSpec{ Params: tt.params, diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index 31d4ecc2382..4570104afa3 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -367,11 +367,6 @@ func missingKeysofObjectResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskR } 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...) @@ -412,20 +407,26 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec * outofBoundParams := sets.String{} // Validate array param in steps fields. - validateStepsParamArrayIndexing(spec.Steps, arrayParams, &outofBoundParams) - + if err := validateStepsParamArrayIndexing(ctx, spec.Steps, arrayParams, &outofBoundParams); err != nil { + return err + } // Validate array param in StepTemplate fields. - validateStepsTemplateParamArrayIndexing(spec.StepTemplate, arrayParams, &outofBoundParams) - + if err := validateStepsTemplateParamArrayIndexing(ctx, spec.StepTemplate, arrayParams, &outofBoundParams); err != nil { + return err + } // Validate array param in build's volumes - validateVolumesParamArrayIndexing(spec.Volumes, arrayParams, &outofBoundParams) - + if err := validateVolumesParamArrayIndexing(ctx, spec.Volumes, arrayParams, &outofBoundParams); err != nil { + return err + } for _, v := range spec.Workspaces { - extractParamIndex(v.MountPath, arrayParams, &outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.MountPath, arrayParams, &outofBoundParams); err != nil { + return err + } } - validateSidecarsParamArrayIndexing(spec.Sidecars, arrayParams, &outofBoundParams) - + if err := validateSidecarsParamArrayIndexing(ctx, spec.Sidecars, arrayParams, &outofBoundParams); err != nil { + return err + } if outofBoundParams.Len() > 0 { return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) } @@ -433,10 +434,18 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec * return nil } -func extractParamIndex(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) { +// FindInvalidParamArrayReference checks if the alpha feature gate is enabled when array +// indexing is used. And also find out of bound params +func FindInvalidParamArrayReference(ctx context.Context, paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) error { list := substitution.ExtractParamsExpressions(paramReference) for _, val := range list { indexString := substitution.ExtractIndexString(paramReference) + if indexString != "" { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { + return fmt.Errorf(`indexing into array param %s requires "enable-api-fields" feature gate to be "alpha"`, paramReference) + } + } idx, _ := substitution.ExtractIndex(indexString) v := substitution.TrimArrayIndex(val) if paramLength, ok := arrayParams[v]; ok { @@ -445,119 +454,197 @@ func extractParamIndex(paramReference string, arrayParams map[string]int, outofB } } } + return nil } -func validateStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, outofBoundParams *sets.String) { +// validateStepsParamArrayIndexing validates array indexing params references in steps +func validateStepsParamArrayIndexing(ctx context.Context, steps []v1beta1.Step, arrayParams map[string]int, outofBoundParams *sets.String) error { for _, step := range steps { - extractParamIndex(step.Script, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, step.Script, arrayParams, outofBoundParams); err != nil { + return err + } container := step.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + if err := validateContainerParamArrayIndexing(ctx, container, arrayParams, outofBoundParams); err != nil { + return err + } } + return nil } -func validateStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, outofBoundParams *sets.String) { +// validateStepsTemplateParamArrayIndexing validates array indexing params references in steps template +func validateStepsTemplateParamArrayIndexing(ctx context.Context, stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, outofBoundParams *sets.String) error { if stepTemplate == nil { - return + return nil } container := stepTemplate.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + return validateContainerParamArrayIndexing(ctx, container, arrayParams, outofBoundParams) } -func validateSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, outofBoundParams *sets.String) { +// validateSidecarsParamArrayIndexing validates array indexing params references in sidecars +func validateSidecarsParamArrayIndexing(ctx context.Context, sidecars []v1beta1.Sidecar, arrayParams map[string]int, outofBoundParams *sets.String) error { for _, s := range sidecars { - extractParamIndex(s.Script, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, s.Script, arrayParams, outofBoundParams); err != nil { + return err + } container := s.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + if err := validateContainerParamArrayIndexing(ctx, container, arrayParams, outofBoundParams); err != nil { + return err + } } + return nil } -func validateVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, outofBoundParams *sets.String) { +// validateVolumesParamArrayIndexing validates array indexing params references in volumes +func validateVolumesParamArrayIndexing(ctx context.Context, volumes []corev1.Volume, arrayParams map[string]int, outofBoundParams *sets.String) error { for i, v := range volumes { - extractParamIndex(v.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.Name, arrayParams, outofBoundParams); err != nil { + return err + } if v.VolumeSource.ConfigMap != nil { - extractParamIndex(v.ConfigMap.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.ConfigMap.Name, arrayParams, outofBoundParams); err != nil { + return err + } for _, item := range v.ConfigMap.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, item.Key, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, item.Path, arrayParams, outofBoundParams); err != nil { + return err + } } } if v.VolumeSource.Secret != nil { - extractParamIndex(v.Secret.SecretName, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.Secret.SecretName, arrayParams, outofBoundParams); err != nil { + return err + } for _, item := range v.Secret.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, item.Key, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, item.Path, arrayParams, outofBoundParams); err != nil { + return err + } } } if v.PersistentVolumeClaim != nil { - extractParamIndex(v.PersistentVolumeClaim.ClaimName, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.PersistentVolumeClaim.ClaimName, arrayParams, outofBoundParams); err != nil { + return err + } } if v.Projected != nil { for _, s := range volumes[i].Projected.Sources { if s.ConfigMap != nil { - extractParamIndex(s.ConfigMap.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, s.ConfigMap.Name, arrayParams, outofBoundParams); err != nil { + return err + } } if s.Secret != nil { - extractParamIndex(s.Secret.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, s.Secret.Name, arrayParams, outofBoundParams); err != nil { + return err + } } if s.ServiceAccountToken != nil { - extractParamIndex(s.ServiceAccountToken.Audience, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, s.ServiceAccountToken.Audience, arrayParams, outofBoundParams); err != nil { + return err + } } } } if v.CSI != nil { if v.CSI.NodePublishSecretRef != nil { - extractParamIndex(v.CSI.NodePublishSecretRef.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.CSI.NodePublishSecretRef.Name, arrayParams, outofBoundParams); err != nil { + return err + } } if v.CSI.VolumeAttributes != nil { for _, value := range v.CSI.VolumeAttributes { - extractParamIndex(value, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, value, arrayParams, outofBoundParams); err != nil { + return err + } } } } } + return nil } -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) +// validateContainerParamArrayIndexing validates array indexing params references in container +func validateContainerParamArrayIndexing(ctx context.Context, c *corev1.Container, arrayParams map[string]int, outofBoundParams *sets.String) error { + if err := FindInvalidParamArrayReference(ctx, c.Name, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, c.Image, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, string(c.ImagePullPolicy), arrayParams, outofBoundParams); err != nil { + return err + } for _, a := range c.Args { - extractParamIndex(a, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, a, arrayParams, outofBoundParams); err != nil { + return err + } } for ie, e := range c.Env { - extractParamIndex(e.Value, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, e.Value, arrayParams, outofBoundParams); err != nil { + return err + } 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 err := FindInvalidParamArrayReference(ctx, e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, e.ValueFrom.SecretKeyRef.Key, arrayParams, outofBoundParams); err != nil { + return err + } } if e.ValueFrom.ConfigMapKeyRef != nil { - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, outofBoundParams); err != nil { + return err + } } } } for _, e := range c.EnvFrom { - extractParamIndex(e.Prefix, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, e.Prefix, arrayParams, outofBoundParams); err != nil { + return err + } if e.ConfigMapRef != nil { - extractParamIndex(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, e.ConfigMapRef.LocalObjectReference.Name, arrayParams, outofBoundParams); err != nil { + return err + } } if e.SecretRef != nil { - extractParamIndex(e.SecretRef.LocalObjectReference.Name, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, e.SecretRef.LocalObjectReference.Name, arrayParams, outofBoundParams); err != nil { + return err + } } } - extractParamIndex(c.WorkingDir, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, c.WorkingDir, arrayParams, outofBoundParams); err != nil { + return err + } for _, cc := range c.Command { - extractParamIndex(cc, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, cc, arrayParams, outofBoundParams); err != nil { + return err + } } for _, v := range c.VolumeMounts { - extractParamIndex(v.Name, arrayParams, outofBoundParams) - extractParamIndex(v.MountPath, arrayParams, outofBoundParams) - extractParamIndex(v.SubPath, arrayParams, outofBoundParams) + if err := FindInvalidParamArrayReference(ctx, v.Name, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, v.MountPath, arrayParams, outofBoundParams); err != nil { + return err + } + if err := FindInvalidParamArrayReference(ctx, v.SubPath, arrayParams, outofBoundParams); err != nil { + return err + } } + return nil } diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index 19db70401af..682533cef62 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -952,6 +952,7 @@ func TestValidateParamArrayIndex(t *testing.T) { name string params []v1beta1.Param taskspec *v1beta1.TaskSpec + apifields string expectedError error }{{ name: "steps reference invalid", @@ -1120,11 +1121,32 @@ func TestValidateParamArrayIndex(t *testing.T) { }, }, expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "alpha gate not enabled", + 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])", + }, + }, + }, + apifields: config.StableAPIFields, + expectedError: fmt.Errorf(`indexing into array param %s requires "enable-api-fields" feature gate to be "alpha"`, "$(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"}}) + ctx := context.Background() + if tc.apifields != config.StableAPIFields { + ctx = config.ToContext(ctx, &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))