[PLAT-12178]Storage config associated with scheduled backups should n…

…ot allowed to be deleted Summary: Throw `BAD_REQUEST` if a backup config associated with any backup schedule is tried to be deleted. Also deprecated the v1 delete API. Test Plan: Manually verified that appropriate error is thrown if a backup config associated with backup schedule is tried to be deleted. Modified existing UTs for new behavior. Reviewers: #yba-api-review!, vbansal Reviewed By: vbansal Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D40506
yugabyte · Dec 13, 2024 · 5648ffb · 5648ffb
1 parent eebcfe3
commit 5648ffb
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 122 deletions.
diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerConfig.java
@@ -23,7 +23,6 @@
 import com.yugabyte.yw.common.config.RuntimeConfGetter;
 import com.yugabyte.yw.forms.UniverseTaskParams;
 import com.yugabyte.yw.models.Backup;
-import com.yugabyte.yw.models.Schedule;
 import com.yugabyte.yw.models.Universe;
 import com.yugabyte.yw.models.configs.CustomerConfig;
 import java.util.List;
@@ -79,10 +78,6 @@ public void run() {
       if (!runtimeConfGetter.getGlobalConf(GlobalConfKeys.enforceCertVerificationBackupRestore)) {
         System.setProperty(SDKGlobalConfiguration.DISABLE_CERT_CHECKING_SYSTEM_PROPERTY, "true");
       }
-      List<Schedule> scheduleList = Schedule.findAllScheduleWithCustomerConfig(params().configUUID);
-      for (Schedule schedule : scheduleList) {
-        schedule.stopSchedule();
-      }
       CustomerConfig customerConfig =
           CustomerConfig.get(params().customerUUID, params().configUUID);
       List<Backup> backupList =

diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerStorageConfig.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerStorageConfig.java
@@ -9,7 +9,6 @@
 import com.yugabyte.yw.forms.AbstractTaskParams;
 import com.yugabyte.yw.models.Backup;
 import com.yugabyte.yw.models.Backup.BackupState;
-import com.yugabyte.yw.models.Schedule;
 import com.yugabyte.yw.models.configs.CustomerConfig;
 import com.yugabyte.yw.models.configs.CustomerConfig.ConfigState;
 import com.yugabyte.yw.models.helpers.CustomerConfigValidator;
@@ -45,9 +44,6 @@ public void run() {
       CustomerConfig customerConfig =
           customerConfigService.getOrBadRequest(params().customerUUID, params().configUUID);
 
-      Schedule.findAllScheduleWithCustomerConfig(params().configUUID)
-          .forEach(Schedule::stopSchedule);
-
       if (params().isDeleteBackups) {
         List<Backup> backupList =
             Backup.findAllNonProgressBackupsWithCustomerConfig(

diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/CustomerConfigController.java b/managed/src/main/java/com/yugabyte/yw/controllers/CustomerConfigController.java
@@ -20,6 +20,9 @@
 import com.yugabyte.yw.models.Backup;
 import com.yugabyte.yw.models.Customer;
 import com.yugabyte.yw.models.CustomerTask;
+import com.yugabyte.yw.models.Schedule;
+import com.yugabyte.yw.models.common.YbaApi;
+import com.yugabyte.yw.models.common.YbaApi.YbaApiVisibility;
 import com.yugabyte.yw.models.configs.CustomerConfig;
 import com.yugabyte.yw.models.configs.CustomerConfig.ConfigState;
 import com.yugabyte.yw.models.configs.data.CustomerConfigData;
@@ -35,8 +38,10 @@
 import io.swagger.annotations.ApiImplicitParams;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.Authorization;
+import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
+import java.util.stream.Collectors;
 import javax.inject.Inject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -94,8 +99,13 @@ public Result create(UUID customerUUID, Http.Request request) {
     return PlatformResults.withData(this.customerConfigService.getConfigMasked(customerConfig));
   }
 
+  @Deprecated
+  @YbaApi(visibility = YbaApiVisibility.DEPRECATED, sinceYBAVersion = "2.25.0.0")
   @ApiOperation(
       value = "Delete a customer configuration",
+      notes =
+          "<b style=\"color:#ff0000\">Deprecated since YBA version 2.25.0.0.</b></p>"
+              + "Use 'Delete a customer configuration V2' instead.",
       response = YBPTask.class,
       nickname = "deleteCustomerConfig")
   @AuthzPath({
@@ -118,11 +128,8 @@ public Result delete(
                 + " is in progress.");
       }
 
-      // If storageConfig is used by DR, do not allow deletion.
-      if (customerConfig.isStorageConfigUsedForDr()) {
-        throw new PlatformServiceException(
-            BAD_REQUEST, "DR Config is associated with Configuration " + configUUID.toString());
-      }
+      preBackupConfigDeleteValidation(customerConfig);
+
       if (isDeleteBackups) {
         DeleteCustomerConfig.Params taskParams = new DeleteCustomerConfig.Params();
         taskParams.customerUUID = customerUUID;
@@ -184,11 +191,7 @@ public Result deleteYb(
                 + " is in progress.");
       }
 
-      // If storageConfig is used by DR, do not allow deletion.
-      if (customerConfig.isStorageConfigUsedForDr()) {
-        throw new PlatformServiceException(
-            BAD_REQUEST, "DR Config is associated with Configuration " + configUUID.toString());
-      }
+      preBackupConfigDeleteValidation(customerConfig);
 
       DeleteCustomerStorageConfig.Params taskParams = new DeleteCustomerStorageConfig.Params();
       taskParams.customerUUID = customerUUID;
@@ -350,4 +353,25 @@ public Result listBuckets(UUID customerUUID, String cloud, Http.Request request)
     CloudUtil cloudUtil = cloudUtilFactory.getCloudUtil(cloud);
     return PlatformResults.withData(cloudUtil.listBuckets(configData));
   }
+
+  /** Don't allow storage config deletion if its in use by DR config or backup schedule. */
+  private void preBackupConfigDeleteValidation(CustomerConfig customerConfig) {
+    if (customerConfig.isStorageConfigUsedForDr()) {
+      throw new PlatformServiceException(
+          BAD_REQUEST,
+          "DR Config is associated with Configuration "
+              + customerConfig.getConfigUUID().toString());
+    }
+    List<Schedule> schedules =
+        Schedule.findAllScheduleWithCustomerConfig(customerConfig.getConfigUUID());
+    if (schedules != null && !schedules.isEmpty()) {
+      String error =
+          String.format(
+              "Backup config %s is associated with the following backup schedules and can't be"
+                  + " deleted: %s",
+              customerConfig.getConfigName(),
+              schedules.stream().map(Schedule::getScheduleName).collect(Collectors.joining(", ")));
+      throw new PlatformServiceException(BAD_REQUEST, error);
+    }
+  }
 }
diff --git a/managed/src/main/java/com/yugabyte/yw/models/Schedule.java b/managed/src/main/java/com/yugabyte/yw/models/Schedule.java
@@ -666,14 +666,12 @@ public static List<Schedule> findAllScheduleWithCustomerConfig(UUID customerConf
     List<Schedule> scheduleList =
         find.query()
             .where()
-            .or()
-            .eq("task_type", TaskType.BackupUniverse)
-            .eq("task_type", TaskType.MultiTableBackup)
-            .endOr()
-            .or()
-            .eq("status", "Paused")
-            .eq("status", "Active")
-            .endOr()
+            .in(
+                "task_type",
+                TaskType.BackupUniverse,
+                TaskType.MultiTableBackup,
+                TaskType.CreateBackup)
+            .notIn("status", State.Deleting, State.Error)
             .findList();
     // This should be safe to do since storageConfigUUID is a required constraint.
     scheduleList =

diff --git a/managed/src/main/resources/swagger-strict.json b/managed/src/main/resources/swagger-strict.json
@@ -19356,54 +19356,6 @@
       }
     },
     "/api/v1/customers/{cUUID}/configs/{configUUID}" : {
-      "delete" : {
-        "description" : "",
-        "operationId" : "deleteCustomerConfig",
-        "parameters" : [ {
-          "format" : "uuid",
-          "in" : "path",
-          "name" : "cUUID",
-          "required" : true,
-          "type" : "string"
-        }, {
-          "format" : "uuid",
-          "in" : "path",
-          "name" : "configUUID",
-          "required" : true,
-          "type" : "string"
-        }, {
-          "default" : false,
-          "in" : "query",
-          "name" : "isDeleteBackups",
-          "required" : false,
-          "type" : "boolean"
-        }, {
-          "in" : "query",
-          "name" : "request",
-          "required" : false
-        } ],
-        "responses" : {
-          "200" : {
-            "description" : "successful operation",
-            "schema" : {
-              "$ref" : "#/definitions/YBPTask"
-            }
-          }
-        },
-        "responsesObject" : {
-          "200" : {
-            "description" : "successful operation",
-            "schema" : {
-              "$ref" : "#/definitions/YBPTask"
-            }
-          }
-        },
-        "security" : [ {
-          "apiKeyAuth" : [ ]
-        } ],
-        "summary" : "Delete a customer configuration",
-        "tags" : [ "Customer Configuration" ]
-      },
       "put" : {
         "description" : "",
         "operationId" : "editCustomerConfig",

diff --git a/managed/src/main/resources/swagger.json b/managed/src/main/resources/swagger.json
@@ -19781,7 +19781,8 @@
     },
     "/api/v1/customers/{cUUID}/configs/{configUUID}" : {
       "delete" : {
-        "description" : "",
+        "deprecated" : true,
+        "description" : "<b style=\"color:#ff0000\">Deprecated since YBA version 2.25.0.0.</b></p>Use 'Delete a customer configuration V2' instead.",
         "operationId" : "deleteCustomerConfig",
         "parameters" : [ {
           "format" : "uuid",

diff --git a/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerConfigTest.java b/managed/src/test/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerConfigTest.java
@@ -69,15 +69,4 @@ public void testDeleteCustomerConfigWithoutBackups() throws InterruptedException
     submitTask(TaskType.DeleteCustomerConfig, params, true);
     verify(mockTableManager, times(0)).deleteBackup(any());
   }
-
-  @Test
-  public void testDeleteCustomerConfigWithSchedules() {
-    DeleteCustomerConfig.Params params = new DeleteCustomerConfig.Params();
-    params.customerUUID = defaultCustomer.getUuid();
-    params.configUUID = nfsStorageConfig.getConfigUUID();
-    submitTask(TaskType.DeleteCustomerConfig, params, true);
-    schedule = Schedule.getOrBadRequest(schedule.getScheduleUUID());
-    assertEquals(Schedule.State.Stopped, schedule.getStatus());
-    verify(mockTableManager, times(0)).deleteBackup(any());
-  }
 }
diff --git a/...ged/src/test/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerStorageConfigTest.java b/...ged/src/test/java/com/yugabyte/yw/commissioner/tasks/DeleteCustomerStorageConfigTest.java
@@ -9,7 +9,6 @@
 import com.yugabyte.yw.models.Backup;
 import com.yugabyte.yw.models.Backup.BackupState;
 import com.yugabyte.yw.models.Customer;
-import com.yugabyte.yw.models.Schedule;
 import com.yugabyte.yw.models.configs.CustomerConfig;
 import com.yugabyte.yw.models.configs.CustomerConfig.ConfigState;
 import java.util.UUID;
@@ -91,33 +90,6 @@ public void testCustomerConfigDeleteStateUpdateSuccessWithIsDeleteBackup() {
     assertEquals(ConfigState.QueuedForDeletion, s3StorageConfig.getState());
   }
 
-  @Test
-  public void testAssocicatedScheduleStop() {
-    UUID universeUUID = UUID.randomUUID();
-    CustomerConfig s3StorageConfig = ModelFactory.createS3StorageConfig(defaultCustomer, "TEST3");
-    Schedule schedule =
-        ModelFactory.createScheduleBackup(
-            defaultCustomer.getUuid(), universeUUID, s3StorageConfig.getConfigUUID());
-    Backup backup =
-        ModelFactory.createBackup(
-            defaultCustomer.getUuid(), universeUUID, s3StorageConfig.getConfigUUID());
-    backup.transitionState(BackupState.Completed);
-    DeleteCustomerStorageConfig deleteCustomerStorageConfigTask =
-        AbstractTaskBase.createTask(DeleteCustomerStorageConfig.class);
-    DeleteCustomerStorageConfig.Params params = new DeleteCustomerStorageConfig.Params();
-    params.customerUUID = defaultCustomer.getUuid();
-    params.configUUID = s3StorageConfig.getConfigUUID();
-    params.isDeleteBackups = true;
-    deleteCustomerStorageConfigTask.initialize(params);
-    deleteCustomerStorageConfigTask.run();
-    backup.refresh();
-    s3StorageConfig.refresh();
-    schedule.refresh();
-    assertEquals(BackupState.QueuedForDeletion, backup.getState());
-    assertEquals(ConfigState.QueuedForDeletion, s3StorageConfig.getState());
-    assertEquals(Schedule.State.Stopped, schedule.getStatus());
-  }
-
   @Test
   public void testDeleteInvalidCustomerConfig() {
     UUID invalidStorageConfigUUID = UUID.randomUUID();

diff --git a/managed/src/test/java/com/yugabyte/yw/controllers/CustomerConfigControllerTest.java b/managed/src/test/java/com/yugabyte/yw/controllers/CustomerConfigControllerTest.java
@@ -38,6 +38,7 @@
 import com.yugabyte.yw.models.Backup;
 import com.yugabyte.yw.models.Customer;
 import com.yugabyte.yw.models.CustomerTask;
+import com.yugabyte.yw.models.Schedule;
 import com.yugabyte.yw.models.Users;
 import com.yugabyte.yw.models.configs.CustomerConfig;
 import com.yugabyte.yw.models.configs.CustomerConfig.ConfigState;
@@ -267,17 +268,22 @@ public void testDeleteInUseStorageConfigWithDeleteBackups() {
     CustomerTask customerTask = CustomerTask.findByTaskUUID(fakeTaskUUID);
     assertEquals(customerTask.getTargetUUID(), configUUID);
     fakeTaskUUID = buildTaskInfo(null, TaskType.DeleteDrConfig);
-    when(mockCommissioner.submit(any(), any())).thenReturn(fakeTaskUUID);
 
     // Set http context
     TestUtils.setFakeHttpContext(defaultUser);
 
-    ModelFactory.createScheduleBackup(defaultCustomer.getUuid(), UUID.randomUUID(), configUUID);
-    result = doRequestWithAuthToken("DELETE", url, defaultUser.createAuthToken());
-    assertOk(result);
-    customerTask = CustomerTask.findByTaskUUID(fakeTaskUUID);
-    assertEquals(customerTask.getTargetUUID(), configUUID);
-    assertAuditEntry(2, defaultCustomer.getUuid());
+    Schedule schedule =
+        ModelFactory.createScheduleBackup(defaultCustomer.getUuid(), UUID.randomUUID(), configUUID);
+    result =
+        assertPlatformException(
+            () -> doRequestWithAuthToken("DELETE", url, defaultUser.createAuthToken()));
+    assertBadRequest(
+        result,
+        String.format(
+            "Backup config TEST11 is associated with the following backup schedules and can't be"
+                + " deleted: [%s]",
+            schedule.getScheduleName()));
+    assertAuditEntry(1, defaultCustomer.getUuid());
   }
 
   @Test