`initdb.scriptsConfigMap` might be using a non-resolvable default

[pat-s]

Thanks for this chart!

I think the following line must be updated, the default name resolution didn't work in my case

https://github.com/small-hack/matrix-sliding-sync-chart/blob/541b2dd4e99c70b2f77d7b4284ee61043f00ec63/charts/matrix-sliding-sync/values.yaml#L188

I.e. {{ .Release.Name }} didn't result in a name which could be resolved. I finally set it to scriptsConfigMap: "sliding-sync-matrix-sliding-sync-postgresql-initdb" because fullName (with the chart default) also didn't work.

Did not spend too much time looking into it, maybe you want to 🙃

[jessebot]

Thanks for pointing this out! Full name should work because that's what's listed here:

https://github.com/small-hack/matrix-sliding-sync-chart/blob/541b2dd4e99c70b2f77d7b4284ee61043f00ec63/charts/matrix-sliding-sync/templates/initdb-configmap.yaml#L5

Perhaps I can add an option to deploy the initdb configmap optionally and allow the user to set the name themselves? 🤔 I'll play with it unless you have any further suggestions or would like to work on this 🙏

[jessebot]

How does this change feel? It removes the configMap entirely and instead templates out the script directly:

https://github.com/small-hack/matrix-sliding-sync-chart/blob/25d062e68303bb389f2bb8307b52c6cde84a63ca/charts/matrix-sliding-sync/values.yaml#L182-L195

Otherwise, we have to keep hardcode a specific name for the ConfigMap, which could require templating in the values.yaml, and that really shouldn't be done anyway as that's almost always a recipe for failure if a user uses something like Argo CD or another gitops solution that also supports goTemplating. The other alternative, which I mentioned earlier is to have the user specify a name of a configMap and then we deploy the configMap with that name, but that also means that a user can't deploy their own configMap, which is kind of the state of things right now. I think this is a fair compromise. We could also keep this, but comment it out and leave a note to un-comment it to do basic initDB script creation?

[pat-s]

Full name should work because that's what's listed here:

Yes, I thought so as well - likely I made a mistake then. At least, .Release.Name didn't work 😄

How does this change feel?

Not much different than the previous one (when it is fixed). Not meant in a negative way.

One thing you could consider (which I see in other charts usually) is that the full initdb logic must be provided in a secret (with an example commented included). Right now you have the downside of having a dependency on username and database as noted - which will at some point cause troubles.

[jessebot]

Thank you for the feedback :)

What if we commented what I have changed in that PR above and added the note that it needs to be uncommented if you'd like to do the initial database setup as you suggest? We can also mention in the comment that you're also free to store this info in a ConfigMap or Secret and point to the other parameters you'd configure for using those? Have I hit the best of all three worlds? 😄