EVEREST-1228 don't allow duplicated backup storages (#486)

* EVEREST-1228 don't allow duplicated backup storages --------- Co-authored-by: Mayank Shah <[email protected]>
percona · Jul 9, 2024 · 01db87a · 01db87a
1 parent fd0b9e9
commit 01db87a
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 7 deletions.
diff --git a/api-tests/tests/database-cluster-backups.spec.ts b/api-tests/tests/database-cluster-backups.spec.ts
@@ -14,7 +14,7 @@
 // limitations under the License.
 import { expect, test } from '@playwright/test'
 import * as th from './helpers'
-import {checkError, testsNs, checkObjectDeletion} from "./helpers";
+import {checkError, testsNs, checkObjectDeletion, waitClusterDeletion} from "./helpers";
 
 test('create/delete database cluster backups', async ({ request, page }) => {
   const bsName = th.suffixedName('storage')
@@ -50,6 +50,7 @@ test('create/delete database cluster backups', async ({ request, page }) => {
 
   await th.deleteBackup(request, backupName)
   await th.deleteDBCluster(request, page, clName)
+  await waitClusterDeletion(request, page, clName)
   await th.deleteBackupStorage(request, bsName)
 })
 
@@ -164,10 +165,12 @@ test('list backups', async ({ request, page }) => {
   for (const payload of payloads) {
     await request.delete(`/v1/namespaces/${testsNs}/database-cluster-backups/${payload.metadata.name}`)
     response = await request.get(`/v1/namespaces/${testsNs}/database-cluster-backups/${payload.metadata.name}`)
-    checkObjectDeletion(response)
+    await checkObjectDeletion(response)
   }
 
   await th.deleteDBCluster(request, page, clusterName1)
   await th.deleteDBCluster(request, page, clusterName2)
+  await waitClusterDeletion(request, page, clusterName1)
+  await waitClusterDeletion(request, page, clusterName2)
   await th.deleteBackupStorage(request, bsName)
 })
diff --git a/api-tests/tests/database-cluster-restores.spec.ts b/api-tests/tests/database-cluster-restores.spec.ts
@@ -14,7 +14,7 @@
 // limitations under the License.
 import { expect, test } from '@playwright/test'
 import * as th from './helpers'
-import {checkError, testsNs} from "./helpers";
+import {checkError, testsNs, waitClusterDeletion} from "./helpers";
 
 
 test('create/update/delete database cluster restore', async ({ request, page }) => {
@@ -80,6 +80,8 @@ test('create/update/delete database cluster restore', async ({ request, page })
   await th.deleteBackup(request, backupName)
   await th.deleteDBCluster(request, page, clName)
   await th.deleteDBCluster(request, page, clName2)
+  await waitClusterDeletion(request, page, clName)
+  await waitClusterDeletion(request, page, clName2)
   await th.deleteBackupStorage(request, bsName)
 })
 
@@ -157,6 +159,8 @@ test('list restores', async ({ request, page }) => {
   await th.deleteBackup(request, backupName)
   await th.deleteDBCluster(request, page, clName1)
   await th.deleteDBCluster(request, page, clName2)
+  await waitClusterDeletion(request, page, clName1)
+  await waitClusterDeletion(request, page, clName2)
   await th.deleteBackupStorage(request, bsName)
 })
 
@@ -209,5 +213,6 @@ test('create restore: validation errors', async ({ request, page }) => {
 
   await th.deleteBackup(request, backupName)
   await th.deleteDBCluster(request, page, clName)
+  await waitClusterDeletion(request, page, clName)
   await th.deleteBackupStorage(request, bsName)
 })
diff --git a/api-tests/tests/helpers.ts b/api-tests/tests/helpers.ts
@@ -74,7 +74,7 @@ export const createBackupStorage = async (request, name) => {
     name,
     url: 'http://custom-url',
     description: 'Dev storage',
-    bucketName: 'percona-test-backup-storage',
+    bucketName: suffixedName('percona-test-backup-storage'),
     region: 'us-east-2',
     accessKey: 'sdfs',
     secretKey: 'sdfsdfsd',

diff --git a/api/backup_storage.go b/api/backup_storage.go
@@ -67,12 +67,18 @@ func (e *EverestServer) CreateBackupStorage(ctx echo.Context) error { //nolint:f
 			Message: pointer.ToString("Failed getting watched namespaces"),
 		})
 	}
+	c := ctx.Request().Context()
+	existingStorages, err := e.kubeClient.ListBackupStorages(c, e.kubeClient.Namespace())
+	if err != nil {
+		return ctx.JSON(http.StatusInternalServerError, Error{
+			Message: pointer.ToString("Failed getting existing backup storages"),
+		})
+	}
 
-	params, err := validateCreateBackupStorageRequest(ctx, namespaces, e.l)
+	params, err := validateCreateBackupStorageRequest(ctx, namespaces, e.l, existingStorages)
 	if err != nil {
 		return ctx.JSON(http.StatusBadRequest, Error{Message: pointer.ToString(err.Error())})
 	}
-	c := ctx.Request().Context()
 	s, err := e.kubeClient.GetBackupStorage(c, e.kubeClient.Namespace(), params.Name)
 	if err != nil && !k8serrors.IsNotFound(err) {
 		e.l.Error(err)

diff --git a/api/validation.go b/api/validation.go
@@ -102,6 +102,7 @@ var (
 	errDuplicatedStoragePG           = errors.New("postgres clusters can't use the same storage for the different schedules")
 	errStorageDeletionPG             = errors.New("the existing postgres schedules can't be deleted")
 	errStorageChangePG               = errors.New("the existing postgres schedules can't change their storage")
+	errDuplicatedBackupStorage       = errors.New("backup storages with the same url, bucket and url are not allowed")
 
 	//nolint:gochecknoglobals
 	operatorEngine = map[everestv1alpha1.EngineType]string{
@@ -415,12 +416,25 @@ func (e *EverestServer) validateUpdateBackupStorageRequest(
 	return &params, nil
 }
 
-func validateCreateBackupStorageRequest(ctx echo.Context, namespaces []string, l *zap.SugaredLogger) (*CreateBackupStorageParams, error) {
+func validateCreateBackupStorageRequest(
+	ctx echo.Context,
+	namespaces []string,
+	l *zap.SugaredLogger,
+	existingStorages *everestv1alpha1.BackupStorageList,
+) (*CreateBackupStorageParams, error) {
 	var params CreateBackupStorageParams
 	if err := ctx.Bind(&params); err != nil {
 		return nil, err
 	}
 
+	for _, storage := range existingStorages.Items {
+		if storage.Spec.Region == params.Region &&
+			storage.Spec.EndpointURL == pointer.GetString(params.Url) &&
+			storage.Spec.Bucket == params.BucketName {
+			return nil, errDuplicatedBackupStorage
+		}
+	}
+
 	if err := validateRFC1035(params.Name, "name"); err != nil {
 		return nil, err
 	}