amazon-archives / aws-service-operator

AWS Service Operator allows you to create AWS resources using kubectl.
Apache License 2.0
732 stars 97 forks source link

Fix Helm Chart so extraArgs and extraEnv render correctly #151

Open mattparkes opened 5 years ago

mattparkes commented 5 years ago

Issue: https://github.com/awslabs/aws-service-operator/issues/150

Description of changes: Fix Helm Chart so extraArgs and extraEnv render correctly.

Testing: Can test/validate by using a values file (test.yaml) whose contents are exactly as per the default values.yaml, but with the addition of testing --key=value extraArgs items render properly:

extraEnv:
  - name: my_env
    value: my_value

extraArgs:
  - --help
  - --foo=bar

helm template ./aws-service-operator/ -f test.yaml

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

mattparkes commented 5 years ago

I have not incremented the version in Chart.yaml.

Let me know if you think it is preferred/appropriate to do so and I'll increment it to 0.0.2.

mattparkes commented 5 years ago

A colleague of mine just pointed out that technically the existing template did work if you used:

extraEnv:
  my_env: my_value

So rather than changing the template, I could have just changed the default values.yaml file to something like the above.

extraArgs doesn't seem to work no matter the format though:

extraArgs:
 - help
 - foo: bar

renders as:

args:
- --0=help
- --1=map[foo:bar]

So a change is still needed to fix that.

I guess this totally a stylistic choice, so if anyone has a strong preference, I'm happy to adjust this PR to suit.

christopherhein commented 5 years ago

So sounds like we can actually just change the Values.yaml to represent extraEnv.my_env: my_value instead and that will solve the first issue? Rather than changing the template?

And the second change lgtm. Would you mind just going back to using the $key $value reference and we'll be good to merge? Thanks!