fix: Enforce NonAdminBackup spec field values

api/v1alpha1/oadp_types.go

@@ -439,6 +439,15 @@ type NonAdmin struct {
 	// Enables non admin feature, by default is disabled
 	// +optional
 	Enable *bool `json:"enable,omitempty"`
+
+	// which bakup spec field values to enforce
+	// +optional
+	EnforceBackupSpec *velero.BackupSpec `json:"enforceBackupSpec,omitempty"`
+
+	// which restore spec field values to enforce
+	// +optional
+	// EnforceRestoreSpecs *velero.RestoreSpec `json:"enforceRestoreSpecs,omitempty"`
+	// TODO not allow enforcing backupName!!!!
 }
 
 // DataMover defines the various config for DPA data mover

bundle/manifests/oadp-operator.clusterserviceversion.yaml

@@ -603,6 +603,12 @@ spec:
     spec:
       clusterPermissions:
       - rules:
+        - apiGroups:
+          - oadp.openshift.io
+          resources:
+          - dataprotectionapplications
+          verbs:
+          - list
         - apiGroups:
           - oadp.openshift.io
           resources:

config/non-admin-controller_rbac/role.yaml

@@ -4,6 +4,12 @@ kind: ClusterRole
 metadata:
   name: non-admin-controller-role
 rules:
+- apiGroups:
+  - oadp.openshift.io
+  resources:
+  - dataprotectionapplications
+  verbs:
+  - list
 - apiGroups:
   - oadp.openshift.io
   resources:

controllers/nonadmin_controller.go

