From e34f25956659cb4ae2ce51455e690292c8a6b015 Mon Sep 17 00:00:00 2001 From: Jakob Meng Date: Thu, 12 Jan 2023 10:32:51 +0100 Subject: [PATCH] Added support for 'any' protocol in security group rules Story: 2007849 Task: 40143 Story: 2008064 Task: 40747 Change-Id: I1e2f6dde4c7a17b6c5c9fcffad1b5748a1d44a15 --- ci/roles/security_group_rule/tasks/main.yml | 163 ++++++++++++++++++++ plugins/modules/security_group.py | 28 +++- plugins/modules/security_group_rule.py | 25 ++- 3 files changed, 205 insertions(+), 11 deletions(-) diff --git a/ci/roles/security_group_rule/tasks/main.yml b/ci/roles/security_group_rule/tasks/main.yml index 0ad4c148..25cf9a4e 100644 --- a/ci/roles/security_group_rule/tasks/main.yml +++ b/ci/roles/security_group_rule/tasks/main.yml @@ -296,3 +296,166 @@ cloud: "{{ cloud }}" name: ansible_security_group state: absent + +- name: Create security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: [] + state: present + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group without rules + assert: + that: + - security_group_rules.security_group_rules | length == 0 + +- name: Create 'any' rule + openstack.cloud.security_group_rule: + cloud: "{{ cloud }}" + security_group: ansible_security_group + state: present + protocol: any + remote_ip_prefix: 0.0.0.0/0 + register: security_group_rule + +- name: Assert 'any' rule + assert: + that: + - security_group_rule is changed + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group with a single rule + assert: + that: + - security_group_rules.security_group_rules | length == 1 + +- name: Create 'any' rule with port range [-1, -1] + openstack.cloud.security_group_rule: + cloud: "{{ cloud }}" + security_group: ansible_security_group + state: present + protocol: any + port_range_min: -1 + port_range_max: -1 + remote_ip_prefix: 0.0.0.0/0 + register: security_group_rule + +- name: Assert 'any' rule with port range [-1, -1] + assert: + that: + - security_group_rule is not changed + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group with a single rule + assert: + that: + - security_group_rules.security_group_rules | length == 1 + +- name: Delete security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: absent + +- name: Create security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: [] + state: present + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group without rules + assert: + that: + - security_group_rules.security_group_rules | length == 0 + +- name: Create rule with remote ip prefix having no cidr slash + openstack.cloud.security_group_rule: + cloud: "{{ cloud }}" + security_group: ansible_security_group + state: present + protocol: any + remote_ip_prefix: 5.6.7.8 + register: security_group_rule + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group with a rule with remote ip prefix having no cidr slash + assert: + that: + - security_group_rules.security_group_rules | length == 1 + - security_group_rules.security_group_rules.0.remote_ip_prefix == '5.6.7.8/32' + +- name: Create rule with remote ip prefix having no cidr slash again + openstack.cloud.security_group_rule: + cloud: "{{ cloud }}" + security_group: ansible_security_group + state: present + protocol: any + remote_ip_prefix: 5.6.7.8 + register: security_group_rule + ignore_errors: true + +- name: Assert rule with remote ip prefix having no cidr slash + assert: + that: + - security_group_rule is failed + - "'Security group rule already exists.' in security_group_rule.msg" + +- name: Create rule with remote ip prefix having a cidr slash again + openstack.cloud.security_group_rule: + cloud: "{{ cloud }}" + security_group: ansible_security_group + state: present + protocol: any + remote_ip_prefix: 5.6.7.8/32 + register: security_group_rule + +- name: Assert rule with remote ip prefix having a cidr slash + assert: + that: + - security_group_rule is not changed + +- name: List all available rules of a specific security group + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert security group with a rule with remote ip prefix having a cidr slash again + assert: + that: + - security_group_rules.security_group_rules | length == 1 + - security_group_rules.security_group_rules.0.remote_ip_prefix == '5.6.7.8/32' + +- name: Delete security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: absent diff --git a/plugins/modules/security_group.py b/plugins/modules/security_group.py index ebdc2f92..74da83d1 100644 --- a/plugins/modules/security_group.py +++ b/plugins/modules/security_group.py @@ -102,6 +102,9 @@ options: remote_ip_prefix: description: - Source IP address(es) in CIDR notation. + - When a netmask such as C(/32) is missing from I(remote_ip_prefix), + then this module will fail on updates with OpenStack error message + C(Security group rule already exists.). - Mutually exclusive with I(remote_group). type: str state: @@ -349,12 +352,6 @@ class SecurityGroupModule(OpenStackModule): and security_group_rule['ethertype'] != prototype['ether_type']: return False - if 'protocol' in prototype \ - and prototype['protocol'] in ['any', '0']: - if security_group_rule['protocol'] is not None: - return False - skip_keys.append('protocol') - if 'protocol' in prototype \ and prototype['protocol'] in ['tcp', 'udp']: # Check if the user is supplying -1, 1 to 65535 or None values @@ -445,9 +442,23 @@ class SecurityGroupModule(OpenStackModule): def _generate_security_group_rule(params): prototype = dict( (k, params[k]) - for k in ['direction', 'protocol', 'remote_ip_prefix'] + for k in ['direction', 'remote_ip_prefix'] if params[k] is not None) + # When remote_ip_prefix is missing a netmask, then Neutron will add + # a netmask using Python library netaddr [0] and its IPNetwork + # class [1]. We do not want to introduce additional Python + # dependencies to our code base and neither want to replicate + # netaddr's parse_ip_network code here. So we do not handle + # remote_ip_prefix without a netmask and instead let Neutron handle + # it. + # [0] https://opendev.org/openstack/neutron/src/commit/\ + # 43d94640568828f5e98bbb1e9df985ec3f1bb2d2/neutron/db/securitygroups_db.py#L775 + # [1] https://github.com/netaddr/netaddr/blob/\ + # b1d8f016abee00c8a93e35b928acdc22797c800a/netaddr/ip/__init__.py#L841 + # [2] https://github.com/netaddr/netaddr/blob/\ + # b1d8f016abee00c8a93e35b928acdc22797c800a/netaddr/ip/__init__.py#L773 + prototype['project_id'] = security_group.project_id prototype['security_group_id'] = security_group.id @@ -469,6 +480,9 @@ class SecurityGroupModule(OpenStackModule): prototype['ether_type'] = ether_type protocol = params['protocol'] + if protocol is not None and protocol not in ['any', '0']: + prototype['protocol'] = protocol + port_range_max = params['port_range_max'] port_range_min = params['port_range_min'] diff --git a/plugins/modules/security_group_rule.py b/plugins/modules/security_group_rule.py index f897192c..51b573b3 100644 --- a/plugins/modules/security_group_rule.py +++ b/plugins/modules/security_group_rule.py @@ -82,6 +82,9 @@ options: remote_ip_prefix: description: - Source IP address(es) in CIDR notation. + - When a netmask such as C(/32) is missing from I(remote_ip_prefix), then + this module will fail on updates with OpenStack error message + C(Security group rule already exists.). - Mutually exclusive with I(remote_group). type: str security_group: @@ -308,9 +311,23 @@ class SecurityGroupRuleModule(OpenStackModule): def _define_prototype(self): filters = {} prototype = dict((k, self.params[k]) - for k in ['direction', 'protocol', 'remote_ip_prefix'] + for k in ['direction', 'remote_ip_prefix'] if self.params[k] is not None) + # When remote_ip_prefix is missing a netmask, then Neutron will add + # a netmask using Python library netaddr [0] and its IPNetwork + # class [1]. We do not want to introduce additional Python + # dependencies to our code base and neither want to replicate + # netaddr's parse_ip_network code here. So we do not handle + # remote_ip_prefix without a netmask and instead let Neutron handle + # it. + # [0] https://opendev.org/openstack/neutron/src/commit/\ + # 43d94640568828f5e98bbb1e9df985ec3f1bb2d2/neutron/db/securitygroups_db.py#L775 + # [1] https://github.com/netaddr/netaddr/blob/\ + # b1d8f016abee00c8a93e35b928acdc22797c800a/netaddr/ip/__init__.py#L841 + # [2] https://github.com/netaddr/netaddr/blob/\ + # b1d8f016abee00c8a93e35b928acdc22797c800a/netaddr/ip/__init__.py#L773 + project_name_or_id = self.params['project'] if project_name_or_id is not None: project = self.conn.identity.find_project(project_name_or_id, @@ -335,6 +352,9 @@ class SecurityGroupRuleModule(OpenStackModule): prototype['ether_type'] = ether_type protocol = self.params['protocol'] + if protocol is not None and protocol not in ['any', '0']: + prototype['protocol'] = protocol + port_range_max = self.params['port_range_max'] port_range_min = self.params['port_range_min'] @@ -384,9 +404,6 @@ class SecurityGroupRuleModule(OpenStackModule): if 'ether_type' in prototype: prototype['ethertype'] = prototype.pop('ether_type') - if 'protocol' in prototype and prototype['protocol'] in ['any', '0']: - prototype.pop('protocol') - if 'protocol' in prototype and prototype['protocol'] in ['tcp', 'udp']: # Check if the user is supplying -1, 1 to 65535 or None values # for full TPC or UDP port range.