Merge branch 'feature-port-operations' into 'master'

Feature port operations

👌

See merge request !256

circle/dashboard/forms.py

@@ -40,7 +40,7 @@ from django.contrib.auth.forms import UserCreationForm as OrgUserCreationForm
 from django.forms.widgets import TextInput, HiddenInput
 from django.template import Context
 from django.template.loader import render_to_string
-from django.utils.html import escape
+from django.utils.html import escape, format_html
 from django.utils.translation import ugettext_lazy as _
 from sizefield.widgets import FileSizeWidget
 from django.core.urlresolvers import reverse_lazy
@@ -935,6 +935,56 @@ class VmDeployForm(OperationForm):
                     "(blank allows scheduling automatically).")))
 
 
+class VmPortRemoveForm(OperationForm):
+    def __init__(self, *args, **kwargs):
+        choices = kwargs.pop('choices')
+        self.rule = kwargs.pop('default')
+
+        super(VmPortRemoveForm, self).__init__(*args, **kwargs)
+
+        self.fields.insert(0, 'rule', forms.ModelChoiceField(
+            queryset=choices, initial=self.rule, required=True,
+            empty_label=None, label=_('Port')))
+        if self.rule:
+            self.fields['rule'].widget = HiddenInput()
+
+
+class VmPortAddForm(OperationForm):
+    port = forms.IntegerField(required=True, label=_('Port'),
+                              min_value=1, max_value=65535)
+    proto = forms.ChoiceField((('tcp', 'tcp'), ('udp', 'udp')),
+                              required=True, label=_('Protocol'))
+
+    def __init__(self, *args, **kwargs):
+        choices = kwargs.pop('choices')
+        self.host = kwargs.pop('default')
+
+        super(VmPortAddForm, self).__init__(*args, **kwargs)
+
+        self.fields.insert(0, 'host', forms.ModelChoiceField(
+            queryset=choices, initial=self.host, required=True,
+            empty_label=None, label=_('Host')))
+        if self.host:
+            self.fields['host'].widget = HiddenInput()
+
+    @property
+    def helper(self):
+        helper = super(VmPortAddForm, self).helper
+        if self.host:
+            helper.layout = Layout(
+                AnyTag(
+                    "div",
+                    HTML(format_html(
+                        _("<label>Host:</label> {0}"), self.host)),
+                    css_class="form-group",
+                ),
+                Field("host"),
+                Field("proto"),
+                Field("port"),
+            )
+        return helper
+
+
 class CircleAuthenticationForm(AuthenticationForm):
     # fields: username, password
 

circle/dashboard/templates/dashboard/_vm-remove-port.html

+{% extends "dashboard/operate.html" %}
+{% load i18n %}
+{% load crispy_forms_tags %}
+
+{% block formfields %}
+  {% if form %}
+    {% crispy form %}
+  {% endif %}
+
+  {% if form.fields.rule.initial != None %}
+    {% with rule=form.fields.rule.initial %}
+      <dl>
+        <dt>{% trans "Port" %}:</dt>
+        <dd>{{ rule.dport }}/{{ rule.proto }}</dd>
+        <dt>{% trans "Host" %}:</dt>
+        <dd>{{ rule.host.hostname }}</dd>
+        <dt>{% trans "Vlan" %}:</dt>
+        <dd>{{ rule.host.vlan.name }}</dd>
+      </dl>
+    {% endwith %}
+  {% endif %}
+
+{% endblock %}

circle/dashboard/templates/dashboard/vm-detail/_network-port-add.html

 {% load i18n %}
 <div class="vm-details-network-port-add pull-right">
-  <form action="" method="POST">
+  <form action="{{ op.add_port.get_url }}" method="POST">
     {% csrf_token %}
-    <input type="hidden" name="host_pk" value="{{ i.host.pk }}"/>
+    <input type="hidden" name="host" value="{{ i.host.pk }}"/>
     <div class="input-group input-group-sm">
       <span class="input-group-addon">
         <i class="fa fa-plus"></i> <i class="fa fa-long-arrow-right"></i>
       </span>
-      <input type="text" class="form-control" size="5" style="width: 80px;" name="port"/>
+      <input type="number" class="form-control" size="5" min="1" max="65535"
+             style="width: 80px;" name="port" required/>
       <span class="input-group-addon">/</span>
       <select class="form-control" name="proto" style="width: 70px;"><option>tcp</option><option>udp</option></select>
       <div class="input-group-btn">

circle/dashboard/templates/dashboard/vm-detail/network.html

@@ -78,7 +78,7 @@
                         {{ l.private }}/{{ l.proto }}
                       </td>
                       <td>
