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]

Reply via email to