diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 9544dd5..29d5010 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -39,12 +39,7 @@ const ( // NonAdminBackupSpec defines the desired state of NonAdminBackup type NonAdminBackupSpec struct { // BackupSpec defines the specification for a Velero backup. - BackupSpec *velerov1.BackupSpec `json:"backupSpec,omitempty"` - - // NonAdminBackup log level (use debug for the most logging, leave unset for default) - // +optional - // +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic - LogLevel string `json:"logLevel,omitempty"` + BackupSpec *velerov1.BackupSpec `json:"backupSpec"` // DeleteBackup removes the NonAdminBackup and its associated VeleroBackup from the cluster, // as well as the corresponding object storage diff --git a/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml index b76a026..39aab8f 100644 --- a/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml @@ -525,18 +525,8 @@ spec: ForceDeleteBackup removes the NonAdminBackup and its associated VeleroBackup from the cluster, regardless of whether deletion from object storage succeeds or fails type: boolean - logLevel: - description: NonAdminBackup log level (use debug for the most logging, - leave unset for default) - enum: - - trace - - debug - - info - - warning - - error - - fatal - - panic - type: string + required: + - backupSpec type: object status: description: NonAdminBackupStatus defines the observed state of NonAdminBackup diff --git a/internal/common/function/function.go b/internal/common/function/function.go index f4fc425..c26a7a4 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -80,11 +80,6 @@ func containsOnlyNamespace(namespaces []string, namespace string) bool { // ValidateBackupSpec return nil, if NonAdminBackup is valid; error otherwise func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup, enforcedBackupSpec *velerov1.BackupSpec) error { - // this should be Kubernetes API validation - if nonAdminBackup.Spec.BackupSpec == nil { - return fmt.Errorf("BackupSpec is not defined") - } - if nonAdminBackup.Spec.BackupSpec.IncludedNamespaces != nil { if !containsOnlyNamespace(nonAdminBackup.Spec.BackupSpec.IncludedNamespaces, nonAdminBackup.Namespace) { return fmt.Errorf("NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than: %s", nonAdminBackup.Namespace) diff --git a/internal/common/function/function_test.go b/internal/common/function/function_test.go index 004456e..4c42f25 100644 --- a/internal/common/function/function_test.go +++ b/internal/common/function/function_test.go @@ -91,11 +91,6 @@ func TestValidateBackupSpec(t *testing.T) { name string errMessage string }{ - { - name: "nil spec", - spec: nil, - errMessage: "BackupSpec is not defined", - }, { name: "namespace different than NonAdminBackup namespace", spec: &velerov1.BackupSpec{ diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index e9b86ed..6814528 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -41,6 +41,10 @@ import ( "github.com/migtools/oadp-non-admin/internal/common/function" ) +type nonAdminBackupClusterValidationScenario struct { + spec nacv1alpha1.NonAdminBackupSpec +} + type nonAdminBackupSingleReconcileScenario struct { resultError error nonAdminBackupPriorStatus *nacv1alpha1.NonAdminBackupStatus @@ -160,6 +164,50 @@ func deleteTestNamespaces(ctx context.Context, nonAdminNamespaceName string, oad return k8sClient.Delete(ctx, nonAdminNamespace) } +var _ = ginkgo.Describe("Test NonAdminBackup in cluster validation", func() { + var ( + ctx context.Context + nonAdminBackupName string + nonAdminBackupNamespace string + counter int + ) + + ginkgo.BeforeEach(func() { + ctx = context.Background() + counter++ + nonAdminBackupName = fmt.Sprintf("non-admin-backup-object-%v", counter) + nonAdminBackupNamespace = fmt.Sprintf("test-non-admin-backup-cluster-validation-%v", counter) + + nonAdminNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminBackupNamespace, + }, + } + gomega.Expect(k8sClient.Create(ctx, nonAdminNamespace)).To(gomega.Succeed()) + }) + + ginkgo.AfterEach(func() { + nonAdminNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminBackupNamespace, + }, + } + gomega.Expect(k8sClient.Delete(ctx, nonAdminNamespace)).To(gomega.Succeed()) + }) + + ginkgo.DescribeTable("Validation is false", + func(scenario nonAdminBackupClusterValidationScenario) { + nonAdminBackup := buildTestNonAdminBackup(nonAdminBackupNamespace, nonAdminBackupName, scenario.spec) + err := k8sClient.Create(ctx, nonAdminBackup) + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(err.Error()).To(gomega.ContainSubstring("Required value")) + }, + ginkgo.Entry("Should NOT create NonAdminBackup without spec.backupSpec", nonAdminBackupClusterValidationScenario{ + spec: nacv1alpha1.NonAdminBackupSpec{}, + }), + ) +}) + var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile function", func() { var ( ctx = context.Background() @@ -322,7 +370,12 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func // gomega.Expect(err).To(gomega.Not(gomega.HaveOccurred())) // gomega.Expect(currentResourceVersion - priorResourceVersion).To(gomega.Equal(1)) }, - ginkgo.Entry("When triggered by NonAdminBackup Create event without BackupSpec, should update NonAdminBackup phase to BackingOff and exit with terminal error", nonAdminBackupSingleReconcileScenario{ + ginkgo.Entry("When triggered by NonAdminBackup Create event with invalid BackupSpec, should update NonAdminBackup phase to BackingOff and exit with terminal error", nonAdminBackupSingleReconcileScenario{ + nonAdminBackupSpec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{ + IncludedNamespaces: []string{"wrong", "wrong-again"}, + }, + }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ Phase: nacv1alpha1.NonAdminPhaseBackingOff, Conditions: []metav1.Condition{ @@ -330,14 +383,17 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", - Message: "BackupSpec is not defined", + Message: "NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than: ", }, }, }, - resultError: reconcile.TerminalError(fmt.Errorf("BackupSpec is not defined")), + resultError: reconcile.TerminalError(fmt.Errorf("NonAdminBackup spec.backupSpec.includedNamespaces can not contain namespaces other than: ")), }), ginkgo.Entry("When triggered by NonAdminBackup deleteNonAdmin spec field when BackupSpec is invalid, should delete NonAdminBackup without error", nonAdminBackupSingleReconcileScenario{ nonAdminBackupSpec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{ + IncludedNamespaces: []string{"wrong", "wrong-again"}, + }, DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ @@ -358,6 +414,7 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func ginkgo.Entry("When triggered by NonAdminBackup deleteNonAdmin spec field with Finalizer set, should not delete NonAdminBackup as it's waiting for finalizer to be removed", nonAdminBackupSingleReconcileScenario{ addFinalizer: true, nonAdminBackupSpec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{}, DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ @@ -467,6 +524,7 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }), ginkgo.Entry("When triggered by NonAdminBackup deleteNonAdmin spec field with Finalizer unset, should delete NonAdminBackup", nonAdminBackupSingleReconcileScenario{ nonAdminBackupSpec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{}, DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ @@ -495,6 +553,7 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func addFinalizer: true, addNabDeletionTimestamp: false, nonAdminBackupSpec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{}, ForceDeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index aaa3bcf..4b2efbd 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -178,7 +178,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" gomega.Expect(createTestNamespaces(ctx, nonAdminRestoreNamespace, oadpNamespace)).To(gomega.Succeed()) - nonAdminBackup := buildTestNonAdminBackup(nonAdminRestoreNamespace, scenario.spec.RestoreSpec.BackupName, nacv1alpha1.NonAdminBackupSpec{}) + nonAdminBackup := buildTestNonAdminBackup(nonAdminRestoreNamespace, scenario.spec.RestoreSpec.BackupName, nacv1alpha1.NonAdminBackupSpec{BackupSpec: &velerov1.BackupSpec{}}) gomega.Expect(k8sClient.Create(ctx, nonAdminBackup)).To(gomega.Succeed()) nonAdminBackup.Status = scenario.backupStatus gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackup)).To(gomega.Succeed())