Skip to content

Commit ece9d85

Browse files
[yugabyte#9331] Platform: Allow editing "Configuration 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). Test Plan: Scenario: 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: amalyshev Reviewed By: amalyshev Subscribers: jenkins-bot, yugaware Differential Revision: https://phabricator.dev.yugabyte.com/D12353
1 parent 3f15bfc commit ece9d85

File tree

2 files changed

+49
-13
lines changed

2 files changed

+49
-13
lines changed

managed/src/main/java/com/yugabyte/yw/controllers/CustomerConfigController.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import io.swagger.annotations.ApiOperation;
1717
import io.swagger.annotations.Authorization;
1818
import java.util.UUID;
19+
import org.apache.commons.lang3.StringUtils;
1920
import org.slf4j.Logger;
2021
import org.slf4j.LoggerFactory;
2122
import play.libs.Json;
@@ -97,16 +98,24 @@ public Result edit(UUID customerUUID, UUID configUUID) {
9798
throw new YWServiceException(BAD_REQUEST, errorJson);
9899
}
99100

100-
errorJson = configValidator.validateDataContent(formData);
101-
if (errorJson.size() > 0) {
102-
throw new YWServiceException(BAD_REQUEST, errorJson);
103-
}
104101
CustomerConfig config = CustomerConfig.getOrBadRequest(customerUUID, configUUID);
105102
JsonNode data = Json.toJson(formData.get("data"));
106-
if (data != null && data.get("BACKUP_LOCATION") != null) {
107-
((ObjectNode) data).put("BACKUP_LOCATION", config.data.get("BACKUP_LOCATION"));
103+
if ((data != null) && (data.get("BACKUP_LOCATION") != null)) {
104+
if (!StringUtils.equals(
105+
data.get("BACKUP_LOCATION").textValue(),
106+
config.data.get("BACKUP_LOCATION").textValue())) {
107+
throw new YWServiceException(BAD_REQUEST, "BACKUP_LOCATION field is read-only.");
108+
}
108109
}
110+
109111
JsonNode updatedData = CommonUtils.unmaskConfig(config.data, data);
112+
((ObjectNode) formData).put("data", updatedData);
113+
114+
errorJson = configValidator.validateDataContent(formData);
115+
if (errorJson.size() > 0) {
116+
throw new YWServiceException(BAD_REQUEST, errorJson);
117+
}
118+
110119
config.data = Json.toJson(updatedData);
111120
config.configName = formData.get("configName").textValue();
112121
config.name = formData.get("name").textValue();

managed/src/test/java/com/yugabyte/yw/controllers/CustomerConfigControllerTest.java

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void testEditInUseStorageConfig() {
199199
ObjectNode bodyJson = Json.newObject();
200200
JsonNode data =
201201
Json.parse(
202-
"{\"BACKUP_LOCATION\": \"test\", \"ACCESS_KEY\": \"A-KEY\", "
202+
"{\"BACKUP_LOCATION\": \"s3://foo\", \"ACCESS_KEY\": \"A-KEY\", "
203203
+ "\"ACCESS_SECRET\": \"A-SECRET\"}");
204204
bodyJson.put("name", "test1");
205205
bodyJson.set("data", data);
@@ -270,17 +270,44 @@ public void testEditWithBackupLocation() {
270270
bodyJson.put("type", "STORAGE");
271271
bodyJson.put("configName", "test2");
272272
UUID configUUID = ModelFactory.createS3StorageConfig(defaultCustomer).configUUID;
273+
String url = "/api/customers/" + defaultCustomer.uuid + "/configs/" + configUUID;
274+
Result result =
275+
assertYWSE(
276+
() ->
277+
FakeApiHelper.doRequestWithAuthTokenAndBody(
278+
"PUT", url, defaultUser.createAuthToken(), bodyJson));
279+
280+
assertBadRequest(result, "BACKUP_LOCATION field is read-only.");
281+
282+
// Should not update the field BACKUP_LOCATION to "test".
283+
CustomerConfig fromDb = CustomerConfig.get(configUUID);
284+
assertEquals("s3://foo", fromDb.data.get("BACKUP_LOCATION").textValue());
285+
}
286+
287+
@Test
288+
public void testEditStorageNameOnly_SecretKeysPersist() {
289+
UUID configUUID = ModelFactory.createS3StorageConfig(defaultCustomer).configUUID;
290+
CustomerConfig fromDb = CustomerConfig.get(configUUID);
291+
292+
ObjectNode bodyJson = Json.newObject();
293+
JsonNode data = fromDb.data;
294+
bodyJson.put("name", "test1");
295+
bodyJson.set("data", data);
296+
bodyJson.put("type", "STORAGE");
297+
bodyJson.put("configName", fromDb.configName);
298+
273299
String url = "/api/customers/" + defaultCustomer.uuid + "/configs/" + configUUID;
274300
Result result =
275301
FakeApiHelper.doRequestWithAuthTokenAndBody(
276302
"PUT", url, defaultUser.createAuthToken(), bodyJson);
277303
assertOk(result);
278-
JsonNode json = Json.parse(contentAsString(result));
279-
// Should not update the field BACKUP_LOCATION to "test".
280-
assertEquals("s3://foo", json.get("data").get("BACKUP_LOCATION").textValue());
281-
// SHould be updated and the API response should give asked data.
282-
assertEquals("A-*****EW", json.get("data").get("ACCESS_KEY").textValue());
283-
assertEquals("********", json.get("data").get("ACCESS_SECRET").textValue());
304+
305+
CustomerConfig newFromDb = CustomerConfig.get(configUUID);
306+
assertEquals(
307+
fromDb.data.get("ACCESS_KEY").textValue(), newFromDb.data.get("ACCESS_KEY").textValue());
308+
assertEquals(
309+
fromDb.data.get("ACCESS_SECRET").textValue(),
310+
newFromDb.data.get("ACCESS_SECRET").textValue());
284311
}
285312

286313
public void testInvalidPasswordPolicy() {

0 commit comments

Comments
 (0)