neils-dev commented on code in PR #3382:
URL: https://github.com/apache/ozone/pull/3382#discussion_r872580918
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java:
##########
@@ -158,7 +158,7 @@ private OMResponse processRequest(OMRequest request) throws
// if current OM is leader and then proceed with processing the
request.
if (request.hasS3Authentication()) {
s3Auth = true;
- checkLeaderStatus();
+ OzoneManagerRatisUtils.checkLeaderStatus(ozoneManager);
S3SecurityUtil.validateS3Credential(request, ozoneManager);
}
Review Comment:
Hi @ChenSammi, thanks for the comment and for the help you've given me when
I started with ozone and ozone development.
> With HDDS-4440 and HDDS-5881, S3 authentication will be carried as part of
the OMRequest.
Yes, authentication is done using the `S3Authentication` fields of the
`OMRequest`. This` processRequest()` in the `ProtocalServerSideTranslatorPB`
is part of the path for processing requests with the authentication through
`S3SecurityUtil.validateS3Credential`.
> and these requests will not be processed by SASL. So the leader check is
still needed here.
Yes, the leader check is needed here, however isn't the leader check done
along with the call to `S3SecurityUtil.validateS3Credential`s as well? So in
this code path the leader is checked twice. Once just prior to the call to
`S3SecurityUtil.ValidateS3Credential` and once inside the call (within
`retrievePassword`). Is the call to
`OzoneManagerRatisUtils.checkLeaderStatus(ozoneManager)` still needed?
--
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]
