Skip to content

Commit d5e0875

Browse files
committed
Add memory-safe option
1 parent d6769a4 commit d5e0875

File tree

2 files changed

+166
-8
lines changed

2 files changed

+166
-8
lines changed

ansible_base/rbac/triggers.py

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,36 @@ def _get_bulk_cache_state():
3737
_bulk_cache_state.active = False
3838
_bulk_cache_state.needs_team_update = False
3939
_bulk_cache_state.object_roles_to_update = set()
40+
_bulk_cache_state.memory_safe = False
41+
_bulk_cache_state.needs_object_role_update = False
4042
return _bulk_cache_state
4143

4244

4345
@contextmanager
44-
def bulk_rbac_caching():
46+
def bulk_rbac_caching(memory_safe=False):
4547
"""
4648
Context manager that defers expensive RBAC cache updates during bulk operations.
4749
4850
Instead of calling update_after_assignment for each permission change,
4951
this collects all the updates and performs them once when exiting the context.
5052
53+
Args:
54+
memory_safe (bool): If True, doesn't store individual object roles to save memory.
55+
Instead performs a full re-computation of all object roles
56+
when exiting the context, similar to post_migrate behavior.
57+
5158
Usage:
5259
with bulk_rbac_caching():
5360
# Multiple permission assignments/removals
5461
role_def.give_permission(user1, obj1)
5562
role_def.give_permission(user2, obj2)
5663
role_def.remove_permission(user3, obj3)
5764
# Cache updates happen here when exiting the context
65+
66+
with bulk_rbac_caching(memory_safe=True):
67+
# For very large bulk operations where memory usage is a concern
68+
# This will recompute all object roles instead of tracking specific ones
69+
# ... many operations ...
5870
"""
5971
state = _get_bulk_cache_state()
6072

@@ -67,26 +79,39 @@ def bulk_rbac_caching():
6779
state.active = True
6880
state.needs_team_update = False
6981
state.object_roles_to_update = set()
82+
state.memory_safe = memory_safe
83+
state.needs_object_role_update = False
7084

7185
try:
7286
yield
7387
finally:
7488
# Exit bulk mode and perform deferred updates
7589
needs_team_update = state.needs_team_update
7690
object_roles_to_update = state.object_roles_to_update.copy()
91+
is_memory_safe = state.memory_safe
92+
needs_object_role_update = state.needs_object_role_update
7793

7894
# Reset state
7995
state.active = False
8096
state.needs_team_update = False
8197
state.object_roles_to_update = set()
98+
state.memory_safe = False
99+
state.needs_object_role_update = False
82100

83101
# Perform the global update
84-
if needs_team_update or object_roles_to_update:
85-
logger.info(f'Performing bulk RBAC cache update: teams={needs_team_update}, object_roles={len(object_roles_to_update)}')
86-
if needs_team_update:
87-
compute_team_member_roles()
88-
if object_roles_to_update:
89-
compute_object_role_permissions(object_roles=object_roles_to_update)
102+
if needs_team_update or object_roles_to_update or (is_memory_safe and needs_object_role_update):
103+
if is_memory_safe and needs_object_role_update:
104+
logger.info('Performing bulk RBAC cache update: memory_safe=True, recomputing all object roles')
105+
if needs_team_update:
106+
compute_team_member_roles()
107+
# In memory-safe mode, always recompute all object roles
108+
compute_object_role_permissions()
109+
elif not is_memory_safe:
110+
logger.info(f'Performing bulk RBAC cache update: teams={needs_team_update}, object_roles={len(object_roles_to_update)}')
111+
if needs_team_update:
112+
compute_team_member_roles()
113+
if object_roles_to_update:
114+
compute_object_role_permissions(object_roles=object_roles_to_update)
90115

91116

92117
def team_ancestor_roles(team):
@@ -155,7 +180,12 @@ def update_after_assignment(update_teams, to_update):
155180
if update_teams:
156181
state.needs_team_update = True
157182
if to_update:
158-
state.object_roles_to_update.update(to_update)
183+
if state.memory_safe:
184+
# In memory-safe mode, don't store individual object roles to save memory,
185+
# but track that we need to do a full recomputation
186+
state.needs_object_role_update = True
187+
else:
188+
state.object_roles_to_update.update(to_update)
159189
return
160190

161191
# Normal mode - perform updates immediately

