From 08b2f3191537b95b6056067aad0416ea0b881b82 Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Tue, 31 Mar 2015 16:37:07 -0400
Subject: [PATCH 01/17] Add OpenStack Security Group Rule module

---
 cloud/openstack/os_security_group_rule.py | 154 ++++++++++++++++++++++
 1 file changed, 154 insertions(+)
 create mode 100644 cloud/openstack/os_security_group_rule.py

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
new file mode 100644
index 00000000000..d539cf91ee5
--- /dev/null
+++ b/cloud/openstack/os_security_group_rule.py
@@ -0,0 +1,154 @@
+#!/usr/bin/python
+
+# Copyright (c) 2015 Hewlett-Packard Development Company, L.P.
+# Copyright (c) 2013, Benno Joy <benno@ansible.com>
+#
+# This module is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This software is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this software.  If not, see <http://www.gnu.org/licenses/>.
+
+try:
+    import shade
+except ImportError:
+    print("failed=True msg='shade is required for this module'")
+
+
+DOCUMENTATION = '''
+---
+module: os_security_group_rule
+short_description: Add/Delete rule from an existing security group
+extends_documentation_fragment: openstack
+version_added: "1.10"
+description:
+   - Add or Remove rule from an existing security group
+options:
+   security_group:
+     description:
+        - Name of the security group
+     required: true
+   protocol:
+      description:
+        - IP protocol
+      choices: ['tcp', 'udp', 'icmp']
+      default: tcp
+   port_range_min:
+      description:
+        - Starting port
+      required: true
+   port_range_max:
+      description:
+        - Ending port
+     required: true
+   remote_ip_prefix:
+      description:
+        - Source IP address(es) in CIDR notation (exclusive with remote_group)
+     required: false
+   remote_group:
+      description:
+        - ID of Security group to link (exclusive with remote_ip_prefix)
+     required: false
+   state:
+     description:
+       - Should the resource be present or absent.
+     choices: [present, absent]
+     default: present
+
+requirements: ["shade"]
+'''
+# TODO(mordred): add ethertype and direction
+
+EXAMPLES = '''
+# Create a security group rule
+- os_security_group_rule: cloud=mordred
+                          security_group=group foo
+                          protocol: tcp
+                          port_range_min: 80
+                          port_range_max: 80
+                          remote_ip_prefix: 0.0.0.0/0
+'''
+
+
+def _security_group_rule(module, nova_client, action='create', **kwargs):
+    f = getattr(nova_client.security_group_rules, action)
+    try:
+        secgroup = f(**kwargs)
+    except Exception, e:
+        module.fail_json(msg='Failed to %s security group rule: %s' %
+                         (action, e.message))
+
+
+def _get_rule_from_group(module, secgroup):
+    for rule in secgroup.rules:
+        if (rule['ip_protocol'] == module.params['protocol'] and
+            rule['from_port'] == module.params['port_range_min'] and
+            rule['to_port'] == module.params['port_range_max'] and
+            rule['ip_range']['cidr'] == module.params['remote_ip_prefix']):
+            return rule
+    return None
+
+def main():
+
+    argument_spec = openstack_full_argument_spec(
+        security_group      = dict(required=True),
+        protocol            = dict(default='tcp', choices=['tcp', 'udp', 'icmp']),
+        port_range_min      = dict(required=True),
+        port_range_max      = dict(required=True),
+        remote_ip_prefix    = dict(required=False, default=None),
+        # TODO(mordred): Make remote_group handle name and id
+        remote_group        = dict(required=False, default=None),
+        state               = dict(default='present', choices=['absent', 'present']),
+    )
+    module_kwargs = openstack_module_kwargs(
+        mutually_exclusive=[
+            ['remote_ip_prefix', 'remote_group'],
+    )
+    module = AnsibleModule(argument_spec, **module_kwargs)
+
+    try:
+        cloud = shade.openstack_cloud(**module.params)
+        nova_client = cloud.nova_client
+        changed = False
+
+        secgroup = cloud.get_security_group(module.params['security_group'])
+
+        if module.params['state'] == 'present':
+            if not secgroup:
+                module.fail_json(msg='Could not find security group %s' %
+                                 module.params['security_group'])
+
+            if not _get_rule_from_group(module, secgroup):
+                _security_group_rule(module, nova_client, 'create',
+                                     parent_group_id=secgroup.id,
+                                     ip_protocol=module.params['protocol'],
+                                     from_port=module.params['port_range_min'],
+                                     to_port=module.params['port_range_max'],
+                                     cidr=module.params['remote_ip'],
+                                     group_id=module.params['remote_group'],
+                changed = True
+
+
+        if module.params['state'] == 'absent' and secgroup:
+            rule = _get_rule_from_group(module, secgroup)
+            if secgroup and rule:
+                _security_group_rule(module, nova_client, 'delete',
+                                     rule=rule['id'])
+                changed = True
+
+        module.exit_json(changed=changed, result="success")
+
+    except shade.OpenStackCloudException as e:
+        module.fail_json(msg=e.message)
+
+# this is magic, see lib/ansible/module_common.py
+from ansible.module_utils.basic import *
+from ansible.module_utils.openstack import *
+main()

From 08b4bb42c4ad99a9c43193b40f62220240da0af8 Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Thu, 4 Jun 2015 12:03:05 -0400
Subject: [PATCH 02/17] Fix example code syntax

---
 cloud/openstack/os_security_group_rule.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index d539cf91ee5..8422a920791 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -68,12 +68,13 @@ requirements: ["shade"]
 
 EXAMPLES = '''
 # Create a security group rule
-- os_security_group_rule: cloud=mordred
-                          security_group=group foo
-                          protocol: tcp
-                          port_range_min: 80
-                          port_range_max: 80
-                          remote_ip_prefix: 0.0.0.0/0
+- os_security_group_rule:
+    cloud: mordred
+    security_group: foo
+    protocol: tcp
+    port_range_min: 80
+    port_range_max: 80
+    remote_ip_prefix: 0.0.0.0/0
 '''
 
 

From a9301ba918736db08cf5b2b160f91ed4724ba7e8 Mon Sep 17 00:00:00 2001
From: Davide Guerri <davide.guerri@gmail.com>
Date: Thu, 4 Jun 2015 17:34:21 +0100
Subject: [PATCH 03/17] Fix invalid syntax in openstack_module_kwargs call

---
 cloud/openstack/os_security_group_rule.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 8422a920791..903d694bab3 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -111,6 +111,7 @@ def main():
     module_kwargs = openstack_module_kwargs(
         mutually_exclusive=[
             ['remote_ip_prefix', 'remote_group'],
+        ]
     )
     module = AnsibleModule(argument_spec, **module_kwargs)
 

From d35df1f2170f8347af4b49548a03d265c9b69e15 Mon Sep 17 00:00:00 2001
From: dagnello <dagnello@hp.com>
Date: Mon, 8 Jun 2015 18:27:40 -0700
Subject: [PATCH 04/17] Minor fixes for os_security_group_rule module

Was not able to use this module as it was.  The changes submitted
resolved the issues I ran into in order to get it working.
---
 cloud/openstack/os_security_group_rule.py | 24 +++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 903d694bab3..fc5397439c0 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -21,13 +21,12 @@ try:
 except ImportError:
     print("failed=True msg='shade is required for this module'")
 
-
 DOCUMENTATION = '''
 ---
 module: os_security_group_rule
 short_description: Add/Delete rule from an existing security group
 extends_documentation_fragment: openstack
-version_added: "1.10"
+version_added: "2.0"
 description:
    - Add or Remove rule from an existing security group
 options:
@@ -61,7 +60,6 @@ options:
        - Should the resource be present or absent.
      choices: [present, absent]
      default: present
-
 requirements: ["shade"]
 '''
 # TODO(mordred): add ethertype and direction
@@ -84,7 +82,7 @@ def _security_group_rule(module, nova_client, action='create', **kwargs):
         secgroup = f(**kwargs)
     except Exception, e:
         module.fail_json(msg='Failed to %s security group rule: %s' %
-                         (action, e.message))
+                             (action, e.message))
 
 
 def _get_rule_from_group(module, secgroup):
@@ -92,12 +90,14 @@ def _get_rule_from_group(module, secgroup):
         if (rule['ip_protocol'] == module.params['protocol'] and
             rule['from_port'] == module.params['port_range_min'] and
             rule['to_port'] == module.params['port_range_max'] and
-            rule['ip_range']['cidr'] == module.params['remote_ip_prefix']):
+            (rule['ip_range']['cidr'] if 'cidr' in rule['ip_range']
+             else None) == (module.params['remote_ip_prefix'] if
+             'remote_ip_prefix' in module.params else None)):
             return rule
     return None
 
