Skip to content

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
pvillard31:NIFI-15697
Open

NIFI-15697 - Fix inconsistent versioning state when using external controller services in versioned Process Groups#11006
pvillard31 wants to merge 1 commit intoapache:mainfrom
pvillard31:NIFI-15697

Conversation

@pvillard31
Copy link
Contributor

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:

  1. Missing resolution in "Show Local Changes" path: getLocalModifications() in StandardNiFiServiceFacade fetches the original unresolved snapshot from the registry but does not run the controller service resolver before comparing. The state badge path (getModifications() in StandardProcessGroup) uses the locally stored snapshot which was already resolved at import time. This means the two paths can produce different results.

  2. 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.

  3. Stale cached differences: removeControllerService() in StandardProcessGroup only 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 comparing

In StandardNiFiServiceFacade.getLocalModifications(), after fetching the registry snapshot, we now call controllerServiceResolver.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 caching

In 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 false LOCALLY_MODIFIED states after cross-instance imports or NiFi restarts.

3. Remove the externallyAccessibleServiceIds suppression from StandardFlowComparator

Removed the externallyAccessibleServiceIds field, constructor parameter, and the suppression block in compareProperties(). This suppression is now redundant because fixes 1 and 2 handle the cross-instance case properly via name-based resolution. Removed getAncestorServiceIds() from StandardProcessGroup and the ProcessGroup interface, and updated all StandardFlowComparator call sites accordingly.

4. Improve cache invalidation in removeControllerService()

In StandardProcessGroup.removeControllerService(), added notification to all descendant versioned Process Groups via onComponentModified() when an ancestor service is removed. This ensures the state badge is recomputed even when no component currently references the deleted service.

Why removing getAncestorServiceIds and the suppression is safe

The externallyAccessibleServiceIds suppression 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, StandardControllerServiceResolver was 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

  • Unit test: Added testExternalControllerServicePropertyChangeDetected() in TestStandardFlowComparator to verify property changes between external services are detected.
  • System tests: Added ExternalControllerServiceVersioningIT with four scenarios:
    • Cross-instance import with name-based resolution → UP_TO_DATE
    • Cross-instance upgrade (same service, non-service property change) → UP_TO_DATE
    • NIFI-15697 reproduction (switch service, delete original) → LOCALLY_MODIFIED with correct dialog
    • Baseline unmodified flow with external service → UP_TO_DATE

Scenario coverage

Scenario Behavior
Cross-instance import, service resolved by name Resolver normalizes IDs → UP_TO_DATE
Cross-instance import, no matching name on target Foreign ID stays → PROPERTY_CHANGED
Same instance, changed X→Y, X deleted (NIFI-15697) Resolver can't find X → PROPERTY_CHANGED
Same instance, changed X→Y, X still exists X resolves but differs from Y → PROPERTY_CHANGED
Cross-instance upgrade, same service by name Resolver maps to local ID → UP_TO_DATE

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
  • Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

…ntroller services in versioned Process Groups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant