Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/ec2 instance mod sgs #22

Merged
merged 10 commits into from
Aug 16, 2020
2 changes: 2 additions & 0 deletions changelogs/fragments/22-ec2_instance-mod-sgs.yml
Original file line number Diff line number Diff line change
@@ -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
54 changes: 43 additions & 11 deletions plugins/modules/ec2_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -808,9 +808,9 @@
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,
ansible_dict_to_boto3_filter_list,
Expand Down Expand Up @@ -1337,15 +1337,47 @@ 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,
)
arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key))
changes_to_apply.append(arguments)
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 modify security groups.")
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
Expand Down
1 change: 1 addition & 0 deletions tests/integration/targets/ec2_instance/inventory
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ iam_instance_role
termination_protection
tags_and_vpc_settings
checkmode_tests
security_group

[all:vars]
ansible_connection=local
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
- 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_info:
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_info:
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

- 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

- 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
- add_secondary_security_group is changed