diff --git a/internal/cmd/restore/main.go b/internal/cmd/restore/main.go index 72d9cd02..9448d5be 100644 --- a/internal/cmd/restore/main.go +++ b/internal/cmd/restore/main.go @@ -57,6 +57,8 @@ func NewCmd() *cobra.Command { _ = viper.BindEnv("pod-name", "POD_NAME") _ = viper.BindEnv("pgdata", "PGDATA") _ = viper.BindEnv("spool-directory", "SPOOL_DIRECTORY") + _ = viper.BindEnv("custom-cnpg-group", "CUSTOM_CNPG_GROUP") + _ = viper.BindEnv("custom-cnpg-version", "CUSTOM_CNPG_VERSION") return cmd } diff --git a/internal/cnpgi/instance/manager.go b/internal/cnpgi/instance/manager.go index 7c148789..c13919af 100644 --- a/internal/cnpgi/instance/manager.go +++ b/internal/cnpgi/instance/manager.go @@ -28,16 +28,15 @@ import ( "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "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" - "sigs.k8s.io/controller-runtime/pkg/scheme" barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" 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 @@ -127,26 +126,7 @@ func generateScheme(ctx context.Context) *runtime.Scheme { 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) + pluginscheme.AddCNPGToScheme(ctx, result) return result } diff --git a/internal/cnpgi/metadata/constants.go b/internal/cnpgi/metadata/constants.go index dad413a5..aed103bc 100644 --- a/internal/cnpgi/metadata/constants.go +++ b/internal/cnpgi/metadata/constants.go @@ -26,6 +26,10 @@ import "github.com/cloudnative-pg/cnpg-i/pkg/identity" 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. + ClusterLabelName = "barmancloud.cnpg.io/cluster" + // 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/manager.go b/internal/cnpgi/operator/manager.go index 94dfa728..53db11e8 100644 --- a/internal/cnpgi/operator/manager.go +++ b/internal/cnpgi/operator/manager.go @@ -24,7 +24,6 @@ import ( "crypto/tls" // +kubebuilder:scaffold:imports - 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" @@ -49,7 +48,6 @@ var scheme = runtime.NewScheme() func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(barmancloudv1.AddToScheme(scheme)) - utilruntime.Must(cnpgv1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/internal/cnpgi/operator/rbac/doc.go b/internal/cnpgi/operator/rbac/doc.go new file mode 100644 index 00000000..802e058b --- /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 new file mode 100644 index 00000000..44bf706a --- /dev/null +++ b/internal/cnpgi/operator/rbac/ensure.go @@ -0,0 +1,174 @@ +/* +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 + +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" + "k8s.io/client-go/util/retry" + "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" +) + +// EnsureRole ensures the RBAC Role for the given Cluster matches +// the desired state derived from the given ObjectStores. On creation, +// the Cluster is set as the owner of the Role for garbage collection. +// +// 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. +func EnsureRole( + ctx context.Context, + c client.Client, + cluster *cnpgv1.Cluster, + barmanObjects []barmancloudv1.ObjectStore, +) error { + newRole := specs.BuildRole(cluster, barmanObjects) + roleKey := client.ObjectKeyFromObject(newRole) + + if err := ensureRoleExists(ctx, c, cluster, newRole); err != nil { + return err + } + + return patchRole(ctx, c, roleKey, newRole.Rules, map[string]string{ + metadata.ClusterLabelName: cluster.Name, + }) +} + +// EnsureRoleRules updates the rules of an existing Role to match +// the desired state derived from the given ObjectStores. Unlike +// EnsureRole, this function does not create Roles or set owner +// references — it only patches rules on Roles that already exist. +// It is intended for the ObjectStore controller path where no +// Cluster object is available. Returns nil if the Role does not +// exist (the Pre hook has not created it yet). +func EnsureRoleRules( + ctx context.Context, + c client.Client, + roleKey client.ObjectKey, + barmanObjects []barmancloudv1.ObjectStore, +) error { + err := patchRole(ctx, c, roleKey, specs.BuildRoleRules(barmanObjects), nil) + if apierrs.IsNotFound(err) { + log.FromContext(ctx).Debug("Role not found, skipping rule update", + "name", roleKey.Name, "namespace", roleKey.Namespace) + return nil + } + + return err +} + +// 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. +func ensureRoleExists( + ctx context.Context, + c client.Client, + cluster *cnpgv1.Cluster, + newRole *rbacv1.Role, +) error { + contextLogger := log.FromContext(ctx) + + var existing rbacv1.Role + err := c.Get(ctx, client.ObjectKeyFromObject(newRole), &existing) + if err == nil { + return nil + } + if !apierrs.IsNotFound(err) { + return err + } + + if err := specs.SetControllerReference(cluster, newRole); err != nil { + return err + } + + contextLogger.Info("Creating role", + "name", newRole.Name, "namespace", newRole.Namespace) + + createErr := c.Create(ctx, newRole) + if createErr == nil || apierrs.IsAlreadyExists(createErr) { + return nil + } + + return createErr +} + +// patchRole patches the Role's rules and optionally its labels to +// match the desired state. When desiredLabels is nil, labels are +// not modified. Uses retry.RetryOnConflict for concurrent +// modification handling. +func patchRole( + ctx context.Context, + c client.Client, + roleKey client.ObjectKey, + desiredRules []rbacv1.PolicyRule, + desiredLabels map[string]string, +) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + var role rbacv1.Role + if err := c.Get(ctx, roleKey, &role); err != nil { + return err + } + + rulesMatch := equality.Semantic.DeepEqual(desiredRules, role.Rules) + labelsMatch := desiredLabels == nil || !labelsNeedUpdate(role.Labels, desiredLabels) + + if rulesMatch && labelsMatch { + return nil + } + + contextLogger := log.FromContext(ctx) + contextLogger.Info("Patching role", + "name", role.Name, "namespace", role.Namespace) + + oldRole := role.DeepCopy() + role.Rules = desiredRules + + 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)) + }) +} + +// labelsNeedUpdate returns true if any key in desired 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 { + return true + } + } + return false +} diff --git a/internal/cnpgi/operator/rbac/ensure_test.go b/internal/cnpgi/operator/rbac/ensure_test.go new file mode 100644 index 00000000..23f364e9 --- /dev/null +++ b/internal/cnpgi/operator/rbac/ensure_test.go @@ -0,0 +1,306 @@ +/* +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" + + barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api" + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + 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" + utilruntime "k8s.io/apimachinery/pkg/util/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/metadata" + "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/rbac" +) + +func newScheme() *runtime.Scheme { + s := runtime.NewScheme() + utilruntime.Must(rbacv1.AddToScheme(s)) + utilruntime.Must(cnpgv1.AddToScheme(s)) + utilruntime.Must(barmancloudv1.AddToScheme(s)) + return s +} + +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, + }, + } +} + +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 with owner reference and label", 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)) + + Expect(role.OwnerReferences).To(HaveLen(1)) + 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")) + }) + }) + + 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() + 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 and preserve owner reference", 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()) + + secretsRule := role.Rules[2] + Expect(secretsRule.ResourceNames).To(ContainElement("aws-creds")) + Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret")) + + Expect(role.OwnerReferences).To(HaveLen(1)) + Expect(role.OwnerReferences[0].Name).To(Equal("test-cluster")) + }) + }) + + Context("when the Role exists without the cluster label (upgrade scenario)", func() { + BeforeEach(func() { + fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build() + + // Create a Role without the label (simulates pre-upgrade state) + unlabeledRole := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster-barman-cloud", + Namespace: "default", + }, + Rules: []rbacv1.PolicyRule{}, + } + Expect(fakeClient.Create(ctx, unlabeledRole)).To(Succeed()) + }) + + It("should add the label and update rules", 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(metadata.ClusterLabelName, "test-cluster")) + Expect(role.Rules).To(HaveLen(3)) + }) + }) +}) + +var _ = Describe("EnsureRoleRules", func() { + var ( + ctx context.Context + fakeClient client.Client + objects []barmancloudv1.ObjectStore + ) + + BeforeEach(func() { + ctx = context.Background() + objects = []barmancloudv1.ObjectStore{ + newObjectStore("my-store", "default", "aws-creds"), + } + }) + + Context("when the Role exists", func() { + BeforeEach(func() { + fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build() + + // Seed a labeled Role with old rules + cluster := newCluster("test-cluster", "default") + oldObjects := []barmancloudv1.ObjectStore{ + newObjectStore("my-store", "default", "old-secret"), + } + Expect(rbac.EnsureRole(ctx, fakeClient, cluster, oldObjects)).To(Succeed()) + }) + + It("should patch the rules", func() { + roleKey := client.ObjectKey{ + Namespace: "default", + Name: "test-cluster-barman-cloud", + } + err := rbac.EnsureRoleRules(ctx, fakeClient, roleKey, objects) + Expect(err).NotTo(HaveOccurred()) + + var role rbacv1.Role + Expect(fakeClient.Get(ctx, roleKey, &role)).To(Succeed()) + + secretsRule := role.Rules[2] + Expect(secretsRule.ResourceNames).To(ContainElement("aws-creds")) + Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret")) + }) + + It("should not patch when rules already match", func() { + // Seed with the same objects so rules match + cluster := newCluster("test-cluster", "default") + Expect(rbac.EnsureRole(ctx, fakeClient, cluster, objects)).To(Succeed()) + + 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)) + }) + + It("should not modify labels", func() { + 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.Labels).To(Equal(before.Labels)) + }) + }) + + Context("when the Role does not exist", func() { + BeforeEach(func() { + fakeClient = fake.NewClientBuilder().WithScheme(newScheme()).Build() + }) + + It("should return nil", func() { + roleKey := client.ObjectKey{ + Namespace: "default", + Name: "nonexistent-barman-cloud", + } + err := rbac.EnsureRoleRules(ctx, fakeClient, roleKey, objects) + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) diff --git a/internal/cnpgi/operator/rbac/suite_test.go b/internal/cnpgi/operator/rbac/suite_test.go new file mode 100644 index 00000000..42fedae8 --- /dev/null +++ b/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") +} diff --git a/internal/cnpgi/operator/reconciler.go b/internal/cnpgi/operator/reconciler.go index 9d64deb0..87647ebc 100644 --- a/internal/cnpgi/operator/reconciler.go +++ b/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,7 @@ 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); err != nil { return nil, err } @@ -137,57 +137,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, @@ -213,7 +162,7 @@ func (r ReconcilerImplementation) createRoleBinding( cluster *cnpgv1.Cluster, ) error { roleBinding := specs.BuildRoleBinding(cluster) - if err := setOwnerReference(cluster, roleBinding); err != nil { + if err := specs.SetControllerReference(cluster, roleBinding); err != nil { return err } return r.Client.Create(ctx, roleBinding) diff --git a/internal/cnpgi/operator/ownership.go b/internal/cnpgi/operator/specs/ownership.go similarity index 57% rename from internal/cnpgi/operator/ownership.go rename to internal/cnpgi/operator/specs/ownership.go index e0aadcb0..7bc6c747 100644 --- a/internal/cnpgi/operator/ownership.go +++ b/internal/cnpgi/operator/specs/ownership.go @@ -17,7 +17,7 @@ limitations under the License. SPDX-License-Identifier: Apache-2.0 */ -package operator +package specs import ( "fmt" @@ -27,26 +27,27 @@ import ( "k8s.io/utils/ptr" ) -// setOwnerReference explicitly set the owner reference between an -// owner object and a controller one. -// -// Important: this function won't use any registered scheme and will -// fail unless the metadata has been correctly set into the owner -// object. -func setOwnerReference(owner, controlled metav1.Object) error { +// 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 setOwnerReference", owner) + return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner) } - if len(ro.DeepCopyObject().GetObjectKind().GroupVersionKind().Group) == 0 { - return fmt.Errorf("%T metadata have not been set, cannot call setOwnerReference", 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: ro.GetObjectKind().GroupVersionKind().GroupVersion().String(), - Kind: ro.GetObjectKind().GroupVersionKind().Kind, + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, Name: owner.GetName(), UID: owner.GetUID(), BlockOwnerDeletion: ptr.To(true), diff --git a/internal/cnpgi/operator/specs/ownership_test.go b/internal/cnpgi/operator/specs/ownership_test.go new file mode 100644 index 00000000..35a0673d --- /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/cnpgi/operator/specs/role.go b/internal/cnpgi/operator/specs/role.go index 0c5fa705..0972f473 100644 --- a/internal/cnpgi/operator/specs/role.go +++ b/internal/cnpgi/operator/specs/role.go @@ -21,6 +21,7 @@ package specs import ( "fmt" + "slices" cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/machinery/pkg/stringset" @@ -28,6 +29,7 @@ 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 @@ -35,15 +37,20 @@ func BuildRole( cluster *cnpgv1.Cluster, barmanObjects []barmancloudv1.ObjectStore, ) *rbacv1.Role { - role := &rbacv1.Role{ + return &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Namespace: cluster.Namespace, Name: GetRBACName(cluster.Name), + Labels: map[string]string{ + metadata.ClusterLabelName: cluster.Name, + }, }, - - Rules: []rbacv1.PolicyRule{}, + Rules: BuildRoleRules(barmanObjects), } +} +// BuildRoleRules builds the RBAC PolicyRules for the given ObjectStores. +func BuildRoleRules(barmanObjects []barmancloudv1.ObjectStore) []rbacv1.PolicyRule { secretsSet := stringset.New() barmanObjectsSet := stringset.New() @@ -54,11 +61,10 @@ func BuildRole( } } - role.Rules = append( - role.Rules, - rbacv1.PolicyRule{ + return []rbacv1.PolicyRule{ + { APIGroups: []string{ - "barmancloud.cnpg.io", + barmancloudv1.GroupVersion.Group, }, Verbs: []string{ "get", @@ -70,9 +76,9 @@ func BuildRole( }, ResourceNames: barmanObjectsSet.ToSortedList(), }, - rbacv1.PolicyRule{ + { APIGroups: []string{ - "barmancloud.cnpg.io", + barmancloudv1.GroupVersion.Group, }, Verbs: []string{ "update", @@ -82,7 +88,7 @@ func BuildRole( }, ResourceNames: barmanObjectsSet.ToSortedList(), }, - rbacv1.PolicyRule{ + { APIGroups: []string{ "", }, @@ -96,9 +102,25 @@ func BuildRole( }, ResourceNames: secretsSet.ToSortedList(), }, - ) + } +} + +// ObjectStoreNamesFromRole extracts the ObjectStore names referenced +// by a plugin-managed Role. It finds the objectstores rule +// semantically (by APIGroup and Resource, not by index) and returns +// a copy of its ResourceNames. Returns nil if no matching rule is +// found. +func ObjectStoreNamesFromRole(role *rbacv1.Role) []string { + for _, rule := range role.Rules { + if len(rule.APIGroups) == 1 && + rule.APIGroups[0] == barmancloudv1.GroupVersion.Group && + len(rule.Resources) == 1 && + rule.Resources[0] == "objectstores" { + return slices.Clone(rule.ResourceNames) + } + } - return role + return nil } // BuildRoleBinding builds the role binding object for this cluster diff --git a/internal/cnpgi/operator/specs/role_test.go b/internal/cnpgi/operator/specs/role_test.go new file mode 100644 index 00000000..bb5c9a13 --- /dev/null +++ b/internal/cnpgi/operator/specs/role_test.go @@ -0,0 +1,210 @@ +/* +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 ( + barmanapi "github.com/cloudnative-pg/barman-cloud/pkg/api" + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + 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" + + barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" + "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata" +) + +func newTestObjectStore(name, secretName string) barmancloudv1.ObjectStore { + return barmancloudv1.ObjectStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + 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("BuildRoleRules", func() { + It("should produce 3 rules with correct ResourceNames", func() { + objects := []barmancloudv1.ObjectStore{ + newTestObjectStore("store-a", "secret-a"), + newTestObjectStore("store-b", "secret-b"), + } + rules := BuildRoleRules(objects) + Expect(rules).To(HaveLen(3)) + + Expect(rules[0].APIGroups).To(Equal([]string{barmancloudv1.GroupVersion.Group})) + Expect(rules[0].Resources).To(Equal([]string{"objectstores"})) + Expect(rules[0].ResourceNames).To(ConsistOf("store-a", "store-b")) + + Expect(rules[1].APIGroups).To(Equal([]string{barmancloudv1.GroupVersion.Group})) + Expect(rules[1].Resources).To(Equal([]string{"objectstores/status"})) + Expect(rules[1].ResourceNames).To(ConsistOf("store-a", "store-b")) + + Expect(rules[2].APIGroups).To(Equal([]string{""})) + Expect(rules[2].Resources).To(Equal([]string{"secrets"})) + Expect(rules[2].ResourceNames).To(ConsistOf("secret-a", "secret-b")) + }) + + It("should produce rules with empty ResourceNames for empty input", func() { + rules := BuildRoleRules(nil) + Expect(rules).To(HaveLen(3)) + Expect(rules[0].ResourceNames).To(BeEmpty()) + Expect(rules[0].ResourceNames).NotTo(BeNil()) + Expect(rules[1].ResourceNames).To(BeEmpty()) + Expect(rules[2].ResourceNames).To(BeEmpty()) + }) + + It("should deduplicate secret names across ObjectStores", func() { + objects := []barmancloudv1.ObjectStore{ + newTestObjectStore("store-a", "shared-secret"), + newTestObjectStore("store-b", "shared-secret"), + } + rules := BuildRoleRules(objects) + Expect(rules[2].ResourceNames).To(Equal([]string{"shared-secret"})) + }) +}) + +var _ = Describe("BuildRole", func() { + It("should set the cluster label", func() { + cluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + } + role := BuildRole(cluster, nil) + Expect(role.Labels).To(HaveKeyWithValue(metadata.ClusterLabelName, "my-cluster")) + Expect(role.Name).To(Equal("my-cluster-barman-cloud")) + Expect(role.Namespace).To(Equal("default")) + }) +}) + +var _ = Describe("BuildRoleRules / ObjectStoreNamesFromRole round-trip", func() { + It("should recover the same ObjectStore names from built rules", func() { + objects := []barmancloudv1.ObjectStore{ + newTestObjectStore("store-a", "secret-a"), + newTestObjectStore("store-b", "secret-b"), + } + rules := BuildRoleRules(objects) + role := &rbacv1.Role{Rules: rules} + names := ObjectStoreNamesFromRole(role) + Expect(names).To(ConsistOf("store-a", "store-b")) + }) + + It("should recover empty names from rules built with no ObjectStores", func() { + rules := BuildRoleRules(nil) + role := &rbacv1.Role{Rules: rules} + names := ObjectStoreNamesFromRole(role) + Expect(names).To(BeEmpty()) + }) +}) + +var _ = Describe("ObjectStoreNamesFromRole", func() { + It("should extract ObjectStore names from a well-formed Role", func() { + role := &rbacv1.Role{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{barmancloudv1.GroupVersion.Group}, + Resources: []string{"objectstores"}, + ResourceNames: []string{"store-a", "store-b"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + ResourceNames: []string{"secret-a"}, + }, + }, + } + Expect(ObjectStoreNamesFromRole(role)).To(Equal([]string{"store-a", "store-b"})) + }) + + It("should return nil for a Role with no matching rule", func() { + role := &rbacv1.Role{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + ResourceNames: []string{"secret-a"}, + }, + }, + } + Expect(ObjectStoreNamesFromRole(role)).To(BeNil()) + }) + + It("should return nil for a Role with empty rules", func() { + role := &rbacv1.Role{} + Expect(ObjectStoreNamesFromRole(role)).To(BeNil()) + }) + + It("should not match a rule with a different APIGroup", func() { + role := &rbacv1.Role{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"other.io"}, + Resources: []string{"objectstores"}, + ResourceNames: []string{"store-a"}, + }, + }, + } + Expect(ObjectStoreNamesFromRole(role)).To(BeNil()) + }) + + It("should not match a rule with multiple APIGroups", func() { + role := &rbacv1.Role{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{barmancloudv1.GroupVersion.Group, "other.io"}, + Resources: []string{"objectstores"}, + ResourceNames: []string{"store-a"}, + }, + }, + } + Expect(ObjectStoreNamesFromRole(role)).To(BeNil()) + }) + + It("should not match a rule for objectstores/status", func() { + role := &rbacv1.Role{ + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{barmancloudv1.GroupVersion.Group}, + Resources: []string{"objectstores/status"}, + ResourceNames: []string{"store-a"}, + }, + }, + } + Expect(ObjectStoreNamesFromRole(role)).To(BeNil()) + }) +}) diff --git a/internal/cnpgi/restore/manager.go b/internal/cnpgi/restore/manager.go index 575feb74..339eab2f 100644 --- a/internal/cnpgi/restore/manager.go +++ b/internal/cnpgi/restore/manager.go @@ -22,7 +22,6 @@ package restore import ( "context" - cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/machinery/pkg/log" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" @@ -33,14 +32,20 @@ import ( "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" ) -var scheme = runtime.NewScheme() +// 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() -func init() { - utilruntime.Must(barmancloudv1.AddToScheme(scheme)) - utilruntime.Must(cnpgv1.AddToScheme(scheme)) - utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + 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 @@ -48,6 +53,8 @@ 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, Client: client.Options{ diff --git a/internal/controller/objectstore_controller.go b/internal/controller/objectstore_controller.go index 31fe83f7..a79b7c95 100644 --- a/internal/controller/objectstore_controller.go +++ b/internal/controller/objectstore_controller.go @@ -21,14 +21,23 @@ package controller import ( "context" + "errors" "fmt" + "slices" "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/predicate" 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/rbac" + "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs" ) // ObjectStoreReconciler reconciles a ObjectStore object. @@ -40,33 +49,89 @@ 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/finalizers,verbs=update // +kubebuilder:rbac:groups=postgresql.cnpg.io,resources=backups,verbs=get;list;watch +// +kubebuilder:rbac:groups=postgresql.cnpg.io,resources=clusters/finalizers,verbs=update // +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) - - // TODO(user): your logic here - - return ctrl.Result{}, nil +// Reconcile ensures that the RBAC Role for each Cluster referencing +// this ObjectStore is up to date with the current ObjectStore spec. +// It discovers affected Roles by listing plugin-managed Roles and +// inspecting their rules, without needing access to Cluster objects. +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) + + contextLogger.Info("ObjectStore reconciliation start") + + var roleList rbacv1.RoleList + if err := r.List(ctx, &roleList, + client.InNamespace(req.Namespace), + client.HasLabels{metadata.ClusterLabelName}, + ); err != nil { + return ctrl.Result{}, fmt.Errorf("while listing roles: %w", err) + } + + var errs []error + for i := range roleList.Items { + role := &roleList.Items[i] + + objectStoreNames := specs.ObjectStoreNamesFromRole(role) + if !slices.Contains(objectStoreNames, req.Name) { + continue + } + + contextLogger.Info("Reconciling RBAC for role", + "roleName", role.Name) + + if err := r.reconcileRoleRules(ctx, role, objectStoreNames); err != nil { + contextLogger.Error(err, "Failed to reconcile RBAC for role", + "roleName", role.Name) + errs = append(errs, fmt.Errorf("while reconciling role %s: %w", role.Name, err)) + } + } + + contextLogger.Info("ObjectStore reconciliation completed") + return ctrl.Result{}, errors.Join(errs...) +} + +// reconcileRoleRules fetches the ObjectStores referenced by the +// Role and patches its rules to match the current specs. +func (r *ObjectStoreReconciler) reconcileRoleRules( + ctx context.Context, + role *rbacv1.Role, + objectStoreNames []string, +) error { + contextLogger := log.FromContext(ctx) + barmanObjects := make([]barmancloudv1.ObjectStore, 0, len(objectStoreNames)) + + for _, name := range objectStoreNames { + var barmanObject barmancloudv1.ObjectStore + if err := r.Get(ctx, client.ObjectKey{ + Namespace: role.Namespace, + Name: name, + }, &barmanObject); err != nil { + if apierrs.IsNotFound(err) { + contextLogger.Info("ObjectStore not found, skipping", + "objectStoreName", name) + continue + } + return fmt.Errorf("while getting ObjectStore %s: %w", name, err) + } + barmanObjects = append(barmanObjects, barmanObject) + } + + return rbac.EnsureRoleRules(ctx, r.Client, client.ObjectKeyFromObject(role), barmanObjects) } // 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) diff --git a/internal/controller/objectstore_controller_test.go b/internal/controller/objectstore_controller_test.go index 6c163d38..6779415d 100644 --- a/internal/controller/objectstore_controller_test.go +++ b/internal/controller/objectstore_controller_test.go @@ -23,69 +23,329 @@ import ( "context" 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" + utilruntime "k8s.io/apimachinery/pkg/util/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" + "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/specs" ) -var _ = Describe("ObjectStore Controller", func() { - Context("When reconciling a resource", func() { - const resourceName = "test-resource" - - ctx := context.Background() - - typeNamespacedName := types.NamespacedName{ - Name: resourceName, - Namespace: "default", // TODO(user):Modify as needed - } - objectstore := &barmancloudv1.ObjectStore{} - - 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", +func newFakeScheme() *runtime.Scheme { + s := runtime.NewScheme() + utilruntime.Must(rbacv1.AddToScheme(s)) + utilruntime.Must(barmancloudv1.AddToScheme(s)) + return s +} + +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", + }, }, - Spec: barmancloudv1.ObjectStoreSpec{ - Configuration: barmanapi.BarmanObjectStoreConfiguration{DestinationPath: "/tmp"}, + }, + }, + }, + } +} + +func newLabeledRole(clusterName, namespace string, objectStores []barmancloudv1.ObjectStore) *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: specs.GetRBACName(clusterName), + Namespace: namespace, + Labels: map[string]string{ + metadata.ClusterLabelName: clusterName, + }, + }, + Rules: specs.BuildRoleRules(objectStores), + } +} + +var _ = Describe("ObjectStoreReconciler", func() { + var ( + ctx context.Context + scheme *runtime.Scheme + ) + + BeforeEach(func() { + ctx = context.Background() + scheme = newFakeScheme() + }) + + Describe("Reconcile", func() { + It("should update Role rules when ObjectStore credentials change", func() { + oldStore := newTestObjectStore("my-store", "default", "old-secret") + role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*oldStore}) + + // Update the ObjectStore with new credentials + newStore := newTestObjectStore("my-store", "default", "new-secret") + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(role, newStore). + 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{})) + + var updatedRole rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: "my-cluster-barman-cloud", + }, &updatedRole)).To(Succeed()) + + secretsRule := updatedRole.Rules[2] + Expect(secretsRule.ResourceNames).To(ContainElement("new-secret")) + Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret")) + }) + + It("should skip Roles that don't reference the ObjectStore", func() { + otherStore := newTestObjectStore("other-store", "default", "other-creds") + role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*otherStore}) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(role). + Build() + + reconciler := &ObjectStoreReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + var before rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: "my-cluster-barman-cloud", + }, &before)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "unrelated-store", + Namespace: "default", + }, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + var after rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: "my-cluster-barman-cloud", + }, &after)).To(Succeed()) + + Expect(after.ResourceVersion).To(Equal(before.ResourceVersion)) + }) + + It("should succeed with no labeled Roles 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{})) + }) + + It("should handle deleted ObjectStores gracefully", func() { + storeA := newTestObjectStore("store-a", "default", "secret-a") + storeB := newTestObjectStore("store-b", "default", "secret-b") + role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*storeA, *storeB}) + + // Only store-a exists; store-b was deleted + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(role, storeA). + Build() + + reconciler := &ObjectStoreReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + result, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "store-b", + Namespace: "default", + }, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + var updatedRole rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: "my-cluster-barman-cloud", + }, &updatedRole)).To(Succeed()) + + objectStoreRule := updatedRole.Rules[0] + Expect(objectStoreRule.ResourceNames).To(ContainElement("store-a")) + Expect(objectStoreRule.ResourceNames).NotTo(ContainElement("store-b")) + }) + + It("should not panic on a Role with empty rules", func() { + emptyRole := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-barman-cloud", + Namespace: "default", + Labels: map[string]string{ + metadata.ClusterLabelName: "empty", }, - // TODO(user): Specify other spec details if needed. - } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }, } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(emptyRole). + 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{})) }) - AfterEach(func() { - // TODO(user): Cleanup logic after each test, like removing the resource instance. - resource := &barmancloudv1.ObjectStore{} - err := k8sClient.Get(ctx, typeNamespacedName, resource) + It("should produce empty ResourceNames when all ObjectStores are deleted", func() { + store := newTestObjectStore("my-store", "default", "aws-creds") + role := newLabeledRole("my-cluster", "default", []barmancloudv1.ObjectStore{*store}) + + // Don't add the ObjectStore to the fake client (simulates deletion) + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(role). + 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{})) - By("Cleanup the specific resource instance ObjectStore") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + var updatedRole rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: "my-cluster-barman-cloud", + }, &updatedRole)).To(Succeed()) + + // All rules should have empty ResourceNames + Expect(updatedRole.Rules[0].ResourceNames).To(BeEmpty()) + Expect(updatedRole.Rules[1].ResourceNames).To(BeEmpty()) + Expect(updatedRole.Rules[2].ResourceNames).To(BeEmpty()) }) - It("should successfully reconcile the resource", func() { - By("Reconciling the created resource") - controllerReconciler := &ObjectStoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), + + It("should reconcile multiple Roles referencing the same ObjectStore", func() { + store := newTestObjectStore("shared-store", "default", "new-secret") + oldStore := barmancloudv1.ObjectStore{ + ObjectMeta: metav1.ObjectMeta{Name: "shared-store", Namespace: "default"}, + Spec: barmancloudv1.ObjectStoreSpec{ + Configuration: barmanapi.BarmanObjectStoreConfiguration{ + DestinationPath: "s3://bucket/path", + BarmanCredentials: barmanapi.BarmanCredentials{ + AWS: &barmanapi.S3Credentials{ + AccessKeyIDReference: &machineryapi.SecretKeySelector{ + LocalObjectReference: machineryapi.LocalObjectReference{Name: "old-secret"}, + Key: "ACCESS_KEY_ID", + }, + }, + }, + }, + }, + } + + role1 := newLabeledRole("cluster-1", "default", []barmancloudv1.ObjectStore{oldStore}) + role2 := newLabeledRole("cluster-2", "default", []barmancloudv1.ObjectStore{oldStore}) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(role1, role2, store). + 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: "shared-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{})) + + for _, clusterName := range []string{"cluster-1", "cluster-2"} { + var updatedRole rbacv1.Role + Expect(fakeClient.Get(ctx, client.ObjectKey{ + Namespace: "default", + Name: specs.GetRBACName(clusterName), + }, &updatedRole)).To(Succeed()) + + secretsRule := updatedRole.Rules[2] + Expect(secretsRule.ResourceNames).To(ContainElement("new-secret")) + Expect(secretsRule.ResourceNames).NotTo(ContainElement("old-secret")) + } }) }) }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 18a4029a..711c56ad 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -20,81 +20,14 @@ SPDX-License-Identifier: Apache-2.0 package controller import ( - "context" - "fmt" - "path/filepath" - "runtime" "testing" - // +kubebuilder:scaffold:imports - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var ( - cfg *rest.Config - k8sClient client.Client - testEnv *envtest.Environment - ctx context.Context - cancel context.CancelFunc -) - func TestControllers(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Controller Suite") } - -var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - ctx, cancel = context.WithCancel(context.TODO()) - - By("bootstrapping test environment") - testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, - ErrorIfCRDPathMissing: true, - - // The BinaryAssetsDirectory is only required if you want to run the tests directly - // without call the makefile target test. If not informed it will look for the - // default path defined in controller-runtime which is /usr/local/kubebuilder/. - // Note that you must have the required binaries setup under the bin directory to perform - // the tests directly. When we run make test it will be setup and used automatically. - BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s", - fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)), - } - - var err error - // cfg is defined in this file globally. - cfg, err = testEnv.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) - - err = barmancloudv1.AddToScheme(scheme.Scheme) - Expect(err).NotTo(HaveOccurred()) - - // +kubebuilder:scaffold:scheme - - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) -}) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - cancel() - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}) diff --git a/internal/scheme/cnpg.go b/internal/scheme/cnpg.go new file mode 100644 index 00000000..a012e9dc --- /dev/null +++ b/internal/scheme/cnpg.go @@ -0,0 +1,56 @@ +/* +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 + +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" + crscheme "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +// AddCNPGToScheme registers CNPG types into the given scheme using +// the API group configured via CUSTOM_CNPG_GROUP/CUSTOM_CNPG_VERSION +// environment variables, defaulting to postgresql.cnpg.io/v1. +// This allows the plugin to work with any CNPG-based operator. +func AddCNPGToScheme(ctx context.Context, s *runtime.Scheme) { + 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 + } + + schemeGroupVersion := schema.GroupVersion{Group: cnpgGroup, Version: cnpgVersion} + schemeBuilder := &crscheme.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(s)) + + log.FromContext(ctx).Info("CNPG types registration", "schemeGroupVersion", schemeGroupVersion) +} diff --git a/internal/scheme/doc.go b/internal/scheme/doc.go new file mode 100644 index 00000000..71285235 --- /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