ashishkumar50 commented on code in PR #4538:
URL: https://github.com/apache/ozone/pull/4538#discussion_r1161453347
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java:
##########
@@ -168,9 +168,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
assignS3SecretValue = null;
}
} else {
- // Found in S3SecretTable.
- awsSecret.set(s3SecretValue.getAwsSecret());
- assignS3SecretValue = null;
+ // Found in S3SecretTable. No secret returned.
Review Comment:
There is an impact on TenantGetSecret after this change.
TenantGetSecret only meant for retrieving secret of existing user, which
will become obsolete as it will always return error. So please consider
removing this for tenant.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java:
##########
@@ -258,6 +259,51 @@ public void testGetSecretOfAnotherUserAsOzoneAdmin()
throws IOException {
processSuccessSecretRequest(USER_ALICE, 2, false);
}
+ @Test
+ public void testGetOwnSecretTwice() throws IOException {
+
+ // This effectively makes alice an S3 Admin.
+ when(ozoneManager.isS3Admin(ugiAlice)).thenReturn(true);
+ String userPrincipalId=USER_ALICE;
+
+ S3GetSecretRequest s3GetSecretRequest =
+ new S3GetSecretRequest(
+ new S3GetSecretRequest(
+ s3GetSecretRequest(userPrincipalId)
+ ).preExecute(ozoneManager)
+ );
+ // Run validateAndUpdateCache for the first time
+ OMClientResponse omClientResponse1 =
+ s3GetSecretRequest.validateAndUpdateCache(ozoneManager,
+ 1, ozoneManagerDoubleBufferHelper);
+ // Check response type and cast
+ Assert.assertTrue(omClientResponse1 instanceof S3GetSecretResponse);
+ final S3GetSecretResponse s3GetSecretResponse1 =
+ (S3GetSecretResponse) omClientResponse1;
+ //Secret is returned the first time
Review Comment:
Give space after "//" and correct in other places too.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java:
##########
@@ -225,7 +225,8 @@ public void testGetOwnSecretAsNonAdmin() throws IOException
{
final S3Secret s3Secret2 = processSuccessSecretRequest(
USER_ALICE, 2, false);
- Assert.assertEquals(s3Secret1.getAwsSecret(), s3Secret2.getAwsSecret());
+ //no secret is returned as secret already exists in the DB
Review Comment:
Give space after "//", and do run checkstyle script.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
