NIFI-16242: Inherit default values for newly added Connector properties by markap14 · Pull Request #11581 · apache/nifi

markap14 · 2026-08-21T18:58:35Z

Summary

After inheriting a saved Connector configuration, apply the NAR's default for any property that has no persisted value.
This keeps a Connector valid across NAR upgrades that add a required property with a default, matching Processors, Controller Services, and Reporting Tasks.
Jira: https://issues.apache.org/jira/browse/NIFI-16242

Test plan

TestStandardConnectorNode.testVerifyCanStartAfterInheritingConfigurationMissingRequiredPropertyWithDefault inherits a saved step that omits a required property with a default, then asserts verifyCanStart() succeeds and the default is present
Confirmed that test fails without the change (Repeat Count is required) and passes with it

Made with Cursor

Made-with: Cursor

ncover21 · 2026-08-21T19:19:54Z

+            return migratedProperties;
+        }
+
+        final Map<String, StepConfiguration> propertiesWithDefaults = new HashMap<>(migratedProperties);


I think this needs to be a LinkedHashMap. migrateProperties builds initial as a LinkedHashMap with a comment about preserving persisted step order for the notifyStepConfigured loop at line 407, and copying into a HashMap reorders by hash. So a connector with steps Connection, Filter, Destination would get its onConfigurationStepConfigured callbacks in hash order on every flow load, but in declaration order when configured through the UI. If a later step reads values an earlier one established it would build a different managed flow after a restart.

I can use a LinkedHashMap just to maintain consistency. But in reality, tracing through the code, the assertion made by the comment is entirely false. LinkedHashMap is not needed. Which actually makes sense - if our interface says Map and it secretly depends on that Map having a specific order that would be a rather critical bug!

Kept it as a LinkedHashMap for consistency with migrateProperties and StandardConnectorPropertyConfiguration so the migration path stays deterministic. As noted in the other thread, the ordering isn't actually load-bearing here since ConnectorConfiguration stores steps in a HashSet, so nothing downstream depends on step order regardless.

ncover21 · 2026-08-21T19:21:46Z

