From 187da0e53d846fcdce6b22b9a760f10451c424c9 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Mon, 11 Nov 2024 12:15:39 -0300 Subject: [PATCH] fixup! fix: Enforce NonAdminBackup spec field values apply enforcing to Velero Backup Signed-off-by: Mateus Oliveira --- internal/common/function/function_test.go | 2 +- .../controller/nonadminbackup_controller.go | 12 +++++ .../nonadminbackup_controller_test.go | 53 ++++++++++++++----- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/internal/common/function/function_test.go b/internal/common/function/function_test.go index 4daebf3..eaf7382 100644 --- a/internal/common/function/function_test.go +++ b/internal/common/function/function_test.go @@ -97,7 +97,7 @@ func TestValidateBackupSpec(t *testing.T) { spec: &velerov1.BackupSpec{ IncludedNamespaces: []string{"namespace1", "namespace2", "namespace3"}, }, - errMessage: "spec.backupSpec.IncludedNamespaces can not contain namespaces other than: non-admin-backup-namespace", + errMessage: "NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than: non-admin-backup-namespace", }, { name: "valid spec", diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index fd45f2a..5b51a5a 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -20,6 +20,7 @@ package controller import ( "context" "errors" + "fmt" "reflect" "github.com/go-logr/logr" @@ -247,6 +248,17 @@ func (r *NonAdminBackupReconciler) createVeleroBackupAndSyncWithNonAdminBackup(c backupSpec := nab.Spec.BackupSpec.DeepCopy() backupSpec.IncludedNamespaces = []string{nab.Namespace} + enforcedSpec := reflect.ValueOf(r.EnforcedBackupSpec).Elem() + for index := range enforcedSpec.NumField() { + enforcedField := enforcedSpec.Field(index) + enforcedFieldName := enforcedSpec.Type().Field(index).Name + fmt.Printf("%v\n", enforcedFieldName) + currentField := reflect.ValueOf(backupSpec).Elem().FieldByName(enforcedFieldName) + if !enforcedField.IsZero() && currentField.IsZero() { + currentField.Set(enforcedField) + } + } + veleroBackup := velerov1.Backup{ ObjectMeta: metav1.ObjectMeta{ Name: veleroBackupNameUUID, diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index 5e5a371..c670b8e 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -50,8 +51,9 @@ type nonAdminBackupSingleReconcileScenario struct { } type nonAdminBackupFullReconcileScenario struct { - spec nacv1alpha1.NonAdminBackupSpec - status nacv1alpha1.NonAdminBackupStatus + spec nacv1alpha1.NonAdminBackupSpec + status nacv1alpha1.NonAdminBackupStatus + enforcedBackupSpec *velerov1.BackupSpec } func buildTestNonAdminBackup(nonAdminNamespace string, nonAdminName string, spec nacv1alpha1.NonAdminBackupSpec) *nacv1alpha1.NonAdminBackup { @@ -242,9 +244,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func // priorResourceVersion, err := strconv.Atoi(nonAdminBackup.ResourceVersion) // gomega.Expect(err).To(gomega.Not(gomega.HaveOccurred())) result, err := (&NonAdminBackupReconciler{ - Client: k8sClient, - Scheme: testEnv.Scheme, - OADPNamespace: oadpNamespace, + Client: k8sClient, + Scheme: testEnv.Scheme, + OADPNamespace: oadpNamespace, + EnforcedBackupSpec: &velerov1.BackupSpec{}, }).Reconcile( context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{ @@ -436,11 +439,11 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func Type: "Accepted", Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", - Message: "spec.backupSpec.IncludedNamespaces can not contain namespaces other than:", + Message: "NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than:", }, }, }, - resultError: reconcile.TerminalError(fmt.Errorf("spec.backupSpec.IncludedNamespaces can not contain namespaces other than: ")), + resultError: reconcile.TerminalError(fmt.Errorf("NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than: ")), })) }) @@ -480,10 +483,15 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", }) gomega.Expect(err).ToNot(gomega.HaveOccurred()) + enforcedBackupSpec := &velerov1.BackupSpec{} + if scenario.enforcedBackupSpec != nil { + enforcedBackupSpec = scenario.enforcedBackupSpec + } err = (&NonAdminBackupReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - OADPNamespace: oadpNamespace, + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + OADPNamespace: oadpNamespace, + EnforcedBackupSpec: enforcedBackupSpec, }).SetupWithManager(k8sManager) gomega.Expect(err).ToNot(gomega.HaveOccurred()) @@ -537,8 +545,6 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", scenario.spec, )).To(gomega.BeTrue()) - ginkgo.By("Simulating VeleroBackup update to finished state") - veleroBackup := &velerov1.Backup{} gomega.Expect(k8sClient.Get( ctxTimeout, @@ -549,6 +555,15 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", veleroBackup, )).To(gomega.Succeed()) + if scenario.enforcedBackupSpec != nil { + ginkgo.By("Validating Velero Backup Spec") + expectedSpec := scenario.enforcedBackupSpec.DeepCopy() + expectedSpec.IncludedNamespaces = []string{nonAdminObjectNamespace} + gomega.Expect(reflect.DeepEqual(veleroBackup.Spec, *expectedSpec)).To(gomega.BeTrue()) + } + + ginkgo.By("Simulating VeleroBackup update to finished state") + // TODO can not call .Status().Update() for veleroBackup object: backups.velero.io "name..." not found error veleroBackup.Status = velerov1.BackupStatus{ Phase: velerov1.BackupPhaseCompleted, @@ -608,11 +623,18 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", }, }, }, + enforcedBackupSpec: &velerov1.BackupSpec{ + SnapshotVolumes: ptr.To(false), + TTL: metav1.Duration{ + Duration: 36 * time.Hour, + }, + DefaultVolumesToFsBackup: ptr.To(true), + }, }), ginkgo.Entry("Should update NonAdminBackup until it invalidates and then delete it", nonAdminBackupFullReconcileScenario{ spec: nacv1alpha1.NonAdminBackupSpec{ BackupSpec: &velerov1.BackupSpec{ - IncludedNamespaces: []string{"not-valid"}, + SnapshotVolumes: ptr.To(true), }, }, status: nacv1alpha1.NonAdminBackupStatus{ @@ -622,10 +644,13 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", Type: "Accepted", Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", - Message: "spec.backupSpec.IncludedNamespaces can not contain namespaces other than:", + Message: "spec.backupSpec.snapshotVolumes field value is enforced by admin user", }, }, }, + enforcedBackupSpec: &velerov1.BackupSpec{ + SnapshotVolumes: ptr.To(false), + }, }), ) })