configmaps to secrets, missing db config in inject block and ingress example structure.

[ricosega]

Hope this changes can be useful:

• Changed configmaps to secrets at inject block, so in values.yaml they must be added as base64.
• Added missing configuration in values.yaml in the inject block. Lines from 81-84. Without this default db config it won't work because of a http panic server bug.
• At the end of values.yaml, added an ingress example structure. Lines from 220-222.

[maraino]

Certs are not "secrets" meaning that they are public, but it can make sense to store them in a secret

@estenrye, you did the injected changes, what do you think?

[estenrye]

Commented on secrets.yaml.

@maraino I don't have any objection to this change, but would be very interested to hear why @ricosega believes these values should move from ConfigMaps to Secrets.

[estenrye]

@maraino I don't have an issue with this moving from ConfigMaps to Secrets if @ricosega has a compelling reason for doing so. However, part of the reason I didn't when I introduced the inject capability was that I didn't have a security reason for doing so. If Kubernetes supported TLS ingress configurations with key passwords, I would have merged these two configuration objects into a single secret to take advantage of that capability. But without that support, I kept them separate to discourage people from creating root certificates without passwords to leverage the TLS Ingress capability.

At the end of the day, my thought was that I was going to end up sprinkling the CA certificates absolutely everywhere in my environment so why does it need to be secret.

I don't have any objection to this change, but would be very interested to hear why @ricosega thinks it should move.

[ricosega]

I agree when we say that a cert is public, the point is that probably many people can use a helm chart in a CI/CD environment, so they will have to substitute variables in the values.yaml and here it comes the problem.

A certificate or a key, because of their structure and length are difficult to store in an environment variable to be replaced later, if you try it you will get an error. The only way to do it is to save the certificate in oneline and then do tricks and things to have it the same way. Instead if you save your certificate or key in base64 in oneline it will always preserves the structure and it is easier then to replace it.

[maraino]

@ricosega Moving the ConfigMaps to secrets might be a breaking change for some users. Can you investigate if storing the data as binaryData in the ConfigMap will solve your use case?
See https://kubernetes.io/docs/concepts/configuration/configmap/#configmap-object

PS: I've been on vacation.

[ricosega]

Sorry for the delay I've been on vacation too.
If we do not want to create a breaking change to any current user the only thing we can do is to make it retrocompatible, keeping the previous config and adding this new one for example with a value like, isBase64Encoded = bool.
What do you think?

[estenrye]

@ricosega I think its best not to introduce a breaking change for this. I would be open to seeing a isBase64Encoded option that allows the user to pass their value base64 encoded with the value of said flag defaulting to false. The helm chart could then decode the value and place it in the configmap.

Also, you may want to update your branch to the latest master. Support for registration authorities was added while you were away and the latest version of the chart is now 1.16.1

[estenrye]

One note on the version bump.

Since this PR changed the expected format of several of the values parameters, we may want to bump a minor version instead of a patch version if anyone has taken a dependency on 1.15.17.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}