Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow . in param/result names via subscript. #4215

Merged
merged 2 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,10 @@ You can specify parameters, such as compilation flags or artifact names, that yo

Parameter names:

- Must only contain alphanumeric characters, hyphens (`-`), and underscores (`_`).
- Must only contain alphanumeric characters, hyphens (`-`), underscores (`_`), and dots (`.`).
- Must begin with a letter or an underscore (`_`).

For example, `fooIs-Bar_` is a valid parameter name, but `barIsBa$` or `0banana` are not.
For example, `foo.Is-Bar_` is a valid parameter name, but `barIsBa$` or `0banana` are not.

Each declared parameter has a `type` field, which can be set to either `array` or `string`. `array` is useful in cases where the number
of compilation flags being supplied to a task varies throughout the `Task's` execution. If not specified, the `type` field defaults to
Expand All @@ -432,7 +432,14 @@ spec:
steps:
- name: build
image: my-builder
args: ["build", "$(params.flags[*])", "url=$(params.someURL)"]
args: [
"build",
"$(params.flags[*])",
# It would be equivalent to use $(params["someURL"]) here,
# which is necessary when the parameter name contains '.'
# characters (e.g. `$(params["some.other.URL"])`)
'url=$(params.someURL)',
]
```

The following `TaskRun` supplies a dynamic number of strings within the `flags` parameter:
Expand Down Expand Up @@ -744,6 +751,8 @@ variable values as follows:
- To reference a parameter in a `Task`, use the following syntax, where `<name>` is the name of the parameter:
```shell
$(params.<name>)
# or subscript form:
$(params["<name>"])
```
- To access parameter values from resources, see [variable substitution](resources.md#variable-substitution)

Expand Down
4 changes: 4 additions & 0 deletions docs/variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ For instructions on using variable substitutions see the relevant section of [th
| Variable | Description |
| -------- | ----------- |
| `params.<param name>` | The value of the parameter at runtime. |
| `params["<param name>"]` | (see above) |
| `tasks.<taskName>.results.<resultName>` | The value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) |
| `tasks.<taskName>.results["<resultName>"]` | (see above)) |
| `workspaces.<workspaceName>.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. |
| `context.pipelineRun.namespace` | The namespace of the `PipelineRun` that this `Pipeline` is running in. |
Expand All @@ -32,9 +34,11 @@ For instructions on using variable substitutions see the relevant section of [th
| Variable | Description |
| -------- | ----------- |
| `params.<param name>` | The value of the parameter at runtime. |
| `params["<param name>"]` | (see above) |
| `resources.inputs.<resourceName>.path` | The path to the input resource's directory. |
| `resources.outputs.<resourceName>.path` | The path to the output resource's directory. |
| `results.<resultName>.path` | The path to the file where the `Task` writes its results data. |
| `results["<resultName>"].path` | (see above) |
| `workspaces.<workspaceName>.path` | The path to the mounted `Workspace`. Empty string if an optional `Workspace` has not been provided by the TaskRun. |
| `workspaces.<workspaceName>.bound` | Whether a `Workspace` has been bound or not. "false" if an optional`Workspace` has not been provided by the TaskRun. |
| `workspaces.<workspaceName>.claim` | The name of the `PersistentVolumeClaim` specified as a volume source for the `Workspace`. Empty string for other volume types. |
Expand Down
21 changes: 17 additions & 4 deletions pkg/reconciler/pipelinerun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,35 @@ func ApplyParameters(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1.
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}

patterns := []string{
"params.%s",
"params[%q]",
}

// Set all the default stringReplacements
for _, p := range p.Params {
if p.Default != nil {
if p.Default.Type == v1beta1.ParamTypeString {
stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.StringVal
for _, pattern := range patterns {
stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.StringVal
}
} else {
arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.ArrayVal
for _, pattern := range patterns {
arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal
}
}
}
}
// Set and overwrite params with the ones from the PipelineRun
for _, p := range pr.Spec.Params {
if p.Value.Type == v1beta1.ParamTypeString {
stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.StringVal
for _, pattern := range patterns {
stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.StringVal
}
} else {
arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.ArrayVal
for _, pattern := range patterns {
arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal
}
}
}

Expand Down
33 changes: 31 additions & 2 deletions pkg/reconciler/pipelinerun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,35 @@ func TestApplyParameters(t *testing.T) {
}},
}},
},
}, {
name: "parameter references with subscript 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},
},
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")},
},
}},
},
params: []v1beta1.Param{{Name: "second/param", Value: *v1beta1.NewArrayOrString("second-value")}},
expected: v1beta1.PipelineSpec{
Params: []v1beta1.ParamSpec{
{Name: "first.param", Type: v1beta1.ParamTypeString, Default: v1beta1.NewArrayOrString("default-value")},
{Name: "second/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")},
},
}},
},
}} {
tt := tt // capture range variable
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -300,7 +329,7 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) {
Value: *v1beta1.NewArrayOrString("aResultValue"),
ResultReference: v1beta1.ResultRef{
PipelineTask: "aTask",
Result: "aResult",
Result: "a.Result",
},
FromTaskRun: "aTaskRun",
}},
Expand All @@ -310,7 +339,7 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) {
TaskRef: &v1beta1.TaskRef{Name: "bTask"},
Params: []v1beta1.Param{{
Name: "bParam",
Value: *v1beta1.NewArrayOrString("$(tasks.aTask.results.aResult)"),
Value: *v1beta1.NewArrayOrString(`$(tasks.aTask.results["a.Result"])`),
}},
},
}},
Expand Down
12 changes: 8 additions & 4 deletions pkg/reconciler/pipelinerun/resources/resultrefresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,16 @@ func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultR
func (rs ResolvedResultRefs) getStringReplacements() map[string]string {
replacements := map[string]string{}
for _, r := range rs {
replaceTarget := r.getReplaceTarget()
replacements[replaceTarget] = r.Value.StringVal
for _, target := range r.getReplaceTarget() {
replacements[target] = r.Value.StringVal
}
}
return replacements
}

