adoroszlai commented on code in PR #5887:
URL: https://github.com/apache/ozone/pull/5887#discussion_r1438415058
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java:
##########
@@ -55,15 +56,26 @@ public Response generate(@PathParam("username") String
username)
return generateInternal(username);
}
- private Response generateInternal(@Nullable String username)
- throws IOException {
- S3SecretResponse s3SecretResponse = new S3SecretResponse();
- S3SecretValue s3SecretValue = generateS3Secret(username);
- s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
- s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
- AUDIT.logReadSuccess(buildAuditMessageForSuccess(
- S3GAction.GENERATE_SECRET, getAuditParameters()));
- return Response.ok(s3SecretResponse).build();
+ private Response generateInternal(@Nullable String username) throws
IOException {
+ try {
+ S3SecretValue s3SecretValue = generateS3Secret(username);
+
+ S3SecretResponse s3SecretResponse = new S3SecretResponse();
+ s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
+ s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
+ AUDIT.logReadSuccess(buildAuditMessageForSuccess(
+ S3GAction.GENERATE_SECRET, getAuditParameters()));
+ return Response.ok(s3SecretResponse).build();
+ } catch (OMException e) {
+ AUDIT.logWriteFailure(buildAuditMessageForFailure(
+ S3GAction.REVOKE_SECRET, getAuditParameters(), e));
Review Comment:
```suggestion
S3GAction.GENERATE_SECRET, getAuditParameters(), e));
```
##########
hadoop-ozone/dist/src/main/smoketest/commonlib.robot:
##########
@@ -51,3 +53,7 @@ Requires admin privilege
Pass Execution If '${SECURITY_ENABLED}' == 'false' Skip privilege
check in unsecure cluster
Kinit test user testuser2 testuser2.keytab
Access should be denied ${command}
+
+Revoke S3 secrets
+ Execute and Ignore Error ozone s3 revokesecret -y
+ Execute and Ignore Error ozone s3 revokesecret -y -u testuser
Review Comment:
Please move S3-specific keyword to `s3/commonawslib.robot`.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java:
##########
@@ -55,15 +56,26 @@ public Response generate(@PathParam("username") String
username)
return generateInternal(username);
}
- private Response generateInternal(@Nullable String username)
- throws IOException {
- S3SecretResponse s3SecretResponse = new S3SecretResponse();
- S3SecretValue s3SecretValue = generateS3Secret(username);
- s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
- s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
- AUDIT.logReadSuccess(buildAuditMessageForSuccess(
- S3GAction.GENERATE_SECRET, getAuditParameters()));
- return Response.ok(s3SecretResponse).build();
+ private Response generateInternal(@Nullable String username) throws
IOException {
+ try {
+ S3SecretValue s3SecretValue = generateS3Secret(username);
+
+ S3SecretResponse s3SecretResponse = new S3SecretResponse();
+ s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
+ s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
+ AUDIT.logReadSuccess(buildAuditMessageForSuccess(
+ S3GAction.GENERATE_SECRET, getAuditParameters()));
+ return Response.ok(s3SecretResponse).build();
+ } catch (OMException e) {
+ AUDIT.logWriteFailure(buildAuditMessageForFailure(
+ S3GAction.REVOKE_SECRET, getAuditParameters(), e));
+ if (e.getResult() == OMException.ResultCodes.S3_SECRET_ALREADY_EXISTS) {
+ return Response.status(BAD_REQUEST.getStatusCode(),
e.getResult().toString()).build();
+ } else {
+ LOG.error("Can't execute revoke secret request: ", e);
Review Comment:
```suggestion
LOG.error("Can't execute get secret request: ", e);
```
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java:
##########
@@ -55,15 +56,26 @@ public Response generate(@PathParam("username") String
username)
return generateInternal(username);
}
- private Response generateInternal(@Nullable String username)
- throws IOException {
- S3SecretResponse s3SecretResponse = new S3SecretResponse();
- S3SecretValue s3SecretValue = generateS3Secret(username);
- s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
- s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
- AUDIT.logReadSuccess(buildAuditMessageForSuccess(
- S3GAction.GENERATE_SECRET, getAuditParameters()));
- return Response.ok(s3SecretResponse).build();
+ private Response generateInternal(@Nullable String username) throws
IOException {
+ try {
+ S3SecretValue s3SecretValue = generateS3Secret(username);
+
+ S3SecretResponse s3SecretResponse = new S3SecretResponse();
+ s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
+ s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
+ AUDIT.logReadSuccess(buildAuditMessageForSuccess(
Review Comment:
I think this should have been a write request.
```suggestion
AUDIT.logWriteSuccess(buildAuditMessageForSuccess(
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
