pvillard31 opened a new pull request, #11006:
URL: https://github.com/apache/nifi/pull/11006
## 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
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue
created
### 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](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits)
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](https://apache.org/licenses/LICENSE-2.0) according to the [License
Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE`
files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
