Skip to content

Commit f0db0e4

Browse files
chengjoeychengjoey
committed
fix task parameter value substitution error due to propagateParams
1 parent 12457e2 commit f0db0e4

File tree

2 files changed

+321
-23
lines changed

2 files changed

+321
-23
lines changed

‎pkg/reconciler/pipelinerun/resources/apply.go

+41-23
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string,
236236
if p.Tasks[i].TaskRef != nil && p.Tasks[i].TaskRef.Params != nil {
237237
p.Tasks[i].TaskRef.Params = replaceParamValues(p.Tasks[i].TaskRef.Params, replacements, arrayReplacements, objectReplacements)
238238
}
239-
p.Tasks[i], replacements, arrayReplacements, objectReplacements = propagateParams(p.Tasks[i], replacements, arrayReplacements, objectReplacements)
239+
p.Tasks[i] = propagateParams(p.Tasks[i], replacements, arrayReplacements, objectReplacements)
240240
}
241241

242242
for i := range p.Finally {
@@ -251,38 +251,56 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string,
251251
if p.Finally[i].TaskRef != nil && p.Finally[i].TaskRef.Params != nil {
252252
p.Finally[i].TaskRef.Params = replaceParamValues(p.Finally[i].TaskRef.Params, replacements, arrayReplacements, objectReplacements)
253253
}
254-
p.Finally[i], replacements, arrayReplacements, objectReplacements = propagateParams(p.Finally[i], replacements, arrayReplacements, objectReplacements)
254+
p.Finally[i] = propagateParams(p.Finally[i], replacements, arrayReplacements, objectReplacements)
255255
}
256256

257257
return p
258258
}
259259

