From c1ee09b69e97baa06653bb2539bccaf085d50971 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 11 Oct 2022 13:36:39 +0200 Subject: [PATCH] ec2_instance - update build_run_instance_spec to skip TagSpecification if None --- .../1148-build_run_instance_spec.yml | 7 + plugins/modules/ec2_instance.py | 44 ++++--- .../targets/ec2_vpc_nat_gateway/aliases | 3 + tests/integration/targets/kms_key/aliases | 3 + tests/integration/targets/rds_cluster/aliases | 2 + .../targets/rds_cluster_snapshot/aliases | 2 + .../test_build_run_instance_spec.py | 123 ++++++++++++++++++ .../ec2_instance/test_determine_iam_role.py | 101 ++++++++++++++ 8 files changed, 265 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/1148-build_run_instance_spec.yml create mode 100644 tests/unit/plugins/modules/ec2_instance/test_build_run_instance_spec.py create mode 100644 tests/unit/plugins/modules/ec2_instance/test_determine_iam_role.py diff --git a/changelogs/fragments/1148-build_run_instance_spec.yml b/changelogs/fragments/1148-build_run_instance_spec.yml new file mode 100644 index 00000000000..2e8a5f5db52 --- /dev/null +++ b/changelogs/fragments/1148-build_run_instance_spec.yml @@ -0,0 +1,7 @@ +minor_changes: +- ec2_instance - the ``instance_role`` parameter has been renamed to ``iam_instance_profile`` to + better reflect what it is, ``instance_role`` remains as an alias + (https://github.com/ansible-collections/amazon.aws/pull/1151). +bugfixes: +- ec2_instance - fixes ``Invalid type for parameter TagSpecifications, value None`` error when + tags aren't specified (https://github.com/ansible-collections/amazon.aws/issues/1148). diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 1e9687fec6b..65440a20885 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -293,13 +293,14 @@ - By default, instances are filtered for counting by their "Name" tag, base AMI, state (running, by default), and subnet ID. Any queryable filter can be used. Good candidates are specific tags, SSH keys, or security groups. type: dict - instance_role: + iam_instance_profile: description: - - The ARN or name of an EC2-enabled instance role to be used. + - The ARN or name of an EC2-enabled IAM instance profile to be used. - If a name is not provided in ARN format then the ListInstanceProfiles permission must also be granted. U(https://docs.aws.amazon.com/IAM/latest/APIReference/API_ListInstanceProfiles.html) - If no full ARN is provided, the role with a matching name will be used from the active AWS account. type: str + aliases: ['instance_role'] placement_group: description: - The placement group that needs to be assigned to the instance. @@ -1354,28 +1355,31 @@ def build_run_instance_spec(params): MaxCount=1, MinCount=1, ) - # network parameters + spec.update(**build_top_level_options(params)) + spec['NetworkInterfaces'] = build_network_spec(params) spec['BlockDeviceMappings'] = build_volume_spec(params) - spec.update(**build_top_level_options(params)) - spec['TagSpecifications'] = build_instance_tags(params) + + tag_spec = build_instance_tags(params) + if tag_spec is not None: + spec['TagSpecifications'] = tag_spec # IAM profile - if params.get('instance_role'): - spec['IamInstanceProfile'] = dict(Arn=determine_iam_role(params.get('instance_role'))) + if params.get('iam_instance_profile'): + spec['IamInstanceProfile'] = dict(Arn=determine_iam_role(params.get('iam_instance_profile'))) - if module.params.get('exact_count'): - spec['MaxCount'] = module.params.get('to_launch') - spec['MinCount'] = module.params.get('to_launch') + if params.get('exact_count'): + spec['MaxCount'] = params.get('to_launch') + spec['MinCount'] = params.get('to_launch') - if module.params.get('count'): - spec['MaxCount'] = module.params.get('count') - spec['MinCount'] = module.params.get('count') + if params.get('count'): + spec['MaxCount'] = params.get('count') + spec['MinCount'] = params.get('count') - if not module.params.get('launch_template'): - spec['InstanceType'] = params['instance_type'] if module.params.get('instance_type') else 't2.micro' + if not params.get('launch_template'): + spec['InstanceType'] = params['instance_type'] if params.get('instance_type') else 't2.micro' - if module.params.get('launch_template') and module.params.get('instance_type'): + if params.get('launch_template') and params.get('instance_type'): spec['InstanceType'] = params['instance_type'] return spec @@ -1794,9 +1798,9 @@ def determine_iam_role(name_or_arn): role = iam.get_instance_profile(InstanceProfileName=name_or_arn, aws_retry=True) return role['InstanceProfile']['Arn'] except is_boto3_error_code('NoSuchEntity') as e: - module.fail_json_aws(e, msg="Could not find instance_role {0}".format(name_or_arn)) + module.fail_json_aws(e, msg="Could not find iam_instance_profile {0}".format(name_or_arn)) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except - module.fail_json_aws(e, msg="An error occurred while searching for instance_role {0}. Please try supplying the full ARN.".format(name_or_arn)) + module.fail_json_aws(e, msg="An error occurred while searching for iam_instance_profile {0}. Please try supplying the full ARN.".format(name_or_arn)) def handle_existing(existing_matches, state): @@ -1827,7 +1831,7 @@ def handle_existing(existing_matches, state): module.fail_json_aws(e, msg="Could not apply change {0} to existing instance.".format(str(c))) all_changes.extend(changes) changed |= bool(changes) - changed |= add_or_update_instance_profile(existing_matches[0], module.params.get('instance_role')) + changed |= add_or_update_instance_profile(existing_matches[0], module.params.get('iam_instance_profile')) changed |= change_network_attachments(existing_matches[0], module.params) altered = find_instances(ids=[i['InstanceId'] for i in existing_matches]) @@ -2022,7 +2026,7 @@ def main(): availability_zone=dict(type='str'), security_groups=dict(default=[], type='list', elements='str'), security_group=dict(type='str'), - instance_role=dict(type='str'), + iam_instance_profile=dict(type='str', aliases=['instance_role']), name=dict(type='str'), tags=dict(type='dict', aliases=['resource_tags']), purge_tags=dict(type='bool', default=True), diff --git a/tests/integration/targets/ec2_vpc_nat_gateway/aliases b/tests/integration/targets/ec2_vpc_nat_gateway/aliases index f5291520b8b..5a9dd5bcdce 100644 --- a/tests/integration/targets/ec2_vpc_nat_gateway/aliases +++ b/tests/integration/targets/ec2_vpc_nat_gateway/aliases @@ -1,2 +1,5 @@ +time=10m + cloud/aws + ec2_vpc_nat_gateway_info diff --git a/tests/integration/targets/kms_key/aliases b/tests/integration/targets/kms_key/aliases index 967fd7fe094..36c332ab4ba 100644 --- a/tests/integration/targets/kms_key/aliases +++ b/tests/integration/targets/kms_key/aliases @@ -3,6 +3,9 @@ # No KMS supported waiters, and manual waiting for updates didn't fix the issue either. # Issue likely from AWS side - added waits on updates in integration tests to workaround this. +# Some KMS operations are just slow +time=10m + cloud/aws kms_key_info diff --git a/tests/integration/targets/rds_cluster/aliases b/tests/integration/targets/rds_cluster/aliases index 0d778791cb1..6e9f239e073 100644 --- a/tests/integration/targets/rds_cluster/aliases +++ b/tests/integration/targets/rds_cluster/aliases @@ -1,3 +1,5 @@ +time=10m + cloud/aws rds_cluster_info diff --git a/tests/integration/targets/rds_cluster_snapshot/aliases b/tests/integration/targets/rds_cluster_snapshot/aliases index e9ea741b75a..7f2c75f2604 100644 --- a/tests/integration/targets/rds_cluster_snapshot/aliases +++ b/tests/integration/targets/rds_cluster_snapshot/aliases @@ -1,3 +1,5 @@ +time=10m + cloud/aws rds_snapshot_info diff --git a/tests/unit/plugins/modules/ec2_instance/test_build_run_instance_spec.py b/tests/unit/plugins/modules/ec2_instance/test_build_run_instance_spec.py new file mode 100644 index 00000000000..17a5ea4ec70 --- /dev/null +++ b/tests/unit/plugins/modules/ec2_instance/test_build_run_instance_spec.py @@ -0,0 +1,123 @@ +# (c) 2022 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest + +from ansible_collections.amazon.aws.tests.unit.compat.mock import sentinel +import ansible_collections.amazon.aws.plugins.modules.ec2_instance as ec2_instance_module + + +@pytest.fixture +def params_object(): + params = { + 'iam_instance_profile': None, + 'exact_count': None, + 'count': None, + 'launch_template': None, + 'instance_type': None, + } + return params + + +@pytest.fixture +def ec2_instance(monkeypatch): + # monkey patches various ec2_instance module functions, we'll separately test the operation of + # these functions, we just care that it's passing the results into the right place in the + # instance spec. + monkeypatch.setattr(ec2_instance_module, 'build_top_level_options', lambda params: {'TOP_LEVEL_OPTIONS': sentinel.TOP_LEVEL}) + monkeypatch.setattr(ec2_instance_module, 'build_network_spec', lambda params: sentinel.NETWORK_SPEC) + monkeypatch.setattr(ec2_instance_module, 'build_volume_spec', lambda params: sentinel.VOlUME_SPEC) + monkeypatch.setattr(ec2_instance_module, 'build_instance_tags', lambda params: sentinel.TAG_SPEC) + monkeypatch.setattr(ec2_instance_module, 'determine_iam_role', lambda params: sentinel.IAM_PROFILE_ARN) + return ec2_instance_module + + +def _assert_defaults(instance_spec, to_skip=[]): + assert isinstance(instance_spec, dict) + + if 'TagSpecifications' not in to_skip: + assert 'TagSpecifications' in instance_spec + assert instance_spec['TagSpecifications'] is sentinel.TAG_SPEC + + if 'NetworkInterfaces' not in to_skip: + assert 'NetworkInterfaces' in instance_spec + assert instance_spec['NetworkInterfaces'] is sentinel.NETWORK_SPEC + + if 'BlockDeviceMappings' not in to_skip: + assert 'BlockDeviceMappings' in instance_spec + assert instance_spec['BlockDeviceMappings'] is sentinel.VOlUME_SPEC + + if 'IamInstanceProfile' not in to_skip: + # By default, this shouldn't be returned + assert 'IamInstanceProfile' not in instance_spec + + if 'MinCount' not in to_skip: + assert 'MinCount' in instance_spec + instance_spec['MinCount'] == 1 + + if 'MaxCount' not in to_skip: + assert 'MaxCount' in instance_spec + instance_spec['MaxCount'] == 1 + + if 'TOP_LEVEL_OPTIONS' not in to_skip: + assert 'TOP_LEVEL_OPTIONS' in instance_spec + assert instance_spec['TOP_LEVEL_OPTIONS'] is sentinel.TOP_LEVEL + + +def test_build_run_instance_spec_defaults(params_object, ec2_instance): + instance_spec = ec2_instance.build_run_instance_spec(params_object) + _assert_defaults(instance_spec) + + +def test_build_run_instance_spec_tagging(params_object, ec2_instance, monkeypatch): + # build_instance_tags can return None, RunInstance doesn't like this + monkeypatch.setattr(ec2_instance_module, 'build_instance_tags', lambda params: None) + instance_spec = ec2_instance.build_run_instance_spec(params_object) + _assert_defaults(instance_spec, ['TagSpecifications']) + assert 'TagSpecifications' not in instance_spec + + # if someone *explicitly* passes {} (rather than not setting it), then [] can be returned + monkeypatch.setattr(ec2_instance_module, 'build_instance_tags', lambda params: []) + instance_spec = ec2_instance.build_run_instance_spec(params_object) + _assert_defaults(instance_spec, ['TagSpecifications']) + assert 'TagSpecifications' in instance_spec + assert instance_spec['TagSpecifications'] == [] + + +def test_build_run_instance_spec_instance_profile(params_object, ec2_instance): + params_object['iam_instance_profile'] = sentinel.INSTANCE_PROFILE_NAME + instance_spec = ec2_instance.build_run_instance_spec(params_object) + _assert_defaults(instance_spec, ['IamInstanceProfile']) + assert 'IamInstanceProfile' in instance_spec + assert instance_spec['IamInstanceProfile'] == {'Arn': sentinel.IAM_PROFILE_ARN} + + +def test_build_run_instance_spec_count(params_object, ec2_instance): + # When someone passes 'count', that number of instances will be *launched* + params_object['count'] = sentinel.COUNT + instance_spec = ec2_instance.build_run_instance_spec(params_object) + _assert_defaults(instance_spec, ['MaxCount', 'MinCount']) + assert 'MaxCount' in instance_spec + assert 'MinCount' in instance_spec + assert instance_spec['MaxCount'] == sentinel.COUNT + assert instance_spec['MinCount'] == sentinel.COUNT + + +def test_build_run_instance_spec_exact_count(params_object, ec2_instance): + # The "exact_count" logic relies on enforce_count doing the math to figure out how many + # instances to start/stop. The enforce_count call is responsible for ensuring that 'to_launch' + # is set and is a positive integer. + params_object['exact_count'] = sentinel.EXACT_COUNT + params_object['to_launch'] = sentinel.TO_LAUNCH + instance_spec = ec2_instance.build_run_instance_spec(params_object) + + _assert_defaults(instance_spec, ['MaxCount', 'MinCount']) + assert 'MaxCount' in instance_spec + assert 'MinCount' in instance_spec + assert instance_spec['MaxCount'] == sentinel.TO_LAUNCH + assert instance_spec['MinCount'] == sentinel.TO_LAUNCH diff --git a/tests/unit/plugins/modules/ec2_instance/test_determine_iam_role.py b/tests/unit/plugins/modules/ec2_instance/test_determine_iam_role.py new file mode 100644 index 00000000000..892e36dd096 --- /dev/null +++ b/tests/unit/plugins/modules/ec2_instance/test_determine_iam_role.py @@ -0,0 +1,101 @@ +# (c) 2022 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest + +from ansible_collections.amazon.aws.tests.unit.compat.mock import MagicMock +from ansible_collections.amazon.aws.tests.unit.compat.mock import sentinel +import ansible_collections.amazon.aws.plugins.modules.ec2_instance as ec2_instance_module +import ansible_collections.amazon.aws.plugins.module_utils.arn as utils_arn +from ansible_collections.amazon.aws.plugins.module_utils.botocore import HAS_BOTO3 + +try: + import botocore +except: + pass + +pytest.mark.skipif(not HAS_BOTO3, reason="test_determine_iam_role.py requires the python modules 'boto3' and 'botocore'") + + +def _client_error(code='GenericError'): + return botocore.exceptions.ClientError( + {'Error': {'Code': code, 'Message': 'Something went wrong'}, + 'ResponseMetadata': {'RequestId': '01234567-89ab-cdef-0123-456789abcdef'}}, + 'some_called_method') + + +@pytest.fixture +def params_object(): + params = { + 'instance_role': None, + 'exact_count': None, + 'count': None, + 'launch_template': None, + 'instance_type': None, + } + return params + + +class FailJsonException(Exception): + def __init__(self): + pass + + +@pytest.fixture +def ec2_instance(monkeypatch): + monkeypatch.setattr(ec2_instance_module, 'parse_aws_arn', lambda arn: None) + monkeypatch.setattr(ec2_instance_module, 'module', MagicMock()) + ec2_instance_module.module.fail_json.side_effect = FailJsonException() + ec2_instance_module.module.fail_json_aws.side_effect = FailJsonException() + return ec2_instance_module + + +def test_determine_iam_role_arn(params_object, ec2_instance, monkeypatch): + # Revert the default monkey patch to make it simple to try passing a valid ARNs + monkeypatch.setattr(ec2_instance, 'parse_aws_arn', utils_arn.parse_aws_arn) + + # Simplest example, someone passes a valid instance profile ARN + arn = ec2_instance.determine_iam_role('arn:aws:iam::123456789012:instance-profile/myprofile') + assert arn == 'arn:aws:iam::123456789012:instance-profile/myprofile' + + +def test_determine_iam_role_name(params_object, ec2_instance): + profile_description = {'InstanceProfile': {'Arn': sentinel.IAM_PROFILE_ARN}} + iam_client = MagicMock(**{"get_instance_profile.return_value": profile_description}) + ec2_instance_module.module.client.return_value = iam_client + + arn = ec2_instance.determine_iam_role(sentinel.IAM_PROFILE_NAME) + assert arn == sentinel.IAM_PROFILE_ARN + + +def test_determine_iam_role_missing(params_object, ec2_instance): + missing_exception = _client_error('NoSuchEntity') + iam_client = MagicMock(**{"get_instance_profile.side_effect": missing_exception}) + ec2_instance_module.module.client.return_value = iam_client + + with pytest.raises(FailJsonException) as exception: + arn = ec2_instance.determine_iam_role(sentinel.IAM_PROFILE_NAME) + + assert ec2_instance_module.module.fail_json_aws.call_count == 1 + assert ec2_instance_module.module.fail_json_aws.call_args.args[0] is missing_exception + assert 'Could not find' in ec2_instance_module.module.fail_json_aws.call_args.kwargs['msg'] + + +@pytest.mark.skipif(sys.version_info < (3, 8, 'call_args behaviour changed in Python 3.8') +def test_determine_iam_role_missing(params_object, ec2_instance): + missing_exception = _client_error() + iam_client = MagicMock(**{"get_instance_profile.side_effect": missing_exception}) + ec2_instance_module.module.client.return_value = iam_client + + with pytest.raises(FailJsonException) as exception: + arn = ec2_instance.determine_iam_role(sentinel.IAM_PROFILE_NAME) + + assert ec2_instance_module.module.fail_json_aws.call_count == 1 + assert ec2_instance_module.module.fail_json_aws.call_args.args[0] is missing_exception + assert 'An error occurred while searching' in ec2_instance_module.module.fail_json_aws.call_args.kwargs['msg'] + assert 'Please try supplying the full ARN' in ec2_instance_module.module.fail_json_aws.call_args.kwargs['msg']