From 5147a2db9120b4efff2e3d4c2a3a1fcd7a666d08 Mon Sep 17 00:00:00 2001 From: Thomas Sauvage Date: Wed, 19 Aug 2026 14:43:31 +0200 Subject: [PATCH 1/3] feat: implement endpointCA certificate mounting for TLS verification The endpointCA field was defined in the CRD and the PGBACKREST_REPO_HOST_CA_FILE env var was injected, but the CA certificate was never written to the file the env var pointed at, and the env var name was wrong (HOST_CA_FILE instead of STORAGE_CA_FILE). This left pgBackRest unable to verify self-signed S3 endpoints, failing with 'unable to get local issuer certificate'. Changes: - writeEndpointCACertificate: new function in credentials.go that reads the EndpointCA Kubernetes Secret (via the existing extractValueFromSecret helper) and writes the CA cert to /controller/certificates/ before pgBackRest runs. Called from both EnvSetBackupCloudCredentials (backup + WAL archive paths) and EnvSetRestoreCloudCredentials (restore path), covering all code paths that invoke pgBackRest. - Fix env var name: changed HOST_CA_FILE to STORAGE_CA_FILE, the correct pgBackRest env var for S3 storage CA verification (maps to --repo-storage-ca-file). The previous HOST_CA_FILE maps to --repo-host-ca-file, which is for remote pgBackRest server TLS, not S3 storage. - Fix restore CA path: EnvSetRestoreCloudCredentials was using BarmanBackupEndpointCACertificateLocation (the backup path) instead of BarmanRestoreEndpointCACertificateLocation (the restore path). Now correct. - RBAC: include endpointCA Secret: new CollectSecretNamesFromRepositories in secrets.go collects EndpointCA and EncryptionKey Secret names from repository configs. BuildRole in role.go calls it alongside the existing CollectSecretNamesFromCredentials, adding the CA Secret name to the per-namespace Role's resourceNames so the sidecar's service account can read it. - Remove stale TODO: removed the 'TODO: Properly mount/read and pass CA file for each pgbackrest repository.' comment; it is now implemented. Signed-off-by: Thomas Sauvage --- internal/cnpgi/operator/specs/role.go | 4 ++ internal/cnpgi/operator/specs/secrets.go | 18 ++++++++- .../pgbackrest/credentials/credentials.go | 39 +++++++++++++++++-- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/internal/cnpgi/operator/specs/role.go b/internal/cnpgi/operator/specs/role.go index 50a889e..9f7b815 100644 --- a/internal/cnpgi/operator/specs/role.go +++ b/internal/cnpgi/operator/specs/role.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" pgbackrestv1 "github.com/operasoftware/cnpg-plugin-pgbackrest/api/v1" + pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" ) // BuildRole builds the Role object for this cluster @@ -51,6 +52,9 @@ func BuildRole( for _, secret := range CollectSecretNamesFromCredentials(&repo.PgbackrestCredentials) { secretsSet.Put(secret) } + for _, secret := range CollectSecretNamesFromRepositories([]pgbackrestApi.PgbackrestRepository{repo}) { + secretsSet.Put(secret) + } } } diff --git a/internal/cnpgi/operator/specs/secrets.go b/internal/cnpgi/operator/specs/secrets.go index 15c6ce6..1c09754 100644 --- a/internal/cnpgi/operator/specs/secrets.go +++ b/internal/cnpgi/operator/specs/secrets.go @@ -23,7 +23,8 @@ import ( pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" ) -// CollectSecretNamesFromCredentials collects the names of the secrets +// CollectSecretNamesFromCredentials collects the names of the S3 credential +// secrets referenced by the backup credentials. func CollectSecretNamesFromCredentials(pgbackrestCredentials *pgbackrestApi.PgbackrestCredentials) []string { var references []*machineryapi.SecretKeySelector if pgbackrestCredentials.AWS != nil { @@ -44,3 +45,18 @@ func CollectSecretNamesFromCredentials(pgbackrestCredentials *pgbackrestApi.Pgba return result } + +// CollectSecretNamesFromRepositories collects the names of all secrets referenced +// by the repository configurations, including the endpoint CA certificate secret. +func CollectSecretNamesFromRepositories(repositories []pgbackrestApi.PgbackrestRepository) []string { + var result []string + for _, repo := range repositories { + if repo.EndpointCA != nil { + result = append(result, repo.EndpointCA.Name) + } + if repo.EncryptionKey != nil { + result = append(result, repo.EncryptionKey.Name) + } + } + return result +} diff --git a/internal/pgbackrest/credentials/credentials.go b/internal/pgbackrest/credentials/credentials.go index feb0917..245ba8c 100644 --- a/internal/pgbackrest/credentials/credentials.go +++ b/internal/pgbackrest/credentials/credentials.go @@ -22,6 +22,7 @@ package credentials import ( "context" "fmt" + "os" machineryapi "github.com/cloudnative-pg/machinery/pkg/api" corev1 "k8s.io/api/core/v1" @@ -38,8 +39,6 @@ const ( // CertificatesDir location to store the certificates CertificatesDir = ScratchDataDirectory + "/certificates/" - // TODO: Properly mount/read and pass CA file for each pgbackrest repository. - // BarmanBackupEndpointCACertificateLocation is the location where the barman endpoint // CA certificate is stored BarmanBackupEndpointCACertificateLocation = CertificatesDir + BarmanBackupEndpointCACertificateFileName @@ -72,7 +71,10 @@ func EnvSetBackupCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - env = append(env, utils.FormatRepoEnv(index, "HOST_CA_FILE", BarmanBackupEndpointCACertificateLocation)) + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, BarmanBackupEndpointCACertificateLocation); err != nil { + return nil, fmt.Errorf("writing backup endpoint CA certificate: %w", err) + } + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", BarmanBackupEndpointCACertificateLocation)) } } @@ -90,7 +92,10 @@ func EnvSetRestoreCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - env = append(env, utils.FormatRepoEnv(index, "HOST_CA_FILE", BarmanBackupEndpointCACertificateLocation)) + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, BarmanRestoreEndpointCACertificateLocation); err != nil { + return nil, fmt.Errorf("writing restore endpoint CA certificate: %w", err) + } + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", BarmanRestoreEndpointCACertificateLocation)) } } @@ -229,3 +234,29 @@ func extractValueFromSecret( return value, nil } + +// writeEndpointCACertificate reads the CA certificate from the referenced Kubernetes +// Secret and writes it to the given file path so pgBackRest can verify the S3 +// endpoint's TLS certificate via the PGBACKREST_REPO_STORAGE_CA_FILE env var. +func writeEndpointCACertificate( + ctx context.Context, + c client.Client, + secretReference *machineryapi.SecretKeySelector, + namespace string, + filePath string, +) error { + caData, err := extractValueFromSecret(ctx, c, secretReference, namespace) + if err != nil { + return fmt.Errorf("reading endpoint CA secret %s/%s: %w", namespace, secretReference.Name, err) + } + + if err := os.MkdirAll(CertificatesDir, 0o700); err != nil { + return fmt.Errorf("creating certificates directory %s: %w", CertificatesDir, err) + } + + if err := os.WriteFile(filePath, caData, 0o600); err != nil { + return fmt.Errorf("writing endpoint CA certificate to %s: %w", filePath, err) + } + + return nil +} From cf0b709835a975d035b6e3440a4277e4726827ea Mon Sep 17 00:00:00 2001 From: Thomas Sauvage Date: Fri, 21 Aug 2026 11:33:56 +0200 Subject: [PATCH 2/3] feat: properly handle per-repository CA certificates and clean up legacy barman-cloud env vars Replace single CA certificate file paths with per-repository paths so each repository gets its own CA file. Remove GetRestoreCABundleEnv and MergeEnv from common.go, which set AWS_CA_BUNDLE (a barman-cloud legacy env var that pgBackRest does not read). Fix WAL restore to use EnvSetRestoreCloudCredentials instead of EnvSetBackupCloudCredentials. Signed-off-by: Thomas Sauvage --- internal/cnpgi/common/common.go | 69 ------------------- internal/cnpgi/common/wal.go | 4 +- internal/cnpgi/instance/backup.go | 4 +- .../pgbackrest/credentials/credentials.go | 42 ++++++----- 4 files changed, 22 insertions(+), 97 deletions(-) diff --git a/internal/cnpgi/common/common.go b/internal/cnpgi/common/common.go index ce727f4..093e56e 100644 --- a/internal/cnpgi/common/common.go +++ b/internal/cnpgi/common/common.go @@ -16,72 +16,3 @@ limitations under the License. */ package common - -import ( - "fmt" - "strings" - - pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" -) - -// TODO: refactor. -const ( - // ScratchDataDirectory is the directory to be used for scratch data. - ScratchDataDirectory = "/controller" - - // CertificatesDir location to store the certificates. - CertificatesDir = ScratchDataDirectory + "/certificates/" - - // BarmanBackupEndpointCACertificateLocation is the location where the barman endpoint - // CA certificate is stored. - BarmanBackupEndpointCACertificateLocation = CertificatesDir + BarmanBackupEndpointCACertificateFileName - - // BarmanBackupEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate for backups is stored. - BarmanBackupEndpointCACertificateFileName = "backup-" + BarmanEndpointCACertificateFileName - - // BarmanRestoreEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate for restores is stored. - BarmanRestoreEndpointCACertificateFileName = "restore-" + BarmanEndpointCACertificateFileName - - // BarmanEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate is stored. - BarmanEndpointCACertificateFileName = "barman-ca.crt" -) - -// GetRestoreCABundleEnv gets the environment variables to be used when custom -// Object Store CA is present -func GetRestoreCABundleEnv(configuration *pgbackrestApi.PgbackrestConfiguration) []string { - var env []string - - if configuration.Repositories[0].EndpointCA != nil && configuration.Repositories[0].AWS != nil { - env = append(env, fmt.Sprintf("AWS_CA_BUNDLE=%s", BarmanBackupEndpointCACertificateLocation)) - } - return env -} - -// MergeEnv merges all the values inside incomingEnv into env. -func MergeEnv(env []string, incomingEnv []string) []string { - result := make([]string, len(env), len(env)+len(incomingEnv)) - copy(result, env) - - for _, incomingItem := range incomingEnv { - incomingKV := strings.SplitAfterN(incomingItem, "=", 2) - if len(incomingKV) != 2 { - continue - } - - found := false - for idx, item := range result { - if strings.HasPrefix(item, incomingKV[0]) { - result[idx] = incomingItem - found = true - } - } - if !found { - result = append(result, incomingItem) - } - } - - return result -} diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index 42d2d0f..de354af 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -284,8 +284,7 @@ func (w WALServiceImplementation) restoreFromPgbackrestArchive( pgbackrestConfiguration := &archive.Spec.Configuration - env := GetRestoreCABundleEnv(pgbackrestConfiguration) - credentialsEnv, err := pgbackrestCredentials.EnvSetBackupCloudCredentials( + env, err := pgbackrestCredentials.EnvSetRestoreCloudCredentials( ctx, w.Client, archive.Namespace, @@ -295,7 +294,6 @@ func (w WALServiceImplementation) restoreFromPgbackrestArchive( if err != nil { return fmt.Errorf("while getting recover credentials: %w", err) } - env = MergeEnv(env, credentialsEnv) options, err := pgbackrestCommand.CloudWalRestoreOptions(ctx, pgbackrestConfiguration, stanza, w.PGDataPath) if err != nil { diff --git a/internal/cnpgi/instance/backup.go b/internal/cnpgi/instance/backup.go index 98431d6..979d0b3 100644 --- a/internal/cnpgi/instance/backup.go +++ b/internal/cnpgi/instance/backup.go @@ -31,7 +31,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" pgbackrestv1 "github.com/operasoftware/cnpg-plugin-pgbackrest/api/v1" - "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/common" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/metadata" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/operator/config" pgbackrestBackup "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/backup" @@ -106,13 +105,12 @@ func (b BackupServiceImplementation) Backup( // We need to connect to PostgreSQL and to do that we need // PGHOST (and the like) to be available osEnvironment := utils.SanitizedEnviron() - caBundleEnvironment := common.GetRestoreCABundleEnv(&archive.Spec.Configuration) env, err := pgbackrestCredentials.EnvSetBackupCloudCredentials( ctx, b.Client, archive.Namespace, &archive.Spec.Configuration, - common.MergeEnv(osEnvironment, caBundleEnvironment)) + osEnvironment) if err != nil { contextLogger.Error(err, "while setting backup cloud credentials") return nil, err diff --git a/internal/pgbackrest/credentials/credentials.go b/internal/pgbackrest/credentials/credentials.go index 245ba8c..a47ead4 100644 --- a/internal/pgbackrest/credentials/credentials.go +++ b/internal/pgbackrest/credentials/credentials.go @@ -39,26 +39,24 @@ const ( // CertificatesDir location to store the certificates CertificatesDir = ScratchDataDirectory + "/certificates/" - // BarmanBackupEndpointCACertificateLocation is the location where the barman endpoint - // CA certificate is stored - BarmanBackupEndpointCACertificateLocation = CertificatesDir + BarmanBackupEndpointCACertificateFileName - - // BarmanBackupEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate for backups is stored - BarmanBackupEndpointCACertificateFileName = "backup-" + BarmanEndpointCACertificateFileName - - // BarmanRestoreEndpointCACertificateLocation is the location where the barman endpoint - // CA certificate is stored - BarmanRestoreEndpointCACertificateLocation = CertificatesDir + BarmanRestoreEndpointCACertificateFileName + // PgBackRestEndpointCACertificateFileName is the base name of the file in + // which the pgBackRest endpoint CA certificate is stored. + PgBackRestEndpointCACertificateFileName = "pgbackrest-ca.crt" +) - // BarmanRestoreEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate for restores is stored - BarmanRestoreEndpointCACertificateFileName = "restore-" + BarmanEndpointCACertificateFileName +// PgBackRestBackupEndpointCACertificateLocation returns the file path where +// the pgBackRest endpoint CA certificate for backups of the given repository +// (zero-based index) is stored. +func PgBackRestBackupEndpointCACertificateLocation(repoIndex int) string { + return CertificatesDir + fmt.Sprintf("backup-repo%d-%s", repoIndex+1, PgBackRestEndpointCACertificateFileName) +} - // BarmanEndpointCACertificateFileName is the name of the file in which the barman endpoint - // CA certificate is stored - BarmanEndpointCACertificateFileName = "barman-ca.crt" -) +// PgBackRestRestoreEndpointCACertificateLocation returns the file path where +// the pgBackRest endpoint CA certificate for restores of the given repository +// (zero-based index) is stored. +func PgBackRestRestoreEndpointCACertificateLocation(repoIndex int) string { + return CertificatesDir + fmt.Sprintf("restore-repo%d-%s", repoIndex+1, PgBackRestEndpointCACertificateFileName) +} // EnvSetBackupCloudCredentials sets the AWS environment variables needed for backups // given the configuration inside the cluster @@ -71,10 +69,10 @@ func EnvSetBackupCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, BarmanBackupEndpointCACertificateLocation); err != nil { + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, PgBackRestBackupEndpointCACertificateLocation(index)); err != nil { return nil, fmt.Errorf("writing backup endpoint CA certificate: %w", err) } - env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", BarmanBackupEndpointCACertificateLocation)) + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", PgBackRestBackupEndpointCACertificateLocation(index))) } } @@ -92,10 +90,10 @@ func EnvSetRestoreCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, BarmanRestoreEndpointCACertificateLocation); err != nil { + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, PgBackRestRestoreEndpointCACertificateLocation(index)); err != nil { return nil, fmt.Errorf("writing restore endpoint CA certificate: %w", err) } - env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", BarmanRestoreEndpointCACertificateLocation)) + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", PgBackRestRestoreEndpointCACertificateLocation(index))) } } From e58cf90e5d09c7a99265693b6deab1fdfbdd550f Mon Sep 17 00:00:00 2001 From: Thomas Sauvage Date: Fri, 21 Aug 2026 13:23:51 +0200 Subject: [PATCH 3/3] fix: delete empty common.go and fix lll lint violations in credentials.go Remove the now-empty common.go file (addressing review feedback). Fix four line-length violations by extracting CA certificate paths into local variables. Signed-off-by: Thomas Sauvage --- internal/cnpgi/common/common.go | 18 ------------------ internal/pgbackrest/credentials/credentials.go | 10 ++++++---- 2 files changed, 6 insertions(+), 22 deletions(-) delete mode 100644 internal/cnpgi/common/common.go diff --git a/internal/cnpgi/common/common.go b/internal/cnpgi/common/common.go deleted file mode 100644 index 093e56e..0000000 --- a/internal/cnpgi/common/common.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright The CloudNativePG Contributors -Copyright 2025, Opera Norway AS - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package common diff --git a/internal/pgbackrest/credentials/credentials.go b/internal/pgbackrest/credentials/credentials.go index a47ead4..9068eae 100644 --- a/internal/pgbackrest/credentials/credentials.go +++ b/internal/pgbackrest/credentials/credentials.go @@ -69,10 +69,11 @@ func EnvSetBackupCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, PgBackRestBackupEndpointCACertificateLocation(index)); err != nil { + caPath := PgBackRestBackupEndpointCACertificateLocation(index) + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, caPath); err != nil { return nil, fmt.Errorf("writing backup endpoint CA certificate: %w", err) } - env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", PgBackRestBackupEndpointCACertificateLocation(index))) + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", caPath)) } } @@ -90,10 +91,11 @@ func EnvSetRestoreCloudCredentials( ) ([]string, error) { for index, repo := range configuration.Repositories { if repo.EndpointCA != nil { - if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, PgBackRestRestoreEndpointCACertificateLocation(index)); err != nil { + caPath := PgBackRestRestoreEndpointCACertificateLocation(index) + if err := writeEndpointCACertificate(ctx, c, repo.EndpointCA, namespace, caPath); err != nil { return nil, fmt.Errorf("writing restore endpoint CA certificate: %w", err) } - env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", PgBackRestRestoreEndpointCACertificateLocation(index))) + env = append(env, utils.FormatRepoEnv(index, "STORAGE_CA_FILE", caPath)) } }