-def main():
 
+def main():
     argument_spec = openstack_full_argument_spec(
         security_group      = dict(required=True),
         protocol            = dict(default='tcp', choices=['tcp', 'udp', 'icmp']),
@@ -133,11 +133,14 @@ def main():
                                      ip_protocol=module.params['protocol'],
                                      from_port=module.params['port_range_min'],
                                      to_port=module.params['port_range_max'],
-                                     cidr=module.params['remote_ip'],
-                                     group_id=module.params['remote_group'],
+                                     cidr=module.params['remote_ip_prefix']
+                                     if 'remote_ip_prefix' in module.params else None,
+                                     group_id=module.params['remote_group']
+                                     if 'remote_group' in module.params else
+                                     None
+                )
                 changed = True
 
-
         if module.params['state'] == 'absent' and secgroup:
             rule = _get_rule_from_group(module, secgroup)
             if secgroup and rule:
@@ -153,4 +156,5 @@ def main():
 # this is magic, see lib/ansible/module_common.py
 from ansible.module_utils.basic import *
 from ansible.module_utils.openstack import *
-main()
+
+main()
\ No newline at end of file

From b98e6663e8cdd29eecb6614ae12a814df972441e Mon Sep 17 00:00:00 2001
From: dagnello <dagnello@hp.com>
Date: Mon, 8 Jun 2015 18:27:40 -0700
Subject: [PATCH 05/17] Minor fixes for os_security_group_rule module

