cyclades: Use NIC UUID in firewall tags

Use NICs UUID instead of index to generate firewall tags. Make the
'set_firewall_profile' action to take the NICs ID as argument. Also,
update the firewall settings.

snf-cyclades-app/conf/20-snf-cyclades-app-api.conf

@@ -49,11 +49,11 @@
 #DEFAULT_MAC_FILTERED_BRIDGE = 'prv0'
 #
 #
-## Firewall tags should contain '%d' to be filled with the NIC
-## index.
-#GANETI_FIREWALL_ENABLED_TAG = 'synnefo:network:0:protected'
-#GANETI_FIREWALL_DISABLED_TAG = 'synnefo:network:0:unprotected'
-#GANETI_FIREWALL_PROTECTED_TAG = 'synnefo:network:0:limited'
+## Firewall tags should contain '%s' to be filled with the NIC
+## ID.
+#GANETI_FIREWALL_ENABLED_TAG = 'synnefo:network:%s:protected'
+#GANETI_FIREWALL_DISABLED_TAG = 'synnefo:network:%s:unprotected'
+#GANETI_FIREWALL_PROTECTED_TAG = 'synnefo:network:%s:limited'
 #
 ## The default firewall profile that will be in effect if no tags are defined
 #DEFAULT_FIREWALL_PROFILE = 'DISABLED'

snf-cyclades-app/synnefo/api/servers.py

@@ -756,8 +756,11 @@ def set_firewall_profile(request, vm, args):
     profile = args.get("profile")
     if profile is None:
         raise faults.BadRequest("Missing 'profile' attribute")
-    index = args.get("index", 0)
-    servers.set_firewall_profile(vm, profile=profile, index=index)
+    nic_id = args.get("nic")
+    if nic_id is None:
+        raise faults.BadRequest("Missing 'nic' attribute")
+    nic = util.get_nic(vm, nic_id)
+    servers.set_firewall_profile(vm, profile=profile, nic=nic)
     return HttpResponse(status=202)
 
 

snf-cyclades-app/synnefo/api/tests/servers.py

@@ -672,7 +672,16 @@ class ServerActionAPITest(ComputeAPITest):
         request = {'firewallProfile': {'profile': 'PROTECTED'}}
         response = self.mypost('servers/%d/action' % vm.id,
                                vm.userid, json.dumps(request), 'json')
-        self.assertEqual(response.status_code, 202)
+        self.assertBadRequest(response)
+        request = {'firewallProfile': {'profile': 'PROTECTED', "nic": "10"}}
+        response = self.mypost('servers/%d/action' % vm.id,
+                               vm.userid, json.dumps(request), 'json')
+        self.assertItemNotFound(response)
+        nic = mfactory.NetworkInterfaceFactory(machine=vm)
+        request = {'firewallProfile': {'profile': 'PROTECTED', "nic": nic.id}}
+        response = self.mypost('servers/%d/action' % vm.id,
+                               vm.userid, json.dumps(request), 'json')
+        self.assertSuccess(response)
         mrapi().ModifyInstance.assert_called_once()
 
     def test_unsupported_firewall(self, mrapi, mimage):

snf-cyclades-app/synnefo/api/util.py

@@ -176,7 +176,6 @@ def get_image_dict(image_id, user_id):
     image["metadata"] = dict((key.upper(), val)
                              for key, val in properties.items())
 
-
     return image
 
 
@@ -286,10 +285,11 @@ def get_network_free_address(network):
 
 
 def get_nic(vm, nic_id):
+    """Get a VMs NIC by its ID."""
     try:
         return vm.nics.get(id=nic_id)
     except NetworkInterface.DoesNotExist:
-        raise faults.ItemNotFound('Server not connected to this network.')
+        raise faults.ItemNotFound("NIC '%s' not found" % nic_id)
 
 
 def render_metadata(request, metadata, use_values=False, status=200):

snf-cyclades-app/synnefo/app_settings/default/api.py

@@ -49,10 +49,10 @@ DEFAULT_MAC_FILTERED_BRIDGE = 'prv0'
 
 
 # Firewalling. Firewall tags should contain '%d' to be filled with the NIC
-# index.
-GANETI_FIREWALL_ENABLED_TAG = 'synnefo:network:%d:protected'
-GANETI_FIREWALL_DISABLED_TAG = 'synnefo:network:%d:unprotected'
-GANETI_FIREWALL_PROTECTED_TAG = 'synnefo:network:%d:limited'
+# ID.
+GANETI_FIREWALL_ENABLED_TAG = 'synnefo:network:%s:protected'
+GANETI_FIREWALL_DISABLED_TAG = 'synnefo:network:%s:unprotected'
+GANETI_FIREWALL_PROTECTED_TAG = 'synnefo:network:%s:limited'
 
 # The default firewall profile that will be in effect if no tags are defined
 DEFAULT_FIREWALL_PROFILE = 'DISABLED'

