From 4dfd20b2efd3c30e7555a66e08e81f0a6760a787 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Fri, 10 Apr 2026 10:25:54 +0200 Subject: [PATCH] fix: read owner GVK from object metadata instead of scheme The operator does not know the CNPG API group at runtime (it is not a sidecar injected by the CNPG operator, so CUSTOM_CNPG_GROUP and CUSTOM_CNPG_VERSION are not available). Move SetControllerReference to the specs package and read the GVK from the decoded Cluster's TypeMeta rather than looking it up in the scheme. Remove CNPG types from the operator's scheme and the env var bindings from cmd/operator since they are no longer needed. Signed-off-by: Marco Nenciarini --- internal/cmd/operator/main.go | 2 - internal/cnpgi/operator/manager.go | 17 +--- internal/cnpgi/operator/rbac/doc.go | 22 +++++ internal/cnpgi/operator/rbac/ensure.go | 5 +- internal/cnpgi/operator/rbac/ensure_test.go | 4 + internal/cnpgi/operator/reconciler.go | 3 +- internal/cnpgi/operator/specs/ownership.go | 59 ++++++++++++ .../cnpgi/operator/specs/ownership_test.go | 90 +++++++++++++++++++ internal/scheme/cnpg.go | 2 - internal/scheme/doc.go | 22 +++++ 10 files changed, 203 insertions(+), 23 deletions(-) create mode 100644 internal/cnpgi/operator/rbac/doc.go create mode 100644 internal/cnpgi/operator/specs/ownership.go create mode 100644 internal/cnpgi/operator/specs/ownership_test.go create mode 100644 internal/scheme/doc.go diff --git a/internal/cmd/operator/main.go b/internal/cmd/operator/main.go index 49f9ad9..3357054 100644 --- a/internal/cmd/operator/main.go +++ b/internal/cmd/operator/main.go @@ -102,8 +102,6 @@ func NewCmd() *cobra.Command { _ = viper.BindPFlag("server-address", cmd.Flags().Lookup("server-address")) _ = viper.BindEnv("sidecar-image", "SIDECAR_IMAGE") - _ = viper.BindEnv("custom-cnpg-group", "CUSTOM_CNPG_GROUP") - _ = viper.BindEnv("custom-cnpg-version", "CUSTOM_CNPG_VERSION") return cmd } diff --git a/internal/cnpgi/operator/manager.go b/internal/cnpgi/operator/manager.go index 87cc8ee..53db11e 100644 --- a/internal/cnpgi/operator/manager.go +++ b/internal/cnpgi/operator/manager.go @@ -37,33 +37,24 @@ import ( barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" "github.com/cloudnative-pg/plugin-barman-cloud/internal/controller" - pluginscheme "github.com/cloudnative-pg/plugin-barman-cloud/internal/scheme" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" ) -// generateScheme creates a runtime.Scheme with all type definitions -// needed by the operator. 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() +var scheme = runtime.NewScheme() - utilruntime.Must(clientgoscheme.AddToScheme(result)) - utilruntime.Must(barmancloudv1.AddToScheme(result)) - pluginscheme.AddCNPGToScheme(ctx, result) +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(barmancloudv1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme - - return result } // Start starts the manager func Start(ctx context.Context) error { setupLog := log.FromContext(ctx) - scheme := generateScheme(ctx) - var tlsOpts []func(*tls.Config) // if the enable-http2 flag is false (the default), http/2 should be disabled diff --git a/internal/cnpgi/operator/rbac/doc.go b/internal/cnpgi/operator/rbac/doc.go new file mode 100644 index 0000000..802e058 --- /dev/null +++ b/internal/cnpgi/operator/rbac/doc.go @@ -0,0 +1,22 @@ +/* +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 diff --git a/internal/cnpgi/operator/rbac/ensure.go b/internal/cnpgi/operator/rbac/ensure.go index c92821c..44bf706 100644 --- a/internal/cnpgi/operator/rbac/ensure.go +++ b/internal/cnpgi/operator/rbac/ensure.go @@ -17,8 +17,6 @@ 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 ( @@ -31,7 +29,6 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata" @@ -106,7 +103,7 @@ func ensureRoleExists( return err } - if err := controllerutil.SetControllerReference(cluster, newRole, c.Scheme()); err != nil { + if err := specs.SetControllerReference(cluster, newRole); err != nil { return err } diff --git a/internal/cnpgi/operator/rbac/ensure_test.go b/internal/cnpgi/operator/rbac/ensure_test.go index 380c2f6..23f364e 100644 --- a/internal/cnpgi/operator/rbac/ensure_test.go +++ b/internal/cnpgi/operator/rbac/ensure_test.go @@ -49,6 +49,10 @@ func newScheme() *runtime.Scheme { func newCluster(name, namespace string) *cnpgv1.Cluster { return &cnpgv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: cnpgv1.SchemeGroupVersion.String(), + Kind: cnpgv1.ClusterKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, diff --git a/internal/cnpgi/operator/reconciler.go b/internal/cnpgi/operator/reconciler.go index fa4b6fc..87647eb 100644 --- a/internal/cnpgi/operator/reconciler.go +++ b/internal/cnpgi/operator/reconciler.go @@ -30,7 +30,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config" @@ -163,7 +162,7 @@ func (r ReconcilerImplementation) createRoleBinding( cluster *cnpgv1.Cluster, ) error { roleBinding := specs.BuildRoleBinding(cluster) - if err := controllerutil.SetControllerReference(cluster, roleBinding, r.Client.Scheme()); err != nil { + if err := specs.SetControllerReference(cluster, roleBinding); err != nil { return err } return r.Client.Create(ctx, roleBinding) diff --git a/internal/cnpgi/operator/specs/ownership.go b/internal/cnpgi/operator/specs/ownership.go new file mode 100644 index 0000000..7bc6c74 --- /dev/null +++ b/internal/cnpgi/operator/specs/ownership.go @@ -0,0 +1,59 @@ +/* +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 ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" +) + +// SetControllerReference sets an owner reference on controlled +// pointing to owner, reading the GVK from the owner object's +// metadata rather than from a scheme. This is necessary because +// 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. +func SetControllerReference(owner, controlled metav1.Object) error { + ro, ok := owner.(runtime.Object) + if !ok { + return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner) + } + + gvk := ro.GetObjectKind().GroupVersionKind() + if gvk.Kind == "" { + return fmt.Errorf("%T has no GVK set in its metadata, cannot call SetControllerReference", owner) + } + + controlled.SetOwnerReferences([]metav1.OwnerReference{ + { + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + Name: owner.GetName(), + UID: owner.GetUID(), + BlockOwnerDeletion: ptr.To(true), + Controller: ptr.To(true), + }, + }) + + return nil +} diff --git a/internal/cnpgi/operator/specs/ownership_test.go b/internal/cnpgi/operator/specs/ownership_test.go new file mode 100644 index 0000000..35a0673 --- /dev/null +++ b/internal/cnpgi/operator/specs/ownership_test.go @@ -0,0 +1,90 @@ +/* +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/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/types" +) + +var _ = Describe("SetControllerReference", func() { + It("should set the owner reference from the owner's TypeMeta", func() { + owner := &cnpgv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "postgresql.cnpg.io/v1", + Kind: "Cluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + UID: types.UID("test-uid"), + }, + } + controlled := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-role", + Namespace: "default", + }, + } + + Expect(SetControllerReference(owner, controlled)).To(Succeed()) + Expect(controlled.OwnerReferences).To(HaveLen(1)) + Expect(controlled.OwnerReferences[0].APIVersion).To(Equal("postgresql.cnpg.io/v1")) + Expect(controlled.OwnerReferences[0].Kind).To(Equal("Cluster")) + Expect(controlled.OwnerReferences[0].Name).To(Equal("my-cluster")) + Expect(controlled.OwnerReferences[0].UID).To(Equal(types.UID("test-uid"))) + Expect(*controlled.OwnerReferences[0].Controller).To(BeTrue()) + Expect(*controlled.OwnerReferences[0].BlockOwnerDeletion).To(BeTrue()) + }) + + It("should work with a custom CNPG API group", func() { + owner := &cnpgv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "mycompany.io/v1", + Kind: "Cluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + UID: types.UID("test-uid"), + }, + } + controlled := &rbacv1.Role{} + + Expect(SetControllerReference(owner, controlled)).To(Succeed()) + Expect(controlled.OwnerReferences[0].APIVersion).To(Equal("mycompany.io/v1")) + }) + + It("should fail when the owner has no GVK set", func() { + owner := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + } + controlled := &rbacv1.Role{} + + err := SetControllerReference(owner, controlled) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("has no GVK set")) + }) +}) diff --git a/internal/scheme/cnpg.go b/internal/scheme/cnpg.go index 5ebd4b3..a012e9d 100644 --- a/internal/scheme/cnpg.go +++ b/internal/scheme/cnpg.go @@ -17,8 +17,6 @@ limitations under the License. SPDX-License-Identifier: Apache-2.0 */ -// Package scheme provides utilities for building runtime schemes -// with support for custom CNPG API groups. package scheme import ( diff --git a/internal/scheme/doc.go b/internal/scheme/doc.go new file mode 100644 index 0000000..7128523 --- /dev/null +++ b/internal/scheme/doc.go @@ -0,0 +1,22 @@ +/* +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 provides utilities for building runtime schemes +// with support for custom CNPG API groups. +package scheme