Validate namespaces, so non admin can not create backups outside of ns (

#56)

Signed-off-by: Michal Pryc <[email protected]>

internal/common/function/function.go

@@ -77,21 +77,39 @@ func AddNonAdminBackupAnnotations(ownerNamespace string, ownerName string, owner
 	return mergedAnnotations
 }
 
+// containsOnlyNamespace checks if the given namespaces slice contains only the specified namespace
+func containsOnlyNamespace(namespaces []string, namespace string) bool {
+	for _, ns := range namespaces {
+		if ns != namespace {
+			return false
+		}
+	}
+	return true
+}
+
 // GetBackupSpecFromNonAdminBackup return BackupSpec object from NonAdminBackup spec, if no error occurs
 func GetBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) {
 	if nonAdminBackup == nil {
 		return nil, fmt.Errorf("nonAdminBackup is nil")
 	}
 
-	// TODO check spec?
-
 	if nonAdminBackup.Spec.BackupSpec == nil {
-		return nil, fmt.Errorf("BackupSpec is nil")
+		return nil, fmt.Errorf("BackupSpec is not defined")
 	}
 
+	veleroBackupSpec := nonAdminBackup.Spec.BackupSpec.DeepCopy()
+
 	// TODO: Additional validations, before continuing
 
-	return nonAdminBackup.Spec.BackupSpec.DeepCopy(), nil
+	if veleroBackupSpec.IncludedNamespaces == nil {
+		veleroBackupSpec.IncludedNamespaces = []string{nonAdminBackup.Namespace}
+	} else {
+		if !containsOnlyNamespace(veleroBackupSpec.IncludedNamespaces, nonAdminBackup.Namespace) {
+			return nil, fmt.Errorf("spec.backupSpec.IncludedNamespaces can not contain namespaces other then: %s", nonAdminBackup.Namespace)
+		}
+	}
+
+	return veleroBackupSpec, nil
 }
 
 // GenerateVeleroBackupName generates a Velero backup name based on the provided namespace and NonAdminBackup name.
@@ -185,7 +203,7 @@ func UpdateNonAdminBackupCondition(ctx context.Context, r client.Client, logger
 
 	// Check if the condition is already set to the desired status
 	currentCondition := apimeta.FindStatusCondition(nab.Status.Conditions, string(condition))
-	if currentCondition != nil && currentCondition.Status == conditionStatus {
+	if currentCondition != nil && currentCondition.Status == conditionStatus && currentCondition.Reason == reason && currentCondition.Message == message {
 		// Condition is already set to the desired status, no need to update
 		logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition is already set to: %s", condition))
 		return false, nil

internal/common/function/function_test.go

@@ -177,17 +177,19 @@ func TestGetBackupSpecFromNonAdminBackup(t *testing.T) {
 	backupSpec, err = GetBackupSpecFromNonAdminBackup(nonAdminBackup)
 	assert.Error(t, err)
 	assert.Nil(t, backupSpec)
-	assert.Equal(t, "BackupSpec is nil", err.Error())
+	assert.Equal(t, "BackupSpec is not defined", err.Error())
 
 	// Test case: NonAdminBackup with valid BackupSpec
 	backupSpecInput := &velerov1api.BackupSpec{
-		IncludedNamespaces:      []string{"namespace1", "namespace2"},
-		ExcludedNamespaces:      []string{"namespace3"},
+		IncludedNamespaces:      []string{"namespace1"},
 		StorageLocation:         "s3://bucket-name/path/to/backup",
 		VolumeSnapshotLocations: []string{"volume-snapshot-location"},
 	}
 
 	nonAdminBackup = &nacv1alpha1.NonAdminBackup{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "namespace1", // Set the namespace of NonAdminBackup
+		},
 		Spec: nacv1alpha1.NonAdminBackupSpec{
 			BackupSpec: backupSpecInput,
 		},
@@ -196,7 +198,46 @@ func TestGetBackupSpecFromNonAdminBackup(t *testing.T) {
 
 	assert.NoError(t, err)
 	assert.NotNil(t, backupSpec)
-	assert.Equal(t, backupSpecInput, backupSpec)
+	assert.Equal(t, []string{"namespace1"}, backupSpec.IncludedNamespaces) // Check that only the namespace from NonAdminBackup is included
+	assert.Equal(t, backupSpecInput.ExcludedNamespaces, backupSpec.ExcludedNamespaces)
+	assert.Equal(t, backupSpecInput.StorageLocation, backupSpec.StorageLocation)
+	assert.Equal(t, backupSpecInput.VolumeSnapshotLocations, backupSpec.VolumeSnapshotLocations)
+
+	backupSpecInput = &velerov1api.BackupSpec{
+		IncludedNamespaces: []string{"namespace2", "namespace3"},
+	}
+
+	nonAdminBackup = &nacv1alpha1.NonAdminBackup{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "namespace2", // Set the namespace of NonAdminBackup
+		},
+		Spec: nacv1alpha1.NonAdminBackupSpec{
+			BackupSpec: backupSpecInput,
+		},
+	}
+	backupSpec, err = GetBackupSpecFromNonAdminBackup(nonAdminBackup)
+
+	assert.Error(t, err)
+	assert.Nil(t, backupSpec)
+	assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other then: namespace2", err.Error())
+
+	backupSpecInput = &velerov1api.BackupSpec{
+		IncludedNamespaces: []string{"namespace3"},
+	}
+
+	nonAdminBackup = &nacv1alpha1.NonAdminBackup{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "namespace4", // Set the namespace of NonAdminBackup
+		},
+		Spec: nacv1alpha1.NonAdminBackupSpec{
+			BackupSpec: backupSpecInput,
+		},
+	}
+	backupSpec, err = GetBackupSpecFromNonAdminBackup(nonAdminBackup)
+
+	assert.Error(t, err)
+	assert.Nil(t, backupSpec)
+	assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other then: namespace4", err.Error())
 }
 
 func TestGenerateVeleroBackupName(t *testing.T) {

internal/controller/nonadminbackup_controller.go

@@ -164,7 +164,11 @@ func (r *NonAdminBackupReconciler) ValidateVeleroBackupSpec(ctx context.Context,
 	_, err := function.GetBackupSpecFromNonAdminBackup(nab)
 
 	if err != nil {
+		// Use errMsg if errMsgFromErr is not available, otherwise use errMsgFromErr
 		errMsg := "NonAdminBackup CR does not contain valid BackupSpec"
+		if errMsgFromErr := err.Error(); errMsgFromErr != "" {
+			errMsg = errMsgFromErr
+		}
 		logger.Error(err, errMsg)
 
 		updatedStatus, errUpdateStatus := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseBackingOff)
@@ -248,7 +252,7 @@ func (r *NonAdminBackupReconciler) CreateVeleroBackupSpec(ctx context.Context, l
 			},
 			Spec: *backupSpec,
 		}
-	} else if err != nil && !apierrors.IsNotFound(err) {
+	} else if err != nil {
 		logger.Error(err, "Unable to fetch VeleroBackup")
 		return true, false, err
 	} else {