Was not able to use this module as it was.  The changes submitted
resolved the issues I ran into in order to get it working.
---
 cloud/openstack/os_security_group_rule.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index fc5397439c0..efbc41f1148 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -99,14 +99,14 @@ def _get_rule_from_group(module, secgroup):
 
 def main():
     argument_spec = openstack_full_argument_spec(
-        security_group      = dict(required=True),
-        protocol            = dict(default='tcp', choices=['tcp', 'udp', 'icmp']),
-        port_range_min      = dict(required=True),
-        port_range_max      = dict(required=True),
-        remote_ip_prefix    = dict(required=False, default=None),
+        security_group     = dict(required=True),
+        protocol           = dict(default='tcp', choices=['tcp', 'udp', 'icmp']),
+        port_range_min     = dict(required=True),
+        port_range_max     = dict(required=True),
+        remote_ip_prefix   = dict(required=False, default=None),
         # TODO(mordred): Make remote_group handle name and id
-        remote_group        = dict(required=False, default=None),
-        state               = dict(default='present', choices=['absent', 'present']),
+        remote_group       = dict(required=False, default=None),
+        state              = dict(default='present', choices=['absent', 'present']),
     )
     module_kwargs = openstack_module_kwargs(
         mutually_exclusive=[
@@ -157,4 +157,4 @@ def main():
 from ansible.module_utils.basic import *
 from ansible.module_utils.openstack import *
 
-main()
\ No newline at end of file
+main()

From 8f2e70a1c156bde17a117c5a212c71d4d67ccd8f Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Wed, 10 Jun 2015 17:31:43 -0400
Subject: [PATCH 06/17] Update rules mode for latest shade

Shade 0.7.0 normalized the security group data that is returned,
when using nova, to look more like neutron security group data.
This adjusts for that change.
---
 cloud/openstack/os_security_group_rule.py | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index efbc41f1148..42e0a6bc6ed 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -86,13 +86,11 @@ def _security_group_rule(module, nova_client, action='create', **kwargs):
 
 
 def _get_rule_from_group(module, secgroup):
-    for rule in secgroup.rules:
-        if (rule['ip_protocol'] == module.params['protocol'] and
-            rule['from_port'] == module.params['port_range_min'] and
-            rule['to_port'] == module.params['port_range_max'] and
-            (rule['ip_range']['cidr'] if 'cidr' in rule['ip_range']
-             else None) == (module.params['remote_ip_prefix'] if
-             'remote_ip_prefix' in module.params else None)):
+    for rule in secgroup['security_group_rules']:
+        if (rule['protocol'] == module.params['protocol'] and
+                rule['port_range_min'] == module.params['port_range_min'] and
+                rule['port_range_max'] == module.params['port_range_max'] and
+                rule['remote_ip_prefix'] == module.params['remote_ip_prefix']):
             return rule
     return None
 
@@ -136,9 +134,8 @@ def main():
                                      cidr=module.params['remote_ip_prefix']
                                      if 'remote_ip_prefix' in module.params else None,
                                      group_id=module.params['remote_group']
-                                     if 'remote_group' in module.params else
-                                     None
-                )
+                                     if 'remote_group' in module.params else None
+                                     )
                 changed = True
 
         if module.params['state'] == 'absent' and secgroup:

From 5758b4ebdce00bbd18ef8e6967122ebcc6de0cde Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Wed, 10 Jun 2015 17:51:59 -0400
Subject: [PATCH 07/17] Fix id value reference

