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
EmmaMunley committed Mar 3, 2023
1 parent 67abc4a commit e897520
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 23 deletions.
20 changes: 15 additions & 5 deletions pkg/apis/pipeline/v1/param_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,22 @@ 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 (matrix *Matrix) validatePipelineParametersVariablesInMatrixParameters(prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
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("", idx).ViaField("matrix.include.params", ""))
}
}
}
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
View file
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(task.Matrix.validatePipelineParametersVariablesInMatrixParameters(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
errs = errs.Also(task.When.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
Expand Down
80 changes: 74 additions & 6 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,21 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
Name: "a-param-intended-to-be-array", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.myArray[*])"},
}},
}},
}, {
name: "valid string parameter variables in matrix include",
params: []ParamSpec{{
Name: "baz", Type: ParamTypeString,
}},
tasks: []PipelineTask{{
Name: "bar",
TaskRef: &TaskRef{Name: "bar-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "build-1",
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}},
}}}},
}},
}, {
name: "object param - using single individual variable in string param",
params: []ParamSpec{{
Expand Down Expand Up @@ -1550,7 +1565,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 +1582,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 @@ -1581,12 +1596,32 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)"}},
}, {
Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}},
Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}}}}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
}, {
name: "invalid pipeline task with two matrix include parameters and one of them missing from the param declarations",
params: []ParamSpec{{
Name: "foo", Type: ParamTypeString,
}},
tasks: []PipelineTask{{
Name: "foo-task",
TaskRef: &TaskRef{Name: "foo-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foo)"},
}, {
Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.does-not-exist)"},
}},
}}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.include.params[1]"},
},
}, {
name: "invalid object key in the input of the when expression",
Expand Down Expand Up @@ -1730,7 +1765,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 Expand Up @@ -2409,7 +2444,6 @@ func TestValidateFinalTasks_Failure(t *testing.T) {
})
}
}

func TestContextValid(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -3063,6 +3097,40 @@ func Test_validateMatrix(t *testing.T) {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
}},
}, {
name: "parameters in include matrix are strings",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "test",
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}},
}}},
},
}},
}, {
name: "parameters in include matrix are arrays",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "test",
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo"}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar"}}},
}}},
},
}},
wantErrs: &apis.FieldError{
Message: "invalid value: parameters of type string only are allowed, but got param type array",
Paths: []string{"[0].matrix.include.params[barfoo], [0].matrix.include.params[foobar]"},
},
}, {
name: "parameters in matrix contain results references",
tasks: PipelineTaskList{{
Expand Down
20 changes: 15 additions & 5 deletions pkg/apis/pipeline/v1beta1/param_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,12 +524,22 @@ 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 (matrix *Matrix) validatePipelineParametersVariablesInMatrixParameters(prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) {
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("", idx).ViaField("matrix.include.params", ""))
}
}
}
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
View file
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(task.Matrix.validatePipelineParametersVariablesInMatrixParameters(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
errs = errs.Also(task.WhenExpressions.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaIndex(idx))
}
Expand Down
79 changes: 74 additions & 5 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,21 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
Name: "a-param-intended-to-be-array", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.myArray[*])"},
}},
}},
}, {
name: "valid string parameter variables in matrix include",
params: []ParamSpec{{
Name: "baz", Type: ParamTypeString,
}},
tasks: []PipelineTask{{
Name: "bar",
TaskRef: &TaskRef{Name: "bar-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "build-1",
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}},
}}}},
}},
}, {
name: "object param - using single individual variable in string param",
params: []ParamSpec{{
Expand Down Expand Up @@ -1943,7 +1958,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 +1975,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 @@ -1974,12 +1989,32 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)"}},
}, {
Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}},
Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}}}}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix.params[b-param].value[0]"},
},
}, {
name: "invalid pipeline task with two matrix include parameters and one of them missing from the param declarations",
params: []ParamSpec{{
Name: "foo", Type: ParamTypeString,
}},
tasks: []PipelineTask{{
Name: "foo-task",
TaskRef: &TaskRef{Name: "foo-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Params: []Param{{
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foo)"},
}, {
Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.does-not-exist)"},
}},
}}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
Paths: []string{"[0].matrix.include.params[1]"},
},
}, {
name: "invalid object key in the input of the when expression",
Expand Down Expand Up @@ -2123,7 +2158,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 Expand Up @@ -3507,6 +3542,40 @@ func Test_validateMatrix(t *testing.T) {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
}},
}, {
name: "parameters in include matrix are strings",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "test",
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}},
}}},
},
}},
}, {
name: "parameters in include matrix are arrays",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: &Matrix{
Include: []MatrixInclude{{
Name: "test",
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo"}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar"}}},
}}},
},
}},
wantErrs: &apis.FieldError{
Message: "invalid value: parameters of type string only are allowed, but got param type array",
Paths: []string{"[0].matrix.include.params[barfoo], [0].matrix.include.params[foobar]"},
},
}, {
name: "parameters in matrix contain results references",
tasks: PipelineTaskList{{
Expand Down

0 comments on commit e897520

Please sign in to comment.