Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 47dda5e

Browse files
YongxuanzhangYongxuanzhang
committedFeb 16, 2023
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. Signed-off-by: Yongxuan Zhang [email protected]
1 parent f3e9fc1 commit 47dda5e

File tree

2 files changed

+101
-140
lines changed

2 files changed

+101
-140
lines changed
 

‎pkg/reconciler/pipelinerun/resources/validate_params.go

+16-72
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import (
2424
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
2525
"github.com/tektoncd/pipeline/pkg/list"
2626
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
27-
"github.com/tektoncd/pipeline/pkg/substitution"
28-
"k8s.io/apimachinery/pkg/util/sets"
2927
)
3028

3129
// 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
9593
return nil
9694
}
9795

98-
arrayParams := extractParamIndexes(p.Params, pr.Spec.Params)
99-
100-
outofBoundParams := sets.String{}
96+
arrayParams := taskrun.ExtractParamArrayLengths(p.Params, pr.Spec.Params)
10197

98+
arrayIndexingParams := []string{}
10299
// collect all the references
103100
for i := range p.Tasks {
104-
findInvalidParamArrayReferences(p.Tasks[i].Params, arrayParams, &outofBoundParams)
101+
extractArrayIndexingReferencesFromParams(p.Tasks[i].Params, arrayParams, &arrayIndexingParams)
105102
if p.Tasks[i].IsMatrixed() {
106-
findInvalidParamArrayReferences(p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams)
103+
extractArrayIndexingReferencesFromParams(p.Tasks[i].Matrix.Params, arrayParams, &arrayIndexingParams)
107104
}
108105
for j := range p.Tasks[i].Workspaces {
109-
findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams)
106+
taskrun.ExtractArrayIndexingParamReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &arrayIndexingParams)
110107
}
111108
for _, wes := range p.Tasks[i].WhenExpressions {
112-
findInvalidParamArrayReference(wes.Input, arrayParams, &outofBoundParams)
109+
taskrun.ExtractArrayIndexingParamReference(wes.Input, arrayParams, &arrayIndexingParams)
113110
for _, v := range wes.Values {
114-
findInvalidParamArrayReference(v, arrayParams, &outofBoundParams)
111+
taskrun.ExtractArrayIndexingParamReference(v, arrayParams, &arrayIndexingParams)
115112
}
116113
}
117114
}
118115

119116
for i := range p.Finally {
120-
findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams, &outofBoundParams)
117+
extractArrayIndexingReferencesFromParams(p.Finally[i].Params, arrayParams, &arrayIndexingParams)
121118
if p.Finally[i].IsMatrixed() {
122-
findInvalidParamArrayReferences(p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams)
119+
extractArrayIndexingReferencesFromParams(p.Finally[i].Matrix.Params, arrayParams, &arrayIndexingParams)
123120
}
124121
for _, wes := range p.Finally[i].WhenExpressions {
125122
for _, v := range wes.Values {
126-
findInvalidParamArrayReference(v, arrayParams, &outofBoundParams)
123+
taskrun.ExtractArrayIndexingParamReference(v, arrayParams, &arrayIndexingParams)
127124
}
128125
}
129126
}
130-
131-
if outofBoundParams.Len() > 0 {
132-
return fmt.Errorf("non-existent param references:%v", outofBoundParams.List())
133-
}
134-
return nil
127+
return taskrun.ValidateOutofBoundArrayParams(arrayIndexingParams, arrayParams)
135128
}
136129

