You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "grishick (via GitHub)" <gi...@apache.org> on 2024/11/04 18:19:35 UTC
[PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
grishick opened a new pull request, #9483:
URL: https://github.com/apache/nifi/pull/9483
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-12061](https://issues.apache.org/jira/browse/NIFI-12061)
Add `Secret Name` parameter to `AwsSecretsManagerParameterProvider` to allow the provider to work without `ListSecrets` permission. Original behavior where the provider lists all secrets and matches them to `Secret Name Pattern` is preserved by leaving `Secret Name` parameter undefined. The new behavior will skip listing secrets and ignore `Secret Name Pattern` only if `Secret Name` is provided. I made this change in [Anetac's Nifi fork](https://github.com/Eng-Anetac/nifi) months ago and we are using it in production.
Corresponding PR for 1.x branch: https://github.com/apache/nifi/pull/9386
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-12061`
- [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-12061`
### Pull Request Formatting
- [x] Pull Request based on current revision of the `support/nifi-1.x` branch
- [x] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [x] Build completed using `mvn clean install -P contrib-check`
- [x] JDK 21
### Licensing
- [x] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [x] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [x] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
Posted by "grishick (via GitHub)" <gi...@apache.org>.
grishick commented on PR #9483:
URL: https://github.com/apache/nifi/pull/9483#issuecomment-2458005535
Thanks a lot for the reviews and suggestions and apologies for the messy commits! I've addressed the comments and pushed the update.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory closed pull request #9483: NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi…
URL: https://github.com/apache/nifi/pull/9483
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
Posted by "EndzeitBegins (via GitHub)" <gi...@apache.org>.
EndzeitBegins commented on code in PR #9483:
URL: https://github.com/apache/nifi/pull/9483#discussion_r1828706307
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -117,13 +178,15 @@ public class AwsSecretsManagerParameterProvider extends AbstractParameterProvide
private static final String DEFAULT_USER_AGENT = "NiFi";
private static final Protocol DEFAULT_PROTOCOL = Protocol.HTTPS;
- private static final List<PropertyDescriptor> PROPERTIES = List.of(
+ private static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList(
Review Comment:
For the `main` branch the declaration of `PROPERTIES` can remain to use `List.of`.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-12061: allow using AWS Secrets Manager without ListSecrets permi… [nifi]
Posted by "grishick (via GitHub)" <gi...@apache.org>.
grishick commented on code in PR #9483:
URL: https://github.com/apache/nifi/pull/9483#discussion_r1829885775
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-parameter-providers/src/main/java/org/apache/nifi/parameter/aws/AwsSecretsManagerParameterProvider.java:
##########
@@ -117,13 +178,15 @@ public class AwsSecretsManagerParameterProvider extends AbstractParameterProvide
private static final String DEFAULT_USER_AGENT = "NiFi";
private static final Protocol DEFAULT_PROTOCOL = Protocol.HTTPS;
- private static final List<PropertyDescriptor> PROPERTIES = List.of(
+ private static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList(
Review Comment:
OK. I switched it back to List.of
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org