Skip to content

Commit

Permalink
[BACKPORT 2.7.2][yugabyte#9331] Platform: Allow editing "Configuratio…
Browse files Browse the repository at this point in the history
…n Name" for backup storage provider without security credentials

Summary:
  Attempts to update backup configurations which have credentials, but without changing them, fail
  on a step of the credentials check.

  As example:
    - S3 storage configuration with valid credentials (the credentials are verified on a stage of
      the configuration creation);
    - We are changing only the configuration name;
    - Save changes -> "Tha AWS Access Key Id you provided does not exist in our records".

The fix is to reorder operations: credentials check should go after the passed credentials are unmasked (currently it goes before).

Original diff:
https://phabricator.dev.yugabyte.com/D12353

Test Plan:
Jenkins: rebase: 2.7.2

Test scenario is the same:
  1. Create valid S3 configuration, save it.
  2. Go to another page and then return back to the S3 backup configuration page;
  3. Change the configuration name only and press "Save".

Reviewers: hsu, spotachev, amalyshev

Subscribers: yugaware, jenkins-bot

Differential Revision: https://phabricator.dev.yugabyte.com/D12377
  • Loading branch information
SergeyPotachev authored and mchiddy committed Jul 23, 2021
1 parent f103b30 commit b0ae570
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@
import com.yugabyte.yw.models.CustomerConfig;
import com.yugabyte.yw.models.helpers.CommonUtils;
import com.yugabyte.yw.models.helpers.CustomerConfigValidator;
import io.swagger.annotations.*;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiImplicitParam;
import io.swagger.annotations.ApiImplicitParams;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.Authorization;
import java.util.UUID;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.libs.Json;
Expand Down Expand Up @@ -94,16 +100,24 @@ public Result edit(UUID customerUUID, UUID configUUID) {
throw new YWServiceException(BAD_REQUEST, errorJson);
}

errorJson = configValidator.validateDataContent(formData);
if (errorJson.size() > 0) {
throw new YWServiceException(BAD_REQUEST, errorJson);
}
CustomerConfig config = CustomerConfig.getOrBadRequest(customerUUID, configUUID);
JsonNode data = Json.toJson(formData.get("data"));
if (data != null && data.get("BACKUP_LOCATION") != null) {
((ObjectNode) data).put("BACKUP_LOCATION", config.data.get("BACKUP_LOCATION"));
if ((data != null) && (data.get("BACKUP_LOCATION") != null)) {
if (!StringUtils.equals(
data.get("BACKUP_LOCATION").textValue(),
config.data.get("BACKUP_LOCATION").textValue())) {
throw new YWServiceException(BAD_REQUEST, "BACKUP_LOCATION field is read-only.");
}
}

JsonNode updatedData = CommonUtils.unmaskConfig(config.data, data);
((ObjectNode) formData).put("data", updatedData);

errorJson = configValidator.validateDataContent(formData);
if (errorJson.size() > 0) {
throw new YWServiceException(BAD_REQUEST, errorJson);
}

config.data = Json.toJson(updatedData);
config.configName = formData.get("configName").textValue();
config.name = formData.get("name").textValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void testEditInUseStorageConfig() {
ObjectNode bodyJson = Json.newObject();
JsonNode data =
Json.parse(
"{\"BACKUP_LOCATION\": \"test\", \"ACCESS_KEY\": \"A-KEY\", "
"{\"BACKUP_LOCATION\": \"s3://foo\", \"ACCESS_KEY\": \"A-KEY\", "
+ "\"ACCESS_SECRET\": \"A-SECRET\"}");
bodyJson.put("name", "test1");
bodyJson.set("data", data);
Expand Down Expand Up @@ -262,17 +262,44 @@ public void testEditWithBackupLocation() {
bodyJson.put("type", "STORAGE");
bodyJson.put("configName", "test2");
UUID configUUID = ModelFactory.createS3StorageConfig(defaultCustomer).configUUID;
String url = "/api/customers/" + defaultCustomer.uuid + "/configs/" + configUUID;
Result result =
assertYWSE(
() ->
FakeApiHelper.doRequestWithAuthTokenAndBody(
"PUT", url, defaultUser.createAuthToken(), bodyJson));

assertBadRequest(result, "BACKUP_LOCATION field is read-only.");

// Should not update the field BACKUP_LOCATION to "test".
CustomerConfig fromDb = CustomerConfig.get(configUUID);
assertEquals("s3://foo", fromDb.data.get("BACKUP_LOCATION").textValue());
}

@Test
public void testEditStorageNameOnly_SecretKeysPersist() {
UUID configUUID = ModelFactory.createS3StorageConfig(defaultCustomer).configUUID;
CustomerConfig fromDb = CustomerConfig.get(configUUID);

ObjectNode bodyJson = Json.newObject();
JsonNode data = fromDb.data;
bodyJson.put("name", "test1");
bodyJson.set("data", data);
bodyJson.put("type", "STORAGE");
bodyJson.put("configName", fromDb.configName);

String url = "/api/customers/" + defaultCustomer.uuid + "/configs/" + configUUID;
Result result =
FakeApiHelper.doRequestWithAuthTokenAndBody(
"PUT", url, defaultUser.createAuthToken(), bodyJson);
assertOk(result);
JsonNode json = Json.parse(contentAsString(result));
// Should not update the field BACKUP_LOCATION to "test".
assertEquals("s3://foo", json.get("data").get("BACKUP_LOCATION").textValue());
// SHould be updated and the API response should give asked data.
assertEquals("A-*****EW", json.get("data").get("ACCESS_KEY").textValue());
assertEquals("********", json.get("data").get("ACCESS_SECRET").textValue());

CustomerConfig newFromDb = CustomerConfig.get(configUUID);
assertEquals(
fromDb.data.get("ACCESS_KEY").textValue(), newFromDb.data.get("ACCESS_KEY").textValue());
assertEquals(
fromDb.data.get("ACCESS_SECRET").textValue(),
newFromDb.data.get("ACCESS_SECRET").textValue());
}

public void testInvalidPasswordPolicy() {
Expand Down

0 comments on commit b0ae570

Please sign in to comment.