From 120366b3e24e53d06e1ccb9432aeb7802c330b18 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Wed, 30 Sep 2026 15:35:29 +0200 Subject: [PATCH] fix: keep a separate Google credentials file for each secret key A single process can run barman-cloud commands for different object stores at the same time. A replica cluster, for instance, archives WAL to its own object store while restoring it from the source one. The Google application credentials were always written to the same file, so a command could run with the credentials of another object store, and an object store using the GKE environment emptied the file the other commands were reading. Write each credentials file to a path that depends on the referenced secret and key, point GOOGLE_APPLICATION_CREDENTIALS at it, and serialize the writes. The GKE environment no longer touches any file. Signed-off-by: Armando Ruocco --- pkg/credentials/env.go | 50 ++++++++++------- pkg/credentials/env_test.go | 103 ++++++++++++++++++++++++++++++++++ pkg/credentials/suite_test.go | 32 +++++++++++ 3 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 pkg/credentials/env_test.go create mode 100644 pkg/credentials/suite_test.go diff --git a/pkg/credentials/env.go b/pkg/credentials/env.go index d7810288..58fb509e 100644 --- a/pkg/credentials/env.go +++ b/pkg/credentials/env.go @@ -22,6 +22,8 @@ package credentials import ( "context" "fmt" + "path" + "sync" machineryapi "github.com/cloudnative-pg/machinery/pkg/api" "github.com/cloudnative-pg/machinery/pkg/fileutils" @@ -287,6 +289,18 @@ func envSetAzureCredentials( return env, nil } +// googleCredentialsDirectory is where the Google application credentials are +// materialized from their secrets. It is a variable so that tests can +// redirect it. +var googleCredentialsDirectory = path.Join(ScratchDataDirectory, ".google-credentials") + +// googleCredentialsMutex serializes the writes of the Google application +// credentials files. fileutils.WriteFileAtomic names its temporary file after +// the current second, so two concurrent writes of the same file would share +// it, and one of them could truncate the file the other has just renamed +// into place. +var googleCredentialsMutex sync.Mutex + func envSetGoogleCredentials( ctx context.Context, c client.Client, @@ -294,11 +308,9 @@ func envSetGoogleCredentials( googleCredentials *barmanApi.GoogleCredentials, env []string, ) ([]string, error) { - var applicationCredentialsContent []byte - if googleCredentials.GKEEnvironment && googleCredentials.ApplicationCredentials == nil { - return env, reconcileGoogleCredentials(googleCredentials, applicationCredentialsContent) + return env, nil } applicationCredentialsContent, err := extractValueFromSecret( @@ -311,30 +323,28 @@ func envSetGoogleCredentials( return nil, err } - if err := reconcileGoogleCredentials(googleCredentials, applicationCredentialsContent); err != nil { + // The file path depends on the referenced secret and key: a single + // process can serve object stores with different credentials + // concurrently (e.g. a replica cluster archiving to its own object store + // while restoring from the source one), and a shared file would let a + // command run with the credentials of another object store. + credentialsPath := path.Join( + googleCredentialsDirectory, + googleCredentials.ApplicationCredentials.Name, + googleCredentials.ApplicationCredentials.Key, + ) + + googleCredentialsMutex.Lock() + defer googleCredentialsMutex.Unlock() + if _, err := fileutils.WriteFileAtomic(credentialsPath, applicationCredentialsContent, 0o600); err != nil { return nil, err } - env = append(env, "GOOGLE_APPLICATION_CREDENTIALS=/controller/.application_credentials.json") + env = append(env, "GOOGLE_APPLICATION_CREDENTIALS="+credentialsPath) return env, nil } -func reconcileGoogleCredentials( - googleCredentials *barmanApi.GoogleCredentials, - applicationCredentialsContent []byte, -) error { - credentialsPath := "/controller/.application_credentials.json" - - if googleCredentials == nil { - return fileutils.RemoveFile(credentialsPath) - } - - _, err := fileutils.WriteFileAtomic(credentialsPath, applicationCredentialsContent, 0o600) - - return err -} - func extractValueFromSecret( ctx context.Context, c client.Client, diff --git a/pkg/credentials/env_test.go b/pkg/credentials/env_test.go new file mode 100644 index 00000000..760c710e --- /dev/null +++ b/pkg/credentials/env_test.go @@ -0,0 +1,103 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package credentials + +import ( + "os" + "strings" + + machineryapi "github.com/cloudnative-pg/machinery/pkg/api" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + barmanApi "github.com/cloudnative-pg/barman-cloud/pkg/api" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Google credentials", func() { + const namespace = "default" + var c client.Client + + newSecret := func(name, content string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Data: map[string][]byte{"credentials": []byte(content)}, + } + } + credentialsFrom := func(secretName string) *barmanApi.GoogleCredentials { + return &barmanApi.GoogleCredentials{ + ApplicationCredentials: &machineryapi.SecretKeySelector{ + LocalObjectReference: machineryapi.LocalObjectReference{Name: secretName}, + Key: "credentials", + }, + } + } + credentialsFile := func(env []string) string { + for _, entry := range env { + if value, found := strings.CutPrefix(entry, "GOOGLE_APPLICATION_CREDENTIALS="); found { + return value + } + } + return "" + } + + BeforeEach(func() { + previousDirectory := googleCredentialsDirectory + googleCredentialsDirectory = GinkgoT().TempDir() + DeferCleanup(func() { googleCredentialsDirectory = previousDirectory }) + + c = fake.NewClientBuilder().WithObjects( + newSecret("store-a", "credentials-a"), + newSecret("store-b", "credentials-b"), + ).Build() + }) + + It("keeps the credentials of each object store in its own file", func(ctx SpecContext) { + envA, err := envSetGoogleCredentials(ctx, c, namespace, credentialsFrom("store-a"), nil) + Expect(err).ToNot(HaveOccurred()) + envB, err := envSetGoogleCredentials(ctx, c, namespace, credentialsFrom("store-b"), nil) + Expect(err).ToNot(HaveOccurred()) + + fileA, fileB := credentialsFile(envA), credentialsFile(envB) + Expect(fileA).ToNot(Equal(fileB)) + Expect(os.ReadFile(fileA)).To(BeEquivalentTo("credentials-a")) // #nosec G304 + Expect(os.ReadFile(fileB)).To(BeEquivalentTo("credentials-b")) // #nosec G304 + }) + + It("leaves the other object stores' credentials alone in a GKE environment", func(ctx SpecContext) { + envA, err := envSetGoogleCredentials(ctx, c, namespace, credentialsFrom("store-a"), nil) + Expect(err).ToNot(HaveOccurred()) + + envGKE, err := envSetGoogleCredentials(ctx, c, namespace, + &barmanApi.GoogleCredentials{GKEEnvironment: true}, []string{"FOO=bar"}) + Expect(err).ToNot(HaveOccurred()) + Expect(envGKE).To(Equal([]string{"FOO=bar"})) + Expect(os.ReadFile(credentialsFile(envA))).To(BeEquivalentTo("credentials-a")) + }) + + It("fails when the referenced secret does not exist", func(ctx SpecContext) { + _, err := envSetGoogleCredentials(ctx, c, namespace, credentialsFrom("missing"), nil) + Expect(err).To(HaveOccurred()) + }) +}) diff --git a/pkg/credentials/suite_test.go b/pkg/credentials/suite_test.go new file mode 100644 index 00000000..47b28041 --- /dev/null +++ b/pkg/credentials/suite_test.go @@ -0,0 +1,32 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package credentials + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCredentials(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "credentials test suite") +}