---
 cloud/openstack/os_security_group_rule.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 42e0a6bc6ed..287f3021a35 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -127,7 +127,7 @@ def main():
 
             if not _get_rule_from_group(module, secgroup):
                 _security_group_rule(module, nova_client, 'create',
-                                     parent_group_id=secgroup.id,
+                                     parent_group_id=secgroup['id'],
                                      ip_protocol=module.params['protocol'],
                                      from_port=module.params['port_range_min'],
                                      to_port=module.params['port_range_max'],

From 5b6c6cac20bc6e1111e0175b2e77c7c3f61a69b5 Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Wed, 10 Jun 2015 18:06:11 -0400
Subject: [PATCH 08/17] Recongnize None and -1 port equivalency

shade 0.7.0 represents disabled min/max ports as None (in the
neutron style) rather than -1. Recognize this as the same as -1.
---
 cloud/openstack/os_security_group_rule.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 287f3021a35..64f67fbeec1 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -87,9 +87,12 @@ def _security_group_rule(module, nova_client, action='create', **kwargs):
 
 def _get_rule_from_group(module, secgroup):
     for rule in secgroup['security_group_rules']:
+        # No port, or -1, will be returned as None
+        port_range_min = rule['port_range_min'] or -1
+        port_range_max = rule['port_range_max'] or -1
         if (rule['protocol'] == module.params['protocol'] and
-                rule['port_range_min'] == module.params['port_range_min'] and
-                rule['port_range_max'] == module.params['port_range_max'] and
+                port_range_min == module.params['port_range_min'] and
+                port_range_max == module.params['port_range_max'] and
                 rule['remote_ip_prefix'] == module.params['remote_ip_prefix']):
             return rule
     return None

From 16b3b72294ec509b7d327af73ce750c9c25c437a Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Tue, 16 Jun 2015 14:56:04 -0400
Subject: [PATCH 09/17] Update secgroup rules module for latest shade

This allows the rules module to work against either nova or neutron
for handling security groups. New parameters for 'direction' and
'ethertype' are added.

Check mode is supported with this version.
---
 cloud/openstack/os_security_group_rule.py | 141 ++++++++++++++--------
 1 file changed, 93 insertions(+), 48 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 64f67fbeec1..a5596558710 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -46,15 +46,27 @@ options:
    port_range_max:
       description:
         - Ending port
-     required: true
+      required: true
    remote_ip_prefix:
       description:
         - Source IP address(es) in CIDR notation (exclusive with remote_group)
-     required: false
+      required: false
    remote_group:
       description:
         - ID of Security group to link (exclusive with remote_ip_prefix)
-     required: false
+      required: false
+   ethertype:
+      description:
+        - Must be IPv4 or IPv6, and addresses represented in CIDR must
+          match the ingress or egress rules. Not all providers support IPv6.
+      choices: ['IPv4', 'IPv6']
+      default: IPv4
+   direction:
+     description:
+        - The direction in which the security group rule is applied. Not
+          all providers support egress.
+     choices: ['egress', 'ingress']
+     default: ingress
    state:
      description:
        - Should the resource be present or absent.
@@ -76,79 +88,112 @@ EXAMPLES = '''
 '''
 
 
-def _security_group_rule(module, nova_client, action='create', **kwargs):
-    f = getattr(nova_client.security_group_rules, action)
-    try:
-        secgroup = f(**kwargs)
-    except Exception, e:
-        module.fail_json(msg='Failed to %s security group rule: %s' %
-                             (action, e.message))
+def _find_matching_rule(module, secgroup):
+    """
+    Find a rule in the group that matches the module parameters.
 
+    :returns: The matching rule dict, or None if no matches.
+    """
+    protocol = module.params['protocol']
+    port_range_min = module.params['port_range_min']
+    port_range_max = module.params['port_range_max']
+    remote_ip_prefix = module.params['remote_ip_prefix']
+    ethertype = module.params['ethertype']
+    direction = module.params['direction']
 
-def _get_rule_from_group(module, secgroup):
     for rule in secgroup['security_group_rules']:
-        # No port, or -1, will be returned as None
-        port_range_min = rule['port_range_min'] or -1
-        port_range_max = rule['port_range_max'] or -1
-        if (rule['protocol'] == module.params['protocol'] and
-                port_range_min == module.params['port_range_min'] and
-                port_range_max == module.params['port_range_max'] and
-                rule['remote_ip_prefix'] == module.params['remote_ip_prefix']):
+        # No port, or -1, will be returned from shade as None
+        rule_port_range_min = rule['port_range_min'] or -1
+        rule_port_range_max = rule['port_range_max'] or -1
+
+        if (protocol == rule['protocol']
+                and port_range_min == rule_port_range_min
+                and port_range_max == rule_port_range_max
+                and remote_ip_prefix == rule['remote_ip_prefix']
+                and ethertype == rule['ethertype']
+                and direction == rule['direction']):
             return rule
     return None
 
 
+def _system_state_change(module, secgroup):
+    state = module.params['state']
+    if secgroup:
+        rule_exists = _find_matching_rule(module, secgroup)
+    else:
+        return False
+
+    if state == 'present' and not rule_exists:
+        return True
+    if state == 'absent' and rule_exists:
+        return True
+    return False
+
+
 def main():
     argument_spec = openstack_full_argument_spec(
-        security_group     = dict(required=True),
-        protocol           = dict(default='tcp', choices=['tcp', 'udp', 'icmp']),
-        port_range_min     = dict(required=True),
-        port_range_max     = dict(required=True),
-        remote_ip_prefix   = dict(required=False, default=None),
+        security_group   = dict(required=True),
+        protocol         = dict(default='tcp',
+                                choices=['tcp', 'udp', 'icmp']),
+        port_range_min   = dict(required=True),
+        port_range_max   = dict(required=True),
+        remote_ip_prefix = dict(required=False, default=None),
         # TODO(mordred): Make remote_group handle name and id
-        remote_group       = dict(required=False, default=None),
-        state              = dict(default='present', choices=['absent', 'present']),
+        remote_group     = dict(required=False, default=None),
+        ethertype        = dict(default='IPv4',
+                                choices=['IPv4', 'IPv6']),
+        direction        = dict(default='ingress',
+                                choices=['egress', 'ingress']),
+        state            = dict(default='present',
+                                choices=['absent', 'present']),
     )
+
     module_kwargs = openstack_module_kwargs(
         mutually_exclusive=[
             ['remote_ip_prefix', 'remote_group'],
         ]
     )
-    module = AnsibleModule(argument_spec, **module_kwargs)
+
+    module = AnsibleModule(argument_spec,
+                           supports_check_mode=True,
+                           **module_kwargs)
+
+    state = module.params['state']
+    security_group = module.params['security_group']
+    changed = False
 
     try:
         cloud = shade.openstack_cloud(**module.params)
-        nova_client = cloud.nova_client
-        changed = False
+        secgroup = cloud.get_security_group(security_group)
 
-        secgroup = cloud.get_security_group(module.params['security_group'])
+        if module.check_mode:
+            module.exit_json(changed=_system_state_change(module, secgroup))
 
-        if module.params['state'] == 'present':
+        if state == 'present':
             if not secgroup:
                 module.fail_json(msg='Could not find security group %s' %
-                                 module.params['security_group'])
+                                 security_group)
 
