From b67b4f66da8136e0b0f8c54782622c54312c491d Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 16 May 2024 10:45:32 -0700 Subject: [PATCH 1/5] 16145 script api use module.script name instead of pk --- netbox/extras/api/views.py | 21 +++++++++++++++++---- netbox/extras/tests/test_api.py | 8 ++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 1f76467b5e2..1be171dfa99 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -1,3 +1,4 @@ +from django.http import Http404 from django.shortcuts import get_object_or_404 from django_rq.queues import get_connection from rest_framework import status @@ -13,7 +14,7 @@ from core.models import Job, ObjectType from extras import filtersets from extras.models import * -from extras.scripts import run_script +from extras.scripts import get_module_and_script, run_script from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired from netbox.api.features import SyncedDataMixin from netbox.api.metadata import ContentTypeMetadata @@ -215,21 +216,33 @@ class ScriptViewSet(ModelViewSet): _ignore_model_permissions = True lookup_value_regex = '[^/]+' # Allow dots + def _get_script(self, pk): + try: + module_name, script_name = pk.split('.', maxsplit=1) + except ValueError: + raise Http404 + + module, script = get_module_and_script(module_name, script_name) + if script is None: + raise Http404 + + return module, script + def retrieve(self, request, pk): - script = get_object_or_404(self.queryset, pk=pk) + module, script = self._get_script(pk) serializer = serializers.ScriptDetailSerializer(script, context={'request': request}) return Response(serializer.data) def post(self, request, pk): """ - Run a Script identified by the id and return the pending Job as the result + Run a Script identified by the name and return the pending Job as the result """ if not request.user.has_perm('extras.run_script'): raise PermissionDenied("This user does not have permission to run scripts.") - script = get_object_or_404(self.queryset, pk=pk) + module, script = self._get_script(pk) input_serializer = serializers.ScriptInputSerializer( data=request.data, context={'script': script} diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 5d243ae1a9d..e4e2edb4f28 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -780,12 +780,16 @@ def setUpTestData(cls): def python_class(self): return self.TestScriptClass + def get_test_script(self, *args): + module = ScriptModule.objects.first() + return module, module.scripts.first() + def setUp(self): super().setUp() # Monkey-patch the Script model to return our TestScriptClass above from extras.api.views import ScriptViewSet - Script.python_class = self.python_class + ScriptViewSet._get_script = self.get_test_script def test_get_script(self): module = ScriptModule.objects.get( @@ -793,7 +797,7 @@ def test_get_script(self): file_path='/var/tmp/script.py' ) script = module.scripts.all().first() - url = reverse('extras-api:script-detail', kwargs={'pk': script.pk}) + url = reverse('extras-api:script-detail', kwargs={'pk': None}) response = self.client.get(url, **self.header) self.assertEqual(response.data['name'], self.TestScriptClass.Meta.name) From 452ba22bd8f903a083d7812284ebfcf8a35f32b8 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 16 May 2024 15:04:51 -0700 Subject: [PATCH 2/5] 16145 fix test --- netbox/extras/tests/test_api.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index e4e2edb4f28..40635a936f3 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -777,8 +777,10 @@ def setUpTestData(cls): is_executable=True, ) - def python_class(self): - return self.TestScriptClass + def get_vars(self, *args): + return { + k: v.__class__.__name__ for k, v in self.TestScriptClass._get_vars().items() + } def get_test_script(self, *args): module = ScriptModule.objects.first() @@ -790,6 +792,8 @@ def setUp(self): # Monkey-patch the Script model to return our TestScriptClass above from extras.api.views import ScriptViewSet ScriptViewSet._get_script = self.get_test_script + from extras.api.serializers_.scripts import ScriptSerializer + ScriptSerializer.get_vars = self.get_vars def test_get_script(self): module = ScriptModule.objects.get( From 754ab82a99009953b9ed25dd017a904ecf7d15b7 Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 17 May 2024 11:16:06 -0700 Subject: [PATCH 3/5] 16145 allow both pk and script name --- netbox/extras/api/views.py | 15 ++++++++++----- netbox/extras/tests/test_api.py | 16 ++++------------ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 1be171dfa99..89cf9c5dfad 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -217,12 +217,17 @@ class ScriptViewSet(ModelViewSet): lookup_value_regex = '[^/]+' # Allow dots def _get_script(self, pk): - try: - module_name, script_name = pk.split('.', maxsplit=1) - except ValueError: - raise Http404 + if pk.isnumeric(): + script = get_object_or_404(self.queryset, pk=pk) + module = script.module + else: + try: + module_name, script_name = pk.split('.', maxsplit=1) + except ValueError: + raise Http404 + + module, script = get_module_and_script(module_name, script_name) - module, script = get_module_and_script(module_name, script_name) if script is None: raise Http404 diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 40635a936f3..5d243ae1a9d 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -777,23 +777,15 @@ def setUpTestData(cls): is_executable=True, ) - def get_vars(self, *args): - return { - k: v.__class__.__name__ for k, v in self.TestScriptClass._get_vars().items() - } - - def get_test_script(self, *args): - module = ScriptModule.objects.first() - return module, module.scripts.first() + def python_class(self): + return self.TestScriptClass def setUp(self): super().setUp() # Monkey-patch the Script model to return our TestScriptClass above from extras.api.views import ScriptViewSet - ScriptViewSet._get_script = self.get_test_script - from extras.api.serializers_.scripts import ScriptSerializer - ScriptSerializer.get_vars = self.get_vars + Script.python_class = self.python_class def test_get_script(self): module = ScriptModule.objects.get( @@ -801,7 +793,7 @@ def test_get_script(self): file_path='/var/tmp/script.py' ) script = module.scripts.all().first() - url = reverse('extras-api:script-detail', kwargs={'pk': None}) + url = reverse('extras-api:script-detail', kwargs={'pk': script.pk}) response = self.client.get(url, **self.header) self.assertEqual(response.data['name'], self.TestScriptClass.Meta.name) From 1253d9188fdcf39d7995fab9d172b99d959d69f3 Mon Sep 17 00:00:00 2001 From: Arthur Date: Fri, 17 May 2024 11:17:40 -0700 Subject: [PATCH 4/5] 16145 update doc string --- netbox/extras/api/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 89cf9c5dfad..579af077d5c 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -241,7 +241,7 @@ def retrieve(self, request, pk): def post(self, request, pk): """ - Run a Script identified by the name and return the pending Job as the result + Run a Script identified by the name or pk and return the pending Job as the result """ if not request.user.has_perm('extras.run_script'): From 1d5d283cb979a0aae2ece1baa09e01a1b352186c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 22 May 2024 09:02:47 -0400 Subject: [PATCH 5/5] Simplify retrieval logic --- netbox/extras/api/views.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index 579af077d5c..05087b2d5ea 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -14,7 +14,7 @@ from core.models import Job, ObjectType from extras import filtersets from extras.models import * -from extras.scripts import get_module_and_script, run_script +from extras.scripts import run_script from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired from netbox.api.features import SyncedDataMixin from netbox.api.metadata import ContentTypeMetadata @@ -217,37 +217,31 @@ class ScriptViewSet(ModelViewSet): lookup_value_regex = '[^/]+' # Allow dots def _get_script(self, pk): + # If pk is numeric, retrieve script by ID if pk.isnumeric(): - script = get_object_or_404(self.queryset, pk=pk) - module = script.module - else: - try: - module_name, script_name = pk.split('.', maxsplit=1) - except ValueError: - raise Http404 - - module, script = get_module_and_script(module_name, script_name) + return get_object_or_404(self.queryset, pk=pk) - if script is None: + # Default to retrieval by module & name + try: + module_name, script_name = pk.split('.', maxsplit=1) + except ValueError: raise Http404 - - return module, script + return get_object_or_404(self.queryset, module__file_path=f'{module_name}.py', name=script_name) def retrieve(self, request, pk): - module, script = self._get_script(pk) + script = self._get_script(pk) serializer = serializers.ScriptDetailSerializer(script, context={'request': request}) return Response(serializer.data) def post(self, request, pk): """ - Run a Script identified by the name or pk and return the pending Job as the result + Run a Script identified by its numeric PK or module & name and return the pending Job as the result """ - if not request.user.has_perm('extras.run_script'): raise PermissionDenied("This user does not have permission to run scripts.") - module, script = self._get_script(pk) + script = self._get_script(pk) input_serializer = serializers.ScriptInputSerializer( data=request.data, context={'script': script}