+                }
+            }
+
+            if (appliedMissingDefault) {


Correct me if Im wrong here but this puts a brand new entry in when the persisted flow had no such step at all, and that entry then feeds the notifyStepConfigured loop in inheritConfiguration. So the connector gets onConfigurationStepConfigured for a step it was never configured with, and since the callback exception gets wrapped into a RuntimeException further down, I think that would fail flow loading at startup rather than just leaving the connector invalid.

Also if a connector calls removeStep(...) in migrateProperties, this would put the step straight back with defaults. Would it be safer to only fill defaults for steps already present in the migrated map?

Fixed in 9e5afff. migrateProperties now captures the persisted step names and passes them in, so two cases can be told apart:

A step that was persisted but dropped from the migrated map (the connector called removeStep(...)) is not re-created.

A declared step that appears in neither the persisted flow nor the migrated map is genuinely new in this NAR version, so it is created with its required defaults. Otherwise a NAR that adds a new required-with-default step would leave the connector invalid. A new step is only created when at least one required default actually applies, so we never materialize an empty step just to fire a callback.

The onConfigurationStepConfigured callback that fires for a newly added step is the same path a normally-configured step takes. Added tests for both the removed-step and new-step cases.

ncover21 · 2026-08-21T19:24:16Z

+            boolean appliedMissingDefault = false;
+            for (final ConnectorPropertyGroup propertyGroup : configurationStep.getPropertyGroups()) {
+                for (final ConnectorPropertyDescriptor descriptor : propertyGroup.getProperties()) {
+                    if (propertyValues.containsKey(descriptor.getName()) || descriptor.getDefaultValue() == null) {


One thing im not sure if im reading correctly here, the defaults get applied without looking at property or step dependencies. AbstractConnector.isDependencySatisfied reads the controlling value through the name-based getProperty(stepName, name) overload, which never returns null, so an unset controlling property reads as null and the dependent property stays gated off.

So for a NAR that adds SSL Mode (default REQUIRED) plus a required Truststore Filename that dependsOn(SSL Mode, "REQUIRED"): before this change the connector stays valid because SSL Mode is unset, after it the default makes the dependency satisfied and Truststore Filename reports as required. That would be the opposite of the intent. Does that hold or am I misreading the dependency check?

You read it correctly. Fixed in 9e5afff: defaults are now filled only for required properties, so an optional controlling property like SSL Mode stays unset and its dependent Truststore Filename remains gated off. Added testInheritingConfigurationDoesNotApplyOptionalPropertyDefault covering exactly the SSL Mode / Truststore Filename case.

…heritance Only inherit defaults for required properties so an optional property's default cannot activate a dependent property. Do not re-create a step that the Connector removed during migration, and create a newly declared step only when at least one required default applies to it. Added tests covering optional defaults, newly added steps, and steps removed during migration. Made-with: Cursor

bobpaulin

Thanks for this @markap14! A few more comments concerning both step and property dependencies that might require attention

bobpaulin · 2026-08-21T21:10:45Z

+     * upgrade that adds a required property with a default does not make the Connector invalid. Only required
+     * properties are filled, so inheriting a default cannot activate a dependent property. A step the Connector


So a dependent property could also be required and in this case wouldn't we want the dependent property's default value to be set?

Yes — a dependent property that is required, has a default, and is actually relevant does get its default materialized, since the back-fill fills every required-with-default property. If it's gated off it's irrelevant and doesn't need a value. Both directions are covered by the test in 5a5a34b.

Ok the tests adds clarity. I find the comment confusing then. Why is it important to the reader that "so inheriting a default cannot activate a dependent property"?

Can this comment be more clear that it's just going to set everything and the dependencies are really the irrelevant part?

Agreed — that sentence is leftover and the wrong thing to emphasize. This method just fills missing required properties that have a default. Reworded the comment in 7c7ef13 to describe that.

bobpaulin · 2026-08-21T21:27:42Z

+            boolean appliedMissingDefault = false;
+            for (final ConnectorPropertyGroup propertyGroup : configurationStep.getPropertyGroups()) {
+                for (final ConnectorPropertyDescriptor descriptor : propertyGroup.getProperties()) {
+                    if (!descriptor.isRequired() || descriptor.getDefaultValue() == null || propertyValues.containsKey(descriptor.getName())) {


The above considers if a specific property is required and has a default value. If the property also has a dependency that is not met I assume it should not be set.

Good question — rather than reason about it I wrote a test, and the current behavior turns out to be safe because dependency evaluation is transitive. I set up a step with an optional Authentication, a required Username (default admin) that dependsOn(Authentication, "Basic"), and a required Password (no default) that dependsOn(Username). With Authentication unset, the back-fill still materializes Username=admin, but Password stays irrelevant and the connector remains valid.

The reason is AbstractConnector.isDependencySatisfied: when it evaluates Password it recurses into Username's own dependency, and since Authentication is unset (the name-based getProperty returns null, no default fallback), Username isn't relevant, so Password isn't either. Materializing a gated-off property's default can't activate anything downstream, because the downstream property is gated by the same unsatisfied ancestor.

So filling a required-with-default property that's currently gated off is harmless — validation ignores it and everything beneath it. Added testInheritingConfigurationKeepsTransitivelyGatedRequiredPropertyIrrelevant (5a5a34b) to lock this in.

bobpaulin · 2026-08-21T21:29:52Z

+        }
+
+        final Map<String, StepConfiguration> propertiesWithDefaults = new LinkedHashMap<>(migratedProperties);
+        for (final ConfigurationStep configurationStep : configurationSteps) {


Should we also consider if the step dependency is satisfied?

Same answer as the property-dependency thread below: isStepDependencySatisfied gates the whole step the same transitive way, so a step whose dependency is unmet has all of its properties treated as irrelevant, and materializing their defaults changes nothing. Covered by the test added in 5a5a34b.

…lt back-fill Confirms that materializing a required property's default for a property that is gated off by an unset controlling property does not make a transitively dependent required property report as required, because Connector dependency evaluation is transitive. Made-with: Cursor

Describe what the method does rather than implying it manages property dependencies. Made-with: Cursor

bobpaulin

+1 Non-Binding

NIFI-16242: Inherit default values for newly added Connector properties

46d5d9b

Made-with: Cursor

markap14 force-pushed the NIFI-16242 branch from a938f8f to 46d5d9b Compare August 21, 2026 19:06

ncover21 reviewed Aug 21, 2026

View reviewed changes

bobpaulin reviewed Aug 21, 2026

View reviewed changes

markap14 added 2 commits August 21, 2026 19:34

NIFI-16242: Clarify applyMissingRequiredPropertyDefaults Javadoc

7c7ef13

Describe what the method does rather than implying it manages property dependencies. Made-with: Cursor

bobpaulin approved these changes Aug 24, 2026

View reviewed changes

kevdoran approved these changes Aug 25, 2026

View reviewed changes

kevdoran merged commit e54dc5e into apache:main Aug 25, 2026
12 checks passed

		* upgrade that adds a required property with a default does not make the Connector invalid. Only required
		* properties are filled, so inheriting a default cannot activate a dependent property. A step the Connector

Uh oh!

Conversation

markap14 commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobpaulin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobpaulin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

markap14 commented Aug 21, 2026 •

edited

Loading