You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "pvillard31 (via GitHub)" <gi...@apache.org> on 2026/04/02 18:58:18 UTC
[PR] NIFI-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
pvillard31 opened a new pull request, #11100:
URL: https://github.com/apache/nifi/pull/11100
# Summary
NIFI-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager
Adds a TTL-based cache for resolved secret values in ParameterProviderSecretsManager to avoid redundant calls to ParameterProvider.fetchParameterValues() on every secret lookup. Each secret is cached independently by its fully qualified name and expires after a configurable TTL (default 5 minutes). Caching can be disabled by setting the TTL to 0 sec.
A follow-up improvement will be done to expose an API to invalidate the cache in case a user wants to force the refresh on the secrets while configuring a connector.
#### Changes
- **`SecretsManager`** — Added `invalidateCache()` to support programmatic cache clearing.
- **`SecretsManagerInitializationContext`** — Added `getProperty(String)` to receive configuration properties without coupling the API to `NiFiProperties`.
- **`NiFiProperties`** — Added `nifi.secrets.manager.cache.ttl` property key.
- **`StandardSecretsManagerInitializationContext`** — Extended to accept and expose an immutable properties map, implementing the new `getProperty()` method.
- **`FlowController.createSecretsManager()`** — Reads the cache TTL from `NiFiProperties` and passes it through the initialization context.
- **`ParameterProviderSecretsManager`** — Core caching implementation:
- Per-secret `ConcurrentHashMap` cache keyed by FQN with individual timestamps.
- TTL of `0` disables all cache reads and writes, preserving existing behavior.
- `getSecret()` checks cache before calling the provider; `getSecrets()` partitions references into cache hits and misses, batch-fetching only the misses.
- `findProvider()` refactored to accept a pre-fetched provider set to avoid redundant `getSecretProviders()` calls.
- `invalidateCache()` clears all cached entries.
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
- [ ] Pull request contains [commits signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) with a registered key indicating `Verified` status
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] 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
- [ ] Build completed using `./mvnw clean install -P contrib-check`
- [ ] JDK 21
- [ ] JDK 25
### Licensing
- [ ] 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)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] 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-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #11100:
URL: https://github.com/apache/nifi/pull/11100#discussion_r3034421269
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/components/connector/secrets/SecretsManager.java:
##########
@@ -37,4 +37,10 @@ public interface SecretsManager {
Map<SecretReference, Secret> getSecrets(Set<SecretReference> secretReferences);
+ /**
+ * Invalidates any cached secret data, forcing the next access to fetch fresh data
+ * from the underlying secret providers.
+ */
+ void invalidateCache();
Review Comment:
I initially added a default in the first version but decided to remove it since none of this code has been released yet and the parameter provider secrets manager is the only implementation. If this PR is not included in 2.9.0, then I agree that I should add a default.
--
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-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11100:
URL: https://github.com/apache/nifi/pull/11100#discussion_r3034330701
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/secrets/ParameterProviderSecretsManager.java:
##########
@@ -33,16 +36,29 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
public class ParameterProviderSecretsManager implements SecretsManager {
private static final Logger logger = LoggerFactory.getLogger(ParameterProviderSecretsManager.class);
+ private static final String DEFAULT_CACHE_DURATION = "5 mins";
+
private FlowManager flowManager;
+ private Duration cacheDuration;
+ private final Map<String, CachedSecret> secretCache = new ConcurrentHashMap<>();
+
+ private record CachedSecret(Secret secret, long timestampNanos) {
+ }
@Override
public void initialize(final SecretsManagerInitializationContext initializationContext) {
this.flowManager = initializationContext.getFlowManager();
+
+ final String cacheDurationValue = initializationContext.getProperty(NiFiProperties.SECRETS_MANAGER_CACHE_DURATION);
+ final String effectiveDuration = cacheDurationValue != null ? cacheDurationValue : DEFAULT_CACHE_DURATION;
Review Comment:
```suggestion
final String effectiveDuration = cacheDurationValue == null ? DEFAULT_CACHE_DURATION ? cacheDurationValue;
```
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/secrets/ParameterProviderSecretsManager.java:
##########
@@ -136,9 +177,77 @@ public Map<SecretReference, Secret> getSecrets(final Set<SecretReference> secret
return secrets;
}
- private SecretProvider findProvider(final SecretReference secretReference) {
+ private Map<SecretReference, Secret> fetchSecretsWithCache(final Set<SecretReference> secretReferences) {
final Set<SecretProvider> providers = getSecretProviders();
+ final Map<SecretReference, Secret> results = new HashMap<>();
+
+ // Partition references into cache hits vs. misses that need fetching
+ final Map<SecretProvider, Set<SecretReference>> uncachedByProvider = new HashMap<>();
+ for (final SecretReference secretReference : secretReferences) {
+ final String fqn = secretReference.getFullyQualifiedName();
+
+ if (fqn != null) {
+ final CachedSecret cached = secretCache.get(fqn);
+ if (cached != null && !isExpired(cached)) {
+ logger.debug("Cache hit for secret [{}]", fqn);
+ results.put(secretReference, cached.secret());
+ continue;
+ }
+ }
+
+ final SecretProvider provider = findProvider(secretReference, providers);
+ uncachedByProvider.computeIfAbsent(provider, k -> new HashSet<>()).add(secretReference);
+ }
+
+ // Batch fetch uncached secrets grouped by provider
+ for (final Map.Entry<SecretProvider, Set<SecretReference>> entry : uncachedByProvider.entrySet()) {
+ final SecretProvider provider = entry.getKey();
+ final Set<SecretReference> references = entry.getValue();
+
+ if (provider == null) {
+ for (final SecretReference secretReference : references) {
+ results.put(secretReference, null);
+ }
+ continue;
+ }
+
+ final List<String> secretNames = new ArrayList<>();
+ references.forEach(ref -> secretNames.add(ref.getFullyQualifiedName()));
+ final List<Secret> retrievedSecrets = provider.getSecrets(secretNames);
+ final Map<String, Secret> secretsByName = retrievedSecrets.stream()
+ .collect(Collectors.toMap(Secret::getFullyQualifiedName, Function.identity()));
+
+ for (final SecretReference secretReference : references) {
+ final String fqn = secretReference.getFullyQualifiedName();
+ final Secret secret = secretsByName.get(fqn);
+ results.put(secretReference, secret);
+
+ if (secret != null && fqn != null) {
+ cacheSecret(fqn, secret);
+ }
+ }
+ }
+
+ return results;
+ }
+
+ @Override
+ public void invalidateCache() {
+ secretCache.clear();
+ logger.debug("Secret cache invalidated");
+ }
+
+ private boolean isExpired(final CachedSecret cached) {
+ return Duration.ofNanos(System.nanoTime() - cached.timestampNanos()).compareTo(cacheDuration) >= 0;
Review Comment:
Recommend breaking this out to multiple lines to declare intermediate variables for easier reading. It should also be possible to compare on Duration to another.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/components/connector/secrets/SecretsManager.java:
##########
@@ -37,4 +37,10 @@ public interface SecretsManager {
Map<SecretReference, Secret> getSecrets(Set<SecretReference> secretReferences);
+ /**
+ * Invalidates any cached secret data, forcing the next access to fetch fresh data
+ * from the underlying secret providers.
+ */
+ void invalidateCache();
Review Comment:
Is it necessary to add a `default` implementation for this method?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/components/connector/secrets/TestParameterProviderSecretsManager.java:
##########
@@ -370,5 +379,199 @@ public void testGetSecretProvidersFiltersOutInvalidProviders() {
assertEquals(1, secretProviders.size());
assertEquals("valid-id", secretProviders.iterator().next().getProviderId());
}
+
+ @Test
+ public void testZeroDurationDisablesCaching() {
+ final ParameterProviderNode providerNode = createMockedParameterProviderNode(PROVIDER_1_ID, PROVIDER_1_NAME, GROUP_1_NAME,
+ createParameter(SECRET_1_NAME, SECRET_1_DESCRIPTION, SECRET_1_VALUE));
+ final ParameterProviderSecretsManager manager = createManagerWithCacheDuration("0 sec", providerNode);
+
+ final SecretReference reference = createSecretReference(PROVIDER_1_ID, null, SECRET_1_NAME);
+
+ manager.getSecret(reference);
+ manager.getSecret(reference);
+
+ verify(providerNode, times(2)).fetchParameterValues(anyList());
+ }
+
+ @Test
+ public void testSecretCacheReturnsFromCacheWithinDuration() {
+ final ParameterProviderNode providerNode = createMockedParameterProviderNode(PROVIDER_1_ID, PROVIDER_1_NAME, GROUP_1_NAME,
+ createParameter(SECRET_1_NAME, SECRET_1_DESCRIPTION, SECRET_1_VALUE));
+ final ParameterProviderSecretsManager manager = createManagerWithCacheDuration("5 mins", providerNode);
+
+ final SecretReference reference = createSecretReference(PROVIDER_1_ID, null, SECRET_1_NAME);
+
+ final Optional<Secret> first = manager.getSecret(reference);
+ final Optional<Secret> second = manager.getSecret(reference);
+
+ assertTrue(first.isPresent());
+ assertTrue(second.isPresent());
+ assertEquals(SECRET_1_VALUE, first.get().getValue());
+ assertEquals(SECRET_1_VALUE, second.get().getValue());
+
+ verify(providerNode, times(1)).fetchParameterValues(anyList());
+ }
+
+ @Test
+ public void testSecretCacheRefreshesAfterDurationExpiry() throws InterruptedException {
+ final ParameterProviderNode providerNode = createMockedParameterProviderNode(PROVIDER_1_ID, PROVIDER_1_NAME, GROUP_1_NAME,
+ createParameter(SECRET_1_NAME, SECRET_1_DESCRIPTION, SECRET_1_VALUE));
+ final ParameterProviderSecretsManager manager = createManagerWithCacheDuration("100 ms", providerNode);
+
+ final SecretReference reference = createSecretReference(PROVIDER_1_ID, null, SECRET_1_NAME);
+
+ manager.getSecret(reference);
+ verify(providerNode, times(1)).fetchParameterValues(anyList());
+
+ Thread.sleep(200);
+
+ manager.getSecret(reference);
+ verify(providerNode, times(2)).fetchParameterValues(anyList());
+ }
+
+ @Test
+ public void testInvalidateCacheForcesSecretRefresh() {
+ final ParameterProviderNode providerNode = createMockedParameterProviderNode(PROVIDER_1_ID, PROVIDER_1_NAME, GROUP_1_NAME,
+ createParameter(SECRET_1_NAME, SECRET_1_DESCRIPTION, SECRET_1_VALUE));
+ final ParameterProviderSecretsManager manager = createManagerWithCacheDuration("5 mins", providerNode);
Review Comment:
Recommend declaring a static variable and reusing for this default `5 mins` value.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/components/connector/secrets/SecretsManagerInitializationContext.java:
##########
@@ -23,4 +23,12 @@ public interface SecretsManagerInitializationContext {
FlowManager getFlowManager();
+ /**
+ * Returns the value of the given configuration property, or {@code null} if the property is not set.
+ *
+ * @param key the property key
+ * @return the property value, or {@code null}
+ */
+ String getProperty(String key);
Review Comment:
Recommend naming this `getApplicationProperty()` for clarity, since it sources from NiFi application properties.
--
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-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11100:
URL: https://github.com/apache/nifi/pull/11100#discussion_r3029837995
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/secrets/ParameterProviderSecretsManager.java:
##########
@@ -33,16 +35,30 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
public class ParameterProviderSecretsManager implements SecretsManager {
private static final Logger logger = LoggerFactory.getLogger(ParameterProviderSecretsManager.class);
+ private static final String DEFAULT_CACHE_TTL = "5 mins";
+
private FlowManager flowManager;
+ private long cacheTtlNanos;
Review Comment:
Recommend using `java.time.Duration` for clarity instead of using the `Nanos` naming.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/secrets/ParameterProviderSecretsManager.java:
##########
@@ -33,16 +35,30 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
public class ParameterProviderSecretsManager implements SecretsManager {
private static final Logger logger = LoggerFactory.getLogger(ParameterProviderSecretsManager.class);
+ private static final String DEFAULT_CACHE_TTL = "5 mins";
+
private FlowManager flowManager;
+ private long cacheTtlNanos;
+ private final Map<String, CachedSecret> secretCache = new ConcurrentHashMap<>();
+
+ private record CachedSecret(Secret secret, long timestampNanos) {
+ }
@Override
public void initialize(final SecretsManagerInitializationContext initializationContext) {
this.flowManager = initializationContext.getFlowManager();
+
+ final String cacheTtlValue = initializationContext.getProperty(NiFiProperties.SECRETS_MANAGER_CACHE_TTL);
+ final String effectiveTtl = cacheTtlValue != null ? cacheTtlValue : DEFAULT_CACHE_TTL;
+ this.cacheTtlNanos = FormatUtils.getTimeDuration(effectiveTtl.trim(), TimeUnit.NANOSECONDS);
+ logger.info("Secret cache TTL configured to {}", effectiveTtl);
Review Comment:
Logging the value seems unnecessary in general, and uncommon for other configuration properties, is there a reason for logging it here?
--
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-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on PR #11100:
URL: https://github.com/apache/nifi/pull/11100#issuecomment-4180127307
Thanks for the initial review. There is no documentation yet for Connectors but I added a section for the configuration of the Secrets Manager.
--
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-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory merged PR #11100:
URL: https://github.com/apache/nifi/pull/11100
--
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