Skip to content

Commit de998ba

Browse files
adamsachsclaude
andcommitted
ENG-3324: Add system-stewards change hook for inheritance propagation
Temporary single-purpose callback registry under ``fides.api`` that lets the fidesplus monitor-stewardship-inheritance feature react when a user is added/removed as a data steward (system manager) of a system. The three v1 system-manager mutation routes (``bulk_assign_steward``, ``update_managed_systems``, ``remove_user_as_system_manager``, and the approver-demotion branch of ``update_user_permissions``) gain a ``BackgroundTasks`` injection and call ``notify_system_stewards_changed(background_tasks, system.id)`` after each successful set/remove. The model methods themselves are unchanged so callers from tests/conftest don't trigger propagation. This is intentionally a tiny ~50-line shim. The module docstring documents the cutover: when the event framework lands (fides PR #8096), trigger sites swap to ``publish_after_commit(...)`` and this file is deleted. Do not generalize this into a ``hooks`` package. Pairs with fidesplus ``ENG-3324/monitor-steward-inheritance-propagation``, which registers the inheritance-reconcile callback at app startup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 550c392 commit de998ba

5 files changed

Lines changed: 139 additions & 4 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
"""
2+
System data-steward change hook — TEMPORARY EVENT-FRAMEWORK SHIM.
3+
4+
A single-purpose callback registry that lets fidesplus react when a user is
5+
added/removed as a data steward (system manager) of a system. The only known
6+
consumer today is fidesplus's monitor-stewardship-inheritance propagation.
7+
8+
This module exists ONLY because the event framework in fides PR #8096 is
9+
not yet merged. When it lands:
10+
11+
- Call sites of ``notify_system_stewards_changed`` (in the three v1
12+
system-manager routes) are replaced with
13+
``publish_after_commit(session, SystemDataStewardsChanged(...))``.
14+
- Fidesplus's registered callback is replaced with a
15+
``@subscribes_to(SystemDataStewardsChanged)`` handler.
16+
- This file is deleted.
17+
18+
DO NOT use this as a precedent for adding more cross-repo hooks. If you need
19+
a similar shim before the framework lands, create a separate single-purpose
20+
module — do not generalize this into a ``hooks`` package.
21+
"""
22+
23+
from typing import Callable, List
24+
25+
from fastapi import BackgroundTasks
26+
from loguru import logger
27+
28+
SystemStewardChangeHook = Callable[[BackgroundTasks, str], None]
29+
30+
_HOOKS: List[SystemStewardChangeHook] = []
31+
32+
33+
def register_system_steward_change_hook(hook: SystemStewardChangeHook) -> None:
34+
"""Register a callback invoked when a system's data stewards change.
35+
36+
Idempotent: registering the same hook twice is a no-op.
37+
"""
38+
if hook not in _HOOKS:
39+
_HOOKS.append(hook)
40+
41+
42+
def notify_system_stewards_changed(
43+
background_tasks: BackgroundTasks, system_id: str
44+
) -> None:
45+
"""Invoke every registered hook with the given ``system_id``.
46+
47+
Each hook is wrapped in try/except so one failure doesn't suppress the rest;
48+
failures are logged and swallowed. Hooks are responsible for scheduling
49+
their own background work via ``background_tasks``.
50+
"""
51+
for hook in _HOOKS:
52+
try:
53+
hook(background_tasks, system_id)
54+
except Exception:
55+
logger.exception(
56+
"System-stewards-change hook {} raised for system_id={}",
57+
hook,
58+
system_id,
59+
)

src/fides/api/v1/endpoints/system.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from typing import Annotated, Dict, List, Literal, Optional, Union
22

