[GitHub] [ozone] ChenSammi commented on pull request #4523: HDDS-8050. REST Endpoint for S3 secret manipulation

["ChenSammi (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org