[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2775: HDDS-5883 Change S3G client to set S3 Auth per req

[GitBox <gi...@apache.org>]

bharatviswa504 commented on a change in pull request #2775:
URL: https://github.com/apache/ozone/pull/2775#discussion_r737915899



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -752,4 +753,13 @@ void setBucketQuota(String volumeName, String bucketName,
    */
   OzoneKey headObject(String volumeName, String bucketName,
       String keyName) throws IOException;
+
+  /**
+   * Sets the S3 Authentication information for the requests executed on behalf
+   * of the S3 API implementation within Ozone.
+   * @param s3Authentication the authentication information for each S3 API call.
+   */
+  void setTheadLocalS3Authentication(S3Authentication s3Authentication);
+  S3Authentication getThreadLocalS3Authentication();

Review comment:
       Missing JavaDoc for other API's

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
##########
@@ -223,11 +224,24 @@ public void close() throws IOException {
    */
   private OMResponse submitRequest(OMRequest omRequest)
       throws IOException {
-    OMRequest payload = OMRequest.newBuilder(omRequest)
-        .setTraceID(TracingUtil.exportCurrentSpan())
-        .build();
-
-    return transport.submitRequest(payload);
+    OMRequest.Builder  b = OMRequest.newBuilder(omRequest);
+    // Insert S3 Authentication information for each request.
+    if (getThreadLocalS3Authentication() != null) {
+      b.setS3Authentication(
+          org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.

Review comment:
       Minor NIT: Here org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.S3Authentication can be added as import and we can directly use S3Authentication

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -136,6 +140,10 @@ public ObjectEndpoint() {
 
   @PostConstruct
   public void init() {
+    LOG.debug("S3 access id: {}", s3Authentication.getAccessID());
+    getClient().getObjectStore().

Review comment:
       Do we need similar thing in other other end point classes also.
   Or can we move to this to Base class?

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
##########
@@ -223,11 +224,24 @@ public void close() throws IOException {
    */
   private OMResponse submitRequest(OMRequest omRequest)
       throws IOException {
-    OMRequest payload = OMRequest.newBuilder(omRequest)
-        .setTraceID(TracingUtil.exportCurrentSpan())
-        .build();
-
-    return transport.submitRequest(payload);
+    OMRequest.Builder  b = OMRequest.newBuilder(omRequest);

Review comment:
       Minor NIT:
   b -> some meaningful name

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
##########
@@ -172,12 +172,13 @@
 
 @InterfaceAudience.Private
 public final class OzoneManagerProtocolClientSideTranslatorPB
-    implements OzoneManagerProtocol {
+    implements OzoneManagerClientProtocol {
 
   private final String clientID;
 
   private OmTransport transport;
-
+  private ThreadLocal<S3Authentication> ozoneSharedSecretAuthThreadLocal

Review comment:
       minor NIT: ozoneSharedSecretAuthThreadLocal -> s3Auth/s3Authentication. IT matches with methodNames which do set/get




-- 
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