exceptionfactory commented on code in PR #9483:
URL: https://github.com/apache/nifi/pull/9483#discussion_r1828684670
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
+ "Provides a set of secret names to fetch, separated by a comma
delimiter ','. " +
+ "This strategy requires the GetSecretValue permission
only."
+ ),
+ Pattern(
Review Comment:
```suggestion
PATTERN(
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
+ "Provides a set of secret names to fetch, separated by a comma
delimiter ','. " +
+ "This strategy requires the GetSecretValue permission
only."
+ ),
+ Pattern(
+ "Pattern",
+ "Use regular expression",
Review Comment:
```suggestion
"Match Regular Expression",
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -137,21 +200,40 @@ public List<ParameterGroup> fetchParameters(final
ConfigurationContext context)
AWSSecretsManager secretsManager = this.configureClient(context);
final List<ParameterGroup> groups = new ArrayList<>();
- ListSecretsRequest listSecretsRequest = new ListSecretsRequest();
- ListSecretsResult listSecretsResult =
secretsManager.listSecrets(listSecretsRequest);
- while (!listSecretsResult.getSecretList().isEmpty()) {
- for (final SecretListEntry entry :
listSecretsResult.getSecretList()) {
- groups.addAll(fetchSecret(secretsManager, context,
entry.getName()));
+
+ // Fetch either by pattern or by enumerated list. See description of
SECRET_LISTING_STRATEGY for more details.
+ final String listingStrategy =
context.getProperty(SECRET_LISTING_STRATEGY).getValue();
+ final ListingStrategy linstingStrategyType =
ListingStrategy.valueOf(listingStrategy);
Review Comment:
The `asAllowableValue()` method can be used instead of getting the `String`
and calling `valueOf()`.
```suggestion
final ListingStrategy listingStrategy =
context.getProperty(SECRET_LISTING_STRATEGY).asAllowableValue(ListingStrategy.class);
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -137,21 +200,40 @@ public List<ParameterGroup> fetchParameters(final
ConfigurationContext context)
AWSSecretsManager secretsManager = this.configureClient(context);
final List<ParameterGroup> groups = new ArrayList<>();
- ListSecretsRequest listSecretsRequest = new ListSecretsRequest();
- ListSecretsResult listSecretsResult =
secretsManager.listSecrets(listSecretsRequest);
- while (!listSecretsResult.getSecretList().isEmpty()) {
- for (final SecretListEntry entry :
listSecretsResult.getSecretList()) {
- groups.addAll(fetchSecret(secretsManager, context,
entry.getName()));
+
+ // Fetch either by pattern or by enumerated list. See description of
SECRET_LISTING_STRATEGY for more details.
+ final String listingStrategy =
context.getProperty(SECRET_LISTING_STRATEGY).getValue();
+ final ListingStrategy linstingStrategyType =
ListingStrategy.valueOf(listingStrategy);
+ Set<String> secretsToFetch = new java.util.HashSet<>();
+ if (linstingStrategyType == ListingStrategy.Enumeration) {
+ String secretNames = context.getProperty(SECRET_NAMES).getValue();
+ secretsToFetch = new
java.util.HashSet<>(Arrays.asList(secretNames.split(",")));
Review Comment:
```suggestion
secretsToFetch = new
HashSet<>(Arrays.asList(secretNames.split(",")));
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
+ "Provides a set of secret names to fetch, separated by a comma
delimiter ','. " +
+ "This strategy requires the GetSecretValue permission
only."
+ ),
+ Pattern(
+ "Pattern",
+ "Use regular expression",
+ "Provides a regular expression to match keys of attributes to
filter for. " +
+ "This strategy requires both ListSecrets and
GetSecretValue permissions."
+ );
+
+ private final String value;
Review Comment:
```suggestion
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
+ "Provides a set of secret names to fetch, separated by a comma
delimiter ','. " +
+ "This strategy requires the GetSecretValue permission
only."
+ ),
+ Pattern(
+ "Pattern",
+ "Use regular expression",
+ "Provides a regular expression to match keys of attributes to
filter for. " +
+ "This strategy requires both ListSecrets and
GetSecretValue permissions."
+ );
+
+ private final String value;
+ private final String displayName;
+ private final String description;
+
+ ListingStrategy(final String value, final String displayName, final
String description) {
+ this.displayName = displayName;
+ this.value = value;
+ this.description = description;
+ }
+
+ @Override
+ public String getValue() {
+ return this.value;
+ }
+
+ @Override
+ public String getDisplayName() {
+ return this.displayName;
+ }
+
+ @Override
+ public String getDescription() {
+ return this.description;
+ }
+ }
+ public static final PropertyDescriptor SECRET_LISTING_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("secret-listing-strategy")
+ .displayName("Secret Listing Strategy")
+ .description("Strategy to use when listing secrets.")
+ .required(true)
+ .allowableValues(ListingStrategy.class)
+ .defaultValue(ListingStrategy.Pattern.getValue())
Review Comment:
```suggestion
.defaultValue(ListingStrategy.PATTERN)
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
Review Comment:
By convention, `enum` values should be uppercase.
```suggestion
ENUMERATION(
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
Review Comment:
It is not necessary to have a `value` member, instead the `name()` of the
enum can be used. Display names should use Title Case.
```suggestion
"Enumerate Secret Names",
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -68,16 +72,73 @@
@CapabilityDescription("Fetches parameters from AWS SecretsManager. Each
secret becomes a Parameter group, which can map to a Parameter Context, with " +
"key/value pairs in the secret mapping to Parameters in the group.")
public class AwsSecretsManagerParameterProvider extends
AbstractParameterProvider implements VerifiableParameterProvider {
+ enum ListingStrategy implements DescribedValue {
+ Enumeration(
+ "Enumeration",
+ "Enumerate secret names",
+ "Provides a set of secret names to fetch, separated by a comma
delimiter ','. " +
+ "This strategy requires the GetSecretValue permission
only."
+ ),
+ Pattern(
+ "Pattern",
+ "Use regular expression",
+ "Provides a regular expression to match keys of attributes to
filter for. " +
+ "This strategy requires both ListSecrets and
GetSecretValue permissions."
+ );
+
+ private final String value;
+ private final String displayName;
+ private final String description;
+
+ ListingStrategy(final String value, final String displayName, final
String description) {
+ this.displayName = displayName;
+ this.value = value;
+ this.description = description;
+ }
+
+ @Override
+ public String getValue() {
+ return this.value;
Review Comment:
```suggestion
return name();
```
--
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]
