Skip to content

Commit

Permalink
TEP-0118: Add validation for matrix pipeline parameter variables incl…
Browse files Browse the repository at this point in the history
…ude Matrix.Include.Params

[TEP-0090: Matrix] introduced `Matrix` to the `PipelineTask` specification such that the `PipelineTask` executes a list of `TaskRuns` or `Runs` in parallel with the specified list of inputs for a `Parameter` or with different combinations of the inputs for a set of `Parameters`.

To build on this, Tep-0018 introduced Matrix.Include, which allows passing in a specific combinations of `Parameters` into the `Matrix`.

This commit validates that matrix pipeline parameter variables  including include params that may contain the reference(s) to other params are used appropriately.

Note: This is still in preview mode. Other forms of validation and implementation logic will be added in subsequent commits.
  • Loading branch information
EmmaMunley committed Feb 28, 2023
1 parent 5cc8bf2 commit 595eae1
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 20 deletions.
22 changes: 17 additions & 5 deletions pkg/apis/pipeline/v1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,24 @@ func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix
return errs
}

// validatePipelineParametersVariablesInMatrixParameters validates matrix param value
// validatePipelineParametersVariablesInMatrixParameters validates all pipeline paramater variables including Matrix.Params and Matrix.Include.Params
// that may contain the reference(s) to other params to make sure those references are used appropriately.
func validatePipelineParametersVariablesInMatrixParameters(matrix []Param, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
for _, param := range matrix {
for idx, arrayElement := range param.Value.ArrayVal {
errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix", param.Name))
func validatePipelineParametersVariablesInMatrixParameters(matrix *Matrix, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
if matrix != nil {
if matrix.MatrixHasInclude() {
for _, include := range matrix.Include {
for idx, param := range include.Params {
stringElement := param.Value.StringVal
errs = errs.Also(validateStringVariable(stringElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix.include.params", param.Name))
}
}
}
if matrix.MatrixHasParams() {
for _, param := range matrix.Params {
for idx, arrayElement := range param.Value.ArrayVal {
errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix.params", param.Name))
}
}
}
}
return errs
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, pa
for idx, task := range tasks {
errs = errs.Also(validatePipelineParametersVariablesInTaskParameters(task.Params, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
if task.IsMatrixed() {
errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix.Params, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
errs = errs.Also(task.When.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[0]"},
Paths: []string{"[0].matrix.params[a-param].value[0]"},
},
}, {
name: "invalid pipeline task with a matrix parameter combined with missing param from the param declarations",
Expand All @@ -1567,7 +1567,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[2]"},
Paths: []string{"[0].matrix.params[a-param].value[2]"},
},
}, {
name: "invalid pipeline task with two matrix parameters and one of them missing from the param declarations",
Expand All @@ -1586,7 +1586,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
}, {
name: "invalid object key in the input of the when expression",
Expand Down Expand Up @@ -1730,7 +1730,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.myObject.non-exist-key)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
api: "alpha",
}, {
Expand Down
22 changes: 17 additions & 5 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,24 @@ func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix
return errs
}

// validatePipelineParametersVariablesInMatrixParameters validates matrix param value
// validatePipelineParametersVariablesInMatrixParameters validates all pipeline paramater variables including Matrix.Params and Matrix.Include.Params
// that may contain the reference(s) to other params to make sure those references are used appropriately.
func validatePipelineParametersVariablesInMatrixParameters(matrix []Param, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
for _, param := range matrix {
for idx, arrayElement := range param.Value.ArrayVal {
errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix", param.Name))
func validatePipelineParametersVariablesInMatrixParameters(matrix *Matrix, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
if matrix != nil {
if matrix.MatrixHasInclude() {
for _, include := range matrix.Include {
for idx, param := range include.Params {
stringElement := param.Value.StringVal
errs = errs.Also(validateStringVariable(stringElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix.include.params", param.Name))
}
}
}
if matrix.MatrixHasParams() {
for _, param := range matrix.Params {
for idx, arrayElement := range param.Value.ArrayVal {
errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix.params", param.Name))
}
}
}
}
return errs
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, pa
for idx, task := range tasks {
errs = errs.Also(validatePipelineParametersVariablesInTaskParameters(task.Params, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
if task.IsMatrixed() {
errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix.Params, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
errs = errs.Also(task.WhenExpressions.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1943,7 +1943,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[0]"},
Paths: []string{"[0].matrix.params[a-param].value[0]"},
},
}, {
name: "invalid pipeline task with a matrix parameter combined with missing param from the param declarations",
Expand All @@ -1960,7 +1960,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[2]"},
Paths: []string{"[0].matrix.params[a-param].value[2]"},
},
}, {
name: "invalid pipeline task with two matrix parameters and one of them missing from the param declarations",
Expand All @@ -1979,7 +1979,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
}, {
name: "invalid object key in the input of the when expression",
Expand Down Expand Up @@ -2123,7 +2123,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.myObject.non-exist-key)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
api: "alpha",
}, {
Expand Down

0 comments on commit 595eae1

Please sign in to comment.