diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index 0b65d3683b9..5bf6559e6a6 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -24,8 +24,6 @@ import ( "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. @@ -95,103 +93,49 @@ func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v return nil } - arrayParams := extractParamIndexes(p.Params, pr.Spec.Params) - - outofBoundParams := sets.String{} + arrayParams := taskrun.ExtractParamArrayLengths(p.Params, pr.Spec.Params) + arrayIndexingParams := []string{} // collect all the references for i := range p.Tasks { - findInvalidParamArrayReferences(p.Tasks[i].Params, arrayParams, &outofBoundParams) + extractArrayIndexingReferencesFromParams(p.Tasks[i].Params, arrayParams, &arrayIndexingParams) if p.Tasks[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams) + extractArrayIndexingReferencesFromParams(p.Tasks[i].Matrix.Params, arrayParams, &arrayIndexingParams) } for j := range p.Tasks[i].Workspaces { - findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &arrayIndexingParams) } for _, wes := range p.Tasks[i].WhenExpressions { - findInvalidParamArrayReference(wes.Input, arrayParams, &outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(wes.Input, arrayParams, &arrayIndexingParams) for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(v, arrayParams, &arrayIndexingParams) } } } for i := range p.Finally { - findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams, &outofBoundParams) + extractArrayIndexingReferencesFromParams(p.Finally[i].Params, arrayParams, &arrayIndexingParams) if p.Finally[i].IsMatrixed() { - findInvalidParamArrayReferences(p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams) + extractArrayIndexingReferencesFromParams(p.Finally[i].Matrix.Params, arrayParams, &arrayIndexingParams) } for _, wes := range p.Finally[i].WhenExpressions { for _, v := range wes.Values { - findInvalidParamArrayReference(v, arrayParams, &outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(v, arrayParams, &arrayIndexingParams) } } } - - if outofBoundParams.Len() > 0 { - return fmt.Errorf("non-existent param references:%v", outofBoundParams.List()) - } - return nil + return taskrun.ValidateOutofBoundArrayParams(arrayIndexingParams, arrayParams) } -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) { +// extractArrayIndexingReferencesFromParams get all array indexing references from params +func extractArrayIndexingReferencesFromParams(params []v1beta1.Param, arrayParams map[string]int, arrayIndexingParams *[]string) { for i := range params { - findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams, outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(params[i].Value.StringVal, arrayParams, arrayIndexingParams) for _, v := range params[i].Value.ArrayVal { - findInvalidParamArrayReference(v, arrayParams, outofBoundParams) + taskrun.ExtractArrayIndexingParamReference(v, arrayParams, arrayIndexingParams) } 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) - } + taskrun.ExtractArrayIndexingParamReference(v, arrayParams, arrayIndexingParams) } } } diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index 685b06dd829..db177726d59 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -647,10 +647,15 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { Operator: selection.In, Values: []string{"$(params.second-param[2])"}, }}, + Matrix: &v1beta1.Matrix{ + Params: []v1beta1.Param{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[4])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[4])")}, + }}, }}, }, 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])]"), + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), }, { name: "parameter references with bracket notation and special characters reference out of bound", original: v1beta1.PipelineSpec{ @@ -698,6 +703,24 @@ 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: "parameter in matrix 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{{ + Matrix: &v1beta1.Matrix{ + 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")}, + }, + }, + }}, + }, + 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])]"), }, } { tt := tt // capture range variable diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index c4aac26e7a6..2b9415686c7 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -366,6 +366,7 @@ func missingKeysofObjectResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskR return findMissingKeys(neededKeys, providedKeys) } +// validateParamArrayIndex validate if the array indexing param reference target is existent func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec *v1beta1.TaskSpec) error { cfg := config.FromContextOrDefaults(ctx) if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { @@ -377,7 +378,25 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec * defaults = append(defaults, spec.Params...) } // Collect all array params - arrayParams := make(map[string]int) + arrayParams := ExtractParamArrayLengths(defaults, params) + + // extract all array indexing references + arrayIndexingParams := []string{} + extractStepsParamArrayIndexing(spec.Steps, arrayParams, &arrayIndexingParams) + extractStepsTemplateParamArrayIndexing(spec.StepTemplate, arrayParams, &arrayIndexingParams) + extractVolumesParamArrayIndexing(spec.Volumes, arrayParams, &arrayIndexingParams) + for _, v := range spec.Workspaces { + ExtractArrayIndexingParamReference(v.MountPath, arrayParams, &arrayIndexingParams) + } + extractSidecarsParamArrayIndexing(spec.Sidecars, arrayParams, &arrayIndexingParams) + + return ValidateOutofBoundArrayParams(arrayIndexingParams, arrayParams) +} + +// ExtractParamArrayLengths extract and return the lengths of all array params +func ExtractParamArrayLengths(defaults []v1beta1.ParamSpec, params []v1beta1.Param) map[string]int { + // Collect all array params + arrayParamsLengths := make(map[string]int) patterns := []string{ "$(params.%s)", @@ -391,173 +410,172 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec * 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) + arrayParamsLengths[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal) } } } } } - // Collect array params lengths from pipeline + // Collect array params lengths from params 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) + arrayParamsLengths[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal) } } } } + return arrayParamsLengths +} +// ValidateOutofBoundArrayParams validates if the array indexing params are out of bound +func ValidateOutofBoundArrayParams(arrayIndexingParams []string, arrayParams map[string]int) error { 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) + for _, val := range arrayIndexingParams { + indexString := substitution.ExtractIndexString(val) + idx, _ := substitution.ExtractIndex(indexString) + v := substitution.TrimArrayIndex(val) + if paramLength, ok := arrayParams[v]; ok { + if idx >= paramLength { + outofBoundParams.Insert(val) + } + } } - - 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) { +// ExtractArrayIndexingParamReference extract the array indexing references +func ExtractArrayIndexingParamReference(paramReference string, arrayParams map[string]int, arrayIndexingParams *[]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) - } + indexString := substitution.ExtractIndexString(val) + if indexString != "" { + *arrayIndexingParams = append(*arrayIndexingParams, val) } } } -func validateStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, outofBoundParams *sets.String) { +// extractStepsParamArrayIndexing get all array indexing references from steps +func extractStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, arrayIndexingParams *[]string) { for _, step := range steps { - extractParamIndex(step.Script, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(step.Script, arrayParams, arrayIndexingParams) container := step.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams) } } -func validateStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, outofBoundParams *sets.String) { +// extractStepsTemplateParamArrayIndexing get all array indexing references from StepsTemplate +func extractStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, arrayIndexingParams *[]string) { if stepTemplate == nil { return } container := stepTemplate.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams) } -func validateSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, outofBoundParams *sets.String) { +// extractSidecarsParamArrayIndexing get all array indexing references from sidecars +func extractSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, arrayIndexingParams *[]string) { for _, s := range sidecars { - extractParamIndex(s.Script, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(s.Script, arrayParams, arrayIndexingParams) container := s.ToK8sContainer() - validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams) + extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams) } } -func validateVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, outofBoundParams *sets.String) { +// extractVolumesParamArrayIndexing get all array indexing references from volumes +func extractVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, arrayIndexingParams *[]string) { for i, v := range volumes { - extractParamIndex(v.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.Name, arrayParams, arrayIndexingParams) if v.VolumeSource.ConfigMap != nil { - extractParamIndex(v.ConfigMap.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.ConfigMap.Name, arrayParams, arrayIndexingParams) for _, item := range v.ConfigMap.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(item.Key, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(item.Path, arrayParams, arrayIndexingParams) } } if v.VolumeSource.Secret != nil { - extractParamIndex(v.Secret.SecretName, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.Secret.SecretName, arrayParams, arrayIndexingParams) for _, item := range v.Secret.Items { - extractParamIndex(item.Key, arrayParams, outofBoundParams) - extractParamIndex(item.Path, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(item.Key, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(item.Path, arrayParams, arrayIndexingParams) } } if v.PersistentVolumeClaim != nil { - extractParamIndex(v.PersistentVolumeClaim.ClaimName, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.PersistentVolumeClaim.ClaimName, arrayParams, arrayIndexingParams) } if v.Projected != nil { for _, s := range volumes[i].Projected.Sources { if s.ConfigMap != nil { - extractParamIndex(s.ConfigMap.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(s.ConfigMap.Name, arrayParams, arrayIndexingParams) } if s.Secret != nil { - extractParamIndex(s.Secret.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(s.Secret.Name, arrayParams, arrayIndexingParams) } if s.ServiceAccountToken != nil { - extractParamIndex(s.ServiceAccountToken.Audience, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(s.ServiceAccountToken.Audience, arrayParams, arrayIndexingParams) } } } if v.CSI != nil { if v.CSI.NodePublishSecretRef != nil { - extractParamIndex(v.CSI.NodePublishSecretRef.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.CSI.NodePublishSecretRef.Name, arrayParams, arrayIndexingParams) } if v.CSI.VolumeAttributes != nil { for _, value := range v.CSI.VolumeAttributes { - extractParamIndex(value, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(value, arrayParams, arrayIndexingParams) } } } } } -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) +// extractContainerParamArrayIndexing get all array indexing references from container +func extractContainerParamArrayIndexing(c *corev1.Container, arrayParams map[string]int, arrayIndexingParams *[]string) { + ExtractArrayIndexingParamReference(c.Name, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(c.Image, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(string(c.ImagePullPolicy), arrayParams, arrayIndexingParams) for _, a := range c.Args { - extractParamIndex(a, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(a, arrayParams, arrayIndexingParams) } for ie, e := range c.Env { - extractParamIndex(e.Value, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(e.Value, arrayParams, arrayIndexingParams) 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) + ExtractArrayIndexingParamReference(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(e.ValueFrom.SecretKeyRef.Key, arrayParams, arrayIndexingParams) } if e.ValueFrom.ConfigMapKeyRef != nil { - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams) - extractParamIndex(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, arrayIndexingParams) } } } for _, e := range c.EnvFrom { - extractParamIndex(e.Prefix, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(e.Prefix, arrayParams, arrayIndexingParams) if e.ConfigMapRef != nil { - extractParamIndex(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams) } if e.SecretRef != nil { - extractParamIndex(e.SecretRef.LocalObjectReference.Name, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(e.SecretRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams) } } - extractParamIndex(c.WorkingDir, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(c.WorkingDir, arrayParams, arrayIndexingParams) for _, cc := range c.Command { - extractParamIndex(cc, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(cc, arrayParams, arrayIndexingParams) } for _, v := range c.VolumeMounts { - extractParamIndex(v.Name, arrayParams, outofBoundParams) - extractParamIndex(v.MountPath, arrayParams, outofBoundParams) - extractParamIndex(v.SubPath, arrayParams, outofBoundParams) + ExtractArrayIndexingParamReference(v.Name, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(v.MountPath, arrayParams, arrayIndexingParams) + ExtractArrayIndexingParamReference(v.SubPath, arrayParams, arrayIndexingParams) } }