exceptionfactory commented on code in PR #6661:
URL: https://github.com/apache/nifi/pull/6661#discussion_r1023281573


##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/credentials/provider/factory/CredentialsProviderFactory.java:
##########
@@ -122,4 +123,28 @@ public AWSCredentialsProvider getCredentialsProvider(final 
Map<PropertyDescripto
             return primaryCredentialsProvider;
         }
     }
+
+    /**
+     * Produces the AwsCredentialsProvider according to the given property set 
and the strategies configured in
+     * the factory.
+     * @return AwsCredentialsProvider implementation
+     */
+    public AwsCredentialsProvider getV2CredentialsProvider(final 
Map<PropertyDescriptor, String> properties) {

Review Comment:
   ```suggestion
       public AwsCredentialsProvider getAwsCredentialsProvider(final 
Map<PropertyDescriptor, String> properties) {
   ```



##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/credentials/provider/factory/strategies/AssumeRoleCredentialsStrategy.java:
##########
@@ -177,4 +187,62 @@ public AWSCredentialsProvider 
getDerivedCredentialsProvider(Map<PropertyDescript
 
         return credsProvider;
     }
+
+    @Override
+    public AwsCredentialsProvider getAwsCredentialsProvider(final 
Map<PropertyDescriptor, String> properties) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public AwsCredentialsProvider getDerivedAwsCredentialsProvider(final 
Map<PropertyDescriptor, String> properties,
+                                                                   
AwsCredentialsProvider primaryCredentialsProvider) {
+        final String assumeRoleArn = properties.get(ASSUME_ROLE_ARN);
+        final String assumeRoleName = properties.get(ASSUME_ROLE_NAME);
+        String rawMaxSessionTime = properties.get(MAX_SESSION_TIME);
+        rawMaxSessionTime = (rawMaxSessionTime != null) ? rawMaxSessionTime : 
MAX_SESSION_TIME.getDefaultValue();

Review Comment:
   Recommend adjusting for clarity:
   ```suggestion
           rawMaxSessionTime = rawMaxSessionTime == null ? 
MAX_SESSION_TIME.getDefaultValue() : rawMaxSessionTime;
   ```



##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/credentials/provider/factory/strategies/AbstractCredentialsStrategy.java:
##########
@@ -80,22 +81,27 @@ public Collection<ValidationResult> validate(final 
ValidationContext validationC
         return validationFailureResults;
     }
 
-    public abstract AWSCredentialsProvider 
getCredentialsProvider(Map<PropertyDescriptor, String> properties);
+    public abstract AWSCredentialsProvider getCredentialsProvider(final 
Map<PropertyDescriptor, String> properties);
 
     public String getName() {
         return name;
     }
 
 
     @Override
-    public boolean canCreateDerivedCredential(Map<PropertyDescriptor, String> 
properties) {
+    public boolean canCreateDerivedCredential(final Map<PropertyDescriptor, 
String> properties) {
         return false;
     }
 
     @Override
-    public AWSCredentialsProvider 
getDerivedCredentialsProvider(Map<PropertyDescriptor, String> properties,
-                                                                
AWSCredentialsProvider primaryCredentialsProvider) {
+    public AWSCredentialsProvider getDerivedCredentialsProvider(final 
Map<PropertyDescriptor, String> properties,
+                                                                final 
AWSCredentialsProvider primaryCredentialsProvider) {
         throw new UnsupportedOperationException();
     }
 
+    @Override
+    public AwsCredentialsProvider getDerivedAwsCredentialsProvider(final 
Map<PropertyDescriptor, String> properties,
+                                                                   
AwsCredentialsProvider primaryCredentialsProvider) {
+        throw new UnsupportedOperationException();
+    }

Review Comment:
   Although this follows the above pattern, is this necessary? Seems like it is 
best left unimplemented since the class is abstract.



##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/credentials/provider/factory/CredentialsProviderFactory.java:
##########
@@ -122,4 +123,28 @@ public AWSCredentialsProvider getCredentialsProvider(final 
Map<PropertyDescripto
             return primaryCredentialsProvider;
         }
     }
+
+    /**
+     * Produces the AwsCredentialsProvider according to the given property set 
and the strategies configured in
+     * the factory.
+     * @return AwsCredentialsProvider implementation
+     */
+    public AwsCredentialsProvider getV2CredentialsProvider(final 
Map<PropertyDescriptor, String> properties) {
+        final CredentialsStrategy primaryStrategy = 
selectPrimaryStrategy(properties);
+        AwsCredentialsProvider primaryCredentialsProvider = 
primaryStrategy.getAwsCredentialsProvider(properties);
+        AwsCredentialsProvider derivedCredentialsProvider = null;
+
+        for (final CredentialsStrategy strategy : strategies) {
+            if (strategy.canCreateDerivedCredential(properties)) {
+                derivedCredentialsProvider = 
strategy.getDerivedAwsCredentialsProvider(properties, 
primaryCredentialsProvider);
+                break;
+            }
+        }
+
+        if (derivedCredentialsProvider != null) {
+            return derivedCredentialsProvider;
+        } else {
+            return primaryCredentialsProvider;
+        }

Review Comment:
   This could be streamlined using a ternary:
   ```suggestion
           return derivedCredentialsProvider == null ? 
primaryCredentialsProvider : derivedCredentialsProvider;
   ```



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

Reply via email to