From 686b5575c998c3da76a3decde10d6461fa191dfd Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 13 Sep 2024 15:24:19 -0400 Subject: [PATCH 1/3] Closes #122: Implement a mechanism to enforce policy before executing branch actions --- netbox_branching/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index 56e5b50..fe64bc8 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -53,9 +53,6 @@ def ready(self): "netbox_branching: DATABASE_ROUTERS must contain 'netbox_branching.database.BranchAwareRouter'." ) - # Register models which support branching - register_models() - # Validate branch action validators for action in BRANCH_ACTIONS: for validator_path in get_plugin_config('netbox_branching', f'{action}_validators'): @@ -64,5 +61,8 @@ def ready(self): except ImportError: raise ImproperlyConfigured(f"Branch {action} validator not found: {validator_path}") + # Register models which support branching + register_models() + config = AppConfig From 477afa9fe966bfdf68dda5e737f8f054d73499ce Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 13 Dec 2024 14:21:06 -0500 Subject: [PATCH 2/3] Closes #189: Introduce a mechanism to automatically register pre-action branch validators --- netbox_branching/__init__.py | 6 ++- netbox_branching/models/branches.py | 41 +++++++++++++++---- .../netbox_branching/branch_action.html | 5 +-- netbox_branching/utilities.py | 13 ++++++ 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index fe64bc8..f8c0358 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -41,6 +41,7 @@ class AppConfig(PluginConfig): def ready(self): super().ready() from . import constants, events, search, signal_receivers + from .models import Branch from .utilities import DynamicSchemaDict # Validate required settings @@ -53,13 +54,14 @@ def ready(self): "netbox_branching: DATABASE_ROUTERS must contain 'netbox_branching.database.BranchAwareRouter'." ) - # Validate branch action validators + # Validate & register configured branch action validators for action in BRANCH_ACTIONS: for validator_path in get_plugin_config('netbox_branching', f'{action}_validators'): try: - import_string(validator_path) + func = import_string(validator_path) except ImportError: raise ImproperlyConfigured(f"Branch {action} validator not found: {validator_path}") + Branch.register_preaction_check(func, action) # Register models which support branching register_models() diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 747ac77..b711cbb 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -28,7 +28,8 @@ from netbox_branching.contextvars import active_branch from netbox_branching.signals import * from netbox_branching.utilities import ( - ChangeSummary, activate_branch, get_branchable_object_types, get_tables_to_replicate, record_applied_change, + BranchActionIndicator, ChangeSummary, activate_branch, get_branchable_object_types, get_tables_to_replicate, + record_applied_change, ) from utilities.exceptions import AbortRequest, AbortTransaction from .changes import ObjectChange @@ -83,6 +84,13 @@ class Branch(JobsMixin, PrimaryModel): related_name='+' ) + _preaction_validators = { + 'sync': set(), + 'merge': set(), + 'revert': set(), + 'archive': set(), + } + class Meta: ordering = ('name',) verbose_name = _('branch') @@ -190,6 +198,15 @@ def _generate_schema_id(length=8): chars = [*string.ascii_lowercase, *string.digits] return ''.join(random.choices(chars, k=length)) + @classmethod + def register_preaction_check(cls, func, action): + """ + Register a validator to run before a specific branch action (i.e. sync or merge). + """ + if action not in BRANCH_ACTIONS: + raise ValueError(f"Invalid branch action: {action}") + cls._preaction_validators[action].add(func) + def get_changes(self): """ Return a queryset of all ObjectChange records created within the Branch. @@ -265,33 +282,39 @@ def _can_do_action(self, action): """ if action not in BRANCH_ACTIONS: raise Exception(f"Unrecognized branch action: {action}") - for validator_path in get_plugin_config('netbox_branching', f'{action}_validators'): - if not import_string(validator_path)(self): - return False - return True - @property + # Run any pre-action validators + for func in self._preaction_validators[action]: + if not (indicator := func(self)): + # Backward compatibility for pre-v0.6.0 validators + if type(indicator) is not BranchActionIndicator: + return BranchActionIndicator(False, _(f"Validation failed for {action}: {func}")) + return indicator + + return BranchActionIndicator(True) + + @cached_property def can_sync(self): """ Indicates whether the branch can be synced. """ return self._can_do_action('sync') - @property + @cached_property def can_merge(self): """ Indicates whether the branch can be merged. """ return self._can_do_action('merge') - @property + @cached_property def can_revert(self): """ Indicates whether the branch can be reverted. """ return self._can_do_action('revert') - @property + @cached_property def can_archive(self): """ Indicates whether the branch can be archived. diff --git a/netbox_branching/templates/netbox_branching/branch_action.html b/netbox_branching/templates/netbox_branching/branch_action.html index 885e2d0..535872b 100644 --- a/netbox_branching/templates/netbox_branching/branch_action.html +++ b/netbox_branching/templates/netbox_branching/branch_action.html @@ -20,9 +20,8 @@ {% if not action_permitted %}
- {% blocktrans %} - This action is disallowed per policy, however dry runs are permitted. - {% endblocktrans %} + {{ action_permitted.message }} + {% blocktrans %}Only dry runs are permitted.{% endblocktrans %}
{% endif %} {% if conflicts_table.rows %} diff --git a/netbox_branching/utilities.py b/netbox_branching/utilities.py index 56fbc6e..7db4267 100644 --- a/netbox_branching/utilities.py +++ b/netbox_branching/utilities.py @@ -17,6 +17,7 @@ from .contextvars import active_branch __all__ = ( + 'BranchActionIndicator', 'ChangeSummary', 'DynamicSchemaDict', 'ListHandler', @@ -261,3 +262,15 @@ def ActiveBranchContextManager(request): if branch := get_active_branch(request): return activate_branch(branch) return nullcontext() + + +@dataclass +class BranchActionIndicator: + """ + An indication of whether a particular branch action is permitted. If not, an explanatory message must be provided. + """ + permitted: bool + message: str = '' + + def __bool__(self): + return self.permitted From 9b08ca0d49191793e7ce76bb3a589ad0d1b1b909 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 5 Feb 2025 13:16:03 -0500 Subject: [PATCH 3/3] Move register_models() earlier --- netbox_branching/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index f8c0358..34d5d08 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -54,6 +54,9 @@ def ready(self): "netbox_branching: DATABASE_ROUTERS must contain 'netbox_branching.database.BranchAwareRouter'." ) + # Register models which support branching + register_models() + # Validate & register configured branch action validators for action in BRANCH_ACTIONS: for validator_path in get_plugin_config('netbox_branching', f'{action}_validators'): @@ -63,8 +66,5 @@ def ready(self): raise ImproperlyConfigured(f"Branch {action} validator not found: {validator_path}") Branch.register_preaction_check(func, action) - # Register models which support branching - register_models() - config = AppConfig