fix: Enforce NonAdminRestore spec field values (#1600)

Signed-off-by: Mateus Oliveira <[email protected]>

Author: Mateus Oliveira

api/v1alpha1/oadp_types.go

@@ -446,8 +446,7 @@ type NonAdmin struct {
 
 	// which restore spec field values to enforce
 	// +optional
-	// EnforceRestoreSpecs *velero.RestoreSpec `json:"enforceRestoreSpecs,omitempty"`
-	// TODO not allow enforcing backupName!!!!
+	EnforceRestoreSpec *velero.RestoreSpec `json:"enforceRestoreSpec,omitempty"`
 }
 
 // DataMover defines the various config for DPA data mover

api/v1alpha1/zz_generated.deepcopy.go

@@ -25,7 +25,8 @@ const (
 	nonAdminObjectName = "non-admin-controller"
 	controlPlaneKey    = "control-plane"
 
-	enforcedBackupSpecKey = "enforced-backup-spec"
+	enforcedBackupSpecKey  = "enforced-backup-spec"
+	enforcedRestoreSpecKey = "enforced-restore-spec"
 )
 
 var (
@@ -41,8 +42,10 @@ var (
 		"app.kubernetes.io/part-of":    common.OADPOperator,
 	}
 
-	previousEnforcedBackupSpec   *velero.BackupSpec = nil
-	dpaBackupSpecResourceVersion                    = ""
+	previousEnforcedBackupSpec    *velero.BackupSpec  = nil
+	dpaBackupSpecResourceVersion                      = ""
+	previousEnforcedRestoreSpec   *velero.RestoreSpec = nil
+	dpaRestoreSpecResourceVersion                     = ""
 )
 
 func (r *DPAReconciler) ReconcileNonAdminController(log logr.Logger) (bool, error) {
@@ -156,9 +159,13 @@ func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, dpa *oadpv1alpha1.
 		dpaBackupSpecResourceVersion = dpa.GetResourceVersion()
 	}
 	previousEnforcedBackupSpec = dpa.Spec.NonAdmin.EnforceBackupSpec
-	// TODO same thing for restore
+	if len(dpaRestoreSpecResourceVersion) == 0 || !reflect.DeepEqual(dpa.Spec.NonAdmin.EnforceRestoreSpec, previousEnforcedRestoreSpec) {
+		dpaRestoreSpecResourceVersion = dpa.GetResourceVersion()
+	}
+	previousEnforcedRestoreSpec = dpa.Spec.NonAdmin.EnforceRestoreSpec
 	enforcedSpecAnnotation := map[string]string{
-		enforcedBackupSpecKey: dpaBackupSpecResourceVersion,
+		enforcedBackupSpecKey:  dpaBackupSpecResourceVersion,
+		enforcedRestoreSpecKey: dpaRestoreSpecResourceVersion,
 	}
 
 	deploymentObject.Spec.Replicas = ptr.To(int32(1))
@@ -179,7 +186,7 @@ func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, dpa *oadpv1alpha1.
 		deploymentObject.Spec.Template.SetAnnotations(enforcedSpecAnnotation)
 	} else {
 		templateObjectAnnotations[enforcedBackupSpecKey] = enforcedSpecAnnotation[enforcedBackupSpecKey]
-		// TODO same thing for restore
+		templateObjectAnnotations[enforcedRestoreSpecKey] = enforcedSpecAnnotation[enforcedRestoreSpecKey]
 		deploymentObject.Spec.Template.SetAnnotations(templateObjectAnnotations)
 	}
 

bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml

