Skip to content

Commit 2e10695

Browse files
committed
add index validation for pipeline param
1 parent 17660f5 commit 2e10695

File tree

13 files changed

+562
-317
lines changed

13 files changed

+562
-317
lines changed

docs/variables.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ For instructions on using variable substitutions see the relevant section of [th
1919
| `params.<param name>` | The value of the parameter at runtime. |
2020
| `params['<param name>']` | (see above) |
2121
| `params["<param name>"]` | (see above) |
22-
| `params.<param name>[i]` | Get the i element of param array. This is alpha feature, set `enable-api-fields` to `alpha` to use it.|
22+
| `params.<param name>[i]` | Get the i-th element of param array. This is alpha feature, set `enable-api-fields` to `alpha` to use it.|
2323
| `params['<param name>'][i]` | (see above) |
2424
| `params["<param name>"][i]` | (see above) |
2525
| `tasks.<taskName>.results.<resultName>` | The value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) |
@@ -47,9 +47,6 @@ For instructions on using variable substitutions see the relevant section of [th
4747
| `params.<param name>` | The value of the parameter at runtime. |
4848
| `params['<param name>']` | (see above) |
4949
| `params["<param name>"]` | (see above) |
50-
| `params.<param name>[i]` | Get the i element of param array. This is alpha feature, set `enable-api-fields` to `alpha` to use it.|
51-
| `params['<param name>'][i]` | (see above) |
52-
| `params["<param name>"][i]` | (see above) |
5350
| `resources.inputs.<resourceName>.path` | The path to the input resource's directory. |
5451
| `resources.outputs.<resourceName>.path` | The path to the output resource's directory. |
5552
| `results.<resultName>.path` | The path to the file where the `Task` writes its results data. |

examples/v1beta1/pipelineruns/alpha/pipelinerun-param-array-indexing.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ spec:
2020
- name: environment2
2121
type: string
2222
steps:
23-
# this step should echo "foo"
23+
# this step should echo "staging"
2424
- name: echo-params-1
2525
image: bash:3.2
2626
args: [
2727
"echo",
2828
"$(params.environment1)",
2929
]
30-
# this step should echo "bar"
30+
# this step should echo "staging"
3131
- name: echo-params-2
3232
image: ubuntu
3333
script: |

examples/v1beta1/taskruns/alpha/param_array_indexing.yaml

Lines changed: 0 additions & 37 deletions
This file was deleted.

pkg/apis/pipeline/v1beta1/param_types_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,13 @@ func TestArrayOrString_ApplyReplacements(t *testing.T) {
230230
arrayReplacements: map[string][]string{"params.myarray": {"a", "b", "c"}},
231231
},
232232
expectedOutput: v1beta1.NewArrayOrString("a", "b", "c"),
233+
}, {
234+
name: "array indexing replacement on string val",
235+
args: args{
236+
input: v1beta1.NewArrayOrString("$(params.myarray[0])"),
237+
stringReplacements: map[string]string{"params.myarray[0]": "a", "params.myarray[1]": "b"},
238+
},
239+
expectedOutput: v1beta1.NewArrayOrString("a"),
233240
}, {
234241
name: "object replacement on string val",
235242
args: args{

pkg/reconciler/pipelinerun/pipelinerun.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,15 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
447447
return controller.NewPermanentError(err)
448448
}
449449

450+
// Ensure that the array reference is not out of bound
451+
if err := resources.ValidateParamArrayIndex(ctx, pipelineSpec, pr); err != nil {
452+
// This Run has failed, so we need to mark it as failed and stop reconciling it
453+
pr.Status.MarkFailed(ReasonObjectParameterMissKeys,
454+
"PipelineRun %s/%s parameters is missing object keys required by Pipeline %s/%s's parameters: %s",
455+
pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err)
456+
return controller.NewPermanentError(err)
457+
}
458+
450459
// Ensure that the workspaces expected by the Pipeline are provided by the PipelineRun.
451460
if err := resources.ValidateWorkspaceBindings(pipelineSpec, pr); err != nil {
452461
pr.Status.MarkFailed(ReasonInvalidWorkspaceBinding,

pkg/reconciler/pipelinerun/resources/apply.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ func ApplyParameters(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.P
5757
case v1beta1.ParamTypeArray:
5858
for _, pattern := range patterns {
5959
// array indexing for param is alpha feature
60-
// TODO(#4723): Validate the array reference is not out of bound
6160
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
6261
for i := 0; i < len(p.Default.ArrayVal); i++ {
6362
stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Default.ArrayVal[i]

pkg/reconciler/pipelinerun/resources/validate_params.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@ limitations under the License.
1717
package resources
1818

1919
import (
20+
"context"
2021
"fmt"
22+
"strconv"
23+
"strings"
2124

25+
"github.com/tektoncd/pipeline/pkg/apis/config"
2226
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
2327
"github.com/tektoncd/pipeline/pkg/list"
2428
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
29+
"github.com/tektoncd/pipeline/pkg/substitution"
2530
)
2631

2732
// ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
@@ -84,3 +89,110 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip
8489

8590
return nil
8691
}
92+
93+
// ValidateParamArrayIndex validate if the array indexing param reference target is existent
94+
func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error {
95+
cfg := config.FromContextOrDefaults(ctx)
96+
if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields {
97+
return nil
98+
}
99+
100+
arrayParams := extractParamIndexes(p.Params, pr.Spec.Params)
101+
102+
var outofBoundParams []string
103+
104+
// collect all the references
105+
for i := range p.Tasks {
106+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReferences(p.Tasks[i].Params, arrayParams)...)
107+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReferences(p.Tasks[i].Matrix, arrayParams)...)
108+
for j := range p.Tasks[i].Workspaces {
109+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams)...)
110+
}
111+
for _, wes := range p.Tasks[i].WhenExpressions {
112+
for _, v := range wes.Values {
113+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(v, arrayParams)...)
114+
}
115+
}
116+
}
117+
118+
for i := range p.Finally {
119+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams)...)
120+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReferences(p.Finally[i].Matrix, arrayParams)...)
121+
for _, wes := range p.Finally[i].WhenExpressions {
122+
for _, v := range wes.Values {
123+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(v, arrayParams)...)
124+
}
125+
}
126+
}
127+
128+
if len(outofBoundParams) > 0 {
129+
return fmt.Errorf("non-existent param references:%v", outofBoundParams)
130+
}
131+
return nil
132+
}
133+
134+
func extractParamIndexes(defaults []v1beta1.ParamSpec, params []v1beta1.Param) map[string]int {
135+
// Collect all array params
136+
arrayParams := make(map[string]int)
137+
138+
patterns := []string{
139+
"$(params.%s)",
140+
"$(params[%q])",
141+
"$(params['%s'])",
142+
}
143+
144+
// Collect array params lengths from defaults
145+
for _, p := range defaults {
146+
if p.Default != nil {
147+
if p.Default.Type == v1beta1.ParamTypeArray {
148+
for _, pattern := range patterns {
149+
for i := 0; i < len(p.Default.ArrayVal); i++ {
150+
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Default.ArrayVal)
151+
}
152+
}
153+
}
154+
}
155+
}
156+
157+
// Collect array params lengths from pipelinerun or taskrun
158+
for _, p := range params {
159+
if p.Value.Type == v1beta1.ParamTypeArray {
160+
for _, pattern := range patterns {
161+
for i := 0; i < len(p.Value.ArrayVal); i++ {
162+
arrayParams[fmt.Sprintf(pattern, p.Name)] = len(p.Value.ArrayVal)
163+
}
164+
}
165+
}
166+
}
167+
return arrayParams
168+
}
169+
170+
func findInvalidParamArrayReferences(params []v1beta1.Param, arrayParams map[string]int) []string {
171+
var outofBoundParams []string
172+
for i := range params {
173+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams)...)
174+
for _, v := range params[i].Value.ArrayVal {
175+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(v, arrayParams)...)
176+
}
177+
for _, v := range params[i].Value.ObjectVal {
178+
outofBoundParams = append(outofBoundParams, findInvalidParamArrayReference(v, arrayParams)...)
179+
}
180+
}
181+
return outofBoundParams
182+
}
183+
184+
func findInvalidParamArrayReference(paramReference string, arrayParams map[string]int) []string {
185+
var outofBoundParams []string
186+
list := substitution.ExtractParamsExpressions(paramReference)
187+
for _, val := range list {
188+
indexString := substitution.ExtractIntIndex(paramReference)
189+
idx, _ := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(indexString, "["), "]"))
190+
v := substitution.TrimArrayIndex(val)
191+
if paramLength, ok := arrayParams[v]; ok {
192+
if idx >= paramLength {
193+
outofBoundParams = append(outofBoundParams, val)
194+
}
195+
}
196+
}
197+
return outofBoundParams
198+
}

0 commit comments

Comments
 (0)