From 807e8624223a0fedc301fd1a6a3ef7e8d8e7ea72 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 28 Apr 2026 10:09:48 +0200 Subject: [PATCH] fix: align label values with issue #545 spec The original PR set the recommended labels to: app.kubernetes.io/name="postgresql" (utils.AppName, the cnpg operator's identity) app.kubernetes.io/version= Per issue #545 the plugin's RBAC objects should advertise the plugin's own identity: app.kubernetes.io/name="barman-cloud-plugin" app.kubernetes.io/version= Use metadata.Data.Version for the version value (always set, never conditional on cluster image), drop the cnpg GetPostgresqlMajorVersion lookup (irrelevant to plugin RBAC and fragile against the cnpg PostgresImageName default), and extract the plugin-local AppLabelValue and ManagedByLabelValue constants so test and production share a single source of truth. The labels_test.go helper now also asserts the version's value, not just the key. Document on metadata.ClusterLabelName that it's a discovery contract for objectstore_controller.go (which selects on the key), so the dual-labeling scheme (the legacy cluster label alongside the K8s recommended labels) is explicit and the legacy key cannot be silently removed by a later cleanup. Signed-off-by: Armando Ruocco --- internal/cnpgi/metadata/constants.go | 16 ++++++++++++++ internal/cnpgi/operator/rbac/ensure_test.go | 6 +++--- internal/cnpgi/operator/specs/labels.go | 22 +++++++------------- internal/cnpgi/operator/specs/labels_test.go | 11 +++++----- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/internal/cnpgi/metadata/constants.go b/internal/cnpgi/metadata/constants.go index 83ff086..fb45da6 100644 --- a/internal/cnpgi/metadata/constants.go +++ b/internal/cnpgi/metadata/constants.go @@ -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. diff --git a/internal/cnpgi/operator/rbac/ensure_test.go b/internal/cnpgi/operator/rbac/ensure_test.go index 6214b63..e3d7885 100644 --- a/internal/cnpgi/operator/rbac/ensure_test.go +++ b/internal/cnpgi/operator/rbac/ensure_test.go @@ -45,11 +45,11 @@ import ( func expectRequiredLabels(labels map[string]string, clusterName string) { ExpectWithOffset(1, labels).To(HaveKeyWithValue(metadata.ClusterLabelName, clusterName)) - ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, utils.AppName)) + ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, metadata.AppLabelValue)) ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, clusterName)) - ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppManagedByLabelName, "plugin-barman-cloud")) + ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppManagedByLabelName, metadata.ManagedByLabelValue)) ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppComponentLabelName, utils.DatabaseComponentName)) - ExpectWithOffset(1, labels).To(HaveKey(utils.KubernetesAppVersionLabelName)) + ExpectWithOffset(1, labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, metadata.Data.Version)) } func newScheme() *runtime.Scheme { diff --git a/internal/cnpgi/operator/specs/labels.go b/internal/cnpgi/operator/specs/labels.go index cdb1b9f..3d9042b 100644 --- a/internal/cnpgi/operator/specs/labels.go +++ b/internal/cnpgi/operator/specs/labels.go @@ -20,28 +20,22 @@ SPDX-License-Identifier: Apache-2.0 package specs import ( - "fmt" - 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 labels that must be present on all plugin-managed objects for a given Cluster. +// 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 { - requiredLabels := map[string]string{ - metadata.ClusterLabelName: cluster.Name, - // Kubernetes recommended labels - utils.KubernetesAppLabelName: utils.AppName, + return map[string]string{ + metadata.ClusterLabelName: cluster.Name, + utils.KubernetesAppLabelName: metadata.AppLabelValue, utils.KubernetesAppInstanceLabelName: cluster.Name, - utils.KubernetesAppManagedByLabelName: "plugin-barman-cloud", + utils.KubernetesAppVersionLabelName: metadata.Data.Version, utils.KubernetesAppComponentLabelName: utils.DatabaseComponentName, + utils.KubernetesAppManagedByLabelName: metadata.ManagedByLabelValue, } - - if version, err := cluster.GetPostgresqlMajorVersion(); err == nil && version != 0 { - requiredLabels[utils.KubernetesAppVersionLabelName] = fmt.Sprint(version) - } - - return requiredLabels } diff --git a/internal/cnpgi/operator/specs/labels_test.go b/internal/cnpgi/operator/specs/labels_test.go index a0cca62..82fe8ae 100644 --- a/internal/cnpgi/operator/specs/labels_test.go +++ b/internal/cnpgi/operator/specs/labels_test.go @@ -30,7 +30,7 @@ import ( ) var _ = Describe("BuildLabels", func() { - It("should return all expected labels for a cluster without an image", func() { + It("should return the recommended labels with plugin identity", func() { cluster := &cnpgv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "my-cluster", @@ -40,14 +40,15 @@ var _ = Describe("BuildLabels", func() { labels := BuildLabels(cluster) Expect(labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "my-cluster")) - Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, utils.AppName)) + Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppLabelName, metadata.AppLabelValue)) Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, "my-cluster")) - Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppManagedByLabelName, "plugin-barman-cloud")) + 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 use the major version from the image catalog ref", func() { + It("should report the plugin version regardless of the cluster's Postgres image", func() { cluster := &cnpgv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "pg16-cluster", @@ -61,7 +62,7 @@ var _ = Describe("BuildLabels", func() { } labels := BuildLabels(cluster) - Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, "16")) + Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppVersionLabelName, metadata.Data.Version)) Expect(labels).To(HaveKeyWithValue(utils.KubernetesAppInstanceLabelName, "pg16-cluster")) }) })