Skip to content

Commit

Permalink
Allow dots inside single/double quotes when referencing them.
Browse files Browse the repository at this point in the history 
Prior to this, when using `$params["<NAME>"]` or `$params['<NAME>']`,
if `NAME` contained `dots(.)` (eg. `org.foo.blah`), only `org` would be
extracted, instead of `org.foo.blah`. This PR now properly fetches the
entire string and throws an error if using `dots (.)` without `single`
or `double` quotes.

The following references show whats valid and whats not:

`$params.org.foo.blah` : Not Valid: the webhook will throw a validation error.

`$params["org.foo.blah"]`: Valid

`$params['org.foo.blah']`: Valid
  • Loading branch information
@chitrangpatel
chitrangpatel committed May 17, 2022
1 parent 155179b commit 4237895
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 15 deletions.
59 changes: 44 additions & 15 deletions pkg/substitution/substitution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ import (
)

const parameterSubstitution = `[_a-zA-Z][_a-zA-Z0-9.-]*(\[\*\])?`

const braceMatchingRegex = "(\\$(\\(%s(\\.(?P<var1>%s)|\\[\"(?P<var2>%s)\"\\]|\\['(?P<var3>%s)'\\])\\)))"

// 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 {
if vs, present, _ := extractVariablesFromString(value, prefix); present {
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
if !vars.Has(v) {
Expand All @@ -46,7 +47,14 @@ func ValidateVariable(name, value, prefix, locationName, path string, vars sets.

// ValidateVariableP makes sure all variables for a parameter in the provided string are known
func ValidateVariableP(value, prefix string, vars sets.String) *apis.FieldError {
if vs, present := extractVariablesFromString(value, prefix); present {
if vs, present, errString := extractVariablesFromString(value, prefix); present {
if errString != "" {
return &apis.FieldError{
Message: errString,
Paths: []string{""},
}

}
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
if !vars.Has(v) {
Expand All @@ -63,7 +71,7 @@ func ValidateVariableP(value, prefix string, vars sets.String) *apis.FieldError

// ValidateVariableProhibited verifies that variables matching the relevant string expressions do not reference any of the names present in vars.
func ValidateVariableProhibited(name, value, prefix, locationName, path string, vars sets.String) *apis.FieldError {
if vs, present := extractVariablesFromString(value, prefix); present {
if vs, present, _ := extractVariablesFromString(value, prefix); present {
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
if vars.Has(v) {
Expand All @@ -79,7 +87,14 @@ func ValidateVariableProhibited(name, value, prefix, locationName, path string,

// ValidateVariableProhibitedP verifies that variables for a parameter matching the relevant string expressions do not reference any of the names present in vars.
func ValidateVariableProhibitedP(value, prefix string, vars sets.String) *apis.FieldError {
if vs, present := extractVariablesFromString(value, prefix); present {
if vs, present, errString := extractVariablesFromString(value, prefix); present {
if errString != "" {
return &apis.FieldError{
Message: errString,
Paths: []string{""},
}

}
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
if vars.Has(v) {
Expand All @@ -96,7 +111,7 @@ func ValidateVariableProhibitedP(value, prefix string, vars sets.String) *apis.F

// ValidateVariableIsolated verifies that variables matching the relevant string expressions are completely isolated if present.
func ValidateVariableIsolated(name, value, prefix, locationName, path string, vars sets.String) *apis.FieldError {
if vs, present := extractVariablesFromString(value, prefix); present {
if vs, present, _ := extractVariablesFromString(value, prefix); present {
firstMatch, _ := extractExpressionFromString(value, prefix)
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
Expand All @@ -115,7 +130,14 @@ func ValidateVariableIsolated(name, value, prefix, locationName, path string, va

// ValidateVariableIsolatedP verifies that variables matching the relevant string expressions are completely isolated if present.
func ValidateVariableIsolatedP(value, prefix string, vars sets.String) *apis.FieldError {
if vs, present := extractVariablesFromString(value, prefix); present {
if vs, present, errString := extractVariablesFromString(value, prefix); present {
if errString != "" {
return &apis.FieldError{
Message: errString,
Paths: []string{""},
}

}
firstMatch, _ := extractExpressionFromString(value, prefix)
for _, v := range vs {
v = strings.TrimSuffix(v, "[*]")
Expand Down Expand Up @@ -145,28 +167,35 @@ func extractExpressionFromString(s, prefix string) (string, bool) {
return match[0], true
}

func extractVariablesFromString(s, prefix string) ([]string, bool) {
func extractVariablesFromString(s, prefix string) ([]string, bool, string) {
pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, parameterSubstitution, parameterSubstitution)
re := regexp.MustCompile(pattern)
matches := re.FindAllStringSubmatch(s, -1)
errString := ""
if len(matches) == 0 {
return []string{}, false
return []string{}, false, ""
}
vars := make([]string, len(matches))
for i, match := range matches {
groups := matchGroups(match, re)
// foo -> foo
// foo.bar -> foo
// foo.bar.baz -> foo
for _, v := range []string{"var1", "var2", "var3"} {
val := strings.SplitN(groups[v], ".", 2)[0]
if strings.SplitN(groups[v], ".", 2)[0] != "" {
for j, v := range []string{"var1", "var2", "var3"} {
val := groups[v]
if j == 0 && strings.Contains(val, ".") {
errString = fmt.Sprintf(`Invalid referencing of parameters in %s !!! You can only use the dots inside single or double quotes. eg. $(params["org.foo.blah"]) or $(params['org.foo.blah']) are valid references but NOT $params.org.foo.blah.`, s)
// foo -> foo
// foo.bar -> foo
// foo.bar.baz -> foo
vars[i] = strings.SplitN(val, ".", 2)[0]
return vars, true, errString
}
if groups[v] != "" {
vars[i] = val
break
}

}
}
return vars, true
return vars, true, errString
}

func matchGroups(matches []string, pattern *regexp.Regexp) map[string]string {
Expand Down
27 changes: 27 additions & 0 deletions pkg/substitution/substitution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,33 @@ func TestValidateVariablePs(t *testing.T) {
vars: sets.NewString("baz"),
},
expectedError: nil,
}, {
name: "valid variable with double quote bracket and dots",
args: args{
input: "--flag=$(inputs.params[\"foo.bar.baz\"])",
prefix: "inputs.params",
vars: sets.NewString("foo.bar.baz"),
},
expectedError: nil,
}, {
name: "valid variable with single quote bracket and dots",
args: args{
input: "--flag=$(inputs.params['foo.bar.baz'])",
prefix: "inputs.params",
vars: sets.NewString("foo.bar.baz"),
},
expectedError: nil,
}, {
name: "valid variable with only dots",
args: args{
input: "--flag=$(inputs.params.foo.bar.baz)",
prefix: "inputs.params",
vars: sets.NewString("foo.bar.baz"),
},
expectedError: &apis.FieldError{
Message: `Invalid referencing of parameters in --flag=$(inputs.params.foo.bar.baz) !!! You can only use the dots inside single or double quotes. eg. $(params["org.foo.blah"]) or $(params['org.foo.blah']) are valid references but NOT $params.org.foo.blah.`,
Paths: []string{""},
},
}, {
name: "valid variable contains diffetent chars",
args: args{
Expand Down

0 comments on commit 4237895

Please sign in to comment.