diff --git a/awslimitchecker/services/vpc.py b/awslimitchecker/services/vpc.py index 33d5dcce..66fd5f69 100644 --- a/awslimitchecker/services/vpc.py +++ b/awslimitchecker/services/vpc.py @@ -38,8 +38,6 @@ """ import abc # noqa -import boto -import boto.vpc import logging from collections import defaultdict @@ -58,10 +56,8 @@ def connect(self): """Connect to API if not already connected; set self.conn.""" if self.conn is not None: return - elif self.region: - self.conn = self.connect_via(boto.vpc.connect_to_region) else: - self.conn = boto.connect_vpc() + self.conn = self.connect_client('ec2') def find_usage(self): """ @@ -84,9 +80,10 @@ def find_usage(self): def _find_usage_vpcs(self): """find usage for VPCs""" # overall number of VPCs - vpcs = boto_query_wrapper(self.conn.get_all_vpcs) + vpcs = boto_query_wrapper(self.conn.describe_vpcs, + alc_no_paginate=True) self.limits['VPCs']._add_current_usage( - len(vpcs), + len(vpcs['Vpcs']), aws_type='AWS::EC2::VPC' ) @@ -94,9 +91,9 @@ def _find_usage_subnets(self): """find usage for Subnets""" # subnets per VPC subnets = defaultdict(int) - for subnet in boto_query_wrapper(self.conn.get_all_subnets): - # boto.vpc.subnet.Subnet - subnets[subnet.vpc_id] += 1 + for subnet in boto_query_wrapper(self.conn.describe_subnets, + alc_no_paginate=True)['Subnets']: + subnets[subnet['VpcId']] += 1 for vpc_id in subnets: self.limits['Subnets per VPC']._add_current_usage( subnets[vpc_id], @@ -108,14 +105,15 @@ def _find_usage_ACLs(self): """find usage for ACLs""" # Network ACLs per VPC acls = defaultdict(int) - for acl in boto_query_wrapper(self.conn.get_all_network_acls): - # boto.vpc.networkacl.NetworkAcl - acls[acl.vpc_id] += 1 + result = boto_query_wrapper(self.conn.describe_network_acls, + alc_no_paginate=True) + for acl in result['NetworkAcls']: + acls[acl['VpcId']] += 1 # Rules per network ACL self.limits['Rules per network ACL']._add_current_usage( - len(acl.network_acl_entries), + len(acl['Entries']), aws_type='AWS::EC2::NetworkAcl', - resource_id=acl.id + resource_id=acl['NetworkAclId'] ) for vpc_id in acls: self.limits['Network ACLs per VPC']._add_current_usage( @@ -128,14 +126,15 @@ def _find_usage_route_tables(self): """find usage for route tables""" # Route tables per VPC tables = defaultdict(int) - for table in boto_query_wrapper(self.conn.get_all_route_tables): - # boto.vpc.routetable.RouteTable - tables[table.vpc_id] += 1 + result = boto_query_wrapper(self.conn.describe_route_tables, + alc_no_paginate=True) + for table in result['RouteTables']: + tables[table['VpcId']] += 1 # Entries per route table self.limits['Entries per route table']._add_current_usage( - len(table.routes), + len(table['Routes']), aws_type='AWS::EC2::RouteTable', - resource_id=table.id + resource_id=table['RouteTableId'] ) for vpc_id in tables: self.limits['Route tables per VPC']._add_current_usage( @@ -147,9 +146,10 @@ def _find_usage_route_tables(self): def _find_usage_gateways(self): """find usage for Internet Gateways""" # Internet gateways - gws = boto_query_wrapper(self.conn.get_all_internet_gateways) + gws = boto_query_wrapper(self.conn.describe_internet_gateways, + alc_no_paginate=True) self.limits['Internet gateways']._add_current_usage( - len(gws), + len(gws['InternetGateways']), aws_type='AWS::EC2::InternetGateway', ) diff --git a/awslimitchecker/tests/services/test_vpc.py b/awslimitchecker/tests/services/test_vpc.py index de5d94e5..15b4ef4e 100644 --- a/awslimitchecker/tests/services/test_vpc.py +++ b/awslimitchecker/tests/services/test_vpc.py @@ -39,14 +39,6 @@ import sys -from boto.vpc import VPCConnection -from boto.vpc.vpc import VPC -from boto.vpc.subnet import Subnet -from boto.vpc.networkacl import NetworkAcl -from boto.vpc.routetable import RouteTable -from boto.vpc.internetgateway import InternetGateway -from boto.vpc import connect_to_region - from awslimitchecker.services.vpc import _VpcService # https://code.google.com/p/mock/issues/detail?id=249 @@ -76,50 +68,24 @@ def test_init(self): def test_connect(self): """test connect()""" mock_conn = Mock() - mock_conn_via = Mock() cls = _VpcService(21, 43) - with patch('%s.boto.connect_vpc' % self.pbm) as mock_vpc: - with patch('%s.connect_via' % self.pb) as mock_connect_via: - mock_vpc.return_value = mock_conn - mock_connect_via.return_value = mock_conn_via + with patch('%s.connect_client' % self.pb) as mock_connect_client: + mock_connect_client.return_value = mock_conn cls.connect() - assert mock_vpc.mock_calls == [call()] assert mock_conn.mock_calls == [] - assert mock_connect_via.mock_calls == [] + assert mock_connect_client.mock_calls == [call('ec2')] assert cls.conn == mock_conn - def test_connect_region(self): - """test connect()""" - mock_conn = Mock() - mock_conn_via = Mock() - cls = _VpcService(21, 43, region='foo') - with patch('%s.boto.connect_vpc' % self.pbm) as mock_vpc: - with patch('%s.connect_via' % self.pb) as mock_connect_via: - mock_vpc.return_value = mock_conn - mock_connect_via.return_value = mock_conn_via - cls.connect() - assert mock_vpc.mock_calls == [] - assert mock_conn.mock_calls == [] - assert mock_connect_via.mock_calls == [ - call(connect_to_region) - ] - assert cls.conn == mock_conn_via - def test_connect_again(self): - """test connect()""" + """make sure we re-use the connection""" mock_conn = Mock() - mock_conn_via = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn - with patch('%s.boto.connect_vpc' % self.pbm) as mock_vpc: - with patch('%s.connect_via' % self.pb) as mock_connect_via: - mock_vpc.return_value = mock_conn - mock_connect_via.return_value = mock_conn_via + with patch('%s.connect_client' % self.pb) as mock_connect_client: + mock_connect_client.return_value = mock_conn cls.connect() - assert mock_vpc.mock_calls == [] assert mock_conn.mock_calls == [] - assert mock_connect_via.mock_calls == [] - assert cls.conn == mock_conn + assert mock_connect_client.mock_calls == [] def test_get_limits(self): cls = _VpcService(21, 43) @@ -148,7 +114,7 @@ def test_get_limits_again(self): assert res == mock_limits def test_find_usage(self): - mock_conn = Mock(spec_set=VPCConnection) + mock_conn = Mock() with patch('%s.connect' % self.pb) as mock_connect: with patch.multiple( @@ -176,44 +142,72 @@ def test_find_usage(self): assert mocks[x].mock_calls == [call()] def test_find_usage_vpcs(self): - mock1 = Mock(spec_set=VPC) - type(mock1).id = 'vpc-1' - mock2 = Mock(spec_set=VPC) - type(mock2).id = 'vpc-2' + response = { + 'Vpcs': [ + { + 'VpcId': 'vpc-1', + 'State': 'available', + 'CidrBlock': 'string', + 'DhcpOptionsId': 'string', + 'Tags': [ + { + 'Key': 'fooTag', + 'Value': 'fooVal' + }, + ], + 'InstanceTenancy': 'default', + 'IsDefault': False + }, + {'VpcId': 'vpc-2'}, + ] + } - vpcs = [mock1, mock2] - - mock_conn = Mock(spec_set=VPCConnection) + mock_conn = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn with patch('%s.boto_query_wrapper' % self.pbm) as mock_wrapper: - mock_wrapper.return_value = vpcs + mock_wrapper.return_value = response cls._find_usage_vpcs() assert len(cls.limits['VPCs'].get_current_usage()) == 1 assert cls.limits['VPCs'].get_current_usage()[0].get_value() == 2 assert mock_conn.mock_calls == [] assert mock_wrapper.mock_calls == [ - call(mock_conn.get_all_vpcs) + call(mock_conn.describe_vpcs, alc_no_paginate=True) ] def test_find_usage_subnets(self): - mock1 = Mock(spec_set=Subnet) - type(mock1).vpc_id = 'vpc-1' - mock2 = Mock(spec_set=Subnet) - type(mock2).vpc_id = 'vpc-1' - mock3 = Mock(spec_set=Subnet) - type(mock3).vpc_id = 'vpc-2' - - subnets = [mock1, mock2, mock3] - mock_conn = Mock(spec_set=VPCConnection) + response = { + 'Subnets': [ + { + 'SubnetId': 'string', + 'State': 'available', + 'VpcId': 'vpc-1', + 'CidrBlock': 'string', + 'AvailableIpAddressCount': 123, + 'AvailabilityZone': 'string', + 'DefaultForAz': False, + 'MapPublicIpOnLaunch': True, + 'Tags': [ + { + 'Key': 'tagKey', + 'Value': 'tagVal' + }, + ] + }, + {'VpcId': 'vpc-1'}, + {'VpcId': 'vpc-2'}, + ] + } + + mock_conn = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn with patch('%s.boto_query_wrapper' % self.pbm) as mock_wrapper: - mock_wrapper.return_value = subnets + mock_wrapper.return_value = response cls._find_usage_subnets() usage = sorted(cls.limits['Subnets per VPC'].get_current_usage()) @@ -224,31 +218,97 @@ def test_find_usage_subnets(self): assert usage[1].resource_id == 'vpc-1' assert mock_conn.mock_calls == [] assert mock_wrapper.mock_calls == [ - call(mock_conn.get_all_subnets) + call(mock_conn.describe_subnets, alc_no_paginate=True) ] def test_find_usage_acls(self): - mock1 = Mock(spec_set=NetworkAcl) - type(mock1).id = 'acl-1' - type(mock1).vpc_id = 'vpc-1' - type(mock1).network_acl_entries = [1, 2, 3] - mock2 = Mock(spec_set=NetworkAcl) - type(mock2).id = 'acl-2' - type(mock2).vpc_id = 'vpc-1' - type(mock2).network_acl_entries = [1] - mock3 = Mock(spec_set=NetworkAcl) - type(mock3).id = 'acl-3' - type(mock3).vpc_id = 'vpc-2' - type(mock3).network_acl_entries = [1, 2, 3, 4, 5] - - acls = [mock1, mock2, mock3] - mock_conn = Mock(spec_set=VPCConnection) + response = { + 'NetworkAcls': [ + { + 'NetworkAclId': 'acl-1', + 'VpcId': 'vpc-1', + 'IsDefault': True, + 'Entries': [ + { + 'RuleNumber': 123, + 'Protocol': 'string', + 'RuleAction': 'allow', + 'Egress': True, + 'CidrBlock': 'string', + 'IcmpTypeCode': { + 'Type': 123, + 'Code': 123 + }, + 'PortRange': { + 'From': 123, + 'To': 123 + } + }, + { + 'RuleNumber': 124, + 'Protocol': 'string', + 'RuleAction': 'allow', + 'Egress': False, + 'CidrBlock': 'string', + 'IcmpTypeCode': { + 'Type': 123, + 'Code': 123 + }, + 'PortRange': { + 'From': 124, + 'To': 124 + } + }, + { + 'RuleNumber': 125, + 'Protocol': 'string', + 'RuleAction': 'deny', + 'Egress': False, + 'CidrBlock': 'string', + 'IcmpTypeCode': { + 'Type': 123, + 'Code': 123 + }, + 'PortRange': { + 'From': 125, + 'To': 125 + } + }, + ], + 'Associations': [ + { + 'NetworkAclAssociationId': 'string', + 'NetworkAclId': 'string', + 'SubnetId': 'string' + }, + ], + 'Tags': [ + { + 'Key': 'tagKey', + 'Value': 'tagVal' + }, + ] + }, + { + 'NetworkAclId': 'acl-2', + 'VpcId': 'vpc-1', + 'Entries': [1], + }, + { + 'NetworkAclId': 'acl-3', + 'VpcId': 'vpc-2', + 'Entries': [1, 2, 3, 4, 5], + }, + ] + } + + mock_conn = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn with patch('%s.boto_query_wrapper' % self.pbm) as mock_wrapper: - mock_wrapper.return_value = acls + mock_wrapper.return_value = response cls._find_usage_ACLs() usage = sorted(cls.limits['Network ACLs per VPC'].get_current_usage()) @@ -268,32 +328,79 @@ def test_find_usage_acls(self): assert entries[2].get_value() == 5 assert mock_conn.mock_calls == [] assert mock_wrapper.mock_calls == [ - call(mock_conn.get_all_network_acls) + call(mock_conn.describe_network_acls, alc_no_paginate=True) ] def test_find_usage_route_tables(self): - mock1 = Mock(spec_set=RouteTable) - type(mock1).id = 'rt-1' - type(mock1).vpc_id = 'vpc-1' - type(mock1).routes = [1, 2, 3] - mock2 = Mock(spec_set=RouteTable) - type(mock2).id = 'rt-2' - type(mock2).vpc_id = 'vpc-1' - type(mock2).routes = [1] - mock3 = Mock(spec_set=RouteTable) - type(mock3).id = 'rt-3' - type(mock3).vpc_id = 'vpc-2' - type(mock3).routes = [1, 2, 3, 4, 5] - - tables = [mock1, mock2, mock3] - - mock_conn = Mock(spec_set=VPCConnection) + response = { + 'RouteTables': [ + { + 'RouteTableId': 'rt-1', + 'VpcId': 'vpc-1', + 'Routes': [ + { + 'DestinationCidrBlock': 'string', + 'DestinationPrefixListId': 'string', + 'GatewayId': 'string', + 'InstanceId': 'string', + 'InstanceOwnerId': 'string', + 'NetworkInterfaceId': 'string', + 'VpcPeeringConnectionId': 'string', + 'NatGatewayId': 'string', + 'State': 'active', + 'Origin': 'CreateRouteTable' + }, + {'foo': 'bar', 'baz': 'blam'}, + {'foo': 'bar', 'baz': 'blam'}, + ], + 'Associations': [ + { + 'RouteTableAssociationId': 'string', + 'RouteTableId': 'string', + 'SubnetId': 'string', + 'Main': True + }, + ], + 'Tags': [ + { + 'Key': 'tagKey', + 'Value': 'tagVal' + }, + ], + 'PropagatingVgws': [ + { + 'GatewayId': 'string' + }, + ] + }, + { + 'RouteTableId': 'rt-2', + 'VpcId': 'vpc-1', + 'Routes': [ + {'foo': 'bar', 'baz': 'blam'}, + ], + }, + { + 'RouteTableId': 'rt-3', + 'VpcId': 'vpc-2', + 'Routes': [ + {'foo': 'bar', 'baz': 'blam'}, + {'foo': 'bar', 'baz': 'blam'}, + {'foo': 'bar', 'baz': 'blam'}, + {'foo': 'bar', 'baz': 'blam'}, + {'foo': 'bar', 'baz': 'blam'}, + ], + } + ] + } + + mock_conn = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn with patch('%s.boto_query_wrapper' % self.pbm) as mock_wrapper: - mock_wrapper.return_value = tables + mock_wrapper.return_value = response cls._find_usage_route_tables() usage = sorted(cls.limits['Route tables per VPC'].get_current_usage()) @@ -313,24 +420,38 @@ def test_find_usage_route_tables(self): assert entries[2].get_value() == 5 assert mock_conn.mock_calls == [] assert mock_wrapper.mock_calls == [ - call(mock_conn.get_all_route_tables) + call(mock_conn.describe_route_tables, alc_no_paginate=True) ] def test_find_usage_internet_gateways(self): - mock1 = Mock(spec_set=InternetGateway) - type(mock1).id = 'gw-1' - mock2 = Mock(spec_set=InternetGateway) - type(mock2).id = 'gw-2' + response = { + 'InternetGateways': [ + { + 'InternetGatewayId': 'gw-1', + 'Attachments': [ + { + 'VpcId': 'string', + 'State': 'attached' + }, + ], + 'Tags': [ + { + 'Key': 'tagKey', + 'Value': 'tagVal' + }, + ] + }, + {'InternetGatewayId': 'gw-2'} + ] + } - gateways = [mock1, mock2] - - mock_conn = Mock(spec_set=VPCConnection) + mock_conn = Mock() cls = _VpcService(21, 43) cls.conn = mock_conn with patch('%s.boto_query_wrapper' % self.pbm) as mock_wrapper: - mock_wrapper.return_value = gateways + mock_wrapper.return_value = response cls._find_usage_gateways() assert len(cls.limits['Internet gateways'].get_current_usage()) == 1 @@ -338,7 +459,7 @@ def test_find_usage_internet_gateways(self): 0].get_value() == 2 assert mock_conn.mock_calls == [] assert mock_wrapper.mock_calls == [ - call(mock_conn.get_all_internet_gateways) + call(mock_conn.describe_internet_gateways, alc_no_paginate=True) ] def test_required_iam_permissions(self): diff --git a/awslimitchecker/tests/test_trustedadvisor.py b/awslimitchecker/tests/test_trustedadvisor.py index d063c1d1..552cd46e 100644 --- a/awslimitchecker/tests/test_trustedadvisor.py +++ b/awslimitchecker/tests/test_trustedadvisor.py @@ -530,3 +530,38 @@ def se_set(lname, val): call._set_ta_limit('blam', 10), call._set_ta_limit('VPC Elastic IP addresses (EIPs)', 11) ] + + def test_update_services_no_ec2(self): + + mock_autoscale = Mock(spec_set=_AwsService) + mock_vpc = Mock(spec_set=_AwsService) + services = { + 'AutoScaling': mock_autoscale, + 'VPC': mock_vpc, + } + ta_results = { + 'AutoScaling': { + 'foo': 20, + 'bar': 40, + }, + 'EC2': { + 'baz': 5, + }, + 'VPC': { + 'VPC Elastic IP addresses (EIPs)': 11, + } + } + with patch('awslimitchecker.trustedadvisor' + '.logger', autospec=True) as mock_logger: + self.cls._update_services(ta_results, services) + assert mock_logger.mock_calls == [ + call.debug("Updating TA limits on all services"), + call.info("TrustedAdvisor returned check results for unknown " + "service '%s'", 'EC2'), + call.info("Done updating TA limits on all services"), + ] + assert mock_autoscale.mock_calls == [ + call._set_ta_limit('bar', 40), + call._set_ta_limit('foo', 20), + ] + assert mock_vpc.mock_calls == [] diff --git a/awslimitchecker/trustedadvisor.py b/awslimitchecker/trustedadvisor.py index a6b58105..b3aa5fee 100644 --- a/awslimitchecker/trustedadvisor.py +++ b/awslimitchecker/trustedadvisor.py @@ -224,9 +224,13 @@ def _update_services(self, ta_results, services): # better way of handling this - maybe with a mapping if svc_name == 'VPC' and lim_name == 'VPC Elastic IP ' \ 'addresses (EIPs)': - services['EC2']._set_ta_limit( - lim_name, limits[lim_name] - ) + # this limit belongs under EC2, but if we're only + # checking a specific list of services that doesn't + # include EC2, we don't want to set it. + if 'EC2' in services: + services['EC2']._set_ta_limit( + lim_name, limits[lim_name] + ) else: service._set_ta_limit(lim_name, limits[lim_name]) except ValueError: