Merge 064864ba72 into 376e178ab5

When an ObjectStore's credentials change (e.g., secret rename), the
RBAC Role granting the Cluster's ServiceAccount access to those secrets
was not updated because nothing triggered a Cluster reconciliation.

Implement the ObjectStore controller's Reconcile to detect referencing
Clusters and update their Roles directly. Extract ensureRole into a
shared rbac.EnsureRole function used by both the Pre hook and the
ObjectStore controller.

The shared EnsureRole accepts optional beforeCreate callbacks so the
Pre hook can set owner references on Role creation, while the
ObjectStore controller path skips ownership (it only updates existing
Roles).

Handle deleted ObjectStores gracefully by skipping NotFound errors
during Role reconciliation, and filter status-only changes with
GenerationChangedPredicate.

Add RBAC permission for the plugin to list/watch Clusters so the
ObjectStore controller can find affected Clusters. Regenerate
config/rbac/role.yaml.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>

config/rbac/role.yaml

@@ -44,6 +44,7 @@ rules:
   - postgresql.cnpg.io
   resources:
   - backups
+  - clusters
   verbs:
   - get
   - list

internal/cnpgi/operator/rbac/ensure.go

@@ -0,0 +1,91 @@
+/*
+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 rbac contains utilities to reconcile RBAC resources
+// for the barman-cloud plugin.
+package rbac
+
+import (
+	"context"
+
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
+	"github.com/cloudnative-pg/machinery/pkg/log"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs"
+)
+
+// EnsureRole ensures the RBAC Role for the given Cluster matches
+// the desired state derived from the given ObjectStores.
+// Optional beforeCreate callbacks are applied to the Role before creation
+// (e.g. to set owner references). They are not called on updates.
+func EnsureRole(
+	ctx context.Context,
+	c client.Client,
+	cluster *cnpgv1.Cluster,
+	barmanObjects []barmancloudv1.ObjectStore,
+	beforeCreate ...func(*rbacv1.Role) error,
+) error {
+	contextLogger := log.FromContext(ctx)
+	newRole := specs.BuildRole(cluster, barmanObjects)
+
+	var role rbacv1.Role
+	if err := c.Get(ctx, client.ObjectKey{
+		Namespace: newRole.Namespace,
+		Name:      newRole.Name,
+	}, &role); err != nil {
+		if !apierrs.IsNotFound(err) {
+			return err
+		}
+
+		contextLogger.Info(
+			"Creating role",
+			"name", newRole.Name,
+			"namespace", newRole.Namespace,
+		)
+
+		for _, fn := range beforeCreate {
+			if err := fn(newRole); err != nil {
+				return err
+			}
+		}
+
+		return c.Create(ctx, newRole)
+	}
+
+	if equality.Semantic.DeepEqual(newRole.Rules, role.Rules) {
+		return nil
+	}
+
+	contextLogger.Info(
+		"Patching role",
+		"name", newRole.Name,
+		"namespace", newRole.Namespace,
+		"rules", newRole.Rules,
+	)
+
+	oldRole := role.DeepCopy()
+	role.Rules = newRole.Rules
+
+	return c.Patch(ctx, &role, client.MergeFrom(oldRole))
+}

internal/cnpgi/operator/rbac/ensure_test.go

@@ -0,0 +1,206 @@
+/*
+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 rbac_test
+
+import (
+	"context"
+	"fmt"
+
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api"
+	machineryapi "github.com/cloudnative-pg/machinery/pkg/api"
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/rbac"
+)
+
+func newScheme() *runtime.Scheme {
+	s := runtime.NewScheme()
+	_ = rbacv1.AddToScheme(s)
+	_ = cnpgv1.AddToScheme(s)
+	_ = barmancloudv1.AddToScheme(s)
+	return s
+}
+
+func newCluster(name, namespace string) *cnpgv1.Cluster {
+	return &cnpgv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+	}
+}
+
+func newObjectStore(name, namespace, secretName string) barmancloudv1.ObjectStore {
+	return barmancloudv1.ObjectStore{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: barmancloudv1.ObjectStoreSpec{
+			Configuration: barmanapi.BarmanObjectStoreConfiguration{
+				DestinationPath: "s3://bucket/path",
+				BarmanCredentials: barmanapi.BarmanCredentials{
+					AWS: &barmanapi.S3Credentials{
+						AccessKeyIDReference: &machineryapi.SecretKeySelector{
+							LocalObjectReference: machineryapi.LocalObjectReference{
+								Name: secretName,
+							},
+							Key: "ACCESS_KEY_ID",
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+var _ = Describe("EnsureRole", func() {
+	var (
+		ctx       context.Context
+		cluster   *cnpgv1.Cluster
+		objects   []barmancloudv1.ObjectStore
+		fakeClient client.Client
+	)
+
+	BeforeEach(func() {
+		ctx = context.Background()
+		cluster = newCluster("test-cluster", "default")
+		objects = []barmancloudv1.ObjectStore{
+			newObjectStore("my-store", "default", "aws-creds"),
+		}
+	})
+
+	Context("when the Role does not exist", func() {
+		BeforeEach(func() {
+			fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
+		})
+
+		It("should create the Role", func() {
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects)
+			Expect(err).NotTo(HaveOccurred())
+
+			var role rbacv1.Role
+			err = fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "test-cluster-barman-cloud",
+			}, &role)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(role.Rules).To(HaveLen(3))
+		})
+
+		It("should apply beforeCreate callbacks", func() {
+			called := false
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects, func(role *rbacv1.Role) error {
+				called = true
+				role.Labels = map[string]string{"owner": "test-cluster"}
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(called).To(BeTrue())
+
+			var role rbacv1.Role
+			err = fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "test-cluster-barman-cloud",
+			}, &role)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(role.Labels).To(HaveKeyWithValue("owner", "test-cluster"))
+		})
+
+		It("should return error if beforeCreate callback fails", func() {
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects, func(_ *rbacv1.Role) error {
+				return fmt.Errorf("callback error")
+			})
+			Expect(err).To(MatchError("callback error"))
+		})
+	})
+
+	Context("when the Role exists with matching rules", func() {
+		BeforeEach(func() {
+			fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
+			Expect(rbac.EnsureRole(ctx, fakeClient, cluster, objects)).To(Succeed())
+		})
+
+		It("should not patch the Role", func() {
+			var before rbacv1.Role
+			Expect(fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "test-cluster-barman-cloud",
+			}, &before)).To(Succeed())
+
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects)
+			Expect(err).NotTo(HaveOccurred())
+
+			var after rbacv1.Role
+			Expect(fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "test-cluster-barman-cloud",
+			}, &after)).To(Succeed())
+
+			Expect(after.ResourceVersion).To(Equal(before.ResourceVersion))
+		})
+	})
+
+	Context("when the Role exists with different rules", func() {
+		BeforeEach(func() {
+			fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
+			// Create with old secret
+			oldObjects := []barmancloudv1.ObjectStore{
+				newObjectStore("my-store", "default", "old-secret"),
+			}
+			Expect(rbac.EnsureRole(ctx, fakeClient, cluster, oldObjects)).To(Succeed())
+		})
+
+		It("should patch the Role with new rules", func() {
+			// Now call with new secret name
+			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())
+
+			// The secrets rule should reference the new secret
+			secretsRule := role.Rules[2]
+			Expect(secretsRule.ResourceNames).To(ContainElement("aws-creds"))
+			Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret"))
+		})
+
+		It("should not call beforeCreate callbacks on update", func() {
+			called := false
+			err := rbac.EnsureRole(ctx, fakeClient, cluster, objects, func(_ *rbacv1.Role) error {
+				called = true
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(called).To(BeFalse())
+		})
+	})
+})

internal/cnpgi/operator/rbac/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 rbac_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestRBAC(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "RBAC Suite")
+}

internal/cnpgi/operator/reconciler.go

@@ -28,12 +28,12 @@ import (
 	"github.com/cloudnative-pg/cnpg-i/pkg/reconciler"
 	"github.com/cloudnative-pg/machinery/pkg/log"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/rbac"
 	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs"
 )
 
@@ -113,7 +113,9 @@ func (r ReconcilerImplementation) Pre(
 		barmanObjects = append(barmanObjects, barmanObject)
 	}
 
-	if err := r.ensureRole(ctx, &cluster, barmanObjects); err != nil {
+	if err := rbac.EnsureRole(ctx, r.Client, &cluster, barmanObjects, func(role *rbacv1.Role) error {
+		return setOwnerReference(&cluster, role)
+	}); err != nil {
 		return nil, err
 	}
 
@@ -137,57 +139,6 @@ func (r ReconcilerImplementation) Post(
 	}, nil
 }
 
-func (r ReconcilerImplementation) ensureRole(
-	ctx context.Context,
-	cluster *cnpgv1.Cluster,
-	barmanObjects []barmancloudv1.ObjectStore,
-) error {
-	contextLogger := log.FromContext(ctx)
-	newRole := specs.BuildRole(cluster, barmanObjects)
-
-	var role rbacv1.Role
-	if err := r.Client.Get(ctx, client.ObjectKey{
-		Namespace: newRole.Namespace,
-		Name:      newRole.Name,
-	}, &role); err != nil {
-		if !apierrs.IsNotFound(err) {
-			return err
-		}
-
-		contextLogger.Info(
-			"Creating role",
-			"name", newRole.Name,
-			"namespace", newRole.Namespace,
-		)
-
-		if err := setOwnerReference(cluster, newRole); err != nil {
-			return err
-		}
-
-		return r.Client.Create(ctx, newRole)
-	}
-
-	if equality.Semantic.DeepEqual(newRole.Rules, role.Rules) {
-		// There's no need to hit the API server again
-		return nil
-	}
-
-	contextLogger.Info(
-		"Patching role",
-		"name", newRole.Name,
-		"namespace", newRole.Namespace,
-		"rules", newRole.Rules,
-	)
-
-	oldRole := role.DeepCopy()
-
-	// Apply to the role the new rules
-	role.Rules = newRole.Rules
-
-	// Push it back to the API server
-	return r.Client.Patch(ctx, &role, client.MergeFrom(oldRole))
-}
-
 func (r ReconcilerImplementation) ensureRoleBinding(
 	ctx context.Context,
 	cluster *cnpgv1.Cluster,

internal/controller/objectstore_controller.go

@@ -23,12 +23,20 @@ import (
 	"context"
 	"fmt"
 
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	"github.com/cloudnative-pg/machinery/pkg/log"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/rbac"
 )
 
 // ObjectStoreReconciler reconciles a ObjectStore object.
@@ -40,33 +48,98 @@ type ObjectStoreReconciler struct {
 // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=create;patch;update;get;list;watch
 // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=create;patch;update;get;list;watch
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=create;list;get;watch;delete
+// +kubebuilder:rbac:groups=postgresql.cnpg.io,resources=clusters,verbs=get;list;watch
 // +kubebuilder:rbac:groups=postgresql.cnpg.io,resources=clusters/finalizers,verbs=update
 // +kubebuilder:rbac:groups=postgresql.cnpg.io,resources=backups,verbs=get;list;watch
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=barmancloud.cnpg.io,resources=objectstores/finalizers,verbs=update
 
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
-// move the current state of the cluster closer to the desired state.
-// TODO(user): Modify the Reconcile function to compare the state specified by
-// the ObjectStore object against the actual cluster state, and then
-// perform operations to make the cluster state reflect the state specified by
-// the user.
-//
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.19.0/pkg/reconcile
-func (r *ObjectStoreReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) {
-	_ = log.FromContext(ctx)
+// Reconcile ensures that the RBAC Role for each Cluster referencing
+// this ObjectStore is up to date with the current ObjectStore spec.
+func (r *ObjectStoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	contextLogger := log.FromContext(ctx).WithValues(
+		"objectStoreName", req.Name,
+		"namespace", req.Namespace,
+	)
+	ctx = log.IntoContext(ctx, contextLogger)
 
-	// TODO(user): your logic here
+	contextLogger.Info("ObjectStore reconciliation start")
 
+	// List all Clusters in the same namespace
+	var clusterList cnpgv1.ClusterList
+	if err := r.List(ctx, &clusterList, client.InNamespace(req.Namespace)); err != nil {
+		return ctrl.Result{}, fmt.Errorf("while listing clusters: %w", err)
+	}
+
+	// For each Cluster that references this ObjectStore, reconcile the Role
+	for i := range clusterList.Items {
+		cluster := &clusterList.Items[i]
+
+		pluginConfiguration := config.NewFromCluster(cluster)
+		referredObjects := pluginConfiguration.GetReferredBarmanObjectsKey()
+
+		if !referencesObjectStore(referredObjects, req.NamespacedName) {
+			continue
+		}
+
+		contextLogger.Info("Reconciling RBAC for cluster",
+			"clusterName", cluster.Name)
+
+		if err := r.reconcileRBACForCluster(ctx, cluster, referredObjects); err != nil {
+			return ctrl.Result{}, fmt.Errorf("while reconciling RBAC for cluster %s: %w", cluster.Name, err)
+		}
+	}
+
+	contextLogger.Info("ObjectStore reconciliation completed")
 	return ctrl.Result{}, nil
 }
 
+// reconcileRBACForCluster ensures the Role for the given Cluster is
+// up to date with the current ObjectStore specs.
+func (r *ObjectStoreReconciler) reconcileRBACForCluster(
+	ctx context.Context,
+	cluster *cnpgv1.Cluster,
+	referredObjectKeys []client.ObjectKey,
+) error {
+	contextLogger := log.FromContext(ctx)
+	barmanObjects := make([]barmancloudv1.ObjectStore, 0, len(referredObjectKeys))
+	for _, key := range referredObjectKeys {
+		var barmanObject barmancloudv1.ObjectStore
+		if err := r.Get(ctx, key, &barmanObject); err != nil {
+			if apierrs.IsNotFound(err) {
+				contextLogger.Info("ObjectStore not found, skipping",
+					"objectStoreName", key.Name)
+				continue
+			}
+			return fmt.Errorf("while getting ObjectStore %s: %w", key, err)
+		}
+		barmanObjects = append(barmanObjects, barmanObject)
+	}
+
+	return rbac.EnsureRole(ctx, r.Client, cluster, barmanObjects, func(role *rbacv1.Role) error {
+		return controllerutil.SetControllerReference(cluster, role, r.Scheme)
+	})
+}
+
+// referencesObjectStore checks if the given ObjectStore is in the list
+// of referred barman objects.
+func referencesObjectStore(
+	referredObjects []client.ObjectKey,
+	objectStore client.ObjectKey,
+) bool {
+	for _, ref := range referredObjects {
+		if ref.Name == objectStore.Name && ref.Namespace == objectStore.Namespace {
+			return true
+		}
+	}
+	return false
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *ObjectStoreReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	err := ctrl.NewControllerManagedBy(mgr).
-		For(&barmancloudv1.ObjectStore{}).
+		For(&barmancloudv1.ObjectStore{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
 		Complete(r)
 	if err != nil {
 		return fmt.Errorf("unable to create controller: %w", err)

internal/controller/objectstore_controller_test.go

@@ -22,70 +22,301 @@ package controller
 import (
 	"context"
 
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api"
-	"k8s.io/apimachinery/pkg/api/errors"
+	machineryapi "github.com/cloudnative-pg/machinery/pkg/api"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/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/reconcile"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
 )
 
-var _ = Describe("ObjectStore Controller", func() {
-	Context("When reconciling a resource", func() {
-		const resourceName = "test-resource"
+func newFakeScheme() *runtime.Scheme {
+	s := runtime.NewScheme()
+	_ = rbacv1.AddToScheme(s)
+	_ = cnpgv1.AddToScheme(s)
+	_ = barmancloudv1.AddToScheme(s)
+	return s
+}
 
-		ctx := context.Background()
+func newTestCluster(name, namespace, objectStoreName string) *cnpgv1.Cluster {
+	return &cnpgv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: cnpgv1.ClusterSpec{
+			Plugins: []cnpgv1.PluginConfiguration{
+				{
+					Name: metadata.PluginName,
+					Parameters: map[string]string{
+						"barmanObjectName": objectStoreName,
+					},
+				},
+			},
+		},
+	}
+}
 
-		typeNamespacedName := types.NamespacedName{
-			Name:      resourceName,
-			Namespace: "default", // TODO(user):Modify as needed
+func newTestObjectStore(name, namespace, secretName string) *barmancloudv1.ObjectStore {
+	return &barmancloudv1.ObjectStore{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: barmancloudv1.ObjectStoreSpec{
+			Configuration: barmanapi.BarmanObjectStoreConfiguration{
+				DestinationPath: "s3://bucket/path",
+				BarmanCredentials: barmanapi.BarmanCredentials{
+					AWS: &barmanapi.S3Credentials{
+						AccessKeyIDReference: &machineryapi.SecretKeySelector{
+							LocalObjectReference: machineryapi.LocalObjectReference{
+								Name: secretName,
+							},
+							Key: "ACCESS_KEY_ID",
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+var _ = Describe("referencesObjectStore", func() {
+	It("should return true when ObjectStore is in the list", func() {
+		refs := []client.ObjectKey{
+			{Name: "store-a", Namespace: "default"},
+			{Name: "store-b", Namespace: "default"},
 		}
-		objectstore := &barmancloudv1.ObjectStore{}
+		Expect(referencesObjectStore(refs, client.ObjectKey{
+			Name: "store-b", Namespace: "default",
+		})).To(BeTrue())
+	})
 
-		BeforeEach(func() {
-			By("creating the custom resource for the Kind ObjectStore")
-			err := k8sClient.Get(ctx, typeNamespacedName, objectstore)
-			if err != nil && errors.IsNotFound(err) {
-				resource := &barmancloudv1.ObjectStore{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      resourceName,
-						Namespace: "default",
-					},
-					Spec: barmancloudv1.ObjectStoreSpec{
-						Configuration: barmanapi.BarmanObjectStoreConfiguration{DestinationPath: "/tmp"},
-					},
-					// TODO(user): Specify other spec details if needed.
-				}
-				Expect(k8sClient.Create(ctx, resource)).To(Succeed())
-			}
-		})
+	It("should return false when ObjectStore is not in the list", func() {
+		refs := []client.ObjectKey{
+			{Name: "store-a", Namespace: "default"},
+		}
+		Expect(referencesObjectStore(refs, client.ObjectKey{
+			Name: "store-b", Namespace: "default",
+		})).To(BeFalse())
+	})
 
-		AfterEach(func() {
-			// TODO(user): Cleanup logic after each test, like removing the resource instance.
-			resource := &barmancloudv1.ObjectStore{}
-			err := k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
+	It("should return false when namespace differs", func() {
+		refs := []client.ObjectKey{
+			{Name: "store-a", Namespace: "ns1"},
+		}
+		Expect(referencesObjectStore(refs, client.ObjectKey{
+			Name: "store-a", Namespace: "ns2",
+		})).To(BeFalse())
+	})
 
-			By("Cleanup the specific resource instance ObjectStore")
-			Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
-		})
-		It("should successfully reconcile the resource", func() {
-			By("Reconciling the created resource")
-			controllerReconciler := &ObjectStoreReconciler{
-				Client: k8sClient,
-				Scheme: k8sClient.Scheme(),
+	It("should return false for empty list", func() {
+		Expect(referencesObjectStore(nil, client.ObjectKey{
+			Name: "store-a", Namespace: "default",
+		})).To(BeFalse())
+	})
+})
+
+var _ = Describe("ObjectStoreReconciler", func() {
+	var (
+		ctx        context.Context
+		scheme     *runtime.Scheme
+	)
+
+	BeforeEach(func() {
+		ctx = context.Background()
+		scheme = newFakeScheme()
+	})
+
+	Describe("Reconcile", func() {
+		It("should create a Role for a Cluster that references the ObjectStore", func() {
+			objectStore := newTestObjectStore("my-store", "default", "aws-creds")
+			cluster := newTestCluster("my-cluster", "default", "my-store")
+
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(objectStore, cluster).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{
+				Client: fakeClient,
+				Scheme: scheme,
 			}
 
-			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
-				NamespacedName: typeNamespacedName,
+			result, err := reconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      "my-store",
+					Namespace: "default",
+				},
 			})
 			Expect(err).NotTo(HaveOccurred())
-			// TODO(user): Add more specific assertions depending on your controller's reconciliation logic.
-			// Example: If you expect a certain status condition after reconciliation, verify it here.
+			Expect(result).To(Equal(reconcile.Result{}))
+
+			var role rbacv1.Role
+			err = fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "my-cluster-barman-cloud",
+			}, &role)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(role.Rules).To(HaveLen(3))
+
+			// Verify the secrets rule contains the expected secret
+			secretsRule := role.Rules[2]
+			Expect(secretsRule.ResourceNames).To(ContainElement("aws-creds"))
+
+			// Verify owner reference is set to the Cluster
+			Expect(role.OwnerReferences).To(HaveLen(1))
+			Expect(role.OwnerReferences[0].Name).To(Equal("my-cluster"))
+			Expect(role.OwnerReferences[0].Kind).To(Equal("Cluster"))
+		})
+
+		It("should skip Clusters that don't reference the ObjectStore", func() {
+			objectStore := newTestObjectStore("my-store", "default", "aws-creds")
+			cluster := newTestCluster("my-cluster", "default", "other-store")
+
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(objectStore, cluster).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{
+				Client: fakeClient,
+				Scheme: scheme,
+			}
+
+			result, err := reconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      "my-store",
+					Namespace: "default",
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(reconcile.Result{}))
+
+			// No Role should have been created
+			var role rbacv1.Role
+			err = fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "my-cluster-barman-cloud",
+			}, &role)
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("should succeed with no Clusters in the namespace", func() {
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{
+				Client: fakeClient,
+				Scheme: scheme,
+			}
+
+			result, err := reconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      "my-store",
+					Namespace: "default",
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(reconcile.Result{}))
+		})
+	})
+
+	Describe("reconcileRBACForCluster", func() {
+		It("should skip deleted ObjectStores and still reconcile the Role", func() {
+			// Cluster references two ObjectStores, but one is deleted
+			cluster := newTestCluster("my-cluster", "default", "store-a")
+			existingStore := newTestObjectStore("store-a", "default", "aws-creds")
+
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(existingStore).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{
+				Client: fakeClient,
+				Scheme: scheme,
+			}
+
+			// Pass two keys, but "store-b" doesn't exist
+			err := reconciler.reconcileRBACForCluster(ctx, cluster, []client.ObjectKey{
+				{Name: "store-a", Namespace: "default"},
+				{Name: "store-b", Namespace: "default"},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// Role should be created with only store-a's secrets
+			var role rbacv1.Role
+			err = fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "my-cluster-barman-cloud",
+			}, &role)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(role.Rules).To(HaveLen(3))
+
+			// ObjectStore rule should only reference store-a
+			objectStoreRule := role.Rules[0]
+			Expect(objectStoreRule.ResourceNames).To(ContainElement("store-a"))
+			Expect(objectStoreRule.ResourceNames).NotTo(ContainElement("store-b"))
+
+			// Verify owner reference is set
+			Expect(role.OwnerReferences).To(HaveLen(1))
+			Expect(role.OwnerReferences[0].Name).To(Equal("my-cluster"))
+		})
+
+		It("should update Role when ObjectStore credentials change", func() {
+			cluster := newTestCluster("my-cluster", "default", "my-store")
+			oldStore := newTestObjectStore("my-store", "default", "old-secret")
+
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(oldStore).
+				Build()
+
+			reconciler := &ObjectStoreReconciler{
+				Client: fakeClient,
+				Scheme: scheme,
+			}
+
+			// First reconcile - creates Role with old-secret
+			err := reconciler.reconcileRBACForCluster(ctx, cluster, []client.ObjectKey{
+				{Name: "my-store", Namespace: "default"},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// Update the ObjectStore with new credentials
+			var currentStore barmancloudv1.ObjectStore
+			Expect(fakeClient.Get(ctx, client.ObjectKey{
+				Name: "my-store", Namespace: "default",
+			}, &currentStore)).To(Succeed())
+			currentStore.Spec.Configuration.BarmanCredentials.AWS.AccessKeyIDReference.LocalObjectReference.Name = "new-secret"
+			Expect(fakeClient.Update(ctx, &currentStore)).To(Succeed())
+
+			// Second reconcile - should patch Role with new-secret
+			err = reconciler.reconcileRBACForCluster(ctx, cluster, []client.ObjectKey{
+				{Name: "my-store", Namespace: "default"},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			var role rbacv1.Role
+			Expect(fakeClient.Get(ctx, client.ObjectKey{
+				Namespace: "default",
+				Name:      "my-cluster-barman-cloud",
+			}, &role)).To(Succeed())
+
+			secretsRule := role.Rules[2]
+			Expect(secretsRule.ResourceNames).To(ContainElement("new-secret"))
+			Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret"))
 		})
 	})
 })