This is an automated email from the ASF dual-hosted git repository.
pvillard31 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new 22a071f2c79 NIFI-16114 - Fix version change bug when controller
service property has a parameter reference (#11432)
22a071f2c79 is described below
commit 22a071f2c79a632adabacea903642685caf99f3e
Author: Eric Secules <[email protected]>
AuthorDate: Wed Jul 29 13:40:19 2026 -0700
NIFI-16114 - Fix version change bug when controller service property has a
parameter reference (#11432)
---
.../StandardVersionedComponentSynchronizer.java | 20 ++-
...StandardVersionedComponentSynchronizerTest.java | 191 ++++++++++++++++++++-
.../ExternalControllerServiceVersioningIT.java | 53 ++++++
3 files changed, 255 insertions(+), 9 deletions(-)
diff --git
a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizer.java
b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizer.java
index 7315d8d441d..62c666d664c 100644
---
a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizer.java
+++
b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizer.java
@@ -96,11 +96,13 @@ import org.apache.nifi.groups.VersionedComponentAdditions;
import org.apache.nifi.logging.LogLevel;
import org.apache.nifi.migration.ControllerServiceFactory;
import org.apache.nifi.migration.StandardControllerServiceFactory;
+import org.apache.nifi.parameter.ExpressionLanguageAgnosticParameterParser;
import org.apache.nifi.parameter.Parameter;
import org.apache.nifi.parameter.ParameterContext;
import org.apache.nifi.parameter.ParameterContextManager;
import org.apache.nifi.parameter.ParameterContextNameUtils;
import org.apache.nifi.parameter.ParameterDescriptor;
+import org.apache.nifi.parameter.ParameterParser;
import org.apache.nifi.parameter.ParameterProviderConfiguration;
import org.apache.nifi.parameter.ParameterReferenceManager;
import org.apache.nifi.parameter.ParameterReferencedControllerServiceData;
@@ -164,6 +166,7 @@ public class StandardVersionedComponentSynchronizer
implements VersionedComponen
public static final String ENC_PREFIX = "enc{";
public static final String ENC_SUFFIX = "}";
+ private static final ParameterParser agnosticParameterParser = new
ExpressionLanguageAgnosticParameterParser();
private final VersionedFlowSynchronizationContext context;
private final Set<String> updatedVersionedComponentIds = new HashSet<>();
private final List<CreatedOrModifiedExtension>
createdAndModifiedExtensions = new ArrayList<>();
@@ -1666,11 +1669,11 @@ public class StandardVersionedComponentSynchronizer
implements VersionedComponen
|| (versionedDescriptor != null &&
versionedDescriptor.getIdentifiesControllerService());
final boolean sensitive = (descriptor != null &&
descriptor.isSensitive())
|| (versionedDescriptor != null &&
versionedDescriptor.isSensitive());
-
+ final String proposedValue =
proposedProperties.get(propertyName);
final String value;
- if (descriptor != null && referencesService &&
(proposedProperties.get(propertyName) != null)) {
+ if (descriptor != null && referencesService && proposedValue
!= null && !isReferencingParameter(proposedValue)) {
// Need to determine if the component's property
descriptor for this service is already set to an id
- // of an existing service that is outside the current
processor group, and if it is we want to leave
+ // of an existing service that is outside the current
processor group, and if it is, we want to leave
// the property set to that value
String existingExternalServiceId = null;
final String componentDescriptorValue =
componentNode.getEffectivePropertyValue(descriptor);
@@ -1687,9 +1690,8 @@ public class StandardVersionedComponentSynchronizer
implements VersionedComponen
// If the component's property descriptor is not already
set to an id of an existing external service,
// then we need to take the Versioned Component ID and
resolve this to the instance ID of the service
if (existingExternalServiceId == null) {
- final String serviceVersionedComponentId =
proposedProperties.get(propertyName);
- String instanceId =
getServiceInstanceId(serviceVersionedComponentId, group);
- value = (instanceId == null) ?
serviceVersionedComponentId : instanceId;
+ String instanceId =
getServiceInstanceId(proposedValue, group);
+ value = (instanceId == null) ? proposedValue :
instanceId;
// Find the same property descriptor in the
component's CreatedExtension and replace it with the
// instance ID of the service
@@ -1700,7 +1702,7 @@ public class StandardVersionedComponentSynchronizer
implements VersionedComponen
value = existingExternalServiceId;
}
} else {
- value = proposedProperties.get(propertyName);
+ value = proposedValue;
}
// skip any sensitive properties that are not populated so we
can retain whatever is currently set. We do this because sensitive properties
are not stored in the registry
@@ -1730,6 +1732,10 @@ public class StandardVersionedComponentSynchronizer
implements VersionedComponen
return fullPropertyMap;
}
+ private static boolean isReferencingParameter(String proposedValue) {
+ return
!agnosticParameterParser.parseTokens(proposedValue).toReferenceList().isEmpty();
+ }
+
private Map<String, String> getDecryptedProperties(final Map<String,
String> properties) {
final Map<String, String> decryptedProperties = new LinkedHashMap<>();
diff --git
a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizerTest.java
b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizerTest.java
index 0c2f28f7aed..b93ab48cb11 100644
---
a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizerTest.java
+++
b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/flow/synchronization/StandardVersionedComponentSynchronizerTest.java
@@ -1022,7 +1022,6 @@ public class StandardVersionedComponentSynchronizerTest {
}
@Test
- @SuppressWarnings("unchecked")
public void testExternalControllerServiceReferenceRemoved() throws
FlowSynchronizationException, InterruptedException, TimeoutException {
final PropertyDescriptor descriptorB = new
PropertyDescriptor.Builder().name("b").build();
final PropertyDescriptor descriptorCS = new
PropertyDescriptor.Builder().name("cs")
@@ -1064,7 +1063,7 @@ public class StandardVersionedComponentSynchronizerTest {
versionedProcessor.setPropertyDescriptors(proposedDescriptors);
versionedProcessor.setProperties(proposedProperties);
- final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.forClass(Map.class);
+ final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.captor();
synchronizer.synchronize(processorNode, versionedProcessor, group,
synchronizationOptions);
verify(processorNode).setProperties(captorProperties.capture(),
anyBoolean(), any());
final Map<String, String> properties = captorProperties.getValue();
@@ -1072,6 +1071,194 @@ public class StandardVersionedComponentSynchronizerTest
{
assertNull(properties.get("cs"));
}
+ @Test
+ public void testExternalControllerServiceParameterReferencePreserved()
throws FlowSynchronizationException, InterruptedException, TimeoutException {
+ // A controller-service-identifying property is configured with a
Parameter reference (#{svc}) that resolves to a
+ // controller service living outside this process group. The proposed
(versioned) flow references the property via
+ // the SAME parameter reference. Synchronizing must PRESERVE the
parameter reference.
+ //
+ // Bug: populatePropertiesMap resolves the effective value (#{svc} ->
concrete service id), finds that id as an
+ // existing external controller service, and pins the property to the
concrete instance id -- silently dropping the
+ // parameterization. On the next flow comparison this surfaces as
DifferenceType.PROPERTY_PARAMETERIZATION_REMOVED.
+ // This test asserts the correct behavior and therefore fails against
the current implementation.
+ final String parameterReference = "#{svc}";
+ final String externalServiceId = "external-service-id";
+
+ final PropertyDescriptor descriptorCS = new
PropertyDescriptor.Builder().name("cs")
+ .identifiesControllerService(ControllerService.class).build();
+
+ final Map<PropertyDescriptor, String> rawPropertyValues = new
HashMap<>();
+ rawPropertyValues.put(descriptorCS, parameterReference);
+
+ final VersionedPropertyDescriptor versionedDescriptorCS = new
VersionedPropertyDescriptor();
+ versionedDescriptorCS.setName(descriptorCS.getName());
+ final Map<String, VersionedPropertyDescriptor> proposedDescriptors =
new HashMap<>();
+ proposedDescriptors.put(versionedDescriptorCS.getName(),
versionedDescriptorCS);
+
+ final Map<PropertyDescriptor, PropertyConfiguration> propertiesBefore
= new HashMap<>();
+ propertiesBefore.put(descriptorCS, new
PropertyConfiguration(parameterReference, null, null, null));
+
+ final ProcessorNode processorNode = createMockProcessor();
+
when(processorNode.getPropertyDescriptor(eq("cs"))).thenReturn(descriptorCS);
+ when(processorNode.getProperties()).thenReturn(propertiesBefore);
+
when(processorNode.getRawPropertyValues()).thenReturn(rawPropertyValues);
+ // #{svc} resolves to the concrete id of a controller service outside
this group.
+
when(processorNode.getEffectivePropertyValue(eq(descriptorCS))).thenReturn(externalServiceId);
+
when(processorNode.isReferencingParameter(eq(descriptorCS.getName()))).thenReturn(true);
+
+ final ProcessGroup processGroup = processorNode.getProcessGroup();
+ final ProcessGroup processGroupParent = mock(ProcessGroup.class);
+ final ControllerServiceNode externalService =
createMockControllerService();
+ when(processGroup.getParent()).thenReturn(processGroupParent);
+ when(processGroupParent.findControllerService(eq(externalServiceId),
eq(false), eq(true))).thenReturn(externalService);
+
+ // Proposed flow references the property via the SAME parameter
reference.
+ final Map<String, String> proposedProperties = new HashMap<>();
+ proposedProperties.put("cs", parameterReference);
+ final VersionedProcessor versionedProcessor =
createMinimalVersionedProcessor();
+ versionedProcessor.setPropertyDescriptors(proposedDescriptors);
+ versionedProcessor.setProperties(proposedProperties);
+
+ final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.captor();
+ synchronizer.synchronize(processorNode, versionedProcessor, group,
synchronizationOptions);
+ verify(processorNode).setProperties(captorProperties.capture(),
anyBoolean(), any());
+ final Map<String, String> properties = captorProperties.getValue();
+
+ assertEquals(parameterReference, properties.get("cs"),
+ "Controller-service property configured with a Parameter
reference was flattened to the resolved service id");
+ }
+
+ @Test
+ public void testNonControllerServiceParameterReferencePreserved() throws
FlowSynchronizationException, InterruptedException, TimeoutException {
+ // A regular (non-CS-identifying) property is configured with a
Parameter reference (#{my_param}).
+ // The proposed (versioned) flow references the same parameter.
Synchronizing must preserve the
+ // parameter reference rather than substituting any resolved effective
value.
+ final String parameterReference = "#{my_param}";
+ final String effectiveValue = "resolved-value";
+
+ final PropertyDescriptor descriptorA = new
PropertyDescriptor.Builder().name("a").build();
+
+ final Map<PropertyDescriptor, String> rawPropertyValues = new
HashMap<>();
+ rawPropertyValues.put(descriptorA, parameterReference);
+
+ final VersionedPropertyDescriptor versionedDescriptorA = new
VersionedPropertyDescriptor();
+ versionedDescriptorA.setName(descriptorA.getName());
+ final Map<String, VersionedPropertyDescriptor> proposedDescriptors =
new HashMap<>();
+ proposedDescriptors.put(versionedDescriptorA.getName(),
versionedDescriptorA);
+
+ final Map<PropertyDescriptor, PropertyConfiguration> propertiesBefore
= new HashMap<>();
+ propertiesBefore.put(descriptorA, new
PropertyConfiguration(parameterReference, null, null, null));
+
+ final ProcessorNode processorNode = createMockProcessor();
+
when(processorNode.getPropertyDescriptor(eq("a"))).thenReturn(descriptorA);
+ when(processorNode.getProperties()).thenReturn(propertiesBefore);
+
when(processorNode.getRawPropertyValues()).thenReturn(rawPropertyValues);
+
when(processorNode.getEffectivePropertyValue(eq(descriptorA))).thenReturn(effectiveValue);
+
when(processorNode.isReferencingParameter(eq(descriptorA.getName()))).thenReturn(true);
+
+ final Map<String, String> proposedProperties = new HashMap<>();
+ proposedProperties.put("a", parameterReference);
+ final VersionedProcessor versionedProcessor =
createMinimalVersionedProcessor();
+ versionedProcessor.setPropertyDescriptors(proposedDescriptors);
+ versionedProcessor.setProperties(proposedProperties);
+
+ final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.captor();
+ synchronizer.synchronize(processorNode, versionedProcessor, group,
synchronizationOptions);
+ verify(processorNode).setProperties(captorProperties.capture(),
anyBoolean(), any());
+ final Map<String, String> properties = captorProperties.getValue();
+
+ assertEquals(parameterReference, properties.get("a"),
+ "Non-CS property configured with a Parameter reference should
preserve the reference, not resolve it");
+ }
+
+ @Test
+ public void testNonControllerServiceParameterChangedToHardCoded() throws
FlowSynchronizationException, InterruptedException, TimeoutException {
+ // When the proposed flow replaces a Parameter reference with a
hard-coded value on a non-CS property,
+ // the synchronizer must use the hard-coded proposed value, not the
old parameter reference.
+ final String parameterReference = "#{my_param}";
+ final String newExplicitValue = "new-explicit-value";
+
+ final PropertyDescriptor descriptorA = new
PropertyDescriptor.Builder().name("a").build();
+
+ final Map<PropertyDescriptor, String> rawPropertyValues = new
HashMap<>();
+ rawPropertyValues.put(descriptorA, parameterReference);
+
+ final VersionedPropertyDescriptor versionedDescriptorA = new
VersionedPropertyDescriptor();
+ versionedDescriptorA.setName(descriptorA.getName());
+ final Map<String, VersionedPropertyDescriptor> proposedDescriptors =
new HashMap<>();
+ proposedDescriptors.put(versionedDescriptorA.getName(),
versionedDescriptorA);
+
+ final Map<PropertyDescriptor, PropertyConfiguration> propertiesBefore
= new HashMap<>();
+ propertiesBefore.put(descriptorA, new
PropertyConfiguration(parameterReference, null, null, null));
+
+ final ProcessorNode processorNode = createMockProcessor();
+
when(processorNode.getPropertyDescriptor(eq("a"))).thenReturn(descriptorA);
+ when(processorNode.getProperties()).thenReturn(propertiesBefore);
+
when(processorNode.getRawPropertyValues()).thenReturn(rawPropertyValues);
+
when(processorNode.isReferencingParameter(eq(descriptorA.getName()))).thenReturn(true);
+
+ final Map<String, String> proposedProperties = new HashMap<>();
+ proposedProperties.put("a", newExplicitValue);
+ final VersionedProcessor versionedProcessor =
createMinimalVersionedProcessor();
+ versionedProcessor.setPropertyDescriptors(proposedDescriptors);
+ versionedProcessor.setProperties(proposedProperties);
+
+ final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.captor();
+ synchronizer.synchronize(processorNode, versionedProcessor, group,
synchronizationOptions);
+ verify(processorNode).setProperties(captorProperties.capture(),
anyBoolean(), any());
+ final Map<String, String> properties = captorProperties.getValue();
+
+ assertEquals(newExplicitValue, properties.get("a"),
+ "Non-CS property previously set via Parameter reference should
adopt the hard-coded value from the proposed flow");
+ }
+
+ @Test
+ public void testControllerServiceParameterChangedToHardCoded() throws
FlowSynchronizationException, InterruptedException, TimeoutException {
+ // When the proposed flow replaces a Parameter reference with a
hard-coded versioned component ID on
+ // a CS-identifying property, the synchronizer must resolve the
versioned ID to the local instance ID.
+ final String parameterReference = "#{svc}";
+ final String versionedCsId = "versioned-cs-uuid";
+
+ final PropertyDescriptor descriptorCS = new
PropertyDescriptor.Builder().name("cs")
+ .identifiesControllerService(ControllerService.class).build();
+
+ final Map<PropertyDescriptor, String> rawPropertyValues = new
HashMap<>();
+ rawPropertyValues.put(descriptorCS, parameterReference);
+
+ final VersionedPropertyDescriptor versionedDescriptorCS = new
VersionedPropertyDescriptor();
+ versionedDescriptorCS.setName(descriptorCS.getName());
+ final Map<String, VersionedPropertyDescriptor> proposedDescriptors =
new HashMap<>();
+ proposedDescriptors.put(versionedDescriptorCS.getName(),
versionedDescriptorCS);
+
+ final Map<PropertyDescriptor, PropertyConfiguration> propertiesBefore
= new HashMap<>();
+ propertiesBefore.put(descriptorCS, new
PropertyConfiguration(parameterReference, null, null, null));
+
+ final ProcessorNode processorNode = createMockProcessor();
+
when(processorNode.getPropertyDescriptor(eq("cs"))).thenReturn(descriptorCS);
+ when(processorNode.getProperties()).thenReturn(propertiesBefore);
+
when(processorNode.getRawPropertyValues()).thenReturn(rawPropertyValues);
+
when(processorNode.isReferencingParameter(eq(descriptorCS.getName()))).thenReturn(true);
+
+ // Wire up a local controller service that the versioned ID resolves
to.
+ final ControllerServiceNode localService =
createMockControllerService();
+
when(localService.getVersionedComponentId()).thenReturn(Optional.of(versionedCsId));
+
when(group.getControllerServices(false)).thenReturn(Set.of(localService));
+
+ final Map<String, String> proposedProperties = new HashMap<>();
+ proposedProperties.put("cs", versionedCsId);
+ final VersionedProcessor versionedProcessor =
createMinimalVersionedProcessor();
+ versionedProcessor.setPropertyDescriptors(proposedDescriptors);
+ versionedProcessor.setProperties(proposedProperties);
+
+ final ArgumentCaptor<Map<String, String>> captorProperties =
ArgumentCaptor.captor();
+ synchronizer.synchronize(processorNode, versionedProcessor, group,
synchronizationOptions);
+ verify(processorNode).setProperties(captorProperties.capture(),
anyBoolean(), any());
+ final Map<String, String> properties = captorProperties.getValue();
+
+ assertEquals(localService.getIdentifier(), properties.get("cs"),
+ "CS-identifying property previously set via Parameter
reference should be resolved to the local service instance ID");
+ }
+
@Test
public void testControllerServiceRemoved() throws
FlowSynchronizationException, InterruptedException, TimeoutException {
final ControllerServiceNode service = createMockControllerService();
diff --git
a/nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/registry/ExternalControllerServiceVersioningIT.java
b/nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/registry/ExternalControllerServiceVersioningIT.java
index 84d5d799136..b96ae82bb27 100644
---
a/nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/registry/ExternalControllerServiceVersioningIT.java
+++
b/nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/registry/ExternalControllerServiceVersioningIT.java
@@ -30,6 +30,7 @@ import org.apache.nifi.web.api.entity.ConnectionEntity;
import org.apache.nifi.web.api.entity.ControllerServiceEntity;
import org.apache.nifi.web.api.entity.FlowComparisonEntity;
import org.apache.nifi.web.api.entity.FlowRegistryClientEntity;
+import org.apache.nifi.web.api.entity.ParameterContextEntity;
import org.apache.nifi.web.api.entity.ProcessGroupEntity;
import org.apache.nifi.web.api.entity.ProcessGroupFlowEntity;
import org.apache.nifi.web.api.entity.ProcessorEntity;
@@ -385,6 +386,58 @@ public class ExternalControllerServiceVersioningIT extends
NiFiSystemIT {
"A migration-marked Controller Service must not be reported as
a local modification, but it was");
}
+ /**
+ * Reproduces Show Local Changes gap where a processor property that
identifies a Controller Service is set via
+ * a Parameter reference (#{svc}) pointing at a service defined outside
the versioned PG. Both
+ * v1 and v2 of the flow reference the service through the same Parameter.
+ * <p>
+ * When the PG is downgraded to v1 and then upgraded back to v2, the
synchronizer used to resolve
+ * "#{svc}" to the external service's concrete instance ID and pin the
property to that literal
+ * value, silently dropping the Parameter reference. This showed up as a
false "local
+ * modification" (Property Value Changed) even though the flow and the
versioned snapshot were
+ * otherwise identical.
+ */
+ @Test
+ public void
testExternalControllerServiceParameterReferencePreservedOnUpgrade() throws
NiFiClientException, IOException, InterruptedException {
+ final FlowRegistryClientEntity registryClient = registerClient();
+ final NiFiClientUtil util = getClientUtil();
+
+ final ControllerServiceEntity service =
util.createControllerService(COUNT_SERVICE_TYPE, "root");
+ util.enableControllerService(service);
+
+ final ParameterContextEntity paramContext =
util.createParameterContext(
+ "svc-context", Collections.singletonMap("svc",
service.getComponent().getId()));
+
+ final ProcessGroupEntity child = util.createProcessGroup("Child",
"root");
+ util.setParameterContext(child.getId(), paramContext);
+
+ ProcessorEntity counter = util.createProcessor("CountFlowFiles",
child.getId());
+ util.updateProcessorProperties(counter,
Collections.singletonMap("Count Service", "#{svc}"));
+ final ProcessorEntity terminate =
util.createProcessor("TerminateFlowFile", child.getId());
+ util.createConnection(counter, terminate, "success");
+
+ final VersionControlInformationEntity vci =
util.startVersionControl(child, registryClient, TEST_FLOWS_BUCKET,
"param-ref-external-cs");
+ util.assertFlowUpToDate(child.getId());
+
+ // v2 differs only in scheduling period. "Count Service" remains
#{svc} in both versions.
+ counter = util.updateProcessorSchedulingPeriod(counter, "10 sec");
+ util.saveFlowVersion(child, registryClient, vci);
+ util.assertFlowUpToDate(child.getId());
+
+ // Downgrade then upgrade to force the synchronizer to re-resolve the
property, which is
+ // the code path affected by NIFI-16114.
+ util.changeFlowVersion(child.getId(), "1");
+ util.changeFlowVersion(child.getId(), "2");
+
+ final ProcessorEntity refreshed =
getNifiClient().getProcessorClient().getProcessor(counter.getId());
+ assertEquals("#{svc}",
refreshed.getComponent().getConfig().getProperties().get("Count Service"),
+ "Parameter reference to external Controller Service should
survive version upgrade, not be flattened to the service's instance ID");
+
+ final FlowComparisonEntity localMods =
getNifiClient().getProcessGroupClient().getLocalModifications(child.getId());
+ assertTrue(localMods.getComponentDifferences().isEmpty(),
+ "Show Local Changes should report no differences after
upgrading between versions that both reference the service via the same
Parameter");
+ }
+
/**
* Deletes only the connections and processors within a Process Group,
without touching
* Controller Services (which may be inherited from ancestor groups).