-                        <a href="{% url "dashboard.views.remove-port" pk=instance.pk rule=l.ipv4.pk %}" class="btn btn-link btn-xs vm-details-remove-port" data-rule="{{ l.ipv4.pk }}" title="{% trans "Remove" %}"><i class="fa fa-times"><span class="sr-only">{% trans "Remove" %}</span></i></a>
+                        <a href="{{ op.remove_port.get_url }}?rule={{ l.ipv4.pk }}" class="btn btn-link btn-xs vm-details-remove-port" data-rule="{{ l.ipv4.pk }}" title="{% trans "Remove" %}"><i class="fa fa-times"><span class="sr-only">{% trans "Remove" %}</span></i></a>
                       </td>
                     </tr>
                   {% endif %}
@@ -110,7 +110,7 @@
                         {{ l.private }}/{{ l.proto }}
                       </td>
                       <td>
-                        <a href="{% url "dashboard.views.remove-port" pk=instance.pk rule=l.ipv4.pk %}" class="btn btn-link btn-xs vm-details-remove-port" data-rule="{{ l.ipv6.pk }}" title="{% trans "Remove" %}"><i class="fa fa-times"><span class="sr-only">{% trans "Remove" %}</span></i></a>
+                        <a href="{{ op.remove_port.get_url }}?rule={{ l.ipv4.pk }}" class="btn btn-link btn-xs vm-details-remove-port" data-rule="{{ l.ipv6.pk }}" title="{% trans "Remove" %}"><i class="fa fa-times"><span class="sr-only">{% trans "Remove" %}</span></i></a>
                       </td>
                     </tr>
                   {% endif %}

circle/dashboard/tests/test_views.py

@@ -26,7 +26,8 @@ from django.contrib.auth import authenticate
 
 from dashboard.views import VmAddInterfaceView
 from vm.models import Instance, InstanceTemplate, Lease, Node, Trait
-from vm.operations import WakeUpOperation, AddInterfaceOperation
+from vm.operations import (WakeUpOperation, AddInterfaceOperation,
+                           AddPortOperation)
 from ..models import Profile
 from firewall.models import Vlan, Host, VlanGroup
 from mock import Mock, patch
@@ -335,51 +336,48 @@ class VmDetailTest(LoginMixin, TestCase):
         self.login(c, "user2")
         inst = Instance.objects.get(pk=1)
         inst.set_level(self.u2, 'owner')
-        response = c.post("/dashboard/vm/1/", {'port': True,
-                                               'proto': 'tcp',
-                                               'port': '1337'})
+        vlan = Vlan.objects.get(id=1)
+        vlan.set_level(self.u2, 'user')
+        inst.add_interface(user=self.u2, vlan=vlan)
+        host = Host.objects.get(
+            interface__in=inst.interface_set.all())
+        with patch.object(AddPortOperation, 'async') as mock_method:
+            mock_method.side_effect = inst.add_port
+            response = c.post("/dashboard/vm/1/op/add_port/", {
+                'proto': 'tcp', 'host': host.pk, 'port': '1337'})
         self.assertEqual(response.status_code, 403)
 
     def test_unpermitted_add_port_wo_obj_levels(self):
         c = Client()
         self.login(c, "user2")
-        self.u2.user_permissions.add(Permission.objects.get(
-            name='Can configure port forwards.'))
-        response = c.post("/dashboard/vm/1/", {'port': True,
-                                               'proto': 'tcp',
-                                               'port': '1337'})
-        self.assertEqual(response.status_code, 403)
-
-    def test_unpermitted_add_port_w_bad_host(self):
-        c = Client()
-        self.login(c, "user2")
         inst = Instance.objects.get(pk=1)
-        inst.set_level(self.u2, 'owner')
+        vlan = Vlan.objects.get(id=1)
+        vlan.set_level(self.u2, 'user')
+        inst.add_interface(user=self.u2, vlan=vlan, system=True)
+        host = Host.objects.get(
+            interface__in=inst.interface_set.all())
         self.u2.user_permissions.add(Permission.objects.get(
             name='Can configure port forwards.'))
-        response = c.post("/dashboard/vm/1/", {'proto': 'tcp',
-                                               'host_pk': '9999',
-                                               'port': '1337'})
+        with patch.object(AddPortOperation, 'async') as mock_method:
+            mock_method.side_effect = inst.add_port
+            response = c.post("/dashboard/vm/1/op/add_port/", {
+                'proto': 'tcp', 'host': host.pk, 'port': '1337'})
+            assert not mock_method.called
         self.assertEqual(response.status_code, 403)
 
