-
Notifications
You must be signed in to change notification settings - Fork 169
Add support for additionalCommandArgs and configurable secretKey fields in barmanObjectStore. (fixes #653) #654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ds in barmanObjectStore. Signed-off-by: Austin Marten <[email protected]>
+1 |
+1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
I appreciate exposing the additional functionality via values to save having to fork the chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that if custom S3 credential key fields are set and a user passes S3 creds directly in the values (such as backups.s3.accessKey
and backups.s3.secretKey
), the generated secret will still use the hardcoded ACCESS_KEY_ID
and ACCESS_SECRET_KEY
strings, causing a mismatch in the barman storage config.
The same applies to the recovery S3 secret.
Additionally, this change doesn't impact the barman config when recovery is configured in the chart.
You would need to add the same fields from .Values.backups.secret
to .Values.recovery.secret
.
(I guess recovery is technically out of scope here, but this would also greatly help my use case 😃)
Helps my use case as well! I can add it in here. |
Signed-off-by: Austin Marten <[email protected]>
974431c
to
c504163
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, small nitpick about formatting.
charts/cluster/values.yaml
Outdated
# Definable accessKeyIdField to use (Left blank defaults to ACCESS_KEY_ID) | ||
accessKeyIdField: "" | ||
# Definable accessKeyIdField to use (Left blank defaults to ACCESS_SECRET_KEY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have comments be in the format of # -- My comment
like the rest to be consistent.
Done |
Signed-off-by: Austin Marten <[email protected]>
c821550
to
9f8d3dd
Compare
Add accessKeyIdField and secretAccessKeyField to backups.secret for custom S3 credential key fields
Add additionalCommandArgs support for both wal and data sections in barmanObjectStore
Fixes Add support for additionalCommandArgs and configuratable S3 secret keys. #653