bjw-s / helm-charts

A collection of Helm charts
https://bjw-s.github.io/helm-charts/
Apache License 2.0
606 stars 108 forks source link

fix(common): quote command and args strings #124

Closed maurits-funda closed 1 year ago

maurits-funda commented 1 year ago

Description of the change

Quote the command and args strings to support multiline arguments.

Changed

Quote the command and args strings in the main container.

Benefits

Without the quoting of the command and args strings, the following gives an error:

command:
  - "/bin/sh"
  - "-c"
args: |
  echo hello
  echo world

The error (which is quite vague):

Error: YAML parse error on cronjob/templates/render.yaml: error converting YAML to JSON: yaml: line 48: could not find expected ':'

args: > does work without this change, but that requires you to add semicolons to each line.

Possible drawbacks

If someone manually quoted the command or args string, it will now be double quoted.

Checklist

onedr0p commented 1 year ago

Like command, args should be a list. Try this instead.

args:
  - |
    echo hello
    echo world

For a more detailed explanation see https://stackoverflow.com/questions/33887194/how-to-set-multiple-commands-in-one-yaml-file-with-kubernetes

maurits-funda commented 1 year ago

You are right. However _mainContainer.tpl also accepts a string and then converts it to a list of one: https://github.com/bjw-s/helm-charts/blob/main/charts/library/common/templates/lib/controller/_mainContainer.tpl#L16.

I noticed that I've accidentally added the quote to command instead of args. I've now added it to both, since it applies in both cases.

bjw-s commented 1 year ago

Hi! Apologies for the delay in response... Devin is right that command and args should be a list of strings. I'll take look at the PR/behavior soon and see if I can align it a bit better