test_app/tests/rbac/test_triggers.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,131 @@ def test_bulk_caching_empty_context(self):
346346
# Should not call expensive functions if no changes were made
347347
mock_team_update.assert_not_called()
348348
mock_obj_update.assert_not_called()
349+
350+
def test_bulk_caching_memory_safe_mode(self, rando, team, inv_rd, org_inv_rd, inventory, organization):
351+
"""Test that memory_safe=True mode doesn't store object roles but still recomputes"""
352+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
353+
354+
with bulk_rbac_caching(memory_safe=True):
355+
# Multiple assignments that would normally collect object roles
356+
inv_rd.give_permission(rando, inventory)
357+
org_inv_rd.give_permission(team, organization)
358+
359+
# Should not be called during bulk mode
360+
mock_obj_update.assert_not_called()
361+
362+
# Should be called once with no specific object roles (full recomputation)
363+
mock_obj_update.assert_called_once_with()
364+
365+
def test_bulk_caching_memory_safe_vs_normal_mode(self, rando, inv_rd, inventory):
366+
"""Test that memory_safe mode calls recomputation while normal mode passes specific roles"""
367+
# Test normal mode
368+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
369+
with bulk_rbac_caching(memory_safe=False):
370+
assignment = inv_rd.give_permission(rando, inventory)
371+
372+
# Should be called with specific object roles
373+
mock_obj_update.assert_called_once()
374+
call_args = mock_obj_update.call_args
375+
object_roles = call_args.kwargs['object_roles']
376+
assert assignment.object_role in object_roles
377+
378+
# Clean up the assignment
379+
inv_rd.remove_permission(rando, inventory)
380+
381+
# Test memory-safe mode
382+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
383+
with bulk_rbac_caching(memory_safe=True):
384+
inv_rd.give_permission(rando, inventory)
385+
386+
# Should be called with no specific object roles (full recomputation)
387+
mock_obj_update.assert_called_once_with()
388+
389+
def test_bulk_caching_memory_safe_with_team_updates(self, rando, team, member_rd):
390+
"""Test that memory_safe mode works correctly with team updates"""
391+
with (
392+
patch('ansible_base.rbac.triggers.compute_team_member_roles') as mock_team_update,
393+
patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update,
394+
):
395+
396+
with bulk_rbac_caching(memory_safe=True):
397+
# Assignment that affects team membership
398+
member_rd.give_permission(rando, team)
399+
400+
# Should not be called during bulk mode
401+
mock_team_update.assert_not_called()
402+
mock_obj_update.assert_not_called()
403+
404+
# Both should be called, but object role update should be full recomputation
405+
mock_team_update.assert_called_once()
406+
mock_obj_update.assert_called_once_with()
407+
408+
def test_bulk_caching_memory_safe_empty_context(self):
409+
"""Test that empty memory_safe context doesn't call cache functions"""
410+
with (
411+
patch('ansible_base.rbac.triggers.compute_team_member_roles') as mock_team_update,
412+
patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update,
413+
):
414+
415+
with bulk_rbac_caching(memory_safe=True):
416+
# No operations performed
417+
pass
418+
419+
# Should not call expensive functions if no changes were made, even in memory_safe mode
420+
mock_team_update.assert_not_called()
421+
mock_obj_update.assert_not_called()
422+
423+
def test_bulk_caching_memory_safe_nested_contexts(self, rando, inv_rd, inventory):
424+
"""Test that nested contexts with memory_safe work correctly"""
425+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
426+
427+
with bulk_rbac_caching(memory_safe=True):
428+
inv_rd.give_permission(rando, inventory)
429+
430+
# Nested context (memory_safe is ignored in nested calls)
431+
with bulk_rbac_caching(memory_safe=False):
432+
# Another assignment in nested context
433+
# Should still not trigger updates
434+
pass
435+
436+
# Still in outer context, should not be called yet
437+
mock_obj_update.assert_not_called()
438+
439+
# Only called once when exiting outermost context, with full recomputation
440+
mock_obj_update.assert_called_once_with()
441+
442+
def test_bulk_caching_memory_safe_with_removal(self, rando, inv_rd, inventory):
443+
"""Test memory_safe mode when object role gets deleted during removal"""
444+
# First give permission normally
445+
inv_rd.give_permission(rando, inventory)
446+
447+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
448+
449+
with bulk_rbac_caching(memory_safe=True):
450+
# Remove permission in bulk mode - this will delete the object role
451+
inv_rd.remove_permission(rando, inventory)
452+
mock_obj_update.assert_not_called()
453+
454+
# Should not call recomputation since object role was deleted (nothing to update)
455+
# This matches the existing behavior in regular bulk mode
456+
mock_obj_update.assert_not_called()
457+
458+
def test_bulk_caching_memory_safe_mixed_operations(self, rando, team, inv_rd, org_inv_rd, inventory, organization):
459+
"""Test memory_safe mode with mixed add/remove operations that result in net changes"""
460+
with patch('ansible_base.rbac.triggers.compute_object_role_permissions') as mock_obj_update:
461+
462+
with bulk_rbac_caching(memory_safe=True):
463+
# Add permissions (creates object roles)
464+
inv_rd.give_permission(rando, inventory)
465+
org_inv_rd.give_permission(team, organization)
466+
467+
# Remove one permission (may or may not delete object role)
468+
inv_rd.remove_permission(rando, inventory)
469+
470+
# Add it back
471+
inv_rd.give_permission(rando, inventory)
472+
473+
mock_obj_update.assert_not_called()
474+
475+
# Should call full recomputation since we had net updates
476+
mock_obj_update.assert_called_once_with()

0 commit comments

Comments
 (0)