graphops/launchpad-charts

proxyd: allow to supply a complete ConfigMap

Closed this issue · 2 comments

cebidhem commented

Hi 👋

First, thanks a lot for publishing those helm charts !

For proxyd toml configuration file, the chart is templating the config based on some values. The thing is, some of those values may contain authentication credentials that would be best to not be input in plain text.

Would it be possible to allow for supplying an already created ConfigMap, and condition the creation of the computed CM with a flag ?
The idea would be to have a new value exstingConfigMap which default to null or "", but when filled don't create the CM.

If you'd be willing to accept a contribution, I can propose a PR for this.

Thanks for reading!

cjorge-graphops commented

Hi @cebidhem, and welcome 🙂

I've talked with the team about this, and we agree that the option of disabling the templating and override it with an existing ConfigMap is desirable and fits well.
As such, we very much encourage you to move ahead with a PR if you're willing.

Even though that based on the presence or absence of an existingConfigMap key, one can induce if configTemplating / backends logic should be enabled or not; We're thinking it should be done behind a proxyd.configTemplating.enabled flag, and not left to be inferred from existingConfigMap alone. The rational for that being that not only on this chart, but across all launchpad-charts, there's a recurring pattern of having "enabled" true/false" as feature flags and would be more consistent that way.

This also raises the question if it should be changed to a Secret as well, instead of a ConfigMap, but we'll handle that question as a different issue.

Thank you for the feedback and initiative 🙌

  • 👍1
cebidhem commented

Thanks for the feedback on how you'd prefer this to be handled, having guidelines to keep it consistent is super helpful !

This also raises the question if it should be changed to a Secret as well, instead of a ConfigMap, but we'll handle that question as a different issue.

I'm fine either way. I didn't want to change the current logic hence the ConfigMap proposal, but I agree a Secret makes more sense for the use-case.

I'll create the PR for this issue and as you mentioned, let's manage the Secret part in another one !