NIFI-16318 Preserve existing Parameter Contexts on registry import by mattcasters · Pull Request #11645 · apache/nifi

mattcasters · 2026-09-08T12:54:29Z

Summary

When importing a versioned flow or upgrading flow versions with Parameter Contexts, StandardVersionedComponentSynchronizer.addMissingConfiguration previously used currentParameterContext.getParameter() to check for parameter existence. Because getParameter() returns the resolved effective parameter (resolving across inherited contexts and dereferencing parameter references #{...}):

For an inherited parameter whose description differed in the versioned flow, addMissingConfiguration constructed an update and called setParameters() on the child context. This materialized an unwanted local override on the child context, severing parent inheritance.
During verifyCanSetParameters(), because the parameter was inherited and not in the child's local map (this.parameters), it was treated as a runtime-affecting addition. If a referencing component (such as a Controller Service) was active (ENABLED), the import failed with an IllegalStateException.
For parameter references (#{otherParam}), using getParameter() returned the resolved literal string value. Updating the description with fromParameter() replaced the reference syntax with the literal string, destroying the alias reference.

Root cause

StandardVersionedComponentSynchronizer.addMissingConfiguration:

Checked existence with currentParameterContext.getParameter(name) (effective) rather than checking local parameters (getParameters()) and inherited parameters (getRawEffectiveParameters()) separately.
Materialized local overrides on child contexts for inherited parameters when descriptions differed.
Dereferenced alias syntax (#{...}) into literal values during description updates.
Invoked setParameters() even when no additions or modifications were present.

Fix

In StandardVersionedComponentSynchronizer.addMissingConfiguration:

Check local parameters using currentParameterContext.getParameters().get(...) and build description updates using fromParameter(localParameter). This preserves the raw local value and reference syntax (#{otherParam}).
Check inherited parameters using currentParameterContext.getRawEffectiveParameters().get(...) and skip them (continue;). This prevents materializing local overrides on child contexts for inherited parameters, preserves parent inheritance, and avoids triggering component state verification on active referencing components.
Continue adding parameters that are absent from the effective set.
Only invoke setParameters() when parameters is non-empty.
Updated the UI tooltip in import-from-registry.component.html to clarify that parameter values and inheritance chains are preserved.

Tracking

Apache NiFi Jira issue: NIFI-16318
Pull Request title starts with NIFI-16318
Pull Request commit message starts with NIFI-16318
Single signed commit on a feature branch from current main

Verification

CheckStyle: ./mvnw -pl nifi-framework-bundle/nifi-framework/nifi-framework-components,nifi-system-tests/nifi-system-test-suite checkstyle:check
Unit tests: StandardVersionedComponentSynchronizerTest (all 60 tests passing, including tests for inherited parameters with diverging descriptions, and parameter reference #{...} preservation)
System tests: ParameterContextPreservationIT

mattcasters · 2026-09-08T14:48:36Z

I think the failure on system-tests / ubuntu-24.04 Java 21 is caused by github and not the code changes I made:

Started Server on https://localhost:5670/nifi
ERROR [main] org.apache.nifi.runtime.Application Start Server failed
java.io.UncheckedIOException: Management Server start failed
Caused by: java.net.BindException: Address already in use

Jetty bound HTTPS on 5670, then the management server (standalone tests use 127.0.0.1:56730 in bootstrap.conf) could not bind. NiFi shut itself down. The test client then waited 5 minutes, got connection refused, and reported “Failed to start NiFi”.

Why it is unrelated to this PR

• The hang is a port conflict on the management server, not Parameter Context merge.
• Flow load had already succeeded (Successfully synchronized dataflow… Process Groups=0).
• ParameterContextPreservationIT (the tests added in this PR) passed (8 tests, ~3s) later in the same job.
• macos-15 Java 21/25 and the other Ubuntu jobs passed.

The previous class, ContentClaimTruncationAfterRestartIT, uses isAllowFactoryReuse() = false and restarts NiFi. If that process (or a leftover standalone) still held 56730, the next standalone instance fails exactly this way. That is a known Linux CI flake under load (TIME_WAIT / leftover PID), not something this PR introduced.

bbende

Thanks for looking into this! Left some comments/questions for discussion

bbende · 2026-09-09T19:47:16Z

+
+        assertEquals(VALUE_XYZ, paramContext.getParameter(PARAM_ABC).get().getValue(),
+            "KEEP_EXISTING import must not overwrite an existing parameter value");
+        assertEquals(ORIGINAL_PARAMETER_DESCRIPTION, paramContext.getParameter(PARAM_ABC).get().getDescriptor().getDescription(),


Interestingly, if you run these 3 new tests without the rest of the changes in this PR (i.e. using the behavior on main), they all pass except for this line about the description, the value check above passes indicating that the value is not altered.

Should we try to alter these tests to reproduce the problem you experienced to ensure it is fixed?

bbende · 2026-09-09T19:49:59Z

+                        && !Objects.equals(localParameter.getDescriptor().getDescription(), versionedParameter.getDescription())) {
                    final Parameter updatedParameter = new Parameter.Builder()
-                        .fromParameter(existingParameter)
+                        .fromParameter(localParameter)


Just wanted to clarify that the original intent here of using fromParameter was to preserve the value.

I think the actual bug was in the previous code at line 2428 when it used currentParameterContext.getParameter which returns the resolved effective parameter. So for a parameter whose value is exactly #{other} in a context that has inheritance, a description-only update materialized a local parameter holding the resolved literal and destroyed the reference.

Is that what you ran into?

See the later comment about the tests, I think we should try to update them to test this scenario with losing the reference

bbende · 2026-09-09T20:02:38Z

                        // To accomplish this, we call updateProcessGroupContents() passing 'true' for the updateSettings flag but null out the position.
                        flowSnapshot.getFlowContents().setPosition(null);
+                        final boolean preserveExistingParameterContextEntries =
+                                ParameterContextHandlingStrategy.KEEP_EXISTING.equals(parameterContextHandlingStrategy);


The existing code reads parameterContextHandlingStrategy exactly once, and it does so inside a block that is guarded so it only runs on the originating node:

ProcessGroupResource.java
Ln 1070–1091

if (versionControlInfo != null && requestProcessGroupEntity.getVersionedFlowSnapshot() == null) { // ... fetch snapshot from registry ... // Step 4: Replace parameter contexts if necessary if (ParameterContextHandlingStrategy.REPLACE.equals(parameterContextHandlingStrategy)) { parameterContextReplacer.replaceParameterContexts(flowSnapshot, serviceFacade.getParameterContexts()); }

The getVersionedFlowSnapshot() == null condition is the key. Step 6 of that same block sets the snapshot on the entity, and the entity is what gets replicated. So on the receiving nodes the snapshot is already populated, the whole block is skipped, and the query param is never consulted there. The renaming decision has already been baked into the request body. That's why REPLACE works correctly in a cluster today despite the query param being dropped by getAbsolutePath().

The PR adds a second read of the strategy, and it puts it inside the withWriteLock callback — which is the one place that runs on every node, always after replication. On the receiving nodes that read has no query param to work with, so it always resolves to the @DefaultValue("KEEP_EXISTING").

So the underlying mechanism (query params don't survive replication) is pre-existing, and the existing code is correctly written around it. The divergence between standalone and clustered behavior is new in this PR.

bbende · 2026-09-09T20:10:04Z

+        // Use the effective parameter map so inherited parameters and resolved aliases are compared against
+        // the values components actually see. Using the local map treats an inherited name as "new" and
+        // incorrectly requires referencing components to be stopped.
+        verifyCanSetParameters(getEffectiveParameters(), updatedParameters, duringUpdate);


This is a correct and worthwhile fix, but it technically isn't required now that the synchronizer skips inherited parameters.

It's a separate behavioral change and is used by every parameter-context update in the code base. I would consider separating this to it's own JIRA with the new contract stated explicitly and tests for the provider-backed and sensitivity-shadowing cases.

During flow synchronization, using getParameter() returned the resolved effective parameter instead of local raw parameters. As a result, description-only updates materialized local overrides for inherited parameters and flattened parameter reference aliases (#{other}) to literal resolved values. Furthermore, materializing inherited parameters on child contexts caused verification to treat them as runtime-affecting additions, failing import when referencing components were active. In StandardVersionedComponentSynchronizer: - Preserve existing local parameter values and reference syntax (#{other}) using raw local parameters from getParameters(). - Do not materialize local overrides for inherited parameters found in getRawEffectiveParameters(). - Continue adding parameters that are absent from the effective set. - Only invoke setParameters when updates or additions exist.

mattcasters · 2026-09-11T08:33:46Z

Thanks @bbende for the thorough review and insights!

In response to your feedback, we have streamlined the PR to focus squarely on the root cause in StandardVersionedComponentSynchronizer:

Reverted preserveExistingParameterContextEntries plumbing (11 files):
- Reverted changes to ProcessGroupResource, NiFiServiceFacade, ProcessGroupDAO, ProcessGroup, FlowSynchronizationOptions, and related classes.
- Eliminating the second read of the strategy inside withWriteLock resolves the cluster replication discrepancy.
Reverted StandardParameterContext changes:
- Reverted the verifyCanSetParameters(getEffectiveParameters(), ...) change and its test so this broader behavior can be discussed and tested separately in its own dedicated JIRA.
Streamlined StandardVersionedComponentSynchronizer.addMissingConfiguration:
- Local Parameters: Uses currentParameterContext.getParameters().get(...) to build description updates from the raw local parameter via fromParameter(localParameter). This preserves the raw value and parameter reference syntax (#{targetParam}) without flattening it to a resolved literal.
- Inherited Parameters: Checks currentParameterContext.getRawEffectiveParameters() and skips inherited parameters (continue;). Inherited parameters are never materialized as local overrides on child contexts, and referencing components are unaffected.
- Missing Parameters: Continues adding genuinely absent parameters.
- No-op avoidance: Only calls setParameters() when parameters is non-empty.
Updated Unit Tests (StandardVersionedComponentSynchronizerTest):
- Added testInheritedParameterNotMaterializedAsLocalOverrideWhenDescriptionDiffers to test that divergent descriptions do not cause child contexts to materialize local overrides of inherited parameters.
- Added testParameterReferencePreservedWhenDescriptionUpdated to test that alias references (#{targetParam}) retain their reference syntax and are not flattened to literals upon description updates.
- Switched tests to use standard synchronizationOptions.
- All 60 unit tests pass, and Checkstyle reports 0 violations.

bbende reviewed Sep 9, 2026

View reviewed changes

mattcasters force-pushed the NIFI-16318 branch from 269985b to 2751706 Compare September 11, 2026 08:32

Navigation Menu

Uh oh!

NIFI-16318 Preserve existing Parameter Contexts on registry import - #11645

NIFI-16318 Preserve existing Parameter Contexts on registry import#11645
mattcasters wants to merge 1 commit into
apache:mainfrom
mattcasters:NIFI-16318

mattcasters commented Sep 8, 2026 •

edited

Loading

Uh oh!

mattcasters commented Sep 8, 2026

Uh oh!

bbende left a comment

Uh oh!

bbende Sep 9, 2026

Uh oh!

bbende Sep 9, 2026

Uh oh!

bbende Sep 9, 2026

Uh oh!

bbende Sep 9, 2026

Uh oh!

bbende Sep 9, 2026

Uh oh!

mattcasters commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mattcasters commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix

Tracking

Verification

Uh oh!

mattcasters commented Sep 8, 2026

Uh oh!

bbende left a comment

Choose a reason for hiding this comment

Uh oh!

bbende Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

bbende Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

bbende Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

bbende Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

bbende Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

mattcasters commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mattcasters commented Sep 8, 2026 •

edited

Loading