No longer check if postgresql existingSecret has a custom hostname set #660
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jessebot
reviewed
Jun 7, 2025
| @@ -1,2 +1,3 @@ | |||
| .idea/ | |||
| .DS_Store | |||
| .aider* | |||
Collaborator
There was a problem hiding this comment.
Suggested change
| .aider* | |
| .aider* |
🤔 what's this?
jessebot
reviewed
Jun 7, 2025
Comment on lines
66
to
-74
| - name: DATABASE_HOSTNAME | ||
| {{- if not .Values.postgresql.global.postgresql.auth.existingSecret }} | ||
| value: {{ template "postgresql.v1.primary.fullname" .Subcharts.postgresql }} | ||
| {{ else }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "matrix.postgresql.secretName" . }} | ||
| key: {{ .Values.postgresql.global.postgresql.auth.secretKeys.databaseHostname }} | ||
| {{- end }} |
Collaborator
There was a problem hiding this comment.
will this break people using an external database though?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the first initContainer of the matrix-synapse deployment
postgresql-isreadytries to respect a potentialdatabaseHostnameenvironment variable, if.Values.postgresql.global.postgresql.auth.existingSecretis set. However, this had lead to a bit of a goose chase on my side, as trying to useexistingSecretwill produce the following error when trying to perform an ArgoCD sync:I found this issue really hard to debug, as running my Values-file from helm actually does not result in this issue. However, when playing around with a fork of this repo, I realized that the second initContainer
add-secret-vlaues-to-configdoes not respect thedatabaseHostbasein the existingSecret, meaning that if someone where to make the first initContainer working with a custom databaseHostname, they would then realize that the second initContainer will always ignore their custom hostname anyways.Therefore, I propose the "lazy fix" to also ignore a potential custom hostname in the first initContainer. This way, potential issues for users who just want to set a custom postgres password in
existingSecretwill be avoided. And it won't be a breaking change, as nobody could be using custom hostnames to begin with.