From de3f9b49f2af94f1f3dd8160deb9399c7528dcbc Mon Sep 17 00:00:00 2001 From: Andrea Tartaglia Date: Wed, 24 Apr 2019 14:45:31 +0100 Subject: [PATCH 01/10] Added SG handling for existing instances + some cleanup --- plugins/modules/ec2_instance.py | 45 +++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 0b268a6f05a..d3d9f1a9d51 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -808,11 +808,12 @@ except ImportError: pass # caught by AnsibleAWSModule -from ansible.module_utils.six import text_type, string_types +from ansible.module_utils.six import string_types from ansible.module_utils.six.moves.urllib import parse as urlparse -from ansible.module_utils._text import to_bytes, to_native +from ansible.module_utils._text import to_native import ansible_collections.amazon.aws.plugins.module_utils.ec2 as ec2_utils from ansible_collections.amazon.aws.plugins.module_utils.ec2 import (AWSRetry, + ec2_argument_spec, ansible_dict_to_boto3_filter_list, compare_aws_tags, boto3_tag_list_to_ansible_dict, @@ -1332,6 +1333,8 @@ def value_wrapper(v): param_mappings = [ ParamMapper('ebs_optimized', 'EbsOptimized', 'ebsOptimized', value_wrapper), ParamMapper('termination_protection', 'DisableApiTermination', 'disableApiTermination', value_wrapper), + ParamMapper('security_groups', 'Groups', 'groupSet', value_wrapper), + ParamMapper('security_group', 'Groups', 'groupSet', value_wrapper), # user data is an immutable property # ParamMapper('user_data', 'UserData', 'userData', value_wrapper), ] @@ -1339,13 +1342,39 @@ def value_wrapper(v): for mapping in param_mappings: if params.get(mapping.param_key) is not None and mapping.instance_key not in skip: value = AWSRetry.jittered_backoff()(ec2.describe_instance_attribute)(Attribute=mapping.attribute_name, InstanceId=id_) - if params.get(mapping.param_key) is not None and value[mapping.instance_key]['Value'] != params.get(mapping.param_key): - arguments = dict( - InstanceId=instance['InstanceId'], - # Attribute=mapping.attribute_name, + if mapping.param_key not in ['security_groups', 'security_group']: + if params.get(mapping.param_key) is not None and value[mapping.instance_key]['Value'] != params.get(mapping.param_key): + arguments = dict( + InstanceId=instance['InstanceId'], + # Attribute=mapping.attribute_name, + ) + arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key)) + changes_to_apply.append(arguments) + elif bool(params.get(mapping.param_key)) is not False: + if params.get('vpc_subnet_id'): + subnet_id = params.get('vpc_subnet_id') + else: + default_vpc = get_default_vpc(ec2) + if default_vpc is None: + raise module.fail_json( + msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to create an instance") + else: + sub = get_default_subnet(ec2, default_vpc) + subnet_id = sub['SubnetId'] + + groups = discover_security_groups( + group=params.get('security_group'), + groups=params.get('security_groups'), + subnet_id=subnet_id, + ec2=ec2 ) - arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key)) - changes_to_apply.append(arguments) + expected_groups = [g['GroupId'] for g in groups] + instance_groups = [g['GroupId'] for g in value['Groups']] + if set(instance_groups) != set(expected_groups): + changes_to_apply.append(dict( + Groups=expected_groups, + InstanceId=instance['InstanceId'] + )) if (params.get('network') or {}).get('source_dest_check') is not None: # network.source_dest_check is nested, so needs to be treated separately From 0408c88b0ae5ab923ddc949d625e24507b7e1e09 Mon Sep 17 00:00:00 2001 From: Jokajak Date: Wed, 26 Feb 2020 14:21:07 -0500 Subject: [PATCH 02/10] tests(ec2_instance): Tests for SG modifications to existing instances --- .../targets/ec2_instance/inventory | 1 + .../roles/ec2_instance/tasks/env_cleanup.yml | 11 +++ .../roles/ec2_instance/tasks/env_setup.yml | 16 +++++ .../ec2_instance/tasks/security_group.yml | 67 +++++++++++++++++++ 4 files changed, 95 insertions(+) create mode 100644 tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml diff --git a/tests/integration/targets/ec2_instance/inventory b/tests/integration/targets/ec2_instance/inventory index 44b46ec88f7..09bae76beb1 100644 --- a/tests/integration/targets/ec2_instance/inventory +++ b/tests/integration/targets/ec2_instance/inventory @@ -11,6 +11,7 @@ iam_instance_role termination_protection tags_and_vpc_settings checkmode_tests +security_group [all:vars] ansible_connection=local diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_cleanup.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_cleanup.yml index 1b6c79e0d95..07c7f72bd8e 100644 --- a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_cleanup.yml +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_cleanup.yml @@ -33,6 +33,17 @@ ignore_errors: yes retries: 10 +- name: "remove the second security group" + ec2_group: + name: "{{ resource_prefix }}-sg-2" + description: a security group for ansible tests + vpc_id: "{{ testing_vpc.vpc.id }}" + state: absent + register: removed + until: removed is not failed + ignore_errors: yes + retries: 10 + - name: "remove routing rules" ec2_vpc_route_table: state: absent diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_setup.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_setup.yml index 6c76b7bf79f..7c99f807177 100644 --- a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_setup.yml +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/env_setup.yml @@ -77,3 +77,19 @@ to_port: 80 cidr_ip: 0.0.0.0/0 register: sg + + - name: "create secondary security group with the vpc" + ec2_group: + name: "{{ resource_prefix }}-sg-2" + description: a secondary security group for ansible tests + vpc_id: "{{ testing_vpc.vpc.id }}" + rules: + - proto: tcp + from_port: 22 + to_port: 22 + cidr_ip: 0.0.0.0/0 + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + register: sg2 diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml new file mode 100644 index 00000000000..426d87b830b --- /dev/null +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml @@ -0,0 +1,67 @@ +- block: + - name: "New instance with 2 security groups" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + wait: false + security_groups: + - "{{ sg.group_id }}" + - "{{ sg2.group_id }}" + register: security_groups_test + + - name: "Recreate same instance with 2 security groups ( Idempotency )" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + wait: false + security_groups: + - "{{ sg.group_id }}" + - "{{ sg2.group_id }}" + register: security_groups_test_idempotency + + - name: "Gather ec2 facts to check SGs have been added" + ec2_instance_facts: + filters: + "tag:Name": "{{ resource_prefix }}-test-security-groups" + "instance-state-name": "running" + register: dual_sg_instance_facts + until: dual_sg_instance_facts.instances | length > 0 + retries: 10 + + - name: "Remove secondary security group from instance" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + security_groups: + - "{{ sg.group_id }}" + register: remove_secondary_security_group + + - name: "Gather ec2 facts to check seconday SG has been removed" + ec2_instance_facts: + filters: + "tag:Name": "{{ resource_prefix }}-test-security-groups" + "instance-state-name": "running" + register: single_sg_instance_facts + until: single_sg_instance_facts.instances | length > 0 + retries: 10 + + - assert: + that: + - security_groups_test is not failed + - security_groups_test is changed + - security_groups_test_idempotency is not changed + - remove_secondary_security_group is changed + - single_sg_instance_facts.instances.0.security_groups | length == 1 + - dual_sg_instance_facts.instances.0.security_groups | length == 2 From 5236c85f375e313f623a5299df431313e0b3bae8 Mon Sep 17 00:00:00 2001 From: Jokajak Date: Wed, 26 Feb 2020 21:08:30 -0500 Subject: [PATCH 03/10] tests(ec2_instance): Test simultaneous state and SG changes Fixes #54174 --- .../ec2_instance/tasks/security_group.yml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml index 426d87b830b..1a73553e4b6 100644 --- a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml @@ -57,6 +57,86 @@ until: single_sg_instance_facts.instances | length > 0 retries: 10 + - name: "Add secondary security group to instance" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + security_groups: + - "{{ sg.group_id }}" + - "{{ sg2.group_id }}" + register: add_secondary_security_group + + - name: "Remove secondary security group and stop instance" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + security_groups: + - "{{ sg.group_id }}" + state: stopped + register: state_remove_secondary_sg + + - name: "Gather ec2 facts to check seconday SG has been removed and instance stopped" + ec2_instance_facts: + filters: + "tag:Name": "{{ resource_prefix }}-test-security-groups" + "instance-state-name": "stopped" + register: stopping_sg_change + until: stopping_sg_change.instances | length > 0 + retries: 10 + + - name: "Add secondary security group and stop instance" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + security_groups: + - "{{ sg.group_id }}" + - "{{ sg2.group_id }}" + state: stopped + register: state_add_secondary_sg + + - name: "Gather ec2 facts to check seconday SG has been added and instance started" + ec2_instance_facts: + filters: + "tag:Name": "{{ resource_prefix }}-test-security-groups" + "instance-state-name": "stopped" + register: stopped_sg_change + until: stopped_sg_change.instances | length > 0 + retries: 10 + + - name: "Add secondary security group and start instance" + ec2_instance: + name: "{{ resource_prefix }}-test-security-groups" + image_id: "{{ ec2_ami_image }}" + vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" + tags: + TestId: "{{ resource_prefix }}" + instance_type: t2.micro + security_groups: + - "{{ sg.group_id }}" + state: running + register: starting_rem_secondary_sg + + - name: "Gather ec2 facts to check seconday SG has been added and instance started" + ec2_instance_facts: + filters: + "tag:Name": "{{ resource_prefix }}-test-security-groups" + "instance-state-name": "running" + register: starting_sg_change + until: starting_sg_change.instances | length > 0 + retries: 10 + - assert: that: - security_groups_test is not failed @@ -65,3 +145,10 @@ - remove_secondary_security_group is changed - single_sg_instance_facts.instances.0.security_groups | length == 1 - dual_sg_instance_facts.instances.0.security_groups | length == 2 + - add_secondary_security_group is changed + - stopping_sg_change.instances.0.security_groups | length == 1 + - stopping_sg_change.instances.0.state.name == "stopped" + - stopped_sg_change.instances.0.security_groups | length == 2 + - stopped_sg_change.instances.0.state.name == "stopped" + - starting_sg_change.instances.0.security_groups | length == 1 + - starting_sg_change.instances.0.state.name == "running" From 8c8b253590135809e13234e3214ce81af471a5c1 Mon Sep 17 00:00:00 2001 From: Jokajak Date: Wed, 26 Feb 2020 22:04:55 -0500 Subject: [PATCH 04/10] tests(ec2_instance): Disable state changing tests Additionally switch from *_facts to *_info --- .../ec2_instance/tasks/security_group.yml | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml index 1a73553e4b6..cbdfb04ec32 100644 --- a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml @@ -28,7 +28,7 @@ register: security_groups_test_idempotency - name: "Gather ec2 facts to check SGs have been added" - ec2_instance_facts: + ec2_instance_info: filters: "tag:Name": "{{ resource_prefix }}-test-security-groups" "instance-state-name": "running" @@ -49,7 +49,7 @@ register: remove_secondary_security_group - name: "Gather ec2 facts to check seconday SG has been removed" - ec2_instance_facts: + ec2_instance_info: filters: "tag:Name": "{{ resource_prefix }}-test-security-groups" "instance-state-name": "running" @@ -81,10 +81,11 @@ security_groups: - "{{ sg.group_id }}" state: stopped + wait: yes register: state_remove_secondary_sg - name: "Gather ec2 facts to check seconday SG has been removed and instance stopped" - ec2_instance_facts: + ec2_instance_info: filters: "tag:Name": "{{ resource_prefix }}-test-security-groups" "instance-state-name": "stopped" @@ -92,7 +93,7 @@ until: stopping_sg_change.instances | length > 0 retries: 10 - - name: "Add secondary security group and stop instance" + - name: "Add secondary security group to stopped instance" ec2_instance: name: "{{ resource_prefix }}-test-security-groups" image_id: "{{ ec2_ami_image }}" @@ -106,8 +107,8 @@ state: stopped register: state_add_secondary_sg - - name: "Gather ec2 facts to check seconday SG has been added and instance started" - ec2_instance_facts: + - name: "Gather ec2 facts to check seconday SG has been added and instance stayed stopped" + ec2_instance_info: filters: "tag:Name": "{{ resource_prefix }}-test-security-groups" "instance-state-name": "stopped" @@ -126,10 +127,11 @@ security_groups: - "{{ sg.group_id }}" state: running + wait: yes register: starting_rem_secondary_sg - name: "Gather ec2 facts to check seconday SG has been added and instance started" - ec2_instance_facts: + ec2_instance_info: filters: "tag:Name": "{{ resource_prefix }}-test-security-groups" "instance-state-name": "running" @@ -145,10 +147,11 @@ - remove_secondary_security_group is changed - single_sg_instance_facts.instances.0.security_groups | length == 1 - dual_sg_instance_facts.instances.0.security_groups | length == 2 - - add_secondary_security_group is changed - - stopping_sg_change.instances.0.security_groups | length == 1 - - stopping_sg_change.instances.0.state.name == "stopped" - - stopped_sg_change.instances.0.security_groups | length == 2 - - stopped_sg_change.instances.0.state.name == "stopped" - - starting_sg_change.instances.0.security_groups | length == 1 - - starting_sg_change.instances.0.state.name == "running" + # These tests currently fail because instance state can't be changed with parameters + # - add_secondary_security_group is changed + # - stopping_sg_change.instances.0.security_groups | length == 1 + # - stopping_sg_change.instances.0.state.name == "stopped" + # - stopped_sg_change.instances.0.security_groups | length == 2 + # - stopped_sg_change.instances.0.state.name == "stopped" + # - starting_sg_change.instances.0.security_groups | length == 1 + # - starting_sg_change.instances.0.state.name == "running" From f28dd0f2d7f7c6d8ede8396cf89f1427dc5bd396 Mon Sep 17 00:00:00 2001 From: Jokajak Date: Thu, 27 Feb 2020 07:35:43 -0500 Subject: [PATCH 05/10] tests(ec2_instance): Remove expected to fail tests --- .../ec2_instance/tasks/security_group.yml | 78 +------------------ 1 file changed, 1 insertion(+), 77 deletions(-) diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml index cbdfb04ec32..c0e52a5f386 100644 --- a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml +++ b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/security_group.yml @@ -70,75 +70,6 @@ - "{{ sg2.group_id }}" register: add_secondary_security_group - - name: "Remove secondary security group and stop instance" - ec2_instance: - name: "{{ resource_prefix }}-test-security-groups" - image_id: "{{ ec2_ami_image }}" - vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" - tags: - TestId: "{{ resource_prefix }}" - instance_type: t2.micro - security_groups: - - "{{ sg.group_id }}" - state: stopped - wait: yes - register: state_remove_secondary_sg - - - name: "Gather ec2 facts to check seconday SG has been removed and instance stopped" - ec2_instance_info: - filters: - "tag:Name": "{{ resource_prefix }}-test-security-groups" - "instance-state-name": "stopped" - register: stopping_sg_change - until: stopping_sg_change.instances | length > 0 - retries: 10 - - - name: "Add secondary security group to stopped instance" - ec2_instance: - name: "{{ resource_prefix }}-test-security-groups" - image_id: "{{ ec2_ami_image }}" - vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" - tags: - TestId: "{{ resource_prefix }}" - instance_type: t2.micro - security_groups: - - "{{ sg.group_id }}" - - "{{ sg2.group_id }}" - state: stopped - register: state_add_secondary_sg - - - name: "Gather ec2 facts to check seconday SG has been added and instance stayed stopped" - ec2_instance_info: - filters: - "tag:Name": "{{ resource_prefix }}-test-security-groups" - "instance-state-name": "stopped" - register: stopped_sg_change - until: stopped_sg_change.instances | length > 0 - retries: 10 - - - name: "Add secondary security group and start instance" - ec2_instance: - name: "{{ resource_prefix }}-test-security-groups" - image_id: "{{ ec2_ami_image }}" - vpc_subnet_id: "{{ testing_subnet_a.subnet.id }}" - tags: - TestId: "{{ resource_prefix }}" - instance_type: t2.micro - security_groups: - - "{{ sg.group_id }}" - state: running - wait: yes - register: starting_rem_secondary_sg - - - name: "Gather ec2 facts to check seconday SG has been added and instance started" - ec2_instance_info: - filters: - "tag:Name": "{{ resource_prefix }}-test-security-groups" - "instance-state-name": "running" - register: starting_sg_change - until: starting_sg_change.instances | length > 0 - retries: 10 - - assert: that: - security_groups_test is not failed @@ -147,11 +78,4 @@ - remove_secondary_security_group is changed - single_sg_instance_facts.instances.0.security_groups | length == 1 - dual_sg_instance_facts.instances.0.security_groups | length == 2 - # These tests currently fail because instance state can't be changed with parameters - # - add_secondary_security_group is changed - # - stopping_sg_change.instances.0.security_groups | length == 1 - # - stopping_sg_change.instances.0.state.name == "stopped" - # - stopped_sg_change.instances.0.security_groups | length == 2 - # - stopped_sg_change.instances.0.state.name == "stopped" - # - starting_sg_change.instances.0.security_groups | length == 1 - # - starting_sg_change.instances.0.state.name == "running" + - add_secondary_security_group is changed From d7d8e7e27a888323c6443fbb2e4e2cb511294295 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 1 Jun 2020 14:36:16 -0400 Subject: [PATCH 06/10] Update plugins/modules/ec2_instance.py Co-authored-by: Mark Chappell --- plugins/modules/ec2_instance.py | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index d3d9f1a9d51..400b90022f1 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -813,7 +813,6 @@ from ansible.module_utils._text import to_native import ansible_collections.amazon.aws.plugins.module_utils.ec2 as ec2_utils from ansible_collections.amazon.aws.plugins.module_utils.ec2 import (AWSRetry, - ec2_argument_spec, ansible_dict_to_boto3_filter_list, compare_aws_tags, boto3_tag_list_to_ansible_dict, From 0e1b12499859d90c72f144015f914d8fffef63cb Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 1 Jun 2020 14:36:30 -0400 Subject: [PATCH 07/10] Update plugins/modules/ec2_instance.py Co-authored-by: Mark Chappell --- plugins/modules/ec2_instance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 400b90022f1..a790215fecd 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -1355,7 +1355,7 @@ def value_wrapper(v): else: default_vpc = get_default_vpc(ec2) if default_vpc is None: - raise module.fail_json( + module.fail_json( msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to create an instance") else: sub = get_default_subnet(ec2, default_vpc) From a48f6b5e70b6638faaea07f38a33948bf0f283b1 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 7 Jun 2020 07:26:46 -0400 Subject: [PATCH 08/10] refactor(ec2_instance): Move security out of for loop Instead of looping over the param_mapping for security groups they are called out specifically. Refs #21 --- plugins/modules/ec2_instance.py | 78 +++++++++++++++++---------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index a790215fecd..85dd480ee75 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -1332,48 +1332,52 @@ def value_wrapper(v): param_mappings = [ ParamMapper('ebs_optimized', 'EbsOptimized', 'ebsOptimized', value_wrapper), ParamMapper('termination_protection', 'DisableApiTermination', 'disableApiTermination', value_wrapper), - ParamMapper('security_groups', 'Groups', 'groupSet', value_wrapper), - ParamMapper('security_group', 'Groups', 'groupSet', value_wrapper), # user data is an immutable property # ParamMapper('user_data', 'UserData', 'userData', value_wrapper), ] for mapping in param_mappings: - if params.get(mapping.param_key) is not None and mapping.instance_key not in skip: - value = AWSRetry.jittered_backoff()(ec2.describe_instance_attribute)(Attribute=mapping.attribute_name, InstanceId=id_) - if mapping.param_key not in ['security_groups', 'security_group']: - if params.get(mapping.param_key) is not None and value[mapping.instance_key]['Value'] != params.get(mapping.param_key): - arguments = dict( - InstanceId=instance['InstanceId'], - # Attribute=mapping.attribute_name, - ) - arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key)) - changes_to_apply.append(arguments) - elif bool(params.get(mapping.param_key)) is not False: - if params.get('vpc_subnet_id'): - subnet_id = params.get('vpc_subnet_id') - else: - default_vpc = get_default_vpc(ec2) - if default_vpc is None: - module.fail_json( - msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to create an instance") - else: - sub = get_default_subnet(ec2, default_vpc) - subnet_id = sub['SubnetId'] - - groups = discover_security_groups( - group=params.get('security_group'), - groups=params.get('security_groups'), - subnet_id=subnet_id, - ec2=ec2 - ) - expected_groups = [g['GroupId'] for g in groups] - instance_groups = [g['GroupId'] for g in value['Groups']] - if set(instance_groups) != set(expected_groups): - changes_to_apply.append(dict( - Groups=expected_groups, - InstanceId=instance['InstanceId'] - )) + if params.get(mapping.param_key) is None: + continue + if mapping.instance_key in skip: + continue + + value = AWSRetry.jittered_backoff()(ec2.describe_instance_attribute)(Attribute=mapping.attribute_name, InstanceId=id_) + if value[mapping.instance_key]['Value'] != params.get(mapping.param_key): + arguments = dict( + InstanceId=instance['InstanceId'], + # Attribute=mapping.attribute_name, + ) + arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key)) + changes_to_apply.append(arguments) + + if params.get('security_group') or params.get('security_groups'): + value = AWSRetry.jittered_backoff()(ec2.describe_instance_attribute)(Attribute="groupSet", InstanceId=id_) + # managing security groups + if params.get('vpc_subnet_id'): + subnet_id = params.get('vpc_subnet_id') + else: + default_vpc = get_default_vpc(ec2) + if default_vpc is None: + module.fail_json( + msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to create an instance") + else: + sub = get_default_subnet(ec2, default_vpc) + subnet_id = sub['SubnetId'] + + groups = discover_security_groups( + group=params.get('security_group'), + groups=params.get('security_groups'), + subnet_id=subnet_id, + ec2=ec2 + ) + expected_groups = [g['GroupId'] for g in groups] + instance_groups = [g['GroupId'] for g in value['Groups']] + if set(instance_groups) != set(expected_groups): + changes_to_apply.append(dict( + Groups=expected_groups, + InstanceId=instance['InstanceId'] + )) if (params.get('network') or {}).get('source_dest_check') is not None: # network.source_dest_check is nested, so needs to be treated separately From 086c09ff4c95d12e27918882a21ed95a0f078f5a Mon Sep 17 00:00:00 2001 From: Jokajak Date: Thu, 13 Aug 2020 10:13:30 -0400 Subject: [PATCH 09/10] style(ec2_instance): Update fail message to reflect security groups When a subnet is not defined and cannot be found generate an error message that reflects operating on a security group. Refs #21 --- plugins/modules/ec2_instance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 85dd480ee75..07f1189664e 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -1360,7 +1360,7 @@ def value_wrapper(v): default_vpc = get_default_vpc(ec2) if default_vpc is None: module.fail_json( - msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to create an instance") + msg="No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter) to modify security groups.") else: sub = get_default_subnet(ec2, default_vpc) subnet_id = sub['SubnetId'] From 544857527a6c0a2b768994388391899d9bc1231b Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sun, 16 Aug 2020 15:22:51 +0200 Subject: [PATCH 10/10] Add changelog --- changelogs/fragments/22-ec2_instance-mod-sgs.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/22-ec2_instance-mod-sgs.yml diff --git a/changelogs/fragments/22-ec2_instance-mod-sgs.yml b/changelogs/fragments/22-ec2_instance-mod-sgs.yml new file mode 100644 index 00000000000..c8145d6624c --- /dev/null +++ b/changelogs/fragments/22-ec2_instance-mod-sgs.yml @@ -0,0 +1,2 @@ +bugfixes: +- ec2_instance - fixes issue where security groups were not changed if the instance already existed. https://github.com/ansible-collections/community.aws/pull/22