[Feature] Added multiple values files support for release

[rodcloutier]

This PR intends to add support for external multiple value "files".
As it stands right now, the only way to use yaml values is to embed the values in the the terraform file.
With this PR, we can reference external use multiple files mimicking the helm install -f <file> functionality.

The file are specified in a list and will, as with Helm, be concatenated in order which makes value override possible. The order of resolution is:

1. The terraform values content merged in order
2. The set values

Usage is:

values = [ "foo: bar", "${file("values.yaml")}" ]

Detection of changes in the file content is done by storing a md5 hash of the content as validating with the current local file. If it does not match, the release is flagged to be updated.

Note: This is an API breaking change.

[mcuadros]

The tests are not passing, see Travis:
resource_release_test.go:304: error merging values from file, expected value file "testdata/get_values_first.yaml" not read

[rodcloutier]

I saw that. I wonder why because, obviously, it works on my machine... I will try to figure it out.

[burdiyan]

@rodcloutier Using external values files is currently possible using terraform's file function and release's raw_values property.

Check the following comment for details: #45 (comment)

To be honest, I think there is not need for additional parameter allowing to specify path to a file.

[rodcloutier]

Thanks for your comment @burdiyan.

I am aware of the file in the raw_values technique and we have been using it.

To be a little clearer, the intent of the PR is not add an additional parameter allowing to specify path to a file but an additional parameter to specify paths to multiple files.

We currently have multiple releases that are sharing common values. Helm allows to use several file in its command and will concatenate the values with the files. Thus we can have a common value file and a specific for each release.

We then use them as such

helm install my_chart_1 -f common.yaml -f specific_1.yaml`
helm install my_chart_2 -f common.yaml -f specific_2.yaml`

We would like to have the same functionality in the provider, hence this PR.

Does it make more sense?

Note: I have renamed the PR to clarify the intent.

[burdiyan]

@rodcloutier Oh, that makes sense. Thanks! Although I guess you could achieve this by doing something like this:

raw_values = <<<EOF
${file("values-1.yaml")}
${file("values-2.yaml")}
EOF

Although a bit hackie it may work. I think the problem may come if you have some clashes in variables in these multiple files, but I invite you to try it out and let us know :)

[madmod]

@burdiyan I have attempted to make TF modules which use the pattern you have suggested and have experienced the problem of being unable to merge deeply nested values. I attempted this in order to be able to take general public charts and apply my company specific defaults in a Terraform module, while still allowing TF that uses the module to set project specific values. Most charts have large root keys such as web and database making it impossible to override something simple like setting an ingress host and TLS configuration without reimplementing any defaults and dealing with indentation issues. (I know about set {} but it is too verbose for many deeply nested keys.)

This PR is a good solution to the problem of using multiple files for values and it also solves the problem of merging deeply nested keys. I would love to see this adapted to support merging multiple values strings in Terraform so that it would be able to merge values from Terraform state such as hostnames, credentials, etc. without persisting them to a local file.

Personally I try to avoid using external files in my Terraform state because they can't support a multi developer workflow since they are not sourced from the locked remote state. (Two developers working on a file at once would overwrite uncommitted/unpulled changes on apply.)

I think that modifying values to also accept an array which is deeply merged would be a more generally applicable solution because it would also support multiple sections of inline YAML. Then for @rodcloutier's use case you could do something like:

values = [
  "${file("common.yaml")}",
  "${file("specific_1.yaml")}"
]

And have it be properly merged like Helm does by default.

[mcuadros]

I like @madmod suggestion, we should avoid read directly a file, and instead of this use file function. So then converting values into a slice of YAMLs solve the problem of having multiple sources.

@rodcloutier what do you think about this approach?

[burdiyan]

I like @madmod approach. I’m not sure it’s possible to make this without breaking compatibility or using a separate parameter for this.

It may also be a good idea to persist compiled and merged values from both set {} and raw values in the state file, but it probably worth a separate discussion.

[rodcloutier]

Interesting discussion. @madmod approach is quite interesting. I am also on the side of @burdiyan in the fact that breaking compatibility is an issue. I don't think you can have "polymorphic" values in a resource, nor that we want to handle that case.

The question is how strict we want to be with compatibility break? There as already been some breaks (the recent rename) and we are not at a 1.0 release yet. We could move forward and enhance the provider at the cost of a little rework for the existing users.

Too bad we had a rename from raw_values to values, it would have been easier to deprecate raw_values ;)

@burdiyan The PR also has the compiled and merged values in the metadata since Helm already provides this information.

[mcuadros]

We are in 0.x.x based on semver this means:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable

[rodcloutier]

If we are ok with @madmod suggestion, I would go ahead and change the PR to implement it.