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]