137-
func extractParamIndexes(defaults []v1beta1.ParamSpec, params []v1beta1.Param) map[string]int {
138-
// Collect all array params
139-
arrayParams := make(map[string]int)
140-
141-
patterns := []string{
142-
"$(params.%s)",
143-
"$(params[%q])",
144-
"$(params['%s'])",
145-
}
146-
147-
// Collect array params lengths from defaults
148-
for _, p := range defaults {
149-
if p.Default != nil {
150-
if p.Default.Type == v1beta1.ParamTypeArray {
151-
for _, pattern := range patterns {
152-
for i := 0; i < len(p.Default.ArrayVal); i++ {
153-
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal)
154-
}
155-
}
156-
}
157-
}
158-
}
159-
160-
// Collect array params lengths from pipelinerun or taskrun
161-
for _, p := range params {
162-
if p.Value.Type == v1beta1.ParamTypeArray {
163-
for _, pattern := range patterns {
164-
for i := 0; i < len(p.Value.ArrayVal); i++ {
165-
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal)
166-
}
167-
}
168-
}
169-
}
170-
return arrayParams
171-
}
172-
173-
func findInvalidParamArrayReferences(params []v1beta1.Param, arrayParams map[string]int, outofBoundParams *sets.String) {
130+
// extractArrayIndexingReferencesFromParams get all array indexing references from params
131+
func extractArrayIndexingReferencesFromParams(params []v1beta1.Param, arrayParams map[string]int, arrayIndexingParams *[]string) {
174132
for i := range params {
175-
findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams, outofBoundParams)
133+
taskrun.ExtractArrayIndexingParamReference(params[i].Value.StringVal, arrayParams, arrayIndexingParams)
176134
for _, v := range params[i].Value.ArrayVal {
177-
findInvalidParamArrayReference(v, arrayParams, outofBoundParams)
135+
taskrun.ExtractArrayIndexingParamReference(v, arrayParams, arrayIndexingParams)
178136
}
179137
for _, v := range params[i].Value.ObjectVal {
180-
findInvalidParamArrayReference(v, arrayParams, outofBoundParams)
181-
}
182-
}
183-
}
184-
185-
func findInvalidParamArrayReference(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) {
186-
list := substitution.ExtractParamsExpressions(paramReference)
187-
for _, val := range list {
188-
indexString := substitution.ExtractIndexString(paramReference)
189-
idx, _ := substitution.ExtractIndex(indexString)
190-
v := substitution.TrimArrayIndex(val)
191-
if paramLength, ok := arrayParams[v]; ok {
192-
if idx >= paramLength {
193-
outofBoundParams.Insert(val)
194-
}
138+
taskrun.ExtractArrayIndexingParamReference(v, arrayParams, arrayIndexingParams)
195139
}
196140
}
197141
}

‎pkg/reconciler/taskrun/validate_resources.go

+85-68
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ func missingKeysofObjectResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskR
366366
return findMissingKeys(neededKeys, providedKeys)
367367
}
368368

369+
// validateParamArrayIndex validate if the array indexing param reference target is existent
369370
func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec *v1beta1.TaskSpec) error {
370371
cfg := config.FromContextOrDefaults(ctx)
371372
if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields {
@@ -377,7 +378,25 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec *
377378
defaults = append(defaults, spec.Params...)
378379
}
379380
// Collect all array params
380-
arrayParams := make(map[string]int)
381+
arrayParams := ExtractParamArrayLengths(defaults, params)
382+
383+
// extract all array indexing references
384+
arrayIndexingParams := []string{}
385+
extractStepsParamArrayIndexing(spec.Steps, arrayParams, &arrayIndexingParams)
386+
extractStepsTemplateParamArrayIndexing(spec.StepTemplate, arrayParams, &arrayIndexingParams)
387+
extractVolumesParamArrayIndexing(spec.Volumes, arrayParams, &arrayIndexingParams)
388+
for _, v := range spec.Workspaces {
389+
ExtractArrayIndexingParamReference(v.MountPath, arrayParams, &arrayIndexingParams)
390+
}
391+
extractSidecarsParamArrayIndexing(spec.Sidecars, arrayParams, &arrayIndexingParams)
392+
393+
return ValidateOutofBoundArrayParams(arrayIndexingParams, arrayParams)
394+
}
395+
396+
// ExtractParamArrayLengths extract and return the lengths of all array params
397+
func ExtractParamArrayLengths(defaults []v1beta1.ParamSpec, params []v1beta1.Param) map[string]int {
398+
// Collect all array params
399+
arrayParamsLengths := make(map[string]int)
381400

382401
patterns := []string{
383402
"$(params.%s)",
@@ -391,173 +410,171 @@ func validateParamArrayIndex(ctx context.Context, params []v1beta1.Param, spec *
391410
if p.Default.Type == v1beta1.ParamTypeArray {
392411
for _, pattern := range patterns {
393412
for i := 0; i < len(p.Default.ArrayVal); i++ {
394-
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal)
413+
arrayParamsLengths[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal)
395414
}
396415
}
397416
}
398417
}
399418
}
400419