-    def test_permitted_add_port_w_unhandled_exception(self):
+    def test_unpermitted_add_port_w_bad_host(self):
         c = Client()
         self.login(c, "user2")
         inst = Instance.objects.get(pk=1)
         inst.set_level(self.u2, 'owner')
-        vlan = Vlan.objects.get(id=1)
-        vlan.set_level(self.u2, 'user')
-        inst.add_interface(user=self.u2, vlan=vlan)
-        host = Host.objects.get(
-            interface__in=inst.interface_set.all())
         self.u2.user_permissions.add(Permission.objects.get(
             name='Can configure port forwards.'))
-        port_count = len(host.list_ports())
-        response = c.post("/dashboard/vm/1/", {'proto': 'tcp',
-                                               'host_pk': host.pk,
-                                               'port': 'invalid_port'})
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(len(host.list_ports()), port_count)
+        with patch.object(AddPortOperation, 'async') as mock_method:
+            mock_method.side_effect = inst.add_port
+            response = c.post("/dashboard/vm/1/op/add_port/", {
+                'proto': 'tcp', 'host': '9999', 'port': '1337'})
+            assert not mock_method.called
+        self.assertEqual(response.status_code, 200)
 
     def test_permitted_add_port(self):
         c = Client()
@@ -394,9 +392,11 @@ class VmDetailTest(LoginMixin, TestCase):
         self.u2.user_permissions.add(Permission.objects.get(
             name='Can configure port forwards.'))
         port_count = len(host.list_ports())
-        response = c.post("/dashboard/vm/1/", {'proto': 'tcp',
-                                               'host_pk': host.pk,
-                                               'port': '1337'})
+        with patch.object(AddPortOperation, 'async') as mock_method:
+            mock_method.side_effect = inst.add_port
+            response = c.post("/dashboard/vm/1/op/add_port/", {
+                'proto': 'tcp', 'host': host.pk, 'port': '1337'})
+            assert mock_method.called
         self.assertEqual(response.status_code, 302)
         self.assertEqual(len(host.list_ports()), port_count + 1)
 

circle/dashboard/urls.py

