Skip to content

Commit

Permalink
fixup! fix: Enforce NonAdminBackup spec field values
Browse files Browse the repository at this point in the history
apply enforcing to Velero Backup

Signed-off-by: Mateus Oliveira <[email protected]>
  • Loading branch information
mateusoliveira43 committed Nov 11, 2024
1 parent 54de7bd commit 187da0e
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 15 deletions.
2 changes: 1 addition & 1 deletion internal/common/function/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions internal/controller/nonadminbackup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package controller
import (
"context"
"errors"
"fmt"
"reflect"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -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,
Expand Down
53 changes: 39 additions & 14 deletions internal/controller/nonadminbackup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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: ")),
}))
})

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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{
Expand All @@ -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),
},
}),
)
})

0 comments on commit 187da0e

Please sign in to comment.