failed parsing on parameters with comma

[privatecloud-username]

Describe the bug
when a parameter with comma[,] is set via CLI leads to the following issue:
rpc error: code = Unknown desc = Error: failed parsing --set data: key "18 * * *" has no value

To Reproduce
If we cannot reproduce, we cannot fix! Steps to reproduce the behavior:

1. create an application:
   argocd app create guestbook --repo https://github.com/argoproj/argocd-example-apps.git --path helm-guestbook --dest-server https://kubernetes.default.svc --dest-namespace default
2. provide the following parameter via cli
   key=schedule
   value=0 6,18 * * *
   command variations tried:
   argocd app set guestbook -p 'gitGC.schedule=0 6,18 * * *'
   argocd app set guestbook -p gitGC.schedule='0 6,18 * * *'
3. check the error in the UI
   rpc error: code = Unknown desc = Error: failed parsing --set data: key "18 * * *" has no value
   If this property is present in the values yaml it will be set correctly but the error will be visible and will prevent the healty status of the application.

PS
real example with git helm chart for k8s.
for the easy reproduction, guestbook app is used.

Expected behavior
If the parameter is not in values yaml to be discarded
if there is such parameter to be set correctly and no additional issue to rise

Version

argocd: v1.0.0+c74ca22
  BuildDate: 2019-05-16T20:04:43Z
  GitCommit: c74ca22023cbee245cb83fb23224978fbe23703c
  GitTreeState: clean
  GoVersion: go1.11.4
  Compiler: gc
  Platform: darwin/amd64
argocd-server: v1.0.0+c74ca22
  BuildDate: 2019-05-16T20:04:21Z
  GitCommit: c74ca22023cbee245cb83fb23224978fbe23703c
  GitTreeState: clean
  GoVersion: go1.11.4
  Compiler: gc
  Platform: linux/amd64
  Ksonnet Version: 0.13.1

[alexec]

Thank you for a fantastic bug report. I've not confirmed it, and I think it is probably how the CLI arg parser works.

You can also achieve this using argocd app patch?

app patch guestbook --patch '[{"op": "add", "path": "/spec/source", "value": {"helm": {"parameters": [{"name": "gitGC.schedule", "value": "0 6,18 * * *"}]}}}]'

Alex

[dimitarKiryakov]

Also as far as I can see the application is in status healthy despite the error. Is this also somehow an issue, since the Application appears to be healthy when is actually not healthy?

[jannfis]

This seems to be an issue with Helm, since ArgoCD calls out the Helm CLI with --set to set application parameters, as far as my investigation went (see also helm/helm#1556).

One possible workaround would be to escape the comma, such as:

argocd app set guestbook -p 'gitGC.schedule=0 6\,18 * * *'

Something to discuss is, should Argo escape all commas in the parameters before doing the call to Helm CLI if the user did not already escape them?

[privatecloud-username]

hi @alexec ,
thank you for the suggestion.
currently, by using such an approach all other previously set properties are reset to default values.
if all properties are set with this command I think that we might hit some limitations related to command character length.
Best regards

[alexec]

I'm only providing a workaround, you can use a patch to add values to the app spec, but as pointed out, this really isn't the issue.

I think this should probably be fixed, and the quickest way to do that would be a community contribution. I think this is where it needs to be made:

argo-cd/util/helm/helm.go

Line 69 in ac3d12c

args = append(args, "--set", fmt.Sprintf("%s=%s", p.Name, p.Value))

[dimitarKiryakov]

Hi @alexec
Did you test it with such a yaml:

key1: a b c/,cba
key2:
    - subkey: val1
    - subkey: val2
key3: {subkey: val1, subkey2: val2}
key4: {subkey, subkey2}
key5:
   - a
   - b
key6: [val1, val2]

[alexec]

No. I have no plan to test this. @simster7 ?

[simster7]

@dimitarKiryakov @alexec This change was only meant to parse arguments set directly with argocd app set guestbook -p 'key=value' (Helm equivalent: helm upgrade mychart --set key=value) and not to parse arguments passed in from argocd app set guestbook --values (Helm equivalent: helm upgrade mychart -f). Such a yaml, if valid, should work regardless of what is discussed here.

If by your question you mean the effects of breaking out your yaml and testing the parameter sets one-by-one I found no issues after making a quick fix (#1732):

$ argocd app set guestbook  -p 'key1=a b c/,cba' -p 'key2[0].subkey=val1' -p 'key2[1].subkey=val2' -p 'key3.subkey=val1' -p 'key3.subkey2=val2' -p 'key4.subkey1=~' -p 'key4.subkey2=~' -p 'key6=[val1, val2]'
$ argocd app get guestbook --show-params
Name:               guestbook
Project:            default
Server:             https://kubernetes.default.svc
Namespace:          default
URL:                http://localhost:8080/applications/guestbook
Repo:               https://github.com/argoproj/argocd-example-apps.git
Target:
Path:               helm-guestbook
Sync Policy:        <none>
Sync Status:        Unknown
Health Status:      Healthy


NAME            VALUE
key1            a b c/\,cba
key2[0].subkey  val1
key2[1].subkey  val2
key3.subkey     val1
key3.subkey2    val2
key4.subkey1    ~
key4.subkey2    ~
key6            [val1\, val2]