ChenSammi commented on PR #4523:
URL: https://github.com/apache/ozone/pull/4523#issuecomment-1632803298
Hi @ivanzlenko , thanks for working on this. The patch is quite big. So
today I have quickly gone through it, and come up with the following general
comments,
- Since the secret is transferred on wire, we probably should only support
this feature with https enabled, http is not safe for sensitive data transfer.
- Currently accessId is full Kerberos principal, not just the short name.
- Auditor is implemented in S3SecretEndpointBase but looks like not used.
- For the bind host, for s3g, there are both http-bind-host and
https-bind-host, since the secret servlet co-exists with s3g servlet, then so
why just use OZONE_S3G_HTTPS_BIND_HOST_KEY instead?
- I don't quite understand the purpose of this "globalAuth", could you
explain it a little bit?
- code indent style. Please find the inline comments.
I will try to go through the patch again later to have a detail review.
--
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]