diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 4f2c594bc3f..920824fdf04 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "regexp" "strings" resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" @@ -210,23 +211,28 @@ func ArrayReference(a string) string { return strings.TrimSuffix(strings.TrimPrefix(a, "$("+ParamsPrefix+"."), "[*])") } -func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) { +func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { for _, param := range params { - if param.Value.Type == ParamTypeString { - errs = errs.Also(validateStringVariable(param.Value.StringVal, prefix, paramNames, arrayParamNames).ViaFieldKey("params", param.Name)) - } else { + switch param.Value.Type { + case ParamTypeArray: for idx, arrayElement := range param.Value.ArrayVal { - errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames).ViaFieldIndex("value", idx).ViaFieldKey("params", param.Name)) + errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("params", param.Name)) } + case ParamTypeObject: + for key, val := range param.Value.ObjectVal { + errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldKey("properties", key).ViaFieldKey("params", param.Name)) + } + default: + errs = errs.Also(validateStringVariableP(param, prefix, paramNames, arrayParamNames, objectParamNameKeys)) } } return errs } -func validatePipelineParametersVariablesInMatrixParameters(matrix []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) { +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).ViaFieldIndex("value", idx).ViaFieldKey("matrix", param.Name)) + errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaFieldIndex("value", idx).ViaFieldKey("matrix", param.Name)) } } return errs @@ -258,12 +264,50 @@ func validateParameterInOneOfMatrixOrParams(matrix []Param, params []Param) (err return errs } -func validateStringVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError { +// validateStringVariableP validates param variable usage in string value +// see https://github.com/tektoncd/pipeline/issues/4879#issuecomment-1133120522 for background +// This will allow using the whole object value to provide value for object param. example: +// params: +// - name: arg +// value: $(params.myObject[*]) +func validateStringVariableP(param Param, prefix string, paramNames sets.String, arrayVars sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { + stringValue := param.Value.StringVal + singleVariableRegex := regexp.MustCompile(fmt.Sprintf("^\\$\\(%s\\.[_a-zA-Z][_a-zA-Z0-9.-]*\\[\\*\\]\\)$", prefix)) + + // if the provided param value is just $(params.myObject[*]) or $(params.myArray[*]), example: + // params: + // - name: arg + // value: $(params.myObject[*]) + if singleVariableRegex.MatchString(stringValue) { + return errs.Also(substitution.ValidateVariableP(stringValue, prefix, paramNames).ViaFieldKey("params", param.Name)) + } + + // if the provided param value is string literal and/or contains multiple variables + // valid example: "$(params.myString) and another $(params.myObject.key1)" + // invalid example: "$(params.myString) and another $(params.myObject[*])" + return errs.Also(validateStringVariable(stringValue, prefix, paramNames, arrayVars, objectParamNameKeys).ViaFieldKey("params", param.Name)) +} + +func validateStringVariable(value, prefix string, stringVars sets.String, arrayVars sets.String, objectParamNameKeys map[string][]string) *apis.FieldError { errs := substitution.ValidateVariableP(value, prefix, stringVars) + errs = errs.Also(validateObjectVariable(value, prefix, objectParamNameKeys)) return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars)) } -func validateArrayVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError { +func validateArrayVariable(value, prefix string, stringVars sets.String, arrayVars sets.String, objectParamNameKeys map[string][]string) *apis.FieldError { errs := substitution.ValidateVariableP(value, prefix, stringVars) + errs = errs.Also(validateObjectVariable(value, prefix, objectParamNameKeys)) return errs.Also(substitution.ValidateVariableIsolatedP(value, prefix, arrayVars)) } + +func validateObjectVariable(value, prefix string, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { + objectNames := sets.NewString() + for objectParamName, keys := range objectParamNameKeys { + objectNames.Insert(objectParamName) + errs = errs.Also(substitution.ValidateVariableP(value, fmt.Sprintf("%s\\.%s", prefix, objectParamName), sets.NewString(keys...))) + } + + // TODO (chuangw6): add this once #4861 is merged, and add tests + // return errs.Also(substitution.ValidateEntireVariableProhibitedP(value, prefix, objectNames)) + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 774ce7a9be3..fd2f2848bce 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -128,6 +128,7 @@ func validatePipelineWorkspacesUsage(wss []PipelineWorkspaceDeclaration, pts []P func validatePipelineParameterVariables(tasks []PipelineTask, params []ParamSpec) (errs *apis.FieldError) { parameterNames := sets.NewString() arrayParameterNames := sets.NewString() + objectParameterNameKeys := map[string][]string{} for _, p := range params { // Verify that p is a valid type. @@ -155,16 +156,21 @@ func validatePipelineParameterVariables(tasks []PipelineTask, params []ParamSpec if p.Type == ParamTypeArray { arrayParameterNames.Insert(p.Name) } - } - return errs.Also(validatePipelineParametersVariables(tasks, "params", parameterNames, arrayParameterNames)) + if p.Type == ParamTypeObject { + for k := range p.Properties { + objectParameterNameKeys[p.Name] = append(objectParameterNameKeys[p.Name], k) + } + } + } + return errs.Also(validatePipelineParametersVariables(tasks, "params", parameterNames, arrayParameterNames, objectParameterNameKeys)) } -func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) { +func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { for idx, task := range tasks { - errs = errs.Also(validatePipelineParametersVariablesInTaskParameters(task.Params, prefix, paramNames, arrayParamNames).ViaIndex(idx)) - errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix, prefix, paramNames, arrayParamNames).ViaIndex(idx)) - errs = errs.Also(task.WhenExpressions.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames).ViaIndex(idx)) + errs = errs.Also(validatePipelineParametersVariablesInTaskParameters(task.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)) } return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 6a6e27bccac..5360d17c531 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -1346,6 +1346,148 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(baz[*])", "and", "$(foo-is-baz[*])"}}, }}, }}, + }, { + name: "array param - using the whole variable as a param's value that is intented to be array type", + params: []ParamSpec{{ + Name: "myArray", + Type: ParamTypeArray, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param-intented-to-be-array", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myArray[*])"}, + }}, + }}, + }, { + name: "object param - using single individual variable in string param", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-string-param", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myObject.key1)"}, + }}, + }}, + }, { + name: "object param - using multiple individual variables in string param", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-string-param", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myObject.key1) and $(params.myObject.key2)"}, + }}, + }}, + }, { + name: "object param - using individual variables in array param", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "an-array-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.key1)", "another one $(params.myObject.key2)"}}, + }}, + }}, + }, { + name: "object param - using individual variables and string param as the value of other object individual keys", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }, { + Name: "myString", + Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "an-object-param", Value: ArrayOrString{Type: ParamTypeObject, ObjectVal: map[string]string{ + "url": "$(params.myObject.key1)", + "commit": "$(params.myString)", + }}, + }}, + }}, + }, { + name: "object param - using individual variables in matrix", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Matrix: []Param{{ + Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.key1)", "and", "$(params.myObject.key2)"}}, + }}, + }}, + }, { + name: "object param - using the whole variable as a param's value that is intented to be object type", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param-intented-to-be-object", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myObject[*])"}, + }}, + }}, + }, { + name: "object param - using individual variable in input of when expression, and using both object individual variable and array reference in values of when expression", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }, { + Name: "foo", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(params.myObject.key1)", + Operator: selection.In, + Values: []string{"$(params.foo[*])", "$(params.myObject.key2)"}, + }}, + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1671,6 +1813,144 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.does-not-exist)"`, Paths: []string{"[0].matrix[b-param].value[0]"}, }, + }, { + name: "invalid object key in the input of the when expression", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(params.myObject.non-exist-key)", + Operator: selection.In, + Values: []string{"foo"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].when[0].input"}, + }, + }, { + name: "invalid object key in the Values of the when expression", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "bax", + Operator: selection.In, + Values: []string{"$(params.myObject.non-exist-key)"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].when[0].values"}, + }, + }, { + name: "invalid object key is used to provide values for array params", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.non-exist-key)", "last"}}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].params[a-param].value[0]"}, + }, + }, { + name: "invalid object key is used to provide values for string params", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myObject.non-exist-key)"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].params[a-param]"}, + }, + }, { + name: "invalid object key is used to provide values for object params", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }, { + Name: "myString", + Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "an-object-param", Value: ArrayOrString{Type: ParamTypeObject, ObjectVal: map[string]string{ + "url": "$(params.myObject.non-exist-key)", + "commit": "$(params.myString)", + }}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].params[an-object-param].properties[url]"}, + }, + }, { + name: "invalid object key is used to provide values for matrix params", + params: []ParamSpec{{ + Name: "myObject", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{ + "key1": {Type: "string"}, + "key2": {Type: "string"}, + }, + }}, + tasks: []PipelineTask{{ + Name: "foo-task", + TaskRef: &TaskRef{Name: "foo-task"}, + Matrix: []Param{{ + Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.key1)"}}, + }, { + Name: "b-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.non-exist-key)"}}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, + Paths: []string{"[0].matrix[b-param].value[0]"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/when_validation.go b/pkg/apis/pipeline/v1beta1/when_validation.go index 316dbf10f24..b5a0b1c8e99 100644 --- a/pkg/apis/pipeline/v1beta1/when_validation.go +++ b/pkg/apis/pipeline/v1beta1/when_validation.go @@ -74,17 +74,17 @@ func (wes WhenExpressions) validateTaskResultsVariables() *apis.FieldError { return nil } -func (wes WhenExpressions) validatePipelineParametersVariables(prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) { +func (wes WhenExpressions) validatePipelineParametersVariables(prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { for idx, we := range wes { - errs = errs.Also(validateStringVariable(we.Input, prefix, paramNames, arrayParamNames).ViaField("input").ViaFieldIndex("when", idx)) + errs = errs.Also(validateStringVariable(we.Input, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaField("input").ViaFieldIndex("when", idx)) for _, val := range we.Values { // one of the values could be a reference to an array param, such as, $(params.foo[*]) // extract the variable name from the pattern $(params.foo[*]), if the variable name matches with one of the array params // validate the param as an array variable otherwise, validate it as a string variable if arrayParamNames.Has(ArrayReference(val)) { - errs = errs.Also(validateArrayVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx)) + errs = errs.Also(validateArrayVariable(val, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaField("values").ViaFieldIndex("when", idx)) } else { - errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx)) + errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames, objectParamNameKeys).ViaField("values").ViaFieldIndex("when", idx)) } } }