Browse Source

Merge pull request #89 from gvlproject/firewall_refactor

Refactor firewalls
Nuwan Goonasekera 8 years ago
parent
commit
1e14c345aa

+ 102 - 39
cloudbridge/cloud/base/resources.py

@@ -34,12 +34,12 @@ from cloudbridge.cloud.interfaces.resources import PlacementZone
 from cloudbridge.cloud.interfaces.resources import Region
 from cloudbridge.cloud.interfaces.resources import ResultList
 from cloudbridge.cloud.interfaces.resources import Router
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
-from cloudbridge.cloud.interfaces.resources import SecurityGroupRule
 from cloudbridge.cloud.interfaces.resources import Snapshot
 from cloudbridge.cloud.interfaces.resources import SnapshotState
 from cloudbridge.cloud.interfaces.resources import Subnet
 from cloudbridge.cloud.interfaces.resources import SubnetState
+from cloudbridge.cloud.interfaces.resources import VMFirewall
+from cloudbridge.cloud.interfaces.resources import VMFirewallRule
 from cloudbridge.cloud.interfaces.resources import VMType
 from cloudbridge.cloud.interfaces.resources import Volume
 from cloudbridge.cloud.interfaces.resources import VolumeState
@@ -395,7 +395,7 @@ class BaseInstance(BaseCloudResource, BaseObjectLifeCycleMixin, Instance):
                 # check from most to least likely mutables
                 self.state == other.state and
                 self.name == other.name and
-                self.security_groups == other.security_groups and
+                self.vm_firewalls == other.vm_firewalls and
                 self.public_ips == other.public_ips and
                 self.private_ips == other.private_ips and
                 self.image_id == other.image_id)
@@ -614,20 +614,19 @@ class BaseKeyPair(BaseCloudResource, KeyPair):
         return "<CBKeyPair: {0}>".format(self.name)
 
 
-class BaseSecurityGroup(BaseCloudResource, SecurityGroup):
+class BaseVMFirewall(BaseCloudResource, VMFirewall):
 
-    def __init__(self, provider, security_group):
-        super(BaseSecurityGroup, self).__init__(provider)
-        self._security_group = security_group
+    def __init__(self, provider, vm_firewall):
+        super(BaseVMFirewall, self).__init__(provider)
+        self._vm_firewall = vm_firewall
 
     def __eq__(self, other):
         """
-        Check if all the defined rules match across both security groups.
+        Check if all the defined rules match across both VM firewalls.
         """
