[bitnami/cassandra] Making TLS existingSecrets more configurable #28746
Conversation
tbalzer
force-pushed
the
cassandra-make-tls-more-configurable
branch
5 times, most recently
from
7230aa0
to
655999d
Compare
|
@carrodher I'd appreciate any help you can give me with getting the pipeline to succeed. I have invited @bitnami-bot to the repository which could be accepted to resolve this. Unfortunately the log of the changelog job doesn't show the diff which I could apply and I seem to be unable to locally reproduce what this job generates. |
carrodher
added
verify
| {{- if (include "cassandra.createTlsSecret" . ) }} | ||
| secret: | ||
| secretName: {{ include "cassandra.tlsSecretName" . }} | ||
| secretName: {{ include "cassandra.keystoreSecretName" . }} | ||
| defaultMode: 256 | ||
| {{- else }} | ||
| projected: | ||
| sources: | ||
| - secret: | ||
| name: {{ include "cassandra.truststoreSecretName" . }} | ||
| items: | ||
| - key: {{ .Values.tls.secretKeys.truststore }} | ||
| path: truststore | ||
| mode: 256 | ||
| - secret: | ||
| name: {{ include "cassandra.keystoreSecretName" . }} | ||
| items: | ||
| - key: {{ .Values.tls.secretKeys.keystore }} | ||
| path: keystore | ||
| mode: 256 | ||
| {{- end }} |
There was a problem hiding this comment.
What about using projected in both cases? As far as cassandra.truststoreSecretName returns cassandra.keystoreSecretName when tls.existingTrustSecret is empty it should be fine.
Because this PR adds tls.secretKeys.keystore and tls.secretKeys.truststore, we can no longer assume files will be named 'keystore' and 'truststore', so using projected would fix that issue.
There was a problem hiding this comment.
The created secret from https://github.com/bitnami/charts/blob/main/bitnami/cassandra/templates/tls-secret.yaml still uses hardcoded keys, as those are then not in JKS format. This is then handled differently in the init-container.
I think it wouldn't make sense there to use a projected secret then?
There was a problem hiding this comment.
@migruiz4 It's been a month now, any thoughts?
|
This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution. |
|
Hey @migruiz4 , this is getting stale right now. Anything I can do to move this forward? |
|
Sorry for the delay! Please note a new version was released in the meantime, you need to rebase from main and bump the chart version in the Chart.yaml again |
tbalzer
force-pushed
the
cassandra-make-tls-more-configurable
branch
from
0ba748f
to
d89e9e8
Compare
I've rebased and updated accordingly. |
Signed-off-by: Tim Balzer <[email protected]>
Co-authored-by: Miguel Ruiz <[email protected]> Signed-off-by: Tim Balzer <[email protected]>
Signed-off-by: Tim Balzer <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
tbalzer
force-pushed
the
cassandra-make-tls-more-configurable
branch
from
d89e9e8
to
00f107a
Compare
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Tim Balzer <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
|
@carrodher @migruiz4 Can I get a review here? |
Signed-off-by: Tim Balzer <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Description of the change
Added values to allow for more configuration of the TLS settings, e.g. alternate secret keys, split secrets.
Benefits
E.g. interoperability with cert-manager and trust-manager.
Possible drawbacks
None really?
Applicable issues
Additional information
TLS configuration is not currently covered by automated tests.
Checklist
Chart.yamlaccording to semver. This is not necessary when the changes only affect README.md files.README.mdusing readme-generator-for-helm