From da3f7a8db1ccb33fb3b2c6d4b8fa2079a3c6bc0d Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Thu, 22 Mar 2018 15:15:36 -0400 Subject: [PATCH] [cloud] ec2_vpc_route_table: ignore routes without DestinationCidrBlock - fixes #37003 (#37010) * [cloud] ec2_vpc_route_table: ignore routes without DestinationCidrBlock Add module warnings rather than silently skipping * Permit warnings for routes tables containing vpc endpoints to be turned off * Add tests to ensure a VPC endpoint associated with a route table does not result in a traceback --- .../cloud/amazon/ec2_vpc_route_table.py | 20 +++++++-- .../ec2_vpc_route_table/tasks/main.yml | 45 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/ec2_vpc_route_table.py b/lib/ansible/modules/cloud/amazon/ec2_vpc_route_table.py index 389ab702f8..94c2c19058 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_vpc_route_table.py +++ b/lib/ansible/modules/cloud/amazon/ec2_vpc_route_table.py @@ -413,12 +413,15 @@ def get_route_table_by_tags(connection, module, vpc_id, tags): def route_spec_matches_route(route_spec, route): if route_spec.get('GatewayId') and 'nat-' in route_spec['GatewayId']: route_spec['NatGatewayId'] = route_spec.pop('GatewayId') + if route_spec.get('GatewayId') and 'vpce-' in route_spec['GatewayId']: + if route_spec.get('DestinationCidrBlock', '').startswith('pl-'): + route_spec['DestinationPrefixListId'] = route_spec.pop('DestinationCidrBlock') return set(route_spec.items()).issubset(route.items()) def route_spec_matches_route_cidr(route_spec, route): - return route_spec['DestinationCidrBlock'] == route['DestinationCidrBlock'] + return route_spec['DestinationCidrBlock'] == route.get('DestinationCidrBlock') def rename_key(d, old_key, new_key): @@ -441,15 +444,26 @@ def ensure_routes(connection=None, module=None, route_table=None, route_specs=No for route_spec in route_specs: match = index_of_matching_route(route_spec, routes_to_match) if match is None: - route_specs_to_create.append(route_spec) + if route_spec.get('DestinationCidrBlock'): + route_specs_to_create.append(route_spec) + else: + module.warn("Skipping creating {0} because it has no destination cidr block. " + "To add VPC endpoints to route tables use the ec2_vpc_endpoint module.".format(route_spec)) else: if match[0] == "replace": - route_specs_to_recreate.append(route_spec) + if route_spec.get('DestinationCidrBlock'): + route_specs_to_recreate.append(route_spec) + else: + module.warn("Skipping recreating route {0} because it has no destination cidr block.".format(route_spec)) del routes_to_match[match[1]] routes_to_delete = [] if purge_routes: for r in routes_to_match: + if not r.get('DestinationCidrBlock'): + module.warn("Skipping purging route {0} because it has no destination cidr block. " + "To remove VPC endpoints from route tables use the ec2_vpc_endpoint module.".format(r)) + continue if r['Origin'] == 'CreateRoute': routes_to_delete.append(r) diff --git a/test/integration/targets/ec2_vpc_route_table/tasks/main.yml b/test/integration/targets/ec2_vpc_route_table/tasks/main.yml index 045976e43c..6467bff1fd 100644 --- a/test/integration/targets/ec2_vpc_route_table/tasks/main.yml +++ b/test/integration/targets/ec2_vpc_route_table/tasks/main.yml @@ -545,10 +545,55 @@ - recreate_private_table.changed - recreate_private_table.route_table.id != create_public_table.route_table.id + - name: create a VPC endpoint to test ec2_vpc_route_table ignores it + ec2_vpc_endpoint: + state: present + vpc_id: "{{ vpc.vpc.id }}" + service: "com.amazonaws.{{ aws_region }}.s3" + route_table_ids: + - "{{ recreate_private_table.route_table.route_table_id }}" + <<: *aws_connection_info + register: vpc_endpoint + + - name: purge routes + ec2_vpc_route_table: + vpc_id: "{{ vpc.vpc.id }}" + tags: + Public: "false" + Name: "Private route table" + routes: + - nat_gateway_id: "{{ nat_gateway.nat_gateway_id }}" + dest: 0.0.0.0/0 + subnets: "{{ vpc_subnets|json_query('subnets[?tags.Public == `False`].id') }}" + purge_routes: true + <<: *aws_connection_info + register: result + + - name: Get endpoint facts to verify that it wasn't purged from the route table + ec2_vpc_endpoint_facts: + query: endpoints + vpc_endpoint_ids: + - "{{ vpc_endpoint.result.vpc_endpoint_id }}" + <<: *aws_connection_info + register: endpoint_details + + - name: assert the route table is associated with the VPC endpoint + assert: + that: + - endpoint_details.vpc_endpoints[0].route_table_ids[0] == recreate_private_table.route_table.route_table_id + always: ############################################################################# # TEAR DOWN STARTS HERE ############################################################################# + - name: remove the VPC endpoint + ec2_vpc_endpoint: + state: absent + vpc_endpoint_id: "{{ vpc_endpoint.result.vpc_endpoint_id }}" + <<: *aws_connection_info + when: vpc_endpoint is defined + ignore_errors: yes + - name: destroy route tables ec2_vpc_route_table: route_table_id: "{{ item.route_table.id }}"