Skip to content

Commit

Permalink
Apply replacements in tasks and finally tasks separately
Browse files Browse the repository at this point in the history
Previously, we created a slice containing both tasks and finally tasks
then applied replacements across all of them. However, the variable
replacements we make in conditions and when expressions in tasks
don't apply to finally tasks. Moreover, this joined slice caused strange
memory allocation side effects when finally tasks were present vs when
they were missing.
  • Loading branch information
jerop authored and tekton-robot committed Sep 17, 2020
1 parent 97129d0 commit 143f812
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
17 changes: 9 additions & 8 deletions pkg/reconciler/pipelinerun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,17 @@ func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResul
func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string, arrayReplacements map[string][]string) *v1beta1.PipelineSpec {
p = p.DeepCopy()

// replace param values for both DAG and final tasks
tasks := append(p.Tasks, p.Finally...)

for i := range tasks {
tasks[i].Params = replaceParamValues(tasks[i].Params, replacements, arrayReplacements)
for j := range tasks[i].Conditions {
c := tasks[i].Conditions[j]
for i := range p.Tasks {
p.Tasks[i].Params = replaceParamValues(p.Tasks[i].Params, replacements, arrayReplacements)
for j := range p.Tasks[i].Conditions {
c := p.Tasks[i].Conditions[j]
c.Params = replaceParamValues(c.Params, replacements, arrayReplacements)
}
tasks[i].WhenExpressions = tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements)
p.Tasks[i].WhenExpressions = p.Tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements)
}

for i := range p.Finally {
p.Finally[i].Params = replaceParamValues(p.Finally[i].Params, replacements, arrayReplacements)
}

return p
Expand Down
29 changes: 29 additions & 0 deletions pkg/reconciler/pipelinerun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,35 @@ func TestApplyParameters(t *testing.T) {
tb.PipelineTaskParam("final-task-first-param", "default-value"),
tb.PipelineTaskParam("final-task-second-param", "second-value"),
))),
}, {
name: "parameter evaluation with both tasks and final tasks",
original: tb.Pipeline("test-pipeline",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1beta1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineParamSpec("second-param", v1beta1.ParamTypeString),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskParam("first-task-first-param", "$(params.first-param)"),
),
tb.PipelineTask("first-task-2", "first-task",
tb.PipelineTaskWhenExpression("$(params.first-param)", selection.In, []string{"$(params.second-param)"})),
tb.FinalPipelineTask("final-task-1", "final-task",
tb.PipelineTaskParam("final-task-second-param", "$(params.second-param)"),
))),
run: tb.PipelineRun("test-pipeline-run",
tb.PipelineRunSpec("test-pipeline",
tb.PipelineRunParam("second-param", "second-value"))),
expected: tb.Pipeline("test-pipeline",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1beta1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineParamSpec("second-param", v1beta1.ParamTypeString),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskParam("first-task-first-param", "default-value"),
),
tb.PipelineTask("first-task-2", "first-task",
tb.PipelineTaskWhenExpression("default-value", selection.In, []string{"second-value"})),
tb.FinalPipelineTask("final-task-1", "final-task",
tb.PipelineTaskParam("final-task-second-param", "second-value"),
))),
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 143f812

Please sign in to comment.