401-
// Collect array params lengths from pipeline
420+
// Collect array params lengths from params
402421
for _, p := range params {
403422
if p.Value.Type == v1beta1.ParamTypeArray {
404423
for _, pattern := range patterns {
405424
for i := 0; i < len(p.Value.ArrayVal); i++ {
406-
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal)
425+
arrayParamsLengths[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal)
407426
}
408427
}
409428
}
410429
}
430+
return arrayParamsLengths
431+
}
411432

433+
// ValidateOutofBoundArrayParams validates if the array indexing params are out of bound
434+
func ValidateOutofBoundArrayParams(arrayIndexingParams []string, arrayParams map[string]int) error {
412435
outofBoundParams := sets.String{}
413-
414-
// Validate array param in steps fields.
415-
validateStepsParamArrayIndexing(spec.Steps, arrayParams, &outofBoundParams)
416-
417-
// Validate array param in StepTemplate fields.
418-
validateStepsTemplateParamArrayIndexing(spec.StepTemplate, arrayParams, &outofBoundParams)
419-
420-
// Validate array param in build's volumes
421-
validateVolumesParamArrayIndexing(spec.Volumes, arrayParams, &outofBoundParams)
422-
423-
for _, v := range spec.Workspaces {
424-
extractParamIndex(v.MountPath, arrayParams, &outofBoundParams)
436+
for _, val := range arrayIndexingParams {
437+
indexString := substitution.ExtractIndexString(val)
438+
idx, _ := substitution.ExtractIndex(indexString)
439+
v := substitution.TrimArrayIndex(val)
440+
if paramLength, ok := arrayParams[v]; ok {
441+
if idx >= paramLength {
442+
outofBoundParams.Insert(val)
443+
}
444+
}
425445
}
426-
427-
validateSidecarsParamArrayIndexing(spec.Sidecars, arrayParams, &outofBoundParams)
428-
429446
if outofBoundParams.Len() > 0 {
430447
return fmt.Errorf("non-existent param references:%v", outofBoundParams.List())
431448
}
432-
433449
return nil
434450
}
435451

436-
func extractParamIndex(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) {
452+
func ExtractArrayIndexingParamReference(paramReference string, arrayParams map[string]int, arrayIndexingParams *[]string) {
437453
list := substitution.ExtractParamsExpressions(paramReference)
438454
for _, val := range list {
439-
indexString := substitution.ExtractIndexString(paramReference)
440-
idx, _ := substitution.ExtractIndex(indexString)
441-
v := substitution.TrimArrayIndex(val)
442-
if paramLength, ok := arrayParams[v]; ok {
443-
if idx >= paramLength {
444-
outofBoundParams.Insert(val)
445-
}
455+
indexString := substitution.ExtractIndexString(val)
456+
if indexString != "" {
457+
*arrayIndexingParams = append(*arrayIndexingParams, val)
446458
}
447459
}
448460
}
449461

450-
func validateStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, outofBoundParams *sets.String) {
462+
// extractStepsParamArrayIndexing get all array indexing references from steps
463+
func extractStepsParamArrayIndexing(steps []v1beta1.Step, arrayParams map[string]int, arrayIndexingParams *[]string) {
451464
for _, step := range steps {
452-
extractParamIndex(step.Script, arrayParams, outofBoundParams)
465+
ExtractArrayIndexingParamReference(step.Script, arrayParams, arrayIndexingParams)
453466
container := step.ToK8sContainer()
454-
validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams)
467+
extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams)
455468
}
456469
}
457470

458-
func validateStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, outofBoundParams *sets.String) {
471+
// extractStepsTemplateParamArrayIndexing get all array indexing references from StepsTemplate
472+
func extractStepsTemplateParamArrayIndexing(stepTemplate *v1beta1.StepTemplate, arrayParams map[string]int, arrayIndexingParams *[]string) {
459473
if stepTemplate == nil {
460474
return
461475
}
462476
container := stepTemplate.ToK8sContainer()
463-
validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams)
477+
extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams)
464478
}
465479

