Skip to content

Commit ef5a83c

Browse files
wlynchtekton-robot
wlynch
authored and
tekton-robot
committed
Implicit params: don't apply PipelineSpec params to TaskRefs.
Previously we were applying the params to all PipelineTasks, including refs. This is incorrect behavior since the parameters should only be propagated to things within the same document.
1 parent dd3d0c2 commit ef5a83c

File tree

2 files changed

+110
-37
lines changed

2 files changed

+110
-37
lines changed

‎pkg/apis/pipeline/v1beta1/pipeline_defaults.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,32 +41,35 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
4141
}
4242
for i, pt := range ps.Tasks {
4343
ctx := ctx // Ensure local scoping per Task
44-
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
45-
ctx = addContextParams(ctx, pt.Params)
46-
ps.Tasks[i].Params = getContextParams(ctx, pt.Params...)
47-
}
44+
4845
if pt.TaskRef != nil {
4946
if pt.TaskRef.Kind == "" {
5047
pt.TaskRef.Kind = NamespacedTaskKind
5148
}
5249
}
5350
if pt.TaskSpec != nil {
51+
// Only propagate param context to the spec - ref params should
52+
// still be explicitly set.
53+
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
54+
ctx = addContextParams(ctx, pt.Params)
55+
ps.Tasks[i].Params = getContextParams(ctx, pt.Params...)
56+
}
5457
pt.TaskSpec.SetDefaults(ctx)
5558
}
5659
}
5760

5861
for i, ft := range ps.Finally {
5962
ctx := ctx // Ensure local scoping per Task
60-
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
61-
ctx = addContextParams(ctx, ft.Params)
62-
ps.Finally[i].Params = getContextParams(ctx, ft.Params...)
63-
}
6463
if ft.TaskRef != nil {
6564
if ft.TaskRef.Kind == "" {
6665
ft.TaskRef.Kind = NamespacedTaskKind
6766
}
6867
}
6968
if ft.TaskSpec != nil {
69+
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
70+
ctx = addContextParams(ctx, ft.Params)
71+
ps.Finally[i].Params = getContextParams(ctx, ft.Params...)
72+
}
7073
ft.TaskSpec.SetDefaults(ctx)
7174
}
7275
}

‎pkg/apis/pipeline/v1beta1/pipelinerun_defaults_test.go‎