-            if not _get_rule_from_group(module, secgroup):
-                _security_group_rule(module, nova_client, 'create',
-                                     parent_group_id=secgroup['id'],
-                                     ip_protocol=module.params['protocol'],
-                                     from_port=module.params['port_range_min'],
-                                     to_port=module.params['port_range_max'],
-                                     cidr=module.params['remote_ip_prefix']
-                                     if 'remote_ip_prefix' in module.params else None,
-                                     group_id=module.params['remote_group']
-                                     if 'remote_group' in module.params else None
-                                     )
+            if not _find_matching_rule(module, secgroup):
+                cloud.create_security_group_rule(
+                    secgroup['id'],
+                    port_range_min=module.params['port_range_min'],
+                    port_range_max=module.params['port_range_max'],
+                    protocol=module.params['protocol'],
+                    remote_ip_prefix=module.params['remote_ip_prefix'],
+                    remote_group_id=module.params['remote_group'],
+                    direction=module.params['direction'],
+                    ethertype=module.params['ethertype']
+                )
                 changed = True
 
-        if module.params['state'] == 'absent' and secgroup:
-            rule = _get_rule_from_group(module, secgroup)
-            if secgroup and rule:
-                _security_group_rule(module, nova_client, 'delete',
-                                     rule=rule['id'])
+        if state == 'absent' and secgroup:
+            rule = _find_matching_rule(module, secgroup)
+            if rule:
+                cloud.delete_security_group_rule(rule['id'])
                 changed = True
 
-        module.exit_json(changed=changed, result="success")
+        module.exit_json(changed=changed)
 
     except shade.OpenStackCloudException as e:
         module.fail_json(msg=e.message)

From 0e5942d7e7bfd703fad5797362a0ebe1572674e6 Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Wed, 17 Jun 2015 07:39:27 -0400
Subject: [PATCH 10/17] Return rule object

---
 cloud/openstack/os_security_group_rule.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index a5596558710..15ce00866ae 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -174,8 +174,9 @@ def main():
                 module.fail_json(msg='Could not find security group %s' %
                                  security_group)
 