@@ -3,8 +3,10 @@ package controllers
 import (
 	"fmt"
 	"os"
+	"reflect"
 
 	"github.com/go-logr/logr"
+	velero "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"golang.org/x/exp/maps"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -22,6 +24,8 @@ import (
 const (
 	nonAdminObjectName = "non-admin-controller"
 	controlPlaneKey    = "control-plane"
+
+	enforcedBackupSpecKey = "enforced-backup-spec"
 )
 
 var (
@@ -36,6 +40,9 @@ var (
 		"app.kubernetes.io/name":       "deployment",
 		"app.kubernetes.io/part-of":    common.OADPOperator,
 	}
+
+	previousEnforcedBackupSpec   *velero.BackupSpec = nil
+	dpaBackupSpecResourceVersion                    = ""
 )
 
 func (r *DPAReconciler) ReconcileNonAdminController(log logr.Logger) (bool, error) {
@@ -116,14 +123,13 @@ func (r *DPAReconciler) ReconcileNonAdminController(log logr.Logger) (bool, erro
 }
 
 func (r *DPAReconciler) buildNonAdminDeployment(deploymentObject *appsv1.Deployment) error {
-	dpa := r.dpa
 	nonAdminImage := r.getNonAdminImage()
-	imagePullPolicy, err := common.GetImagePullPolicy(dpa.Spec.ImagePullPolicy, nonAdminImage)
+	imagePullPolicy, err := common.GetImagePullPolicy(r.dpa.Spec.ImagePullPolicy, nonAdminImage)
 	if err != nil {
 		r.Log.Error(err, "imagePullPolicy regex failed")
 	}
 	ensureRequiredLabels(deploymentObject)
-	err = ensureRequiredSpecs(deploymentObject, nonAdminImage, imagePullPolicy)
+	err = ensureRequiredSpecs(deploymentObject, r.dpa, nonAdminImage, imagePullPolicy)
 	if err != nil {
 		return err
 	}
@@ -143,23 +149,42 @@ func ensureRequiredLabels(deploymentObject *appsv1.Deployment) {
 	}
 }
 
-func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, image string, imagePullPolicy corev1.PullPolicy) error {
+func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, dpa *oadpv1alpha1.DataProtectionApplication, image string, imagePullPolicy corev1.PullPolicy) error {
 	namespaceEnvVar := corev1.EnvVar{
 		Name:  "WATCH_NAMESPACE",
 		Value: deploymentObject.Namespace,
 	}
+	if len(dpaBackupSpecResourceVersion) == 0 || !reflect.DeepEqual(dpa.Spec.NonAdmin.EnforceBackupSpec, previousEnforcedBackupSpec) {
+		dpaBackupSpecResourceVersion = dpa.GetResourceVersion()
+	}
+	previousEnforcedBackupSpec = dpa.Spec.NonAdmin.EnforceBackupSpec
+	// TODO same thing for restore
+	enforcedSpecAnnotation := map[string]string{
+		enforcedBackupSpecKey: dpaBackupSpecResourceVersion,
+	}
 
 	deploymentObject.Spec.Replicas = ptr.To(int32(1))
 	deploymentObject.Spec.Selector = &metav1.LabelSelector{
 		MatchLabels: controlPlaneLabel,
 	}
+
 	templateObjectLabels := deploymentObject.Spec.Template.GetLabels()
 	if templateObjectLabels == nil {
 		deploymentObject.Spec.Template.SetLabels(controlPlaneLabel)
 	} else {
 		templateObjectLabels[controlPlaneKey] = controlPlaneLabel[controlPlaneKey]
 		deploymentObject.Spec.Template.SetLabels(templateObjectLabels)
 	}
+
+	templateObjectAnnotations := deploymentObject.Spec.Template.GetAnnotations()
+	if templateObjectAnnotations == nil {
+		deploymentObject.Spec.Template.SetAnnotations(enforcedSpecAnnotation)
+	} else {
+		templateObjectAnnotations[enforcedBackupSpecKey] = enforcedSpecAnnotation[enforcedBackupSpecKey]
+		// TODO same thing for restore
+		deploymentObject.Spec.Template.SetAnnotations(templateObjectAnnotations)
+	}
+
 	nonAdminContainerFound := false
 	if len(deploymentObject.Spec.Template.Spec.Containers) == 0 {
 		deploymentObject.Spec.Template.Spec.Containers = []corev1.Container{{

controllers/nonadmin_controller_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
+	"github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -241,7 +242,13 @@ var _ = ginkgo.Describe("Test ReconcileNonAdminController function", func() {
 })
 
 func TestDPAReconcilerBuildNonAdminDeployment(t *testing.T) {
-	r := &DPAReconciler{dpa: &oadpv1alpha1.DataProtectionApplication{}}
+	r := &DPAReconciler{dpa: &oadpv1alpha1.DataProtectionApplication{
+		Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+			NonAdmin: &oadpv1alpha1.NonAdmin{
+				Enable: ptr.To(true),
+			},
+		},
+	}}
 	t.Setenv("RELATED_IMAGE_NON_ADMIN_CONTROLLER", defaultNonAdminImage)
 	deployment := createTestDeployment("test-build-deployment")
 	err := r.buildNonAdminDeployment(deployment)
@@ -283,7 +290,17 @@ func TestEnsureRequiredLabels(t *testing.T) {
 
 func TestEnsureRequiredSpecs(t *testing.T) {
 	deployment := createTestDeployment("test-ensure-spec")
-	err := ensureRequiredSpecs(deployment, defaultNonAdminImage, corev1.PullAlways)
+	dpa := &oadpv1alpha1.DataProtectionApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "123456789",
+		},
+		Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+			NonAdmin: &oadpv1alpha1.NonAdmin{
+				Enable: ptr.To(true),
+			},
+		},
+	}
+	err := ensureRequiredSpecs(deployment, dpa, defaultNonAdminImage, corev1.PullAlways)
 	if err != nil {
 		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
 	}
@@ -296,6 +313,68 @@ func TestEnsureRequiredSpecs(t *testing.T) {
 	if deployment.Spec.Template.Spec.Containers[0].Image != defaultNonAdminImage {
 		t.Errorf("Deployment has wrong Image: %v", deployment.Spec.Template.Spec.Containers[0].Image)
 	}
+	if len(deployment.Spec.Template.Annotations[enforcedBackupSpecKey]) == 0 {
+		t.Errorf("Deployment does not have Annotation")
+	}
+	previousEnforcedBackupSpec := deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	updatedDPA := &oadpv1alpha1.DataProtectionApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "147258369",
+		},
+		Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+			NonAdmin: &oadpv1alpha1.NonAdmin{
+				Enable: ptr.To(true),
+			},
+		},
+	}
+	err = ensureRequiredSpecs(deployment, updatedDPA, defaultNonAdminImage, corev1.PullAlways)
+	if err != nil {
+		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
+	}
+	if previousEnforcedBackupSpec != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+		t.Errorf("Deployment have different Annotation")
+	}
+	updatedDPA = &oadpv1alpha1.DataProtectionApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "987654321",
+		},
+		Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+			NonAdmin: &oadpv1alpha1.NonAdmin{
+				Enable: ptr.To(true),
+				EnforceBackupSpec: &v1.BackupSpec{
+					SnapshotMoveData: ptr.To(false),
+				},
+			},
+		},
+	}
+	err = ensureRequiredSpecs(deployment, updatedDPA, defaultNonAdminImage, corev1.PullAlways)
+	if err != nil {
+		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
+	}
+	if previousEnforcedBackupSpec == deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+		t.Errorf("Deployment does not have different Annotation")
+	}
+	previousEnforcedBackupSpec = deployment.DeepCopy().Spec.Template.Annotations[enforcedBackupSpecKey]
+	updatedDPA = &oadpv1alpha1.DataProtectionApplication{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "112233445",
+		},
+		Spec: oadpv1alpha1.DataProtectionApplicationSpec{
+			NonAdmin: &oadpv1alpha1.NonAdmin{
+				Enable: ptr.To(true),
+				EnforceBackupSpec: &v1.BackupSpec{
+					SnapshotMoveData: ptr.To(false),
+				},
+			},
+		},
+	}
+	err = ensureRequiredSpecs(deployment, updatedDPA, defaultNonAdminImage, corev1.PullAlways)
+	if err != nil {
+		t.Errorf("ensureRequiredSpecs() errored out: %v", err)
+	}
+	if previousEnforcedBackupSpec != deployment.Spec.Template.Annotations[enforcedBackupSpecKey] {
+		t.Errorf("Deployment have different Annotation")
+	}
 }
 
 func TestDPAReconcilerCheckNonAdminEnabled(t *testing.T) {

controllers/validator.go

@@ -3,6 +3,7 @@ package controllers
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"strings"
 
 	mapset "github.com/deckarep/golang-set/v2"
@@ -75,7 +76,12 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error)
 	// validate non-admin enable and tech-preview-ack
 	if r.checkNonAdminEnabled() {
 		if !(dpa.Spec.UnsupportedOverrides[oadpv1alpha1.TechPreviewAck] == TrueVal) {
-			return false, errors.New("in order to enable/disable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: 'true'")
+			return false, errors.New("in order to enable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: 'true'")
+		}
+		if r.dpa.Spec.NonAdmin.EnforceBackupSpec != nil {
+			if !reflect.ValueOf(r.dpa.Spec.NonAdmin.EnforceBackupSpec.IncludedNamespaces).IsZero() {
+				return false, errors.New("admin users can not set DPA spec.nonAdmin.enforceBackupSpecs.includedNamespaces field")
+			}
 		}
 	}
 

