Skip to content

Commit

Permalink
Fix Helm parameters with comma
Browse files Browse the repository at this point in the history
This commit fixes Helm parsing of parameters values containing a comma.

The issue was first found as #1660, and fixed in #1720. However, commit
4e9772e, in #1865 removed the call to `cleanHelmParameters`, hence the
regression.
  • Loading branch information
olivierlemasle committed Sep 19, 2019
1 parent a57e37a commit f8b2a28
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
9 changes: 7 additions & 2 deletions util/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func (h *helm) Template(appName, namespace, kubeVersion string, opts *argoappv1.
}
templateOpts.values = append(templateOpts.values, p)
}
cleanHelmParameters(opts.Parameters)
for _, p := range opts.Parameters {
if p.ForceString {
templateOpts.setString[p.Name] = p.Value
Expand Down Expand Up @@ -183,9 +184,13 @@ func flatVals(input map[string]interface{}, output map[string]string, prefixes .
}
}

func cleanHelmParameters(params []string) {
func cleanHelmParameters(params []argoappv1.HelmParameter) {
re := regexp.MustCompile(`([^\\]),`)
for i, param := range params {
params[i] = re.ReplaceAllString(param, `$1\,`)
params[i] = argoappv1.HelmParameter{
Name: param.Name,
Value: re.ReplaceAllString(param.Value, `$1\,`),
ForceString: param.ForceString,
}
}
}
23 changes: 15 additions & 8 deletions util/helm/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,28 @@ func TestHelmTemplateReleaseName(t *testing.T) {
}

func TestHelmArgCleaner(t *testing.T) {
cleanArgs := []string{`--these-args`, `are-clean`, `--foo`, `bar`}
argsToBeCleaned := make([]string, len(cleanArgs))
cleanArgs := []argoappv1.HelmParameter{
{Name: "app", Value: `val`, ForceString: true},
{Name: "foo", Value: `bar`, ForceString: false},
}
argsToBeCleaned := make([]argoappv1.HelmParameter, len(cleanArgs))
copy(argsToBeCleaned, cleanArgs)

cleanHelmParameters(argsToBeCleaned)
assert.Equal(t, cleanArgs, argsToBeCleaned)
assert.EqualValues(t, cleanArgs, argsToBeCleaned)

dirtyArgs := []string{`--these-args`, `are-not, clean`, `--foo`, `b\,a,r`}
argsToBeCleaned = make([]string, len(dirtyArgs))
dirtyArgs := []argoappv1.HelmParameter{
{Name: "app", Value: `val`, ForceString: true},
{Name: "test", Value: `not, clean`, ForceString: false},
{Name: "foo", Value: `a\,b,c`, ForceString: true},
}
argsToBeCleaned = make([]argoappv1.HelmParameter, len(dirtyArgs))
copy(argsToBeCleaned, dirtyArgs)

cleanHelmParameters(argsToBeCleaned)
assert.NotEqual(t, cleanArgs, argsToBeCleaned)
assert.Contains(t, argsToBeCleaned, `are-not\, clean`)
assert.Contains(t, argsToBeCleaned, `b\,a\,r`)
assert.False(t, assert.ObjectsAreEqualValues(cleanArgs, argsToBeCleaned))
assert.Contains(t, argsToBeCleaned, argoappv1.HelmParameter{Name: "test", Value: `not\, clean`, ForceString: false})
assert.Contains(t, argsToBeCleaned, argoappv1.HelmParameter{Name: "foo", Value: `a\,b\,c`, ForceString: true})

}

Expand Down

0 comments on commit f8b2a28

Please sign in to comment.