@@ -316,7 +316,11 @@ func TestEnsureRequiredSpecs(t *testing.T) {
 	if len(deployment.Spec.Template.Annotations[enforcedBackupSpecKey]) == 0 {
 		t.Errorf("Deployment does not have Annotation")
 	}
-	previousAnnotationValue := deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	if len(deployment.Spec.Template.Annotations[enforcedRestoreSpecKey]) == 0 {
+		t.Errorf("Deployment does not have Annotation")
+	}
+	previousBackupAnnotationValue := deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	previousRestoreAnnotationValue := deployment.DeepCopy().Spec.Template.Annotations[enforcedRestoreSpecKey]
 	updatedDPA := &oadpv1alpha1.DataProtectionApplication{
 		ObjectMeta: metav1.ObjectMeta{
 			ResourceVersion: "147258369",
@@ -331,7 +335,10 @@ func TestEnsureRequiredSpecs(t *testing.T) {
 	if err != nil {
 		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
 	}
-	if previousAnnotationValue != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+	if previousBackupAnnotationValue != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+		t.Errorf("Deployment have different Annotation")
+	}
+	if previousRestoreAnnotationValue != deployment.Spec.Template.Annotations[enforcedRestoreSpecKey] {
 		t.Errorf("Deployment have different Annotation")
 	}
 	updatedDPA = &oadpv1alpha1.DataProtectionApplication{
@@ -351,10 +358,14 @@ func TestEnsureRequiredSpecs(t *testing.T) {
 	if err != nil {
 		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
 	}
-	if previousAnnotationValue == deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+	if previousBackupAnnotationValue == deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
 		t.Errorf("Deployment does not have different Annotation")
 	}
-	previousAnnotationValue = deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	if previousRestoreAnnotationValue != deployment.Spec.Template.Annotations[enforcedRestoreSpecKey] {
+		t.Errorf("Deployment have different Annotation")
+	}
+	previousBackupAnnotationValue = deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	previousRestoreAnnotationValue = deployment.DeepCopy().Spec.Template.Annotations[enforcedRestoreSpecKey]
 	updatedDPA = &oadpv1alpha1.DataProtectionApplication{
 		ObjectMeta: metav1.ObjectMeta{
 			ResourceVersion: "112233445",
@@ -365,16 +376,22 @@ func TestEnsureRequiredSpecs(t *testing.T) {
 				EnforceBackupSpec: &v1.BackupSpec{
 					SnapshotMoveData: ptr.To(false),
 				},
+				EnforceRestoreSpec: &v1.RestoreSpec{
+					RestorePVs: ptr.To(true),
+				},
 			},
 		},
 	}
 	err = ensureRequiredSpecs(deployment, updatedDPA, defaultNonAdminImage, corev1.PullAlways)
 	if err != nil {
 		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
 	}
-	if previousAnnotationValue != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+	if previousBackupAnnotationValue != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
 		t.Errorf("Deployment have different Annotation")
 	}
+	if previousRestoreAnnotationValue == deployment.Spec.Template.Annotations[enforcedRestoreSpecKey] {
+		t.Errorf("Deployment does not have different Annotation")
+	}
 }
 
 func TestDPAReconcilerCheckNonAdminEnabled(t *testing.T) {

config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml

@@ -1545,6 +1545,33 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "[valid] DPA CR: spec.nonAdmin.enforceRestoreSpec set",
+			dpa: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-DPA-CR",
+					Namespace: "test-ns",
+				},
+				Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+					Configuration: &oadpv1alpha1.ApplicationConfig{
+						Velero: &oadpv1alpha1.VeleroConfig{
+							NoDefaultBackupLocation: true,
+						},
+					},
+					BackupImages: ptr.To(false),
+					NonAdmin: &oadpv1alpha1.NonAdmin{
+						Enable: ptr.To(true),
+						EnforceRestoreSpec: &v1.RestoreSpec{
+							IncludedNamespaces: []string{"banana"},
+							RestorePVs:         ptr.To(true),
+						},
+					},
+					UnsupportedOverrides: map[oadpv1alpha1.UnsupportedImageKey]string{
+						oadpv1alpha1.TechPreviewAck: "true",
+					},
+				},
+			},
+		},
 		{
 			name: "[valid] DPA CR: spec.nonAdmin.enable true",
 			dpa: &oadpv1alpha1.DataProtectionApplication{