ChenSammi commented on code in PR #11095:
URL: https://github.com/apache/ozone/pull/11095#discussion_r3849631589


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/STSTokenSecretManager.java:
##########
@@ -85,19 +96,29 @@ public Token<STSTokenIdentifier> 
generateToken(STSTokenIdentifier tokenIdentifie
    */
   public String createSTSTokenString(String tempAccessKeyId, String 
originalAccessKeyId, String roleArn,
       int durationSeconds, String secretAccessKey, String sessionPolicy, Clock 
clock) throws IOException {
-    final Instant expiration = clock.instant().plusSeconds(durationSeconds);
+    final Instant creationTime = clock.instant();
+    final Instant expiration = creationTime.plusSeconds(durationSeconds);
 
-    // Get the current secret key for encryption
-    final ManagedSecretKey currentSecretKey = 
secretKeyClient.getCurrentSecretKey();
-    final byte[] encryptionKey = currentSecretKey.getSecretKey().getEncoded();
+    // Get the current secret key once for encryption, secretKeyId, and 
signing.
+    final ManagedSecretKey secretKey = secretKeyClient.getCurrentSecretKey();
+    final byte[] encryptionKey = secretKey.getSecretKey().getEncoded();
 
     // Note - the encryptionKey will NOT be encoded in the token.  When 
generateToken() is called, it eventually calls
     // the write() method in STSTokenIdentifier which calls toProtoBuf(), and 
the encryptionKey is not
     // serialized there.
-    final STSTokenIdentifier identifier = new STSTokenIdentifier(
-        tempAccessKeyId, originalAccessKeyId, roleArn, expiration, 
secretAccessKey, sessionPolicy, encryptionKey);
+    final STSTokenIdentifier identifier = new 
STSTokenIdentifier(STSTokenIdentifier.Params.newBuilder()
+        .setTempAccessKeyId(tempAccessKeyId)
+        .setOriginalAccessKeyId(originalAccessKeyId)
+        .setRoleArn(roleArn)
+        .setCreationTime(creationTime)
+        .setExpiry(expiration)
+        .setSecretAccessKey(secretAccessKey)
+        .setSessionPolicy(sessionPolicy)
+        .setEncryptionKey(encryptionKey)
+        .build());
+    identifier.setSecretKeyId(secretKey.getId());

Review Comment:
   How about set the entire ManagedSecretKey to the STSTokenIdentifier builder? 
So we can avoid check id, and refetch the ManageedSecretKey again?



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