Support template parameters in configmap/secret customData

[ASBishop]

Template parameter expansion has only been supported in template files themselves, and not in any other sources supplied in the customData. This patch extends support for expanding parameters in all customData strings.

The intent is to support template parameters in customServiceConfig data. A common use case is when a service's config parameter needs to be set to a value that can be templated, e.g. the service's own password.

Consider the glance service when it's configured to use cinder for a backend. The final configuration needs to specify the 'cinder_store_password', which currently requires two things.

1. The cloud admin will need to create a secret containing the setting, and reference it in the customServiceConfigSecrets.
2. The cloud admin will need to track down the actual value (i.e. glance's password) in order to put it in the secret.

An alternative approach would allow the cloud admin to use a template parameter, whereby glance's CR could reference the password like this:

glance:
  template:
    customServiceConfig: |
      [cinder_backend]
      cinder_store_password = {{ .ServicePassword }}

The only restriction is the service's controller must support the parameter, meaning it must include it in its templateParameters. If a CR references a parameter that isn't supported, the controller will log an error stating, 'map has no entry for key "BadParam"'.

[ASBishop]

I think a few operators would benefit from this feature.

[stuggi]

The only restriction is the service's controller must support the parameter, meaning it must include it in its templateParameters. If a CR references a parameter that isn't supported, the controller will log an error stating, 'map has no entry for key "BadParam"'.

I like the idea, just thinking how a user would know/where we document which parameters are supported by the controller. should those be documented in the CRD for the customServiceConfig parameter?

[fmount]

Thanks Alan,
it makes sense to me and it's actually a good improvement in terms of UX when we want to configure serviceA as a backend for serviceB (glance and cinder are good examples here).
In particular I like the fact that we can avoid building additional secrets (that should be eventually manually patched by a human operator if a referenced parameter changes), simplifying the documentation as well.
Also, this change maintains backward compatibility with what we had in the past (e.g. the customServiceConfigSecrets param is still relevant for many other reasons), and I guess we can document the use cases that can take advantage of this implementation.
In general I'm +1 to this change: unless we have objections on this I propose to move forward.

[ASBishop]

I agree we map need to document the list of template parameters supported by each operator. That said, I suspect the ones that Users will find useful will be relatively small, primarily consisting of things like service passwords. Using the example I described in the commit message, we already document how to configure glance to use cinder for a backend, so we'd just need to update the existing documentation. BTW, that's in the downstream documentation.

Another idea would be for controllers that know they support template parameters that Users might find useful to log the list of template parameter keys.

[Akrog]

I like the idea, but I'm somewhat concerned that doing the templating blindly like this could create issues.
For example a user could provide a secret using CustomServiceConfigSecrets that coincidentally contained templating formatting but were it was not meant to be templated by the operator.

[ASBishop]

That's in interesting corner case. One path forward would be to catch and log the resulting template failure, and fall back to include the raw customData line as is. This may not be ideal, but it would preserve the benefit of the feature without any havoc wrecked by corner cases.

[Akrog]

That assumes that there will never be a chance of having an accidental valid template.
I believe this scenario would be a corner case that we could just document.

[ASBishop]

I updated the commit message to reflect changes in the last update, but did not update the PR description. Pls see the commit message for the latest update.

[Akrog]

Looks good to me

[Akrog]

It should be possible to consolidate this code and the code for the config map into a single function, but I don't think that's a big deal.

[stuggi]

/lgtm