smengcl commented on a change in pull request #2804:
URL: https://github.com/apache/ozone/pull/2804#discussion_r746077015



##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
##########
@@ -102,16 +92,42 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
     // client does not need any proto changes.
     OMRequest.Builder omRequest = OMRequest.newBuilder()
         .setUserInfo(getUserInfo())
-        .setUpdateGetS3SecretRequest(updateGetS3SecretRequest)
         .setCmdType(getOmRequest().getCmdType())
         .setClientId(getOmRequest().getClientId());
 
+    // createIfNotExist defaults to true if not specified.
+    boolean createIfNotExist = !s3GetSecretRequest.hasCreateIfNotExist()
+            || s3GetSecretRequest.getCreateIfNotExist();
+
+    // Recompose GetS3SecretRequest just in case createIfNotExist is missing
+    final GetS3SecretRequest newGetS3SecretRequest =
+            GetS3SecretRequest.newBuilder()
+                    .setKerberosID(accessId)

Review comment:
       With the introduction of multi-tenancy the meaning of the proto field 
has changed.
   
   Before multi-tenancy, the accessId field is the same as Kerberos ID (and 
that's probably why the field was named `kerberosID`).
   
   Now we are allowing arbitrary accessIds in multi-tenancy so `accessId == 
kerberosID` doesn't apply anymore. And the proto message field should have been 
renamed to `accessId` in previous MT patches.




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