Skip to content

Commit

Permalink
[Backport 2.4][#6680, #6882]: Safeguards for deleting in use storage …
Browse files Browse the repository at this point in the history
…configurations.

Summary:
We currently rely on our storage config for giving us the metadata about backups, without
which schedules, deletes and restores fail. As part of the first step in fixing this issue, we
prevent the user from deleting a storage configuration if it has associated backups with it.
Eventually, we will move to a model that allows us to have multiple configs for storage to allow the
user to customize newers storage buckets.
Original commit: 9b879be

Test Plan:
1) Tried deleting a storage config which had associated schedules and backups. Verified that the call failed.
2) Checked the API call response has the `inUse` field.
3) Verified that failed delete backups don't mark the Backup object as deleted.

Reviewers: sanketh, sb-yb, daniel

Reviewed By: sb-yb, daniel

Subscribers: jenkins-bot, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D10323
  • Loading branch information
Arnav15 committed Jan 16, 2021
1 parent fd00798 commit 1c6d4aa
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 50 deletions.
4 changes: 2 additions & 2 deletions managed/src/main/java/com/yugabyte/yw/common/NodeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ private List<String> getConfigureSubCommand(AnsibleConfigureServers.Params taskP
return subcommand;
}

public ShellProcessHandler.ShellResponse nodeCommand(NodeCommandType type,
NodeTaskParams nodeTaskParam) throws RuntimeException {
public ShellProcessHandler.ShellResponse nodeCommand(
NodeCommandType type, NodeTaskParams nodeTaskParam) throws RuntimeException {
List<String> commandArgs = new ArrayList<>();
UserIntent userIntent = getUserIntentFromParams(nodeTaskParam);
switch (type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,23 @@

package com.yugabyte.yw.controllers;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;

import com.google.inject.Inject;

import com.yugabyte.yw.common.ApiResponse;
import com.yugabyte.yw.models.Audit;
import com.yugabyte.yw.models.CustomerConfig;
import com.yugabyte.yw.models.helpers.CustomerConfigValidator;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import play.libs.Json;
import play.mvc.Result;

import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -44,13 +50,15 @@ public Result delete(UUID customerUUID, UUID configUUID) {
if (customerConfig == null) {
return ApiResponse.error(BAD_REQUEST, "Invalid configUUID: " + configUUID);
}
customerConfig.delete();
if (!customerConfig.delete()) {
return ApiResponse.error(INTERNAL_SERVER_ERROR,
"Customer Configuration could not be deleted.");
}
Audit.createAuditEntry(ctx(), request());
return ApiResponse.success("configUUID deleted");
}

public Result list(UUID customerUUID) {
List<CustomerConfig> configList = CustomerConfig.getAll(customerUUID);
return ApiResponse.success(configList);
return ApiResponse.success(CustomerConfig.getAll(customerUUID));
}
}
18 changes: 17 additions & 1 deletion managed/src/main/java/com/yugabyte/yw/models/Backup.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,25 @@ public void transitionState(BackupState newState) {
// We only allow state transition from InProgress to a valid state
// Or completed to deleted state.
if ((this.state == BackupState.InProgress && this.state != newState) ||
(this.state == BackupState.Completed && newState == BackupState.Deleted)) {
(this.state == BackupState.Completed && newState == BackupState.Deleted) ||
// This condition is for the case in which the delete fails and we need
// to reset the state.
(this.state == BackupState.Deleted && newState == BackupState.Completed)) {
this.state = newState;
save();
}
}

public static boolean existsStorageConfig(UUID customerConfigUUID) {
List<Backup> backupList = find.query().where()
.or()
.eq("state", BackupState.Completed)
.eq("state", BackupState.InProgress)
.endOr()
.findList();
backupList = backupList.stream()
.filter(b -> b.getBackupInfo().storageConfigUUID.equals(customerConfigUUID))
.collect(Collectors.toList());
return backupList.size() != 0;
}
}
18 changes: 18 additions & 0 deletions managed/src/main/java/com/yugabyte/yw/models/CustomerConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,24 @@ public JsonNode getData() {
return maskedData;
}

// Returns if there is an in use reference to the object.
public boolean getInUse() {
if (this.type == ConfigType.STORAGE) {
// Check if a backup or schedule currently has a reference.
return (Backup.existsStorageConfig(this.configUUID) ||
Schedule.existsStorageConfig(this.configUUID));
}
return false;
}

@Override
public boolean delete() {
if (!this.getInUse()) {
return super.delete();
}
return false;
}

public static CustomerConfig createWithFormData(UUID customerUUID, JsonNode formData) {
CustomerConfig customerConfig = Json.fromJson(formData, CustomerConfig.class);
customerConfig.customerUUID = customerUUID;
Expand Down
17 changes: 17 additions & 0 deletions managed/src/main/java/com/yugabyte/yw/models/Schedule.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.text.SimpleDateFormat;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;

@Entity
public class Schedule extends Model {
Expand Down Expand Up @@ -119,6 +120,22 @@ public static List<Schedule> getAllActive() {
return find.query().where().eq("status", "Active").findList();
}

public static boolean existsStorageConfig(UUID customerConfigUUID) {
List<Schedule> scheduleList = find.query().where()
.or()
.eq("task_type", TaskType.BackupUniverse)
.eq("task_type", TaskType.MultiTableBackup)
.endOr()
.eq("status", "Active")
.findList();
// This should be safe to do since storageConfigUUID is a required constraint.
scheduleList = scheduleList.stream()
.filter(s -> s.getTaskParams().path("storageConfigUUID")
.asText().equals(customerConfigUUID.toString()))
.collect(Collectors.toList());
return scheduleList.size() != 0;
}

public void setFailureCount(int count) {
this.failureCount = count;
if (count >= MAX_FAIL_COUNT) {
Expand Down
26 changes: 26 additions & 0 deletions managed/src/test/java/com/yugabyte/yw/common/ModelFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@
import com.yugabyte.yw.commissioner.Common;
import com.yugabyte.yw.common.kms.EncryptionAtRestManager;
import com.yugabyte.yw.common.kms.services.EncryptionAtRestService;
import com.yugabyte.yw.forms.BackupTableParams;
import com.yugabyte.yw.forms.UniverseDefinitionTaskParams;
import com.yugabyte.yw.models.Backup;
import com.yugabyte.yw.models.Customer;
import com.yugabyte.yw.models.CustomerConfig;
import com.yugabyte.yw.models.KmsConfig;
import com.yugabyte.yw.models.Provider;
import com.yugabyte.yw.models.Schedule;
import com.yugabyte.yw.models.Universe;
import com.yugabyte.yw.models.Users;
import com.yugabyte.yw.models.helpers.PlacementInfo;
import com.yugabyte.yw.models.helpers.TaskType;
import play.libs.Json;

import java.util.HashSet;
Expand Down Expand Up @@ -157,6 +161,28 @@ public static CustomerConfig createGcsStorageConfig(Customer customer) {
return CustomerConfig.createWithFormData(customer.uuid, formData);
}

public static Backup createBackup(UUID customerUUID, UUID universeUUID,
UUID configUUID) {
BackupTableParams params = new BackupTableParams();
params.storageConfigUUID = configUUID;
params.universeUUID = universeUUID;
params.keyspace = "foo";
params.tableName = "bar";
params.tableUUID = UUID.randomUUID();
return Backup.create(customerUUID, params);
}

public static Schedule createScheduleBackup(UUID customerUUID,UUID universeUUID,
UUID configUUID) {
BackupTableParams params = new BackupTableParams();
params.storageConfigUUID = configUUID;
params.universeUUID = universeUUID;
params.keyspace = "foo";
params.tableName = "bar";
params.tableUUID = UUID.randomUUID();
return Schedule.create(customerUUID, params, TaskType.BackupUniverse, 1000);
}

public static CustomerConfig setCallhomeLevel(Customer customer, String level) {
return CustomerConfig.createCallHomeConfig(customer.uuid, level);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@
import com.yugabyte.yw.common.FakeApiHelper;
import com.yugabyte.yw.common.FakeDBApplication;
import com.yugabyte.yw.common.ModelFactory;
import com.yugabyte.yw.models.Backup;
import com.yugabyte.yw.models.Customer;
import com.yugabyte.yw.models.CustomerConfig;
import com.yugabyte.yw.models.Schedule;
import com.yugabyte.yw.models.Users;
import org.junit.Before;
import org.junit.Test;
import org.junit.Ignore;
import play.libs.Json;
import play.mvc.Result;

import java.util.UUID;

import static com.yugabyte.yw.common.AssertHelper.assertBadRequest;
import static com.yugabyte.yw.common.AssertHelper.assertErrorNodeValue;
import static com.yugabyte.yw.common.AssertHelper.assertOk;
import static com.yugabyte.yw.common.AssertHelper.assertAuditEntry;
import static com.yugabyte.yw.common.AssertHelper.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static play.mvc.Http.Status.BAD_REQUEST;
Expand Down Expand Up @@ -146,4 +146,27 @@ public void testDeleteInvalidCustomerConfig() {
assertEquals(1, CustomerConfig.getAll(customer.uuid).size());
assertAuditEntry(0, defaultCustomer.uuid);
}

@Test
public void testDeleteInUseStorageConfig() {
UUID configUUID = ModelFactory.createS3StorageConfig(defaultCustomer).configUUID;
Backup backup = ModelFactory.createBackup(defaultCustomer.uuid, UUID.randomUUID(),
configUUID);
String url = "/api/customers/" + defaultCustomer.uuid + "/configs/" + configUUID;
Result result = FakeApiHelper.doRequestWithAuthToken("DELETE", url,
defaultUser.createAuthToken());
assertInternalServerError(result, "Customer Configuration could not be deleted.");
backup.delete();
Schedule schedule = ModelFactory.createScheduleBackup(defaultCustomer.uuid, UUID.randomUUID(),
configUUID);
result = FakeApiHelper.doRequestWithAuthToken("DELETE", url,
defaultUser.createAuthToken());
assertInternalServerError(result, "Customer Configuration could not be deleted.");
schedule.delete();
result = FakeApiHelper.doRequestWithAuthToken("DELETE", url,
defaultUser.createAuthToken());
assertOk(result);
assertEquals(0, CustomerConfig.getAll(defaultCustomer.uuid).size());
assertAuditEntry(1, defaultCustomer.uuid);
}
}
47 changes: 24 additions & 23 deletions managed/src/test/java/com/yugabyte/yw/models/BackupTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,13 @@ public class BackupTest extends FakeDBApplication {
public void setUp() {
defaultCustomer = ModelFactory.testCustomer();
s3StorageConfig = ModelFactory.createS3StorageConfig(defaultCustomer);

}

private Backup createBackup(UUID universeUUID) {
BackupTableParams params = new BackupTableParams();
params.storageConfigUUID = s3StorageConfig.configUUID;
params.universeUUID = universeUUID;
params.keyspace = "foo";
params.tableName = "bar";
params.tableUUID = UUID.randomUUID();
return Backup.create(defaultCustomer.uuid, params);
}

@Test
public void testCreate() {
UUID universeUUID = UUID.randomUUID();
Backup b = createBackup(universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
universeUUID, s3StorageConfig.configUUID);
assertNotNull(b);
String storageRegex = "s3://foo/univ-" + universeUUID +
"/backup-\\d{4}-[0-1]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d\\-\\d+/table-foo.bar-[a-zA-Z0-9]*";
Expand Down Expand Up @@ -99,15 +89,17 @@ public void testCreateWithNonS3StorageUUID() {
@Test
public void testFetchByUniverseWithValidUUID() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
createBackup(u.universeUUID);
ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
List<Backup> backupList = Backup.fetchByUniverseUUID(defaultCustomer.uuid, u.universeUUID);
assertEquals(1, backupList.size());
}

@Test
public void testFetchByUniverseWithInvalidUUID() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
createBackup(u.universeUUID);
ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
List<Backup> backupList = Backup.fetchByUniverseUUID(defaultCustomer.uuid, UUID.randomUUID());
assertEquals(0, backupList.size());
}
Expand All @@ -116,7 +108,8 @@ public void testFetchByUniverseWithInvalidUUID() {
@Test
public void testFetchByTaskWithValidUUID() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
UUID taskUUID = UUID.randomUUID();
b.setTaskUUID(taskUUID);
Backup fb = Backup.fetchByTaskUUID(taskUUID);
Expand All @@ -140,7 +133,8 @@ public void testFetchByTaskWithInvalidBackupUUID() {
@Test
public void testFetchByTaskWithTargetType() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
CustomerTask ct = CustomerTask.create(
defaultCustomer,
b.backupUUID,
Expand All @@ -155,15 +149,17 @@ public void testFetchByTaskWithTargetType() {
@Test
public void testGetWithValidCustomerUUID() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
Backup fb = Backup.get(defaultCustomer.uuid, b.backupUUID);
assertEquals(fb, b);
}

@Test
public void testGetWithInvalidCustomerUUID() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
Backup fb = Backup.get(UUID.randomUUID(), b.backupUUID);
assertNull(fb);
}
Expand All @@ -172,7 +168,8 @@ public void testGetWithInvalidCustomerUUID() {
@Test
public void testTransitionStateValid() throws InterruptedException {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
Date beforeUpdateTime = b.getUpdateTime();
assertNotNull(beforeUpdateTime);
Thread.sleep(1);
Expand All @@ -187,7 +184,8 @@ public void testTransitionStateValid() throws InterruptedException {
@Test
public void testTransitionStateInvalid() throws InterruptedException {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
Date beforeUpdateTime = b.getUpdateTime();
assertNotNull(b.getUpdateTime());
Thread.sleep(1);
Expand All @@ -201,7 +199,8 @@ public void testTransitionStateInvalid() throws InterruptedException {
@Test
public void testSetTaskUUIDWhenNull() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
UUID taskUUID = UUID.randomUUID();
assertNull(b.taskUUID);
b.setTaskUUID(taskUUID);
Expand All @@ -212,7 +211,8 @@ public void testSetTaskUUIDWhenNull() {
@Test
public void testSetTaskUUID() throws InterruptedException {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
UUID taskUUID1 = UUID.randomUUID();
UUID taskUUID2 = UUID.randomUUID();
ExecutorService service = Executors.newFixedThreadPool(2);
Expand All @@ -228,7 +228,8 @@ public void testSetTaskUUID() throws InterruptedException {
@Test
public void testSetTaskUUIDWhenNotNull() {
Universe u = ModelFactory.createUniverse(defaultCustomer.getCustomerId());
Backup b = createBackup(u.universeUUID);
Backup b = ModelFactory.createBackup(defaultCustomer.uuid,
u.universeUUID, s3StorageConfig.configUUID);
b.setTaskUUID(UUID.randomUUID());
UUID taskUUID = UUID.randomUUID();
assertNotNull(b.taskUUID);
Expand Down
Loading

0 comments on commit 1c6d4aa

Please sign in to comment.