@@ -26,7 +26,7 @@ from .views import (
     InstanceActivityDetail, LeaseCreate, LeaseDelete, LeaseDetail,
     MyPreferencesView, NodeAddTraitView, NodeCreate, NodeDelete,
     NodeDetailView, NodeList, NodeStatus,
-    NotificationView, PortDelete, TemplateAclUpdateView, TemplateCreate,
+    NotificationView, TemplateAclUpdateView, TemplateCreate,
     TemplateDelete, TemplateDetail, TemplateList,
     vm_activity, VmCreate, VmDetailView,
     VmDetailVncTokenView, VmList,
@@ -82,8 +82,6 @@ urlpatterns = patterns(
         name="dashboard.views.template-delete"),
     url(r'^template/(?P<pk>\d+)/tx/$', TransferTemplateOwnershipView.as_view(),
         name='dashboard.views.template-transfer-ownership'),
-    url(r'^vm/(?P<pk>\d+)/remove_port/(?P<rule>\d+)/$', PortDelete.as_view(),
-        name='dashboard.views.remove-port'),
     url(r'^vm/(?P<pk>\d+)/$', VmDetailView.as_view(),
         name='dashboard.views.detail'),
     url(r'^vm/(?P<pk>\d+)/vnctoken/$', VmDetailVncTokenView.as_view(),

circle/dashboard/views/vm.py

@@ -63,6 +63,7 @@ from ..forms import (
     VmRenewForm, VmStateChangeForm, VmListSearchForm, VmCustomizeForm,
     VmDiskResizeForm, RedeployForm, VmDiskRemoveForm,
     VmMigrateForm, VmDeployForm,
+    VmPortRemoveForm, VmPortAddForm,
 )
 from ..models import Favourite
 
@@ -175,7 +176,6 @@ class VmDetailView(GraphMixin, CheckedDetailView):
             'new_description': self.__set_description,
             'new_tag': self.__add_tag,
             'to_remove': self.__remove_tag,
-            'port': self.__add_port,
             'abort_operation': self.__abort_operation,
         }
         for k, v in options.iteritems():
@@ -271,40 +271,6 @@ class VmDetailView(GraphMixin, CheckedDetailView):
             return redirect(reverse_lazy("dashboard.views.detail",
                             kwargs={'pk': self.object.pk}))
 
-    def __add_port(self, request):
-        object = self.get_object()
-        if not (object.has_level(request.user, "operator") and
-                request.user.has_perm('vm.config_ports')):
-            raise PermissionDenied()
-
-        port = request.POST.get("port")
-        proto = request.POST.get("proto")
-
-        try:
-            error = None
-            interfaces = object.interface_set.all()
-            host = Host.objects.get(pk=request.POST.get("host_pk"),
-                                    interface__in=interfaces)
-            host.add_port(proto, private=port)
-        except Host.DoesNotExist:
-            logger.error('Tried to add port to nonexistent host %d. User: %s. '
-                         'Instance: %s', request.POST.get("host_pk"),
-                         unicode(request.user), object)
-            raise PermissionDenied()
-        except ValueError:
-            error = _("There is a problem with your input.")
-        except Exception as e:
-            error = _("Unknown error.")
-            logger.error(e)
-
-        if request.is_ajax():
-            pass
-        else:
-            if error:
-                messages.error(request, error)
-            return redirect(reverse_lazy("dashboard.views.detail",
-                                         kwargs={'pk': self.get_object().pk}))
-
     def __abort_operation(self, request):
         self.object = self.get_object()
 
@@ -451,6 +417,62 @@ class VmMigrateView(FormOperationMixin, VmOperationView):
         return val
 
 
+class VmPortRemoveView(FormOperationMixin, VmOperationView):
+
+    template_name = 'dashboard/_vm-remove-port.html'
+    op = 'remove_port'
+    show_in_toolbar = False
+    with_reload = True
+    wait_for_result = 0.5
+    icon = 'times'
+    effect = "danger"
+    form_class = VmPortRemoveForm
+
+    def get_form_kwargs(self):
+        instance = self.get_op().instance
+        choices = Rule.portforwards().filter(
+            host__interface__instance=instance)
+        rule_pk = self.request.GET.get('rule')
+        if rule_pk:
+            try:
+                default = choices.get(pk=rule_pk)
+            except (ValueError, Rule.DoesNotExist):
+                raise Http404()
+        else:
+            default = None
+
+        val = super(VmPortRemoveView, self).get_form_kwargs()
+        val.update({'choices': choices, 'default': default})
+        return val
+
+
+class VmPortAddView(FormOperationMixin, VmOperationView):
+
+    op = 'add_port'
+    show_in_toolbar = False
+    with_reload = True
+    wait_for_result = 0.5
+    icon = 'plus'
+    effect = "success"
+    form_class = VmPortAddForm
+
+    def get_form_kwargs(self):
+        instance = self.get_op().instance
+        choices = Host.objects.filter(interface__instance=instance)
+        host_pk = self.request.GET.get('host')
+        if host_pk:
+            try:
+                default = choices.get(pk=host_pk)
+            except (ValueError, Host.DoesNotExist):
+                raise Http404()
+        else:
+            default = None
+
+        val = super(VmPortAddView, self).get_form_kwargs()
+        val.update({'choices': choices, 'default': default})
+        return val
+
+
 class VmSaveView(FormOperationMixin, VmOperationView):
 
     op = 'save_as_template'
@@ -684,6 +706,8 @@ vm_ops = OrderedDict([
         op='remove_disk', form_class=VmDiskRemoveForm,
         icon='times', effect="danger")),
     ('add_interface', VmAddInterfaceView),
+    ('remove_port', VmPortRemoveView),
+    ('add_port', VmPortAddView),
     ('renew', VmRenewView),
     ('resources_change', VmResourcesChangeView),
     ('password_reset', VmOperationView.factory(
@@ -1167,52 +1191,6 @@ def get_disk_download_status(request, pk):
     )
 
 
-class PortDelete(LoginRequiredMixin, DeleteView):
-    model = Rule
-    pk_url_kwarg = 'rule'
-
-    def get_template_names(self):
-        if self.request.is_ajax():
-            return ['dashboard/confirm/ajax-delete.html']
-        else:
-            return ['dashboard/confirm/base-delete.html']
-
-    def get_context_data(self, **kwargs):
-        context = super(PortDelete, self).get_context_data(**kwargs)
-        rule = kwargs.get('object')
-        instance = rule.host.interface_set.get().instance
-        context['title'] = _("Port delete confirmation")
-        context['text'] = _("Are you sure you want to close %(port)d/"
-                            "%(proto)s on %(vm)s?" % {'port': rule.dport,
-                                                      'proto': rule.proto,
-                                                      'vm': instance})
-        return context
-
-    def delete(self, request, *args, **kwargs):
-        rule = Rule.objects.get(pk=kwargs.get("rule"))
-        instance = rule.host.interface_set.get().instance
-        if not instance.has_level(request.user, 'owner'):
-            raise PermissionDenied()
-
-        super(PortDelete, self).delete(request, *args, **kwargs)
-
-        success_url = self.get_success_url()
-        success_message = _("Port successfully removed.")
-
-        if request.is_ajax():
-            return HttpResponse(
-                json.dumps({'message': success_message}),
-                content_type="application/json",
-            )
-        else:
-            messages.success(request, success_message)
-            return HttpResponseRedirect("%s#network" % success_url)
-
-    def get_success_url(self):
-        return reverse_lazy('dashboard.views.detail',
-                            kwargs={'pk': self.kwargs.get("pk")})
-
-
 class ClientCheck(LoginRequiredMixin, TemplateView):
 
     def get_template_names(self):

circle/firewall/fields.py

@@ -34,6 +34,10 @@ reverse_domain_re = re.compile(r'^(%\([abcd]\)d|[a-z0-9.-])+$')
 ipv6_template_re = re.compile(r'^(%\([abcd]\)[dxX]|[A-Za-z0-9:-])+$')
 
 
+class mac_custom(mac_unix):
+    word_fmt = '%.2X'
+
+
 class MACAddressFormField(forms.Field):
     default_error_messages = {
         'invalid': _(u'Enter a valid MAC address. %s'),
@@ -51,9 +55,6 @@ class MACAddressField(models.Field):
     description = _('MAC Address object')
     __metaclass__ = models.SubfieldBase
 
-    class mac_custom(mac_unix):
-        word_fmt = '%.2X'
-
     def __init__(self, *args, **kwargs):
         kwargs['max_length'] = 17
         super(MACAddressField, self).__init__(*args, **kwargs)
@@ -65,7 +66,7 @@ class MACAddressField(models.Field):
         if isinstance(value, EUI):
             return value
 
-        return EUI(value, dialect=MACAddressField.mac_custom)
+        return EUI(value, dialect=mac_custom)
 
     def get_internal_type(self):
         return 'CharField'

circle/firewall/models.py

@@ -243,6 +243,13 @@ class Rule(models.Model):
 
         return retval
 
+    @classmethod
+    def portforwards(cls, host=None):
+        qs = cls.objects.filter(dport__isnull=False, direction='in')
+        if host is not None:
+            qs = qs.filter(host=host)
+        return qs
+
     class Meta:
         verbose_name = _("rule")
         verbose_name_plural = _("rules")
@@ -762,7 +769,7 @@ class Host(models.Model):
         Return a list of ports with forwarding rules set.
         """
         retval = []
-        for rule in self.rules.filter(dport__isnull=False, direction='in'):
+        for rule in Rule.portforwards(host=self):
             forward = {
                 'proto': rule.proto,
                 'private': rule.dport,

circle/vm/models/instance.py

@@ -817,7 +817,7 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
         return acts
 
     def get_merged_activities(self, user=None):
-        whitelist = ("create_disk", "download_disk")
+        whitelist = ("create_disk", "download_disk", "add_port", "remove_port")
         acts = self.get_activities(user)
         merged_acts = []
         latest = None

circle/vm/operations.py

@@ -27,7 +27,7 @@ from tarfile import TarFile, TarInfo
 import time
 from urlparse import urlsplit
 
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import PermissionDenied, SuspiciousOperation
 from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _, ugettext_noop
 from django.conf import settings
@@ -606,6 +606,41 @@ class RemoveInterfaceOperation(InstanceOperation):
 
 
 @register_operation
+class RemovePortOperation(InstanceOperation):
+    id = 'remove_port'
+    name = _("close port")
+    description = _("Close the specified port.")
+    concurrency_check = False
+    required_perms = ('vm.config_ports', )
+
+    def _operation(self, activity, rule):
+        interface = rule.host.interface_set.get()
+        if interface.instance != self.instance:
+            raise SuspiciousOperation()
+        activity.readable_name = create_readable(
+            ugettext_noop("close %(proto)s/%(port)d on %(host)s"),
+            proto=rule.proto, port=rule.dport, host=rule.host)
+        rule.delete()
+
+
+@register_operation
+class AddPortOperation(InstanceOperation):
+    id = 'add_port'
+    name = _("open port")
+    description = _("Open the specified port.")
+    concurrency_check = False
+    required_perms = ('vm.config_ports', )
+
+    def _operation(self, activity, host, proto, port):
+        if host.interface_set.get().instance != self.instance:
+            raise SuspiciousOperation()
+        host.add_port(proto, private=port)
+        activity.readable_name = create_readable(
+            ugettext_noop("open %(proto)s/%(port)d on %(host)s"),
+            proto=proto, port=port, host=host)
+
+
+@register_operation
 class RemoveDiskOperation(InstanceOperation):
     id = 'remove_disk'
     name = _("remove disk")