From 12a4d8baea8095faf870403798261a1e36710662 Mon Sep 17 00:00:00 2001 From: Pierre Trespeuch Date: Thu, 19 Oct 2023 10:12:34 +0200 Subject: [PATCH] Fix backward incompatible change of VPC endpoint names The commit 2094dfc460d52776213ec13e5daaca2cbdcb56ed introduced a backward incompatible change as VPC endpoints names cannot be changed easily when redeploying a stack. Redeployment failed with the following kind of errors: "private-dns-enabled cannot be set because there is already a conflicting DNS domain for email-smtp.eu-west-1.amazonaws.com" This change reverts default VPC endpoint names and add an option to prefix them by the name of the VPC. --- src/e3/aws/troposphere/ec2/__init__.py | 17 +- .../tests_e3_aws/troposphere/ec2/ec2_test.py | 23 ++ tests/tests_e3_aws/troposphere/ec2/vpc.json | 12 +- .../ec2/vpc_ses_and_other_endpoints.json | 6 +- .../vpc_ses_and_other_endpoints_prefixed.json | 346 ++++++++++++++++++ .../troposphere/ec2/vpc_ses_endpoint.json | 2 +- 6 files changed, 395 insertions(+), 11 deletions(-) create mode 100644 tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints_prefixed.json diff --git a/src/e3/aws/troposphere/ec2/__init__.py b/src/e3/aws/troposphere/ec2/__init__.py index 48853dda..2bf849e4 100644 --- a/src/e3/aws/troposphere/ec2/__init__.py +++ b/src/e3/aws/troposphere/ec2/__init__.py @@ -102,6 +102,7 @@ def __init__( cidr_block: str, vpc: ec2.vpc, interface_endpoints: list[tuple[str, PolicyDocument | None]] | None = None, + vpc_prefixed_endpoints: bool | None = None, ) -> None: """Initialize VPCEndpointsSubnet Construct. @@ -111,11 +112,15 @@ def __init__( :param vpc: attach the subnet to this vpc :param interface_endpoint: list of (, ) tuples for each interface endpoint to create in the vpc endpoints subnet. + :param vpc_prefixed_endpoints: If True prefix endpoint names by VPC name. + This is to avoid conflicts in stacks with multiple VPCs with endpoints for + the same services. This is not the default for backward compatibility. """ self.name = name self.region = region self.cidr_block = cidr_block self.vpc = vpc + self.vpc_prefixed_endpoints = vpc_prefixed_endpoints self.has_ses_endpoint = False if interface_endpoints: @@ -220,9 +225,14 @@ def interface_vpc_endpoints(self) -> list[ec2.VPCEndpoint]: else: security_group_id = Ref(self.security_group) + if self.vpc_prefixed_endpoints: + endpoint_name = f"{self.vpc.name}-{service_name}Endpoint" + else: + endpoint_name = f"{service_name}Endpoint" + endpoints.append( ec2.VPCEndpoint( - name_to_id(f"{self.vpc.name}-{service_name}Endpoint"), + name_to_id(endpoint_name), PrivateDnsEnabled="true", SecurityGroupIds=[security_group_id], ServiceName=f"com.amazonaws.{self.region}.{service_name}", @@ -404,6 +414,7 @@ def __init__( s3_endpoint_policy_document: PolicyDocument | None = None, interface_endpoints: list[tuple[str, PolicyDocument | None]] | None = None, tags: dict[str, str] | None = None, + vpc_prefixed_endpoints: bool | None = None, ) -> None: """Initialize VPC Construct. @@ -424,6 +435,9 @@ def __init__( :param interface_endpoint: list of (, ) tuples for each interface endpoint to create in the vpc endpoints subnet. :param tags: tags for the VPC + :param vpc_prefixed_endpoints: It should be set to True if multiple VPCs in + the same stack have endpoints for the same services to avoid name + conflicts. It is None by default for backward compatibility. """ self.name = name self.region = region @@ -496,6 +510,7 @@ def __init__( cidr_block=vpc_endpoints_subnet_cidr_block, vpc=self.vpc, interface_endpoints=interface_endpoints, + vpc_prefixed_endpoints=vpc_prefixed_endpoints, ) @cached_property diff --git a/tests/tests_e3_aws/troposphere/ec2/ec2_test.py b/tests/tests_e3_aws/troposphere/ec2/ec2_test.py index 1a9de043..018d6761 100644 --- a/tests/tests_e3_aws/troposphere/ec2/ec2_test.py +++ b/tests/tests_e3_aws/troposphere/ec2/ec2_test.py @@ -130,3 +130,26 @@ def test_vpc_with_ses_and_other_endpoints(stack: Stack) -> None: expected_template = json.load(fd) assert stack.export()["Resources"] == expected_template + + +def test_vpc_with_vpc_prefixed_endpoints(stack: Stack) -> None: + """Test creation of a VPC with endpoints prefixed by vpc name.""" + vpc = VPC( + name="TestVPC", + region="eu-west-1", + nat_gateway=False, + interface_endpoints=[ + ("email-smtp", None), + ("logs", None), + ("sts", None), + ], + vpc_prefixed_endpoints=True, + ) + stack.add(vpc) + + with open( + os.path.join(TEST_DIR, "vpc_ses_and_other_endpoints_prefixed.json") + ) as fd: + expected_template = json.load(fd) + + assert stack.export()["Resources"] == expected_template diff --git a/tests/tests_e3_aws/troposphere/ec2/vpc.json b/tests/tests_e3_aws/troposphere/ec2/vpc.json index 6caf23d1..d5e256b9 100644 --- a/tests/tests_e3_aws/troposphere/ec2/vpc.json +++ b/tests/tests_e3_aws/troposphere/ec2/vpc.json @@ -242,7 +242,7 @@ }, "Type": "AWS::EC2::SecurityGroupIngress" }, - "TestVPCLogsEndpoint": { + "LogsEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -278,7 +278,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCEcrapiEndpoint": { + "EcrapiEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -314,7 +314,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCEcrdkrEndpoint": { + "EcrdkrEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -350,7 +350,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCStsEndpoint": { + "StsEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -371,7 +371,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCSecretsmanagerEndpoint": { + "SecretsmanagerEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -501,4 +501,4 @@ }, "Type": "AWS::EC2::SecurityGroup" } -} +} \ No newline at end of file diff --git a/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints.json b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints.json index 2080e5fa..f38b3cd9 100644 --- a/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints.json +++ b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints.json @@ -242,7 +242,7 @@ }, "Type": "AWS::EC2::SecurityGroupIngress" }, - "TestVPCEmailSmtpEndpoint": { + "EmailSmtpEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -263,7 +263,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCLogsEndpoint": { + "LogsEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ @@ -284,7 +284,7 @@ }, "Type": "AWS::EC2::VPCEndpoint" }, - "TestVPCStsEndpoint": { + "StsEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [ diff --git a/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints_prefixed.json b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints_prefixed.json new file mode 100644 index 00000000..2080e5fa --- /dev/null +++ b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_and_other_endpoints_prefixed.json @@ -0,0 +1,346 @@ +{ + "TestVPC": { + "Properties": { + "CidrBlock": "10.10.0.0/16", + "EnableDnsHostnames": true, + "EnableDnsSupport": true, + "Tags": [ + { + "Key": "Name", + "Value": "TestVPC" + } + ] + }, + "Type": "AWS::EC2::VPC" + }, + "TestVPCSecurityGroup": { + "Properties": { + "GroupDescription": "TestVPC main security group", + "SecurityGroupEgress": [], + "SecurityGroupIngress": [], + "VpcId": { + "Ref": "TestVPC" + }, + "Tags": [ + { + "Key": "Name", + "Value": "TestVPCSecurityGroup" + } + ] + }, + "Type": "AWS::EC2::SecurityGroup" + }, + "TestVPCPrivateSubnet": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + }, + "CidrBlock": "10.10.0.0/18", + "Tags": [ + { + "Key": "Name", + "Value": "TestVPCPrivateSubnet" + } + ], + "AvailabilityZone": "eu-west-1a" + }, + "Type": "AWS::EC2::Subnet" + }, + "TestVPCPrivateSubnetRouteTableAssoc": { + "Properties": { + "RouteTableId": { + "Ref": "TestVPCPrivateSubnetRouteTable" + }, + "SubnetId": { + "Ref": "TestVPCPrivateSubnet" + } + }, + "Type": "AWS::EC2::SubnetRouteTableAssociation" + }, + "TestVPCPrivateSubnetRouteTable": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::RouteTable" + }, + "TestVPCPrivateSubnetNATRoute": { + "Properties": { + "RouteTableId": { + "Ref": "TestVPCPrivateSubnetRouteTable" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "NatGatewayId": { + "Ref": "TestVPCPublicSubnetNAT" + } + }, + "Type": "AWS::EC2::Route" + }, + "TestVPCPublicSubnet": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + }, + "CidrBlock": "10.10.64.0/18", + "Tags": [ + { + "Key": "Name", + "Value": "TestVPCPublicSubnet" + } + ], + "AvailabilityZone": "eu-west-1a" + }, + "Type": "AWS::EC2::Subnet" + }, + "TestVPCPublicSubnetRouteTableAssoc": { + "Properties": { + "RouteTableId": { + "Ref": "TestVPCPublicSubnetsRouteTable" + }, + "SubnetId": { + "Ref": "TestVPCPublicSubnet" + } + }, + "Type": "AWS::EC2::SubnetRouteTableAssociation" + }, + "TestVPCPublicSubnetNAT": { + "Properties": { + "AllocationId": { + "Fn::GetAtt": [ + "TestVPCPublicSubnetEIP", + "AllocationId" + ] + }, + "SubnetId": { + "Ref": "TestVPCPublicSubnet" + } + }, + "Type": "AWS::EC2::NatGateway" + }, + "TestVPCPublicSubnetEIP": { + "Type": "AWS::EC2::EIP" + }, + "TestVPCPublicSubnetsRouteTable": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::RouteTable" + }, + "TestVPCSecondaryPublicSubnet": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + }, + "CidrBlock": "10.10.128.0/18", + "Tags": [ + { + "Key": "Name", + "Value": "TestVPCSecondaryPublicSubnet" + } + ], + "AvailabilityZone": "eu-west-1b" + }, + "Type": "AWS::EC2::Subnet" + }, + "TestVPCSecondaryPublicSubnetRouteTableAssoc": { + "Properties": { + "RouteTableId": { + "Ref": "TestVPCPublicSubnetsRouteTable" + }, + "SubnetId": { + "Ref": "TestVPCSecondaryPublicSubnet" + } + }, + "Type": "AWS::EC2::SubnetRouteTableAssociation" + }, + "TestVPCIgw": { + "Type": "AWS::EC2::InternetGateway" + }, + "TestVPCIgwAttachement": { + "Properties": { + "InternetGatewayId": { + "Ref": "TestVPCIgw" + }, + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::VPCGatewayAttachment" + }, + "TestVPCIgwRoute": { + "Properties": { + "RouteTableId": { + "Ref": "TestVPCPublicSubnetsRouteTable" + }, + "DestinationCidrBlock": "0.0.0.0/0", + "GatewayId": { + "Ref": "TestVPCIgw" + } + }, + "Type": "AWS::EC2::Route" + }, + "TestVPCVPCEndpointsSubnetSubnet": { + "Properties": { + "VpcId": { + "Ref": "TestVPC" + }, + "CidrBlock": "10.10.192.0/18", + "Tags": [ + { + "Key": "Name", + "Value": "TestVPCVPCEndpointsSubnetSubnet" + } + ] + }, + "Type": "AWS::EC2::Subnet" + }, + "TestVPCVPCEndpointsSubnetSecurityGroup": { + "Properties": { + "GroupDescription": "TestVPCVPCEndpointsSubnet vpc endpoints security group", + "SecurityGroupEgress": [], + "SecurityGroupIngress": [], + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::SecurityGroup" + }, + "TestVPCVPCEndpointsSubnetDefaultEgress": { + "Properties": { + "CidrIp": "10.10.192.0/18", + "IpProtocol": "-1", + "GroupId": { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + } + }, + "Type": "AWS::EC2::SecurityGroupEgress" + }, + "TestVPCVPCEndpointsSubnetEgressToVPC": { + "Properties": { + "CidrIp": "10.10.0.0/16", + "FromPort": "443", + "ToPort": "443", + "IpProtocol": "tcp", + "GroupId": { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + } + }, + "Type": "AWS::EC2::SecurityGroupEgress" + }, + "TestVPCVPCEndpointsSubnetIngressFromVPC": { + "Properties": { + "CidrIp": "10.10.0.0/16", + "FromPort": "443", + "ToPort": "443", + "IpProtocol": "tcp", + "GroupId": { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + } + }, + "Type": "AWS::EC2::SecurityGroupIngress" + }, + "TestVPCEmailSmtpEndpoint": { + "Properties": { + "PrivateDnsEnabled": true, + "SecurityGroupIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSESSecurityGroup" + } + ], + "ServiceName": "com.amazonaws.eu-west-1.email-smtp", + "SubnetIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSubnet" + } + ], + "VpcEndpointType": "Interface", + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::VPCEndpoint" + }, + "TestVPCLogsEndpoint": { + "Properties": { + "PrivateDnsEnabled": true, + "SecurityGroupIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + } + ], + "ServiceName": "com.amazonaws.eu-west-1.logs", + "SubnetIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSubnet" + } + ], + "VpcEndpointType": "Interface", + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::VPCEndpoint" + }, + "TestVPCStsEndpoint": { + "Properties": { + "PrivateDnsEnabled": true, + "SecurityGroupIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + } + ], + "ServiceName": "com.amazonaws.eu-west-1.sts", + "SubnetIds": [ + { + "Ref": "TestVPCVPCEndpointsSubnetSubnet" + } + ], + "VpcEndpointType": "Interface", + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::VPCEndpoint" + }, + "TestVPCVPCEndpointsSubnetSESSecurityGroup": { + "Properties": { + "GroupDescription": "TestVPCVPCEndpointsSubnet SES vpc endpoint security group", + "SecurityGroupEgress": [ + { + "CidrIp": "10.10.0.0/16", + "IpProtocol": "-1" + } + ], + "SecurityGroupIngress": [ + { + "CidrIp": "10.10.0.0/16", + "FromPort": "587", + "ToPort": "587", + "IpProtocol": "tcp" + } + ], + "VpcId": { + "Ref": "TestVPC" + } + }, + "Type": "AWS::EC2::SecurityGroup" + }, + "TestVPCEndpointsEgress": { + "Properties": { + "DestinationSecurityGroupId": { + "Ref": "TestVPCVPCEndpointsSubnetSecurityGroup" + }, + "Description": "Allows traffic to the subnet holding VPC interface endpoints", + "FromPort": "443", + "ToPort": "443", + "IpProtocol": "tcp", + "GroupId": { + "Ref": "TestVPCSecurityGroup" + } + }, + "Type": "AWS::EC2::SecurityGroupEgress" + } +} diff --git a/tests/tests_e3_aws/troposphere/ec2/vpc_ses_endpoint.json b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_endpoint.json index a42fcfe5..e4dcd1a1 100644 --- a/tests/tests_e3_aws/troposphere/ec2/vpc_ses_endpoint.json +++ b/tests/tests_e3_aws/troposphere/ec2/vpc_ses_endpoint.json @@ -242,7 +242,7 @@ }, "Type": "AWS::EC2::SecurityGroupIngress" }, - "TestVPCEmailSmtpEndpoint": { + "EmailSmtpEndpoint": { "Properties": { "PrivateDnsEnabled": true, "SecurityGroupIds": [