controllers/validator_test.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
@@ -1394,7 +1395,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
 			},
 			objects:    []client.Object{},
 			wantErr:    true,
-			messageErr: "in order to enable/disable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: 'true'",
+			messageErr: "in order to enable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: 'true'",
 		},
 		{
 			name: "given invalid DPA CR aws and legacy-aws plugins both specified",
@@ -1420,6 +1421,58 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
 			wantErr:    true,
 			messageErr: "aws and legacy-aws can not be both specified in DPA spec.configuration.velero.defaultPlugins",
 		},
+		{
+			name: "[invalid] DPA CR: spec.nonAdmin.enforceBackupSpec.includedNamespaces 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),
+						EnforceBackupSpec: &v1.BackupSpec{
+							IncludedNamespaces: []string{"banana"},
+						},
+					},
+					UnsupportedOverrides: map[oadpv1alpha1.UnsupportedImageKey]string{
+						oadpv1alpha1.TechPreviewAck: "true",
+					},
+				},
+			},
+			wantErr:    true,
+			messageErr: "admin users can not set DPA spec.nonAdmin.enforceBackupSpecs.includedNamespaces field",
+		},
+		{
+			name: "[valid] DPA CR: spec.nonAdmin.enable true",
+			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),
+					},
+					UnsupportedOverrides: map[oadpv1alpha1.UnsupportedImageKey]string{
+						oadpv1alpha1.TechPreviewAck: "true",
+					},
+				},
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		tt.objects = append(tt.objects, tt.dpa)