diff --git a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java index dc46a0997037..91b64ee0b335 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java +++ b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java @@ -68,13 +68,9 @@ import org.apache.nifi.controller.service.StandardConfigurationContext; import org.apache.nifi.encrypt.PropertyEncryptor; import org.apache.nifi.flow.ExecutionEngine; -import org.apache.nifi.flow.ExternalControllerServiceReference; import org.apache.nifi.flow.VersionedComponent; -import org.apache.nifi.flow.VersionedControllerService; import org.apache.nifi.flow.VersionedExternalFlow; import org.apache.nifi.flow.VersionedProcessGroup; -import org.apache.nifi.flow.VersionedProcessor; -import org.apache.nifi.flow.VersionedPropertyDescriptor; import org.apache.nifi.flow.synchronization.StandardVersionedComponentSynchronizer; import org.apache.nifi.flow.synchronization.VersionedFlowSynchronizationContext; import org.apache.nifi.lifecycle.ProcessorStopLifecycleMethods; @@ -3867,7 +3863,6 @@ public void synchronizeWithFlowRegistry(final FlowManager flowManager) { final FlowSnapshotContainer registrySnapshotContainer = flowRegistry.getFlowContents( FlowRegistryClientContextFactory.getAnonymousContext(), flowVersionLocation, false); final RegisteredFlowSnapshot registrySnapshot = registrySnapshotContainer.getFlowSnapshot(); - resolveExternalServiceReferences(registrySnapshot); final VersionedProcessGroup registryFlow = registrySnapshot.getFlowContents(); vci.setFlowSnapshot(registryFlow); } catch (final IOException | FlowRegistryException e) { @@ -4091,81 +4086,6 @@ private String generateUuid(final String proposedId, final String destinationGro return uuid.toString(); } - private void resolveExternalServiceReferences(final RegisteredFlowSnapshot snapshot) { - final Map externalRefs = snapshot.getExternalControllerServices(); - if (externalRefs == null || externalRefs.isEmpty()) { - return; - } - - final ProcessGroup parentGroup = getParent(); - if (parentGroup == null) { - return; - } - - final Map serviceNameToVersionedId = new HashMap<>(); - for (final ControllerServiceNode serviceNode : parentGroup.getControllerServices(true)) { - final String versionedId = serviceNode.getVersionedComponentId().orElse( - VersionedComponentFlowMapper.generateVersionedComponentId(serviceNode.getIdentifier())); - serviceNameToVersionedId.put(serviceNode.getName(), versionedId); - } - - final Map foreignToLocalId = new HashMap<>(); - for (final Map.Entry entry : externalRefs.entrySet()) { - final String foreignId = entry.getKey(); - final String serviceName = entry.getValue().getName(); - final String localId = serviceNameToVersionedId.get(serviceName); - if (localId != null && !localId.equals(foreignId)) { - foreignToLocalId.put(foreignId, localId); - } - } - - if (!foreignToLocalId.isEmpty()) { - replaceExternalServiceIds(snapshot.getFlowContents(), foreignToLocalId); - } - } - - private void replaceExternalServiceIds(final VersionedProcessGroup group, final Map foreignToLocalId) { - if (group.getProcessors() != null) { - for (final VersionedProcessor processor : group.getProcessors()) { - replaceServicePropertyIds(processor.getProperties(), processor.getPropertyDescriptors(), foreignToLocalId); - } - } - - if (group.getControllerServices() != null) { - for (final VersionedControllerService service : group.getControllerServices()) { - replaceServicePropertyIds(service.getProperties(), service.getPropertyDescriptors(), foreignToLocalId); - } - } - - if (group.getProcessGroups() != null) { - for (final VersionedProcessGroup child : group.getProcessGroups()) { - replaceExternalServiceIds(child, foreignToLocalId); - } - } - } - - private void replaceServicePropertyIds(final Map properties, final Map descriptors, - final Map foreignToLocalId) { - if (properties == null || descriptors == null) { - return; - } - - for (final Map.Entry entry : properties.entrySet()) { - final String propertyValue = entry.getValue(); - if (propertyValue == null) { - continue; - } - - final VersionedPropertyDescriptor descriptor = descriptors.get(entry.getKey()); - if (descriptor != null && descriptor.getIdentifiesControllerService()) { - final String localId = foreignToLocalId.get(propertyValue); - if (localId != null) { - entry.setValue(localId); - } - } - } - } - private Set getModifications() { final StandardVersionControlInformation vci = versionControlInfo.get(); @@ -4199,7 +4119,7 @@ private Set getModifications() { final FlowComparison comparison = flowComparator.compare(); final Collection comparisonDifferences = comparison.getDifferences(); final FlowDifferenceFilters.EnvironmentalChangeContext environmentalContext = - FlowDifferenceFilters.buildEnvironmentalChangeContext(comparisonDifferences, flowManager); + FlowDifferenceFilters.buildEnvironmentalChangeContext(comparisonDifferences, versionedGroup, flowManager); final Set differences = comparisonDifferences.stream() .filter(difference -> !FlowDifferenceFilters.isEnvironmentalChange(difference, versionedGroup, flowManager, environmentalContext)) diff --git a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/FlowDifferenceFilters.java b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/FlowDifferenceFilters.java index f2d9fa8982d7..3a0405e67827 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/FlowDifferenceFilters.java +++ b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/FlowDifferenceFilters.java @@ -48,7 +48,9 @@ import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedComponent; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedConnection; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedControllerService; +import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedProcessGroup; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedProcessor; +import org.apache.nifi.registry.flow.mapping.VersionedComponentFlowMapper; import java.util.ArrayList; import java.util.Collection; @@ -104,7 +106,8 @@ public static boolean isEnvironmentalChange(final FlowDifference difference, fin || isPropertyParameterizationRename(difference, evaluatedContext) || isPropertyRenameWithMatchingValue(difference, evaluatedContext) || isSelectedRelationshipChangeForNewRelationship(difference, flowManager) - || isPropertyAddedFromMigration(difference, flowManager); + || isPropertyAddedFromMigration(difference, flowManager) + || isExternalControllerServiceReferenceChange(difference, flowManager, evaluatedContext); } /** @@ -706,10 +709,17 @@ private static boolean isLogFileSuffixChange(final FlowDifference flowDifference } public static EnvironmentalChangeContext buildEnvironmentalChangeContext(final Collection differences, final FlowManager flowManager) { + return buildEnvironmentalChangeContext(differences, null, flowManager); + } + + public static EnvironmentalChangeContext buildEnvironmentalChangeContext(final Collection differences, final VersionedProcessGroup localGroup, + final FlowManager flowManager) { if (differences == null || differences.isEmpty() || flowManager == null) { return EnvironmentalChangeContext.empty(); } + final Set ancestorControllerServiceIds = computeAncestorControllerServiceIds(localGroup, flowManager); + final Map> parameterizedAddsByComponent = new HashMap<>(); final Map> parameterizationRemovalsByComponent = new HashMap<>(); final Map> controllerServicePropertyAddsByValue = new HashMap<>(); @@ -855,11 +865,83 @@ && isControllerServiceProperty(difference, flowManager)) { } } - if (serviceIdsWithMatchingAdditions.isEmpty() && parameterizedPropertyRenameDifferences.isEmpty() && propertyRenamesWithMatchingValues.isEmpty()) { + if (serviceIdsWithMatchingAdditions.isEmpty() && parameterizedPropertyRenameDifferences.isEmpty() && propertyRenamesWithMatchingValues.isEmpty() + && ancestorControllerServiceIds.isEmpty()) { return EnvironmentalChangeContext.empty(); } - return new EnvironmentalChangeContext(serviceIdsWithMatchingAdditions, parameterizedPropertyRenameDifferences, propertyRenamesWithMatchingValues); + return new EnvironmentalChangeContext(serviceIdsWithMatchingAdditions, parameterizedPropertyRenameDifferences, propertyRenamesWithMatchingValues, + ancestorControllerServiceIds); + } + + /** + * Determines whether a Flow Difference represents a change to a property that references an external controller service + * (one defined outside the versioned Process Group, e.g. in an ancestor group). Such a reference is environment-specific: the same + * logical service typically has a different identifier in each environment, so pointing the property at a different external service + * is not treated as a local modification. The change is environmental only when the versioned-snapshot value does not resolve to a + * locally-accessible ancestor service while the local value does. + */ + private static boolean isExternalControllerServiceReferenceChange(final FlowDifference difference, final FlowManager flowManager, + final EnvironmentalChangeContext context) { + if (difference.getDifferenceType() != DifferenceType.PROPERTY_CHANGED) { + return false; + } + + final Set ancestorServiceIds = context.ancestorControllerServiceIds(); + if (ancestorServiceIds.isEmpty()) { + return false; + } + + if (!isControllerServiceProperty(difference, flowManager)) { + return false; + } + + final Optional snapshotValue = getPropertyValue(difference, true); + final Optional localValue = getPropertyValue(difference, false); + if (snapshotValue.isEmpty() || localValue.isEmpty()) { + return false; + } + + // Parameter-referenced controller services are environment-portable through Parameter Contexts and are not evaluated here. + if (isParameterReference(snapshotValue.get()) || isParameterReference(localValue.get())) { + return false; + } + + // Environmental only when the snapshot reference is not a locally-accessible ancestor controller service but the local reference + // is. A switch between two locally-accessible ancestor services remains a reported change, and a reference to a service defined + // inside the versioned Process Group is never in the ancestor set, so it always remains a reported change. + final boolean snapshotAccessible = ancestorServiceIds.contains(snapshotValue.get()); + final boolean localAccessible = ancestorServiceIds.contains(localValue.get()); + return !snapshotAccessible && localAccessible; + } + + /** + * Computes the set of versioned component identifiers for all controller services accessible from the ancestors of the given (local) + * versioned Process Group -- i.e. services defined in the parent group and above. These identify references to external controller + * services. Returns an empty set when the group is not an instantiated (live-backed) group, so callers that pass a plain snapshot + * group get no suppression. + */ + private static Set computeAncestorControllerServiceIds(final VersionedProcessGroup localGroup, final FlowManager flowManager) { + if (flowManager == null || !(localGroup instanceof InstantiatedVersionedProcessGroup instantiatedGroup)) { + return Collections.emptySet(); + } + + final ProcessGroup liveGroup = flowManager.getGroup(instantiatedGroup.getInstanceIdentifier()); + if (liveGroup == null) { + return Collections.emptySet(); + } + + final ProcessGroup parentGroup = liveGroup.getParent(); + if (parentGroup == null) { + return Collections.emptySet(); + } + + final Set ancestorServiceIds = new HashSet<>(); + for (final ControllerServiceNode serviceNode : parentGroup.getControllerServices(true)) { + ancestorServiceIds.add(serviceNode.getVersionedComponentId().orElseGet( + () -> VersionedComponentFlowMapper.generateVersionedComponentId(serviceNode.getIdentifier()))); + } + return ancestorServiceIds; } public static boolean isControllerServiceCreatedForNewProperty(final FlowDifference difference, final EnvironmentalChangeContext context) { @@ -1212,18 +1294,22 @@ FlowDifference difference() { } public static final class EnvironmentalChangeContext { - private static final EnvironmentalChangeContext EMPTY = new EnvironmentalChangeContext(Collections.emptySet(), Collections.emptySet(), Collections.emptySet()); + private static final EnvironmentalChangeContext EMPTY = + new EnvironmentalChangeContext(Collections.emptySet(), Collections.emptySet(), Collections.emptySet(), Collections.emptySet()); private final Set serviceIdsCreatedForNewProperties; private final Set parameterizedPropertyRenames; private final Set propertyRenamesWithMatchingValues; + private final Set ancestorControllerServiceIds; private EnvironmentalChangeContext(final Set serviceIdsCreatedForNewProperties, final Set parameterizedPropertyRenames, - final Set propertyRenamesWithMatchingValues) { + final Set propertyRenamesWithMatchingValues, + final Set ancestorControllerServiceIds) { this.serviceIdsCreatedForNewProperties = Collections.unmodifiableSet(new HashSet<>(serviceIdsCreatedForNewProperties)); this.parameterizedPropertyRenames = Collections.unmodifiableSet(new HashSet<>(parameterizedPropertyRenames)); this.propertyRenamesWithMatchingValues = Collections.unmodifiableSet(new HashSet<>(propertyRenamesWithMatchingValues)); + this.ancestorControllerServiceIds = Collections.unmodifiableSet(new HashSet<>(ancestorControllerServiceIds)); } static EnvironmentalChangeContext empty() { @@ -1241,5 +1327,9 @@ Set parameterizedPropertyRenames() { Set propertyRenamesWithMatchingValues() { return propertyRenamesWithMatchingValues; } + + Set ancestorControllerServiceIds() { + return ancestorControllerServiceIds; + } } } diff --git a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/util/TestFlowDifferenceFilters.java b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/util/TestFlowDifferenceFilters.java index dfb672b0c05d..a7df4dacfb51 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/util/TestFlowDifferenceFilters.java +++ b/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/test/java/org/apache/nifi/util/TestFlowDifferenceFilters.java @@ -44,6 +44,7 @@ import org.apache.nifi.registry.flow.diff.StandardFlowDifference; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedConnection; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedControllerService; +import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedProcessGroup; import org.apache.nifi.registry.flow.mapping.InstantiatedVersionedProcessor; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -51,6 +52,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -393,6 +395,94 @@ public void testControllerServiceCreationEnvironmentalChangeWithoutComponentNode assertTrue(FlowDifferenceFilters.isEnvironmentalChange(controllerServiceDifference, null, flowManager, context)); } + /** + * Changing a processor property that references an external (ancestor) controller service is not reported as a local + * modification, because the same logical service typically has a different identifier in each environment. This exercises the + * directional rule over the set of accessible ancestor controller service ids: suppression happens only when the versioned-snapshot + * reference is not a locally-accessible ancestor service while the local reference is. + */ + @Test + public void testExternalControllerServiceReferenceChangeDirectionalRule() { + final String pgInstanceId = "pg-instance"; + final String parentGroupId = "parent-group"; + final String propertyName = "SSL Context Service"; + + final String ancestorServiceX = "ancestor-service-x"; + final String ancestorServiceY = "ancestor-service-y"; + final String foreignServiceId = "foreign-service-id"; + final String internalServiceId = "internal-service-id"; + + // Two controller services defined in the parent (ancestor) group, i.e. external to the versioned Process Group. + final ControllerServiceNode serviceX = Mockito.mock(ControllerServiceNode.class); + Mockito.when(serviceX.getVersionedComponentId()).thenReturn(Optional.of(ancestorServiceX)); + final ControllerServiceNode serviceY = Mockito.mock(ControllerServiceNode.class); + Mockito.when(serviceY.getVersionedComponentId()).thenReturn(Optional.of(ancestorServiceY)); + + final ProcessGroup parentGroup = Mockito.mock(ProcessGroup.class); + Mockito.when(parentGroup.getControllerServices(true)).thenReturn(Set.of(serviceX, serviceY)); + + final ProcessGroup liveGroup = Mockito.mock(ProcessGroup.class); + Mockito.when(liveGroup.getParent()).thenReturn(parentGroup); + + final FlowManager flowManager = Mockito.mock(FlowManager.class); + Mockito.when(flowManager.getGroup(pgInstanceId)).thenReturn(liveGroup); + + final InstantiatedVersionedProcessGroup localGroup = new InstantiatedVersionedProcessGroup(pgInstanceId, parentGroupId); + + // !accessibleA && accessibleB -> environmental: snapshot points at a service that does not exist locally + // (e.g. a dev-environment id), local points at an existing ancestor service. + assertExternalServiceChange(localGroup, flowManager, parentGroupId, propertyName, foreignServiceId, ancestorServiceX, true, + "Switching from an unavailable external service to a local ancestor service must be treated as environmental"); + + // accessibleA && accessibleB -> NOT environmental: a genuine switch between two services that both exist locally. + assertExternalServiceChange(localGroup, flowManager, parentGroupId, propertyName, ancestorServiceY, ancestorServiceX, false, + "Switching between two locally-accessible ancestor services must remain a reported change"); + + // accessibleA && !accessibleB -> NOT environmental: local points at a service that is not an accessible ancestor (e.g. one defined + // inside the versioned Process Group), so the change must surface. + assertExternalServiceChange(localGroup, flowManager, parentGroupId, propertyName, ancestorServiceX, internalServiceId, false, + "Switching to a service that is not an accessible ancestor (e.g. internal to the group) must remain a reported change"); + + // !accessibleA && !accessibleB -> NOT environmental. + assertExternalServiceChange(localGroup, flowManager, parentGroupId, propertyName, "foreign-1", "foreign-2", false, + "A change between two non-ancestor services must remain a reported change"); + } + + private void assertExternalServiceChange(final InstantiatedVersionedProcessGroup localGroup, final FlowManager flowManager, + final String groupId, final String propertyName, final String snapshotValue, + final String localValue, final boolean expectedEnvironmental, final String message) { + final VersionedPropertyDescriptor descriptor = new VersionedPropertyDescriptor(); + descriptor.setName(propertyName); + descriptor.setDisplayName(propertyName); + descriptor.setDynamic(false); + descriptor.setIdentifiesControllerService(true); + + final InstantiatedVersionedProcessor snapshotProcessor = new InstantiatedVersionedProcessor("processor-instance", groupId); + snapshotProcessor.setComponentType(ComponentType.PROCESSOR); + snapshotProcessor.setIdentifier("processor-instance"); + snapshotProcessor.setProperties(Map.of(propertyName, snapshotValue)); + snapshotProcessor.setPropertyDescriptors(Map.of(propertyName, descriptor)); + + final InstantiatedVersionedProcessor localProcessor = new InstantiatedVersionedProcessor("processor-instance", groupId); + localProcessor.setComponentType(ComponentType.PROCESSOR); + localProcessor.setIdentifier("processor-instance"); + localProcessor.setProperties(Map.of(propertyName, localValue)); + localProcessor.setPropertyDescriptors(Map.of(propertyName, descriptor)); + + final FlowDifference difference = new StandardFlowDifference( + DifferenceType.PROPERTY_CHANGED, snapshotProcessor, localProcessor, propertyName, snapshotValue, localValue, + "Property '" + propertyName + "' changed"); + + final FlowDifferenceFilters.EnvironmentalChangeContext context = + FlowDifferenceFilters.buildEnvironmentalChangeContext(List.of(difference), localGroup, flowManager); + + if (expectedEnvironmental) { + assertTrue(FlowDifferenceFilters.isEnvironmentalChange(difference, localGroup, flowManager, context), message); + } else { + assertFalse(FlowDifferenceFilters.isEnvironmentalChange(difference, localGroup, flowManager, context), message); + } + } + @Test public void testPropertyRenameWithParameterizationObservedAsEnvironmentalChange() { final FlowManager flowManager = Mockito.mock(FlowManager.class); diff --git a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java index 4a50049e5dec..777a7cbba4c4 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java +++ b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java @@ -6525,11 +6525,6 @@ public FlowComparisonEntity getLocalModifications(final String processGroupId) { final FlowSnapshotContainer flowSnapshotContainer = flowRegistry.getFlowContents(FlowRegistryClientContextFactory.getContextForUser(NiFiUserUtils.getNiFiUser()), flowVersionLocation, true); - // Resolve external controller service references by name so that cross-instance - // ID differences do not appear as phantom local modifications. - final String parentGroupId = processGroup.getParent() == null ? processGroup.getIdentifier() : processGroup.getParent().getIdentifier(); - controllerFacade.getControllerServiceResolver().resolveInheritedControllerServices(flowSnapshotContainer, parentGroupId, NiFiUserUtils.getNiFiUser()); - final RegisteredFlowSnapshot versionedFlowSnapshot = flowSnapshotContainer.getFlowSnapshot(); registryGroup = versionedFlowSnapshot.getFlowContents(); } catch (final IOException | FlowRegistryException e) { diff --git a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java index 572978c01997..c66eb89d60f4 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java +++ b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java @@ -2856,7 +2856,7 @@ public Set createComponentDifferenceDtosForLocalModifica final Collection comparisonDifferences = comparison.getDifferences(); final FlowDifferenceFilters.EnvironmentalChangeContext environmentalContext = - FlowDifferenceFilters.buildEnvironmentalChangeContext(comparisonDifferences, flowManager); + FlowDifferenceFilters.buildEnvironmentalChangeContext(comparisonDifferences, localGroup, flowManager); for (final FlowDifference difference : comparisonDifferences) { // capture bundle differences and dedupe those differences 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 d74000f79357..1ee3a7a029a8 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 @@ -61,9 +61,9 @@ public class ExternalControllerServiceVersioningIT extends NiFiSystemIT { * then the original PG and service are removed and a new service with the same name (but * different ID) is created before re-importing from the registry. * - * After import, the flow should be UP_TO_DATE because the external service was resolved - * by name during import and the cached snapshot should also be resolved. - * Both the state badge and the "Show Local Changes" dialog should agree. + * After import, the flow should be UP_TO_DATE: the committed reference points at the (now-removed) dev service id, which + * is not a locally-accessible ancestor service, while the local reference points at the same-named prod service, so the + * external-service reference change is treated as environment-specific. Both the badge and the dialog should agree. */ @Test public void testCrossInstanceImportWithExternalServiceShowsUpToDate() throws NiFiClientException, IOException, InterruptedException { @@ -106,6 +106,57 @@ public void testCrossInstanceImportWithExternalServiceShowsUpToDate() throws NiF "After cross-instance import, Show Local Changes should report no differences"); } + /** + * A flow committed against one external service is imported into an instance whose equivalent external service has a + * DIFFERENT name (and a different id). After pointing the processor at the local service, the flow is UP_TO_DATE with no + * local modifications, because an external controller service reference is environment-specific regardless of its name or id. + */ + @Test + public void testCrossInstanceImportWithDifferentlyNamedExternalServiceShowsUpToDate() throws NiFiClientException, IOException, InterruptedException { + final FlowRegistryClientEntity registryClient = registerClient(); + final NiFiClientUtil util = getClientUtil(); + + final ControllerServiceEntity devService = util.createControllerService(COUNT_SERVICE_TYPE, "root"); + util.enableControllerService(devService); + + final ProcessGroupEntity child = util.createProcessGroup("Child", "root"); + final ProcessorEntity counter = util.createProcessor("CountFlowFiles", child.getId()); + util.updateProcessorProperties(counter, Collections.singletonMap("Count Service", devService.getComponent().getId())); + final ProcessorEntity terminate = util.createProcessor("TerminateFlowFile", child.getId()); + util.createConnection(counter, terminate, "success"); + + final VersionControlInformationEntity vci = util.startVersionControl(child, registryClient, TEST_FLOWS_BUCKET, "cross-instance-diff-name-flow"); + util.assertFlowUpToDate(child.getId()); + final VersionControlInformationDTO vciDto = vci.getVersionControlInformation(); + + getNifiClient().getVersionsClient().stopVersionControl( + getNifiClient().getProcessGroupClient().getProcessGroup(child.getId())); + deleteProcessGroupContents(child.getId()); + getNifiClient().getProcessGroupClient().deleteProcessGroup( + getNifiClient().getProcessGroupClient().getProcessGroup(child.getId())); + + deleteControllerService(devService); + + ControllerServiceEntity prodService = util.createControllerService(COUNT_SERVICE_TYPE, "root"); + prodService = renameControllerService(prodService, "DifferentlyNamedCountService"); + assertNotEquals(devService.getComponent().getId(), prodService.getComponent().getId(), + "Prod service should have a different ID than dev service"); + util.enableControllerService(prodService); + + final ProcessGroupEntity imported = util.importFlowFromRegistry("root", vciDto.getRegistryId(), + vciDto.getBucketId(), vciDto.getFlowId(), vciDto.getVersion()); + + // Point the imported processor at the differently-named local external service. + final ProcessorEntity importedCounter = findProcessorByType(imported.getId(), "CountFlowFiles"); + util.updateProcessorProperties(importedCounter, Collections.singletonMap("Count Service", prodService.getComponent().getId())); + + waitForVersionedFlowState(imported.getId(), "root", "UP_TO_DATE"); + + final FlowComparisonEntity localMods = getNifiClient().getProcessGroupClient().getLocalModifications(imported.getId()); + assertTrue(localMods.getComponentDifferences().isEmpty(), + "After pointing at a differently-named external service, Show Local Changes should report no differences"); + } + /** * Simulates a cross-instance upgrade where v1 and v2 both reference the same external * service but differ in a non-service property (scheduling period). The original PG and @@ -167,17 +218,16 @@ public void testCrossInstanceUpgradeWithExternalServiceShowsUpToDate() throws Ni } /** - * Reproduces the NIFI-15697 scenario on a single instance: - * - * 1. Create an external service "StandardCountService" and a child PG referencing it, commit. - * 2. Create a second external service "AlternateCountService", switch the processor to it. - * 3. Delete the original service. - * 4. Verify that both the state badge (LOCALLY_MODIFIED) and the dialog (shows the change) agree. + * Switching a processor's reference from one external service to another and then deleting the originally-referenced service + * keeps the state badge and the "Show Local Changes" dialog consistent at every step. * - * The two services have different names so the name-based resolver cannot falsely reconcile them. + * While both services exist, the switch is a genuine, reported local change (both are locally-accessible ancestor services). + * Once the originally-referenced service is removed, the committed reference no longer resolves to a local service, so the + * change becomes environment-specific and the flow returns to UP_TO_DATE. The badge and the dialog agree throughout -- there + * is never a badge that reports LOCALLY_MODIFIED while the dialog reports no changes. */ @Test - public void testSwitchExternalServiceAndDeleteOriginalShowsLocalModification() throws NiFiClientException, IOException, InterruptedException { + public void testSwitchExternalServiceAndDeleteOriginalKeepsBadgeAndDialogConsistent() throws NiFiClientException, IOException, InterruptedException { final FlowRegistryClientEntity registryClient = registerClient(); final NiFiClientUtil util = getClientUtil(); @@ -199,22 +249,19 @@ public void testSwitchExternalServiceAndDeleteOriginalShowsLocalModification() t util.updateProcessorProperties(counter, Collections.singletonMap("Count Service", serviceB.getComponent().getId())); - String state = util.getVersionedFlowState(child.getId(), "root"); - assertEquals("LOCALLY_MODIFIED", state, "After switching external service reference, PG should be LOCALLY_MODIFIED"); + // While both external services exist, switching between them is a genuine local change; badge and dialog must agree. + final String stateAfterSwitch = util.getVersionedFlowState(child.getId(), "root"); + assertEquals("LOCALLY_MODIFIED", stateAfterSwitch, "While both external services exist, switching the reference should be LOCALLY_MODIFIED"); + assertFalse(getNifiClient().getProcessGroupClient().getLocalModifications(child.getId()).getComponentDifferences().isEmpty(), + "Badge is LOCALLY_MODIFIED, so the dialog must also report differences"); deleteControllerService(serviceA); - state = util.getVersionedFlowState(child.getId(), "root"); - assertEquals("LOCALLY_MODIFIED", state, "After deleting original service, PG should still be LOCALLY_MODIFIED"); - - final FlowComparisonEntity localMods = getNifiClient().getProcessGroupClient().getLocalModifications(child.getId()); - assertFalse(localMods.getComponentDifferences().isEmpty(), "Show Local Changes should report differences"); - - final boolean hasPropertyValueChange = localMods.getComponentDifferences().stream() - .flatMap(dto -> dto.getDifferences().stream()) - .map(DifferenceDTO::getDifferenceType) - .anyMatch(type -> type.contains("Property Value Changed")); - assertTrue(hasPropertyValueChange, "Differences should include a Property Value Changed difference"); + // Once the originally-referenced external service is gone, the reference change is environment-specific: the flow + // returns to UP_TO_DATE and the dialog reports no differences. Badge and dialog agree (no stuck LOCALLY_MODIFIED). + waitForVersionedFlowState(child.getId(), "root", "UP_TO_DATE"); + assertTrue(getNifiClient().getProcessGroupClient().getLocalModifications(child.getId()).getComponentDifferences().isEmpty(), + "After removing the original external service, the dialog must report no differences (consistent with the UP_TO_DATE badge)"); } /** @@ -262,6 +309,14 @@ private void deleteProcessGroupContents(final String groupId) throws NiFiClientE } } + private ProcessorEntity findProcessorByType(final String groupId, final String simpleType) throws NiFiClientException, IOException { + final ProcessGroupFlowEntity flowEntity = getNifiClient().getFlowClient().getProcessGroup(groupId); + return flowEntity.getProcessGroupFlow().getFlow().getProcessors().stream() + .filter(processor -> processor.getComponent().getType().endsWith(simpleType)) + .findFirst() + .orElseThrow(() -> new AssertionError("Could not find processor of type " + simpleType + " in group " + groupId)); + } + private ControllerServiceEntity renameControllerService(final ControllerServiceEntity service, final String newName) throws NiFiClientException, IOException { final ControllerServiceDTO dto = new ControllerServiceDTO();