snf-cyclades-app/synnefo/logic/backend.py

@@ -844,34 +844,29 @@ def disconnect_from_network(vm, nic):
 
     with pooled_rapi_client(vm) as client:
         jobID = client.ModifyInstance(**kwargs)
-        # If the NIC has a tag for a firewall profile it must be deleted,
-        # otherwise it may affect another NIC. XXX: Deleting the tag should
-        # depend on the removing the NIC, but currently RAPI client does not
-        # support this, this may result in clearing the firewall profile
-        # without successfully removing the NIC. This issue will be fixed with
-        # use of NIC UUIDs.
         firewall_profile = nic.firewall_profile
         if firewall_profile and firewall_profile != "DISABLED":
-            tag = _firewall_tags[firewall_profile] % nic.index
+            tag = _firewall_tags[firewall_profile] % nic.backend_uuid
             client.DeleteInstanceTags(vm.backend_vm_id, [tag],
                                       dry_run=settings.TEST)
 
         return jobID
 
 
-def set_firewall_profile(vm, profile, index=0):
+def set_firewall_profile(vm, profile, nic):
+    uuid = nic.backend_uuid
     try:
-        tag = _firewall_tags[profile] % index
+        tag = _firewall_tags[profile] % uuid
     except KeyError:
         raise ValueError("Unsopported Firewall Profile: %s" % profile)
 
-    log.debug("Setting tag of VM %s, NIC index %d, to %s", vm, index, profile)
+    log.debug("Setting tag of VM %s, NIC %s, to %s", vm, nic, profile)
 
     with pooled_rapi_client(vm) as client:
         # Delete previous firewall tags
         old_tags = client.GetInstanceTags(vm.backend_vm_id)
-        delete_tags = [(t % index) for t in _firewall_tags.values()
-                       if (t % index) in old_tags]
+        delete_tags = [(t % uuid) for t in _firewall_tags.values()
+                       if (t % uuid) in old_tags]
         if delete_tags:
             client.DeleteInstanceTags(vm.backend_vm_id, delete_tags,
                                       dry_run=settings.TEST)

snf-cyclades-app/synnefo/logic/reconciliation.py

@@ -454,14 +454,10 @@ def nics_from_instance(i):
             if len(t) != 4:
                 logger.error("Malformed synefo tag %s", tag)
                 continue
-            try:
-                index = int(t[2])
-                nics[index]['firewall'] = t[3]
-            except ValueError:
-                logger.error("Malformed synnefo tag %s", tag)
-            except IndexError:
-                logger.error("Found tag %s for non-existent NIC %d",
-                             tag, index)
+            nic_name = t[2]
+            firewall = t[3]
+            [nic.setdefault("firewall", firewall)
+             for nic in nics if nic["name"] == nic_name]
     return nics
 
 

snf-cyclades-app/synnefo/logic/servers.py

@@ -337,12 +337,12 @@ def resize(vm, flavor):
 
 
 @server_command("SET_FIREWALL_PROFILE")
-def set_firewall_profile(vm, profile, index=0):
-    log.info("Setting VM %s, NIC index %s, firewall %s", vm, index, profile)
+def set_firewall_profile(vm, profile, nic):
+    log.info("Setting VM %s, NIC %s, firewall %s", vm, nic, profile)
 
     if profile not in [x[0] for x in NetworkInterface.FIREWALL_PROFILES]:
         raise faults.BadRequest("Unsupported firewall profile")
-    backend.set_firewall_profile(vm, profile=profile, index=index)
+    backend.set_firewall_profile(vm, profile=profile, nic=nic)
     return None
 
 

snf-cyclades-gtools/synnefo/ganeti/eventd.py

@@ -138,14 +138,10 @@ def get_instance_nics(instance, logger):
             if len(t) != 4:
                 logger.error("Malformed synefo tag %s", tag)
                 continue
-            try:
-                index = int(t[2])
-                nics[index]['firewall'] = t[3]
-            except ValueError:
-                logger.error("Malformed synnefo tag %s", tag)
-            except IndexError:
-                logger.error("Found tag %s for non-existent NIC %d",
-                             tag, index)
+            nic_name = t[2]
+            firewall = t[3]
+            [nic.setdefault("firewall", firewall)
+             for nic in nics if nic["name"] == nic_name]
     return nics