466-
func validateSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, outofBoundParams *sets.String) {
480+
// extractSidecarsParamArrayIndexing get all array indexing references from sidecars
481+
func extractSidecarsParamArrayIndexing(sidecars []v1beta1.Sidecar, arrayParams map[string]int, arrayIndexingParams *[]string) {
467482
for _, s := range sidecars {
468-
extractParamIndex(s.Script, arrayParams, outofBoundParams)
483+
ExtractArrayIndexingParamReference(s.Script, arrayParams, arrayIndexingParams)
469484
container := s.ToK8sContainer()
470-
validateContainerParamArrayIndexing(container, arrayParams, outofBoundParams)
485+
extractContainerParamArrayIndexing(container, arrayParams, arrayIndexingParams)
471486
}
472487
}
473488

474-
func validateVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, outofBoundParams *sets.String) {
489+
// extractVolumesParamArrayIndexing get all array indexing references from volumes
490+
func extractVolumesParamArrayIndexing(volumes []corev1.Volume, arrayParams map[string]int, arrayIndexingParams *[]string) {
475491
for i, v := range volumes {
476-
extractParamIndex(v.Name, arrayParams, outofBoundParams)
492+
ExtractArrayIndexingParamReference(v.Name, arrayParams, arrayIndexingParams)
477493
if v.VolumeSource.ConfigMap != nil {
478-
extractParamIndex(v.ConfigMap.Name, arrayParams, outofBoundParams)
494+
ExtractArrayIndexingParamReference(v.ConfigMap.Name, arrayParams, arrayIndexingParams)
479495
for _, item := range v.ConfigMap.Items {
480-
extractParamIndex(item.Key, arrayParams, outofBoundParams)
481-
extractParamIndex(item.Path, arrayParams, outofBoundParams)
496+
ExtractArrayIndexingParamReference(item.Key, arrayParams, arrayIndexingParams)
497+
ExtractArrayIndexingParamReference(item.Path, arrayParams, arrayIndexingParams)
482498
}
483499
}
484500
if v.VolumeSource.Secret != nil {
485-
extractParamIndex(v.Secret.SecretName, arrayParams, outofBoundParams)
501+
ExtractArrayIndexingParamReference(v.Secret.SecretName, arrayParams, arrayIndexingParams)
486502
for _, item := range v.Secret.Items {
487-
extractParamIndex(item.Key, arrayParams, outofBoundParams)
488-
extractParamIndex(item.Path, arrayParams, outofBoundParams)
503+
ExtractArrayIndexingParamReference(item.Key, arrayParams, arrayIndexingParams)
504+
ExtractArrayIndexingParamReference(item.Path, arrayParams, arrayIndexingParams)
489505
}
490506
}
491507
if v.PersistentVolumeClaim != nil {
492-
extractParamIndex(v.PersistentVolumeClaim.ClaimName, arrayParams, outofBoundParams)
508+
ExtractArrayIndexingParamReference(v.PersistentVolumeClaim.ClaimName, arrayParams, arrayIndexingParams)
493509
}
494510
if v.Projected != nil {
495511
for _, s := range volumes[i].Projected.Sources {
496512
if s.ConfigMap != nil {
497-
extractParamIndex(s.ConfigMap.Name, arrayParams, outofBoundParams)
513+
ExtractArrayIndexingParamReference(s.ConfigMap.Name, arrayParams, arrayIndexingParams)
498514
}
499515
if s.Secret != nil {
500-
extractParamIndex(s.Secret.Name, arrayParams, outofBoundParams)
516+
ExtractArrayIndexingParamReference(s.Secret.Name, arrayParams, arrayIndexingParams)
501517
}
502518
if s.ServiceAccountToken != nil {
503-
extractParamIndex(s.ServiceAccountToken.Audience, arrayParams, outofBoundParams)
519+
ExtractArrayIndexingParamReference(s.ServiceAccountToken.Audience, arrayParams, arrayIndexingParams)
504520
}
505521
}
506522
}
507523
if v.CSI != nil {
508524
if v.CSI.NodePublishSecretRef != nil {
509-
extractParamIndex(v.CSI.NodePublishSecretRef.Name, arrayParams, outofBoundParams)
525+
ExtractArrayIndexingParamReference(v.CSI.NodePublishSecretRef.Name, arrayParams, arrayIndexingParams)
510526
}
511527
if v.CSI.VolumeAttributes != nil {
512528
for _, value := range v.CSI.VolumeAttributes {
513-
extractParamIndex(value, arrayParams, outofBoundParams)
529+
ExtractArrayIndexingParamReference(value, arrayParams, arrayIndexingParams)
514530
}
515531
}
516532
}
517533
}
518534
}
519535

520-
func validateContainerParamArrayIndexing(c *corev1.Container, arrayParams map[string]int, outofBoundParams *sets.String) {
521-
extractParamIndex(c.Name, arrayParams, outofBoundParams)
522-
extractParamIndex(c.Image, arrayParams, outofBoundParams)
523-
extractParamIndex(string(c.ImagePullPolicy), arrayParams, outofBoundParams)
536+
// extractContainerParamArrayIndexing get all array indexing references from container
537+
func extractContainerParamArrayIndexing(c *corev1.Container, arrayParams map[string]int, arrayIndexingParams *[]string) {
538+
ExtractArrayIndexingParamReference(c.Name, arrayParams, arrayIndexingParams)
539+
ExtractArrayIndexingParamReference(c.Image, arrayParams, arrayIndexingParams)
540+
ExtractArrayIndexingParamReference(string(c.ImagePullPolicy), arrayParams, arrayIndexingParams)
524541

525542
for _, a := range c.Args {
526-
extractParamIndex(a, arrayParams, outofBoundParams)
543+
ExtractArrayIndexingParamReference(a, arrayParams, arrayIndexingParams)
527544
}
528545

529546
for ie, e := range c.Env {
530-
extractParamIndex(e.Value, arrayParams, outofBoundParams)
547+
ExtractArrayIndexingParamReference(e.Value, arrayParams, arrayIndexingParams)
531548
if c.Env[ie].ValueFrom != nil {
532549
if e.ValueFrom.SecretKeyRef != nil {
533-
extractParamIndex(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams)
534-
extractParamIndex(e.ValueFrom.SecretKeyRef.Key, arrayParams, outofBoundParams)
550+
ExtractArrayIndexingParamReference(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams)
551+
ExtractArrayIndexingParamReference(e.ValueFrom.SecretKeyRef.Key, arrayParams, arrayIndexingParams)
535552
}
536553
if e.ValueFrom.ConfigMapKeyRef != nil {
537-
extractParamIndex(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, outofBoundParams)
538-
extractParamIndex(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, outofBoundParams)
554+
ExtractArrayIndexingParamReference(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams)
555+
ExtractArrayIndexingParamReference(e.ValueFrom.ConfigMapKeyRef.Key, arrayParams, arrayIndexingParams)
539556
}
540557
}
541558
}
542559

543560
for _, e := range c.EnvFrom {
544-
extractParamIndex(e.Prefix, arrayParams, outofBoundParams)
561+
ExtractArrayIndexingParamReference(e.Prefix, arrayParams, arrayIndexingParams)
545562
if e.ConfigMapRef != nil {
546-
extractParamIndex(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, outofBoundParams)
563+
ExtractArrayIndexingParamReference(e.ConfigMapRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams)
547564
}
548565
if e.SecretRef != nil {
549-
extractParamIndex(e.SecretRef.LocalObjectReference.Name, arrayParams, outofBoundParams)
566+
ExtractArrayIndexingParamReference(e.SecretRef.LocalObjectReference.Name, arrayParams, arrayIndexingParams)
550567
}
551568
}
552569

553-
extractParamIndex(c.WorkingDir, arrayParams, outofBoundParams)
570+
ExtractArrayIndexingParamReference(c.WorkingDir, arrayParams, arrayIndexingParams)
554571
for _, cc := range c.Command {
555-
extractParamIndex(cc, arrayParams, outofBoundParams)
572+
ExtractArrayIndexingParamReference(cc, arrayParams, arrayIndexingParams)
556573
}
557574

558575
for _, v := range c.VolumeMounts {
559-
extractParamIndex(v.Name, arrayParams, outofBoundParams)
560-
extractParamIndex(v.MountPath, arrayParams, outofBoundParams)
561-
extractParamIndex(v.SubPath, arrayParams, outofBoundParams)
576+
ExtractArrayIndexingParamReference(v.Name, arrayParams, arrayIndexingParams)
577+
ExtractArrayIndexingParamReference(v.MountPath, arrayParams, arrayIndexingParams)
578+
ExtractArrayIndexingParamReference(v.SubPath, arrayParams, arrayIndexingParams)
562579
}
563580
}

0 commit comments

Comments
 (0)
Please sign in to comment.