From 124e174d27cd0befd50f3cf2bc6f89f35c4151ef Mon Sep 17 00:00:00 2001 From: Balazs Pokoradi <bpokorad@redhat.com> Date: Fri, 6 Jan 2023 11:05:41 +0100 Subject: [PATCH] Added parameter for managing rules in security_group module Co-Authored-By: Jakob Meng <code@jakobmeng.de> Change-Id: I571955e8f4023293cce325604de5f1689b855416 --- ci/roles/security_group/tasks/main.yml | 2 + ci/roles/security_group/tasks/rules.yml | 350 ++++++++++++++++++++++++ plugins/modules/security_group.py | 281 ++++++++++++++++++- plugins/modules/security_group_rule.py | 8 + 4 files changed, 640 insertions(+), 1 deletion(-) create mode 100644 ci/roles/security_group/tasks/rules.yml diff --git a/ci/roles/security_group/tasks/main.yml b/ci/roles/security_group/tasks/main.yml index 0aec2590..5ef8a65f 100644 --- a/ci/roles/security_group/tasks/main.yml +++ b/ci/roles/security_group/tasks/main.yml @@ -71,3 +71,5 @@ cloud: "{{ cloud }}" name: ansible_security_group state: absent + +- include_tasks: rules.yml diff --git a/ci/roles/security_group/tasks/rules.yml b/ci/roles/security_group/tasks/rules.yml new file mode 100644 index 00000000..dbcc7393 --- /dev/null +++ b/ci/roles/security_group/tasks/rules.yml @@ -0,0 +1,350 @@ +--- +- name: Create security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: present + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is changed + +- name: Create security group again + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: present + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is not changed + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 0 + +- name: Delete security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: absent + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is changed + +- name: Delete security group again + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + state: absent + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is not changed + +- name: Create security group including security group rules + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: + - ether_type: IPv6 + direction: egress + - ether_type: IPv4 + direction: egress + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is changed + +- name: Create security group including security group rules again + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: + - ether_type: IPv6 + direction: egress + - ether_type: IPv4 + direction: egress + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is not changed + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 2 + - security_group_rules.security_group_rules | map(attribute='ether_type') | list | sort == ['IPv4', 'IPv6'] + +- name: Update security group with new set of security group rules, dropping egress rules for IPv4 and IPv6 + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: + - protocol: udp + ether_type: IPv6 + direction: ingress + port_range_min: 547 + port_range_max: 547 + - protocol: tcp + ether_type: IPv4 + direction: ingress + port_range_min: 22 + port_range_max: 22 + remote_ip_prefix: 1.2.3.40/32 + - protocol: tcp + ether_type: IPv4 + direction: ingress + port_range_min: 22 + port_range_max: 22 + remote_ip_prefix: 1.2.3.41/32 + - protocol: tcp + ether_type: IPv4 + direction: ingress + port_range_min: 22 + port_range_max: 22 + remote_ip_prefix: 1.2.3.42/32 + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 4 + - security_group_rules.security_group_rules | map(attribute='direction') | list | unique == ['ingress'] + +- name: Remove all security group rules from security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: [] + register: security_group + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 0 + +- 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: + - ether_type: IPv6 + direction: egress + - ether_type: IPv4 + direction: egress + state: present + register: security_group + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group.security_group.security_group_rules | length == 2 + +- name: Define set of additional security group rules + set_fact: + security_group_rules: + - protocol: udp + ether_type: IPv6 + direction: ingress + port_range_min: 547 + port_range_max: 547 + - protocol: tcp + ether_type: IPv4 + direction: ingress + port_range_min: 22 + port_range_max: 22 + remote_ip_prefix: 1.2.3.40/32 + +- name: Prepare existing security group rules for appending + loop: '{{ security_group.security_group.security_group_rules | default([]) }}' + set_fact: + security_group_rule: + description: '{{ item.description or omit }}' + direction: '{{ item.direction or omit }}' + ether_type: '{{ item.ethertype or omit }}' + port_range_max: '{{ item.port_range_max or omit }}' + port_range_min: '{{ item.port_range_min or omit }}' + protocol: '{{ item.protocol or omit }}' + remote_group: '{{ item.remote_group_id or omit }}' + remote_ip_prefix: '{{ item.remote_ip_prefix or omit }}' + register: previous_security_group_rules + +- name: Flatten existing security group rules + set_fact: + previous_security_group_rules: "{{ + previous_security_group_rules.results + | map(attribute='ansible_facts.security_group_rule') + | flatten(levels=1) + }}" + +- name: Append security group rules to security group + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: '{{ previous_security_group_rules + security_group_rules }}' + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is changed + +- name: Append security group rules to security group again + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: '{{ previous_security_group_rules + security_group_rules }}' + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is not changed + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + # 2 ingress rules and egress rules for IPv4 and IPv6 + - security_group_rules.security_group_rules | length == 4 + +- 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 + state: present + register: security_group + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 0 + +- name: Define dense representation of security group rules with multiple remote ip prefixes per rule + set_fact: + security_group_rules: + - protocol: udp + ether_type: IPv6 + direction: ingress + port_range_min: 547 + port_range_max: 547 + - protocol: tcp + ether_type: IPv4 + direction: ingress + port_range_min: 22 + port_range_max: 22 + remote_ip_prefixes: + - 1.2.3.40/32 + - 1.2.3.41/32 + - 1.2.3.42/32 + +- name: Convert dense representation into default representation of security group rules + loop: '{{ security_group_rules }}' + set_fact: + security_group_rules: >- + {{ [item] + if 'remote_ip_prefixes' not in item + else item.remote_ip_prefixes + | map('community.general.dict_kv', 'remote_ip_prefix') + | map('combine', item | dict2items | rejectattr('key', 'eq', 'remote_ip_prefixes') | list | items2dict) + | list + }} + register: security_group_rules + +- name: Flatten security group rules + set_fact: + security_group_rules: "{{ + security_group_rules.results + | map(attribute='ansible_facts.security_group_rules') + | flatten(levels=1) | list + }}" + +- name: Update security group with set of security group rules + openstack.cloud.security_group: + cloud: "{{ cloud }}" + name: ansible_security_group + security_group_rules: '{{ security_group_rules }}' + register: security_group + +- name: Assert return values of security_group module + assert: + that: + - security_group is changed + +- name: Fetch security group rules + openstack.cloud.security_group_rule_info: + cloud: "{{ cloud }}" + security_group: ansible_security_group + register: security_group_rules + +- name: Assert return values of security_group_rule_info module + assert: + that: + - security_group_rules.security_group_rules | length == 4 + +- 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 19be936f..ebdc2f92 100644 --- a/plugins/modules/security_group.py +++ b/plugins/modules/security_group.py @@ -27,6 +27,83 @@ options: description: - Unique name or ID of the project. type: str + security_group_rules: + description: + - List of security group rules. + - When I(security_group_rules) is not defined, Neutron might create this + security group with a default set of rules. + - Security group rules which are listed in I(security_group_rules) + but not defined in this security group will be created. + - Existing security group rules which are not listed in + I(security_group_rules) will be deleted. + - When updating a security group, one has to explicitly list rules from + Neutron's defaults in I(security_group_rules) if those rules should be + kept. Rules which are not listed in I(security_group_rules) will be + deleted. + type: list + elements: dict + suboptions: + description: + description: + - Description of the security group rule. + type: str + direction: + description: + - The direction in which the security group rule is applied. + - Not all providers support C(egress). + choices: ['egress', 'ingress'] + default: ingress + type: str + ether_type: + 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 + type: str + port_range_max: + description: + - The maximum port number in the range that is matched by the + security group rule. + - If the protocol is TCP, UDP, DCCP, SCTP or UDP-Lite this value must + be greater than or equal to the I(port_range_min) attribute value. + - If the protocol is ICMP, this value must be an ICMP code. + type: int + port_range_min: + description: + - The minimum port number in the range that is matched by the + security group rule. + - If the protocol is TCP, UDP, DCCP, SCTP or UDP-Lite this value must + be less than or equal to the port_range_max attribute value. + - If the protocol is ICMP, this value must be an ICMP type. + type: int + protocol: + description: + - The IP protocol can be represented by a string, an integer, or + null. + - Valid string or integer values are C(any) or C(0), C(ah) or C(51), + C(dccp) or C(33), C(egp) or C(8), C(esp) or C(50), C(gre) or C(47), + C(icmp) or C(1), C(icmpv6) or C(58), C(igmp) or C(2), C(ipip) or + C(4), C(ipv6-encap) or C(41), C(ipv6-frag) or C(44), C(ipv6-icmp) + or C(58), C(ipv6-nonxt) or C(59), C(ipv6-opts) or C(60), + C(ipv6-route) or C(43), C(ospf) or C(89), C(pgm) or C(113), C(rsvp) + or C(46), C(sctp) or C(132), C(tcp) or C(6), C(udp) or C(17), + C(udplite) or C(136), C(vrrp) or C(112). + - Additionally, any integer value between C([0-255]) is also valid. + - The string any (or integer 0) means all IP protocols. + - See the constants in neutron_lib.constants for the most up-to-date + list of supported strings. + type: str + remote_group: + description: + - Name or ID of the security group to link. + - Mutually exclusive with I(remote_ip_prefix). + type: str + remote_ip_prefix: + description: + - Source IP address(es) in CIDR notation. + - Mutually exclusive with I(remote_group). + type: str state: description: - Should the resource be present or absent. @@ -137,17 +214,51 @@ EXAMPLES = r''' state: present name: foo project: myproj + +- name: Create (or update) a security group with security group rules + openstack.cloud.security_group: + cloud: mordred + state: present + name: foo + security_group_rules: + - ether_type: IPv6 + direction: egress + - ether_type: IPv4 + direction: egress + +- name: Create (or update) security group without security group rules + openstack.cloud.security_group: + cloud: mordred + state: present + name: foo + security_group_rules: [] ''' from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule class SecurityGroupModule(OpenStackModule): + # NOTE: Keep handling of security group rules synchronized with + # security_group_rule.py! argument_spec = dict( description=dict(), name=dict(required=True), project=dict(), + security_group_rules=dict( + type="list", elements="dict", + options=dict( + description=dict(), + direction=dict(default="ingress", + choices=["egress", "ingress"]), + ether_type=dict(default="IPv4", choices=["IPv4", "IPv6"]), + port_range_max=dict(type="int"), + port_range_min=dict(type="int"), + protocol=dict(), + remote_group=dict(), + remote_ip_prefix=dict(), + ), + ), state=dict(default='present', choices=['absent', 'present']), ) @@ -190,6 +301,11 @@ class SecurityGroupModule(OpenStackModule): self.exit_json(changed=False) def _build_update(self, security_group): + return { + **self._build_update_security_group(security_group), + **self._build_update_security_group_rules(security_group)} + + def _build_update_security_group(self, security_group): update = {} # module options name and project are used to find security group @@ -213,6 +329,80 @@ class SecurityGroupModule(OpenStackModule): return update + def _build_update_security_group_rules(self, security_group): + + def find_security_group_rule_match(prototype, security_group_rules): + matches = [r for r in security_group_rules + if is_security_group_rule_match(prototype, r)] + if len(matches) > 1: + self.fail_json(msg='Found more a single matching security' + ' group rule which match the given' + ' parameters.') + elif len(matches) == 1: + return matches[0] + else: # len(matches) == 0 + return None + + def is_security_group_rule_match(prototype, security_group_rule): + skip_keys = ['ether_type'] + if 'ether_type' in prototype \ + 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 + # for full TPC or UDP port range. + # (None, None) == (1, 65535) == (-1, -1) + if 'port_range_max' in prototype \ + and prototype['port_range_max'] in [-1, 65535]: + if security_group_rule['port_range_max'] is not None: + return False + skip_keys.append('port_range_max') + if 'port_range_min' in prototype \ + and prototype['port_range_min'] in [-1, 1]: + if security_group_rule['port_range_min'] is not None: + return False + skip_keys.append('port_range_min') + + if all(security_group_rule[k] == prototype[k] + for k in (set(prototype.keys()) - set(skip_keys))): + return security_group_rule + else: + return None + + update = {} + keep_security_group_rules = {} + create_security_group_rules = [] + delete_security_group_rules = [] + + for prototype in self._generate_security_group_rules(security_group): + match = find_security_group_rule_match( + prototype, security_group.security_group_rules) + if match: + keep_security_group_rules[match['id']] = match + else: + create_security_group_rules.append(prototype) + + for security_group_rule in security_group.security_group_rules: + if (security_group_rule['id'] + not in keep_security_group_rules.keys()): + delete_security_group_rules.append(security_group_rule) + + if create_security_group_rules: + update['create_security_group_rules'] = create_security_group_rules + + if delete_security_group_rules: + update['delete_security_group_rules'] = delete_security_group_rules + + return update + def _create(self): kwargs = dict((k, self.params[k]) for k in ['description', 'name'] @@ -224,7 +414,14 @@ class SecurityGroupModule(OpenStackModule): name_or_id=project_name_or_id, ignore_missing=False) kwargs['project_id'] = project.id - return self.conn.network.create_security_group(**kwargs) + security_group = self.conn.network.create_security_group(**kwargs) + + update = self._build_update_security_group_rules(security_group) + if update: + security_group = self._update_security_group_rules(security_group, + update) + + return security_group def _delete(self, security_group): self.conn.network.delete_security_group(security_group.id) @@ -240,7 +437,71 @@ class SecurityGroupModule(OpenStackModule): return self.conn.network.find_security_group(**kwargs) + def _generate_security_group_rules(self, security_group): + security_group_cache = {} + security_group_cache[security_group.name] = security_group + security_group_cache[security_group.id] = security_group + + def _generate_security_group_rule(params): + prototype = dict( + (k, params[k]) + for k in ['direction', 'protocol', 'remote_ip_prefix'] + if params[k] is not None) + + prototype['project_id'] = security_group.project_id + prototype['security_group_id'] = security_group.id + + remote_group_name_or_id = params['remote_group'] + if remote_group_name_or_id is not None: + if remote_group_name_or_id in security_group_cache: + remote_group = \ + security_group_cache[remote_group_name_or_id] + else: + remote_group = self.conn.network.find_security_group( + remote_group_name_or_id, ignore_missing=False) + security_group_cache[remote_group_name_or_id] = \ + remote_group + + prototype['remote_group_id'] = remote_group.id + + ether_type = params['ether_type'] + if ether_type is not None: + prototype['ether_type'] = ether_type + + protocol = params['protocol'] + port_range_max = params['port_range_max'] + port_range_min = params['port_range_min'] + + if protocol in ['icmp', 'ipv6-icmp']: + # Check if the user is supplying -1 for ICMP. + if port_range_max is not None and int(port_range_max) != -1: + prototype['port_range_max'] = int(port_range_max) + if port_range_min is not None and int(port_range_min) != -1: + prototype['port_range_min'] = int(port_range_min) + elif protocol in ['tcp', 'udp']: + if port_range_max is not None and int(port_range_max) != -1: + prototype['port_range_max'] = int(port_range_max) + if port_range_min is not None and int(port_range_min) != -1: + prototype['port_range_min'] = int(port_range_min) + elif protocol in ['any', '0']: + # Rules with 'any' protocol do not match ports + pass + else: + if port_range_max is not None: + prototype['port_range_max'] = int(port_range_max) + if port_range_min is not None: + prototype['port_range_min'] = int(port_range_min) + + return prototype + + return [_generate_security_group_rule(r) + for r in (self.params['security_group_rules'] or [])] + def _update(self, security_group, update): + security_group = self._update_security_group(security_group, update) + return self._update_security_group_rules(security_group, update) + + def _update_security_group(self, security_group, update): attributes = update.get('attributes') if attributes: security_group = self.conn.network.update_security_group( @@ -248,6 +509,24 @@ class SecurityGroupModule(OpenStackModule): return security_group + def _update_security_group_rules(self, security_group, update): + create_security_group_rules = update.get('create_security_group_rules') + if create_security_group_rules: + self.conn.network.\ + create_security_group_rules(create_security_group_rules) + + delete_security_group_rules = update.get('delete_security_group_rules') + if delete_security_group_rules: + for security_group_rule in delete_security_group_rules: + self.conn.network.\ + delete_security_group_rule(security_group_rule['id']) + + if create_security_group_rules or delete_security_group_rules: + # Update security group with created and deleted rules + return self.conn.network.get_security_group(security_group.id) + else: + return security_group + def _will_change(self, state, security_group): if state == 'present' and not security_group: return True diff --git a/plugins/modules/security_group_rule.py b/plugins/modules/security_group_rule.py index d302a30a..f897192c 100644 --- a/plugins/modules/security_group_rule.py +++ b/plugins/modules/security_group_rule.py @@ -13,6 +13,11 @@ author: OpenStack Ansible SIG description: - Add or remove security group rule to/from OpenStack network (Neutron) service. + - Use I(security_group_rules) in M(openstack.cloud.security_group) to define + a set of security group rules. It will be much faster than using this + module when creating or removing several security group rules because the + latter will do individual calls to OpenStack network (Neutron) API for each + security group rule. options: description: description: @@ -243,6 +248,9 @@ from ansible_collections.openstack.cloud.plugins.module_utils.openstack import ( class SecurityGroupRuleModule(OpenStackModule): + # NOTE: Keep handling of security group rules synchronized with + # security_group.py! + argument_spec = dict( description=dict(), direction=dict(default='ingress', choices=['egress', 'ingress']),