Repository: localstack/localstack
Pull request: Fix subnet tags going missing
<!-- Please refer to the contribution guidelines before raising a PR: https://github.com/localstack/localstack/blob/master/docs/CONTRIBUTING.md -->

<!-- Why am I raising this PR? Add context such as related issues, PRs, or documentation. -->
## Motivation
#11853 introduced a regression whereby the structure of the tags dict was altered before passing through to moto.  This was reported in #12377 and is fixed by this PR.

Whilst fixing this PR, I noticed that the tags were also lost when creating a subnet, SG or VPC with a custom ID.

<!-- What changes does this PR make? How does LocalStack behave differently now? -->
## Changes
* Pass the original tags dict through to moto on subnet creation, rather than the modified one.
* Because tags are not stored within the subnet, VPC or SG objects within moto, but instead in a separate tags dict, the tags needed to be reindexed in that dict when using a custom id.

<!-- Optional section: How to test these changes? -->
<!--
## Testing
* Tested subnet change manually using the AWS CLI.
* Added UTs.
-->

<!-- Optional section: What's left to do before it can be merged? -->
<!--
## TODO

What's left to do:

- [ ] ...
- [ ] ...
-->


Problem statement:
LocalStack does not detect subnet tags with Terraform
### Description:

**Problem Description:**  
I am using **LocalStack** version `4.2` to perform AWS infrastructure testing with **Terraform**. However, when creating subnets in LocalStack, the **tags** are not correctly applied to the subnets, even though the tags are properly defined in Terraform.

---

### Steps to Reproduce:

1. I created a VPC in LocalStack.
2. I tried to create private subnets with tags using Terraform.
3. I used the following Terraform code to define the private subnets:

   ```hcl
   resource "aws_subnet" "private" {
     count             = var.az_count
     cidr_block        = cidrsubnet(aws_vpc.main.cidr_block, 8, count.index)
     availability_zone = data.aws_availability_zones.available.names[count.index]
     vpc_id            = aws_vpc.main.id
     tags = {
       Domain      = "example.com"
       Environment = "shared"
       Application = "example-com"
       Name        = "ts-sub-net-priv-prod"
     }
   }

### Output of the created subnets
```json private_subnet_ids = [
  {
    "arn": "arn:aws:ec2:us-east-1:000000000000:subnet/subnet-ad9e8d62f9513fef3",
    "assign_ipv6_address_on_creation": false,
    "availability_zone": "us-east-1a",
    "availability_zone_id": "use1-az6",
    "cidr_block": "172.17.0.0/24",
    "customer_owned_ipv4_pool": "",
    "enable_dns64": false,
    "enable_lni_at_device_index": 0,
    "enable_resource_name_dns_a_record_on_launch": false,
    "enable_resource_name_dns_aaaa_record_on_launch": false,
    "id": "subnet-ad9e8d62f9513fef3",
    "ipv6_cidr_block": "",
    "ipv6_cidr_block_association_id": "",
    "ipv6_native": false,
    "map_customer_owned_ip_on_launch": false,
    "map_public_ip_on_launch": false,
    "outpost_arn": "",
    "owner_id": "000000000000",
    "private_dns_hostname_type_on_launch": "ip-name",
    "tags": {},
    "tags_all": {},
    "timeouts": null,
    "vpc_id": "vpc-2accbbae3126d386c"
  },
  {
    "arn": "arn:aws:ec2:us-east-1:000000000000:subnet/subnet-9d6e838804f9b548a",
    "assign_ipv6_address_on_creation": false,
    "availability_zone": "us-east-1b",
    "availability_zone_id": "use1-az1",
    "cidr_block": "172.17.1.0/24",
    "customer_owned_ipv4_pool": "",
    "enable_dns64": false,
    "enable_lni_at_device_index": 0,
    "enable_resource_name_dns_a_record_on_launch": false,
    "enable_resource_name_dns_aaaa_record_on_launch": false,
    "id": "subnet-9d6e838804f9b548a",
    "ipv6_cidr_block": "",
    "ipv6_cidr_block_association_id": "",
    "ipv6_native": false,
    "map_customer_owned_ip_on_launch": false,
    "map_public_ip_on_launch": false,
    "outpost_arn": "",
    "owner_id": "000000000000",
    "private_dns_hostname_type_on_launch": "ip-name",
    "tags": {},
    "tags_all": {},
    "timeouts": null,
    "vpc_id": "vpc-2accbbae3126d386c"
  }
]
```
### Note
This configuration **worked correctly in LocalStack 3.8**, but in **4.2**, the **tags** are missing from the created subnets.

Hints from issue discussion:
Welcome to LocalStack! Thanks for reporting your first issue and our team will be working towards fixing the issue for you or reach out for more background information. We recommend joining our [Slack Community](https://localstack.cloud/contact/) for real-time help and drop a message to LocalStack Pro Support if you are a Pro user! If you are willing to contribute towards fixing this issue, please have a look at our [contributing guidelines](https://github.com/localstack/.github/blob/main/CONTRIBUTING.md) and our [contributing guide](https://docs.localstack.cloud/contributing/).

Commit message: Remove unused import found by lint checker

Patch to review:
diff --git a/localstack-core/localstack/services/ec2/patches.py b/localstack-core/localstack/services/ec2/patches.py
index d9db4cad11e08..6c523d1125d90 100644
--- a/localstack-core/localstack/services/ec2/patches.py
+++ b/localstack-core/localstack/services/ec2/patches.py
@@ -78,15 +78,22 @@ def ec2_create_subnet(
         tags: Optional[dict[str, str]] = None,
         **kwargs,
     ):
+        # Patch this method so that we can create a subnet with a specific "custom"
+        # ID.  The custom ID that we will use is contained within a special tag.
         vpc_id: str = args[0] if len(args) >= 1 else kwargs["vpc_id"]
         cidr_block: str = args[1] if len(args) >= 1 else kwargs["cidr_block"]
         resource_identifier = SubnetIdentifier(
             self.account_id, self.region_name, vpc_id, cidr_block
         )
-        # tags has the format: {"subnet": {"Key": ..., "Value": ...}}
+
+        # tags has the format: {"subnet": {"Key": ..., "Value": ...}}, but we need
+        # to pass this to the generate method as {"Key": ..., "Value": ...}.  Take
+        # care not to alter the original tags dict otherwise moto will not be able
+        # to understand it.
+        subnetTags = None
         if tags is not None:
-            tags = tags.get("subnet", tags)
-        custom_id = resource_identifier.generate(tags=tags)
+            subnetTags = tags.get("subnet", tags)
+        custom_id = resource_identifier.generate(tags=subnetTags)
 
         if custom_id:
             # Check if custom id is unique within a given VPC
@@ -102,9 +109,16 @@ def ec2_create_subnet(
         if custom_id:
             # Remove the subnet from the default dict and add it back with the custom id
             self.subnets[availability_zone].pop(result.id)
+            old_id = result.id
             result.id = custom_id
             self.subnets[availability_zone][custom_id] = result
 
+            # Tags are not stored in the Subnet object, but instead stored in a separate
+            # dict in the EC2 backend, keyed by subnet id.  That therefore requires
+            # updating as well.
+            if old_id in self.tags:
+                self.tags[custom_id] = self.tags.pop(old_id)
+
         # Return the subnet with the patched custom id
         return result
 
@@ -132,9 +146,16 @@ def ec2_create_security_group(
         if custom_id:
             # Remove the security group from the default dict and add it back with the custom id
             self.groups[result.vpc_id].pop(result.group_id)
+            old_id = result.group_id
             result.group_id = result.id = custom_id
             self.groups[result.vpc_id][custom_id] = result
 
+            # Tags are not stored in the Security Group object, but instead are stored in a
+            # separate dict in the EC2 backend, keyed by id.  That therefore requires
+            # updating as well.
+            if old_id in self.tags:
+                self.tags[custom_id] = self.tags.pop(old_id)
+
         return result
 
     @patch(ec2_models.vpcs.VPCBackend.create_vpc)
@@ -175,9 +196,16 @@ def ec2_create_vpc(
 
             # Remove the VPC from the default dict and add it back with the custom id
             self.vpcs.pop(vpc_id)
+            old_id = result.id
             result.id = custom_id
             self.vpcs[custom_id] = result
 
+            # Tags are not stored in the VPC object, but instead stored in a separate
+            # dict in the EC2 backend, keyed by VPC id.  That therefore requires
+            # updating as well.
+            if old_id in self.tags:
+                self.tags[custom_id] = self.tags.pop(old_id)
+
             # Create default network ACL, route table, and security group for custom ID VPC
             self.create_route_table(
                 vpc_id=custom_id,
diff --git a/tests/aws/services/ec2/test_ec2.py b/tests/aws/services/ec2/test_ec2.py
index 0c32ba80386af..863fccaf5c393 100644
--- a/tests/aws/services/ec2/test_ec2.py
+++ b/tests/aws/services/ec2/test_ec2.py
@@ -460,6 +460,9 @@ def test_create_vpc_with_custom_id(self, aws_client, create_vpc):
         # Check if the custom ID is present in the describe_vpcs response as well
         vpc: dict = aws_client.ec2.describe_vpcs(VpcIds=[custom_id])["Vpcs"][0]
         assert vpc["VpcId"] == custom_id
+        assert len(vpc["Tags"]) == 1
+        assert vpc["Tags"][0]["Key"] == TAG_KEY_CUSTOM_ID
+        assert vpc["Tags"][0]["Value"] == custom_id
 
         # Check if an duplicate custom ID exception is thrown if we try to recreate the VPC with the same custom ID
         with pytest.raises(ClientError) as e:
@@ -478,6 +481,48 @@ def test_create_vpc_with_custom_id(self, aws_client, create_vpc):
         assert e.value.response["ResponseMetadata"]["HTTPStatusCode"] == 400
         assert e.value.response["Error"]["Code"] == "InvalidVpc.DuplicateCustomId"
 
+    @markers.aws.only_localstack
+    def test_create_subnet_with_tags(self, aws_client, create_vpc):
+        # Create a VPC.
+        vpc: dict = create_vpc(
+            cidr_block="10.0.0.0/16",
+            tag_specifications=[
+                {
+                    "ResourceType": "vpc",
+                    "Tags": [
+                        {"Key": "Name", "Value": "main-vpc"},
+                    ],
+                }
+            ],
+        )
+        vpc_id: str = vpc["Vpc"]["VpcId"]
+
+        # Create a subnet with a tag.
+        subnet: dict = aws_client.ec2.create_subnet(
+            VpcId=vpc_id,
+            CidrBlock="10.0.0.0/24",
+            TagSpecifications=[
+                {
+                    "ResourceType": "subnet",
+                    "Tags": [
+                        {"Key": "Name", "Value": "main-subnet"},
+                    ],
+                }
+            ],
+        )
+        assert subnet["Subnet"]["VpcId"] == vpc_id
+        subnet_id: str = subnet["Subnet"]["SubnetId"]
+
+        # Now check that the tags make it back on the describe subnets call.
+        subnet: dict = aws_client.ec2.describe_subnets(
+            SubnetIds=[subnet_id],
+        )["Subnets"][0]
+        assert subnet["SubnetId"] == subnet_id
+        assert subnet["VpcId"] == vpc_id
+        assert len(subnet["Tags"]) == 1
+        assert subnet["Tags"][0]["Key"] == "Name"
+        assert subnet["Tags"][0]["Value"] == "main-subnet"
+
     @markers.aws.only_localstack
     def test_create_subnet_with_custom_id(self, aws_client, create_vpc):
         custom_id = random_subnet_id()
@@ -565,6 +610,9 @@ def test_create_subnet_with_custom_id_and_vpc_id(self, aws_client, create_vpc):
         )["Subnets"][0]
         assert subnet["SubnetId"] == custom_subnet_id
         assert subnet["VpcId"] == custom_vpc_id
+        assert len(subnet["Tags"]) == 1
+        assert subnet["Tags"][0]["Key"] == TAG_KEY_CUSTOM_ID
+        assert subnet["Tags"][0]["Value"] == custom_subnet_id
 
     @markers.aws.only_localstack
     def test_create_security_group_with_custom_id(self, aws_client, create_vpc):
@@ -604,6 +652,9 @@ def test_create_security_group_with_custom_id(self, aws_client, create_vpc):
             (sg for sg in security_groups if sg["VpcId"] == vpc["Vpc"]["VpcId"]), None
         )
         assert security_group["GroupId"] == custom_id
+        assert len(security_group["Tags"]) == 1
+        assert security_group["Tags"][0]["Key"] == TAG_KEY_CUSTOM_ID
+        assert security_group["Tags"][0]["Value"] == custom_id
 
         # Check if a duplicate custom ID exception is thrown if we try to recreate the security group with the same custom ID
         with pytest.raises(ClientError) as e:


Code review comments:
diff_hunk: @@ -78,15 +78,22 @@ def ec2_create_subnet(
         tags: Optional[dict[str, str]] = None,
         **kwargs,
     ):
+        # Patch this method so that we can create a subnet with a specific "custom"
+        # ID.  The custom ID that we will use is contained within a special tag.
         vpc_id: str = args[0] if len(args) >= 1 else kwargs["vpc_id"]
         cidr_block: str = args[1] if len(args) >= 1 else kwargs["cidr_block"]
         resource_identifier = SubnetIdentifier(
             self.account_id, self.region_name, vpc_id, cidr_block
         )
-        # tags has the format: {"subnet": {"Key": ..., "Value": ...}}
+
+        # tags has the format: {"subnet": {"Key": ..., "Value": ...}}, but we need
+        # to pass this to the generate method as {"Key": ..., "Value": ...}.  Take
+        # care not to alter the original tags dict otherwise moto will not be able
+        # to understand it.
+        subnetTags = None
path: localstack-core/localstack/services/ec2/patches.py
text: @user1:
In Python, the convention is to use `snake_case` to name variables and functions. `CamelCase` is used for classes.

[PEP8](https://peps.python.org/pep-0008/#naming-conventions) has more details. (PEPs/Python Enhancement Proposals are like RFC documents for Python and contain feature proposals, usage and conventions)