-        return (isinstance(other, SecurityGroup) and
+        return (isinstance(other, VMFirewall) and
                 # pylint:disable=protected-access
                 self._provider == other._provider and
-                len(self.rules) == len(other.rules) and  # Shortcut
                 set(self.rules) == set(other.rules))
 
     def __ne__(self, other):
@@ -636,61 +635,118 @@ class BaseSecurityGroup(BaseCloudResource, SecurityGroup):
     @property
     def id(self):
         """
-        Get the ID of this security group.
+        Get the ID of this VM firewall.
 
         :rtype: str
-        :return: Security group ID
+        :return: VM firewall ID
         """
-        return self._security_group.id
+        return self._vm_firewall.id
 
     @property
     def name(self):
         """
-        Return the name of this security group.
+        Return the name of this VM firewall.
         """
-        return self._security_group.name
+        return self._vm_firewall.name
 
     @property
     def description(self):
         """
-        Return the description of this security group.
+        Return the description of this VM firewall.
         """
-        return self._security_group.description
+        return self._vm_firewall.description
 
     def delete(self):
         """
-        Delete this security group.
+        Delete this VM firewall.
         """
-        return self._security_group.delete()
+        return self._vm_firewall.delete()
 
     def __repr__(self):
         return "<CB-{0}: {1} ({2})>".format(self.__class__.__name__,
                                             self.id, self.name)
 
 
-class BaseSecurityGroupRule(BaseCloudResource, SecurityGroupRule):
+class BaseVMFirewallRuleContainer(BasePageableObjectMixin):
 
-    def __init__(self, provider, rule, parent):
-        super(BaseSecurityGroupRule, self).__init__(provider)
+    def __init__(self, provider, firewall):
+        self.__provider = provider
+        self.firewall = firewall
+
+    @property
+    def _provider(self):
+        return self.__provider
+
+    def get(self, rule_id):
+        matches = [rule for rule in self if rule.id == rule_id]
+        if matches:
+            return matches[0]
+        else:
+            return None
+
+    def find(self, **kwargs):
+        matches = self
+
+        def filter_by(prop_name, rules):
+            prop_val = kwargs.pop(prop_name, None)
+            if prop_val:
+                match = [r for r in rules if getattr(r, prop_name) == prop_val]
+                return match
+            return rules
+
+        matches = filter_by('name', matches)
+        matches = filter_by('direction', matches)
+        matches = filter_by('protocol', matches)
+        matches = filter_by('from_port', matches)
+        matches = filter_by('to_port', matches)
+        matches = filter_by('cidr', matches)
+        matches = filter_by('src_dest_fw', matches)
+        matches = filter_by('src_dest_fw_id', matches)
+        limit = kwargs.pop('limit', None)
+        marker = kwargs.pop('marker', None)
+
+        return ClientPagedResultList(self._provider, matches,
+                                     limit=limit, marker=marker)
+
+    def delete(self, rule_id):
+        rule = self.get(rule_id)
+        if rule:
+            rule.delete()
+
+
+class BaseVMFirewallRule(BaseCloudResource, VMFirewallRule):
+
+    def __init__(self, parent_fw, rule):
+        # pylint:disable=protected-access
+        super(BaseVMFirewallRule, self).__init__(
+            parent_fw._provider)
+        self.firewall = parent_fw
         self._rule = rule
-        self.parent = parent
 
+        # Cache name
+        self._name = "{0}-{1}-{2}-{3}-{4}-{5}".format(
+            self.direction, self.protocol, self.from_port, self.to_port,
+            self.cidr, self.src_dest_fw_id).lower()
+
+    @property
     def name(self):
-        """
-        Security group rules don't support names, so pass
-        """
-        pass
+        return self._name
 
     def __repr__(self):
-        return ("<CBSecurityGroupRule: IP: {0}; from: {1}; to: {2}; grp: {3}>"
-                .format(self.ip_protocol, self.from_port, self.to_port,
-                        self.group))
+        return ("<{0}: id: {1}; direction: {2}; protocol: {3};  from: {4};"
+                " to: {5}; cidr: {6}, src_dest_fw: {7}>"
+                .format(self.__class__.__name__, self.id, self.direction,
+                        self.protocol, self.from_port, self.to_port, self.cidr,
+                        self.src_dest_fw_id))
 
     def __eq__(self, other):
-        return self.ip_protocol == other.ip_protocol and \
-            self.from_port == other.from_port and \
-            self.to_port == other.to_port and \
-            self.cidr_ip == other.cidr_ip
+        return (isinstance(other, VMFirewallRule) and
+                self.direction == other.direction and
+                self.protocol == other.protocol and
+                self.from_port == other.from_port and
+                self.to_port == other.to_port and
+                self.cidr == other.cidr and
+                self.src_dest_fw_id == other.src_dest_fw_id)
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -699,12 +755,19 @@ class BaseSecurityGroupRule(BaseCloudResource, SecurityGroupRule):
         """
         Return a hash-based interpretation of all of the object's field values.
 
-        This is requried for operations on hashed collections including
+        This is requeried for operations on hashed collections including
         ``set``, ``frozenset``, and ``dict``.
         """
-        return hash("{0}{1}{2}{3}{4}".format(self.ip_protocol, self.from_port,
-                                             self.to_port, self.cidr_ip,
-                                             self.group))
+        return hash("{0}{1}{2}{3}{4}{5}".format(
+            self.direction, self.protocol, self.from_port, self.to_port,
+            self.cidr, self.src_dest_fw_id))
+
+    def to_json(self):
+        attr = inspect.getmembers(self, lambda a: not (inspect.isroutine(a)))
+        js = {k: v for (k, v) in attr if not k.startswith('_')}
+        js['src_dest_fw'] = self.src_dest_fw_id
+        js['firewall'] = self.firewall.id
+        return js
 
 
 class BasePlacementZone(BaseCloudResource, PlacementZone):
@@ -891,7 +954,7 @@ class BaseFloatingIP(BaseCloudResource, FloatingIP):
 
     def name(self):
         """
-        Security group rules don't support names, so pass
+        VM firewall rules don't support names, so pass
         """
         pass
 

+ 4 - 4
cloudbridge/cloud/base/services.py

@@ -15,10 +15,10 @@ from cloudbridge.cloud.interfaces.services import NetworkingService
 from cloudbridge.cloud.interfaces.services import ObjectStoreService
 from cloudbridge.cloud.interfaces.services import RegionService
 from cloudbridge.cloud.interfaces.services import RouterService
-from cloudbridge.cloud.interfaces.services import SecurityGroupService
 from cloudbridge.cloud.interfaces.services import SecurityService
 from cloudbridge.cloud.interfaces.services import SnapshotService
 from cloudbridge.cloud.interfaces.services import SubnetService
+from cloudbridge.cloud.interfaces.services import VMFirewallService
 from cloudbridge.cloud.interfaces.services import VMTypeService
 from cloudbridge.cloud.interfaces.services import VolumeService
 
@@ -105,11 +105,11 @@ class BaseKeyPairService(
         return True
 
 
-class BaseSecurityGroupService(
-        BasePageableObjectMixin, SecurityGroupService, BaseCloudService):
+class BaseVMFirewallService(
+        BasePageableObjectMixin, VMFirewallService, BaseCloudService):
 
     def __init__(self, provider):
-        super(BaseSecurityGroupService, self).__init__(provider)
+        super(BaseVMFirewallService, self).__init__(provider)
 
 
 class BaseVMTypeService(

+ 15 - 2
cloudbridge/cloud/interfaces/exceptions.py

@@ -31,7 +31,7 @@ class InvalidConfigurationException(CloudBridgeBaseException):
 class ProviderConnectionException(CloudBridgeBaseException):
     """
     Marker interface for connection errors to a cloud provider.
-    Thrown when cloudbridge is unable to connect with a provider,
+    Thrown when CloudBridge is unable to connect with a provider,
     for example, when credentials are incorrect, or connection
     settings are invalid.
     """
@@ -41,8 +41,21 @@ class ProviderConnectionException(CloudBridgeBaseException):
 class InvalidNameException(CloudBridgeBaseException):
     """
     Marker interface for any attempt to set an invalid name on
-    a cloudbridge resource.An example would be setting uppercase
+    a CloudBridge resource.An example would be setting uppercase
     letters, which are not allowed in a resource name.
     """
     def __init__(self, msg):
         super(InvalidNameException, self).__init__(msg)
+
+
+class InvalidValueException(CloudBridgeBaseException):
+    """
+    Marker interface for any attempt to set an invalid value on a CloudBridge
+    resource.An example would be setting an unrecognised value for the
+    direction of a firewall rule other than TrafficDirection.INBOUND or
+    TrafficDirection.OUTBOUND.
+    """
+    def __init__(self, param, value):
+        super(InvalidNameException, self).__init__(
+            "Param %s has been given an unrecognised value %s" %
+            (param, value))

+ 1 - 1
cloudbridge/cloud/interfaces/provider.py

@@ -163,7 +163,7 @@ class CloudProvider(object):
         .. code-block:: python
 
             keypairs = provider.security.keypairs.list()
-            security_groups = provider.security.security_groups.list()
+            vm_firewalls = provider.security.vm_firewalls.list()
 
 
         :rtype: ``object`` of :class:`.SecurityService`

+ 149 - 63
cloudbridge/cloud/interfaces/resources.py

@@ -2,6 +2,7 @@
 Specifications for data objects exposed through a provider or service
 """
 from abc import ABCMeta, abstractmethod, abstractproperty
+from enum import Enum
 
 
 class CloudServiceType(object):
@@ -539,22 +540,22 @@ class Instance(ObjectLifeCycleMixin, CloudResource):
 #         pass
 
     @abstractproperty
-    def security_groups(self):
+    def vm_firewalls(self):
         """
-        Get the security groups associated with this instance.
+        Get the firewalls (security groups) associated with this instance.
 
-        :rtype: list or :class:`.SecurityGroup` objects
-        :return: A list of SecurityGroup objects associated with this instance.
+        :rtype: list or :class:`.VMFirewall` objects
+        :return: A list of VMFirewall objects associated with this instance.
         """
         pass
 
     @abstractproperty
-    def security_group_ids(self):
+    def vm_firewall_ids(self):
         """
-        Get the IDs of the security groups associated with this instance.
+        Get the IDs of the VM firewalls associated with this instance.
 
         :rtype: list or :class:``str``
-        :return: A list of the SecurityGroup IDs associated with this instance.
+        :return: A list of the VMFirewall IDs associated with this instance.
         """
         pass
 
@@ -599,22 +600,22 @@ class Instance(ObjectLifeCycleMixin, CloudResource):
         pass
 
     @abstractmethod
-    def add_security_group(self, sg):
+    def add_vm_firewall(self, firewall):
         """
-        Add a security group to this instance
+        Add a VM firewall to this instance
 
-        :type sg: ``SecurityGroup``
-        :param sg: The SecurityGroup to associate with the instance.
+        :type firewall: ``VMFirewall``
+        :param firewall: The VMFirewall to associate with the instance.
         """
         pass
 
     @abstractmethod
-    def remove_security_group(self, sg):
+    def remove_vm_firewall(self, firewall):
         """
-        Remove a security group from this instance
+        Remove a VM firewall from this instance
 
-        :type sg: ``SecurityGroup``
-        :param sg: The SecurityGroup to associate with the instance.
+        :type firewall: ``VMFirewall``
+        :param firewall: The VMFirewall to associate with the instance.
         """
         pass
 
@@ -1672,24 +1673,24 @@ class VMType(CloudResource):
         pass
 
 
-class SecurityGroup(CloudResource):
+class VMFirewall(CloudResource):
 
     __metaclass__ = ABCMeta
 
     @abstractproperty
     def description(self):
         """
-        Return the description of this security group.
+        Return the description of this VM firewall.
 
         :rtype: ``str``
-        :return: A description of this security group.
+        :return: A description of this VM firewall.
         """
         pass
 
     @abstractproperty
     def network_id(self):
         """
-        Network ID with which this security group is associated.
+        Network ID with which this VM firewall is associated.
 
         :rtype: ``str``
         :return: Provider-supplied network ID or ``None`` is not available.
@@ -1699,37 +1700,78 @@ class SecurityGroup(CloudResource):
     @abstractproperty
     def rules(self):
         """
-        Get the list of rules for this security group.
+        Get the list of rules for this VM firewall.
 
-        :rtype: list of :class:`.SecurityGroupRule`
-        :return: A list of security group rule objects.
+        :rtype: list of :class:`.VMFirewallRule`
+        :return: A list of VM firewall rule objects.
         """
         pass
 
+
+class VMFirewallRuleContainer(PageableObjectMixin, CloudResource):
+    """
+    Base interface for Firewall rules.
+    """
+    __metaclass__ = ABCMeta
+
     @abstractmethod
-    def delete(self):
+    def get(self, rule_id):
         """
-        Delete this security group.
+        Returns a firewall rule given its ID. Returns ``None`` if the
+        rule does not exist.
 
-        :rtype: ``bool``
-        :return: ``True`` if successful.
+        Example:
+
+        .. code-block:: python
+
+            fw = provider.security.vm_firewalls.get('my_fw_id')
+            rule = fw.rules.get('rule_id')
+            print(rule.id, rule.name)
+
+        :rtype: :class:`.FirewallRule`
+        :return:  a FirewallRule instance
         """
         pass
 
     @abstractmethod
-    def add_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
+    def list(self, limit=None, marker=None):
+        """
+        List all firewall rules associated with this firewall.
+
+        :rtype: ``list`` of :class:`.FirewallRule`
+        :return:  list of Firewall rule objects
+        """
+        pass
+
+    @abstractmethod
+    def create(self,  direction, protocol=None, from_port=None,
+               to_port=None, cidr=None, src_dest_fw=None):
         """
-        Create a security group rule. If the rule already exists, simply
+        Create a VM firewall rule. If the rule already exists, simply
         returns it.
 
-        You need to pass in either ``src_group`` OR ``ip_protocol`` AND
+        Example:
+
+        .. code-block:: python
+            import TafficDirection from cloudbridge.cloud.interfaces.resources
+
+            fw = provider.security.vm_firewalls.get('my_fw_id')
+            fw.rules.create(TrafficDirection.INBOUND, protocol='tcp',
+                            from_port=80, to_port=80, cidr='10.0.0.0/16')
+            fw.rules.create(TrafficDirection.INBOUND, src_dest_fw=fw)
+            fw.rules.create(TrafficDirection.OUTBOUND, src_dest_fw=fw)
+
+        You need to pass in either ``src_dest_fw`` OR ``protocol`` AND
         ``from_port``, ``to_port``, ``cidr_ip``. In other words, either
         you are authorizing another group or you are authorizing some
-        ip-based rule.
+        IP-based rule.
 
-        :type ip_protocol: ``str``
-        :param ip_protocol: Either ``tcp`` | ``udp`` | ``icmp``.
+        :type direction: :class:``.TrafficDirection``
+        :param direction: Either ``TrafficDirection.INBOUND`` |
+                          ``TrafficDirection.OUTBOUND``
+
+        :type protocol: ``str``
+        :param protocol: Either ``tcp`` | ``udp`` | ``icmp``.
 
         :type from_port: ``int``
         :param from_port: The beginning port number you are enabling.
@@ -1737,30 +1779,30 @@ class SecurityGroup(CloudResource):
         :type to_port: ``int``
         :param to_port: The ending port number you are enabling.
 
-        :type cidr_ip: ``str`` or list of ``str``
-        :param cidr_ip: The CIDR block you are providing access to.
+        :type cidr: ``str`` or list of ``str``
+        :param cidr: The CIDR block you are providing access to.
 
-        :type src_group: :class:`.SecurityGroup`
-        :param src_group: The Security Group object you are granting access to.
+        :type src_dest_fw: :class:`.VMFirewall`
+        :param src_dest_fw: The VM firewall object which is the
+                            source/destination of the traffic, depending on
+                            whether it's ingress/egress traffic.
 
-        :rtype: :class:`.SecurityGroupRule`
+        :rtype: :class:`.VMFirewallRule`
         :return: Rule object if successful or ``None``.
         """
         pass
 
-    def get_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
+    @abstractmethod
+    def find(self, **kwargs):
         """
-        Get a security group rule with the specified parameters.
+        Find a firewall rule associated with your account filtered by the given
+        parameters.
 
-        You need to pass in either ``src_group`` OR ``ip_protocol`` AND
-        ``from_port``, ``to_port``, and ``cidr_ip``. Note that when retrieving
-        a group rule, this method will return only one rule although possibly
-        several rules exist for the group rule. In that case, use the
-        ``.rules`` property and filter the results as desired.
+        :type name: str
+        :param name: The name of the VM firewall to retrieve.
 
-        :type ip_protocol: ``str``
-        :param ip_protocol: Either ``tcp`` | ``udp`` | ``icmp``.
+        :type protocol: ``str``
+        :param protocol: Either ``tcp`` | ``udp`` | ``icmp``.
 
         :type from_port: ``int``
         :param from_port: The beginning port number you are enabling.
@@ -1768,27 +1810,61 @@ class SecurityGroup(CloudResource):
         :type to_port: ``int``
         :param to_port: The ending port number you are enabling.
 
-        :type cidr_ip: ``str`` or list of ``str``
-        :param cidr_ip: The CIDR block you are providing access to.
+        :type cidr: ``str`` or list of ``str``
+        :param cidr: The CIDR block you are providing access to.
 
-        :type src_group: :class:`.SecurityGroup`
-        :param src_group: The Security Group object you are granting access to.
+        :type src_dest_fw: :class:`.VMFirewall`
+        :param src_dest_fw: The VM firewall object which is the
+                            source/destination of the traffic, depending on
+                            whether it's ingress/egress traffic.
 
-        :rtype: :class:`.SecurityGroupRule`
-        :return: Role object if one can be found or ``None``.
+        :type src_dest_fw_id: :class:`.str`
+        :param src_dest_fw_id: The VM firewall id which is the
+                               source/destination of the traffic, depending on
+                               whether it's ingress/egress traffic.
+
+        :rtype: list of :class:`VMFirewallRule`
+        :return: A list of VMFirewall objects or an empty list if none
+                 found.
         """
         pass
 
+    @abstractmethod
+    def delete(self, rule_id):
+        """
+        Delete an existing VMFirewall rule.
 
-class SecurityGroupRule(CloudResource):
+        :type rule_id: str
+        :param rule_id: The VM firewall rule to be deleted.
+        """
+        pass
+
+
+class TrafficDirection(Enum):
+    INBOUND = 'inbound'
+    OUTBOUND = 'outbound'
+
+
+class VMFirewallRule(CloudResource):
 
     """
-    Represents a security group rule.
+    Represents a VM firewall rule.
     """
     __metaclass__ = ABCMeta
 
     @abstractproperty
-    def ip_protocol(self):
+    def direction(self):
+        """
+        Direction of traffic to which this rule applies.
+        Either TrafficDirection.INBOUND | TrafficDirection.OUTBOUND
+
+        :rtype: ``str``
+        :return: Direction of traffic to which this rule applies
+        """
+        pass
+
+    @abstractproperty
+    def protocol(self):
         """
         IP protocol used. Either ``tcp`` | ``udp`` | ``icmp``.
 
@@ -1818,9 +1894,9 @@ class SecurityGroupRule(CloudResource):
         pass
 
     @abstractproperty
-    def cidr_ip(self):
+    def cidr(self):
         """
-        CIDR block this security group is providing access to.
+        CIDR block this VM firewall is providing access to.
 
         :rtype: ``str``
         :return: CIDR block.
@@ -1828,12 +1904,22 @@ class SecurityGroupRule(CloudResource):
         pass
 
     @abstractproperty
-    def group(self):
+    def src_dest_fw_id(self):
+        """
+        VM firewall id given access permissions by this rule.
+
+        :rtype: ``str``
+        :return: The VM firewall granted access.
+        """
+        pass
+
+    @abstractproperty
+    def src_dest_fw(self):
         """
-        Security group given access permissions by this rule.
+        VM firewall given access permissions by this rule.
 
-        :rtype: :class:``.SecurityGroup``
-        :return: The Security Group with granting access.
+        :rtype: :class:``.VMFirewall``
+        :return: The VM firewall granted access.
         """
         pass
 

+ 45 - 51
cloudbridge/cloud/interfaces/services.py

@@ -206,7 +206,7 @@ class InstanceService(PageableObjectMixin, CloudService):
 
     @abstractmethod
     def create(self, name, image, vm_type, subnet, zone=None,
-               key_pair=None, security_groups=None, user_data=None,
+               key_pair=None, vm_firewalls=None, user_data=None,
                launch_config=None,
                **kwargs):
         """
@@ -246,16 +246,16 @@ class InstanceService(PageableObjectMixin, CloudService):
         :param key_pair: The KeyPair object or its name, to set for the
                          instance.
 
-        :type  security_groups: A ``list`` of ``SecurityGroup`` objects or a
-                                list of ``str`` object IDs
-        :param security_groups: A list of ``SecurityGroup`` objects or a list
-                                of ``SecurityGroup`` IDs, which should be
-                                assigned to this instance.
+        :type  vm_firewalls: A ``list`` of ``VMFirewall`` objects or a
+                             list of ``str`` object IDs
+        :param vm_firewalls: A list of ``VMFirewall`` objects or a list
+                             of ``VMFirewall`` IDs, which should be
+                             assigned to this instance.
 
-                                The security groups must be associated with the
-                                same network as the supplied subnet. Use
-                                ``network.security_groups`` to retrieve a list
-                                of security groups belonging to a network.
+                             The VM firewalls must be associated with the
+                             same network as the supplied subnet. Use
+                             ``network.vm_firewalls`` to retrieve a list
+                             of firewalls belonging to a network.
 
         :type  user_data: ``str``
         :param user_data: An extra userdata object which is compatible with
@@ -1008,24 +1008,24 @@ class SecurityService(CloudService):
         pass
 
     @abstractproperty
-    def security_groups(self):
+    def vm_firewalls(self):
         """
-        Provides access to security groups for this provider.
+        Provides access to firewalls (security groups) for this provider.
 
         Example:
 
         .. code-block:: python
 
-            # print all security groups
-            for sg in provider.security.security_groups:
-                print(sg.id, sg.name)
+            # print all VM firewalls
+            for fw in provider.security.vm_firewalls:
+                print(fw.id, fw.name)
 
-            # find security group by name
-            sg = provider.security.security_groups.find(name='my_sg')[0]
-            print(sg.id, sg.name)
+            # find firewall by name
+            fw = provider.security.vm_firewalls.find(name='my_vm_fw')[0]
+            print(fw.id, fw.name)
 
-        :rtype: :class:`.SecurityGroupService`
-        :return: a SecurityGroupService object
+        :rtype: :class:`.VMFirewallService`
+        :return: a VMFirewallService object
         """
         pass
 
@@ -1093,7 +1093,7 @@ class KeyPairService(PageableObjectMixin, CloudService):
     @abstractmethod
     def delete(self, key_pair_id):
         """
-        Delete an existing SecurityGroup.
+        Delete an existing VMFirewall.
 
         :type key_pair_id: str
         :param key_pair_id: The id of the key pair to be deleted.
@@ -1106,70 +1106,70 @@ class KeyPairService(PageableObjectMixin, CloudService):
         pass
 
 
-class SecurityGroupService(PageableObjectMixin, CloudService):
+class VMFirewallService(PageableObjectMixin, CloudService):
 
     """
-    Base interface for security groups.
+    Base interface for VM firewalls.
     """
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def get(self, security_group_id):
+    def get(self, vm_firewall_id):
         """
-        Returns a SecurityGroup given its ID. Returns ``None`` if the
-        SecurityGroup does not exist.
+        Returns a VMFirewall given its ID. Returns ``None`` if the
+        VMFirewall does not exist.
 
         Example:
 
         .. code-block:: python
 
-            sg = provider.security.security_groups.get('my_sg_id')
-            print(sg.id, sg.name)
+            fw = provider.security.vm_firewalls.get('my_fw_id')
+            print(fw.id, fw.name)
 
-        :rtype: :class:`.SecurityGroup`
-        :return:  a SecurityGroup instance
+        :rtype: :class:`.VMFirewall`
+        :return:  a VMFirewall instance
         """
         pass
 
     @abstractmethod
     def list(self, limit=None, marker=None):
         """
-        List all security groups associated with this account.
+        List all VM firewalls associated with this account.
 
-        :rtype: ``list`` of :class:`.SecurityGroup`
-        :return:  list of SecurityGroup objects
+        :rtype: ``list`` of :class:`.VMFirewall`
+        :return:  list of VMFirewall objects
         """
         pass
 
     @abstractmethod
     def create(self, name, description, network_id):
         """
-        Create a new SecurityGroup.
+        Create a new VMFirewall.
 
         :type name: str
-        :param name: The name of the new security group.
+        :param name: The name of the new VM firewall.
 
         :type description: str
-        :param description: The description of the new security group.
+        :param description: The description of the new VM firewall.
 
         :type  network_id: ``str``
-        :param network_id: Network ID under which to create the security group.
+        :param network_id: Network ID under which to create the VM firewall.
 
-        :rtype: ``object`` of :class:`.SecurityGroup`
-        :return:  A SecurityGroup instance or ``None`` if one was not created.
+        :rtype: ``object`` of :class:`.VMFirewall`
+        :return:  A VMFirewall instance or ``None`` if one was not created.
         """
         pass
 
     @abstractmethod
     def find(self, name, limit=None, marker=None):
         """
-        Get security groups associated with your account filtered by name.
+        Get VM firewalls associated with your account filtered by name.
 
         :type name: str
-        :param name: The name of the security group to retrieve.
+        :param name: The name of the VM firewall to retrieve.
 
-        :rtype: list of :class:`SecurityGroup`
-        :return: A list of SecurityGroup objects or an empty list if none
+        :rtype: list of :class:`VMFirewall`
+        :return: A list of VMFirewall objects or an empty list if none
                  found.
         """
         pass
@@ -1177,16 +1177,10 @@ class SecurityGroupService(PageableObjectMixin, CloudService):
     @abstractmethod
     def delete(self, group_id):
         """
-        Delete an existing SecurityGroup.
+        Delete an existing VMFirewall.
 
         :type group_id: str
-        :param group_id: The security group ID to be deleted.
-
-        :rtype: ``bool``
-        :return:  ``True`` if the security group does not exist, ``False``
-                  otherwise. Note that this implies that the group may not have
-                  been deleted by this method but instead has not existed in
-                  the first place.
+        :param group_id: The VM firewall ID to be deleted.
         """
         pass
 

+ 120 - 106
cloudbridge/cloud/providers/aws/resources.py

@@ -19,21 +19,23 @@ from cloudbridge.cloud.base.resources import BaseNetwork
 from cloudbridge.cloud.base.resources import BasePlacementZone
 from cloudbridge.cloud.base.resources import BaseRegion
 from cloudbridge.cloud.base.resources import BaseRouter
-from cloudbridge.cloud.base.resources import BaseSecurityGroup
-from cloudbridge.cloud.base.resources import BaseSecurityGroupRule
 from cloudbridge.cloud.base.resources import BaseSnapshot
 from cloudbridge.cloud.base.resources import BaseSubnet
+from cloudbridge.cloud.base.resources import BaseVMFirewall
+from cloudbridge.cloud.base.resources import BaseVMFirewallRule
+from cloudbridge.cloud.base.resources import BaseVMFirewallRuleContainer
 from cloudbridge.cloud.base.resources import BaseVMType
 from cloudbridge.cloud.base.resources import BaseVolume
 from cloudbridge.cloud.base.resources import ClientPagedResultList
+from cloudbridge.cloud.interfaces.exceptions import InvalidValueException
 from cloudbridge.cloud.interfaces.resources import GatewayState
 from cloudbridge.cloud.interfaces.resources import InstanceState
 from cloudbridge.cloud.interfaces.resources import MachineImageState
 from cloudbridge.cloud.interfaces.resources import NetworkState
 from cloudbridge.cloud.interfaces.resources import RouterState
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
 from cloudbridge.cloud.interfaces.resources import SnapshotState
 from cloudbridge.cloud.interfaces.resources import SubnetState
+from cloudbridge.cloud.interfaces.resources import TrafficDirection
 from cloudbridge.cloud.interfaces.resources import VolumeState
 
 from .helpers import find_tag_value
@@ -255,14 +257,14 @@ class AWSInstance(BaseInstance):
         return self._ec2_instance.placement.get('AvailabilityZone')
 
     @property
-    def security_groups(self):
+    def vm_firewalls(self):
         return [
-            self._provider.security.security_groups.get(group_id)
-            for group_id in self.security_group_ids
+            self._provider.security.vm_firewalls.get(fw_id)
+            for fw_id in self.vm_firewall_ids
         ]
 
     @property
-    def security_group_ids(self):
+    def vm_firewall_ids(self):
         return list(set([
             group.get('GroupId') for group in
             self._ec2_instance.security_groups
@@ -310,14 +312,14 @@ class AWSInstance(BaseInstance):
         self._provider.ec2_conn.meta.client.disassociate_address(**params)
         self.refresh()
 
-    def add_security_group(self, sg):
+    def add_vm_firewall(self, firewall):
         self._ec2_instance.modify_attribute(
-            Groups=self.security_group_ids + [sg.id])
+            Groups=self.vm_firewall_ids + [firewall.id])
 
-    def remove_security_group(self, sg):
+    def remove_vm_firewall(self, firewall):
         self._ec2_instance.modify_attribute(
-            Groups=([sg_id for sg_id in self.security_group_ids
-                     if sg_id != sg.id]))
+            Groups=([fw_id for fw_id in self.vm_firewall_ids
+                     if fw_id != firewall.id]))
 
     @property
     def state(self):
@@ -544,74 +546,26 @@ class AWSKeyPair(BaseKeyPair):
             return None
 
 
-class AWSSecurityGroup(BaseSecurityGroup):
+class AWSVMFirewall(BaseVMFirewall):
 
-    def __init__(self, provider, security_group):
-        super(AWSSecurityGroup, self).__init__(provider, security_group)
+    def __init__(self, provider, _vm_firewall):
+        super(AWSVMFirewall, self).__init__(provider, _vm_firewall)
+        self._rule_container = AWSVMFirewallRuleContainer(provider, self)
 
     @property
     def name(self):
-        return self._security_group.group_name
+        return self._vm_firewall.group_name
 
     @property
     def network_id(self):
-        return self._security_group.vpc_id
+        return self._vm_firewall.vpc_id
 
     @property
     def rules(self):
-        return [AWSSecurityGroupRule(self._provider, r, self)
-                for r in self._security_group.ip_permissions]
+        return self._rule_container
 
-    def add_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
-        try:
-            src_group_id = (
-                src_group.id if isinstance(src_group, SecurityGroup)
-                else src_group)
-
-            ip_perm_entry = {
-                'IpProtocol': ip_protocol,
-                'FromPort': from_port,
-                'ToPort': to_port,
-                'IpRanges': [{'CidrIp': cidr_ip}] if cidr_ip else None,
-                'UserIdGroupPairs': [{
-                    'GroupId': src_group_id}
-                ] if src_group_id else None
-            }
-            # Filter out empty values to please Boto
-            ip_perms = [trim_empty_params(ip_perm_entry)]
-            self._security_group.authorize_ingress(IpPermissions=ip_perms)
-            self._security_group.reload()
-            return self.get_rule(ip_protocol, from_port, to_port, cidr_ip,
-                                 src_group_id)
-        except ClientError as ec2e:
-            if ec2e.response['Error']['Code'] == "InvalidPermission.Duplicate":
-                return self.get_rule(ip_protocol, from_port, to_port, cidr_ip,
-                                     src_group)
-            else:
-                raise ec2e
-
-    def get_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
-        src_group_id = (src_group.id if isinstance(src_group, SecurityGroup)
-                        else src_group)
-        for rule in self._security_group.ip_permissions:
-            if ip_protocol and rule['IpProtocol'] != ip_protocol:
-                continue
-            elif from_port and rule['FromPort'] != from_port:
-                continue
-            elif to_port and rule['ToPort'] != to_port:
-                continue
-            elif cidr_ip:
-                if cidr_ip not in [x['CidrIp'] for x in rule['IpRanges']]:
-                    continue
-            elif src_group_id:
-                if src_group_id not in [
-                    group_pair.get('GroupId') for group_pair in
-                        rule.get('UserIdGroupPairs', [])]:
-                    continue
-            return AWSSecurityGroupRule(self._provider, rule, self)
-        return None
+    def refresh(self):
+        self._vm_firewall.reload()
 
     def to_json(self):
         attr = inspect.getmembers(self, lambda a: not inspect.isroutine(a))
@@ -623,77 +577,137 @@ class AWSSecurityGroup(BaseSecurityGroup):
         return js
 
 
-class AWSSecurityGroupRule(BaseSecurityGroupRule):
+class AWSVMFirewallRuleContainer(BaseVMFirewallRuleContainer):
+
+    def __init__(self, provider, firewall):
+        super(AWSVMFirewallRuleContainer, self).__init__(provider, firewall)
+
+    def list(self, limit=None, marker=None):
+        # pylint:disable=protected-access
+        rules = [AWSVMFirewallRule(self.firewall,
+                                   TrafficDirection.INBOUND, r)
+                 for r in self.firewall._vm_firewall.ip_permissions]
+        rules = rules + [
+            AWSVMFirewallRule(
+                self.firewall, TrafficDirection.OUTBOUND, r)
+            for r in self.firewall._vm_firewall.ip_permissions_egress]
+        return ClientPagedResultList(self._provider, rules,
+                                     limit=limit, marker=marker)
+
+    def create(self,  direction, protocol=None, from_port=None,
+               to_port=None, cidr=None, src_dest_fw=None):
+        src_dest_fw_id = (
+            src_dest_fw.id if isinstance(src_dest_fw, AWSVMFirewall)
+            else src_dest_fw)
+
+        ip_perm_entry = AWSVMFirewallRule._construct_ip_perms(
+            protocol, from_port, to_port, cidr, src_dest_fw_id)
+        # Filter out empty values to please Boto
+        ip_perms = [trim_empty_params(ip_perm_entry)]
+
+        try:
+            if direction == TrafficDirection.INBOUND:
+                # pylint:disable=protected-access
+                self.firewall._vm_firewall.authorize_ingress(
+                    IpPermissions=ip_perms)
+            elif direction == TrafficDirection.OUTBOUND:
+                # pylint:disable=protected-access
+                self.firewall._vm_firewall.authorize_egress(
+                    IpPermissions=ip_perms)
+            else:
+                raise InvalidValueException("direction", direction)
+            self.firewall.refresh()
+            return AWSVMFirewallRule(self.firewall, direction, ip_perm_entry)
+        except ClientError as ec2e:
+            if ec2e.response['Error']['Code'] == "InvalidPermission.Duplicate":
+                return AWSVMFirewallRule(
+                    self.firewall, direction, ip_perm_entry)
+            else:
+                raise ec2e
+
+
+class AWSVMFirewallRule(BaseVMFirewallRule):
 
-    def __init__(self, provider, rule, parent):
-        super(AWSSecurityGroupRule, self).__init__(provider, rule, parent)
+    def __init__(self, parent_fw, direction, rule):
+        self._direction = direction
+        super(AWSVMFirewallRule, self).__init__(parent_fw, rule)
+
+        # cache id
+        md5 = hashlib.md5()
+        md5.update(self._name.encode('ascii'))
+        self._id = md5.hexdigest()
 
     @property
     def id(self):
-        md5 = hashlib.md5()
-        md5.update("{0}-{1}-{2}-{3}".format(
-            self.ip_protocol, self.from_port, self.to_port, self.cidr_ip)
-            .encode('ascii'))
-        return md5.hexdigest()
+        return self._id
+
+    @property
+    def direction(self):
+        return self._direction
 
     @property
-    def ip_protocol(self):
+    def protocol(self):
         return self._rule.get('IpProtocol')
 
     @property
     def from_port(self):
-        return self._rule.get('FromPort', 0)
+        return self._rule.get('FromPort')
 
     @property
     def to_port(self):
-        return self._rule.get('ToPort', 0)
+        return self._rule.get('ToPort')
 
     @property
-    def cidr_ip(self):
-        if len(self._rule.get('IpRanges', [])) > 0:
+    def cidr(self):
+        if len(self._rule.get('IpRanges') or []) > 0:
             return self._rule['IpRanges'][0].get('CidrIp')
         return None
 
     @property
-    def group_id(self):
-        if len(self._rule['UserIdGroupPairs']) > 0:
+    def src_dest_fw_id(self):
+        if len(self._rule.get('UserIdGroupPairs') or []) > 0:
             return self._rule['UserIdGroupPairs'][0]['GroupId']
         else:
             return None
 
     @property
-    def group(self):
-        if self.group_id:
-            return AWSSecurityGroup(
+    def src_dest_fw(self):
+        if self.src_dest_fw_id:
+            return AWSVMFirewall(
                 self._provider,
-                self._provider.ec2_conn.SecurityGroup(self.group_id))
+                self._provider.ec2_conn.SecurityGroup(self.src_dest_fw_id))
         else:
             return None
 
-    def to_json(self):
-        attr = inspect.getmembers(self, lambda a: not (inspect.isroutine(a)))
-        js = {k: v for (k, v) in attr if not k.startswith('_')}
-        js['group'] = self.group.id if self.group else ''
-        js['parent'] = self.parent.id if self.parent else ''
-        return js
-
-    def delete(self):
-
-        ip_perm_entry = {
-            'IpProtocol': self.ip_protocol,
-            'FromPort': self.from_port,
-            'ToPort': self.to_port,
-            'IpRanges': [{'CidrIp': self.cidr_ip}] if self.cidr_ip else None,
+    @staticmethod
+    def _construct_ip_perms(protocol, from_port, to_port, cidr,
+                            src_dest_fw_id):
+        return {
+            'IpProtocol': protocol,
+            'FromPort': from_port,
+            'ToPort': to_port,
+            'IpRanges': [{'CidrIp': cidr}] if cidr else None,
             'UserIdGroupPairs': [{
-                'GroupId': self.group_id}
-            ] if self.group_id else None
+                'GroupId': src_dest_fw_id}
+            ] if src_dest_fw_id else None
         }
 
+    def delete(self):
+        ip_perm_entry = self._construct_ip_perms(
+            self.protocol, self.from_port, self.to_port,
+            self.cidr, self.src_dest_fw_id)
+
         # Filter out empty values to please Boto
         ip_perms = [trim_empty_params(ip_perm_entry)]
 
-        self.parent._security_group.revoke_ingress(IpPermissions=ip_perms)
-        self.parent._security_group.reload()
+        # pylint:disable=protected-access
+        if self.direction == TrafficDirection.INBOUND:
+            self.firewall._vm_firewall.revoke_ingress(
+                IpPermissions=ip_perms)
+        else:
+            self.firewall._vm_firewall.revoke_egress(
+                IpPermissions=ip_perms)
+        self.firewall.refresh()
 
 
 class AWSBucketObject(BaseBucketObject):

+ 29 - 29
cloudbridge/cloud/providers/aws/services.py

@@ -15,10 +15,10 @@ from cloudbridge.cloud.base.services import BaseNetworkingService
 from cloudbridge.cloud.base.services import BaseObjectStoreService
 from cloudbridge.cloud.base.services import BaseRegionService
 from cloudbridge.cloud.base.services import BaseRouterService
-from cloudbridge.cloud.base.services import BaseSecurityGroupService
 from cloudbridge.cloud.base.services import BaseSecurityService
 from cloudbridge.cloud.base.services import BaseSnapshotService
 from cloudbridge.cloud.base.services import BaseSubnetService
+from cloudbridge.cloud.base.services import BaseVMFirewallService
 from cloudbridge.cloud.base.services import BaseVMTypeService
 from cloudbridge.cloud.base.services import BaseVolumeService
 from cloudbridge.cloud.interfaces.exceptions \
@@ -26,8 +26,8 @@ from cloudbridge.cloud.interfaces.exceptions \
 from cloudbridge.cloud.interfaces.resources import KeyPair
 from cloudbridge.cloud.interfaces.resources import MachineImage
 from cloudbridge.cloud.interfaces.resources import PlacementZone
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
 from cloudbridge.cloud.interfaces.resources import Snapshot
+from cloudbridge.cloud.interfaces.resources import VMFirewall
 from cloudbridge.cloud.interfaces.resources import VMType
 from cloudbridge.cloud.interfaces.resources import Volume
 
@@ -46,9 +46,9 @@ from .resources import AWSMachineImage
 from .resources import AWSNetwork
 from .resources import AWSRegion
 from .resources import AWSRouter
-from .resources import AWSSecurityGroup
 from .resources import AWSSnapshot
 from .resources import AWSSubnet
+from .resources import AWSVMFirewall
 from .resources import AWSVMType
 from .resources import AWSVolume
 
@@ -60,15 +60,15 @@ class AWSSecurityService(BaseSecurityService):
 
         # Initialize provider services
         self._key_pairs = AWSKeyPairService(provider)
-        self._security_groups = AWSSecurityGroupService(provider)
+        self._vm_firewalls = AWSVMFirewallService(provider)
 
     @property
     def key_pairs(self):
         return self._key_pairs
 
     @property
-    def security_groups(self):
-        return self._security_groups
+    def vm_firewalls(self):
+        return self._vm_firewalls
 
 
 class AWSKeyPairService(BaseKeyPairService):
@@ -94,22 +94,22 @@ class AWSKeyPairService(BaseKeyPairService):
         return self.svc.create('create_key_pair', KeyName=name)
 
 
-class AWSSecurityGroupService(BaseSecurityGroupService):
+class AWSVMFirewallService(BaseVMFirewallService):
 
     def __init__(self, provider):
-        super(AWSSecurityGroupService, self).__init__(provider)
+        super(AWSVMFirewallService, self).__init__(provider)
         self.svc = BotoEC2Service(provider=self.provider,
-                                  cb_resource=AWSSecurityGroup,
+                                  cb_resource=AWSVMFirewall,
                                   boto_collection_name='security_groups')
 
-    def get(self, sg_id):
-        return self.svc.get(sg_id)
+    def get(self, firewall_id):
+        return self.svc.get(firewall_id)
 
     def list(self, limit=None, marker=None):
         return self.svc.list(limit=limit, marker=marker)
 
     def create(self, name, description, network_id):
-        AWSSecurityGroup.assert_valid_resource_name(name)
+        AWSVMFirewall.assert_valid_resource_name(name)
         return self.svc.create('create_security_group', GroupName=name,
                                Description=description, VpcId=network_id)
 
@@ -117,10 +117,10 @@ class AWSSecurityGroupService(BaseSecurityGroupService):
         return self.svc.find(filter_name='group-name', filter_value=name,
                              limit=limit, marker=marker)
 
-    def delete(self, group_id):
-        sg = self.svc.get(group_id)
-        if sg:
-            sg.delete()
+    def delete(self, firewall_id):
+        firewall = self.svc.get(firewall_id)
+        if firewall:
+            firewall.delete()
 
 
 class AWSBlockStoreService(BaseBlockStoreService):
@@ -326,7 +326,7 @@ class AWSInstanceService(BaseInstanceService):
                                   boto_collection_name='instances')
 
     def create(self, name, image, vm_type, subnet, zone=None,
-               key_pair=None, security_groups=None, user_data=None,
+               key_pair=None, vm_firewalls=None, user_data=None,
                launch_config=None, **kwargs):
         AWSInstance.assert_valid_resource_name(name)
 
@@ -344,8 +344,8 @@ class AWSInstanceService(BaseInstanceService):
         else:
             bdm = None
 
-        subnet_id, zone_id, security_group_ids = \
-            self._resolve_launch_options(subnet, zone_id, security_groups)
+        subnet_id, zone_id, vm_firewall_ids = \
+            self._resolve_launch_options(subnet, zone_id, vm_firewalls)
 
         placement = {'AvailabilityZone': zone_id} if zone_id else None
         inst = self.svc.create('create_instances',
@@ -353,7 +353,7 @@ class AWSInstanceService(BaseInstanceService):
                                MinCount=1,
                                MaxCount=1,
                                KeyName=key_pair_name,
-                               SecurityGroupIds=security_group_ids or None,
+                               SecurityGroupIds=vm_firewall_ids or None,
                                UserData=user_data,
                                InstanceType=vm_size,
                                Placement=placement,
@@ -370,7 +370,7 @@ class AWSInstanceService(BaseInstanceService):
             'Expected a single object response, got a list: %s' % inst)
 
     def _resolve_launch_options(self, subnet=None, zone_id=None,
-                                security_groups=None):
+                                vm_firewalls=None):
         """
         Work out interdependent launch options.
 
@@ -383,23 +383,23 @@ class AWSInstanceService(BaseInstanceService):
         :type zone_id: ``str``
         :param zone_id: ID of the zone where the launch should happen.
 
-        :type security_groups: ``list`` of ``id``
-        :param zone_id: List of security group IDs.
+        :type vm_firewalls: ``list`` of ``id``
+        :param vm_firewalls: List of firewall IDs.
 
         :rtype: triplet of ``str``
-        :return: Subnet ID, zone ID and security group IDs for launch.
+        :return: Subnet ID, zone ID and VM firewall IDs for launch.
 
         :raise ValueError: In case a conflicting combination is found.
         """
         if subnet:
             # subnet's zone takes precedence
             zone_id = subnet.zone.id
-        if isinstance(security_groups, list) and isinstance(
-                security_groups[0], SecurityGroup):
-            security_group_ids = [sg.id for sg in security_groups]
+        if isinstance(vm_firewalls, list) and isinstance(
+                vm_firewalls[0], VMFirewall):
+            vm_firewall_ids = [fw.id for fw in vm_firewalls]
         else:
-            security_group_ids = security_groups
-        return subnet.id, zone_id, security_group_ids
+            vm_firewall_ids = vm_firewalls
+        return subnet.id, zone_id, vm_firewall_ids
 
     def _process_block_device_mappings(self, launch_config, zone=None):
         """

+ 25 - 0
cloudbridge/cloud/providers/openstack/provider.py

@@ -17,6 +17,9 @@ from neutronclient.v2_0 import client as neutron_client
 from novaclient import client as nova_client
 from novaclient import shell as nova_shell
 
+from openstack import connection
+from openstack import profile
+
 from swiftclient import client as swift_client
 
 from .services import OpenStackBlockStoreService
@@ -59,6 +62,7 @@ class OpenStackCloudProvider(BaseCloudProvider):
         self._cinder = None
         self._swift = None
         self._neutron = None
+        self._os_conn = None
 
         # Additional cached variables
         self._cached_keystone_session = None
@@ -123,6 +127,21 @@ class OpenStackCloudProvider(BaseCloudProvider):
             self._cached_keystone_session = session.Session(auth=auth)
         return self._cached_keystone_session
 
+    def _connect_openstack(self):
+        prof = profile.Profile()
+        prof.set_region(profile.Profile.ALL, self.region_name)
+
+        return connection.Connection(
+            profile=prof,
+            user_agent='cloudbridge',
+            auth_url=self.auth_url,
+            project_name=self.project_name,
+            username=self.username,
+            password=self.password,
+            user_domain_name=self.user_domain_name,
+            project_domain_name=self.project_domain_name
+        )
+
 #     @property
 #     def glance(self):
 #         if not self._glance:
@@ -147,6 +166,12 @@ class OpenStackCloudProvider(BaseCloudProvider):
             self._neutron = self._connect_neutron()
         return self._neutron
 
+    @property
+    def os_conn(self):
+        if not self._os_conn:
+            self._os_conn = self._connect_openstack()
+        return self._os_conn
+
     @property
     def compute(self):
         return self._compute

+ 112 - 131
cloudbridge/cloud/providers/openstack/resources.py

@@ -18,21 +18,23 @@ from cloudbridge.cloud.base.resources import BaseNetwork
 from cloudbridge.cloud.base.resources import BasePlacementZone
 from cloudbridge.cloud.base.resources import BaseRegion
 from cloudbridge.cloud.base.resources import BaseRouter
-from cloudbridge.cloud.base.resources import BaseSecurityGroup
-from cloudbridge.cloud.base.resources import BaseSecurityGroupRule
 from cloudbridge.cloud.base.resources import BaseSnapshot
 from cloudbridge.cloud.base.resources import BaseSubnet
+from cloudbridge.cloud.base.resources import BaseVMFirewall
+from cloudbridge.cloud.base.resources import BaseVMFirewallRule
+from cloudbridge.cloud.base.resources import BaseVMFirewallRuleContainer
 from cloudbridge.cloud.base.resources import BaseVMType
 from cloudbridge.cloud.base.resources import BaseVolume
 from cloudbridge.cloud.base.resources import ClientPagedResultList
+from cloudbridge.cloud.interfaces.exceptions import InvalidValueException
 from cloudbridge.cloud.interfaces.resources import GatewayState
 from cloudbridge.cloud.interfaces.resources import InstanceState
 from cloudbridge.cloud.interfaces.resources import MachineImageState
 from cloudbridge.cloud.interfaces.resources import NetworkState
 from cloudbridge.cloud.interfaces.resources import RouterState
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
 from cloudbridge.cloud.interfaces.resources import SnapshotState
 from cloudbridge.cloud.interfaces.resources import SubnetState
+from cloudbridge.cloud.interfaces.resources import TrafficDirection
 from cloudbridge.cloud.interfaces.resources import VolumeState
 from cloudbridge.cloud.providers.openstack import helpers as oshelpers
 
@@ -40,6 +42,8 @@ from keystoneclient.v3.regions import Region
 
 import novaclient.exceptions as novaex
 
+from openstack.exceptions import HttpException
+
 import swiftclient
 
 from swiftclient.service import SwiftService, SwiftUploadObject
@@ -350,19 +354,18 @@ class OpenStackInstance(BaseInstance):
         return getattr(self._os_instance, 'OS-EXT-AZ:availability_zone', None)
 
     @property
-    def security_groups(self):
-        """
-        Get the security groups associated with this instance.
-        """
-        return [OpenStackSecurityGroup(self._provider, group)
-                for group in self._os_instance.list_security_group()]
+    def vm_firewalls(self):
+        return [
+            self._provider.security.vm_firewalls.get(group.id)
+            for group in self._os_instance.list_security_group()
+        ]
 
     @property
-    def security_group_ids(self):
+    def vm_firewall_ids(self):
         """
-        Get the security groups IDs associated with this instance.
+        Get the VM firewall IDs associated with this instance.
         """
-        return [group.id for group in self.security_groups]
+        return [fw.id for fw in self.vm_firewalls]
 
     @property
     def key_pair_name(self):
@@ -393,17 +396,17 @@ class OpenStackInstance(BaseInstance):
         """
         self._os_instance.remove_floating_ip(ip_address)
 
-    def add_security_group(self, sg):
+    def add_vm_firewall(self, firewall):
         """
-        Add a security group to this instance
+        Add a VM firewall to this instance
         """
-        self._os_instance.add_security_group(sg.id)
+        self._os_instance.add_security_group(firewall.id)
 
-    def remove_security_group(self, sg):
+    def remove_vm_firewall(self, firewall):
         """
-        Remove a security group from this instance
+        Remove a VM firewall from this instance
         """
-        self._os_instance.remove_security_group(sg.id)
+        self._os_instance.remove_security_group(firewall.id)
 
     @property
     def state(self):
@@ -999,10 +1002,11 @@ class OpenStackKeyPair(BaseKeyPair):
         return getattr(self._key_pair, 'private_key', None)
 
 
-class OpenStackSecurityGroup(BaseSecurityGroup):
+class OpenStackVMFirewall(BaseVMFirewall):
 
-    def __init__(self, provider, security_group):
-        super(OpenStackSecurityGroup, self).__init__(provider, security_group)
+    def __init__(self, provider, vm_firewall):
+        super(OpenStackVMFirewall, self).__init__(provider, vm_firewall)
+        self._rule_svc = OpenStackVMFirewallRuleContainer(provider, self)
 
     @property
     def network_id(self):
@@ -1015,95 +1019,14 @@ class OpenStackSecurityGroup(BaseSecurityGroup):
 
     @property
     def rules(self):
-        # Update SG object; otherwise, recently added rules do now show
-        self._security_group = self._provider.nova.security_groups.get(
-            self._security_group)
-        return [OpenStackSecurityGroupRule(self._provider, r, self)
-                for r in self._security_group.rules]
-
-    def add_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
-        """
-        Create a security group rule.
-
-        You need to pass in either ``src_group`` OR ``ip_protocol`` AND
-        ``from_port``, ``to_port``, ``cidr_ip``.  In other words, either
-        you are authorizing another group or you are authorizing some
-        ip-based rule.
-
-        :type ip_protocol: str
-        :param ip_protocol: Either ``tcp`` | ``udp`` | ``icmp``
-
-        :type from_port: int
-        :param from_port: The beginning port number you are enabling
-
-        :type to_port: int
-        :param to_port: The ending port number you are enabling
-
-        :type cidr_ip: str or list of strings
-        :param cidr_ip: The CIDR block you are providing access to.
-
-        :type src_group: ``object`` of :class:`.SecurityGroup`
-        :param src_group: The Security Group you are granting access to.
-
-        :rtype: :class:``.SecurityGroupRule``
-        :return: Rule object if successful or ``None``.
-        """
-        if src_group:
-            if not isinstance(src_group, SecurityGroup):
-                src_group = self._provider.security.security_groups.get(
-                    src_group)
-            existing_rule = self.get_rule(ip_protocol=ip_protocol,
-                                          from_port=from_port,
-                                          to_port=to_port,
-                                          src_group=src_group)
-            if existing_rule:
-                return existing_rule
-
-            rule = self._provider.nova.security_group_rules.create(
-                parent_group_id=self._security_group.id,
-                ip_protocol=ip_protocol,
-                from_port=from_port,
-                to_port=to_port,
-                group_id=src_group.id)
-            if rule:
-                # We can only return one Rule so default to TCP (ie, last in
-                # the for loop above).
-                return OpenStackSecurityGroupRule(self._provider,
-                                                  rule.to_dict(), self)
-        else:
-            existing_rule = self.get_rule(ip_protocol=ip_protocol,
-                                          from_port=from_port,
-                                          to_port=to_port,
-                                          cidr_ip=cidr_ip)
-            if existing_rule:
-                return existing_rule
-
-            rule = self._provider.nova.security_group_rules.create(
-                parent_group_id=self._security_group.id,
-                ip_protocol=ip_protocol,
-                from_port=from_port,
-                to_port=to_port,
-                cidr=cidr_ip)
-            if rule:
-                return OpenStackSecurityGroupRule(self._provider,
-                                                  rule.to_dict(), self)
-        return None
+        return self._rule_svc
 
-    def get_rule(self, ip_protocol=None, from_port=None, to_port=None,
-                 cidr_ip=None, src_group=None):
-        # Update SG object; otherwise, recently added rules do not show
-        self._security_group = self._provider.nova.security_groups.get(
-            self._security_group)
-        for rule in self._security_group.rules:
-            if (rule['ip_protocol'] == ip_protocol and
-                rule['from_port'] == from_port and
-                rule['to_port'] == to_port and
-                (rule['ip_range'].get('cidr') == cidr_ip or
-                 (rule['group'].get('name') == src_group.name if src_group
-                  else False))):
-                return OpenStackSecurityGroupRule(self._provider, rule, self)
-        return None
+    def delete(self):
+        return self._vm_firewall.delete(self._provider.os_conn.session)
+
+    def refresh(self):
+        self._vm_firewall = self._provider.os_conn.network.get_security_group(
+            self.id)
 
     def to_json(self):
         attr = inspect.getmembers(self, lambda a: not(inspect.isroutine(a)))
@@ -1113,49 +1036,107 @@ class OpenStackSecurityGroup(BaseSecurityGroup):
         return js
 
 
-class OpenStackSecurityGroupRule(BaseSecurityGroupRule):
+class OpenStackVMFirewallRuleContainer(BaseVMFirewallRuleContainer):
+
+    def __init__(self, provider, firewall):
+        super(OpenStackVMFirewallRuleContainer, self).__init__(
+            provider, firewall)
+
+    def list(self, limit=None, marker=None):
+        # pylint:disable=protected-access
+        rules = [OpenStackVMFirewallRule(self.firewall, r)
+                 for r in self.firewall._vm_firewall.security_group_rules]
+        return ClientPagedResultList(self._provider, rules,
+                                     limit=limit, marker=marker)
 
-    def __init__(self, provider, rule, parent):
-        super(OpenStackSecurityGroupRule, self).__init__(
-            provider, rule, parent)
+    def create(self,  direction, protocol=None, from_port=None,
+               to_port=None, cidr=None, src_dest_fw=None):
+        src_dest_fw_id = (src_dest_fw.id if isinstance(src_dest_fw,
+                                                       OpenStackVMFirewall)
+                          else src_dest_fw)
+
+        try:
+            if direction == TrafficDirection.INBOUND:
+                os_direction = 'ingress'
+            elif direction == TrafficDirection.OUTBOUND:
+                os_direction = 'egress'
+            else:
+                raise InvalidValueException("direction", direction)
+            # pylint:disable=protected-access
+            rule = self._provider.os_conn.network.create_security_group_rule(
+                security_group_id=self.firewall.id,
+                direction=os_direction,
+                port_range_max=to_port,
+                port_range_min=from_port,
+                protocol=protocol,
+                remote_ip_prefix=cidr,
+                remote_group_id=src_dest_fw_id)
+            self.firewall.refresh()
+            return OpenStackVMFirewallRule(self.firewall, rule.to_dict())
+        except HttpException as e:
+            self.firewall.refresh()
+            # 409=Conflict, raised for duplicate rule
+            if e.http_status == 409:
+                existing = self.find(direction=direction, protocol=protocol,
+                                     from_port=from_port, to_port=to_port,
+                                     cidr=cidr, src_dest_fw_id=src_dest_fw_id)
+                return existing[0]
+            else:
+                raise e
+
+
+class OpenStackVMFirewallRule(BaseVMFirewallRule):
+
+    def __init__(self, parent_fw, rule):
+        super(OpenStackVMFirewallRule, self).__init__(parent_fw, rule)
 
     @property
     def id(self):
         return self._rule.get('id')
 
     @property
-    def ip_protocol(self):
-        return self._rule.get('ip_protocol')
+    def direction(self):
+        direction = self._rule.get('direction')
+        if direction == 'ingress':
+            return TrafficDirection.INBOUND
+        elif direction == 'egress':
+            return TrafficDirection.OUTBOUND
+        else:
+            return None
+
+    @property
+    def protocol(self):
+        return self._rule.get('protocol')
 
     @property
     def from_port(self):
-        return int(self._rule.get('from_port') or 0)
+        return self._rule.get('port_range_min')
 
     @property
     def to_port(self):
-        return int(self._rule.get('to_port') or 0)
+        return self._rule.get('port_range_max')
 
     @property
-    def cidr_ip(self):
-        return self._rule.get('ip_range', {}).get('cidr')
+    def cidr(self):
+        return self._rule.get('remote_ip_prefix')
 
     @property
-    def group(self):
-        sg_name = self._rule.get('group', {}).get('name')
-        if sg_name:
-            sg = self._provider.security.security_groups.find(name=sg_name)
-            return sg[0] if sg else None
+    def src_dest_fw_id(self):
+        fw = self.src_dest_fw
+        if fw:
+            return fw.id
         return None
 
-    def to_json(self):
-        attr = inspect.getmembers(self, lambda a: not(inspect.isroutine(a)))
-        js = {k: v for(k, v) in attr if not k.startswith('_')}
-        js['group'] = self.group.id if self.group else ''
-        js['parent'] = self.parent.id if self.parent else ''
-        return js
+    @property
+    def src_dest_fw(self):
+        fw_id = self._rule.get('remote_group_id')
+        if fw_id:
+            return self._provider.security.vm_firewalls.get(fw_id)
+        return None
 
     def delete(self):
-        return self._provider.nova.security_group_rules.delete(self.id)
+        self._provider.os_conn.network.delete_security_group_rule(self.id)
+        self.firewall.refresh()
 
 
 class OpenStackBucketObject(BaseBucketObject):

+ 44 - 80
cloudbridge/cloud/providers/openstack/services.py

@@ -20,18 +20,18 @@ from cloudbridge.cloud.base.services import BaseNetworkingService
 from cloudbridge.cloud.base.services import BaseObjectStoreService
 from cloudbridge.cloud.base.services import BaseRegionService
 from cloudbridge.cloud.base.services import BaseRouterService
-from cloudbridge.cloud.base.services import BaseSecurityGroupService
 from cloudbridge.cloud.base.services import BaseSecurityService
 from cloudbridge.cloud.base.services import BaseSnapshotService
 from cloudbridge.cloud.base.services import BaseSubnetService
+from cloudbridge.cloud.base.services import BaseVMFirewallService
 from cloudbridge.cloud.base.services import BaseVMTypeService
 from cloudbridge.cloud.base.services import BaseVolumeService
 from cloudbridge.cloud.interfaces.resources import KeyPair
 from cloudbridge.cloud.interfaces.resources import MachineImage
 from cloudbridge.cloud.interfaces.resources import PlacementZone
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
 from cloudbridge.cloud.interfaces.resources import Snapshot
 from cloudbridge.cloud.interfaces.resources import Subnet
+from cloudbridge.cloud.interfaces.resources import VMFirewall
 from cloudbridge.cloud.interfaces.resources import VMType
 from cloudbridge.cloud.interfaces.resources import Volume
 from cloudbridge.cloud.providers.openstack import helpers as oshelpers
@@ -40,6 +40,8 @@ from neutronclient.common.exceptions import NeutronClientException
 
 from novaclient.exceptions import NotFound as NovaNotFound
 
+from openstack.exceptions import ResourceNotFound
+
 from .resources import OpenStackBucket
 from .resources import OpenStackFloatingIP
 from .resources import OpenStackInstance
@@ -49,9 +51,9 @@ from .resources import OpenStackMachineImage
 from .resources import OpenStackNetwork
 from .resources import OpenStackRegion
 from .resources import OpenStackRouter
-from .resources import OpenStackSecurityGroup
 from .resources import OpenStackSnapshot
 from .resources import OpenStackSubnet
+from .resources import OpenStackVMFirewall
 from .resources import OpenStackVMType
 from .resources import OpenStackVolume
 
@@ -65,7 +67,7 @@ class OpenStackSecurityService(BaseSecurityService):
 
         # Initialize provider services
         self._key_pairs = OpenStackKeyPairService(provider)
-        self._security_groups = OpenStackSecurityGroupService(provider)
+        self._vm_firewalls = OpenStackVMFirewallService(provider)
 
     @property
     def key_pairs(self):
@@ -78,14 +80,14 @@ class OpenStackSecurityService(BaseSecurityService):
         return self._key_pairs
 
     @property
-    def security_groups(self):
+    def vm_firewalls(self):
         """
-        Provides access to security groups for this provider.
+        Provides access to VM firewalls for this provider.
 
-        :rtype: ``object`` of :class:`.SecurityGroupService`
-        :return: a SecurityGroupService object
+        :rtype: ``object`` of :class:`.VMFirewallService`
+        :return: a VMFirewallService object
         """
-        return self._security_groups
+        return self._vm_firewalls
 
     def get_or_create_ec2_credentials(self):
         """
@@ -175,85 +177,47 @@ class OpenStackKeyPairService(BaseKeyPairService):
         return None
 
 
-class OpenStackSecurityGroupService(BaseSecurityGroupService):
+class OpenStackVMFirewallService(BaseVMFirewallService):
 
     def __init__(self, provider):
-        super(OpenStackSecurityGroupService, self).__init__(provider)
+        super(OpenStackVMFirewallService, self).__init__(provider)
 
-    def get(self, sg_id):
-        """
-        Returns a SecurityGroup given its id.
-        """
+    def get(self, firewall_id):
         try:
-            return OpenStackSecurityGroup(
-                self.provider, self.provider.nova.security_groups.get(sg_id))
-        except NovaNotFound:
+            return OpenStackVMFirewall(
+                self.provider,
+                self.provider.os_conn.network.get_security_group(firewall_id))
+        except ResourceNotFound:
             return None
 
     def list(self, limit=None, marker=None):
-        """
-        List all security groups associated with this account.
+        firewalls = [
+            OpenStackVMFirewall(self.provider, fw)
+            for fw in self.provider.os_conn.network.security_groups()]
 
-        :rtype: ``list`` of :class:`.SecurityGroup`
-        :return:  list of SecurityGroup objects
-        """
-
-        sgs = [OpenStackSecurityGroup(self.provider, sg)
-               for sg in self.provider.nova.security_groups.list()]
-
-        return ClientPagedResultList(self.provider, sgs,
+        return ClientPagedResultList(self.provider, firewalls,
                                      limit=limit, marker=marker)
 
     def create(self, name, description, network_id):
-        """
-        Create a new security group under the current account.
-
-        :type name: str
-        :param name: The name of the new security group.
+        OpenStackVMFirewall.assert_valid_resource_name(name)
 
-        :type description: str
-        :param description: The description of the new security group.
-
-        :type  network_id: ``None``
-        :param network_id: Not applicable for OpenStack (yet) so any value is
-                           ignored.
-
-        :rtype: ``object`` of :class:`.SecurityGroup`
-        :return: a SecurityGroup object
-        """
-        OpenStackSecurityGroup.assert_valid_resource_name(name)
-
-        sg = self.provider.nova.security_groups.create(name, description)
+        sg = self.provider.os_conn.network.create_security_group(
+            name=name, description=description)
         if sg:
-            return OpenStackSecurityGroup(self.provider, sg)
+            return OpenStackVMFirewall(self.provider, sg)
         return None
 
     def find(self, name, limit=None, marker=None):
-        """
-        Get all security groups associated with your account.
-        """
-        sgs = self.provider.nova.security_groups.findall(name=name)
-        results = [OpenStackSecurityGroup(self.provider, sg)
-                   for sg in sgs]
+        sgs = [self.provider.os_conn.network.find_security_group(name)]
+        results = [OpenStackVMFirewall(self.provider, sg)
+                   for sg in sgs if sg]
         return ClientPagedResultList(self.provider, results,
                                      limit=limit, marker=marker)
 
     def delete(self, group_id):
-        """
-        Delete an existing SecurityGroup.
-
-        :type group_id: str
-        :param group_id: The security group ID to be deleted.
-
-        :rtype: ``bool``
-        :return:  ``True`` if the security group does not exist, ``False``
-                  otherwise. Note that this implies that the group may not have
-                  been deleted by this method but instead has not existed in
-                  the first place.
-        """
-        sg = self.get(group_id)
-        if sg:
-            sg.delete()
+        firewall = self.get(group_id)
+        if firewall:
+            firewall.delete()
         return True
 
 
@@ -567,7 +531,7 @@ class OpenStackInstanceService(BaseInstanceService):
         super(OpenStackInstanceService, self).__init__(provider)
 
     def create(self, name, image, vm_type, subnet, zone=None,
-               key_pair=None, security_groups=None, user_data=None,
+               key_pair=None, vm_firewalls=None, user_data=None,
                launch_config=None,
                **kwargs):
         """Create a new virtual machine instance."""
@@ -604,13 +568,13 @@ class OpenStackInstanceService(BaseInstanceService):
             log.debug("Creating network port for %s in subnet: %s" %
                       (name, subnet_id))
             sg_list = []
-            if security_groups:
-                if isinstance(security_groups, list) and \
-                        isinstance(security_groups[0], SecurityGroup):
-                    sg_list = security_groups
+            if vm_firewalls:
+                if isinstance(vm_firewalls, list) and \
+                        isinstance(vm_firewalls[0], VMFirewall):
+                    sg_list = vm_firewalls
                 else:
-                    sg_list = (self.provider.security.security_groups
-                               .find(name=sg) for sg in security_groups)
+                    sg_list = (self.provider.security.vm_firewalls
+                               .find(name=sg) for sg in vm_firewalls)
                     sg_list = (sg[0] for sg in sg_list if sg)
             sg_id_list = [sg.id for sg in sg_list]
             port_def = {
@@ -625,12 +589,12 @@ class OpenStackInstanceService(BaseInstanceService):
             port_id = self.provider.neutron.create_port(port_def)['port']['id']
             nics = [{'net-id': net_id, 'port-id': port_id}]
         else:
-            if security_groups:
-                if isinstance(security_groups, list) and \
-                        isinstance(security_groups[0], SecurityGroup):
-                    sg_name_list = [sg.name for sg in security_groups]
+            if vm_firewalls:
+                if isinstance(vm_firewalls, list) and \
+                        isinstance(vm_firewalls[0], VMFirewall):
+                    sg_name_list = [sg.name for sg in vm_firewalls]
                 else:
-                    sg_name_list = security_groups
+                    sg_name_list = vm_firewalls
 
         log.debug("Launching in subnet %s" % subnet_id)
         os_instance = self.provider.nova.servers.create(

+ 1 - 0
setup.py

@@ -25,6 +25,7 @@ REQS_BASE = [
 ]
 REQS_AWS = ['boto3']
 REQS_OPENSTACK = [
+    'openstacksdk',
     'python-novaclient==7.0.0',
     'python-glanceclient>=2.5.0,<=2.6.0',
     'python-cinderclient>=1.9.0,<=2.0.1',

+ 10 - 7
test/helpers/__init__.py

@@ -1,6 +1,7 @@
 import functools
 import os
 import sys
+import traceback
 import unittest
 import uuid
 
@@ -46,11 +47,13 @@ def cleanup_action(cleanup_func):
             cleanup_func()
         except Exception as e:
             print("Error during exception cleanup: {0}".format(e))
+            traceback.print_exc()
         reraise(ex_class, ex_val, ex_traceback)
     try:
         cleanup_func()
     except Exception as e:
         print("Error during cleanup: {0}".format(e))
+        traceback.print_exc()
 
 
 def skipIfNoService(services):
@@ -122,7 +125,7 @@ def delete_test_network(network):
 
 def create_test_instance(
         provider, instance_name, subnet, launch_config=None,
-        key_pair=None, security_groups=None):
+        key_pair=None, vm_firewalls=None):
     return provider.compute.instances.create(
         instance_name,
         get_provider_test_data(provider, 'image'),
@@ -130,11 +133,11 @@ def create_test_instance(
         subnet=subnet,
         zone=get_provider_test_data(provider, 'placement'),
         key_pair=key_pair,
-        security_groups=security_groups,
+        vm_firewalls=vm_firewalls,
         launch_config=launch_config)
 
 
-def get_test_instance(provider, name, key_pair=None, security_groups=None,
+def get_test_instance(provider, name, key_pair=None, vm_firewalls=None,
                       subnet=None):
     launch_config = None
     instance = create_test_instance(
@@ -142,7 +145,7 @@ def get_test_instance(provider, name, key_pair=None, security_groups=None,
         name,
         subnet=subnet,
         key_pair=key_pair,
-        security_groups=security_groups,
+        vm_firewalls=vm_firewalls,
         launch_config=launch_config)
     instance.wait_till_ready()
     return instance
@@ -159,14 +162,14 @@ def delete_test_instance(instance):
                           terminal_states=[InstanceState.ERROR])
 
 
-def cleanup_test_resources(instance=None, network=None, security_group=None,
+def cleanup_test_resources(instance=None, network=None, vm_firewall=None,
                            key_pair=None):
     """Clean up any combination of supplied resources."""
     with cleanup_action(lambda: delete_test_network(network)
                         if network else None):
         with cleanup_action(lambda: key_pair.delete() if key_pair else None):
-            with cleanup_action(lambda: security_group.delete()
-                                if security_group else None):
+            with cleanup_action(lambda: vm_firewall.delete()
+                                if vm_firewall else None):
                 delete_test_instance(instance)
 
 

+ 28 - 26
test/test_compute_service.py

@@ -59,7 +59,7 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         return True
 
     @helpers.skipIfNoService(['compute.instances', 'networking.networks',
-                              'security.security_groups',
+                              'security.vm_firewalls',
                               'security.key_pairs'])
     def test_instance_properties(self):
         name = "cb_inst_props-{0}".format(helpers.get_uuid())
@@ -68,17 +68,17 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         # the cleanup method access to the most current values
         test_instance = None
         net = None
-        sg = None
+        fw = None
         kp = None
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                test_instance, net, sg, kp)):
+                test_instance, net, fw, kp)):
             net, subnet = helpers.create_test_network(self.provider, name)
             kp = self.provider.security.key_pairs.create(name=name)
-            sg = self.provider.security.security_groups.create(
+            fw = self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
             test_instance = helpers.get_test_instance(self.provider,
                                                       name, key_pair=kp,
-                                                      security_groups=[sg],
+                                                      vm_firewalls=[fw],
                                                       subnet=subnet)
             self.assertEqual(
                 test_instance.name, name,
@@ -98,26 +98,28 @@ class CloudComputeServiceTestCase(ProviderTestBase):
             self.assertEqual(
                 test_instance.key_pair_name,
                 kp.name)
-            self.assertIsInstance(test_instance.security_groups, list)
+            self.assertIsInstance(test_instance.vm_firewalls, list)
             self.assertEqual(
-                test_instance.security_groups[0],
-                sg)
-            self.assertIsInstance(test_instance.security_group_ids, list)
+                test_instance.vm_firewalls[0],
+                fw)
+            self.assertIsInstance(test_instance.vm_firewall_ids, list)
             self.assertEqual(
-                test_instance.security_group_ids[0],
-                sg.id)
+                test_instance.vm_firewall_ids[0],
+                fw.id)
             # Must have either a public or a private ip
             ip_private = test_instance.private_ips[0] \
                 if test_instance.private_ips else None
             ip_address = test_instance.public_ips[0] \
                 if test_instance.public_ips and test_instance.public_ips[0] \
                 else ip_private
+            # Convert to unicode for py27 compatibility with ipaddress()
+            ip_address = u"{}".format(ip_address)
             self.assertIsNotNone(
                 ip_address,
                 "Instance must have either a public IP or a private IP")
             self.assertTrue(
                 self._is_valid_ip(ip_address),
-                "Instance must have a valid IP address")
+                "Instance must have a valid IP address. Got: %s" % ip_address)
             self.assertIsInstance(test_instance.vm_type_id,
                                   six.string_types)
             vm_type = self.provider.compute.vm_types.get(
@@ -287,7 +289,7 @@ class CloudComputeServiceTestCase(ProviderTestBase):
                         # correspond to requested mappings
 
     @helpers.skipIfNoService(['compute.instances', 'networking.networks',
-                              'security.security_groups'])
+                              'security.vm_firewalls'])
     def test_instance_methods(self):
         name = "cb_instmethods-{0}".format(helpers.get_uuid())
 
@@ -295,30 +297,30 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         # the cleanup method access to the most current values
         test_inst = None
         net = None
-        sg = None
+        fw = None
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                test_inst, net, sg)):
+                test_inst, net, fw)):
             net, subnet = helpers.create_test_network(self.provider, name)
             test_inst = helpers.get_test_instance(self.provider, name,
                                                   subnet=subnet)
-            sg = self.provider.security.security_groups.create(
+            fw = self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
 
-            # Check adding a security group to a running instance
-            test_inst.add_security_group(sg)
+            # Check adding a VM firewall to a running instance
+            test_inst.add_vm_firewall(fw)
             test_inst.refresh()
             self.assertTrue(
-                sg in test_inst.security_groups, "Expected security group '%s'"
-                " to be among instance security_groups: [%s]" %
-                (sg, test_inst.security_groups))
+                fw in test_inst.vm_firewalls, "Expected VM firewall '%s'"
+                " to be among instance vm_firewalls: [%s]" %
+                (fw, test_inst.vm_firewalls))
 
-            # Check removing a security group from a running instance
-            test_inst.remove_security_group(sg)
+            # Check removing a VM firewall from a running instance
+            test_inst.remove_vm_firewall(fw)
             test_inst.refresh()
             self.assertTrue(
-                sg not in test_inst.security_groups, "Expected security group"
-                " '%s' to be removed from instance security_groups: [%s]" %
-                (sg, test_inst.security_groups))
+                fw not in test_inst.vm_firewalls, "Expected VM firewall"
+                " '%s' to be removed from instance vm_firewalls: [%s]" %
+                (fw, test_inst.vm_firewalls))
 
             # check floating ips
             router = self.provider.networking.routers.create(name, net)

+ 112 - 86
test/test_security_service.py

@@ -4,7 +4,9 @@ from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 
 from cloudbridge.cloud.interfaces.resources import KeyPair
-from cloudbridge.cloud.interfaces.resources import SecurityGroup
+from cloudbridge.cloud.interfaces.resources import TrafficDirection
+from cloudbridge.cloud.interfaces.resources import VMFirewall
+from cloudbridge.cloud.interfaces.resources import VMFirewallRule
 
 
 class CloudSecurityServiceTestCase(ProviderTestBase):
@@ -40,135 +42,159 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
             self.assertIsNone(kp.material,
                               "Keypair material should now be empty")
 
-    @helpers.skipIfNoService(['security.security_groups'])
-    def test_crud_security_group(self):
-        name = 'cb_crudsg-{0}'.format(helpers.get_uuid())
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_crud_vm_firewall(self):
+        name = 'cb_crudfw-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
         net = None
 
-        def create_sg(name):
-            return self.provider.security.security_groups.create(
+        def create_fw(name):
+            return self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
 
-        def cleanup_sg(sg):
-            sg.delete()
+        def cleanup_fw(fw):
+            fw.delete()
 
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
                 network=net)):
             net, _ = helpers.create_test_network(self.provider, name)
 
-            sit.check_crud(self, self.provider.security.security_groups,
-                           SecurityGroup, "cb_crudsg", create_sg, cleanup_sg)
+            sit.check_crud(self, self.provider.security.vm_firewalls,
+                           VMFirewall, "cb_crudfw", create_fw, cleanup_fw)
 
-    @helpers.skipIfNoService(['security.security_groups'])
-    def test_security_group_properties(self):
-        name = 'cb_propsg-{0}'.format(helpers.get_uuid())
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_vm_firewall_properties(self):
+        name = 'cb_propfw-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
         net = None
-        sg = None
+        fw = None
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                network=net, security_group=sg)):
+                network=net, vm_firewall=fw)):
             net, _ = helpers.create_test_network(self.provider, name)
-            sg = self.provider.security.security_groups.create(
+            fw = self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
 
-            self.assertEqual(name, sg.description)
+            self.assertEqual(name, fw.description)
 
-            rule = sg.add_rule(ip_protocol='tcp', from_port=1111, to_port=1111,
-                               cidr_ip='0.0.0.0/0')
-            found_rule = sg.get_rule(ip_protocol='tcp', from_port=1111,
-                                     to_port=1111, cidr_ip='0.0.0.0/0')
-            self.assertTrue(
-                rule == found_rule,
-                "Expected rule {0} not found in security group: {0}".format(
-                    rule, sg.rules))
-
-            object_keys = (
-                sg.rules[0].ip_protocol,
-                sg.rules[0].from_port,
-                sg.rules[0].to_port)
-            self.assertTrue(
-                all(str(key) in repr(sg.rules[0]) for key in object_keys),
-                "repr(obj) should contain ip_protocol, form_port, and to_port"
-                " so that the object can be reconstructed, but does not:"
-                " {0}; {1}".format(sg.rules[0], object_keys))
-            self.assertTrue(
-                sg == sg,
-                "The same security groups should be equal?")
-            self.assertFalse(
-                sg != sg,
-                "The same security groups should still be equal?")
-
-        sit.check_delete(self, self.provider.security.security_groups, sg)
-        sgl = self.provider.security.security_groups.list()
-        found_sg = [g for g in sgl if g.name == name]
-        self.assertTrue(
-            len(found_sg) == 0,
-            "Security group {0} should have been deleted but still exists."
-            .format(name))
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_crud_vm_firewall_rules(self):
+        name = 'cb_crudfw_rules-{0}'.format(helpers.get_uuid())
+
+        # Declare these variables and late binding will allow
+        # the cleanup method access to the most current values
+        net = None
+        with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
+                network=net)):
+            net, _ = helpers.create_test_network(self.provider, name)
+
+            fw = None
+            with helpers.cleanup_action(lambda: fw.delete()):
+                fw = self.provider.security.vm_firewalls.create(
+                    name=name, description=name, network_id=net.id)
 
-    @helpers.skipIfNoService(['security.security_groups'])
-    def test_security_group_rule_add_twice(self):
-        name = 'cb_sgruletwice-{0}'.format(helpers.get_uuid())
+                def create_fw_rule(name):
+                    return fw.rules.create(
+                        direction=TrafficDirection.INBOUND, protocol='tcp',
+                        from_port=1111, to_port=1111, cidr='0.0.0.0/0')
+
+                def cleanup_fw_rule(rule):
+                    rule.delete()
+
+                sit.check_crud(self, fw.rules, VMFirewallRule, "cb_crudfwrule",
+                               create_fw_rule, cleanup_fw_rule,
+                               skip_name_check=True)
+
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_vm_firewall_rule_properties(self):
+        name = 'cb_propfwrule-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
         net = None
-        sg = None
+        fw = None
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                network=net, security_group=sg)):
+                network=net, vm_firewall=fw)):
+            net, _ = helpers.create_test_network(self.provider, name)
+            fw = self.provider.security.vm_firewalls.create(
+                name=name, description=name, network_id=net.id)
+
+            rule = fw.rules.create(
+                direction=TrafficDirection.INBOUND, protocol='tcp',
+                from_port=1111, to_port=1111, cidr='0.0.0.0/0')
+            self.assertEqual(rule.direction, TrafficDirection.INBOUND)
+            self.assertEqual(rule.protocol, 'tcp')
+            self.assertEqual(rule.from_port, 1111)
+            self.assertEqual(rule.to_port, 1111)
+            self.assertEqual(rule.cidr, '0.0.0.0/0')
+
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_vm_firewall_rule_add_twice(self):
+        name = 'cb_fwruletwice-{0}'.format(helpers.get_uuid())
+
+        # Declare these variables and late binding will allow
+        # the cleanup method access to the most current values
+        net = None
+        fw = None
+        with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
+                network=net, vm_firewall=fw)):
 
             net, _ = helpers.create_test_network(self.provider, name)
-            sg = self.provider.security.security_groups.create(
+            fw = self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
 
-            rule = sg.add_rule(ip_protocol='tcp', from_port=1111, to_port=1111,
-                               cidr_ip='0.0.0.0/0')
+            rule = fw.rules.create(
+                direction=TrafficDirection.INBOUND, protocol='tcp',
+                from_port=1111, to_port=1111, cidr='0.0.0.0/0')
             # attempting to add the same rule twice should succeed
-            same_rule = sg.add_rule(ip_protocol='tcp', from_port=1111,
-                                    to_port=1111, cidr_ip='0.0.0.0/0')
-            self.assertTrue(
-                rule == same_rule,
-                "Expected rule {0} not found in security group: {0}".format(
-                    same_rule, sg.rules))
+            same_rule = fw.rules.create(
+                direction=TrafficDirection.INBOUND, protocol='tcp',
+                from_port=1111, to_port=1111, cidr='0.0.0.0/0')
+            self.assertEqual(rule, same_rule)
 
-    @helpers.skipIfNoService(['security.security_groups'])
-    def test_security_group_group_rule(self):
-        name = 'cb_sgrule-{0}'.format(helpers.get_uuid())
+    @helpers.skipIfNoService(['security.vm_firewalls'])
+    def test_vm_firewall_group_rule(self):
+        name = 'cb_fwrule-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
         net = None
-        sg = None
+        fw = None
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                network=net, security_group=sg)):
+                network=net, vm_firewall=fw)):
             net, _ = helpers.create_test_network(self.provider, name)
-            sg = self.provider.security.security_groups.create(
+            fw = self.provider.security.vm_firewalls.create(
                 name=name, description=name, network_id=net.id)
+            rules = list(fw.rules)
             self.assertTrue(
-                len(sg.rules) == 0,
-                "Expected no security group group rule. Got {0}."
-                .format(sg.rules))
-            rule = sg.add_rule(src_group=sg, ip_protocol='tcp', from_port=1,
-                               to_port=65535)
+                # TODO: This should be made consistent across all providers.
+                # Currently, OpenStack creates two rules, one for IPV6 and
+                # another for IPV4
+                len(rules) >= 1, "Expected a single VM firewall rule allowing"
+                " all outbound traffic. Got {0}.".format(rules))
+            self.assertEqual(
+                rules[0].direction, TrafficDirection.OUTBOUND,
+                "Expected rule to be outbound. Got {0}.".format(rules))
+            rule = fw.rules.create(
+                direction=TrafficDirection.INBOUND, src_dest_fw=fw,
+                protocol='tcp', from_port=1, to_port=65535)
             self.assertTrue(
-                rule.group.name == name,
-                "Expected security group rule name {0}. Got {1}."
-                .format(name, rule.group.name))
-            for r in sg.rules:
+                rule.src_dest_fw.name == name,
+                "Expected VM firewall rule name {0}. Got {1}."
+                .format(name, rule.src_dest_fw.name))
+            for r in fw.rules:
                 r.delete()
-            sg = self.provider.security.security_groups.get(sg.id)  # update
+            fw = self.provider.security.vm_firewalls.get(fw.id)  # update
             self.assertTrue(
-                len(sg.rules) == 0,
-                "Deleting SecurityGroupRule should delete it: {0}".format(
-                    sg.rules))
-        sgl = self.provider.security.security_groups.list()
-        found_sg = [g for g in sgl if g.name == name]
+                len(list(fw.rules)) == 0,
+                "Deleting VMFirewallRule should delete it: {0}".format(
+                    fw.rules))
+        fwl = self.provider.security.vm_firewalls.list()
+        found_fw = [f for f in fwl if f.name == name]
         self.assertTrue(
-            len(found_sg) == 0,
-            "Security group {0} should have been deleted but still exists."
+            len(found_fw) == 0,
+            "VM firewall {0} should have been deleted but still exists."
             .format(name))