NIFI-15697 - Fix inconsistent versioning state when using external controller services in versioned Process Groups#11006
Open
pvillard31 wants to merge 1 commit intoapache:mainfrom
Open
NIFI-15697 - Fix inconsistent versioning state when using external controller services in versioned Process Groups#11006pvillard31 wants to merge 1 commit intoapache:mainfrom
pvillard31 wants to merge 1 commit intoapache:mainfrom
Conversation
…ntroller services in versioned Process Groups
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NIFI-15697 - Fix inconsistent versioning state when using external controller services in versioned Process Groups
Fixes the inconsistency where a versioned Process Group is marked as "Locally Modified" but the "Show Local Changes" dialog shows no changes, specifically when external controller services are involved.
Root Cause
The bug is caused by three interacting problems:
Missing resolution in "Show Local Changes" path:
getLocalModifications()inStandardNiFiServiceFacadefetches the original unresolved snapshot from the registry but does not run the controller service resolver before comparing. The state badge path (getModifications()inStandardProcessGroup) uses the locally stored snapshot which was already resolved at import time. This means the two paths can produce different results.Flawed suppression in
StandardFlowComparator.compareProperties(): A suppression block attempted to hide differences when a controller service property changed from an inaccessible ID (e.g., foreign UUID from another instance) to an accessible external service. However, it cannot distinguish "foreign ID from another NiFi instance" from "ID of a local service that was deleted," causing it to incorrectly swallow real modifications.Stale cached differences:
removeControllerService()inStandardProcessGrouponly notifies Process Groups whose components currently reference the deleted service. If a processor was already switched to a different service before the old one was deleted, the cache is not invalidated.These three problems combine in the JIRA scenario: the user commits a PG referencing external service X, changes the processor to reference service Y (PG cached as LOCALLY_MODIFIED), then deletes service X. The cache is not invalidated (problem 3), so the badge still shows LOCALLY_MODIFIED. But "Show Local Changes" recomputes from scratch using the unresolved registry snapshot, and the suppression (problem 2) swallows the X→Y difference because X is no longer accessible — the dialog shows no changes.
Changes
1. Resolve external services in
getLocalModifications()before comparingIn
StandardNiFiServiceFacade.getLocalModifications(), after fetching the registry snapshot, we now callcontrollerServiceResolver.resolveInheritedControllerServices()before comparing. This normalizes external service IDs by name (and API compatibility), exactly like the import/upgrade path already does.2. Resolve external services in
synchronizeWithFlowRegistry()before cachingIn
StandardProcessGroup.synchronizeWithFlowRegistry(), added inline name-based resolution of external service references (resolveExternalServiceReferences()) on the registry snapshot before it is stored as the cached flow snapshot. This ensures the state badge path uses resolved IDs that match the local flow, preventing falseLOCALLY_MODIFIEDstates after cross-instance imports or NiFi restarts.3. Remove the
externallyAccessibleServiceIdssuppression fromStandardFlowComparatorRemoved the
externallyAccessibleServiceIdsfield, constructor parameter, and the suppression block incompareProperties(). This suppression is now redundant because fixes 1 and 2 handle the cross-instance case properly via name-based resolution. RemovedgetAncestorServiceIds()fromStandardProcessGroupand theProcessGroupinterface, and updated allStandardFlowComparatorcall sites accordingly.4. Improve cache invalidation in
removeControllerService()In
StandardProcessGroup.removeControllerService(), added notification to all descendant versioned Process Groups viaonComponentModified()when an ancestor service is removed. This ensures the state badge is recomputed even when no component currently references the deleted service.Why removing
getAncestorServiceIdsand the suppression is safeThe
externallyAccessibleServiceIdssuppression was introduced in NIFI-4436 (2017) as part of the original flow versioning feature, before any name-based resolution mechanism existed. Its purpose was to prevent cross-instance service ID mismatches from appearing as local modifications.Since then,
StandardControllerServiceResolverwas introduced (NIFI-9069) to resolve external controller service references by name and API compatibility during import/upgrade. With fixes 1 and 2 extending this resolution to the "Show Local Changes" and state badge paths respectively, the suppression is now redundant on all code paths. It was also flawed — asymmetric (only suppressed when old value was inaccessible AND new value was accessible) and unable to distinguish a foreign ID from a deleted local service ID.Tests
testExternalControllerServicePropertyChangeDetected()inTestStandardFlowComparatorto verify property changes between external services are detected.ExternalControllerServiceVersioningITwith four scenarios:Scenario coverage
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation