From 082d60c59384b8f48bfa1fc0393562bbec7f5f0e Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 11 May 2022 02:56:41 +0000 Subject: [PATCH] [TEP-0076] Add indexing into array for taskrun params reference This commit provides the indexing into array for params and gated by alpha feature flag. Before this commit we can only refer to the whole array for taskrun params, with this feature we can refer to array's element such as $(params.param-name[0]). --- docs/variables.md | 3 + .../taskruns/alpha/param_array_indexing.yaml | 37 +++ pkg/reconciler/taskrun/resources/apply.go | 15 +- .../taskrun/resources/apply_test.go | 253 +++++++++++++++++- pkg/reconciler/taskrun/taskrun.go | 12 +- pkg/reconciler/taskrun/taskrun_test.go | 4 +- pkg/reconciler/taskrun/validate_resources.go | 213 +++++++++++++++ .../taskrun/validate_resources_test.go | 169 ++++++++++++ 8 files changed, 697 insertions(+), 9 deletions(-) create mode 100644 examples/v1beta1/taskruns/alpha/param_array_indexing.yaml diff --git a/docs/variables.md b/docs/variables.md index c627c0a73c5..f4967dde0b1 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -50,6 +50,9 @@ For instructions on using variable substitutions see the relevant section of [th | `params.` | The value of the parameter at runtime. | | `params['']` | (see above) | | `params[""]` | (see above) | +| `params.[i]` | Get the i-th element of param array. This is alpha feature, set `enable-api-fields` to `alpha` to use it.| +| `params[''][i]` | (see above) | +| `params[""][i]` | (see above) | | `resources.inputs..path` | The path to the input resource's directory. | | `resources.outputs..path` | The path to the output resource's directory. | | `results..path` | The path to the file where the `Task` writes its results data. | diff --git a/examples/v1beta1/taskruns/alpha/param_array_indexing.yaml b/examples/v1beta1/taskruns/alpha/param_array_indexing.yaml new file mode 100644 index 00000000000..505152346e2 --- /dev/null +++ b/examples/v1beta1/taskruns/alpha/param_array_indexing.yaml @@ -0,0 +1,37 @@ +apiVersion: tekton.dev/v1beta1 +kind: TaskRun +metadata: + name: params-array-indexing +spec: + params: + - name: array-to-echo + value: + - "foo" + - "bar" + taskSpec: + params: + - name: array-to-echo + type: array + steps: + # this step should echo "foo" + - name: echo-params-1 + image: bash:3.2 + args: [ + "echo", + "$(params.array-to-echo[0])", + ] + # this step should echo "bar" + - name: echo-params-2 + image: ubuntu + script: | + #!/bin/bash + VALUE=$(params.array-to-echo[1]) + EXPECTED="bar" + diff=$(diff <(printf "%s\n" "${VALUE[@]}") <(printf "%s\n" "${EXPECTED[@]}")) + if [[ -z "$diff" ]]; then + echo "Get expected: ${VALUE}" + exit 0 + else + echo "Want: ${EXPECTED} Got: ${VALUE}" + exit 1 + fi diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 597d827f22e..ab6c5bb4511 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -33,13 +33,14 @@ import ( ) // ApplyParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec -func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1beta1.ParamSpec) *v1beta1.TaskSpec { +func ApplyParameters(ctx context.Context, spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1beta1.ParamSpec) *v1beta1.TaskSpec { // This assumes that the TaskRun inputs have been validated against what the Task requests. // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays // that need to be further processed. stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + cfg := config.FromContextOrDefaults(ctx) patterns := []string{ "params.%s", @@ -58,6 +59,12 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 switch p.Default.Type { case v1beta1.ParamTypeArray: for _, pattern := range patterns { + // array indexing for param is alpha feature + if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + for i := 0; i < len(p.Default.ArrayVal); i++ { + stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Default.ArrayVal[i] + } + } arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal } case v1beta1.ParamTypeObject: @@ -76,6 +83,12 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 switch p.Value.Type { case v1beta1.ParamTypeArray: for _, pattern := range patterns { + // array indexing for param is alpha feature + if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + for i := 0; i < len(p.Value.ArrayVal); i++ { + stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Value.ArrayVal[i] + } + } arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal } case v1beta1.ParamTypeObject: diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 2f8cf80249b..ab4d6634e6e 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -344,6 +344,180 @@ var ( }}, } + simpleTaskSpecArrayIndexing = &v1beta1.TaskSpec{ + Sidecars: []v1beta1.Sidecar{{ + Name: "foo", + Image: `$(params["myimage"][0])`, + Env: []corev1.EnvVar{{ + Name: "foo", + Value: "$(params['FOO'][1])", + }}, + }}, + StepTemplate: &v1beta1.StepTemplate{ + Env: []corev1.EnvVar{{ + Name: "template-var", + Value: `$(params["FOO"][1])`, + }}, + Image: "$(params.myimage[0])", + }, + Steps: []v1beta1.Step{{ + Name: "foo", + Image: "$(params.myimage[0])", + }, { + Name: "baz", + Image: "bat", + WorkingDir: "$(inputs.resources.workspace.path)", + Args: []string{"$(inputs.resources.workspace.url)"}, + }, { + Name: "qux", + Image: "$(params.something[0])", + Args: []string{"$(outputs.resources.imageToUse.url)"}, + }, { + Name: "foo", + Image: `$(params["myimage"][0])`, + }, { + Name: "baz", + Image: "$(params.somethingelse)", + WorkingDir: "$(inputs.resources.workspace.path)", + Args: []string{"$(inputs.resources.workspace.url)"}, + }, { + Name: "qux", + Image: "quux", + Args: []string{"$(outputs.resources.imageToUse.url)"}, + }, { + Name: "foo", + Image: "busybox:$(params.FOO[1])", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(params.FOO[1])", + MountPath: "path/to/$(params.FOO[1])", + SubPath: "sub/$(params.FOO[1])/path", + }}, + }, { + Name: "foo", + Image: "busybox:$(params.FOO[1])", + Env: []corev1.EnvVar{{ + Name: "foo", + Value: "value-$(params.FOO[1])", + }, { + Name: "bar", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "config-$(params.FOO[1])"}, + Key: "config-key-$(params.FOO[1])", + }, + }, + }, { + Name: "baz", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret-$(params.FOO[1])"}, + Key: "secret-key-$(params.FOO[1])", + }, + }, + }}, + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "prefix-0-$(params.FOO[1])", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "config-$(params.FOO[1])"}, + }, + }, { + Prefix: "prefix-1-$(params.FOO[1])", + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret-$(params.FOO[1])"}, + }, + }}, + }, { + Name: "outputs-resources-path-ab", + Image: "$(outputs.resources.imageToUse-ab.path)", + }, { + Name: "outputs-resources-path-re", + Image: "$(outputs.resources.imageToUse-re.path)", + }}, + Volumes: []corev1.Volume{{ + Name: "$(params.FOO[1])", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.FOO[1])", + }, + Items: []corev1.KeyToPath{{ + Key: "$(params.FOO[1])", + Path: "$(params.FOO[1])", + }}, + }, + }, + }, { + Name: "some-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "$(params.FOO[1])", + Items: []corev1.KeyToPath{{ + Key: "$(params.FOO[1])", + Path: "$(params.FOO[1])", + }}, + }, + }, + }, { + Name: "some-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "$(params.FOO[1])", + }, + }, + }, { + Name: "some-projected-volumes", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + ConfigMap: &corev1.ConfigMapProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.FOO[1])", + }, + }, + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.FOO[1])", + }, + }, + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "$(params.FOO[1])", + }, + }}, + }, + }, + }, { + Name: "some-csi", + VolumeSource: corev1.VolumeSource{ + CSI: &corev1.CSIVolumeSource{ + VolumeAttributes: map[string]string{ + "secretProviderClass": "$(params.FOO[1])", + }, + NodePublishSecretRef: &corev1.LocalObjectReference{ + Name: "$(params.FOO[1])", + }, + }, + }, + }}, + Resources: &v1beta1.TaskResources{ + Inputs: []v1beta1.TaskResource{{ + ResourceDeclaration: v1beta1.ResourceDeclaration{ + Name: "workspace", + }, + }}, + Outputs: []v1beta1.TaskResource{{ + ResourceDeclaration: v1beta1.ResourceDeclaration{ + Name: "imageToUse-ab", + TargetPath: "/foo/builtImage", + }, + }, { + ResourceDeclaration: v1beta1.ResourceDeclaration{ + Name: "imageToUse-re", + TargetPath: "foo/builtImage", + }, + }}, + }, + } + gcsTaskSpec = &v1beta1.TaskSpec{ Steps: []v1beta1.Step{{ Name: "foobar", @@ -697,7 +871,7 @@ func TestApplyArrayParameters(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := resources.ApplyParameters(tt.args.ts, tt.args.tr, tt.args.dp...) + got := resources.ApplyParameters(context.Background(), tt.args.ts, tt.args.tr, tt.args.dp...) if d := cmp.Diff(tt.want, got); d != "" { t.Errorf("ApplyParameters() got diff %s", diff.PrintWantGot(d)) } @@ -768,7 +942,80 @@ func TestApplyParameters(t *testing.T) { spec.Sidecars[0].Image = "bar" spec.Sidecars[0].Env[0].Value = "world" }) - got := resources.ApplyParameters(simpleTaskSpec, tr, dp...) + got := resources.ApplyParameters(context.Background(), simpleTaskSpec, tr, dp...) + if d := cmp.Diff(want, got); d != "" { + t.Errorf("ApplyParameters() got diff %s", diff.PrintWantGot(d)) + } +} + +func TestApplyParameters_ArrayIndexing(t *testing.T) { + tr := &v1beta1.TaskRun{ + Spec: v1beta1.TaskRunSpec{ + Params: []v1beta1.Param{{ + Name: "myimage", + Value: *v1beta1.NewArrayOrString("bar", "foo"), + }, { + Name: "FOO", + Value: *v1beta1.NewArrayOrString("hello", "world"), + }}, + }, + } + dp := []v1beta1.ParamSpec{{ + Name: "something", + Default: v1beta1.NewArrayOrString("mydefault", "mydefault2"), + }, { + Name: "somethingelse", + Default: v1beta1.NewArrayOrString(""), + }} + want := applyMutation(simpleTaskSpec, func(spec *v1beta1.TaskSpec) { + spec.StepTemplate.Env[0].Value = "world" + spec.StepTemplate.Image = "bar" + + spec.Steps[0].Image = "bar" + spec.Steps[2].Image = "mydefault" + spec.Steps[3].Image = "bar" + spec.Steps[4].Image = "" + + spec.Steps[6].VolumeMounts[0].Name = "world" + spec.Steps[6].VolumeMounts[0].SubPath = "sub/world/path" + spec.Steps[6].VolumeMounts[0].MountPath = "path/to/world" + spec.Steps[6].Image = "busybox:world" + + spec.Steps[7].Env[0].Value = "value-world" + spec.Steps[7].Env[1].ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name = "config-world" + spec.Steps[7].Env[1].ValueFrom.ConfigMapKeyRef.Key = "config-key-world" + spec.Steps[7].Env[2].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "secret-world" + spec.Steps[7].Env[2].ValueFrom.SecretKeyRef.Key = "secret-key-world" + spec.Steps[7].EnvFrom[0].Prefix = "prefix-0-world" + spec.Steps[7].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "config-world" + spec.Steps[7].EnvFrom[1].Prefix = "prefix-1-world" + spec.Steps[7].EnvFrom[1].SecretRef.LocalObjectReference.Name = "secret-world" + spec.Steps[7].Image = "busybox:world" + spec.Steps[8].Image = "$(outputs.resources.imageToUse-ab.path)" + spec.Steps[9].Image = "$(outputs.resources.imageToUse-re.path)" + + spec.Volumes[0].Name = "world" + spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "world" + spec.Volumes[0].VolumeSource.ConfigMap.Items[0].Key = "world" + spec.Volumes[0].VolumeSource.ConfigMap.Items[0].Path = "world" + spec.Volumes[1].VolumeSource.Secret.SecretName = "world" + spec.Volumes[1].VolumeSource.Secret.Items[0].Key = "world" + spec.Volumes[1].VolumeSource.Secret.Items[0].Path = "world" + spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "world" + spec.Volumes[3].VolumeSource.Projected.Sources[0].ConfigMap.Name = "world" + spec.Volumes[3].VolumeSource.Projected.Sources[0].Secret.Name = "world" + spec.Volumes[3].VolumeSource.Projected.Sources[0].ServiceAccountToken.Audience = "world" + spec.Volumes[4].VolumeSource.CSI.VolumeAttributes["secretProviderClass"] = "world" + spec.Volumes[4].VolumeSource.CSI.NodePublishSecretRef.Name = "world" + + spec.Sidecars[0].Image = "bar" + spec.Sidecars[0].Env[0].Value = "world" + }) + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields + ctx = config.ToContext(ctx, cfg) + got := resources.ApplyParameters(ctx, simpleTaskSpecArrayIndexing, tr, dp...) if d := cmp.Diff(want, got); d != "" { t.Errorf("ApplyParameters() got diff %s", diff.PrintWantGot(d)) } @@ -834,7 +1081,7 @@ func TestApplyObjectParameters(t *testing.T) { spec.Volumes[3].VolumeSource.CSI.VolumeAttributes["secretProviderClass"] = "taskrun-value-for-key1" spec.Volumes[3].VolumeSource.CSI.NodePublishSecretRef.Name = "taskrun-value-for-key1" }) - got := resources.ApplyParameters(objectParamTaskSpec, tr, dp...) + got := resources.ApplyParameters(context.Background(), objectParamTaskSpec, tr, dp...) if d := cmp.Diff(want, got); d != "" { t.Errorf("ApplyParameters() got diff %s", diff.PrintWantGot(d)) } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 90f7beb56e6..9a903d7fdd2 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -372,6 +372,12 @@ 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 { + logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err) + tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) + return nil, nil, controller.NewPermanentError(err) + } + if err := c.updateTaskRunWithDefaultWorkspaces(ctx, tr, taskSpec); err != nil { logger.Errorf("Failed to update taskrun %s with default workspace: %v", tr.Name, err) tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) @@ -418,7 +424,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re logger := logging.FromContext(ctx) recorder := controller.GetEventRecorder(ctx) - ts := updateTaskSpecParamsContextsResults(tr, rtr) + ts := updateTaskSpecParamsContextsResults(ctx, tr, rtr) tr.Status.TaskSpec = ts // Get the TaskRun's Pod if it should have one. Otherwise, create the Pod. @@ -740,14 +746,14 @@ func (c *Reconciler) createPod(ctx context.Context, ts *v1beta1.TaskSpec, tr *v1 return pod, err } -func updateTaskSpecParamsContextsResults(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) *v1beta1.TaskSpec { +func updateTaskSpecParamsContextsResults(ctx context.Context, tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) *v1beta1.TaskSpec { ts := rtr.TaskSpec.DeepCopy() var defaults []v1beta1.ParamSpec if len(ts.Params) > 0 { defaults = append(defaults, ts.Params...) } // Apply parameter substitution from the taskrun. - ts = resources.ApplyParameters(ts, tr, defaults...) + ts = resources.ApplyParameters(ctx, ts, tr, defaults...) // Apply context substitution from the taskrun ts = resources.ApplyContexts(ts, rtr.TaskName, tr) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 878e9a7bf34..7e2b450e69f 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -2297,7 +2297,7 @@ spec: Kind: "Task", TaskSpec: &v1beta1.TaskSpec{Steps: simpleTask.Spec.Steps, Workspaces: simpleTask.Spec.Workspaces}, } - taskSpec := updateTaskSpecParamsContextsResults(taskRun, rtr) + taskSpec := updateTaskSpecParamsContextsResults(context.Background(), taskRun, rtr) pod, err := r.createPod(testAssets.Ctx, taskSpec, taskRun, rtr) if err != nil { @@ -2401,7 +2401,7 @@ spec: TaskSpec: &v1beta1.TaskSpec{Steps: simpleTask.Spec.Steps, Workspaces: simpleTask.Spec.Workspaces}, } - taskSpec := updateTaskSpecParamsContextsResults(taskRun, rtr) + taskSpec := updateTaskSpecParamsContextsResults(context.Background(), taskRun, rtr) _, err := r.createPod(testAssets.Ctx, taskSpec, taskRun, rtr) if err == nil || err.Error() != expectedError { diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index d6c65d6c180..e6ff5e9688f 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -19,17 +19,33 @@ package taskrun import ( "context" "fmt" + "regexp" "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" "k8s.io/apimachinery/pkg/util/sets" "github.com/hashicorp/go-multierror" + corev1 "k8s.io/api/core/v1" ) +const ( + // paramIndex will match all `[int]` for param expression + paramIndexing = `\$\(params(\.[_a-zA-Z0-9.-]+|\[\'[_a-zA-Z0-9.-]+\'\]|\[\"[_a-zA-Z0-9.-]+\"\])\[[0-9]+\]\)` + // paramIndex will match all `[int]` for param expression + paramIndex = `\[[0-9]+\]` +) + +// paramIndexRegex will match all `[int]` for param expression +var paramIndexingRegex = regexp.MustCompile(paramIndexing) + +// paramIndexRegex will match all `[int]` for param expression +var paramIndexRegex = regexp.MustCompile(paramIndex) + func validateResources(requiredResources []v1beta1.TaskResource, providedResources map[string]*resourcev1alpha1.PipelineResource) error { required := make([]string, 0, len(requiredResources)) optional := make([]string, 0, len(requiredResources)) @@ -353,3 +369,200 @@ 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 := paramIndexingRegex.FindAllString(paramReference, -1) + 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 947fbe8be4c..9e50826dfb5 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -18,14 +18,19 @@ package taskrun import ( "context" + "fmt" + "strings" "testing" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/google/go-cmp/cmp" "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" ) func TestValidateResolvedTaskResources_ValidResources(t *testing.T) { @@ -824,3 +829,167 @@ 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.NewArrayOrString("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + 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: "volumes reference invalid", + params: []v1beta1.Param{{ + Name: "array-params", + Value: *v1beta1.NewArrayOrString("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + 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.NewArrayOrString("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + 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.NewArrayOrString("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + 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)) + } + }) + } + +}