fix(e2e): use correct image name in kustomize overlay

Closes #840

The e2e kustomize overlay matches
docker.io/library/plugin-barman-cloud but the base kustomization at
kubernetes/kustomization.yaml matches the bare plugin-barman-cloud
name from kubernetes/deployment.yaml and replaces it with the GHCR
image. Since the overlay name never matches, the locally-built image
is never deployed and e2e tests always run against the main branch
code from GHCR.

Use the bare image name so the overlay rule overrides the base rule.

Broken since b7daaac (#89) changed the base kustomization's newName
from docker.io/library/plugin-barman-cloud to the GHCR image without
updating the e2e overlay.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

internal/cnpgi/operator/rbac/ensure_test.go

@@ -21,6 +21,7 @@ package rbac_test
 
 import (
 	"context"
+	"fmt"
 
 	barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api"
 	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
@@ -28,11 +29,13 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
@@ -179,6 +182,56 @@ var _ = Describe("EnsureRole", func() {
 		})
 	})
 
+	Context("when Role creation fails with a transient error", func() {
+		BeforeEach(func() {
+			internalErr := apierrs.NewInternalError(fmt.Errorf("etcd timeout"))
+			fakeClient = fake.NewClientBuilder().
+				WithScheme(newScheme()).
+				WithInterceptorFuncs(interceptor.Funcs{
+					Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+						return internalErr
+					},
+				}).
+				Build()
+		})
+
+		It("should propagate the error", func() {
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects)
+			Expect(err).To(HaveOccurred())
+			Expect(apierrs.IsInternalError(err)).To(BeTrue())
+		})
+	})
+
+	Context("when the Role has pre-existing unrelated labels", func() {
+		BeforeEach(func() {
+			fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
+			existing := &rbacv1.Role{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-cluster-barman-cloud",
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/managed-by": "helm",
+					},
+				},
+			}
+			Expect(fakeClient.Create(ctx, existing)).To(Succeed())
+		})
+
+		It("should preserve unrelated labels while adding the cluster label", func() {
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects)
+			Expect(err).NotTo(HaveOccurred())
+
+			var role rbacv1.Role
+			Expect(fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "test-cluster-barman-cloud",
+			}, &role)).To(Succeed())
+
+			Expect(role.Labels).To(HaveKeyWithValue("app.kubernetes.io/managed-by", "helm"))
+			Expect(role.Labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "test-cluster"))
+		})
+	})
+
 	Context("when the Role exists without the cluster label (upgrade scenario)", func() {
 		BeforeEach(func() {
 			fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()

internal/cnpgi/operator/reconciler.go

@@ -141,11 +141,11 @@ func (r ReconcilerImplementation) ensureRoleBinding(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,
 ) error {
-	var role rbacv1.RoleBinding
+	var roleBinding rbacv1.RoleBinding
 	if err := r.Client.Get(ctx, client.ObjectKey{
 		Namespace: cluster.Namespace,
 		Name:      specs.GetRBACName(cluster.Name),
-	}, &role); err != nil {
+	}, &roleBinding); err != nil {
 		if apierrs.IsNotFound(err) {
 			return r.createRoleBinding(ctx, cluster)
 		}

internal/cnpgi/operator/specs/ownership_test.go

@@ -87,4 +87,14 @@ var _ = Describe("SetControllerReference", func() {
 		Expect(err).To(HaveOccurred())
 		Expect(err.Error()).To(ContainSubstring("has no GVK set"))
 	})
+
+	It("should fail when the owner does not implement runtime.Object", func() {
+		// metav1.ObjectMeta satisfies metav1.Object but not runtime.Object.
+		owner := &metav1.ObjectMeta{Name: "my-cluster"}
+		controlled := &rbacv1.Role{}
+
+		err := SetControllerReference(owner, controlled)
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("is not a runtime.Object"))
+	})
 })

internal/controller/objectstore_controller_test.go

@@ -21,18 +21,21 @@ package controller
 
 import (
 	"context"
+	"fmt"
 
 	barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api"
 	machineryapi "github.com/cloudnative-pg/machinery/pkg/api"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
@@ -294,6 +297,54 @@ var _ = Describe("ObjectStoreReconciler", func() {
 			Expect(updatedRole.Rules[2].ResourceNames).To(BeEmpty())
 		})
 
+		It("should return an error when listing Roles fails", func() {
+			internalErr := apierrs.NewInternalError(fmt.Errorf("etcd timeout"))
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithInterceptorFuncs(interceptor.Funcs{
+					List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
+						if _, ok := list.(*rbacv1.RoleList); ok {
+							return internalErr
+						}
+						return c.List(ctx, list, opts...)
+					},
+				}).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{Client: fakeClient, Scheme: scheme}
+			_, err := reconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{Name: "my-store", Namespace: "default"},
+			})
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("while listing roles"))
+		})
+
+		It("should return an error when fetching an ObjectStore fails with a transient error", func() {
+			store := newTestObjectStore("my-store", "default", "aws-creds")
+			role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*store})
+
+			internalErr := apierrs.NewInternalError(fmt.Errorf("etcd timeout"))
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(role).
+				WithInterceptorFuncs(interceptor.Funcs{
+					Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+						if _, ok := obj.(*barmancloudv1.ObjectStore); ok {
+							return internalErr
+						}
+						return c.Get(ctx, key, obj, opts...)
+					},
+				}).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{Client: fakeClient, Scheme: scheme}
+			_, err := reconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{Name: "my-store", Namespace: "default"},
+			})
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("while reconciling role"))
+		})
+
 		It("should reconcile multiple Roles referencing the same ObjectStore", func() {
 			store := newTestObjectStore("shared-store", "default", "new-secret")
 			oldStore := barmancloudv1.ObjectStore{

internal/scheme/cnpg_test.go

@@ -0,0 +1,116 @@
+/*
+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 scheme
+
+import (
+	"context"
+
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/spf13/viper"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var _ = Describe("AddCNPGToScheme", func() {
+	var s *runtime.Scheme
+
+	BeforeEach(func() {
+		s = runtime.NewScheme()
+	})
+
+	AfterEach(func() {
+		viper.Reset()
+	})
+
+	It("should register CNPG types under the default group and version", func() {
+		AddCNPGToScheme(context.Background(), s)
+
+		gvks, _, err := s.ObjectKinds(&cnpgv1.Cluster{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(schema.GroupVersionKind{
+			Group:   cnpgv1.SchemeGroupVersion.Group,
+			Version: cnpgv1.SchemeGroupVersion.Version,
+			Kind:    "Cluster",
+		}))
+	})
+
+	It("should register Backup and ScheduledBackup under the default group", func() {
+		AddCNPGToScheme(context.Background(), s)
+
+		gvks, _, err := s.ObjectKinds(&cnpgv1.Backup{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(HaveField("Group", cnpgv1.SchemeGroupVersion.Group)))
+
+		gvks, _, err = s.ObjectKinds(&cnpgv1.ScheduledBackup{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(HaveField("Group", cnpgv1.SchemeGroupVersion.Group)))
+	})
+
+	It("should register CNPG types under a custom group", func() {
+		viper.Set("custom-cnpg-group", "mycompany.io")
+
+		AddCNPGToScheme(context.Background(), s)
+
+		gvks, _, err := s.ObjectKinds(&cnpgv1.Cluster{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(schema.GroupVersionKind{
+			Group:   "mycompany.io",
+			Version: cnpgv1.SchemeGroupVersion.Version,
+			Kind:    "Cluster",
+		}))
+		// The default group must not be registered
+		Expect(s.Recognizes(schema.GroupVersionKind{
+			Group:   cnpgv1.SchemeGroupVersion.Group,
+			Version: cnpgv1.SchemeGroupVersion.Version,
+			Kind:    "Cluster",
+		})).To(BeFalse())
+	})
+
+	It("should register CNPG types under a custom version", func() {
+		viper.Set("custom-cnpg-version", "v2")
+
+		AddCNPGToScheme(context.Background(), s)
+
+		gvks, _, err := s.ObjectKinds(&cnpgv1.Cluster{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(schema.GroupVersionKind{
+			Group:   cnpgv1.SchemeGroupVersion.Group,
+			Version: "v2",
+			Kind:    "Cluster",
+		}))
+	})
+
+	It("should register CNPG types under both a custom group and custom version", func() {
+		viper.Set("custom-cnpg-group", "mycompany.io")
+		viper.Set("custom-cnpg-version", "v2")
+
+		AddCNPGToScheme(context.Background(), s)
+
+		gvks, _, err := s.ObjectKinds(&cnpgv1.Cluster{})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(gvks).To(ContainElement(schema.GroupVersionKind{
+			Group:   "mycompany.io",
+			Version: "v2",
+			Kind:    "Cluster",
+		}))
+	})
+})

internal/scheme/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 scheme
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestScheme(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Scheme Suite")
+}

kubernetes/kustomization.yaml

@@ -9,6 +9,7 @@ resources:
 - service.yaml
 - ../config/crd
 - ../config/rbac
+# If you change newName, update the e2e overlay in test/e2e/e2e_suite_test.go too.
 images:
 - name: plugin-barman-cloud
   newName: ghcr.io/cloudnative-pg/plugin-barman-cloud-testing

test/e2e/e2e_suite_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/kustomize"
 
 	_ "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/tests/backup"
+	_ "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/tests/credentialrotation"
 	_ "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/tests/replicacluster"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -57,9 +58,11 @@ var _ = SynchronizedBeforeSuite(func(ctx SpecContext) []byte {
 	const barmanCloudKustomizationPath = "./kustomize/kubernetes/"
 	barmanCloudKustomization := &kustomizeTypes.Kustomization{
 		Resources: []string{barmanCloudKustomizationPath},
+		// Override the image from the base kustomization (kubernetes/kustomization.yaml)
+		// with the locally-built one. The Name must match the newName in the base.
 		Images: []kustomizeTypes.Image{
 			{
-				Name:    "docker.io/library/plugin-barman-cloud",
+				Name:    "ghcr.io/cloudnative-pg/plugin-barman-cloud-testing",
 				NewName: "registry.barman-cloud-plugin:5000/plugin-barman-cloud",
 				NewTag:  "testing",
 			},

test/e2e/internal/tests/credentialrotation/credential_rotation.go

@@ -0,0 +1,171 @@
+/*
+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 credentialrotation
+
+import (
+	"time"
+
+	cloudnativepgv1 "github.com/cloudnative-pg/api/pkg/api/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs"
+	internalClient "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/client"
+	internalCluster "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/cluster"
+	nmsp "github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/namespace"
+	"github.com/cloudnative-pg/plugin-barman-cloud/test/e2e/internal/objectstore"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+const (
+	clusterName     = "source"
+	objectStoreName = "source"
+	oldSecretName   = "minio"
+	newSecretName   = "minio-rotated"
+)
+
+var _ = Describe("Credential rotation", func() {
+	var namespace *corev1.Namespace
+	var cl client.Client
+
+	BeforeEach(func(ctx SpecContext) {
+		var err error
+		cl, _, err = internalClient.NewClient()
+		Expect(err).NotTo(HaveOccurred())
+		namespace, err = nmsp.CreateUniqueNamespace(ctx, cl, "cred-rotation")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func(ctx SpecContext) {
+		Expect(cl.Delete(ctx, namespace)).To(Succeed())
+	})
+
+	It("should update the Role when the ObjectStore secret reference changes", func(ctx SpecContext) {
+		By("starting the ObjectStore deployment")
+		resources := objectstore.NewMinioObjectStoreResources(namespace.Name, oldSecretName)
+		Expect(resources.Create(ctx, cl)).To(Succeed())
+
+		By("creating the ObjectStore")
+		store := objectstore.NewMinioObjectStore(namespace.Name, objectStoreName, oldSecretName)
+		Expect(cl.Create(ctx, store)).To(Succeed())
+
+		By("creating the Cluster")
+		cluster := newCluster(namespace.Name)
+		Expect(cl.Create(ctx, cluster)).To(Succeed())
+
+		By("waiting for the Cluster to be ready")
+		Eventually(func(g Gomega) {
+			g.Expect(cl.Get(ctx, types.NamespacedName{
+				Name:      cluster.Name,
+				Namespace: cluster.Namespace,
+			}, cluster)).To(Succeed())
+			g.Expect(internalCluster.IsReady(*cluster)).To(BeTrue())
+		}).WithTimeout(10 * time.Minute).WithPolling(10 * time.Second).Should(Succeed())
+
+		roleKey := types.NamespacedName{
+			Name:      specs.GetRBACName(clusterName),
+			Namespace: namespace.Name,
+		}
+
+		By("verifying the Role has the cluster label and references the original secret")
+		var role rbacv1.Role
+		Expect(cl.Get(ctx, roleKey, &role)).To(Succeed())
+		Expect(role.Labels).To(HaveKeyWithValue(metadata.ClusterLabelName, clusterName))
+		Expect(secretNamesFromRole(&role)).To(ContainElement(oldSecretName))
+
+		By("creating a new secret with the same credentials")
+		newSecret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      newSecretName,
+				Namespace: namespace.Name,
+			},
+			Data: map[string][]byte{
+				"ACCESS_KEY_ID":     []byte("minio"),
+				"ACCESS_SECRET_KEY": []byte("minio123"),
+			},
+		}
+		Expect(cl.Create(ctx, newSecret)).To(Succeed())
+
+		By("updating the ObjectStore to reference the new secret")
+		Expect(cl.Get(ctx, types.NamespacedName{
+			Name:      objectStoreName,
+			Namespace: namespace.Name,
+		}, store)).To(Succeed())
+		store.Spec.Configuration.BarmanCredentials.AWS.AccessKeyIDReference.Name = newSecretName
+		store.Spec.Configuration.BarmanCredentials.AWS.SecretAccessKeyReference.Name = newSecretName
+		Expect(cl.Update(ctx, store)).To(Succeed())
+
+		By("waiting for the Role to reference the new secret")
+		Eventually(func(g Gomega) {
+			g.Expect(cl.Get(ctx, roleKey, &role)).To(Succeed())
+			g.Expect(secretNamesFromRole(&role)).To(ContainElement(newSecretName))
+			g.Expect(secretNamesFromRole(&role)).NotTo(ContainElement(oldSecretName))
+		}).WithTimeout(3 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
+	})
+})
+
+func newCluster(namespace string) *cloudnativepgv1.Cluster {
+	return &cloudnativepgv1.Cluster{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Cluster",
+			APIVersion: "postgresql.cnpg.io/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      clusterName,
+			Namespace: namespace,
+		},
+		Spec: cloudnativepgv1.ClusterSpec{
+			Instances:       1,
+			ImagePullPolicy: corev1.PullAlways,
+			Plugins: []cloudnativepgv1.PluginConfiguration{
+				{
+					Name: "barman-cloud.cloudnative-pg.io",
+					Parameters: map[string]string{
+						"barmanObjectName": objectStoreName,
+					},
+					IsWALArchiver: ptr.To(true),
+				},
+			},
+			StorageConfiguration: cloudnativepgv1.StorageConfiguration{
+				Size: "1Gi",
+			},
+		},
+	}
+}
+
+func secretNamesFromRole(role *rbacv1.Role) []string {
+	for _, rule := range role.Rules {
+		if len(rule.APIGroups) == 1 &&
+			rule.APIGroups[0] == "" &&
+			len(rule.Resources) == 1 &&
+			rule.Resources[0] == "secrets" {
+			return rule.ResourceNames
+		}
+	}
+
+	return nil
+}