marco/plugin-barman-cloud
1
0
Fork 0
mirror of https://github.com/cloudnative-pg/plugin-barman-cloud.git synced 2026-07-06 17:52:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

feat: configure k8s recommended labels on subresources (#865)
Some checks failed
release-please / release-please (push) Failing after 5s
Details

Browse Source 
Add [Kubernetes recommended
labels](https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#labels)
to Role and RoleBinding managed Objects.

Closes #545

Signed-off-by: Gabriele Fedi <gabriele.fedi@enterprisedb.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Leonardo Cecchi <leonardo.cecchi@enterprisedb.com>
This commit is contained in:
Gabriele Fedi 2026-04-29 14:35:43 +02:00 committed by GitHub
parent a81f6e3d73
commit 4bbaf18cd3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 754 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
internal/cnpgi/metadata/constants.go
View File

@ -28,8 +28,24 @@ const PluginName = "barman-cloud.cloudnative-pg.io"
const (
// ClusterLabelName is the label applied to RBAC resources created
// by this plugin. Its value is the name of the owning Cluster.
//
// Discovery contract: internal/controller/objectstore_controller.go
// selects Roles by this key when an ObjectStore is reconciled.
// Renaming or removing the label would break that controller; new
// recommended-label keys are added alongside it, never in place
// of it.
ClusterLabelName = "barmancloud.cnpg.io/cluster"
// AppLabelValue is the value applied to app.kubernetes.io/name on
// every plugin-managed object. It identifies the application as
// the Barman Cloud plugin (see issue #545).
AppLabelValue = "barman-cloud-plugin"
// ManagedByLabelValue is the value applied to app.kubernetes.io/managed-by
// on every plugin-managed object. It identifies this plugin as
// the controller responsible for the object.
ManagedByLabelValue = "plugin-barman-cloud"
// CheckEmptyWalArchiveFile is the name of the file in the PGDATA that,
// if present, requires the WAL archiver to check that the backup object
// store is empty.

220
internal/cnpgi/operator/rbac/ensure.go
View File

@ -21,6 +21,7 @@ package rbac
import (
"context"
"fmt"
cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
"github.com/cloudnative-pg/machinery/pkg/log"
@ -31,7 +32,6 @@ import (
"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/metadata"
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs"
)
@ -62,9 +62,7 @@ func EnsureRole(
return err
}
return patchRole(ctx, c, roleKey, newRole.Rules, map[string]string{
metadata.ClusterLabelName: cluster.Name,
})
return patchRole(ctx, c, roleKey, newRole.Rules, specs.BuildLabels(cluster))
}
// EnsureRoleRules updates the rules of an existing Role to match
@ -90,6 +88,143 @@ func EnsureRoleRules(
return err
}
// EnsureRoleBinding ensures the RoleBinding for the given Cluster
// is present and carries the recommended labels.
//
// This function is called from the Pre hook (gRPC). It creates the
// RoleBinding if it does not exist, then reconciles labels and
// Subjects:
// - Labels are written per-key. Keys the plugin manages overwrite
// existing values; unrelated keys (anything outside the desired
// set) are left alone.
// - Subjects are additive. The plugin guarantees its own Subject
// is bound, but never removes Subjects added by other actors —
// a Subject is a grant of access, and silently revoking access
// someone else granted is the wrong default.
//
// RoleRef is immutable in Kubernetes. If the existing RoleBinding
// points to a different Role, the plugin fails loudly so the
// operator notices and recreates the object.
func EnsureRoleBinding(ctx context.Context, c client.Client, cluster *cnpgv1.Cluster) error {
desiredRoleBinding := specs.BuildRoleBinding(cluster)
if err := specs.SetControllerReference(cluster, desiredRoleBinding); err != nil {
return err
}
roleBinding, err := getOrCreateRoleBinding(ctx, c, desiredRoleBinding)
if err != nil || roleBinding == nil {
// Either an error, or we just created the object with the
// desired state — nothing to patch.
return err
}
return reconcileRoleBinding(ctx, c, roleBinding, desiredRoleBinding)
}
// getOrCreateRoleBinding returns the existing RoleBinding when it
// is already present on the API server, or nil after a successful
// Create when the just-created object already carries the desired
// state (so the caller can skip the patch path).
//
// On a stale-informer-cache race during plugin pod startup, where
// Get returns NotFound but Create returns AlreadyExists, the
// function re-Gets to return the racing winner — the caller then
// falls through to reconciliation against that real object.
func getOrCreateRoleBinding(
ctx context.Context,
c client.Client,
desired *rbacv1.RoleBinding,
) (*rbacv1.RoleBinding, error) {
contextLogger := log.FromContext(ctx)
rb := &rbacv1.RoleBinding{}
err := c.Get(ctx, client.ObjectKeyFromObject(desired), rb)
if err == nil {
return rb, nil
}
if !apierrs.IsNotFound(err) {
return nil, err
}
createErr := c.Create(ctx, desired)
switch {
case createErr == nil:
contextLogger.Info("Created RoleBinding",
"name", desired.Name, "namespace", desired.Namespace)
// Just-created with the desired state — caller skips patch.
return nil, nil
case apierrs.IsAlreadyExists(createErr):
contextLogger.Debug(
"RoleBinding already exists, likely a stale informer cache; re-fetching",
"name", desired.Name, "namespace", desired.Namespace)
// Re-Get to return the racing winner so the caller can
// reconcile against the real existing object.
fetched := &rbacv1.RoleBinding{}
if err := c.Get(ctx, client.ObjectKeyFromObject(desired), fetched); err != nil {
return nil, err
}
return fetched, nil
default:
return nil, createErr
}
}
// reconcileRoleBinding brings an existing RoleBinding's labels and
// Subjects into alignment with desired. It re-Gets the object on
// conflict-retry so each attempt observes fresh server state, and
// uses optimistic locking so a competing writer's patch is rejected
// with 409 instead of silently last-write-winning.
//
// On the first attempt the function uses the existing object passed
// in by the caller, avoiding a second Get on the steady-state path.
func reconcileRoleBinding(
ctx context.Context,
c client.Client,
existing, desired *rbacv1.RoleBinding,
) error {
contextLogger := log.FromContext(ctx)
first := true
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
var roleBinding *rbacv1.RoleBinding
if first {
roleBinding = existing
first = false
} else {
roleBinding = &rbacv1.RoleBinding{}
if err := c.Get(ctx, client.ObjectKeyFromObject(desired), roleBinding); err != nil {
return err
}
}
// RoleRef is immutable in Kubernetes; we cannot patch it.
// Divergence at the canonical name is corruption regardless
// of who wrote the existing object — fail loudly so the
// operator notices and deletes the RoleBinding, and the
// next Pre call recreates it correctly.
if !equality.Semantic.DeepEqual(roleBinding.RoleRef, desired.RoleRef) {
return fmt.Errorf(
"RoleBinding %s/%s has divergent immutable RoleRef "+
"(existing=%+v, desired=%+v); delete the RoleBinding to allow recreation",
roleBinding.Namespace, roleBinding.Name,
roleBinding.RoleRef, desired.RoleRef)
}
if !roleBindingNeedsUpdate(roleBinding, desired) {
return nil
}
contextLogger.Info("Patching role binding",
"name", roleBinding.Name, "namespace", roleBinding.Namespace)
oldRoleBinding := roleBinding.DeepCopy()
roleBinding.Labels = mergeLabels(roleBinding.Labels, desired.Labels)
roleBinding.Subjects = mergeSubjects(roleBinding.Subjects, desired.Subjects)
return c.Patch(ctx, roleBinding,
client.MergeFromWithOptions(oldRoleBinding, client.MergeFromWithOptimisticLock{}))
})
}
// ensureRoleExists creates the Role if it does not exist. Returns
// nil on success and nil on AlreadyExists (another writer created
// it concurrently). The caller always follows up with patchRole.
@ -155,22 +290,32 @@ func patchRole(
oldRole := role.DeepCopy()
role.Rules = desiredRules
role.Labels = mergeLabels(role.Labels, desiredLabels)
if desiredLabels != nil {
if role.Labels == nil {
role.Labels = make(map[string]string, len(desiredLabels))
}
for k, v := range desiredLabels {
role.Labels[k] = v
}
}
return c.Patch(ctx, &role, client.MergeFrom(oldRole))
return c.Patch(ctx, &role,
client.MergeFromWithOptions(oldRole, client.MergeFromWithOptimisticLock{}))
})
}
// labelsNeedUpdate returns true if any key in desired is missing
// or has a different value in existing.
// mergeLabels writes the desired labels onto existing per-key.
// Keys in desired overwrite the existing value; keys not in desired
// (any unrelated label a user may have set) are left alone.
func mergeLabels(existing, desired map[string]string) map[string]string {
if len(desired) == 0 {
return existing
}
if existing == nil {
existing = make(map[string]string, len(desired))
}
for k, v := range desired {
existing[k] = v
}
return existing
}
// labelsNeedUpdate returns true if a Patch is required to bring
// existing labels into the state mergeLabels would produce, i.e.
// any desired key is missing or has a different value in existing.
func labelsNeedUpdate(existing, desired map[string]string) bool {
for k, v := range desired {
if existing[k] != v {
@ -179,3 +324,46 @@ func labelsNeedUpdate(existing, desired map[string]string) bool {
}
return false
}
// containsSubject reports whether subjects contains an element that
// is semantically equal to subject.
func containsSubject(subjects []rbacv1.Subject, subject rbacv1.Subject) bool {
for _, s := range subjects {
if equality.Semantic.DeepEqual(s, subject) {
return true
}
}
return false
}
// mergeSubjects appends desired Subjects that are not already
// present in existing.
//
// This is intentionally asymmetric to mergeLabels: labels are
// metadata, so replacing stale plugin-set values is safe. A
// Subject is a grant of access, so removing a Subject silently
// revokes permissions an external operator chose to grant. The
// plugin only requires that ITS Subject is present, not that it
// is exclusive.
func mergeSubjects(existing, desired []rbacv1.Subject) []rbacv1.Subject {
for _, d := range desired {
if !containsSubject(existing, d) {
existing = append(existing, d)
}
}
return existing
}
// roleBindingNeedsUpdate returns true if a Patch is required to
// bring existing into alignment with desired — any desired Subject
// missing (see mergeSubjects), or any desired label key missing
// or holding a stale value (see mergeLabels).
func roleBindingNeedsUpdate(existing, desired *rbacv1.RoleBinding) bool {
for _, s := range desired.Subjects {
if !containsSubject(existing.Subjects, s) {
return true
}
}
return labelsNeedUpdate(existing.Labels, desired.Labels)
}

451
internal/cnpgi/operator/rbac/ensure_test.go
View File

@ -25,6 +25,7 @@ import (
barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api"
cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
machineryapi "github.com/cloudnative-pg/machinery/pkg/api"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -42,6 +43,42 @@ import (
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/rbac"
)
func expectRequiredLabels(labels map[string]string, clusterName string) {
ExpectWithOffset(1, labels).To(HaveKeyWithValue(metadata.ClusterLabelName, clusterName))
ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, metadata.AppLabelValue))
ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, clusterName))
ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppManagedByLabelName, metadata.ManagedByLabelValue))
ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppComponentLabelName, utils.DatabaseComponentName))
ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, metadata.Data.Version))
}
// newPatchCountingClient returns a fake client plus a pointer to a
// counter incremented on every Patch call. Useful for asserting
// that a "no-op" reconcile path issues no Patch — more reliable
// than comparing ResourceVersion across reads, which depends on
// fake-client RV-bumping semantics that are not part of the
// controller-runtime contract.
func newPatchCountingClient(initObjs ...client.Object) (client.Client, *int) {
count := 0
c := fake.NewClientBuilder().
WithScheme(newScheme()).
WithObjects(initObjs...).
WithInterceptorFuncs(interceptor.Funcs{
Patch: func(
ctx context.Context,
client client.WithWatch,
obj client.Object,
patch client.Patch,
opts ...client.PatchOption,
) error {
count++
return client.Patch(ctx, obj, patch, opts...)
},
}).
Build()
return c, &count
}
func newScheme() *runtime.Scheme {
s := runtime.NewScheme()
utilruntime.Must(rbacv1.AddToScheme(s))
@ -124,33 +161,23 @@ var _ = Describe("EnsureRole", func() {
Expect(role.OwnerReferences[0].Name).To(Equal("test-cluster"))
Expect(role.OwnerReferences[0].Kind).To(Equal("Cluster"))
Expect(role.Labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "test-cluster"))
expectRequiredLabels(role.Labels, "test-cluster")
})
})
Context("when the Role exists with matching rules", func() {
var patchCount *int
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
fakeClient, patchCount = newPatchCountingClient()
Expect(rbac.EnsureRole(ctx, fakeClient, cluster, objects)).To(Succeed())
*patchCount = 0
})
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))
Expect(*patchCount).To(BeZero())
})
})
@ -210,7 +237,7 @@ var _ = Describe("EnsureRole", func() {
Name: "test-cluster-barman-cloud",
Namespace: "default",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "helm",
"custom-label": "custom-value",
},
},
}
@ -227,8 +254,40 @@ var _ = Describe("EnsureRole", func() {
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"))
Expect(role.Labels).To(HaveKeyWithValue("custom-label", "custom-value"))
expectRequiredLabels(role.Labels, "test-cluster")
})
})
Context("when the Role exists with a stale label value", 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{
// Stale value as if written by an older plugin
// version.
utils.KubernetesAppVersionLabelName: "0.0.0-stale",
},
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should overwrite the stale value with the current plugin's value", 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(
utils.KubernetesAppVersionLabelName, metadata.Data.Version))
})
})
@ -257,12 +316,348 @@ var _ = Describe("EnsureRole", func() {
Name: "test-cluster-barman-cloud",
}, &role)).To(Succeed())
Expect(role.Labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "test-cluster"))
expectRequiredLabels(role.Labels, "test-cluster")
Expect(role.Rules).To(HaveLen(3))
})
})
})
var _ = Describe("EnsureRoleBinding", func() {
var (
ctx context.Context
cluster *cnpgv1.Cluster
fakeClient client.Client
)
BeforeEach(func() {
ctx = context.Background()
cluster = newCluster("test-cluster", "default")
})
Context("when the RoleBinding does not exist", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
})
It("should create the RoleBinding with owner reference, labels, and correct subjects", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
Expect(rb.OwnerReferences).To(HaveLen(1))
Expect(rb.OwnerReferences[0].Name).To(Equal("test-cluster"))
Expect(rb.OwnerReferences[0].Kind).To(Equal("Cluster"))
expectRequiredLabels(rb.Labels, "test-cluster")
Expect(rb.Subjects).To(HaveLen(1))
Expect(rb.Subjects[0].Name).To(Equal("test-cluster"))
Expect(rb.Subjects[0].Kind).To(Equal("ServiceAccount"))
Expect(rb.RoleRef.Kind).To(Equal("Role"))
Expect(rb.RoleRef.Name).To(Equal("test-cluster-barman-cloud"))
})
})
Context("when the RoleBinding exists with matching state", func() {
var patchCount *int
BeforeEach(func() {
fakeClient, patchCount = newPatchCountingClient()
Expect(rbac.EnsureRoleBinding(ctx, fakeClient, cluster)).To(Succeed())
*patchCount = 0
})
It("should not patch the RoleBinding", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
Expect(*patchCount).To(BeZero())
})
})
Context("when the RoleBinding exists with extra user-added subjects", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
existing := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "user-debug-sa",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "test-cluster-barman-cloud",
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should add the plugin's subject without removing user-added ones", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
// Additive policy: the user-added subject must remain.
Expect(rb.Subjects).To(ContainElement(rbacv1.Subject{
Kind: "ServiceAccount",
Name: "user-debug-sa",
APIGroup: "",
}))
// The plugin's required subject must be present.
Expect(rb.Subjects).To(ContainElement(rbacv1.Subject{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
}))
})
})
Context("when the RoleBinding exists with a stale label value", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
existing := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
Labels: map[string]string{
utils.KubernetesAppVersionLabelName: "0.0.0-stale",
},
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "test-cluster-barman-cloud",
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should overwrite the stale value with the current plugin's value", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
Expect(rb.Labels).To(HaveKeyWithValue(
utils.KubernetesAppVersionLabelName, metadata.Data.Version))
})
})
Context("when the RoleBinding has pre-existing unrelated labels", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
existing := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
Labels: map[string]string{
"custom-label": "custom-value",
},
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "test-cluster-barman-cloud",
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should preserve unrelated labels while adding the required labels", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
Expect(rb.Labels).To(HaveKeyWithValue("custom-label", "custom-value"))
expectRequiredLabels(rb.Labels, "test-cluster")
})
})
Context("when the RoleBinding has a divergent RoleRef", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
existing := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "wrong-role",
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should return a descriptive error since RoleRef is immutable", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("RoleRef"))
Expect(err.Error()).To(ContainSubstring("wrong-role"))
})
})
Context("when an AlreadyExists race happens during a stale-cache create (plugin pod startup)", func() {
var preExisting *rbacv1.RoleBinding
BeforeEach(func() {
preExisting = &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "test-cluster-barman-cloud",
},
}
// First Get returns NotFound (simulates cold informer
// cache after plugin pod restart). Subsequent Gets
// fall through to real fake-client behavior.
gets := 0
fakeClient = fake.NewClientBuilder().
WithScheme(newScheme()).
WithObjects(preExisting).
WithInterceptorFuncs(interceptor.Funcs{
Get: func(
ctx context.Context,
c client.WithWatch,
key client.ObjectKey,
obj client.Object,
opts ...client.GetOption,
) error {
gets++
if gets == 1 {
return apierrs.NewNotFound(
rbacv1.Resource("rolebindings"), key.Name)
}
return c.Get(ctx, key, obj, opts...)
},
}).
Build()
})
It("should tolerate the AlreadyExists and reconcile from the existing object", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
Expect(rb.Subjects).To(ContainElement(rbacv1.Subject{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
}))
})
})
Context("when the RoleBinding exists without labels (upgrade scenario)", func() {
BeforeEach(func() {
fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build()
existing := &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-barman-cloud",
Namespace: "default",
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: "test-cluster",
Namespace: "default",
APIGroup: "",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "test-cluster-barman-cloud",
},
}
Expect(fakeClient.Create(ctx, existing)).To(Succeed())
})
It("should add the required labels", func() {
err := rbac.EnsureRoleBinding(ctx, fakeClient, cluster)
Expect(err).NotTo(HaveOccurred())
var rb rbacv1.RoleBinding
Expect(fakeClient.Get(ctx, client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}, &rb)).To(Succeed())
expectRequiredLabels(rb.Labels, "test-cluster")
})
})
})
var _ = Describe("EnsureRoleRules", func() {
var (
ctx context.Context
@ -306,23 +701,21 @@ var _ = Describe("EnsureRoleRules", func() {
})
It("should not patch when rules already match", func() {
// Seed with the same objects so rules match
// Replace the seeded client with a counting one,
// then re-seed via EnsureRole so the desired rules
// are already in place when EnsureRoleRules runs.
var patchCount *int
fakeClient, patchCount = newPatchCountingClient()
cluster := newCluster("test-cluster", "default")
Expect(rbac.EnsureRole(ctx, fakeClient, cluster, objects)).To(Succeed())
*patchCount = 0
roleKey := client.ObjectKey{
Namespace: "default",
Name: "test-cluster-barman-cloud",
}
var before rbacv1.Role
Expect(fakeClient.Get(ctx, roleKey, &before)).To(Succeed())
Expect(rbac.EnsureRoleRules(ctx, fakeClient, roleKey, objects)).To(Succeed())
var after rbacv1.Role
Expect(fakeClient.Get(ctx, roleKey, &after)).To(Succeed())
Expect(after.ResourceVersion).To(Equal(before.ResourceVersion))
Expect(*patchCount).To(BeZero())
})
It("should not modify labels", func() {

35
internal/cnpgi/operator/reconciler.go
View File

@ -27,14 +27,12 @@ import (
"github.com/cloudnative-pg/cnpg-i-machinery/pkg/pluginhelper/object"
"github.com/cloudnative-pg/cnpg-i/pkg/reconciler"
"github.com/cloudnative-pg/machinery/pkg/log"
rbacv1 "k8s.io/api/rbac/v1"
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"
)
// ReconcilerImplementation implements the Reconciler capability
@ -117,7 +115,7 @@ func (r ReconcilerImplementation) Pre(
return nil, err
}
if err := r.ensureRoleBinding(ctx, &cluster); err != nil {
if err := rbac.EnsureRoleBinding(ctx, r.Client, &cluster); err != nil {
return nil, err
}
@ -136,34 +134,3 @@ func (r ReconcilerImplementation) Post(
Behavior: reconciler.ReconcilerHooksResult_BEHAVIOR_CONTINUE,
}, nil
}
func (r ReconcilerImplementation) ensureRoleBinding(
ctx context.Context,
cluster *cnpgv1.Cluster,
) error {
var roleBinding rbacv1.RoleBinding
if err := r.Client.Get(ctx, client.ObjectKey{
Namespace: cluster.Namespace,
Name: specs.GetRBACName(cluster.Name),
}, &roleBinding); err != nil {
if apierrs.IsNotFound(err) {
return r.createRoleBinding(ctx, cluster)
}
return err
}
// TODO: this assumes role bindings never change.
// Is that true? Should we relax this assumption?
return nil
}
func (r ReconcilerImplementation) createRoleBinding(
ctx context.Context,
cluster *cnpgv1.Cluster,
) error {
roleBinding := specs.BuildRoleBinding(cluster)
if err := specs.SetControllerReference(cluster, roleBinding); err != nil {
return err
}
return r.Client.Create(ctx, roleBinding)
}

41
internal/cnpgi/operator/specs/labels.go Normal file
View File

@ -0,0 +1,41 @@
/*
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 specs
import (
cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
)
// BuildLabels returns the Kubernetes recommended labels applied to
// every object managed by this plugin for the given Cluster. See
// https://github.com/cloudnative-pg/plugin-barman-cloud/issues/545.
func BuildLabels(cluster *cnpgv1.Cluster) map[string]string {
return map[string]string{
metadata.ClusterLabelName: cluster.Name,
utils.KubernetesAppLabelName: metadata.AppLabelValue,
utils.KubernetesAppInstanceLabelName: cluster.Name,
utils.KubernetesAppVersionLabelName: metadata.Data.Version,
utils.KubernetesAppComponentLabelName: utils.DatabaseComponentName,
utils.KubernetesAppManagedByLabelName: metadata.ManagedByLabelValue,
}
}

68
internal/cnpgi/operator/specs/labels_test.go Normal file
View File

@ -0,0 +1,68 @@
/*
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 specs
import (
cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
)
var _ = Describe("BuildLabels", func() {
It("should return the recommended labels with plugin identity", func() {
cluster := &cnpgv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "my-cluster",
Namespace: "default",
},
}
labels := BuildLabels(cluster)
Expect(labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "my-cluster"))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, metadata.AppLabelValue))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, "my-cluster"))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, metadata.Data.Version))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppComponentLabelName, utils.DatabaseComponentName))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppManagedByLabelName, metadata.ManagedByLabelValue))
Expect(labels).To(HaveLen(6))
})
It("should report the plugin version regardless of the cluster's Postgres image", func() {
cluster := &cnpgv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "pg16-cluster",
Namespace: "default",
},
Spec: cnpgv1.ClusterSpec{
ImageCatalogRef: &cnpgv1.ImageCatalogRef{
Major: 16,
},
},
}
labels := BuildLabels(cluster)
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, metadata.Data.Version))
Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, "pg16-cluster"))
})
})

6
internal/cnpgi/operator/specs/role.go
View File

@ -29,7 +29,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
)
// BuildRole builds the Role object for this cluster
@ -41,9 +40,7 @@ func BuildRole(
ObjectMeta: metav1.ObjectMeta{
Namespace: cluster.Namespace,
Name: GetRBACName(cluster.Name),
Labels: map[string]string{
metadata.ClusterLabelName: cluster.Name,
},
Labels: BuildLabels(cluster),
},
Rules: BuildRoleRules(barmanObjects),
}
@ -131,6 +128,7 @@ func BuildRoleBinding(
ObjectMeta: metav1.ObjectMeta{
Namespace: cluster.Namespace,
Name: GetRBACName(cluster.Name),
Labels: BuildLabels(cluster),
},
Subjects: []rbacv1.Subject{
{