chore: add review comments and clean up whitespace

Document the upgrade gap for pre-existing Roles without the
ClusterLabelName label, the single-owner assumption in
SetControllerReference, and the race window between the Pre hook
and the ObjectStore controller.

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

internal/cnpgi/common/scheme.go

@@ -0,0 +1,67 @@
+/*
+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 common
+
+import (
+	"context"
+
+	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
+	"github.com/cloudnative-pg/machinery/pkg/log"
+	"github.com/spf13/viper"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/scheme"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+)
+
+// GenerateScheme creates a runtime.Scheme object with all the
+// definition needed to support the sidecar. This allows
+// the plugin to be used in every CNPG-based operator.
+func GenerateScheme(ctx context.Context) *runtime.Scheme {
+	result := runtime.NewScheme()
+
+	utilruntime.Must(barmancloudv1.AddToScheme(result))
+	utilruntime.Must(clientgoscheme.AddToScheme(result))
+
+	cnpgGroup := viper.GetString("custom-cnpg-group")
+	cnpgVersion := viper.GetString("custom-cnpg-version")
+	if len(cnpgGroup) == 0 {
+		cnpgGroup = cnpgv1.SchemeGroupVersion.Group
+	}
+	if len(cnpgVersion) == 0 {
+		cnpgVersion = cnpgv1.SchemeGroupVersion.Version
+	}
+
+	// Proceed with custom registration of the CNPG scheme
+	schemeGroupVersion := schema.GroupVersion{Group: cnpgGroup, Version: cnpgVersion}
+	schemeBuilder := &scheme.Builder{GroupVersion: schemeGroupVersion}
+	schemeBuilder.Register(&cnpgv1.Cluster{}, &cnpgv1.ClusterList{})
+	schemeBuilder.Register(&cnpgv1.Backup{}, &cnpgv1.BackupList{})
+	schemeBuilder.Register(&cnpgv1.ScheduledBackup{}, &cnpgv1.ScheduledBackupList{})
+	utilruntime.Must(schemeBuilder.AddToScheme(result))
+
+	schemeLog := log.FromContext(ctx)
+	schemeLog.Info("CNPG types registration", "schemeGroupVersion", schemeGroupVersion)
+
+	return result
+}

internal/cnpgi/instance/manager.go

@@ -27,21 +27,18 @@ import (
 	"github.com/cloudnative-pg/machinery/pkg/log"
 	"github.com/spf13/viper"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"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/common"
 	extendedclient "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/instance/internal/client"
-	pluginscheme "github.com/cloudnative-pg/plugin-barman-cloud/internal/scheme"
 )
 
 // Start starts the sidecar informers and CNPG-i server
 func Start(ctx context.Context) error {
-	scheme := generateScheme(ctx)
+	scheme := common.GenerateScheme(ctx)
 
 	setupLog := log.FromContext(ctx)
 	setupLog.Info("Starting barman cloud instance plugin")
@@ -117,16 +114,3 @@ func Start(ctx context.Context) error {
 
 	return nil
 }
-
-// generateScheme creates a runtime.Scheme object with all the
-// definition needed to support the sidecar. This allows
-// the plugin to be used in every CNPG-based operator.
-func generateScheme(ctx context.Context) *runtime.Scheme {
-	result := runtime.NewScheme()
-
-	utilruntime.Must(barmancloudv1.AddToScheme(result))
-	utilruntime.Must(clientgoscheme.AddToScheme(result))
-	pluginscheme.AddCNPGToScheme(ctx, result)
-
-	return result
-}

internal/cnpgi/operator/rbac/ensure.go

@@ -42,6 +42,13 @@ import (
 // This function is called from the Pre hook (gRPC). It creates the
 // Role if it does not exist, then patches rules and labels to match
 // the desired state.
+//
+// Note: the ObjectStore controller (EnsureRoleRules) can patch the
+// same Role concurrently. Both paths use RetryOnConflict but compute
+// desired rules from their own view of ObjectStores. If the Pre hook
+// reads stale ObjectStore data from the informer cache, it may
+// briefly revert a fresher update. This is self-healing: the next
+// ObjectStore reconcile restores the correct state.
 func EnsureRole(
 	ctx context.Context,
 	c client.Client,

internal/cnpgi/operator/specs/ownership.go

@@ -33,6 +33,11 @@ import (
 // the operator does not know the CNPG API group at compile time
 // (it may be customized), while the Cluster object decoded from
 // the gRPC request carries the correct GVK in its TypeMeta.
+//
+// This function replaces all existing owner references rather than
+// merging, so it assumes the controlled object has a single owner.
+// This holds for plugin-managed Roles and RoleBindings, which are
+// exclusively owned by one Cluster.
 func SetControllerReference(owner, controlled metav1.Object) error {
 	ro, ok := owner.(runtime.Object)
 	if !ok {

internal/cnpgi/restore/manager.go

@@ -25,38 +25,20 @@ import (
 	"github.com/cloudnative-pg/machinery/pkg/log"
 	"github.com/spf13/viper"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
-	pluginscheme "github.com/cloudnative-pg/plugin-barman-cloud/internal/scheme"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/common"
 )
 
-// generateScheme creates a runtime.Scheme with all type definitions
-// needed by the restore sidecar. CNPG types are registered under a
-// configurable API group to support custom CNPG-based operators.
-func generateScheme(ctx context.Context) *runtime.Scheme {
-	result := runtime.NewScheme()
-
-	utilruntime.Must(barmancloudv1.AddToScheme(result))
-	utilruntime.Must(clientgoscheme.AddToScheme(result))
-	pluginscheme.AddCNPGToScheme(ctx, result)
-
-	return result
-}
-
 // Start starts the sidecar informers and CNPG-i server
 func Start(ctx context.Context) error {
 	setupLog := log.FromContext(ctx)
 	setupLog.Info("Starting barman cloud instance plugin")
 
-	scheme := generateScheme(ctx)
-
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-		Scheme: scheme,
+		Scheme: common.GenerateScheme(ctx),
 		Client: client.Options{
 			Cache: &client.CacheOptions{
 				DisableFor: []client.Object{

internal/controller/objectstore_controller.go

@@ -68,6 +68,10 @@ func (r *ObjectStoreReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	contextLogger.Info("ObjectStore reconciliation start")
 
+	// NOTE: Roles created before the introduction of ClusterLabelName
+	// are not discovered here. The Pre hook patches the label on every
+	// Cluster reconciliation, so unlabeled Roles are picked up after
+	// the next Cluster reconcile cycle.
 	var roleList rbacv1.RoleList
 	if err := r.List(ctx, &roleList,
 		client.InNamespace(req.Namespace),