cnauroth commented on code in PR #7867:
URL: https://github.com/apache/hadoop/pull/7867#discussion_r2279916489


##########
hadoop-cloud-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java:
##########
@@ -131,4 +134,44 @@ private void validateCredentials(URI uri, Configuration 
configuration)
       }
     }
   }
+
+  @Test
+  public void testTmpTokenCredentialsProvider() throws Throwable {
+    Configuration configuration = new Configuration();
+    // Set DynamicTemporaryCosnCredentialsProvider as the CosCredentials
+    // Provider.
+    configuration.set(CosNConfigKeys.COSN_CREDENTIALS_PROVIDER,
+        
"org.apache.hadoop.fs.cosn.auth.DynamicTemporaryCosnCredentialsProvider");
+    validateTmpTokenCredentials(this.fsUri, configuration);
+  }
+
+  private void validateTmpTokenCredentials(URI uri, Configuration 
configuration)
+      throws IOException {
+    if (null != configuration) {
+      COSCredentialsProvider credentialsProvider =
+          CosNUtils.createCosCredentialsProviderSet(uri, configuration);
+      COSCredentials cosCredentials = credentialsProvider.getCredentials();
+      assertNotNull(cosCredentials, "The cos credentials obtained is null.");
+      assertTrue(
+          
StringUtils.equalsIgnoreCase(configuration.get(CosNConfigKeys.COSN_CREDENTIALS_PROVIDER),
+              
"org.apache.hadoop.fs.cosn.auth.DynamicTemporaryCosnCredentialsProvider"),
+          "CredentialsProvider must be 
DynamicTemporaryCosnCredentialsProvider");
+
+      if (!(cosCredentials instanceof BasicSessionCredentials)) {

Review Comment:
   This could be simplified to:
   
   ```
   assertTrue(cosCredentials instanceof BasicSessionCredentials, "...");
   ```
   



##########
hadoop-cloud-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java:
##########
@@ -131,4 +134,44 @@ private void validateCredentials(URI uri, Configuration 
configuration)
       }
     }
   }
+
+  @Test
+  public void testTmpTokenCredentialsProvider() throws Throwable {
+    Configuration configuration = new Configuration();
+    // Set DynamicTemporaryCosnCredentialsProvider as the CosCredentials
+    // Provider.
+    configuration.set(CosNConfigKeys.COSN_CREDENTIALS_PROVIDER,
+        
"org.apache.hadoop.fs.cosn.auth.DynamicTemporaryCosnCredentialsProvider");
+    validateTmpTokenCredentials(this.fsUri, configuration);
+  }
+
+  private void validateTmpTokenCredentials(URI uri, Configuration 
configuration)
+      throws IOException {
+    if (null != configuration) {

Review Comment:
   Is this null check required? It seems like `configuration` will always be 
non-null.



##########
hadoop-cloud-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java:
##########
@@ -131,4 +134,44 @@ private void validateCredentials(URI uri, Configuration 
configuration)
       }
     }
   }
+
+  @Test
+  public void testTmpTokenCredentialsProvider() throws Throwable {
+    Configuration configuration = new Configuration();
+    // Set DynamicTemporaryCosnCredentialsProvider as the CosCredentials
+    // Provider.
+    configuration.set(CosNConfigKeys.COSN_CREDENTIALS_PROVIDER,
+        
"org.apache.hadoop.fs.cosn.auth.DynamicTemporaryCosnCredentialsProvider");
+    validateTmpTokenCredentials(this.fsUri, configuration);
+  }
+
+  private void validateTmpTokenCredentials(URI uri, Configuration 
configuration)
+      throws IOException {
+    if (null != configuration) {
+      COSCredentialsProvider credentialsProvider =
+          CosNUtils.createCosCredentialsProviderSet(uri, configuration);
+      COSCredentials cosCredentials = credentialsProvider.getCredentials();
+      assertNotNull(cosCredentials, "The cos credentials obtained is null.");
+      assertTrue(
+          
StringUtils.equalsIgnoreCase(configuration.get(CosNConfigKeys.COSN_CREDENTIALS_PROVIDER),
+              
"org.apache.hadoop.fs.cosn.auth.DynamicTemporaryCosnCredentialsProvider"),
+          "CredentialsProvider must be 
DynamicTemporaryCosnCredentialsProvider");
+
+      if (!(cosCredentials instanceof BasicSessionCredentials)) {
+        fail("cosCredentials must be instanceof BasicSessionCredentials");
+      }
+
+      if (!StringUtils.equals(cosCredentials.getCOSAccessKeyId(), "ak") || 
!StringUtils.equals(

Review Comment:
   Instead of conditional logic around `fail`, I suggest that this should be 3 
separate `assertEquals` for access key, secret key and session token.



-- 
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: common-issues-unsubscr...@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to