From 13c40c70dbbb6a4a3b91ef0e6a8701f4ce30cb07 Mon Sep 17 00:00:00 2001 From: Jill R <4121322+jillr@users.noreply.github.com> Date: Tue, 26 Nov 2019 09:29:03 -0700 Subject: [PATCH] Allow updating of ec2_group rules with EC2 classic ELB targets (#62374) * Allow updating of ec2_group rules with EC2 classic ELB targets Fix regression introduced in #45296 with EC2 Classic SGs Fixes: #57247 Also add (unsupported) ec2 classic test suite with test case for this scenario * move ec2 classic tests to conditional within ec2_group target * clean up ec2_classic tests * ec2_classic account can't run most ec2_group tests --- .../fragments/62374-ec2-clb-group-rules.yml | 2 + lib/ansible/modules/cloud/amazon/ec2_group.py | 6 +- .../targets/ec2_group/tasks/ec2_classic.yml | 88 +++++++++++++++++++ .../targets/ec2_group/tasks/main.yml | 64 ++++++++++---- 4 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/62374-ec2-clb-group-rules.yml create mode 100644 test/integration/targets/ec2_group/tasks/ec2_classic.yml diff --git a/changelogs/fragments/62374-ec2-clb-group-rules.yml b/changelogs/fragments/62374-ec2-clb-group-rules.yml new file mode 100644 index 00000000000..447b856cd8f --- /dev/null +++ b/changelogs/fragments/62374-ec2-clb-group-rules.yml @@ -0,0 +1,2 @@ +bugfixes: + - ec2_group - Fix regression with revoking security groups in EC2 Classic Load Balancers. diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index 0669c3fdcc1..4e05e795d97 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -409,7 +409,11 @@ def rule_from_group_permission(perm): ) if 'UserIdGroupPairs' in perm and perm['UserIdGroupPairs']: for pair in perm['UserIdGroupPairs']: - target = pair['GroupId'] + target = ( + pair.get('UserId', None), + pair.get('GroupId', None), + pair.get('GroupName', None), + ) if pair.get('UserId', '').startswith('amazon-'): # amazon-elb and amazon-prefix rules don't need # group-id specified, so remove it when querying diff --git a/test/integration/targets/ec2_group/tasks/ec2_classic.yml b/test/integration/targets/ec2_group/tasks/ec2_classic.yml new file mode 100644 index 00000000000..9019af95d4a --- /dev/null +++ b/test/integration/targets/ec2_group/tasks/ec2_classic.yml @@ -0,0 +1,88 @@ +- module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token }}" + region: "{{ aws_region }}" + block: + - name: Get available AZs + aws_az_facts: + aws_access_key: "{{ aws_connection_info['aws_access_key'] }}" + aws_secret_key: "{{ aws_connection_info['aws_secret_key'] }}" + filters: + region-name: "{{ aws_connection_info['region'] }}" + register: az_facts + + - name: Create a classic ELB with classic networking + ec2_elb_lb: + name: "{{ resource_prefix }}-elb" + state: present + zones: + - "{{ az_facts['availability_zones'][0]['zone_name'] }}" + - "{{ az_facts['availability_zones'][1]['zone_name'] }}" + listeners: + - protocol: http # options are http, https, ssl, tcp + load_balancer_port: 80 + instance_port: 80 + proxy_protocol: True + register: classic_elb + + - name: Assert the elb was created + assert: + that: + - classic_elb.changed + + - name: Create a security group with a classic elb-sg rule + ec2_group: + name: "{{ resource_prefix }}-sg-a" + description: "EC2 classic test security group" + rules: + - proto: tcp + ports: 80 + group_id: amazon-elb/amazon-elb-sg + state: present + register: classic_sg + + - name: Assert the SG was created + assert: + that: + - classic_sg.changed + - "{{ classic_sg.ip_permissions | length }} == 1" + + - set_fact: + elb_sg_id: "{{ classic_sg.ip_permissions[0].user_id_group_pairs[0].user_id }}/{{ classic_sg.ip_permissions[0].user_id_group_pairs[0].group_id }}/{{ classic_sg.ip_permissions[0].user_id_group_pairs[0].group_name }}" + + - name: Update the security group + ec2_group: + name: "{{ resource_prefix }}-sg-a" + description: "EC2 classic test security group" + rules: + - proto: tcp + ports: 8080 + group_id: "{{ elb_sg_id }}" + - proto: tcp + ports: + - 80 + cidr_ip: 0.0.0.0/0 + state: present + register: updated_classic_sg + + + - name: Assert the SG was updated + assert: + that: + - updated_classic_sg.changed + - "{{ updated_classic_sg.ip_permissions | length }} == 2" + - "{{ classic_sg.ip_permissions[0]}} not in {{ updated_classic_sg.ip_permissions }}" + + # =========================================== + always: + - name: Terminate classic ELB + ec2_elb_lb: + name: "{{ resource_prefix }}-classic-elb" + state: absent + + - name: Delete security group + ec2_group: + name: "{{ resource_prefix }}-sg-a" + state: absent diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index e1b86683a9a..9b558656cd1 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -9,35 +9,60 @@ # - include: ../../setup_ec2/tasks/common.yml module_name: ec2_group - include: ./credential_tests.yml -- module_defaults: - group/aws: +# ============================================================ +# EC2 Classic tests can only be run on a pre-2013 AWS account with supported-platforms=EC2 +# Ansible CI does NOT have classic EC2 support; these tests are provided as-is for the +# community and can be run if you have access to a classic account. To check if your account +# has support for EC2 Classic you can use the `aws_account_attribute` plugin. + +- name: determine if this is an EC2 Classic account + set_fact: + has_ec2_classic: "{{ lookup('aws_account_attribute', + attribute='has-ec2-classic', + region=aws_region, + aws_access_key=aws_access_key, + aws_secret_key=aws_secret_key, + aws_security_token=security_token, + wantlist=True) }}" +# ============================================================ +- +- name: set up aws connection info + set_fact: + aws_connection_info: &aws_connection_info aws_access_key: "{{ aws_access_key }}" aws_secret_key: "{{ aws_secret_key }}" security_token: "{{ security_token }}" region: "{{ aws_region }}" + no_log: yes + +# ============================================================ +- name: Run EC2 Classic accounts if account type is EC2 + include: ./ec2_classic.yml + when: has_ec2_classic + +# ============================================================ +# Other tests depend on attribute='default-vpc', ie no vpc_id is set. This is +# incompatible with EC2 classic accounts, so these tests can only be run in a +# VPC-type account. See "Q. I really want a default VPC for my existing EC2 +# account. Is that possible?" in https://aws.amazon.com/vpc/faqs/#Default_VPCs +- name: Run all other tests if account type is VPC + module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token }}" + region: "{{ aws_region }}" block: - # ============================================================ - - name: set up aws connection info - set_fact: - aws_connection_info: &aws_connection_info - aws_access_key: "{{ aws_access_key }}" - aws_secret_key: "{{ aws_secret_key }}" - security_token: "{{ security_token }}" - region: "{{ aws_region }}" - no_log: yes - - # ============================================================ - name: determine if there is a default VPC set_fact: defaultvpc: "{{ lookup('aws_account_attribute', - attribute='default-vpc', - region=aws_region, - aws_access_key=aws_access_key, - aws_secret_key=aws_secret_key, - aws_security_token=security_token) }}" + attribute='default-vpc', + region=aws_region, + aws_access_key=aws_access_key, + aws_secret_key=aws_secret_key, + aws_security_token=security_token) }}" register: default_vpc - # ============================================================ - name: create a VPC ec2_vpc_net: name: "{{ resource_prefix }}-vpc" @@ -1470,6 +1495,7 @@ that: - 'result.changed' - 'not result.group_id' + when: not has_ec2_classic always: # ============================================================