Merge pull request #508 from /issues/498

Fixes #498 - Fix VPC security group limits

CHANGES.rst

@@ -6,11 +6,16 @@ Changelog
 10.0.0 (2020-12-07)
 -------------------
 
-**Important:** This release makes significant changes to how Trusted Advisor is used; see below.
+IMPORTANT - Breaking Changes
+++++++++++++++++++++++++++++
 
-**Important:** This release requires the following new IAM permissions: ``eks:ListClusters``, ``eks:DescribeCluster``, ``eks:ListNodegroups``, ``eks:ListFargateProfiles``, ``eks:DescribeFargateProfile``, ``kinesis:DescribeLimits``.
+* This release makes significant changes to how Trusted Advisor is used; see below.
+* This release requires the following new IAM permissions: ``eks:ListClusters``, ``eks:DescribeCluster``, ``eks:ListNodegroups``, ``eks:ListFargateProfiles``, ``eks:DescribeFargateProfile``, ``kinesis:DescribeLimits``.
+* This release introduces a number of new limits, as well as new services. Please see below for details.
+* This release **removes** the ``EC2/Security groups per VPC`` limit, which no longer exists, and adds the new ``EC2/VPC security groups per Region`` limit.
 
-**Important:** This release introduces a number of new limits, as well as new services. Please see below for details.
+All Changes
++++++++++++
 
 * `Issue #466 <https://github.com/jantman/awslimitchecker/issues/466>`__ - **Significant** changes to Trusted Advisor support.
 
@@ -26,6 +31,11 @@ Changelog
 * `Issue #472 <https://github.com/jantman/awslimitchecker/issues/472>`__ / `PR #494 <https://github.com/jantman/awslimitchecker/pull/494>`__ - Add support for the ``EKS`` service, and 8 new limits for it. Thanks to `sebasrp <https://github.com/sebasrp>`__ for this contribution!
 * `Issue #495 <https://github.com/jantman/awslimitchecker/issues/495>`__ / `PR #496 <https://github.com/jantman/awslimitchecker/pull/496>`__ - Add support for the ``Kinesis`` service, and one new limit for it. Thanks to `sebasrp <https://github.com/sebasrp>`__ for this contribution!
 * `PR #499 <https://github.com/jantman/awslimitchecker/pull/499>`__ - Set quota_name for VPC "Entries per route table" limit, so that the current limit will be automatically retrieved from Service Quotas. Thanks to `patuck <https://github.com/patuck>`__ for this contribution!
+* `Issue #498 <https://github.com/jantman/awslimitchecker/issues/498>`__ - Fix multiple issues relating to VPC limits:
+
+  * Update the EC2 / ``Rules per VPC security group`` limit to support retrieving the current limit value from Service Quotas.
+  * Remove the ``EC2/Security groups per VPC`` limit, which no longer exists.
+  * Add the new ``EC2/VPC security groups per Region`` limit.
 
 .. _changelog.9_0_0:
 

awslimitchecker/services/ec2.py

@@ -640,12 +640,12 @@ def _get_limits_spot(self):
     def _find_usage_networking_sgs(self):
         """calculate usage for VPC-related things"""
         logger.debug("Getting usage for EC2 VPC resources")
-        sgs_per_vpc = defaultdict(int)
+        sg_count = 0
         rules_per_sg = defaultdict(int)
         for sg in self.resource_conn.security_groups.all():
             if sg.vpc_id is None:
                 continue
-            sgs_per_vpc[sg.vpc_id] += 1
+            sg_count += 1
             """
             see: https://github.com/jantman/awslimitchecker/issues/431
 
@@ -676,12 +676,10 @@ def _find_usage_networking_sgs(self):
                 )
             rules_per_sg[sg.id] = max(counts)
         # set usage
-        for vpc_id, count in sgs_per_vpc.items():
-            self.limits['Security groups per VPC']._add_current_usage(
-                count,
-                aws_type='AWS::EC2::VPC',
-                resource_id=vpc_id,
-            )
+        self.limits['VPC security groups per Region']._add_current_usage(
+            sg_count,
+            aws_type='AWS::EC2::SecurityGroup',
+        )
         for sg_id, count in rules_per_sg.items():
             self.limits['Rules per VPC security group']._add_current_usage(
                 count,
@@ -727,14 +725,16 @@ def _get_limits_networking(self):
         :rtype: dict
         """
         limits = {}
-        limits['Security groups per VPC'] = AwsLimit(
-            'Security groups per VPC',
+        limits['VPC security groups per Region'] = AwsLimit(
+            'VPC security groups per Region',
             self,
-            500,
+            2500,
             self.warning_threshold,
             self.critical_threshold,
             limit_type='AWS::EC2::SecurityGroup',
             limit_subtype='AWS::EC2::VPC',
+            quotas_name='VPC security groups per Region',
+            quotas_service_code='vpc'
         )
         limits['Rules per VPC security group'] = AwsLimit(
             'Rules per VPC security group',
@@ -744,6 +744,8 @@ def _get_limits_networking(self):
             self.critical_threshold,
             limit_type='AWS::EC2::SecurityGroup',
             limit_subtype='AWS::EC2::VPC',
+            quotas_name='Inbound or outbound rules per security group',
+            quotas_service_code='vpc'
         )
         limits['VPC Elastic IP addresses (EIPs)'] = AwsLimit(
             'VPC Elastic IP addresses (EIPs)',

awslimitchecker/tests/services/test_ec2.py

@@ -777,18 +777,13 @@ def test_simple(self):
         assert mock_logger.mock_calls == [
             call.debug("Getting usage for EC2 VPC resources"),
         ]
-        limit = cls.limits['Security groups per VPC']
+        limit = cls.limits['VPC security groups per Region']
         # relies on AwsLimitUsage sorting by numeric usage value
         sorted_usage = sorted(limit.get_current_usage())
-        assert len(sorted_usage) == 2
+        assert len(sorted_usage) == 1
         assert sorted_usage[0].limit == limit
-        assert sorted_usage[0].get_value() == 1
-        assert sorted_usage[0].resource_id == 'vpc-bbb'
-        assert sorted_usage[0].aws_type == 'AWS::EC2::VPC'
-        assert sorted_usage[1].limit == limit
-        assert sorted_usage[1].get_value() == 2
-        assert sorted_usage[1].resource_id == 'vpc-aaa'
-        assert sorted_usage[1].aws_type == 'AWS::EC2::VPC'
+        assert sorted_usage[0].get_value() == 3
+        assert sorted_usage[0].aws_type == 'AWS::EC2::SecurityGroup'
 
         limit = cls.limits['Rules per VPC security group']
         sorted_usage = sorted(limit.get_current_usage())
@@ -886,7 +881,7 @@ def test_simple(self):
         cls = _Ec2Service(21, 43, {}, None)
         limits = cls._get_limits_networking()
         expected = [
-            'Security groups per VPC',
+            'VPC security groups per Region',
             'Rules per VPC security group',
             'VPC Elastic IP addresses (EIPs)',
             'Elastic IP addresses (EIPs)',
@@ -913,6 +908,10 @@ def test_simple(self):
         assert limits[
             'Elastic IP addresses (EIPs)'
         ].quotas_unit == 'None'
+        vpcsg = limits['VPC security groups per Region']
+        assert vpcsg.quota_name == 'VPC security groups per Region'
+        assert vpcsg.quotas_service_code == 'vpc'
+        assert vpcsg.default_limit == 2500
 
 
 class TestGetLimitsSpot(object):