3-
from fastapi import Depends, HTTPException, Query, Response, Security
3+
from fastapi import BackgroundTasks, Depends, HTTPException, Query, Response, Security
44
from fastapi_pagination import Page, Params
55
from fastapi_pagination.bases import AbstractPage
66
from fastapi_pagination.ext.sqlalchemy import paginate
@@ -57,6 +57,7 @@
5757
BasicSystemResponse,
5858
SystemResponse,
5959
)
60+
from fides.api.system_steward_change_hooks import notify_system_stewards_changed
6061
from fides.api.util.api_router import APIRouter
6162
from fides.api.util.connection_util import (
6263
delete_connection_config,
@@ -402,6 +403,7 @@ async def system_bulk_delete(
402403
)
403404
async def bulk_assign_steward(
404405
data: AssignStewardRequest,
406+
background_tasks: BackgroundTasks,
405407
db: Session = Depends(deps.get_db),
406408
) -> Dict:
407409
"""Assign the given `data_steward` (username) as a system manager for the list of `system_keys`.
@@ -467,6 +469,7 @@ async def bulk_assign_steward(
467469
for system in systems:
468470
if user not in system.data_stewards:
469471
user.set_as_system_manager(db, system)
472+
notify_system_stewards_changed(background_tasks, system.id)
470473
updated_count += 1
471474

472475
return {

src/fides/api/v1/endpoints/user_endpoints.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from datetime import datetime, timedelta, timezone
55
from typing import Dict, List, Optional
66

7-
from fastapi import Depends, HTTPException, Request, Response, Security
7+
from fastapi import BackgroundTasks, Depends, HTTPException, Request, Response, Security
88
from fastapi.security import SecurityScopes
99
from fastapi_pagination import Page, Params
1010
from fastapi_pagination.bases import AbstractPage
@@ -64,6 +64,7 @@
6464
UserResponse,
6565
UserUpdate,
6666
)
67+
from fides.api.system_steward_change_hooks import notify_system_stewards_changed
6768
from fides.api.util.api_router import APIRouter
6869
from fides.api.util.errors import FidesError, MessageDispatchException
6970
from fides.api.util.rate_limit import fides_limiter
@@ -341,6 +342,7 @@ def update_managed_systems(
341342
db: Session = Depends(deps.get_db),
342343
user_id: str,
343344
systems: List[FidesKey],
345+
background_tasks: BackgroundTasks,
344346
) -> List[SystemSchema]:
345347
"""
346348
Endpoint to override the systems for which a user is "system manager".
@@ -379,11 +381,13 @@ def update_managed_systems(
379381
for system in retrieved_systems:
380382
if user not in system.data_stewards:
381383
user.set_as_system_manager(db, system)
384+
notify_system_stewards_changed(background_tasks, system.id)
382385

383386
# Removing systems for which the user in no longer a manager
384387
for system in user.systems.copy():
385388
if system not in retrieved_systems:
386389
user.remove_as_system_manager(db, system)
390+
notify_system_stewards_changed(background_tasks, system.id)
387391

388392
return user.systems
389393

@@ -473,7 +477,11 @@ async def get_managed_system_details(
473477
status_code=HTTP_204_NO_CONTENT,
474478
)
475479
def remove_user_as_system_manager(
476-
*, db: Session = Depends(deps.get_db), user_id: str, system_key: FidesKey
480+
*,
481+
db: Session = Depends(deps.get_db),
482+
user_id: str,
483+
system_key: FidesKey,
484+
background_tasks: BackgroundTasks,
477485
) -> None:
478486
"""
479487
Endpoint to remove user as system manager from the given system
@@ -488,6 +496,7 @@ def remove_user_as_system_manager(
488496
)
489497

490498
user.remove_as_system_manager(db, system)
499+
notify_system_stewards_changed(background_tasks, system.id)
491500
logger.info("Removed user {} as system manager of {}", user_id, system.fides_key)
492501

493502

src/fides/api/v1/endpoints/user_permission_endpoints.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from typing import List, Optional
22

3-
from fastapi import Depends, HTTPException, Security
3+
from fastapi import BackgroundTasks, Depends, HTTPException, Security
44
from fastapi.security import SecurityScopes
55
from loguru import logger
66
from sqlalchemy.orm import Session
@@ -16,6 +16,7 @@
1616
UserPermissionsEdit,
1717
UserPermissionsResponse,
1818
)
19+
from fides.api.system_steward_change_hooks import notify_system_stewards_changed
1920
from fides.api.util.api_router import APIRouter
2021
from fides.common import urn_registry as urls
2122
from fides.common.scope_registry import (
@@ -97,6 +98,7 @@ async def update_user_permissions(
9798
user_id: str,
9899
authorization: str = Security(oauth2_scheme),
99100
permissions: UserPermissionsEdit,
101+
background_tasks: BackgroundTasks,
100102
) -> FidesUserPermissions:
101103
"""Update a user's role(s). The UI assigns one role at a time, but multiple
102104
roles are technically supported.
@@ -128,6 +130,7 @@ async def update_user_permissions(
128130
system.fides_key,
129131
)
130132
user.remove_as_system_manager(db, system)
133+
notify_system_stewards_changed(background_tasks, system.id)
131134

132135
return updated_user_perms
133136

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""Tests for the system-stewards change hook registry."""
2+
3+
from unittest.mock import MagicMock
4+
5+
import pytest
6+
from fastapi import BackgroundTasks
7+
8+
from fides.api import system_steward_change_hooks as system_stewards
9+
10+
11+
@pytest.fixture(autouse=True)
12+
def isolated_registry(monkeypatch):
13+
"""Each test starts with an empty registry."""
14+
monkeypatch.setattr(system_stewards, "_HOOKS", [])
15+
yield
16+
17+
18+
def test_register_appends_hook():
19+
hook = MagicMock()
20+
system_stewards.register_system_steward_change_hook(hook)
21+
assert system_stewards._HOOKS == [hook]
22+
23+
24+
def test_register_is_idempotent():
25+
hook = MagicMock()
26+
system_stewards.register_system_steward_change_hook(hook)
27+
system_stewards.register_system_steward_change_hook(hook)
28+
assert system_stewards._HOOKS == [hook]
29+
30+
31+
def test_notify_calls_each_registered_hook():
32+
hook_a = MagicMock()
33+
hook_b = MagicMock()
34+
system_stewards.register_system_steward_change_hook(hook_a)
35+
system_stewards.register_system_steward_change_hook(hook_b)
36+
37+
bg = BackgroundTasks()
38+
system_stewards.notify_system_stewards_changed(bg, "sys-1")
39+
40+
hook_a.assert_called_once_with(bg, "sys-1")
41+
hook_b.assert_called_once_with(bg, "sys-1")
42+
43+
44+
def test_notify_isolates_hook_failures():
45+
"""One hook raising must not prevent the rest from firing."""
46+
raising = MagicMock(side_effect=RuntimeError("boom"))
47+
survivor = MagicMock()
48+
system_stewards.register_system_steward_change_hook(raising)
49+
system_stewards.register_system_steward_change_hook(survivor)
50+
51+
bg = BackgroundTasks()
52+
system_stewards.notify_system_stewards_changed(bg, "sys-2")
53+
54+
raising.assert_called_once()
55+
survivor.assert_called_once_with(bg, "sys-2")
56+
57+
58+
def test_notify_with_no_hooks_is_noop():
59+
bg = BackgroundTasks()
60+
# Should not raise even if registry empty
61+
system_stewards.notify_system_stewards_changed(bg, "sys-3")

0 commit comments

Comments
 (0)