From 65f2497ca0344daad06614a837c0d677d13750f5 Mon Sep 17 00:00:00 2001 From: Tomas Cejka <cejkat@cesnet.cz> Date: Tue, 14 Dec 2021 19:41:48 +0000 Subject: [PATCH] restapi: reworked deactivate and delete Added new url for deactivate to split the functionality of deactivation and deletion. Delete process is allowed only for enabled users, the process includes deactivation as well. Reworked "retry" of the Celery tasks and added new settings variable NETCONF_MAX_RETRY_BEFORE_ERROR. --- accounts/models.py | 7 ++ flowspec/models.py | 4 +- flowspec/tasks.py | 198 +++++++++++++++++-------------------- flowspec/views.py | 60 +++++++++-- flowspec/viewsets.py | 77 +++------------ flowspy/settings.py.dist | 3 + flowspy/urls.py | 3 +- templates/user_routes.html | 2 +- 8 files changed, 174 insertions(+), 180 deletions(-) diff --git a/accounts/models.py b/accounts/models.py index a515d51d..d72f2f94 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -18,6 +18,7 @@ # from django.db import models +from django.conf import settings from django.contrib.auth.models import AbstractBaseUser, User, BaseUserManager from peers.models import Peer @@ -44,3 +45,9 @@ class UserProfile(models.Model): if not networks: return False return networks + + def is_delete_allowed(self): + user_is_admin = self.user.is_superuser + username = self.username + return (user_is_admin and settings.ALLOW_DELETE_FULL_FOR_ADMIN) or settings.ALLOW_DELETE_FULL_FOR_USER_ALL or (username in settings.ALLOW_DELETE_FULL_FOR_USER_LIST) + diff --git a/flowspec/models.py b/flowspec/models.py index 2f6b2e3a..251eeb1d 100644 --- a/flowspec/models.py +++ b/flowspec/models.py @@ -339,7 +339,7 @@ class Route(models.Model): } logger.info(mail_body, extra=d) - def commit_delete(self, *args, **kwargs): + def commit_deactivate(self, *args, **kwargs): username = None reason_text = '' reason = '' @@ -366,8 +366,6 @@ class Route(models.Model): reason_text ), peer ) - response = delete.delay(self.pk, reason=reason) - logger.info('Got delete job id: %s' % response) if not settings.DISABLE_EMAIL_NOTIFICATION: fqdn = Site.objects.get_current().domain admin_url = 'https://%s%s' % ( diff --git a/flowspec/tasks.py b/flowspec/tasks.py index 36af4c45..a9bc7985 100644 --- a/flowspec/tasks.py +++ b/flowspec/tasks.py @@ -46,112 +46,84 @@ handler = logging.FileHandler(LOG_FILENAME) handler.setFormatter(formatter) logger.addHandler(handler) -@shared_task(ignore_result=True, autoretry_for=(TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True) +@shared_task(ignore_result=True, autoretry_for=(TimeoutError, TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True, retry_kwargs={'max_retries': settings.NETCONF_MAX_RETRY_BEFORE_ERROR}) def add(routepk, callback=None): from flowspec.models import Route route = Route.objects.get(pk=routepk) - try: - applier = PR.Applier(route_object=route) - commit, response = applier.apply() - if commit: - status = "ACTIVE" - #snmp_add_initial_zero_value.delay(str(route.id), True) - snmp_add_initial_zero_value(str(route.id), True) - else: - status = "ERROR" - route.status = status - route.response = response - route.save() - announce("[%s] Rule add: %s - Result: %s" % (route.applier_username_nice, route.name, response), route.applier, route) - except TimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Rule add: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except SoftTimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Rule add: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except Exception: + applier = PR.Applier(route_object=route) + commit, response = applier.apply() + if commit: + route.status = "ACTIVE" + #snmp_add_initial_zero_value.delay(str(route.id), True) + snmp_add_initial_zero_value(str(route.id), True) + else: + if deactivate_route.request.retries < settings.NETCONF_MAX_RETRY_BEFORE_ERROR: + # repeat the action + raise TimeoutError() route.status = "ERROR" - route.response = "Error" - route.save() - announce("[%s] Rule add: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) + route.response = response + route.save() + announce("[%s] Rule add: %s - Result: %s" % (route.applier_username_nice, route.name, response), route.applier, route) -@shared_task(ignore_result=True, autoretry_for=(TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True) +@shared_task(ignore_result=True, autoretry_for=(TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True, retry_kwargs={'max_retries': settings.NETCONF_MAX_RETRY_BEFORE_ERROR}) def edit(routepk, callback=None): from flowspec.models import Route route = Route.objects.get(pk=routepk) - try: - status_pre = route.status - logger.info("tasks::edit(): route="+str(route)+", status_pre="+str(status_pre)) - applier = PR.Applier(route_object=route) - commit, response = applier.apply(operation="replace") - if commit: - status = "ACTIVE" - try: - #snmp_add_initial_zero_value.delay(str(route.id), True) - snmp_add_initial_zero_value(str(route.id), True) - except Exception as e: - logger.error("tasks::edit(): route="+str(route)+", ACTIVE, add_initial_zero_value failed: "+str(e)) - else: - status = "ERROR" - route.status = status - route.response = response - route.save() - announce("[%s] Rule edit: %s - Result: %s" % (route.applier_username_nice, route.name, response), route.applier, route) - except TimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Rule edit: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except SoftTimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Rule edit: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except Exception as e: + status_pre = route.status + logger.info("tasks::edit(): route="+str(route)+", status_pre="+str(status_pre)) + applier = PR.Applier(route_object=route) + commit, response = applier.apply(operation="replace") + if commit: + route.status = "ACTIVE" + try: + #snmp_add_initial_zero_value.delay(str(route.id), True) + snmp_add_initial_zero_value(str(route.id), True) + except Exception as e: + logger.error("tasks::edit(): route="+str(route)+", ACTIVE, add_initial_zero_value failed: "+str(e)) + else: + if deactivate_route.request.retries < settings.NETCONF_MAX_RETRY_BEFORE_ERROR: + # repeat the action + raise TimeoutError() route.status = "ERROR" - route.response = "Error" - route.save() - logger.error(str(e)) - announce("[%s] Rule edit: %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - + route.response = response + route.save() + announce("[%s] Rule edit: %s - Result: %s" % (route.applier_username_nice, route.name, response), route.applier, route) -@shared_task(ignore_result=True, autoretry_for=(TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True) -def delete(routepk, **kwargs): +@shared_task(ignore_result=True, autoretry_for=(TimeoutError, TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True, retry_kwargs={'max_retries': settings.NETCONF_MAX_RETRY_BEFORE_ERROR}) +def deactivate_route(routepk, **kwargs): + """Deactivate the Route in ACTIVE state. Permissions must be checked before this call.""" from flowspec.models import Route route = Route.objects.get(pk=routepk) initial_status = route.status - try: - applier = PR.Applier(route_object=route) - commit, response = applier.apply(operation="delete") - reason_text = '' - logger.info("tasks::delete(): initial_status="+str(initial_status)) - if commit: - route.status="INACTIVE" - try: - snmp_add_initial_zero_value(str(route.id), False) - except Exception as e: - logger.error("edit(): route="+str(route)+", INACTIVE, add_null_value failed: "+str(e)) - - if initial_status == "PENDING_TODELETE": # special new case for fully deleting a rule via REST API (only for users/admins authorized by special settings) - msg1 = "[%s] Fully deleted route : %s%s- Result %s" % (route.applier, route.name, reason_text, response) - logger.info("tasks::delete(): FULLY DELETED msg="+msg1) - announce(msg1, route.applier, route) - route.delete() - return - else: - msg1 = "[%s] Deleted route : %s%s- Result %s" % (route.applier, route.name, reason_text, response) - logger.info("tasks::delete(): DELETED msg="+msg1) - announce(msg1, route.applier, route) - route.response = response - route.save() - return - else: # removing rule in NETCONF failed, it is still ACTIVE and also collects statistics - # NETCONF "delete" operation failed, keep the object in DB + if initial_status not in ("ACTIVE", "PENDING", "ERROR"): + logger.error("tasks::deactivate(): Cannot deactivate route that is not in ACTIVE or potential ACTIVE status.") + return + + applier = PR.Applier(route_object=route) + # Delete from router via NETCONF + commit, response = applier.apply(operation="delete") + reason_text = '' + logger.info("tasks::delete(): initial_status="+str(initial_status)) + if commit: + route.status="INACTIVE" + try: + snmp_add_initial_zero_value(str(route.id), False) + except Exception as e: + logger.error("edit(): route="+str(route)+", INACTIVE, add_null_value failed: "+str(e)) + + announce("[%s] Suspending rule : %s%s- Result %s" % (route.applier_username_nice, route.name, reason_text, response), route.applier, route) + route.status = "INACTIVE" + route.response = response + route.save() + route.commit_deactivate() + return + else: # removing rule in NETCONF failed, it is still ACTIVE and also collects statistics + # NETCONF "delete" operation failed, keep the object in DB + if deactivate_route.request.retries < settings.NETCONF_MAX_RETRY_BEFORE_ERROR: + # repeat the action + raise TimeoutError() + else: if "reason" in kwargs and kwargs['reason'] == 'EXPIRED': status = 'EXPIRED' reason_text = " Reason: %s " % status @@ -161,23 +133,35 @@ def delete(routepk, **kwargs): route.response = response route.save() announce("[%s] Suspending rule : %s%s- Result %s" % (route.applier_username_nice, route.name, reason_text, response), route.applier, route) - except TimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Suspending rule : %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except SoftTimeLimitExceeded: - route.status = "ERROR" - route.response = "Task timeout" - route.save() - announce("[%s] Suspending rule : %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - except Exception as e: - logger.error("tasks::edit(): route="+str(route)+", got unexpected exception="+str(e)) + +@shared_task(ignore_result=True, autoretry_for=(TimeoutError, TimeLimitExceeded, SoftTimeLimitExceeded), retry_backoff=True, retry_kwargs={'max_retries': settings.NETCONF_MAX_RETRY_BEFORE_ERROR}) +def delete_route(routepk, **kwargs): + """For Route in ACTIVE state, deactivate it at first. Finally, delete the Route from the DB. Permissions must be checked before this call.""" + from flowspec.models import Route + route = Route.objects.get(pk=routepk) + if route.status != "INACTIVE": + logger.info("Deactivating active route...") + # call deactivate_route() directly since we are already on background (celery task) + try: + deactivate_route(routepk) + except TimeoutError: + pass + if route.status != "INACTIVE" and delete_route.request.retries < settings.NETCONF_MAX_RETRY_BEFORE_ERROR: + # Repeat due to error in deactivation + route.status = "PENDING" + route.save() + logger.error("Deactivation failed, repeat the deletion process.") + raise TimeoutError() + + if route.status == "INACTIVE": + logger.info("Deleting inactive route...") + route.delete() + logger.info("Deleting finished.") + else: route.status = "ERROR" - route.response = "Error" route.save() - announce("[%s] Suspending rule : %s - Result: %s" % (route.applier_username_nice, route.name, route.response), route.applier, route) - + logger.error("Deleting Route failed, it could not be deactivated - remaining in DB.") + return # May not work in the first place... proxy is not aware of Route models @shared_task diff --git a/flowspec/views.py b/flowspec/views.py index 3617e9c5..6103d0c1 100644 --- a/flowspec/views.py +++ b/flowspec/views.py @@ -443,11 +443,15 @@ def edit_route(request, route_slug): @login_required @never_cache -def delete_route(request, route_slug): +def delete_route_view(request, route_slug): if request.is_ajax(): route = get_object_or_404(Route, name=route_slug) peers = route.applier.userprofile.peers.all() username = None + if route.status == "ACTIVE": + html = "<html><body>Cannot delete active Route! Deactivate at first</body></html>" + return HttpResponse(html) + for peer in peers: if username: break @@ -469,7 +473,6 @@ def delete_route(request, route_slug): break requester_peer = username if applier_peer == requester_peer or request.user.is_superuser: - route.status = "PENDING" route.expires = datetime.date.today() if not request.user.is_superuser: route.applier = request.user @@ -482,14 +485,57 @@ def delete_route(request, route_slug): username_request = request.user.username user_is_admin = request.user.is_superuser - full_delete_is_allowed = (user_is_admin and settings.ALLOW_DELETE_FULL_FOR_ADMIN) or settings.ALLOW_DELETE_FULL_FOR_USER_ALL or (username_request in settings.ALLOW_DELETE_FULL_FOR_USER_LIST) + full_delete_is_allowed = request.user.userprofile.is_delete_allowed() logger.info("views.delete(): username_request="+str(username_request)+" user_is_admin="+str(user_is_admin)+" => full_delete_is_allowed="+str(full_delete_is_allowed)+", but will not be used in views::delete") - #if full_delete_is_allowed: - # route.status = "PENDING_TODELETE"; - # logger.info("tasks.delete(): => status="+str(route.status)) + if full_delete_is_allowed: + delete_route.delay(route.pk) + html = "<html><body>Processing delete operation...</body></html>" + else: + html = "<html><body>Not enough permissions to delete Route.</body></html>" + return HttpResponse(html) + else: + return HttpResponseRedirect(reverse("group-routes")) +@login_required +@never_cache +def deactivate_route_view(request, route_slug): + if request.is_ajax(): + route = get_object_or_404(Route, name=route_slug) + peers = route.applier.userprofile.peers.all() + username = None + for peer in peers: + if username: + break + for network in peer.networks.all(): + net = ip_network(network) + if ip_network(route.destination) in net: + username = peer + break + applier_peer = username + peers = request.user.userprofile.peers.all() + username = None + for peer in peers: + if username: + break + for network in peer.networks.all(): + net = ip_network(network) + if ip_network(route.destination) in net: + username = peer + break + requester_peer = username + if applier_peer == requester_peer or request.user.is_superuser: + route.status = "PENDING" + route.expires = datetime.date.today() + if not request.user.is_superuser: + route.applier = request.user + route.response = "Deactivating" + try: + route.requesters_address = request.META['HTTP_X_FORWARDED_FOR'] + except: + # in case the header is not provided + route.requesters_address = 'unknown' route.save() - route.commit_delete() + deactivate_route.delay(route.pk) html = "<html><body>Done</body></html>" return HttpResponse(html) else: diff --git a/flowspec/viewsets.py b/flowspec/viewsets.py index 04af364d..22deab17 100644 --- a/flowspec/viewsets.py +++ b/flowspec/viewsets.py @@ -16,6 +16,8 @@ from flowspec.serializers import ( from flowspec.validators import check_if_rule_exists from rest_framework.response import Response +from flowspec.tasks import * + import os import logging FORMAT = '%(asctime)s %(levelname)s: %(message)s' @@ -153,7 +155,7 @@ class RouteViewSet(viewsets.ModelViewSet): Cases: * `ACTIVE` ~> `INACTIVE`: The `Route` must be deleted from the - flowspec device (`commit_delete`) + flowspec device (`commit_deactivate`) * `ACTIVE` ~> `ACTIVE`: The `Route` is present, so it must be edited (`commit_edit`) @@ -164,7 +166,7 @@ class RouteViewSet(viewsets.ModelViewSet): """ set_object_pending(obj) if new_status == 'INACTIVE': - obj.commit_delete() + deactivate_route.delay(obj.pk) else: obj.commit_edit() @@ -221,74 +223,27 @@ class RouteViewSet(viewsets.ModelViewSet): else: return Response(serializer.errors, status=400) - def pre_save(self, obj): - # DEBUG - if settings.DEBUG: - if self.request.user.is_anonymous(): - from django.contrib.auth.models import User - obj.applier = User.objects.all()[0] - elif self.request.user.is_authenticated(): - obj.applier = self.request.user - else: - raise PermissionDenied('User is not Authenticated') - else: - obj.applier = self.request.user - - def post_save(self, obj, created): - if created: - obj.commit_add() - - def pre_delete(self, obj): - logger.info("RouteViewSet::pre delete(): start") - if True or not self.request.user.is_superuser: - raise PermissionDenied('Permission Denied') - logger.info("RouteViewSet::pre delete: pre commit_delete") - obj.commit_delete() - - def delete(self, request, pk=None, partial=False): + def destroy(self, request, pk=None, partial=False): + """ HTTTP DELETE Method """ obj = get_object_or_404(self.queryset, pk=pk) logger.info("RouteViewSet::delete(): pk="+str(pk)+" obj="+str(obj)) username_request = request.user.username user_is_admin = request.user.is_superuser - full_delete_is_allowed = (user_is_admin and settings.ALLOW_DELETE_FULL_FOR_ADMIN) or settings.ALLOW_DELETE_FULL_FOR_USER_ALL or (username_request in settings.ALLOW_DELETE_FULL_FOR_USER_LIST) + full_delete_is_allowed = request.user.userprofile.is_delete_allowed() logger.info("RouteViewSet::delete(): username_request="+str(username_request)+" user_is_admin="+str(user_is_admin)+" => full_delete_is_allowed="+str(full_delete_is_allowed)) + obj.status = "PENDING" + obj.save() #if True or not self.request.user.is_superuser(): - if not full_delete_is_allowed: - raise PermissionDenied('Permission Denied') - logger.info("RouteViewSet::delete(): pre commit_delete") - obj.commit_delete() - - def destroy(self, request, pk=None): - obj = get_object_or_404(self.queryset, pk=pk) - logger.info("RouteViewSet::destroy(): pk="+str(pk)+" obj="+str(obj)) - logger.info("RouteViewSet::destroy(): pre commit_delete") - - username_request = request.user.username - user_is_admin = request.user.is_superuser - full_delete_is_allowed = (user_is_admin and settings.ALLOW_DELETE_FULL_FOR_ADMIN) or settings.ALLOW_DELETE_FULL_FOR_USER_ALL or settings.ALLOW_DELETE_FULL_FOR_USER_LIST.contains(username_request) - logger.info("RouteViewSet::destroy(): username_request="+str(username_request)+" user_is_admin="+str(user_is_admin)+" => full_delete_is_allowed="+str(full_delete_is_allowed)) - - if obj.status == 'ACTIVE': - if full_delete_is_allowed: - obj.status = "PENDING_TODELETE" - else: - obj.status = "PENDING" - obj.response = "N/A" - obj.save() - obj.commit_delete() - serializer = RouteSerializer(obj, context={'request': request}) - return Response(serializer.data) + if full_delete_is_allowed: + job = delete_route.delay(obj.pk) + return Response({"status": "Received task to delete route.", "job_id": str(job)}, 202) else: - try: - #if not settings.ALLOW_ADMIN__FULL_RULEDEL or not self.request.user.is_superuser: - if not full_delete_is_allowed: - raise PermissionDenied('Permission Denied') - except Exception as e: - raise PermissionDenied('Permission Denied') - # this will delete the rule from DB - return super(RouteViewSet, self).destroy(self, request, pk=pk) + if obj.status == "INACTIVE": + return Response({"status": "Cannot deactivate route that is inactive already."}, 406) + job = deactivate_route.delay(obj.pk) + return Response({"status": "Received task to deactivate route.", "job_id": str(job)}, 202) class ThenActionViewSet(viewsets.ModelViewSet): queryset = ThenAction.objects.all() diff --git a/flowspy/settings.py.dist b/flowspy/settings.py.dist index 12f88a65..94a3d424 100644 --- a/flowspy/settings.py.dist +++ b/flowspy/settings.py.dist @@ -592,4 +592,7 @@ ENABLE_SETUP_VIEW = False ############################################################################## ############################################################################## +# Number of retries when NECONF fails before it ends in the ERROR state +NETCONF_MAX_RETRY_BEFORE_ERROR = 5 + from flowspy.settings_local import * diff --git a/flowspy/urls.py b/flowspy/urls.py index 4566c4fe..22d64d32 100644 --- a/flowspy/urls.py +++ b/flowspy/urls.py @@ -40,7 +40,8 @@ urlpatterns = [ url(r'^add/?$', flowspec_views.add_route, name="add-route"), url(r'^addport/?$', flowspec_views.add_port, name="add-port"), url(r'^edit/(?P<route_slug>[\w\-]+)/$', flowspec_views.edit_route, name="edit-route"), - url(r'^delete/(?P<route_slug>[\w\-]+)/$', flowspec_views.delete_route, name="delete-route"), + url(r'^delete/(?P<route_slug>[\w\-]+)/$', flowspec_views.delete_route_view, name="delete-route"), + url(r'^deactivate/(?P<route_slug>[\w\-]+)/$', flowspec_views.deactivate_route_view, name="deactivate-route"), url(r'^login/?', flowspec_views.user_login, name="login"), path('welcome/', flowspec_views.welcome, name="welcome"), url(r'^logout/?', flowspec_views.user_logout, name="logout"), diff --git a/templates/user_routes.html b/templates/user_routes.html index 837d3e92..3d2475bd 100644 --- a/templates/user_routes.html +++ b/templates/user_routes.html @@ -227,7 +227,7 @@ $(document).ready( function(){ var my = $(this); my.html('Deactivating...') var routename = $(this).data("routename"); - var delurl = "{% url 'delete-route' 'route_placeholder'%}".replace('route_placeholder', routename.toString()); + var delurl = "{% url 'deactivate-route' 'route_placeholder'%}".replace('route_placeholder', routename.toString()); $.ajax({ type: 'POST', url: delurl, -- GitLab