fix: scope verify_config controller service verification to target PG#408
Merged
Conversation
ci.verify_config was verifying controller services inherited from ancestor process groups (root and any intermediate PGs) because NiFi's REST API returns ancestor services by default (includeAncestorGroups=true server default) and canvas.list_all_controllers never overrode this. A broken CS on root or a sibling flow's ancestor could cause CI to fail for an unrelated child flow. Changes: - canvas.list_all_controllers: add include_ancestors kwarg (default True preserves existing behaviour and test expectations) - ci.verify_config._verify_controllers: pass include_ancestors=False so only CSes owned by the target PG are verified - Add regression tests for both the helper and the CI function - Update docs/history.rst
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
=======================================
Coverage 76.81% 76.82%
=======================================
Files 41 41
Lines 4831 4832 +1
=======================================
+ Hits 3711 3712 +1
Misses 1120 1120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
ci.verify_config(process_group_id)was silently verifying controller services inherited from ancestor process groups (root and any intermediate PGs), not just those belonging to the target PG. This meant a misconfigured CS on root — or on a completely unrelated flow's parent — could cause CI to fail for an unrelated child flow.Root cause:
canvas.list_all_controllerscallsFlowApi.get_controller_services_from_groupwithout settinginclude_ancestor_groups. The NiFi REST API defaults this flag totrueserver-side, so inherited services are always returned unless explicitly opted out.Processor scoping was already correct (
ProcessGroupsApi.get_processorswithinclude_descendant_groups=True— processors are owned by exactly one PG, no inheritance exists).Changes
nipyapi/canvas.py—list_all_controllers: addinclude_ancestors: bool = Truekwarg. DefaultTruepreserves existing behaviour (existing tests pass unchanged). Threads the flag into both NiFi version branches asinclude_ancestor_groups=include_ancestors.nipyapi/ci/verify_config.py—_verify_controllers: passinclude_ancestors=Falseso CI only sees CSes owned by the target PG. Updated docstrings to state the scope contract explicitly.docs/history.rst— "Unreleased" entry describing the fix.Testing
New test
test_list_all_controllers_excludes_ancestorsintest_canvas.py: creates a three-level PG hierarchy, assertsinclude_ancestors=Falsereturns only the targeted PG's CS; asserts default (True) still returns ancestor CSes (backwards-compat).New test
test_verify_config_ignores_ancestor_controllersintest_ci.py: creates parent + child PGs with a CS on each, callsverify_config(child.id), asserts only the child's CS appears in results.All 8 targeted tests pass; 681 full-suite tests pass; pre-commit (black, isort, flake8, pylint 10.00/10) clean.