func (r *ResolvedResultRef) getReplaceTarget() string {
return fmt.Sprintf("%s.%s.%s.%s", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result)
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),
}
}
40 changes: 27 additions & 13 deletions pkg/reconciler/taskrun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,37 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}

patterns := []string{
"params.%s",
"params[%q]",
// FIXME(vdemeester) Remove that with deprecating v1beta1
"inputs.params.%s",
Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I wrote that FIXME ? I think I meant "with deprecating v1alpha1", maybe ? 🤔 👴🏼

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just moved it 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah.. I saw that 😝 I am confused by my own comment 😹 damn…

}

// Set all the default stringReplacements
for _, p := range defaults {
if p.Default != nil {
if p.Default.Type == v1beta1.ParamTypeString {
stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.StringVal
// FIXME(vdemeester) Remove that with deprecating v1beta1
stringReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Default.StringVal
for _, pattern := range patterns {
stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.StringVal
}
} else {
arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.ArrayVal
// FIXME(vdemeester) Remove that with deprecating v1beta1
arrayReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Default.ArrayVal
for _, pattern := range patterns {
arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal
}
}
}
}
// Set and overwrite params with the ones from the TaskRun
for _, p := range tr.Spec.Params {
if p.Value.Type == v1beta1.ParamTypeString {
stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.StringVal
// FIXME(vdemeester) Remove that with deprecating v1beta1
stringReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value.StringVal
for _, pattern := range patterns {
stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.StringVal
}
} else {
arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.ArrayVal
// FIXME(vdemeester) Remove that with deprecating v1beta1
arrayReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value.ArrayVal
for _, pattern := range patterns {
arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal
}
}
}
return ApplyReplacements(spec, stringReplacements, arrayReplacements)
Expand Down Expand Up @@ -193,8 +200,15 @@ func applyWorkspaceMountPath(variable string, spec *v1beta1.TaskSpec, declaratio
func ApplyTaskResults(spec *v1beta1.TaskSpec) *v1beta1.TaskSpec {
stringReplacements := map[string]string{}

patterns := []string{
"results.%s.path",
"results[%q].path",
}

for _, result := range spec.Results {
stringReplacements[fmt.Sprintf("results.%s.path", result.Name)] = filepath.Join(pipeline.DefaultResultPath, result.Name)
for _, pattern := range patterns {
stringReplacements[fmt.Sprintf(pattern, result.Name)] = filepath.Join(pipeline.DefaultResultPath, result.Name)
}
}
return ApplyReplacements(spec, stringReplacements, map[string][]string{})
}
Expand Down
Loading