[#7706] Platform: Edit backup config credentials. #8050

nishantSharma459 · 2021-04-15T11:17:16Z

Description:
Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups.

This change should be implemented for all three storage configs:

AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey
GCP: Allow editing GCS Credentials [Do not allow editing bucket name]
Azure: Allow editing Azure SAS token [Do not allow editing Container URL]

Test Plan:

Hit the config button from the SideNav.
Click on Backup Tab and then hit the amazon s3 tab.
Create a backup config for s3.
Once it got created then you'll see the Edit configuration button next to the Delete configuration button.
Click on the Edit Configuration button.
AWS access credentials should be editable.

Do the same for the rest of the configs.

SBT test

[info] Passed: Total 1304, Failed 0, Errors 0, Passed 1300, Skipped 4 [success] Total time: 1187 s (19:47), completed 12 May, 2021 3:55:27 PM

…-db into 7706-Edit-Configs

…06-Edit-Configs

lgtm-com · 2021-04-15T11:23:01Z

This pull request introduces 2 alerts when merging bd965bc into 9d5751c - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

jaydeepkumara · 2021-04-16T07:57:33Z

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

+    assertOk(result);
+    assertEquals(data, CustomerConfig.get(configUUID).data);
+  }
+


Required to add more test cases.

testcase to edit not used config. It should allow all fields to edit.

testcase to edit in the used config should config with limited fields.

In used config with all field edit. API should give an error.

Invalid Data, Content, and Invalid configured. you can add 1 test case for each or add one testcase and cover all 3 senario.

@jaydeepkumara for both used and unused configs, editable fields are the same I believe. ( at least the current implementation of UI behaves like that. @nishantSharma459 confirm )

jaydeepkumara

In Test plan clearly mention that we just have tested end to end for AWS.

jaydeepkumara · 2021-04-16T07:58:23Z

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