Lines changed: 99 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,30 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
107107
},
108108
},
109109
PipelineSpec: &v1beta1.PipelineSpec{
110-
Tasks: []v1beta1.PipelineTask{{
111-
TaskSpec: &v1beta1.EmbeddedTask{
112-
TaskSpec: v1beta1.TaskSpec{},
110+
Tasks: []v1beta1.PipelineTask{
111+
{
112+
TaskSpec: &v1beta1.EmbeddedTask{
113+
TaskSpec: v1beta1.TaskSpec{},
114+
},
113115
},
114-
}},
116+
{
117+
TaskRef: &v1beta1.TaskRef{
118+
Name: "baz",
119+
},
120+
},
121+
},
122+
Finally: []v1beta1.PipelineTask{
123+
{
124+
TaskSpec: &v1beta1.EmbeddedTask{
125+
TaskSpec: v1beta1.TaskSpec{},
126+
},
127+
},
128+
{
129+
TaskRef: &v1beta1.TaskRef{
130+
Name: "baz",
131+
},
132+
},
133+
},
115134
},
116135
},
117136
want: &v1beta1.PipelineRunSpec{
@@ -132,38 +151,86 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
132151
},
133152
},
134153
PipelineSpec: &v1beta1.PipelineSpec{
135-
Tasks: []v1beta1.PipelineTask{{
136-
TaskSpec: &v1beta1.EmbeddedTask{
137-
TaskSpec: v1beta1.TaskSpec{
138-
Params: []v1beta1.ParamSpec{
139-
{
140-
Name: "foo",
141-
Type: v1beta1.ParamTypeString,
154+
Tasks: []v1beta1.PipelineTask{
155+
{
156+
TaskSpec: &v1beta1.EmbeddedTask{
157+
TaskSpec: v1beta1.TaskSpec{
158+
Params: []v1beta1.ParamSpec{
159+
{
160+
Name: "foo",
161+
Type: v1beta1.ParamTypeString,
162+
},
163+
{
164+
Name: "bar",
165+
Type: v1beta1.ParamTypeArray,
166+
},
142167
},
143-
{
144-
Name: "bar",
145-
Type: v1beta1.ParamTypeArray,
168+
},
169+
},
170+
Params: []v1beta1.Param{
171+
{
172+
Name: "foo",
173+
Value: v1beta1.ArrayOrString{
174+
Type: v1beta1.ParamTypeString,
175+
StringVal: "$(params.foo)",
176+
},
177+
},
178+
{
179+
Name: "bar",
180+
Value: v1beta1.ArrayOrString{
181+
Type: v1beta1.ParamTypeArray,
182+
ArrayVal: []string{"$(params.bar[*])"},
146183
},
147184
},
148185
},
149186
},
150-
Params: []v1beta1.Param{
151-
{
152-
Name: "foo",
153-
Value: v1beta1.ArrayOrString{
154-
Type: v1beta1.ParamTypeString,
155-
StringVal: "$(params.foo)",
187+
{
188+
TaskRef: &v1beta1.TaskRef{
189+
Name: "baz",
190+
Kind: v1beta1.NamespacedTaskKind,
191+
},
192+
},
193+
},
194+
Finally: []v1beta1.PipelineTask{
195+
{
196+
TaskSpec: &v1beta1.EmbeddedTask{
197+
TaskSpec: v1beta1.TaskSpec{
198+
Params: []v1beta1.ParamSpec{
199+
{
200+
Name: "foo",
201+
Type: v1beta1.ParamTypeString,
202+
},
203+
{
204+
Name: "bar",
205+
Type: v1beta1.ParamTypeArray,
206+
},
207+
},
156208
},
157209
},
158-
{
159-
Name: "bar",
160-
Value: v1beta1.ArrayOrString{
161-
Type: v1beta1.ParamTypeArray,
162-
ArrayVal: []string{"$(params.bar[*])"},
210+
Params: []v1beta1.Param{
211+
{
212+
Name: "foo",
213+
Value: v1beta1.ArrayOrString{
214+
Type: v1beta1.ParamTypeString,
215+
StringVal: "$(params.foo)",
216+
},
217+
},
218+
{
219+
Name: "bar",
220+
Value: v1beta1.ArrayOrString{
221+
Type: v1beta1.ParamTypeArray,
222+
ArrayVal: []string{"$(params.bar[*])"},
223+
},
163224
},
164225
},
165226
},
166-
}},
227+
{
228+
TaskRef: &v1beta1.TaskRef{
229+
Name: "baz",
230+
Kind: v1beta1.NamespacedTaskKind,
231+
},
232+
},
233+
},
167234
Params: []v1beta1.ParamSpec{
168235
{
169236
Name: "foo",
@@ -186,13 +253,16 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
186253
}
187254
tc.prs.SetDefaults(ctx)
188255

189-
sortPS := func(x, y v1beta1.ParamSpec) bool {
256+
sortParamSpecs := func(x, y v1beta1.ParamSpec) bool {
257+
return x.Name < y.Name
258+
}
259+
sortParams := func(x, y v1beta1.Param) bool {
190260
return x.Name < y.Name
191261
}
192-
sortP := func(x, y v1beta1.Param) bool {
262+
sortTasks := func(x, y v1beta1.PipelineTask) bool {
193263
return x.Name < y.Name
194264
}
195-
if d := cmp.Diff(tc.want, tc.prs, cmpopts.SortSlices(sortPS), cmpopts.SortSlices(sortP)); d != "" {
265+
if d := cmp.Diff(tc.want, tc.prs, cmpopts.SortSlices(sortParamSpecs), cmpopts.SortSlices(sortParams), cmpopts.SortSlices(sortTasks)); d != "" {
196266
t.Errorf("Mismatch of PipelineRunSpec %s", diff.PrintWantGot(d))
197267
}
198268
})

0 commit comments

Comments
 (0)