From 9a958da57e56d1ca74ca6d1c1cec2dd0ac40f464 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Wed, 17 May 2017 01:14:05 +1000 Subject: [PATCH] [cloud] Make ec2_vpc_nat_gateway results consistent (#24042) Ensure newly created NAT gateways get converted to snake dict Remove custom code, and associated test, for generating snake dict and use `camel_dict_to_snake_dict` Make use of `required_if` rather than bespoke parameter checks Remove ec2_vpc_nat_gateway from pep8 legacy files list --- .../cloud/amazon/ec2_vpc_nat_gateway.py | 103 ++++-------------- test/sanity/pep8/legacy-files.txt | 1 - .../cloud/amazon/test_ec2_vpc_nat_gateway.py | 30 ----- 3 files changed, 19 insertions(+), 115 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py b/lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py index a993a9d09f..279be66e2c 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py +++ b/lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py @@ -212,20 +212,16 @@ nat_gateway_addresses: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ec2 import ec2_argument_spec, get_aws_connection_info, boto3_conn +from ansible.module_utils.ec2 import camel_dict_to_snake_dict, HAS_BOTO3 import datetime import random -import re import time -from dateutil.tz import tzutc - try: import botocore - import boto3 - HAS_BOTO3 = True except ImportError: - HAS_BOTO3 = False + pass # caught by imported HAS_BOTO3 DRY_RUN_GATEWAYS = [ { @@ -244,23 +240,6 @@ DRY_RUN_GATEWAYS = [ "vpc_id": "vpc-12345678" } ] -DRY_RUN_GATEWAY_UNCONVERTED = [ - { - 'VpcId': 'vpc-12345678', - 'State': 'available', - 'NatGatewayId': 'nat-123456789', - 'SubnetId': 'subnet-123456789', - 'NatGatewayAddresses': [ - { - 'PublicIp': '55.55.55.55', - 'NetworkInterfaceId': 'eni-1234567', - 'AllocationId': 'eipalloc-1234567', - 'PrivateIp': '10.0.0.102' - } - ], - 'CreateTime': datetime.datetime(2016, 3, 5, 5, 19, 20, 282000, tzinfo=tzutc()) - } -] DRY_RUN_ALLOCATION_UNCONVERTED = { 'Addresses': [ @@ -275,45 +254,6 @@ DRY_RUN_ALLOCATION_UNCONVERTED = { DRY_RUN_MSGS = 'DryRun Mode:' -def convert_to_lower(data): - """Convert all uppercase keys in dict with lowercase_ - - Args: - data (dict): Dictionary with keys that have upper cases in them - Example.. FooBar == foo_bar - if a val is of type datetime.datetime, it will be converted to - the ISO 8601 - - Basic Usage: - >>> test = {'FooBar': []} - >>> test = convert_to_lower(test) - { - 'foo_bar': [] - } - - Returns: - Dictionary - """ - results = dict() - if isinstance(data, dict): - for key, val in data.items(): - key = re.sub(r'(([A-Z]{1,3}){1})', r'_\1', key).lower() - if key[0] == '_': - key = key[1:] - if isinstance(val, datetime.datetime): - results[key] = val.isoformat() - elif isinstance(val, dict): - results[key] = convert_to_lower(val) - elif isinstance(val, list): - converted = list() - for item in val: - converted.append(convert_to_lower(item)) - results[key] = converted - else: - results[key] = val - return results - - def get_nat_gateways(client, subnet_id=None, nat_gateway_id=None, states=None, check_mode=False): """Retrieve a list of NAT Gateways @@ -378,7 +318,7 @@ def get_nat_gateways(client, subnet_id=None, nat_gateway_id=None, gateways = client.describe_nat_gateways(**params)['NatGateways'] if gateways: for gw in gateways: - existing_gateways.append(convert_to_lower(gw)) + existing_gateways.append(camel_dict_to_snake_dict(gw)) gateways_retrieved = True else: gateways_retrieved = True @@ -747,22 +687,22 @@ def create(client, subnet_id, allocation_id, client_token=None, try: if not check_mode: - result = client.create_nat_gateway(**params)["NatGateway"] + result = camel_dict_to_snake_dict(client.create_nat_gateway(**params)["NatGateway"]) else: - result = DRY_RUN_GATEWAY_UNCONVERTED[0] - result['CreateTime'] = datetime.datetime.utcnow() - result['NatGatewayAddresses'][0]['AllocationId'] = allocation_id - result['SubnetId'] = subnet_id + result = DRY_RUN_GATEWAYS[0] + result['create_time'] = datetime.datetime.utcnow() + result['nat_gateway_addresses'][0]['Allocation_id'] = allocation_id + result['subnet_id'] = subnet_id success = True changed = True - create_time = result['CreateTime'].replace(tzinfo=None) + create_time = result['create_time'].replace(tzinfo=None) if token_provided and (request_time > create_time): changed = False elif wait: success, err_msg, result = ( wait_for_status( - client, wait_timeout, result['NatGatewayId'], 'available', + client, wait_timeout, result['nat_gateway_id'], 'available', check_mode=check_mode ) ) @@ -949,7 +889,7 @@ def remove(client, nat_gateway_id, wait=False, wait_timeout=0, changed = False err_msg = "" results = list() - states = ['pending', 'available' ] + states = ['pending', 'available'] try: exist, _, gw = ( get_nat_gateways( @@ -1023,7 +963,9 @@ def main(): supports_check_mode=True, mutually_exclusive=[ ['allocation_id', 'eip_address'] - ] + ], + required_if=[['state', 'absent', ['nat_gateway_id']], + ['state', 'present', ['subnet_id']]] ) # Validate Requirements @@ -1059,9 +1001,6 @@ def main(): err_msg = '' if state == 'present': - if not subnet_id: - module.fail_json(msg='subnet_id is required for creation') - success, changed, err_msg, results = ( pre_create( client, subnet_id, allocation_id, eip_address, @@ -1070,16 +1009,12 @@ def main(): ) ) else: - if not nat_gateway_id: - module.fail_json(msg='nat_gateway_id is required for removal') - - else: - success, changed, err_msg, results = ( - remove( - client, nat_gateway_id, wait, wait_timeout, release_eip, - check_mode=check_mode - ) + success, changed, err_msg, results = ( + remove( + client, nat_gateway_id, wait, wait_timeout, release_eip, + check_mode=check_mode ) + ) if not success: module.fail_json( diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 4f017aa38c..695232128a 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -136,7 +136,6 @@ lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options.py lib/ansible/modules/cloud/amazon/ec2_vpc_igw.py lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py lib/ansible/modules/cloud/amazon/ec2_vpc_nacl_facts.py -lib/ansible/modules/cloud/amazon/ec2_vpc_nat_gateway.py lib/ansible/modules/cloud/amazon/ec2_vpc_net.py lib/ansible/modules/cloud/amazon/ec2_vpc_net_facts.py lib/ansible/modules/cloud/amazon/ec2_vpc_peer.py diff --git a/test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py b/test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py index 61ea9a0b8e..3bd13c95c9 100644 --- a/test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py +++ b/test/units/modules/cloud/amazon/test_ec2_vpc_nat_gateway.py @@ -264,36 +264,6 @@ class AnsibleVpcNatGatewayTasks(unittest.TestCase): class AnsibleEc2VpcNatGatewayFunctions(unittest.TestCase): - def test_convert_to_lower(self): - example = ng.DRY_RUN_GATEWAY_UNCONVERTED - converted_example = ng.convert_to_lower(example[0]) - keys = list(converted_example.keys()) - keys.sort() - for i in range(len(keys)): - if i == 0: - self.assertEqual(keys[i], 'create_time') - if i == 1: - self.assertEqual(keys[i], 'nat_gateway_addresses') - gw_addresses_keys = list(converted_example[keys[i]][0].keys()) - gw_addresses_keys.sort() - for j in range(len(gw_addresses_keys)): - if j == 0: - self.assertEqual(gw_addresses_keys[j], 'allocation_id') - if j == 1: - self.assertEqual(gw_addresses_keys[j], 'network_interface_id') - if j == 2: - self.assertEqual(gw_addresses_keys[j], 'private_ip') - if j == 3: - self.assertEqual(gw_addresses_keys[j], 'public_ip') - if i == 2: - self.assertEqual(keys[i], 'nat_gateway_id') - if i == 3: - self.assertEqual(keys[i], 'state') - if i == 4: - self.assertEqual(keys[i], 'subnet_id') - if i == 5: - self.assertEqual(keys[i], 'vpc_id') - def test_get_nat_gateways(self): client = boto3.client('ec2', region_name=aws_region) success, err_msg, stream = (