260-
func propagateParams(t v1beta1.PipelineTask, replacements map[string]string, arrayReplacements map[string][]string, objectReplacements map[string]map[string]string) (v1beta1.PipelineTask, map[string]string, map[string][]string, map[string]map[string]string) {
261-
if t.TaskSpec != nil {
262-
// check if there are task parameters defined that match the params at pipeline level
263-
if len(t.Params) > 0 {
264-
for _, par := range t.Params {
265-
for _, pattern := range paramPatterns {
266-
checkName := fmt.Sprintf(pattern, par.Name)
267-
// Scoping. Task Params will replace Pipeline Params
268-
if _, ok := replacements[checkName]; ok {
269-
replacements[checkName] = par.Value.StringVal
270-
}
271-
if _, ok := arrayReplacements[checkName]; ok {
272-
arrayReplacements[checkName] = par.Value.ArrayVal
273-
}
274-
if _, ok := objectReplacements[checkName]; ok {
275-
objectReplacements[checkName] = par.Value.ObjectVal
276-
for k, v := range par.Value.ObjectVal {
277-
replacements[fmt.Sprintf(objectIndividualVariablePattern, par.Name, k)] = v
278-
}
260+
// propagateParams used to propagate the parameters to the task spec
261+
// modifications to replacements should be limited to this function so as not to interfere with
262+
// the parameters of subsequent tasks
263+
func propagateParams(t v1beta1.PipelineTask, stringReplacements map[string]string, arrayReplacements map[string][]string, objectReplacements map[string]map[string]string) v1beta1.PipelineTask {
264+
if t.TaskSpec == nil {
265+
return t
266+
}
267+
// check if there are task parameters defined that match the params at pipeline level
268+
if len(t.Params) > 0 {
269+
stringReplacementsDup := make(map[string]string)
270+
arrayReplacementsDup := make(map[string][]string)
271+
objectReplacementsDup := make(map[string]map[string]string)
272+
for k, v := range stringReplacements {
273+
stringReplacementsDup[k] = v
274+
}
275+
for k, v := range arrayReplacements {
276+
arrayReplacementsDup[k] = v
277+
}
278+
for k, v := range objectReplacements {
279+
objectReplacementsDup[k] = v
280+
}
281+
for _, par := range t.Params {
282+
for _, pattern := range paramPatterns {
283+
checkName := fmt.Sprintf(pattern, par.Name)
284+
// Scoping. Task Params will replace Pipeline Params
285+
if _, ok := stringReplacementsDup[checkName]; ok {
286+
stringReplacementsDup[checkName] = par.Value.StringVal
287+
}
288+
if _, ok := arrayReplacementsDup[checkName]; ok {
289+
arrayReplacementsDup[checkName] = par.Value.ArrayVal
290+
}
291+
if _, ok := objectReplacementsDup[checkName]; ok {
292+
objectReplacementsDup[checkName] = par.Value.ObjectVal
293+
for k, v := range par.Value.ObjectVal {
294+
stringReplacementsDup[fmt.Sprintf(objectIndividualVariablePattern, par.Name, k)] = v
279295
}
280296
}
281297
}
282298
}
283-
t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, replacements, arrayReplacements)
299+
t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacementsDup, arrayReplacementsDup)
300+
} else {
301+
t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacements, arrayReplacements)
284302
}
285-
return t, replacements, arrayReplacements, objectReplacements
303+
return t
286304
}
287305

288306
func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]string, arrayReplacements map[string][]string, objectReplacements map[string]map[string]string) []v1beta1.Param {

‎pkg/reconciler/pipelinerun/resources/apply_test.go

+280
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,286 @@ func TestApplyParameters(t *testing.T) {
14501450
}},
14511451
},
14521452
},
1453+
{
1454+
name: "tasks with the same parameter name but referencing different values",
1455+
original: v1beta1.PipelineSpec{
1456+
Params: []v1beta1.ParamSpec{
1457+
{
1458+
Name: "end",
1459+
Default: &v1beta1.ParamValue{
1460+
Type: v1beta1.ParamTypeString,
1461+
StringVal: "c",
1462+
},
1463+
Type: v1beta1.ParamTypeString,
1464+
},
1465+
{
1466+
Name: "start",
1467+
Default: &v1beta1.ParamValue{
1468+
Type: v1beta1.ParamTypeString,
1469+
StringVal: "a",
1470+
},
1471+
Type: v1beta1.ParamTypeString,
1472+
},
1473+
{
1474+
Name: "step",
1475+
Default: &v1beta1.ParamValue{
1476+
Type: v1beta1.ParamTypeString,
1477+
StringVal: "b",
1478+
},
1479+
Type: v1beta1.ParamTypeString,
1480+
},
1481+
},
1482+
Tasks: []v1beta1.PipelineTask{
1483+
{
1484+
Name: "start",
1485+
},
1486+
{
1487+
Name: "step",
1488+
},
1489+
{
1490+
Name: "end",
1491+
},
1492+
{
1493+
Name: "print-msg",
1494+
TaskSpec: &v1beta1.EmbeddedTask{
1495+
TaskSpec: v1beta1.TaskSpec{
1496+
Params: []v1beta1.ParamSpec{
1497+
{
1498+
Name: "end",
1499+
Type: v1beta1.ParamTypeString,
1500+
},
1501+
{
1502+
Name: "start",
1503+
Type: v1beta1.ParamTypeString,
1504+
},
1505+
{
1506+
Name: "step",
1507+
Type: v1beta1.ParamTypeString,
1508+
},
1509+
},
1510+
},
1511+
},
1512+
Params: []v1beta1.Param{
1513+
{
1514+
Name: "end",
1515+
Value: v1beta1.ParamValue{
1516+
Type: v1beta1.ParamTypeString,
1517+
StringVal: "$(tasks.end.results.Output)",
1518+
},
1519+
},
1520+
{
1521+
Name: "start",
1522+
Value: v1beta1.ParamValue{
1523+
Type: v1beta1.ParamTypeString,
1524+
StringVal: "$(tasks.start.results.Output)",
1525+
},
1526+
},
1527+
{
1528+
Name: "step",
1529+
Value: v1beta1.ParamValue{
1530+
Type: v1beta1.ParamTypeString,
1531+
StringVal: "$(tasks.step.results.Output)",
1532+
},
1533+
},
1534+
},
1535+
},
1536+
{
1537+
Name: "print-msg-2",
1538+
TaskSpec: &v1beta1.EmbeddedTask{
1539+
TaskSpec: v1beta1.TaskSpec{
1540+
Params: []v1beta1.ParamSpec{
1541+
{
1542+
Name: "end",
1543+
Type: v1beta1.ParamTypeString,
1544+
},
1545+
{
1546+
Name: "start",
1547+
Type: v1beta1.ParamTypeString,
1548+
},
1549+
{
1550+
Name: "step",
1551+
Type: v1beta1.ParamTypeString,
1552+
},
1553+
},
1554+
},
1555+
},
1556+
Params: []v1beta1.Param{
1557+
{
1558+
Name: "end",
1559+
Value: v1beta1.ParamValue{
1560+
Type: v1beta1.ParamTypeString,
1561+
StringVal: "$(params.end)",
1562+
},
1563+
},
1564+
{
1565+
Name: "start",
1566+
Value: v1beta1.ParamValue{
1567+
Type: v1beta1.ParamTypeString,
1568+
StringVal: "$(params.start)",
1569+
},
1570+
},
1571+
{
1572+
Name: "step",
1573+
Value: v1beta1.ParamValue{
1574+
Type: v1beta1.ParamTypeString,
1575+
StringVal: "$(params.step)",
1576+
},
1577+
},
1578+
},
1579+
},
1580+
},
1581+
},
1582+
params: []v1beta1.Param{
1583+
{
1584+
Name: "end",
1585+
Value: v1beta1.ParamValue{
1586+
Type: v1beta1.ParamTypeString,
1587+
StringVal: "e",
1588+
},
1589+
},
1590+
{
1591+
Name: "start",
1592+
Value: v1beta1.ParamValue{
1593+
Type: v1beta1.ParamTypeString,
1594+
StringVal: "d",
1595+
},
1596+
},
1597+
{
1598+
Name: "step",
1599+
Value: v1beta1.ParamValue{
1600+
Type: v1beta1.ParamTypeString,
1601+
StringVal: "f",
1602+
},
1603+
},
1604+
},
1605+
expected: v1beta1.PipelineSpec{
1606+
Params: []v1beta1.ParamSpec{
1607+
{
1608+
Name: "end",
1609+
Default: &v1beta1.ParamValue{
1610+
Type: v1beta1.ParamTypeString,
1611+
StringVal: "c",
1612+
},
1613+
Type: v1beta1.ParamTypeString,
1614+
},
1615+
{
1616+
Name: "start",
1617+
Default: &v1beta1.ParamValue{
1618+
Type: v1beta1.ParamTypeString,
1619+
StringVal: "a",
1620+
},
1621+
Type: v1beta1.ParamTypeString,
1622+
},
1623+
{
1624+
Name: "step",
1625+
Default: &v1beta1.ParamValue{
1626+
Type: v1beta1.ParamTypeString,
1627+
StringVal: "b",
1628+
},
1629+
Type: v1beta1.ParamTypeString,
1630+
},
1631+
},
1632+
Tasks: []v1beta1.PipelineTask{
1633+
{
1634+
Name: "start",
1635+
},
1636+
{
1637+
Name: "step",
1638+
},
1639+
{
1640+
Name: "end",
1641+
},
1642+
{
1643+
Name: "print-msg",
1644+
TaskSpec: &v1beta1.EmbeddedTask{
1645+
TaskSpec: v1beta1.TaskSpec{
1646+
Params: []v1beta1.ParamSpec{
1647+
{
1648+
Name: "end",
1649+
Type: v1beta1.ParamTypeString,
1650+
},
1651+
{
1652+
Name: "start",
1653+
Type: v1beta1.ParamTypeString,
1654+
},
1655+
{
1656+
Name: "step",
1657+
Type: v1beta1.ParamTypeString,
1658+
},
1659+
},
1660+
},
1661+
},
1662+
Params: []v1beta1.Param{
1663+
{
1664+
Name: "end",
1665+
Value: v1beta1.ParamValue{
1666+
Type: v1beta1.ParamTypeString,
1667+
StringVal: "$(tasks.end.results.Output)",
1668+
},
1669+
},
1670+
{
1671+
Name: "start",
1672+
Value: v1beta1.ParamValue{
1673+
Type: v1beta1.ParamTypeString,
1674+
StringVal: "$(tasks.start.results.Output)",
1675+
},
1676+
},
1677+
{
1678+
Name: "step",
1679+
Value: v1beta1.ParamValue{
1680+
Type: v1beta1.ParamTypeString,
1681+
StringVal: "$(tasks.step.results.Output)",
1682+
},
1683+
},
1684+
},
1685+
},
1686+
{
1687+
Name: "print-msg-2",
1688+
TaskSpec: &v1beta1.EmbeddedTask{
1689+
TaskSpec: v1beta1.TaskSpec{
1690+
Params: []v1beta1.ParamSpec{
1691+
{
1692+
Name: "end",
1693+
Type: v1beta1.ParamTypeString,
1694+
},
1695+
{
1696+
Name: "start",
1697+
Type: v1beta1.ParamTypeString,
1698+
},
1699+
{
1700+
Name: "step",
1701+
Type: v1beta1.ParamTypeString,
1702+
},
1703+
},
1704+
},
1705+
},
1706+
Params: []v1beta1.Param{
1707+
{
1708+
Name: "end",
1709+
Value: v1beta1.ParamValue{
1710+
Type: v1beta1.ParamTypeString,
1711+
StringVal: "e",
1712+
},
1713+
},
1714+
{
1715+
Name: "start",
1716+
Value: v1beta1.ParamValue{
1717+
Type: v1beta1.ParamTypeString,
1718+
StringVal: "d",
1719+
},
1720+
},
1721+
{
1722+
Name: "step",
1723+
Value: v1beta1.ParamValue{
1724+
Type: v1beta1.ParamTypeString,
1725+
StringVal: "f",
1726+
},
1727+
},
1728+
},
1729+
},
1730+
},
1731+
},
1732+
},
14531733
} {
14541734
tt := tt // capture range variable
14551735
ctx := context.Background()

0 commit comments

Comments
 (0)