search

vinzscam / backstage-chart

Backstage Helm Chart
27 stars 10 forks source link

feat: define extra app config which generates ConfigMap #24

Closed froblesmartin closed 1 year ago

froblesmartin commented 1 year ago

As it was been discussed a little bit at https://github.com/vinzscam/backstage-chart/issues/4:

I think it would be nice to give the possibility to define an extra app config file in the values.yaml, which then creates a ConfigMap automatically, mounts it and uses it in the config.

Please, let me know your thoughts, and if there is anything I should improve or change. :)

vinzscam commented 1 year ago

Hi @froblesmartin, Thank you for your contribution! ❤️

The chart already has a functionality for passing extra app-config files, as described in Pass extra configuration files.

Try it out and let me know if you find any issue.

froblesmartin commented 1 year ago

Hi @froblesmartin, Thank you for your contribution! ❤️

The chart already has a functionality for passing extra app-config files, as described in Pass extra configuration files.

Try it out and let me know if you find any issue.

Hi @vinzscam ! Thanks a lot for your comment!

The problem with the current approach is that the ConfigMap needs to be provided externally from this Helm chart. It is not a big deal, but I think it would be nice to provide both things, generating the ConfigMap in the Helm chart itself, and also allowing us to define the config in the values.yaml file. In the end the idea of the values.yaml file is to be the place where the applications can be configured.

Maybe we could generate the ConfigMap from multiple keys which map to the possible configurations of Backstage and then generate the ConfigMap with all that input, but that is a different approach, and would require to keep in sync always the Helm chart values.yaml and template for the ConfigMap with the configurations of Backstage.

vinzscam commented 1 year ago

Hi @froblesmartin, Thank you for your contribution! ❤️ The chart already has a functionality for passing extra app-config files, as described in Pass extra configuration files. Try it out and let me know if you find any issue.

Hi @vinzscam ! Thanks a lot for your comment!

The problem with the current approach is that the ConfigMap needs to be provided externally from this Helm chart. It is not a big deal, but I think it would be nice to provide both things, generating the ConfigMap in the Helm chart itself, and also allowing us to define the config in the values.yaml file. In the end the idea of the values.yaml file is to be the place where the applications can be configured.

Maybe we could generate the ConfigMap from multiple keys which map to the possible configurations of Backstage and then generate the ConfigMap with all that input, but that is a different approach, and would require to keep in sync always the Helm chart values.yaml and template for the ConfigMap with the configurations of Backstage.

the only concern I have with this approach is that it looks easier to include sensitive variables in the values.yaml, which isn't something ideal. Perhaps we could mention this in the README? Also, what about renaming extraAppConfigToConfigMap to just appConfig?

froblesmartin commented 1 year ago

Hi @froblesmartin, Thank you for your contribution! ❤️ The chart already has a functionality for passing extra app-config files, as described in Pass extra configuration files. Try it out and let me know if you find any issue.

Hi @vinzscam ! Thanks a lot for your comment! The problem with the current approach is that the ConfigMap needs to be provided externally from this Helm chart. It is not a big deal, but I think it would be nice to provide both things, generating the ConfigMap in the Helm chart itself, and also allowing us to define the config in the values.yaml file. In the end the idea of the values.yaml file is to be the place where the applications can be configured. Maybe we could generate the ConfigMap from multiple keys which map to the possible configurations of Backstage and then generate the ConfigMap with all that input, but that is a different approach, and would require to keep in sync always the Helm chart values.yaml and template for the ConfigMap with the configurations of Backstage.

the only concern I have with this approach is that it looks easier to include sensitive variables in the values.yaml, which isn't something ideal. Perhaps we could mention this in the README? Also, what about renaming extraAppConfigToConfigMap to just appConfig?

Hi @vinzscam ! Sorry for the long delay.

I am closing this one on behalf of https://github.com/backstage/charts/pull/23. I changed there the key used to just appConfig, let's keep the conversation there ;)