Skip to content

Commit

Permalink
Allow . in param/result names via subscript.
Browse files Browse the repository at this point in the history 
This change allow folks to use `.` in parameter names (e.g. `dev.mattmoor.my-param`), and reference them via the subscript operator (e.g. `params["dev.mattmoor.my-param"]`) to avoid ambiguity introduced by the mixing of `.`s.

TEP: tektoncd/community#503
  • Loading branch information
@mattmoor
mattmoor committed Sep 5, 2021
1 parent fef771d commit d5a7d16
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 69 deletions.
21 changes: 17 additions & 4 deletions pkg/reconciler/pipelinerun/resources/apply.go
View file
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
View file
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
View file
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
View file
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",
}

// 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

0 comments on commit d5a7d16

Please sign in to comment.