diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 23e71a52ef4..1f183799c24 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -221,7 +221,7 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string // if the stringVal is a string literal or a string that mixed with var references // just do the normal string replacement - if !exactVariableSubstitutionRegex.MatchString(stringVal) { + if !isolatedResultVariableSubstitutionRegex.MatchString(stringVal) && !substitution.IsIsolatedArrayOrObjectParamRef(stringVal) { arrayOrString.StringVal = substitution.ApplyReplacements(arrayOrString.StringVal, stringReplacements) return } @@ -338,7 +338,7 @@ func validateParamStringValue(param Param, prefix string, paramNames sets.String stringValue := param.Value.StringVal // if the provided param value is an isolated reference to the whole array/object, we just check if the param name exists. - isIsolated, errs := substitution.ValidateWholeArrayOrObjectRefInStringVariable(param.Name, stringValue, prefix, paramNames) + isIsolated, errs := substitution.ValidateIsolatedArrayOrObjectRefInStringVariable(param.Name, stringValue, prefix, paramNames) if isIsolated { return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 2a157db221d..9ca2038ddc1 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -1431,7 +1431,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { }}, }}, }, { - name: "array param - using the whole variable as a param's value that is intended to be array type", + name: "array param - using the whole variable (dot reference) as the value of an array param", params: []ParamSpec{{ Name: "myArray", Type: ParamTypeArray, @@ -1443,6 +1443,32 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param-intended-to-be-array", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myArray[*])"}, }}, }}, + }, { + name: "array param - using the whole variable (bracket with single quote) as the value of an array param", + params: []ParamSpec{{ + Name: "myArray", + Type: ParamTypeArray, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param-intended-to-be-array", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params['myArray'][*])"}, + }}, + }}, + }, { + name: "array param - using the whole variable (bracket with double quote) as the value of an array param", + params: []ParamSpec{{ + Name: "myArray", + Type: ParamTypeArray, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "a-param-intended-to-be-array", Value: ArrayOrString{Type: ParamTypeString, StringVal: `$(params["myArray"][*])`}, + }}, + }}, }, { name: "object param - using single individual variable in string param", params: []ParamSpec{{ @@ -1535,7 +1561,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { }}, }}, }, { - name: "object param - using the whole variable as a param's value that is intended to be object type", + name: "object param - using the whole variable (dot notation) as the value of an object param", params: []ParamSpec{{ Name: "myObject", Type: ParamTypeObject, @@ -1551,6 +1577,40 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param-intended-to-be-object", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params.myObject[*])"}, }}, }}, + }, { + name: "object param - using the whole variable (bracket notation with single quote) as the value of an object 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-param-intended-to-be-object", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(params['myObject'][*])"}, + }}, + }}, + }, { + name: "object param - using the whole variable (bracket notation with double quote) as the value of an object 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-param-intended-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{{ diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 4eb8fa322a1..8ace0ecfab5 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -44,9 +44,9 @@ const ( // TODO(#2462) use one regex across all substitutions // variableSubstitutionFormat matches format like $result.resultname, $result.resultname[int] and $result.resultname[*] variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)` - // exactVariableSubstitutionFormat matches strings that only contain a single reference to result or param variables, but nothing else + // isolatedVariableSubstitutionFormat matches strings that only contain a single reference to result or param variables, but nothing else // i.e. `$(result.resultname)` is a match, but `foo $(result.resultname)` is not. - exactVariableSubstitutionFormat = `^\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)$` + isolatedVariableSubstitutionFormat = `^\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)$` // arrayIndexing will match all `[int]` and `[*]` for parseExpression arrayIndexing = `\[([0-9])*\*?\]` // ResultNameFormat Constant used to define the the regex Result.Name should follow @@ -55,7 +55,7 @@ const ( // VariableSubstitutionRegex is a regex to find all result matching substitutions var VariableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) -var exactVariableSubstitutionRegex = regexp.MustCompile(exactVariableSubstitutionFormat) +var isolatedResultVariableSubstitutionRegex = regexp.MustCompile(isolatedVariableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) // arrayIndexingRegex is used to match `[int]` and `[*]` diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index f98d03569b9..f2c6378e407 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -27,32 +27,58 @@ import ( ) const ( - parameterSubstitution = `.*?(\[\*\])?` + // nameSubstition is the regex pattern for variable names that can catch any names in a notation + nameSubstition = `.*?` + // indexing is the regex pattern for the index suffix in a reference string + // - [i] int indexing to reference an array element + // - [*] start indexing to reference the whole array or object + indexing = `\[([0-9]+|\*)\]` - // braceMatchingRegex is a regex for parameter references including dot notation, bracket notation with single and double quotes. - braceMatchingRegex = "(\\$(\\(%s(\\.(?P%s)|\\[\"(?P%s)\"\\]|\\['(?P%s)'\\])\\)))" - // arrayIndexing will match all `[int]` and `[*]` for parseExpression - arrayIndexing = `\[([0-9])*\*?\]` // paramIndex will match all `$(params.paramName[int])` expressions paramIndexing = `\$\(params(\.[_a-zA-Z0-9.-]+|\[\'[_a-zA-Z0-9.-\/]+\'\]|\[\"[_a-zA-Z0-9.-\/]+\"\])\[[0-9]+\]\)` // intIndex will match all `[int]` expressions intIndex = `\[[0-9]+\]` ) -// arrayIndexingRegex is used to match `[int]` and `[*]` -var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) - -// paramIndexingRegex will match all `$(params.paramName[int])` expressions -var paramIndexingRegex = regexp.MustCompile(paramIndexing) - -// intIndexRegex will match all `[int]` for param expression -var intIndexRegex = regexp.MustCompile(intIndex) +var ( + // .paramName - dot notation + dotNotation = fmt.Sprintf(`\.(?P%s)`, nameSubstition) + // ['paramName'] - bracket notation with single quote + bracketNotationWithSingleQuote = fmt.Sprintf(`\['(?P%s)'\]`, nameSubstition) + // ["paramName"] - bracket notation with double quote + bracketNotationWithDoubleQuote = fmt.Sprintf(`\[\"(?P%s)\"\]`, nameSubstition) + // one of the three notations with optional indexing as the suffix + variableNotationPattern = fmt.Sprintf(`(%s|%s|%s)(?P%s)?`, dotNotation, bracketNotationWithSingleQuote, bracketNotationWithDoubleQuote, indexing) + + // full variable referencing regex: $(xy) + // - x is the prefix i.e. params + // - y is `variableNotationPattern` - one of three notations + optional indexing + fullVariableReferenceRegex = `\$\(%s%s\)` + + // isolatedParamVariablePattern is a regex to check if a string is an isolated + // reference to a param variable. + // (An isolated reference means a string that only contains a reference to a param + // variable but nothing else i.e. no extra characters, no reference to other variables.) + // Some examples: + // - Isolated reference: "$(params.myParam)", "$(params['myArray'][*])" + // - Non-isolated reference: "the param name is $(params.myParam)", "the first param is $(params.first) and the second param is $(params["second"])" + isolatedParamVariablePattern = fmt.Sprintf(fmt.Sprintf("^%s$", fullVariableReferenceRegex), "params", variableNotationPattern) + isolatedParamVariableRegex = regexp.MustCompile(isolatedParamVariablePattern) + + // arrayIndexingRegex is used to match `[int]` and `[*]` + arrayIndexingRegex = regexp.MustCompile(indexing) + + // paramIndexingRegex will match all `$(params.paramName[int])` expressions + paramIndexingRegex = regexp.MustCompile(paramIndexing) + + // intIndexRegex will match all `[int]` for param expression + intIndexRegex = regexp.MustCompile(intIndex) +) // ValidateVariable makes sure all variables in the provided string are known func ValidateVariable(name, value, prefix, locationName, path string, vars sets.String) *apis.FieldError { if vs, present, _ := extractVariablesFromString(value, prefix); present { for _, v := range vs { - v = strings.TrimSuffix(v, "[*]") if !vars.Has(v) { return &apis.FieldError{ Message: fmt.Sprintf("non-existent variable in %q for %s %s", value, locationName, name), @@ -75,7 +101,6 @@ func ValidateVariableP(value, prefix string, vars sets.String) *apis.FieldError } for _, v := range vs { - v = TrimArrayIndex(v) if !vars.Has(v) { return &apis.FieldError{ Message: fmt.Sprintf("non-existent variable in %q", value), @@ -92,7 +117,6 @@ func ValidateVariableP(value, prefix string, vars sets.String) *apis.FieldError func ValidateVariableProhibited(name, value, prefix, locationName, path string, vars sets.String) *apis.FieldError { if vs, present, _ := extractVariablesFromString(value, prefix); present { for _, v := range vs { - v = strings.TrimSuffix(v, "[*]") if vars.Has(v) { return &apis.FieldError{ Message: fmt.Sprintf("variable type invalid in %q for %s %s", value, locationName, name), @@ -115,7 +139,6 @@ func ValidateVariableProhibitedP(value, prefix string, vars sets.String) *apis.F } for _, v := range vs { - v = strings.TrimSuffix(v, "[*]") if vars.Has(v) { return &apis.FieldError{ Message: fmt.Sprintf("variable type invalid in %q", value), @@ -158,7 +181,6 @@ func ValidateVariableIsolated(name, value, prefix, locationName, path string, va if vs, present, _ := extractVariablesFromString(value, prefix); present { firstMatch, _ := extractExpressionFromString(value, prefix) for _, v := range vs { - v = strings.TrimSuffix(v, "[*]") if vars.Has(v) { if len(value) != len(firstMatch) { return &apis.FieldError{ @@ -184,7 +206,6 @@ func ValidateVariableIsolatedP(value, prefix string, vars sets.String) *apis.Fie } firstMatch, _ := extractExpressionFromString(value, prefix) for _, v := range vs { - v = strings.TrimSuffix(v, "[*]") if vars.Has(v) { if len(value) != len(firstMatch) { return &apis.FieldError{ @@ -199,33 +220,34 @@ func ValidateVariableIsolatedP(value, prefix string, vars sets.String) *apis.Fie return nil } -// ValidateWholeArrayOrObjectRefInStringVariable validates if a single string field uses references to the whole array/object appropriately -// valid example: "$(params.myObject[*])" -// invalid example: "$(params.name-not-exist[*])" -func ValidateWholeArrayOrObjectRefInStringVariable(name, value, prefix string, vars sets.String) (isIsolated bool, errs *apis.FieldError) { - nameSubstitution := `[_a-zA-Z0-9.-]+\[\*\]` - - // a regex to check if the stringValue is an isolated reference to the whole array/object param without extra string literal. - isolatedVariablePattern := fmt.Sprintf(fmt.Sprintf("^%s$", braceMatchingRegex), prefix, nameSubstitution, nameSubstitution, nameSubstitution) - isolatedVariableRegex, err := regexp.Compile(isolatedVariablePattern) - if err != nil { - return false, &apis.FieldError{ - Message: fmt.Sprint("Fail to parse the regex: ", err), - Paths: []string{fmt.Sprintf("%s.%s", prefix, name)}, - } - } - - if isolatedVariableRegex.MatchString(value) { +// ValidateIsolatedArrayOrObjectRefInStringVariable checks if a string is an isolated reference to an array/object param as whole +// If that's the case, the variable name in that isolated reference will be validated against a set of available array/object param names. +func ValidateIsolatedArrayOrObjectRefInStringVariable(name, value, prefix string, vars sets.String) (isIsolated bool, errs *apis.FieldError) { + if isIsolated := IsIsolatedArrayOrObjectParamRef(value); isIsolated { return true, ValidateVariableP(value, prefix, vars).ViaFieldKey(prefix, name) } return false, nil } +// IsIsolatedArrayOrObjectParamRef checks if a string is an isolated reference to the whole array/object param with star operator +// (An isolated reference to an array/object param as whole means a string that only contains a single reference +// to an array/object param variable but nothing else) +// Examples: +// - isolated reference: "$(params['myArray'][*])", "$(params.myObject[*])" +// - non-isolated reference: "this whole array param $(params["myArray"][*]) shouldn't appear in a string" +func IsIsolatedArrayOrObjectParamRef(value string) bool { + if isolatedParamVariableRegex.MatchString(value) && strings.HasSuffix(value, "[*])") { + return true + } + + return false +} + // Extract a the first full string expressions found (e.g "$(input.params.foo)"). Return // "" and false if nothing is found. func extractExpressionFromString(s, prefix string) (string, bool) { - pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, parameterSubstitution, parameterSubstitution) + pattern := fmt.Sprintf(fullVariableReferenceRegex, prefix, variableNotationPattern) re := regexp.MustCompile(pattern) match := re.FindStringSubmatch(s) if match == nil { @@ -235,7 +257,7 @@ func extractExpressionFromString(s, prefix string) (string, bool) { } func extractVariablesFromString(s, prefix string) ([]string, bool, string) { - pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, parameterSubstitution, parameterSubstitution) + pattern := fmt.Sprintf(fullVariableReferenceRegex, prefix, variableNotationPattern) re := regexp.MustCompile(pattern) matches := re.FindAllStringSubmatch(s, -1) errString := "" @@ -271,7 +293,7 @@ func extractVariablesFromString(s, prefix string) ([]string, bool, string) { } func extractEntireVariablesFromString(s, prefix string) ([]string, error) { - pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, parameterSubstitution, parameterSubstitution) + pattern := fmt.Sprintf(fullVariableReferenceRegex, prefix, variableNotationPattern) re, err := regexp.Compile(pattern) if err != nil { return nil, fmt.Errorf("Fail to parse regex pattern: %v", err)