From 72388caf15687d6a1158682666606534e9a8022f Mon Sep 17 00:00:00 2001 From: Simon Kaegi Date: Thu, 30 Sep 2021 09:15:53 -0400 Subject: [PATCH] Support for single-quote bracket notation for params This change adds single-quote bracket notation to the work done in #4215. This is consistent with how referencing is done else in K8s in the downwards api. The original patch used the name subscript notation to describe this however the standard name for this approach is bracket notation and updates the doc accordingly. --- docs/tasks.md | 4 +++- docs/variables.md | 4 ++++ pkg/reconciler/pipelinerun/resources/apply.go | 1 + .../pipelinerun/resources/apply_test.go | 19 +++++++++++++++---- .../resources/resultrefresolution.go | 1 + pkg/reconciler/taskrun/resources/apply.go | 2 ++ .../taskrun/resources/apply_test.go | 9 ++++++++- test/pipelinerun_test.go | 6 +++--- 8 files changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/tasks.md b/docs/tasks.md index ab4c36ce92c..a614efa03bc 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -796,8 +796,10 @@ variable values as follows: - To reference a parameter in a `Task`, use the following syntax, where `` is the name of the parameter: ```shell + # dot notation $(params.) - # or subscript form: + # or bracket notation (wrapping with either single or double quotes): + $(params['']) $(params[""]) ``` - To access parameter values from resources, see [variable substitution](resources.md#variable-substitution) diff --git a/docs/variables.md b/docs/variables.md index e831007751d..1c134dfb6fd 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -17,8 +17,10 @@ For instructions on using variable substitutions see the relevant section of [th | Variable | Description | | -------- | ----------- | | `params.` | The value of the parameter at runtime. | +| `params['']` | (see above) | | `params[""]` | (see above) | | `tasks..results.` | The value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) | +| `tasks..results['']` | (see above)) | | `tasks..results[""]` | (see above)) | | `workspaces..bound` | Whether a `Workspace` has been bound or not. "false" if the `Workspace` declaration has `optional: true` and the Workspace binding was omitted by the PipelineRun. | | `context.pipelineRun.name` | The name of the `PipelineRun` that this `Pipeline` is running in. | @@ -34,10 +36,12 @@ For instructions on using variable substitutions see the relevant section of [th | Variable | Description | | -------- | ----------- | | `params.` | The value of the parameter at runtime. | +| `params['']` | (see above) | | `params[""]` | (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. | +| `results[''].path` | (see above) | | `results[""].path` | (see above) | | `workspaces..path` | The path to the mounted `Workspace`. Empty string if an optional `Workspace` has not been provided by the TaskRun. | | `workspaces..bound` | Whether a `Workspace` has been bound or not. "false" if an optional`Workspace` has not been provided by the TaskRun. | diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index a03a473c123..45fa012c4ce 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -38,6 +38,7 @@ func ApplyParameters(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1. patterns := []string{ "params.%s", "params[%q]", + "params['%s']", } // Set all the default stringReplacements diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 07ab36186bb..397f3458bec 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -272,31 +272,42 @@ func TestApplyParameters(t *testing.T) { }}, }, }, { - name: "parameter references with subscript notation and special characters", + name: "parameter references with bracket notation and special characters", original: v1beta1.PipelineSpec{ Params: []v1beta1.ParamSpec{ {Name: "first.param", Type: v1beta1.ParamTypeString, Default: v1beta1.NewArrayOrString("default-value")}, {Name: "second/param", Type: v1beta1.ParamTypeString}, + {Name: "third.param", Type: v1beta1.ParamTypeString, Default: v1beta1.NewArrayOrString("default-value")}, + {Name: "fourth/param", Type: v1beta1.ParamTypeString}, }, Tasks: []v1beta1.PipelineTask{{ Params: []v1beta1.Param{ {Name: "first-task-first-param", Value: *v1beta1.NewArrayOrString(`$(params["first.param"])`)}, {Name: "first-task-second-param", Value: *v1beta1.NewArrayOrString(`$(params["second/param"])`)}, - {Name: "first-task-third-param", Value: *v1beta1.NewArrayOrString("static value")}, + {Name: "first-task-third-param", Value: *v1beta1.NewArrayOrString(`$(params['third.param'])`)}, + {Name: "first-task-fourth-param", Value: *v1beta1.NewArrayOrString(`$(params['fourth/param'])`)}, + {Name: "first-task-fifth-param", Value: *v1beta1.NewArrayOrString("static value")}, }, }}, }, - params: []v1beta1.Param{{Name: "second/param", Value: *v1beta1.NewArrayOrString("second-value")}}, + params: []v1beta1.Param{ + {Name: "second/param", Value: *v1beta1.NewArrayOrString("second-value")}, + {Name: "fourth/param", Value: *v1beta1.NewArrayOrString("fourth-value")}, + }, expected: v1beta1.PipelineSpec{ Params: []v1beta1.ParamSpec{ {Name: "first.param", Type: v1beta1.ParamTypeString, Default: v1beta1.NewArrayOrString("default-value")}, {Name: "second/param", Type: v1beta1.ParamTypeString}, + {Name: "third.param", Type: v1beta1.ParamTypeString, Default: v1beta1.NewArrayOrString("default-value")}, + {Name: "fourth/param", Type: v1beta1.ParamTypeString}, }, Tasks: []v1beta1.PipelineTask{{ Params: []v1beta1.Param{ {Name: "first-task-first-param", Value: *v1beta1.NewArrayOrString("default-value")}, {Name: "first-task-second-param", Value: *v1beta1.NewArrayOrString("second-value")}, - {Name: "first-task-third-param", Value: *v1beta1.NewArrayOrString("static value")}, + {Name: "first-task-third-param", Value: *v1beta1.NewArrayOrString("default-value")}, + {Name: "first-task-fourth-param", Value: *v1beta1.NewArrayOrString("fourth-value")}, + {Name: "first-task-fifth-param", Value: *v1beta1.NewArrayOrString("static value")}, }, }}, }, diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index a33f47eec97..641355415b6 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -196,5 +196,6 @@ func (r *ResolvedResultRef) getReplaceTarget() []string { return []string{ fmt.Sprintf("%s.%s.%s.%s", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), fmt.Sprintf("%s.%s.%s[%q]", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), + fmt.Sprintf("%s.%s.%s['%s']", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), } } diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 190fa965897..fcbe23c13ae 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -44,6 +44,7 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 patterns := []string{ "params.%s", "params[%q]", + "params['%s']", // FIXME(vdemeester) Remove that with deprecating v1beta1 "inputs.params.%s", } @@ -203,6 +204,7 @@ func ApplyTaskResults(spec *v1beta1.TaskSpec) *v1beta1.TaskSpec { patterns := []string{ "results.%s.path", "results[%q].path", + "results['%s'].path", } for _, result := range spec.Results { diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 360aa77810d..6375e74b8b6 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -55,7 +55,7 @@ var ( Image: `$(params["myimage"])`, Env: []corev1.EnvVar{{ Name: "foo", - Value: "$(params.FOO)", + Value: "$(params['FOO'])", }}, }, }}, @@ -1164,12 +1164,19 @@ func TestTaskResults(t *testing.T) { Image: "bash:latest", }, Script: "#!/usr/bin/env bash\ndate | tee $(results.current-date-human-readable.path)", + }, { + Container: corev1.Container{ + Name: "print-date-human-readable-again", + Image: "bash:latest", + }, + Script: "#!/usr/bin/env bash\ndate | tee $(results['current-date-human-readable'].path)", }}, } want := applyMutation(ts, func(spec *v1beta1.TaskSpec) { spec.Steps[0].Script = "#!/usr/bin/env bash\ndate +%s | tee /tekton/results/current.date.unix.timestamp" spec.Steps[0].Args[0] = "/tekton/results/current.date.unix.timestamp" spec.Steps[1].Script = "#!/usr/bin/env bash\ndate | tee /tekton/results/current-date-human-readable" + spec.Steps[2].Script = "#!/usr/bin/env bash\ndate | tee /tekton/results/current-date-human-readable" }) got := resources.ApplyTaskResults(ts) if d := cmp.Diff(want, got); d != "" { diff --git a/test/pipelinerun_test.go b/test/pipelinerun_test.go index 4853b0d2f4d..399c6c35148 100644 --- a/test/pipelinerun_test.go +++ b/test/pipelinerun_test.go @@ -105,14 +105,14 @@ func TestPipelineRun(t *testing.T) { Params: []v1beta1.ParamSpec{{ Name: "the.path", Type: v1beta1.ParamTypeString, }, { - Name: "dest", Type: v1beta1.ParamTypeString, + Name: "the.dest", Type: v1beta1.ParamTypeString, }}, Steps: []v1beta1.Step{{ Container: corev1.Container{ Name: "config-docker", Image: "gcr.io/tekton-releases/dogfooding/skopeo:latest", Command: []string{"skopeo"}, - Args: []string{"copy", `$(params["the.path"])`, "$(params.dest)"}, + Args: []string{"copy", `$(params["the.path"])`, "$(params['the.dest'])"}, }}, }, }, @@ -189,7 +189,7 @@ func TestPipelineRun(t *testing.T) { Name: "config-docker", Image: "gcr.io/tekton-releases/dogfooding/skopeo:latest", Command: []string{"skopeo"}, - Args: []string{"copy", `$(params["the.path"])`, `$(params["dest"])`}, + Args: []string{"copy", `$(params["the.path"])`, `$(params['dest'])`}, }}, }, },