-            if not _find_matching_rule(module, secgroup):
-                cloud.create_security_group_rule(
+            rule = _find_matching_rule(module, secgroup):
+            if not rule:
+                rule = cloud.create_security_group_rule(
                     secgroup['id'],
                     port_range_min=module.params['port_range_min'],
                     port_range_max=module.params['port_range_max'],
@@ -186,6 +187,7 @@ def main():
                     ethertype=module.params['ethertype']
                 )
                 changed = True
+            module.exit_json(changed=changed, rule=rule, id=rule.id)
 
         if state == 'absent' and secgroup:
             rule = _find_matching_rule(module, secgroup)
@@ -193,7 +195,7 @@ def main():
                 cloud.delete_security_group_rule(rule['id'])
                 changed = True
 
-        module.exit_json(changed=changed)
+            module.exit_json(changed=changed)
 
     except shade.OpenStackCloudException as e:
         module.fail_json(msg=e.message)

From 9d0c8b0507a19e82a5dc23c9a8a8cac0b24c9f92 Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Wed, 17 Jun 2015 08:30:55 -0400
Subject: [PATCH 11/17] Fix syntax error

---
 cloud/openstack/os_security_group_rule.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 15ce00866ae..f50c97817e5 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -174,7 +174,7 @@ def main():
                 module.fail_json(msg='Could not find security group %s' %
                                  security_group)
 
-            rule = _find_matching_rule(module, secgroup):
+            rule = _find_matching_rule(module, secgroup)
             if not rule:
                 rule = cloud.create_security_group_rule(
                     secgroup['id'],

From f027e759765e9dd7717b54c308ad1d46410c2cff Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Thu, 18 Jun 2015 19:22:56 -0400
Subject: [PATCH 12/17] Compare ports as strings

Ports as returned from shade are ints. They are strings as they come
in to the module.
---
 cloud/openstack/os_security_group_rule.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index f50c97817e5..eea47c0c3e1 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -103,8 +103,16 @@ def _find_matching_rule(module, secgroup):
 
     for rule in secgroup['security_group_rules']:
         # No port, or -1, will be returned from shade as None
-        rule_port_range_min = rule['port_range_min'] or -1
-        rule_port_range_max = rule['port_range_max'] or -1
+        if rule['port_range_min'] is None:
+            rule_port_range_min = "-1"
+        else:
+            rule_port_range_min = str(rule['port_range_min'])
+
+        if rule['port_range_max'] is None:
+            rule_port_range_max = "-1"
+        else:
+            rule_port_range_max = str(rule['port_range_max'])
+
 
         if (protocol == rule['protocol']
                 and port_range_min == rule_port_range_min

From 2e8daa23309ada2bfba8415ea6ec5d764b565f05 Mon Sep 17 00:00:00 2001
From: dagnello <dagnello@hp.com>
Date: Fri, 19 Jun 2015 11:21:51 -0700
Subject: [PATCH 13/17] Resolving issues in rule comparison algorithm

Port range min/max values are at times represented as string and
compared to int equivalents.  This fix explicitly ensures all
port range values are ints for proper comparisons.
---
 cloud/openstack/os_security_group_rule.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index eea47c0c3e1..fc9283940c0 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -91,12 +91,11 @@ EXAMPLES = '''
 def _find_matching_rule(module, secgroup):
     """
     Find a rule in the group that matches the module parameters.
-
     :returns: The matching rule dict, or None if no matches.
     """
     protocol = module.params['protocol']
-    port_range_min = module.params['port_range_min']
-    port_range_max = module.params['port_range_max']
+    port_range_min = int(module.params['port_range_min'])
+    port_range_max = int(module.params['port_range_max'])
     remote_ip_prefix = module.params['remote_ip_prefix']
     ethertype = module.params['ethertype']
     direction = module.params['direction']
@@ -104,14 +103,14 @@ def _find_matching_rule(module, secgroup):
     for rule in secgroup['security_group_rules']:
         # No port, or -1, will be returned from shade as None
         if rule['port_range_min'] is None:
-            rule_port_range_min = "-1"
+            rule_port_range_min = -1
         else:
-            rule_port_range_min = str(rule['port_range_min'])
+            rule_port_range_min = int(rule['port_range_min'])
 
         if rule['port_range_max'] is None:
-            rule_port_range_max = "-1"
+            rule_port_range_max = -1
         else:
-            rule_port_range_max = str(rule['port_range_max'])
+            rule_port_range_max = int(rule['port_range_max'])
 
 
         if (protocol == rule['protocol']
@@ -195,7 +194,7 @@ def main():
                     ethertype=module.params['ethertype']
                 )
                 changed = True
-            module.exit_json(changed=changed, rule=rule, id=rule.id)
+            module.exit_json(changed=changed, rule=rule, id=rule['id'])
 
         if state == 'absent' and secgroup:
             rule = _find_matching_rule(module, secgroup)
@@ -212,4 +211,4 @@ def main():
 from ansible.module_utils.basic import *
 from ansible.module_utils.openstack import *
 
-main()
+main()
\ No newline at end of file

From 9f03302b68b4038fa664230dcbb66920325dbd1f Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Fri, 19 Jun 2015 17:17:46 -0400
Subject: [PATCH 14/17] Use int in the parameter list instead of casting

---
 cloud/openstack/os_security_group_rule.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index fc9283940c0..7d86408b379 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -94,8 +94,8 @@ def _find_matching_rule(module, secgroup):
     :returns: The matching rule dict, or None if no matches.
     """
     protocol = module.params['protocol']
-    port_range_min = int(module.params['port_range_min'])
-    port_range_max = int(module.params['port_range_max'])
+    port_range_min = module.params['port_range_min']
+    port_range_max = module.params['port_range_max']
     remote_ip_prefix = module.params['remote_ip_prefix']
     ethertype = module.params['ethertype']
     direction = module.params['direction']
@@ -142,8 +142,8 @@ def main():
         security_group   = dict(required=True),
         protocol         = dict(default='tcp',
                                 choices=['tcp', 'udp', 'icmp']),
-        port_range_min   = dict(required=True),
-        port_range_max   = dict(required=True),
+        port_range_min   = dict(required=True, type='int'),
+        port_range_max   = dict(required=True, type='int'),
         remote_ip_prefix = dict(required=False, default=None),
         # TODO(mordred): Make remote_group handle name and id
         remote_group     = dict(required=False, default=None),
@@ -211,4 +211,4 @@ def main():
 from ansible.module_utils.basic import *
 from ansible.module_utils.openstack import *
 
-main()
\ No newline at end of file
+main()

From 8664c884174736b803089c3d4a199461dff0af9e Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Tue, 30 Jun 2015 16:51:18 -0400
Subject: [PATCH 15/17] Change required parameters for rules module

The ports and protocol are no longer required (and now depends on
a new version of shade).
---
 cloud/openstack/os_security_group_rule.py | 55 ++++++++++++++++++++---
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 7d86408b379..2ec8e49b68d 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -18,8 +18,10 @@
 
 try:
     import shade
+    HAS_SHADE = True
 except ImportError:
-    print("failed=True msg='shade is required for this module'")
+    HAS_SHADE = False
+
 
 DOCUMENTATION = '''
 ---
@@ -87,6 +89,41 @@ EXAMPLES = '''
     remote_ip_prefix: 0.0.0.0/0
 '''
 
+RETURN = '''
+id:
+  description: Unique rule UUID.
+  type: string
+direction:
+  description: The direction in which the security group rule is applied.
+  type: string
+  sample: 'egress'
+ethertype:
+  description: One of IPv4 or IPv6.
+  type: string
+  sample: 'IPv4'
+port_range_min:
+  description: The minimum port number in the range that is matched by
+               the security group rule.
+  type: int
+  sample: 8000
+port_range_max:
+  description: The maximum port number in the range that is matched by
+               the security group rule.
+  type: int
+  sample: 8000
+protocol:
+  description: The protocol that is matched by the security group rule.
+  type: string
+  sample: 'tcp'
+remote_ip_prefix:
+  description: The remote IP prefix to be associated with this security group rule.
+  type: string
+  sample: '0.0.0.0/0'
+security_group_id:
+  description: The security group ID to associate with this security group rule.
+  type: string
+'''
+
 
 def _find_matching_rule(module, secgroup):
     """
@@ -140,10 +177,12 @@ def _system_state_change(module, secgroup):
 def main():
     argument_spec = openstack_full_argument_spec(
         security_group   = dict(required=True),
-        protocol         = dict(default='tcp',
-                                choices=['tcp', 'udp', 'icmp']),
-        port_range_min   = dict(required=True, type='int'),
-        port_range_max   = dict(required=True, type='int'),
+        # NOTE(Shrews): None is an acceptable protocol value for
+        # Neutron, but Nova will balk at this.
+        protocol         = dict(default=None,
+                                choices=[None, 'tcp', 'udp', 'icmp']),
+        port_range_min   = dict(required=False, type='int'),
+        port_range_max   = dict(required=False, type='int'),
         remote_ip_prefix = dict(required=False, default=None),
         # TODO(mordred): Make remote_group handle name and id
         remote_group     = dict(required=False, default=None),
@@ -165,6 +204,9 @@ def main():
                            supports_check_mode=True,
                            **module_kwargs)
 
+    if not HAS_SHADE:
+        module.fail_json(msg='shade is required for this module')
+
     state = module.params['state']
     security_group = module.params['security_group']
     changed = False
@@ -211,4 +253,5 @@ def main():
 from ansible.module_utils.basic import *
 from ansible.module_utils.openstack import *
 
-main()
+if __name__ == '__main__':
+    main()

From 6933407cd40bb655eaaa6847336421018a6b9b1e Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Mon, 6 Jul 2015 12:16:29 -0400
Subject: [PATCH 16/17] Correct port matching logic

Port matching logic did not take into account recent shade change
to equate (None, None) to (1, 65535) when Nova is the backend.

Also, this encapsulates the port matching logic into a single function
and heavily documents the logic.
---
 cloud/openstack/os_security_group_rule.py | 102 ++++++++++++++++++----
 1 file changed, 84 insertions(+), 18 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 2ec8e49b68d..7e0486d81db 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -76,7 +76,6 @@ options:
      default: present
 requirements: ["shade"]
 '''
-# TODO(mordred): add ethertype and direction
 
 EXAMPLES = '''
 # Create a security group rule
@@ -87,6 +86,38 @@ EXAMPLES = '''
     port_range_min: 80
     port_range_max: 80
     remote_ip_prefix: 0.0.0.0/0
+
+# Create a security group rule for ping
+- os_security_group_rule:
+    cloud: mordred
+    security_group: foo
+    protocol: icmp
+    remote_ip_prefix: 0.0.0.0/0
+
+# Another way to create the ping rule
+- os_security_group_rule:
+    cloud: mordred
+    security_group: foo
+    protocol: icmp
+    port_range_min: -1
+    port_range_max: -1
+    remote_ip_prefix: 0.0.0.0/0
+
+# Create a TCP rule covering all ports
+- os_security_group_rule:
+    cloud: mordred
+    security_group: foo
+    protocol: tcp
+    port_range_min: 1
+    port_range_max: 65535
+    remote_ip_prefix: 0.0.0.0/0
+
+# Another way to create the TCP rule above (defaults to all ports)
+- os_security_group_rule:
+    cloud: mordred
+    security_group: foo
+    protocol: tcp
+    remote_ip_prefix: 0.0.0.0/0
 '''
 
 RETURN = '''
@@ -125,37 +156,72 @@ security_group_id:
 '''
 
 
+def _ports_match(protocol, module_min, module_max, rule_min, rule_max):
+    """
+    Capture the complex port matching logic.
+
+    The port values coming in for the module might be -1 (for ICMP),
+    which will work only for Nova, but this is handled by shade. Likewise,
+    they might be None, which works for Neutron, but not Nova. This too is
+    handled by shade. Since shade will consistently return these port
+    values as None, we need to convert any -1 values input to the module
+    to None here for comparison.
+
+    For TCP and UDP protocols, None values for both min and max are
+    represented as the range 1-65535 for Nova, but remain None for
+    Neutron. Shade returns the full range when Nova is the backend (since
+    that is how Nova stores them), and None values for Neutron. If None
+    values are input to the module for both values, then we need to adjust
+    for comparison.
+    """
+
+    # Check if the user is supplying -1 for ICMP.
+    if protocol == 'icmp':
+        if module_min and int(module_min) == -1:
+            module_min = None
+        if module_max and int(module_max) == -1:
+            module_max = None
+
+    # Check if user is supplying None values for full TCP/UDP port range.
+    if protocol in ['tcp', 'udp'] and module_min is None and module_max is None:
+        if (rule_min and int(rule_min) == 1
+                and rule_max and int(rule_max) == 65535):
+            # (None, None) == (1, 65535)
+            return True
+
+    # Sanity check to make sure we don't have type comparison issues.
+    if module_min:
+        module_min = int(module_min)
+    if module_max:
+        module_max = int(module_max)
+    if rule_min:
+        rule_min = int(rule_min)
+    if rule_max:
+        rule_max = int(rule_max)
+
+    return module_min == rule_min and module_max == rule_max
+
+
 def _find_matching_rule(module, secgroup):
     """
     Find a rule in the group that matches the module parameters.
     :returns: The matching rule dict, or None if no matches.
     """
     protocol = module.params['protocol']
-    port_range_min = module.params['port_range_min']
-    port_range_max = module.params['port_range_max']
     remote_ip_prefix = module.params['remote_ip_prefix']
     ethertype = module.params['ethertype']
     direction = module.params['direction']
 
     for rule in secgroup['security_group_rules']:
-        # No port, or -1, will be returned from shade as None
-        if rule['port_range_min'] is None:
-            rule_port_range_min = -1
-        else:
-            rule_port_range_min = int(rule['port_range_min'])
-
-        if rule['port_range_max'] is None:
-            rule_port_range_max = -1
-        else:
-            rule_port_range_max = int(rule['port_range_max'])
-
-
         if (protocol == rule['protocol']
-                and port_range_min == rule_port_range_min
-                and port_range_max == rule_port_range_max
                 and remote_ip_prefix == rule['remote_ip_prefix']
                 and ethertype == rule['ethertype']
-                and direction == rule['direction']):
+                and direction == rule['direction']
+                and _ports_match(protocol,
+                                 module.params['port_range_min'],
+                                 module.params['port_range_max'],
+                                 rule['port_range_min'],
+                                 rule['port_range_max'])):
             return rule
     return None
 

From dd9c29286154d7643c0392d576e44ea4421ada3c Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Mon, 6 Jul 2015 18:52:11 -0400
Subject: [PATCH 17/17] Update docstring to show port ranges as optional

---
 cloud/openstack/os_security_group_rule.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/cloud/openstack/os_security_group_rule.py b/cloud/openstack/os_security_group_rule.py
index 7e0486d81db..91059aca015 100644
--- a/cloud/openstack/os_security_group_rule.py
+++ b/cloud/openstack/os_security_group_rule.py
@@ -39,16 +39,18 @@ options:
    protocol:
       description:
         - IP protocol
-      choices: ['tcp', 'udp', 'icmp']
-      default: tcp
+      choices: ['tcp', 'udp', 'icmp', None]
+      default: None
    port_range_min:
       description:
         - Starting port
-      required: true
+      required: false
+      default: None
    port_range_max:
       description:
         - Ending port
-      required: true
+      required: false
+      default: None
    remote_ip_prefix:
       description:
         - Source IP address(es) in CIDR notation (exclusive with remote_group)