+    }
+    CustomerConfig customerConfig = CustomerConfig.get(customerUUID, configUUID);
+    if (customerConfig == null) {
+      return ApiResponse.error(BAD_REQUEST, "Invalid configUUID: " + configUUID);


Invalid Customer configuration UUID.

Consistent with the delete API response.

jaydeepkumara · 2021-04-16T15:50:27Z

Tested below scenario.

NFS doesn't allow edit.
GCS just allowed credentials to update. Edited and saved successfully. Did not tried with creating a universe due to an access issue.
azure allows only token to update. Edited and saved successfully. Did not tried with creating a universe due to an access issue.
S3:- allows credentials to edit. also IAM role toggle.
Tried to edit credentials, first, I tried with the wrong credentials I edited and used the correct one, and looks updated properly. My backup was not working properly(facing some issue on creating a backup in normal flow as well) but with an invalid credential I got an SSH issue but after editing I got a different error. so looks like it has used an updated credential. IAM role required to test from my end.

…-db into 7706-Edit-Configs

lgtm-com · 2021-04-19T16:31:27Z

This pull request introduces 2 alerts when merging 88e5728 into e63ce11 - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

nishantSharma459 · 2021-04-21T07:33:17Z

Ran prettier.
For this ticket I'm not adding any unit testing file. Although I'm working on the advanced ticket which is multiple backup configurations and that will have all the UI testing scenarios covered(discussed already).
AWS scenario is tested. For other (GCP, azure) is not tested as lack of access.
Here're the screenshots:

sshev

Approved with an assumption that thorough testing was done before submitting the PR for review, including possible regression.
Suggest getting another pair of eyes as well, especially on java part as I haven't assessed it.

SergeyPotachev

Still some comments to finalize the BE part.

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

SergeyPotachev · 2021-04-28T12:44:56Z

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

+    }
+    CustomerConfig config = CustomerConfig.get(configUUID);
+    JsonNode data = Json.toJson(formData.get("data"));
+    if (data != null && data.get("BACKUP_LOCATION") != null) {


Please extract "BACKUP_LOCATION" into a constant or use CustomerConfigValidator.BACKUP_LOCATION_FIELDNAME.
P.S. BTW, maybe it is even better to extract these field names into a separate file if we going to use them not only inside CustomerConfigValidator.

we are using the string instead of constants lot of places. Including other tests and utils file for masking. I will create another issue to fix these separately.

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

SergeyPotachev · 2021-04-28T12:55:31Z

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

+  @Test
+  public void testEditInUseStorageConfig() {
+    ObjectNode bodyJson = Json.newObject();
+    JsonNode data = Json.parse("{\"BACKUP_LOCATION\": \"test\", \"ACCESS_KEY\": \"A-KEY\", " +


Putting strings here instead of somewhere defined constants may give us some problems later if someone decides to rename one of these fields. I suggest to create/use Java constants.

Yeah, we are using the string instead of constants lot of places. Including other tests and utils file for masking. I will create another issue to fix these separately.

OK, this works for me.

…-db into 7706-Edit-Configs

SergeyPotachev

One serious issue in BE code. Other things look good.

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

SergeyPotachev · 2021-05-05T08:39:08Z

managed/src/main/java/com/yugabyte/yw/models/helpers/CommonUtils.java

-    return isStrictlySensitiveField(key) || (value == null) ? MASKED_FIELD_VALUE
-        : value.replaceAll(maskRegex, "*");
+    return isStrictlySensitiveField(key) || value.length() < 5
+        || (value == null) ? MASKED_FIELD_VALUE : value.replaceAll(maskRegex, "*");


No-no-no. If you have this in such variant then you don't have a test case for this. IIRC, boolean operations are executed from left to right order. So if you have value=null, you will get NPE here as value.length() < 5 is executed before (value == null).
Update here and update tests to control this case.

Oops!! Yeah, did not check the last condition. Will update and add a test case too.

SergeyPotachev · 2021-05-05T08:39:44Z

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

+  @Test
+  public void testEditInUseStorageConfig() {
+    ObjectNode bodyJson = Json.newObject();
+    JsonNode data = Json.parse("{\"BACKUP_LOCATION\": \"test\", \"ACCESS_KEY\": \"A-KEY\", " +


OK, this works for me.

…06-Edit-Configs

lgtm-com · 2021-05-05T19:46:18Z

This pull request introduces 2 alerts when merging ee6a95d into ba3f65e - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

…-db into 7706-Edit-Configs

…06-Edit-Configs

@sshev

) Description: Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups. This change should be implemented for all three storage configs: AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey GCP: Allow editing GCS Credentials [Do not allow editing bucket name] Azure: Allow editing Azure SAS token [Do not allow editing Container URL] Test Plan: 1. Hit the config button from the SideNav. 2. Click on Backup Tab and then hit the amazon s3 tab. 3. Create a backup config for s3. 4. Once it got created then you'll see the Edit configuration button next to the Delete configuration button. 5. Click on the Edit Configuration button. 6. AWS access credentials should be editable. Do the same for the rest of the configs. Reviewers: @sshev , @jaydeepkumara, @gaurav061, @mjoshi0923, @SergeyPotachev, @Arnav15, @yb-andrew Reviewed by: @SergeyPotachev, @sshev, @jaydeepkumara

@sshev

) Description: Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups. This change should be implemented for all three storage configs: AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey GCP: Allow editing GCS Credentials [Do not allow editing bucket name] Azure: Allow editing Azure SAS token [Do not allow editing Container URL] Test Plan: 1. Hit the config button from the SideNav. 2. Click on Backup Tab and then hit the amazon s3 tab. 3. Create a backup config for s3. 4. Once it got created then you'll see the Edit configuration button next to the Delete configuration button. 5. Click on the Edit Configuration button. 6. AWS access credentials should be editable. Do the same for the rest of the configs. Reviewers: @sshev , @jaydeepkumara, @gaurav061, @mjoshi0923, @SergeyPotachev, @Arnav15, @yb-andrew Reviewed by: @SergeyPotachev, @sshev, @jaydeepkumara

@sshev

) * [#7706] Platform: Edit backup config credentials. (#8050) Description: Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups. This change should be implemented for all three storage configs: AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey GCP: Allow editing GCS Credentials [Do not allow editing bucket name] Azure: Allow editing Azure SAS token [Do not allow editing Container URL] Test Plan: 1. Hit the config button from the SideNav. 2. Click on Backup Tab and then hit the amazon s3 tab. 3. Create a backup config for s3. 4. Once it got created then you'll see the Edit configuration button next to the Delete configuration button. 5. Click on the Edit Configuration button. 6. AWS access credentials should be editable. Do the same for the rest of the configs. Reviewers: @sshev , @jaydeepkumara, @gaurav061, @mjoshi0923, @SergeyPotachev, @Arnav15, @yb-andrew Reviewed by: @SergeyPotachev, @sshev, @jaydeepkumara * [#7706] Platform: Cherry picked the edit backup config credentials commit. * extra test cases removed * fFormatted the code Co-authored-by: mahenranbhat <mahendra.bhat@hashedin.com>

@sshev

) * [#7706] Platform: Edit backup config credentials. (#8050) Description: Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups. This change should be implemented for all three storage configs: AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey GCP: Allow editing GCS Credentials [Do not allow editing bucket name] Azure: Allow editing Azure SAS token [Do not allow editing Container URL] Test Plan: 1. Hit the config button from the SideNav. 2. Click on Backup Tab and then hit the amazon s3 tab. 3. Create a backup config for s3. 4. Once it got created then you'll see the Edit configuration button next to the Delete configuration button. 5. Click on the Edit Configuration button. 6. AWS access credentials should be editable. Do the same for the rest of the configs. Reviewers: @sshev , @jaydeepkumara, @gaurav061, @mjoshi0923, @SergeyPotachev, @Arnav15, @yb-andrew Reviewed by: @SergeyPotachev, @sshev, @jaydeepkumara * [#7706] Platform: Cherry picked the edit backup config credentials commit. * review comment fixes * formatting changes * formatted common.utils Co-authored-by: mahenranbhat <mahendra.bhat@hashedin.com>

@sshev

) Description: Backup config security credentials can change over time. The platform should support editing the security credentials even if there are existing backups associated with a Backup Config. When security credentials expire, the Platform will fail to take new backups and also fail to delete old backups. This change should be implemented for all three storage configs: AWS: Allow editing Access Key/Secret, also toggling between IAM and AccessKey GCP: Allow editing GCS Credentials [Do not allow editing bucket name] Azure: Allow editing Azure SAS token [Do not allow editing Container URL] Test Plan: 1. Hit the config button from the SideNav. 2. Click on Backup Tab and then hit the amazon s3 tab. 3. Create a backup config for s3. 4. Once it got created then you'll see the Edit configuration button next to the Delete configuration button. 5. Click on the Edit Configuration button. 6. AWS access credentials should be editable. Do the same for the rest of the configs. Reviewers: @sshev , @jaydeepkumara, @gaurav061, @mjoshi0923, @SergeyPotachev, @Arnav15, @yb-andrew Reviewed by: @SergeyPotachev, @sshev, @jaydeepkumara

nishantSharma459 and others added 10 commits April 1, 2021 17:33

[yugabyte#7706] Platform: Set initial Values for the form.

95b3138

Added edit API for customer config.

ebc47af

Merge branch '7706-Edit-Configs' of github.com:jaydeepkumara/yugabyte…

1cd4f9a

…-db into 7706-Edit-Configs

Test cases and code improvements

e792ba6

[yugabyte#7706] Platform: Integrated the API.

e152b0d

[yugabyte#7706] Platform: Add Edit button and cancel button.

4f205b3

Merge branch 'master' of github.com:jaydeepkumara/yugabyte-db into 77…

e750ef2

…06-Edit-Configs

[yugabyte#7706] Platform: Hide the update button initially.

d847aac

[yugabyte#7706] Platform: Minor changes.

a5a7011

Merge branch 'master' of github.com:jaydeepkumara/yugabyte-db into 77…

bd965bc

…06-Edit-Configs

nishantSharma459 requested review from andrewc-dev, sshev, SergeyPotachev, Arnav15, mjoshi0923 and gaurav061 April 15, 2021 11:17

nishantSharma459 mentioned this pull request Apr 15, 2021

[Platform] Ability to update expired SAS token without changing any backup config #7706

Closed

nishantSharma459 linked an issue Apr 15, 2021 that may be closed by this pull request

[Platform] Ability to update expired SAS token without changing any backup config #7706

Closed

nishantSharma459 requested a review from mahendranbhat April 15, 2021 11:20

[yugabyte#7706] Platform: removed unsed variables.

cf27671

jaydeepkumara reviewed Apr 16, 2021

View reviewed changes

mahendranbhat and others added 4 commits April 19, 2021 02:18

review comment fixes

6d16016

Merge branch '7706-Edit-Configs' of github.com:jaydeepkumara/yugabyte…

f285d55

…-db into 7706-Edit-Configs

[yugabyte#7706] Platform: Fixed the issues related to the AWS config.

5758038

Merge branch '7706-Edit-Configs' of github.com:jaydeepkumara/yugabyte…

88e5728

…-db into 7706-Edit-Configs

nishantSharma459 requested a review from jaydeepkumara April 19, 2021 16:26

nishantSharma459 added 2 commits April 21, 2021 12:41

[yugabyte#7706] Platform: Fixed review comments.

76df1b8

[yugabyte#7706] Platform: Ran prettier.

2804e86

nishantSharma459 requested a review from sshev April 21, 2021 07:33

jaydeepkumara approved these changes Apr 21, 2021

View reviewed changes

sshev approved these changes Apr 27, 2021

View reviewed changes

SergeyPotachev requested changes Apr 28, 2021

View reviewed changes

nishantSharma459 and others added 3 commits April 29, 2021 19:24

[yugabyte#7706] Platform: Fixed validation on edit.

3baf2c9

review comment fixes

b9b6e46

Merge branch '7706-Edit-Configs' of github.com:jaydeepkumara/yugabyte…

cc7fdfe

…-db into 7706-Edit-Configs

mahendranbhat requested a review from SergeyPotachev April 30, 2021 19:15

SergeyPotachev requested changes May 5, 2021

View reviewed changes

mahendranbhat mentioned this pull request May 5, 2021

[Platform] Filter sensitive data from audit logs #8051

Closed

review comment fixes

77ec4bd

mahendranbhat requested a review from SergeyPotachev May 5, 2021 18:42

SergeyPotachev approved these changes May 5, 2021

View reviewed changes

Merge branch 'master' of github.com:jaydeepkumara/yugabyte-db into 77…

ee6a95d

…06-Edit-Configs

nishantSharma459 and others added 4 commits May 6, 2021 11:10

[yugabyte#7706] Platform: removed unsed variables.

0df5750

Merge branch '7706-Edit-Configs' of github.com:jaydeepkumara/yugabyte…

fbbf385

…-db into 7706-Edit-Configs

Merge branch 'master' of github.com:jaydeepkumara/yugabyte-db into 77…

0f32b7a

…06-Edit-Configs

audit entry added

5c2cf0a

nishantSharma459 merged commit ead3592 into yugabyte:master May 12, 2021

ryan-ally mentioned this pull request Nov 15, 2023

[Snyk] Security upgrade react-scripts from 3.2.0 to 3.3.0 ryan-ally/yugabyte-db#198

Open

nyndyny mentioned this pull request Nov 15, 2023

[Snyk] Security upgrade react-scripts from 3.2.0 to 3.3.0 nyndyny/yugabyte-db#168

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#7706] Platform: Edit backup config credentials. #8050

[#7706] Platform: Edit backup config credentials. #8050

nishantSharma459 commented Apr 15, 2021 •

edited by mahendranbhat

Loading

lgtm-com bot commented Apr 15, 2021

jaydeepkumara Apr 16, 2021

mahendranbhat Apr 18, 2021

jaydeepkumara Apr 21, 2021

jaydeepkumara left a comment

jaydeepkumara Apr 16, 2021

mahendranbhat Apr 18, 2021

jaydeepkumara commented Apr 16, 2021 •

edited

Loading

lgtm-com bot commented Apr 19, 2021

nishantSharma459 commented Apr 21, 2021 •

edited by jaydeepkumara

Loading

sshev left a comment

SergeyPotachev left a comment

SergeyPotachev Apr 28, 2021

mahendranbhat Apr 30, 2021 •

edited

Loading

SergeyPotachev Apr 28, 2021

mahendranbhat Apr 30, 2021 •

edited

Loading

SergeyPotachev May 5, 2021

SergeyPotachev left a comment

SergeyPotachev May 5, 2021

mahendranbhat May 5, 2021

SergeyPotachev May 5, 2021

lgtm-com bot commented May 5, 2021

[#7706] Platform: Edit backup config credentials. #8050

[#7706] Platform: Edit backup config credentials. #8050

Conversation

nishantSharma459 commented Apr 15, 2021 • edited by mahendranbhat Loading

lgtm-com bot commented Apr 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaydeepkumara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaydeepkumara commented Apr 16, 2021 • edited Loading

lgtm-com bot commented Apr 19, 2021

nishantSharma459 commented Apr 21, 2021 • edited by jaydeepkumara Loading

sshev left a comment

Choose a reason for hiding this comment

SergeyPotachev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahendranbhat Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahendranbhat Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyPotachev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented May 5, 2021

nishantSharma459 commented Apr 15, 2021 •

edited by mahendranbhat

Loading

jaydeepkumara commented Apr 16, 2021 •

edited

Loading

nishantSharma459 commented Apr 21, 2021 •

edited by jaydeepkumara

Loading

mahendranbhat Apr 30, 2021 •

edited

Loading

mahendranbhat Apr 30, 2021 •

edited

Loading