From 61ef119c1d153aec45835d9ff314a0172fda05a3 Mon Sep 17 00:00:00 2001 From: astro-stan <36302090+astro-stan@users.noreply.github.com> Date: Wed, 15 Oct 2025 15:01:15 +0100 Subject: [PATCH] feat(common): Add support for specifying default S3 region when it cannot be determined from the URL (#40668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description** This PR adds support for specifying a default S3 region when it cannot be determined from the URL, or the underlying backup/restore solution (or S3 server) does not support redirects to the correct region. See Notes section for more information ⚒️ Fixes # **⚙️ Type of change** - [X] ⚙️ Feature/App addition - [ ] 🪛 Bugfix - [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] 🔃 Refactor of current code - [ ] 📜 Documentation Changes **🧪 How Has This Been Tested?** What I have tested: - Rendered a chart with credentials that specify the new region key - Deployed the chart - Observed CNPG backups succeeding (they were failing if I did not specify the region). See notes for more information - Observed Volsync backups succeeding **📃 Notes:** **Regarding the CNPG backups failiure:** I am using a LAN IP address for the S3 backup endpoint. While Volsync can work with just that, CNPG backups were failing. I am not 100% sure what the underlying reason is. Still, from brief research on Google, I suspect the reason CNPG fails and Volsync doesn't is because Volsync can follow a redirect to the correct region by the S3 server, while CNPG's barman cannot. In the S3 server logs I see that CNPG tries to use `us-east-1` as the region, which the server rejects. I tried alternative solutions such as placing the region in the URL path, however, that did not work. From the S3 server docs (which just reference AWS S3 docs for the most part), I gather that a region can only be specified in the URL as a subdomain - i.e. `my-region.my-endpoint.com/my-bucket`. Since I am using a LAN IP to connect to the server, I cannot specify a subdomain. Adding the region in the CNPG config makes it "default" to the correct region, and backup succeeds. **Regarding CNPG's migration to the plugin-based barman**: I know this project will need to migrate to the CNPG's new plugin-based barman. However, as shown by the [migration guide](https://cloudnative-pg.io/plugin-barman-cloud/docs/migration/#example), similarly to my previous PRs (#40523, #40385), the `region` value can just be migrated to the new `ObjectStore` CRD like so: ```yaml ... barmanObjectStore: endpointURL: ... ... s3Credentials: region: name: secret-name key: REGION ``` Migrates to: ```yaml apiVersion: barmancloud.cnpg.io/v1 kind: ObjectStore ... spec: configuration: endpointURL: ... ... s3Credentials: region: name: secret-name key: REGION ``` --- - [CNPG CRD for the region key](https://github.com/cloudnative-pg/cloudnative-pg/blob/31d5f7acabd75e4b7fdc3ebe61c2cbf9c404f8b1/config/crd/bases/postgresql.cnpg.io_clusters.yaml#L1258-L1271) - [Restic docs for specifying a default region](https://restic.readthedocs.io/en/stable/040_backup.html#environment-variables:~:text=AWS_DEFAULT_REGION%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20Amazon%20S3%20default%20region) **✔️ Checklist:** - [X] ⚖️ My code follows the style guidelines of this project - [X] 👀 I have performed a self-review of my own code - [X] #️⃣ I have commented my code, particularly in hard-to-understand areas - [ ] 📄 I have made changes to the documentation - [X] 🧪 I have added tests to this description that prove my fix is effective or that my feature works - [X] ⬆️ I increased versions for any altered app according to semantic versioning - [X] I made sure the title starts with `feat(chart-name):`, `fix(chart-name):`, `chore(chart-name):`, `docs(chart-name):` or `fix(docs):` **➕ App addition** If this PR is an app addition please make sure you have done the following. - [ ] 🖼️ I have added an icon in the Chart's root directory called `icon.png` --- _Please don't blindly check all the boxes. Read them and only check those that apply. Those checkboxes are there for the reviewer to see what is this all about and the status of this PR with a quick glance._ --- .../backup_provider_secret_spec_test.yaml | 44 ++++++++++ .../tests/cnpg/cluster_backup_spec_test.yaml | 78 ++++++++++++++++++ .../cnpg/cluster_recovery_spec_test.yaml | 82 +++++++++++++++++++ .../volsync/replication_dest_spec_test.yaml | 70 ++++++++++++++++ .../volsync/replication_src_spec_test.yaml | 77 +++++++++++++++++ charts/library/common/Chart.yaml | 3 +- .../lib/cnpg/barmanObjectStore/_s3.tpl | 6 ++ .../templates/lib/cnpg/providers/_s3.tpl | 3 + .../templates/lib/credentials/_validation.tpl | 5 ++ .../library/common/templates/spawner/_pvc.tpl | 21 +++-- charts/library/common/values.yaml | 3 + 11 files changed, 382 insertions(+), 10 deletions(-) diff --git a/charts/library/common-test/tests/cnpg/backup_provider_secret_spec_test.yaml b/charts/library/common-test/tests/cnpg/backup_provider_secret_spec_test.yaml index bae608b43ba..f8811f6c588 100644 --- a/charts/library/common-test/tests/cnpg/backup_provider_secret_spec_test.yaml +++ b/charts/library/common-test/tests/cnpg/backup_provider_secret_spec_test.yaml @@ -47,6 +47,50 @@ tests: ACCESS_SECRET_KEY: some-secret-key ACCESS_KEY_ID: some-access-key + - it: should create secret for s3 provider with overridden region + set: + credentials: + test: + type: s3 + url: http://some-url + region: some-region + encrKey: some-encr-key + bucket: some-bucket + secretKey: some-secret-key + accessKey: some-access-key + cnpg: + my-pg: + enabled: true + user: test-user + database: test-db + password: test-password + backups: + enabled: true + target: primary + retentionPolicy: 10d + credentials: test + destinationPath: some-path + manualBackups: + - name: today + asserts: + - documentIndex: *secretDoc + isKind: + of: Secret + - documentIndex: *secretDoc + isAPIVersion: + of: v1 + - documentIndex: *secretDoc + equal: + path: metadata.name + value: test-release-name-common-test-cnpg-my-pg-provider-backup-s3-creds + - documentIndex: *secretDoc + equal: + path: stringData + value: + REGION: some-region + ACCESS_SECRET_KEY: some-secret-key + ACCESS_KEY_ID: some-access-key + - it: should create secrets for s3 provider with customCA set: credentials: diff --git a/charts/library/common-test/tests/cnpg/cluster_backup_spec_test.yaml b/charts/library/common-test/tests/cnpg/cluster_backup_spec_test.yaml index 4b0ebc586db..dd7e451878b 100644 --- a/charts/library/common-test/tests/cnpg/cluster_backup_spec_test.yaml +++ b/charts/library/common-test/tests/cnpg/cluster_backup_spec_test.yaml @@ -78,6 +78,84 @@ tests: retentionPolicy: 10d target: primary + - it: should generate correct spec with backups and overridden region (s3) + set: + credentials: + test: + type: s3 + accessKey: some-access-key + secretKey: some-secret-key + bucket: some-bucket + encrKey: some-encr-key + url: http://some-url + region: some-region + cnpg: + my-pg: + enabled: true + user: test-user + database: test-db + password: test-password + backups: + enabled: true + target: primary + retentionPolicy: 10d + credentials: test + destinationPath: some-path + asserts: + - documentIndex: *secretDoc + isKind: + of: Secret + - documentIndex: *secretDoc + isAPIVersion: + of: v1 + - documentIndex: *secretDoc + equal: + path: metadata.name + value: test-release-name-common-test-cnpg-my-pg-provider-backup-s3-creds + - documentIndex: *secretDoc + equal: + path: stringData + value: + REGION: some-region + ACCESS_KEY_ID: some-access-key + ACCESS_SECRET_KEY: some-secret-key + - documentIndex: *clusterDoc + isKind: + of: Cluster + - documentIndex: *clusterDoc + isAPIVersion: + of: postgresql.cnpg.io/v1 + - documentIndex: *clusterDoc + equal: + path: metadata.name + value: test-release-name-common-test-cnpg-my-pg + - documentIndex: *clusterDoc + isSubset: + path: spec + content: + backup: + barmanObjectStore: + data: + compression: bzip2 + jobs: 2 + destinationPath: some-path + endpointURL: http://some-url + s3Credentials: + region: + key: REGION + name: test-release-name-common-test-cnpg-my-pg-provider-backup-s3-creds + accessKeyId: + key: ACCESS_KEY_ID + name: test-release-name-common-test-cnpg-my-pg-provider-backup-s3-creds + secretAccessKey: + key: ACCESS_SECRET_KEY + name: test-release-name-common-test-cnpg-my-pg-provider-backup-s3-creds + serverName: test-release-name-common-test-cnpg-my-pg + wal: + compression: bzip2 + retentionPolicy: 10d + target: primary + - it: should generate correct spec with backups using customCA (s3) set: credentials: diff --git a/charts/library/common-test/tests/cnpg/cluster_recovery_spec_test.yaml b/charts/library/common-test/tests/cnpg/cluster_recovery_spec_test.yaml index dc9e7cbf6ba..23b37ad1aa9 100644 --- a/charts/library/common-test/tests/cnpg/cluster_recovery_spec_test.yaml +++ b/charts/library/common-test/tests/cnpg/cluster_recovery_spec_test.yaml @@ -154,6 +154,88 @@ tests: key: ACCESS_SECRET_KEY name: test-release-name-common-test-cnpg-my-pg-provider-recovery-s3-creds + - it: should generate correct spec with recovery/objectStore and overridden region (s3) + set: + credentials: + test: + type: s3 + accessKey: some-access-key + secretKey: some-secret-key + bucket: some-bucket + encrKey: some-encr-key + url: https://s3.some-region.amazonaws.com + region: some-region + path: some-path + cnpg: + my-pg: + enabled: true + user: test-user + database: test-db + password: test-password + mode: recovery + recovery: + method: object_store + credentials: test + asserts: + - documentIndex: *secretDoc + isKind: + of: Secret + - documentIndex: *secretDoc + isAPIVersion: + of: v1 + - documentIndex: *secretDoc + equal: + path: metadata.name + value: test-release-name-common-test-cnpg-my-pg-provider-recovery-s3-creds + - documentIndex: *secretDoc + equal: + path: stringData + value: + REGION: some-region + ACCESS_KEY_ID: some-access-key + ACCESS_SECRET_KEY: some-secret-key + - documentIndex: *clusterDoc + isKind: + of: Cluster + - documentIndex: *clusterDoc + isAPIVersion: + of: postgresql.cnpg.io/v1 + - documentIndex: *clusterDoc + equal: + path: metadata.name + value: test-release-name-common-test-cnpg-my-pg + - documentIndex: *clusterDoc + isSubset: + path: spec + content: + bootstrap: + recovery: + secret: + name: test-release-name-common-test-cnpg-my-pg-user + owner: test-user + database: test-db + source: test-release-name-common-test-cnpg-my-pg + - documentIndex: *clusterDoc + isSubset: + path: spec + content: + externalClusters: + - name: test-release-name-common-test-cnpg-my-pg + barmanObjectStore: + serverName: test-release-name-common-test-cnpg-my-pg + destinationPath: s3://some-bucket/some-path/test-release-name/cnpg + endpointURL: https://s3.some-region.amazonaws.com + s3Credentials: + region: + key: REGION + name: test-release-name-common-test-cnpg-my-pg-provider-recovery-s3-creds + accessKeyId: + key: ACCESS_KEY_ID + name: test-release-name-common-test-cnpg-my-pg-provider-recovery-s3-creds + secretAccessKey: + key: ACCESS_SECRET_KEY + name: test-release-name-common-test-cnpg-my-pg-provider-recovery-s3-creds + - it: should generate correct spec with recovery/objectStore using customCA (s3) set: credentials: diff --git a/charts/library/common-test/tests/volsync/replication_dest_spec_test.yaml b/charts/library/common-test/tests/volsync/replication_dest_spec_test.yaml index 787cfafa755..0fe0db9d17d 100644 --- a/charts/library/common-test/tests/volsync/replication_dest_spec_test.yaml +++ b/charts/library/common-test/tests/volsync/replication_dest_spec_test.yaml @@ -75,6 +75,76 @@ tests: runAsUser: 568 runAsGroup: 568 + - it: should generate correct spec with overridden region + set: + persistence: + destbackup: + enabled: true + type: pvc + mountPath: /backedup + volsync: + - name: mybackup1 + type: restic + credentials: mys3 + dest: + enabled: true + src: + enabled: false + credentials: + mys3: + type: s3 + url: https://s3.some-url + bucket: some-bucket + encrKey: some-key + accessKey: some-access-key + secretKey: some-secret-key + region: some-region + asserts: + - documentIndex: *secretDoc + isKind: + of: Secret + - documentIndex: *secretDoc + isAPIVersion: + of: v1 + - documentIndex: *secretDoc + equal: + path: metadata.name + value: test-release-name-common-test-destbackup-volsync-mybackup1 + - documentIndex: *secretDoc + equal: + path: stringData + value: + AWS_ACCESS_KEY_ID: some-access-key + AWS_SECRET_ACCESS_KEY: some-secret-key + RESTIC_PASSWORD: some-key + RESTIC_REPOSITORY: s3:s3.some-url/some-bucket/test-release-name/volsync/destbackup-volsync-mybackup1 + AWS_DEFAULT_REGION: some-region + - documentIndex: *replicationDestDoc + isKind: + of: ReplicationDestination + - documentIndex: *replicationDestDoc + isAPIVersion: + of: volsync.backube/v1alpha1 + - documentIndex: *replicationDestDoc + equal: + path: spec + value: + trigger: + manual: restore-once + restic: + repository: test-release-name-common-test-destbackup-volsync-mybackup1 + copyMethod: Snapshot + cleanupTempPVC: false + cleanupCachePVC: false + cacheCapacity: 10Gi + capacity: 100Gi + accessModes: + - ReadWriteOnce + moverSecurityContext: + fsGroup: 568 + runAsUser: 568 + runAsGroup: 568 + - it: should generate correct spec with changed defaults set: persistence: diff --git a/charts/library/common-test/tests/volsync/replication_src_spec_test.yaml b/charts/library/common-test/tests/volsync/replication_src_spec_test.yaml index 7d2f8452853..d26a5223521 100644 --- a/charts/library/common-test/tests/volsync/replication_src_spec_test.yaml +++ b/charts/library/common-test/tests/volsync/replication_src_spec_test.yaml @@ -82,6 +82,83 @@ tests: path: spec.restic.unlock pattern: "^[0-9]{14}$" + - it: should generate correct spec with overridden region + set: + persistence: + srcbackup: + enabled: true + type: pvc + mountPath: /backedup + volsync: + - name: mybackup1 + type: restic + credentials: mys3 + dest: + enabled: false + src: + enabled: true + credentials: + mys3: + type: s3 + url: http://some-url + bucket: some-bucket + encrKey: some-key + accessKey: some-access-key + secretKey: some-secret-key + region: some-region + asserts: + - documentIndex: *secretDoc + isKind: + of: Secret + - documentIndex: *secretDoc + isAPIVersion: + of: v1 + - documentIndex: *secretDoc + equal: + path: stringData + value: + AWS_ACCESS_KEY_ID: some-access-key + AWS_SECRET_ACCESS_KEY: some-secret-key + RESTIC_PASSWORD: some-key + RESTIC_REPOSITORY: s3:http://some-url/some-bucket/test-release-name/volsync/srcbackup-volsync-mybackup1 + AWS_DEFAULT_REGION: some-region + - documentIndex: *replicationDestDoc + isKind: + of: ReplicationSource + - documentIndex: *replicationDestDoc + isAPIVersion: + of: volsync.backube/v1alpha1 + - documentIndex: *replicationDestDoc + isSubset: + path: spec + content: + sourcePVC: test-release-name-common-test-srcbackup + trigger: + schedule: "0 0 * * *" + - documentIndex: *replicationDestDoc + isSubset: + path: spec.restic + content: + repository: test-release-name-common-test-srcbackup-volsync-mybackup1 + copyMethod: Snapshot + pruneIntervalDays: 7 + retain: + hourly: 6 + daily: 5 + weekly: 4 + monthly: 3 + yearly: 1 + accessModes: + - ReadWriteOnce + moverSecurityContext: + fsGroup: 568 + runAsUser: 568 + runAsGroup: 568 + - documentIndex: *replicationDestDoc + matchRegex: + path: spec.restic.unlock + pattern: "^[0-9]{14}$" + - it: should generate correct spec with changed defaults set: persistence: diff --git a/charts/library/common/Chart.yaml b/charts/library/common/Chart.yaml index c10c03222b0..f8498b5d918 100644 --- a/charts/library/common/Chart.yaml +++ b/charts/library/common/Chart.yaml @@ -37,5 +37,4 @@ sources: - https://hub.docker.com/_/ - https://hub.docker.com/r/mikefarah/yq type: library -version: 28.25.0 - +version: 28.26.0 diff --git a/charts/library/common/templates/lib/cnpg/barmanObjectStore/_s3.tpl b/charts/library/common/templates/lib/cnpg/barmanObjectStore/_s3.tpl index 84ce4fa1ed6..4f5264b9481 100644 --- a/charts/library/common/templates/lib/cnpg/barmanObjectStore/_s3.tpl +++ b/charts/library/common/templates/lib/cnpg/barmanObjectStore/_s3.tpl @@ -16,6 +16,7 @@ {{- $endpointURL := $calcData.creds.url -}} {{- $customCA := $calcData.creds.customCA -}} {{- $customCASecretRef := $calcData.creds.customCASecretRef -}} + {{- $region := $calcData.creds.region -}} {{- $bucket := $calcData.creds.bucket -}} {{- $path := $calcData.creds.path -}} {{- $key := $calcData.key -}} @@ -59,6 +60,11 @@ endpointCA: destinationPath: {{ $destinationPath }} serverName: {{ $serverName }} s3Credentials: + {{- if $region }} + region: + name: {{ $secretName }} + key: REGION + {{- end }} accessKeyId: name: {{ $secretName }} key: ACCESS_KEY_ID diff --git a/charts/library/common/templates/lib/cnpg/providers/_s3.tpl b/charts/library/common/templates/lib/cnpg/providers/_s3.tpl index 34f51d2509c..aae05ce52df 100644 --- a/charts/library/common/templates/lib/cnpg/providers/_s3.tpl +++ b/charts/library/common/templates/lib/cnpg/providers/_s3.tpl @@ -2,6 +2,9 @@ {{- $creds := .creds }} enabled: true data: + {{- if $creds.region }} + REGION: {{ $creds.region | quote }} + {{- end }} ACCESS_KEY_ID: {{ $creds.accessKey | default "" | quote }} ACCESS_SECRET_KEY: {{ $creds.secretKey | default "" | quote }} {{- end -}} diff --git a/charts/library/common/templates/lib/credentials/_validation.tpl b/charts/library/common/templates/lib/credentials/_validation.tpl index 7b3a6787ba8..425c391525c 100644 --- a/charts/library/common/templates/lib/credentials/_validation.tpl +++ b/charts/library/common/templates/lib/credentials/_validation.tpl @@ -69,4 +69,9 @@ {{- end -}} {{- end -}} + {{- $region := get $credentials "region" -}} + {{- if and $region (not (kindIs "string" $region )) -}} + {{- fail (printf "%s - Expected [region] in [credentials.%s] to be a string. Got [%s]" $caller $credName (kindOf $region)) -}} + {{- end -}} + {{- end -}} diff --git a/charts/library/common/templates/spawner/_pvc.tpl b/charts/library/common/templates/spawner/_pvc.tpl index 17854b3ccc4..97dafa14039 100644 --- a/charts/library/common/templates/spawner/_pvc.tpl +++ b/charts/library/common/templates/spawner/_pvc.tpl @@ -115,19 +115,24 @@ {{- $resticrepository = printf "%s/%s/%s" $baseRepo ($credentials.path | trimSuffix "/") $repoSuffix -}} {{- end -}} - {{- $volsyncSecretData := (dict + {{- $volsyncSecretData := dict + "RESTIC_REPOSITORY" $resticrepository + "RESTIC_PASSWORD" $credentials.encrKey + "AWS_ACCESS_KEY_ID" $credentials.accessKey + "AWS_SECRET_ACCESS_KEY" $credentials.secretKey + -}} + {{- if $credentials.region -}} + {{- $_ := set $volsyncSecretData "AWS_DEFAULT_REGION" $credentials.region -}} + {{- end -}} + + {{- $volsyncSecret := (dict "name" $volsyncSecretName "labels" ($volsync.labels | default dict) "annotations" ($volsync.annotations | default dict) - "data" (dict - "RESTIC_REPOSITORY" $resticrepository - "RESTIC_PASSWORD" $credentials.encrKey - "AWS_ACCESS_KEY_ID" $credentials.accessKey - "AWS_SECRET_ACCESS_KEY" $credentials.secretKey - ) + "data" $volsyncSecretData ) -}} - {{- include "tc.v1.common.class.secret" (dict "rootCtx" $ "objectData" $volsyncSecretData) -}} + {{- include "tc.v1.common.class.secret" (dict "rootCtx" $ "objectData" $volsyncSecret) -}} {{- if $credentials.customCASecretRef -}} {{/* Get the customCA secret name */}} diff --git a/charts/library/common/values.yaml b/charts/library/common/values.yaml index c8f9917af47..6dd6b367259 100644 --- a/charts/library/common/values.yaml +++ b/charts/library/common/values.yaml @@ -308,6 +308,9 @@ credentials: # secretKey: "" # ## Is used in cases where things are encrypted by a backup utility # encrKey: "" + # ## Is used in cases where the region cannot be determined from the URL. If + # ## a backup/restore is failing, try setting the region manually using this field + # region: "" ingressMiddlewares: traefik: