Copilot commented on code in PR #11121:
URL: https://github.com/apache/ozone/pull/11121#discussion_r3911375111
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3sts/TestS3STSEndpoint.java:
##########
@@ -560,6 +566,79 @@ public void testStsWhenActionNotImplemented() throws
Exception {
"Operation GetSessionToken is not supported yet.");
}
+ @Test
+ public void testStsGetCallerIdentitySuccessForGetMethod() throws Exception {
+ final Response response = endpoint.get("GetCallerIdentity", null, null,
null, "2011-06-15", null);
+
+ assertEquals(200, response.getStatus());
+ verify(objectStore).getCallerIdentity();
+ verify(auditLogger).logWriteSuccess(any(AuditMessage.class));
+ verify(auditLogger, never()).logWriteFailure(any(AuditMessage.class));
+
+ final Document doc = parseXml((String) response.getEntity());
+ assertEquals("GetCallerIdentityResponse",
doc.getDocumentElement().getLocalName());
+ assertEquals(STS_NS, doc.getDocumentElement().getNamespaceURI());
+ assertEquals(
+ "123456789012", doc.getElementsByTagNameNS(STS_NS,
"Account").item(0).getTextContent());
+ assertEquals(
+ "arn:aws:iam::123456789012:user/test-user",
doc.getElementsByTagNameNS(STS_NS, "Arn").item(0).getTextContent());
Review Comment:
This assertion line is very likely to exceed the 120-char checkstyle limit,
which will fail the build. Consider wrapping the arguments onto multiple lines
(consistent with nearby assertEquals formatting).
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3sts/S3STSEndpoint.java:
##########
@@ -307,39 +310,76 @@ private Response handleAssumeRole(Set<String>
paramNamesToValidate, String roleA
.header("Content-Type", "text/xml")
.build();
} catch (IOException e) {
- LOG.error("Error during AssumeRole processing", e);
-
+ throw toStsProcessingException(
+ S3GAction.ASSUME_ROLE, auditParams, e, action, "User is not
authorized to perform: sts:AssumeRole on " +
+ "resource: " + roleArn);
+ } catch (Exception e) {
getAuditLogger().logWriteFailure(buildAuditMessageForFailure(S3GAction.ASSUME_ROLE,
auditParams, e));
+ throw e;
+ }
+ }
- if (e instanceof OMException) {
- final OMException omException = (OMException) e;
- if (omException.getResult() == OMException.ResultCodes.ACCESS_DENIED ||
- omException.getResult() ==
OMException.ResultCodes.PERMISSION_DENIED ||
- omException.getResult() == OMException.ResultCodes.TOKEN_EXPIRED) {
- throw new OSTSException(ACCESS_DENIED)
- .withMessage("User is not authorized to perform: sts:AssumeRole
on resource: " + roleArn);
- }
- if (omException.getResult() == OMException.ResultCodes.INVALID_TOKEN) {
- throw new OSTSException(STS_INVALID_CLIENT_TOKEN_ID);
- }
- if (omException.getResult() ==
OMException.ResultCodes.NOT_SUPPORTED_OPERATION ||
- omException.getResult() ==
OMException.ResultCodes.FEATURE_NOT_ENABLED) {
- throw new
OSTSException(STS_UNSUPPORTED_OPERATION).withMessage(omException.getMessage());
- }
- if (omException.getResult() ==
OMException.ResultCodes.INVALID_REQUEST) {
- throw new
OSTSException(STS_VALIDATION_ERROR).withMessage(omException.getMessage());
- }
- if (omException.getResult() ==
OMException.ResultCodes.MALFORMED_POLICY_DOCUMENT) {
- throw new
OSTSException(STS_MALFORMED_POLICY_DOCUMENT).withMessage(omException.getMessage());
- }
- }
- throw new OSTSException(STS_INTERNAL_FAILURE, e).withType("Receiver");
+ private Response handleGetCallerIdentity(String version, String requestId)
throws OSTSException {
+ final String action = GET_CALLER_IDENTITY_ACTION;
+ final Map<String, String> auditParams = getAuditParameters();
+ auditParams.put("action", action);
+ auditParams.put("requestId", requestId);
+
+ if (version == null || !version.equals(EXPECTED_VERSION)) {
+ final OSTSException exception = new OSTSException(STS_INVALID_ACTION)
+ .withMessage("Could not find operation " + action + " for version " +
+ (version == null ? "NO_VERSION_SPECIFIED. Expected version is:
" + EXPECTED_VERSION : version));
Review Comment:
The invalid-version error message for missing Version contains a double
space after the period ("NO_VERSION_SPECIFIED. Expected..."). Since this text
is returned to clients, it should be cleaned up to a single space.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3sts/TestS3STSEndpoint.java:
##########
@@ -560,6 +566,79 @@ public void testStsWhenActionNotImplemented() throws
Exception {
"Operation GetSessionToken is not supported yet.");
}
+ @Test
+ public void testStsGetCallerIdentitySuccessForGetMethod() throws Exception {
+ final Response response = endpoint.get("GetCallerIdentity", null, null,
null, "2011-06-15", null);
+
+ assertEquals(200, response.getStatus());
+ verify(objectStore).getCallerIdentity();
+ verify(auditLogger).logWriteSuccess(any(AuditMessage.class));
+ verify(auditLogger, never()).logWriteFailure(any(AuditMessage.class));
+
+ final Document doc = parseXml((String) response.getEntity());
+ assertEquals("GetCallerIdentityResponse",
doc.getDocumentElement().getLocalName());
+ assertEquals(STS_NS, doc.getDocumentElement().getNamespaceURI());
+ assertEquals(
+ "123456789012", doc.getElementsByTagNameNS(STS_NS,
"Account").item(0).getTextContent());
+ assertEquals(
+ "arn:aws:iam::123456789012:user/test-user",
doc.getElementsByTagNameNS(STS_NS, "Arn").item(0).getTextContent());
+ assertEquals(
+ "test-user", doc.getElementsByTagNameNS(STS_NS,
"UserId").item(0).getTextContent());
+ }
+
+ @Test
+ public void testStsGetCallerIdentityIgnoresExtraParameters() throws
Exception {
+ final Response response = endpoint.get("GetCallerIdentity", ROLE_ARN,
ROLE_SESSION_NAME, 3600, "2011-06-15", null);
Review Comment:
This endpoint.get(...) call line is likely over the 120-char checkstyle
limit due to many arguments. Wrapping it prevents checkstyle failures while
keeping the test readable.
--
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]
