fix(aws-ec2): tag Elastic IPs and VPC endpoints after creation

[aryanmehrotra]

What was broken

CreateTags / DeleteTags on an Elastic IP allocation id or a VPC endpoint id failed. With the aws-sdk-go-v2 EC2 client:

AllocateAddress -> eipalloc-00000010
CreateTags(Resources: [eipalloc-00000010], Tags: [owner=team-a])
=> api error InvalidID.NotFound: resource "eipalloc-00000010" not found

Same for vpce-…. The EC2 tag router in server/aws/ec2/tags.go only sent rtb-/igw-/nat-/acl-/dopt-/pcx-/pl-/eigw-/sgr- to the networking tagger; every other prefix went to the compute tagger, which only knows i-/vol-/snap-/ami-. Tags set at create time via TagSpecifications worked, but nothing could change them afterwards.

What changed

• providers/aws/vpc/tags.go: UpdateResourceTags / RemoveResourceTags now handle eipalloc-, vpce- and vpce-svc- ids, writing to the same records AllocateAddress / CreateVpcEndpoint / CreateVpcEndpointServiceConfiguration store their create-time tags on. vpce-svc- is matched before vpce- since it shares the prefix. The write holds Mock.mu, like the other EIP / endpoint-service mutators.
• providers/aws/vpc/endpoint.go: DescribeVPCEndpoints now takes Mock.mu.RLock, so a tag write cannot race a describe of the same endpoint.
• server/aws/ec2/tags.go: eipalloc- and vpce- (which also covers vpce-svc-) are routed to NetworkResourceTagger. DescribeTags now also reports tags for elastic-ip, vpc-endpoint and vpc-endpoint-service resources.
• DescribeAddresses and DescribeVpcEndpoints already supported tag:<key> and tag-key filters; they now see tags added or removed after creation.

How it was tested

• New provider unit test TestAddressingResourceTagger (providers/aws/vpc/tags_test.go): tag/untag on EIP, endpoint and endpoint service; create-time tags are kept; unknown ids of each prefix are NotFound.
• New SDK round-trip tests (server/aws/ec2/tags_eip_vpce_test.go, real aws-sdk-go-v2 EC2 client against httptest): AllocateAddress / CreateVpcEndpoint with a TagSpecification, then CreateTags, then DescribeAddresses / DescribeVpcEndpoints filtered by tag:owner and by tag-key, then DescribeTags by resource-id (checking the resource type), then DeleteTags, then Describe again. Also: CreateTags on a missing eipalloc- / vpce- id returns InvalidID.NotFound.
• Before the fix, TestCreateTagsOnElasticIP and TestCreateTagsOnVPCEndpoint failed with the InvalidID.NotFound error above. With the fix they pass.

Commands:

go test -count=1 ./...                                           # EXIT=0, 354 packages ok
go test -race -count=1 ./providers/aws/vpc/ ./server/aws/ec2/    # ok
golangci-lint run --timeout=9m --new-from-rev=origin/development ./...   # 0 issues
go generate ./... && go run ./internal/compatgen                 # no changes to generated files

The full golangci-lint run ./... still reports issues that already exist on development; none of them are in lines this PR adds.

[NitinKumar004]

The routing change is correct. Tags land on the same records TagSpecifications writes, DescribeTags reports the right resource types, and the new m.mu usage is needed. A few parity gaps on the new paths:

Medium

1. server/aws/ec2/tags.go:350 (tagNotFoundCode): an unknown eipalloc- or vpce- id returns InvalidID.NotFound. The EC2 error reference defines InvalidAllocationID.NotFound and InvalidVpcEndpointId.NotFound for these resources. This package already emits both (address.go:114, endpoint.go:234), and tagNotFoundCode already special-cases i- and sgr- for the same reason. Terraform's EC2 tag retry matches on the .NotFound suffix, so the resource-specific code matters.
   • Fix: add case strings.HasPrefix(id, "eipalloc-"): return "InvalidAllocationID.NotFound" and a vpce- case returning "InvalidVpcEndpointId.NotFound". Only fall back to the generic code for vpce-svc-.
   • Update tags_eip_vpce_test.go:206, which pins the generic code, and the doc comment at providers/aws/vpc/tags.go:16.
2. providers/aws/vpc/tags.go:30 (RemoveResourceTags) together with vpc.go:1268 (removeTagMapKeys): aws ec2 delete-tags --resources <vpce-id> with no --tags returns success but leaves every tag in place (checked against serve). The EC2 DeleteTags reference says: "If you omit this parameter, we delete all user-defined tags for the specified resources." The compute tagger already does this (providers/aws/ec2/tags.go, UntagResource with empty keys), so the same call now behaves differently on i- and on eipalloc-.
   • Fix: when keys is empty, drop every key that does not start with aws:. Add a wire test for it.

Low

3. server/aws/ec2/tags.go:287 / :336: a mixed batch is not atomic. create-tags --resources <eip> vpce-bogus <vpce> fails with NotFound, but the EIP is still tagged (confirmed with describe-tags). Real EC2 validates every ResourceId before it writes anything. This existed before the PR, but the PR routes more id types into it.
   • Fix: do a first pass that probes each id (tagResource(ctx, id, nil) is already a no-op existence check on both taggers), then write.
4. providers/aws/vpc/endpoint.go:132 (ModifyVPCEndpoint) still mutates ep.SubnetIDs, ep.RouteTableIDs and ep.Tags without m.mu. DescribeVPCEndpoints now reads under RLock, so a concurrent ModifyVPCEndpoint and DescribeVPCEndpoints still trips -race (endpoint.go:152 against 175). Take m.mu.Lock() there too, so the lock covers every writer.
5. Tests:
   • Nothing pins the new lock. With the m.mu.Lock() in mutateAddressingTags removed, every test in the PR still passes under -race. A concurrent UpdateResourceTags + DescribeAddresses test fails right away without the lock.
   • vpce-svc- is only covered at the provider level. The vpc-endpoint-service resource type and the resource-type filter on DescribeTags have no wire test.

Follow-ups (existed before this PR, whole handler)

• validateUserTags counts only the tags in the request. After CreateTags an EIP ended up with 52 tags; real EC2 allows 50 per resource (TagLimitExceeded).
• Keys over 128 and values over 256 characters are accepted.
• DeleteTags Key=env,Value=wrong deletes the tag. Real EC2 deletes only when the value matches.
• aws_vpc_endpoint cannot refresh in Terraform (Gateway or Interface) because DescribePrefixLists returns InvalidAction, so the Terraform check of endpoint tags is blocked for now.

Verified

• go build, go vet and go test -race pass on providers/aws/vpc, server/aws/ec2, services/networking/... and persist; go test ./server/aws/... passes. golangci-lint --new-from-rev=origin/development reports 0 issues. go generate shows no drift, and the branch merges cleanly onto current development.
• serve with the aws CLI: create-tags and delete-tags work on eipalloc-, vpce- and vpce-svc-, and tags from create time are kept. The tag: and tag-key filters work on describe-addresses and describe-vpc-endpoints. describe-tags works with resource-type elastic-ip, vpc-endpoint or vpc-endpoint-service (several at once too) and with resource-id. The aws: prefix is rejected. Tags added after creation survive a --persist restart. Tagging on instances, volumes and security groups is unchanged.
• Terraform aws_eip with tags: apply, "No changes", an in-place tag change, "No changes", destroy.

[aryanmehrotra]

Thanks for the review. The two medium findings, the three lows and three of the four follow-ups are fixed in c1e83513, rebased onto development.

Finding	Fix	Proof
M1: generic InvalidID.NotFound	eipalloc- gives InvalidAllocationID.NotFound and vpce- gives InvalidVpcEndpointId.NotFound, on CreateTags and DeleteTags. The test that pinned the generic code now asserts the specific ones.	TestCreateTagsOnMissingAddressingIDs
M2: DeleteTags with no --tags	Deletes every user tag and keeps aws: tags, in the provider (RemoveResourceTags with empty keys) and on the wire, the same way for every tagger.	TestDeleteTagsWithoutTagsClearsUserTags, TestDeleteTagsSemanticsOnEveryTagger
L3: mixed batch not atomic	Every ResourceId is validated before anything is written, for CreateTags and DeleteTags.	TestTagBatchIsAtomic
L4: ModifyVPCEndpoint without the lock	now under m.mu	TestAddressingTagWriteDoesNotRaceDescribe
L5: nothing pins the lock	That test runs UpdateResourceTags, Describe* and ModifyVPCEndpoint concurrently. It reports 7× DATA RACE without the lock.	same
L5: missing wire tests	Added a wire test for the vpc-endpoint-service resource type and the resource-type filter.	TestTagsOnVPCEndpointServiceOverTheWire
Follow-up: 50-tag limit	Now counts existing tags plus the new ones per resource. An overwrite doesn't count twice.	TestCreateTagsCountsExistingTags
Follow-up: key and value length	Key over 128 or value over 256 characters gives InvalidParameterValue.	TestCreateTagsRejectsOversizedKeyAndValue
Follow-up: DeleteTags with a value	Key=k,Value=v deletes only when the value matches.	TestDeleteTagsMatchesValue

Every test fails with its fix reverted. I also checked on serve with the aws CLI:

• a mixed batch writes nothing
• eipalloc-nope returns InvalidAllocationID.NotFound
• a wrong value keeps the tag
• delete-tags with no --tags clears the user tags

Departures:

• vpce-svc- returns InvalidVpcEndpointServiceId.NotFound, not the generic code. The EC2 error reference defines it and endpoint_service.go already emits it. Tell me if you'd rather have the generic one; it's a one-line change.
• The length error code InvalidParameterValue comes from secondary sources. The EC2 tag-restrictions page gives the limits but no code.
• The count, length and value-match checks sit in the EC2 tag router, because a batch spans several providers. The router reads current tags through a new ResourceTags on each tagger, under the writers' locks.

Still open, and older than this PR: aws_vpc_endpoint refresh in Terraform, which is blocked on DescribePrefixLists.

Gates: go build / go vet; go test -race on aws/vpc, aws/ec2, server/aws/ec2 and networking, plus go test ./server/... ./providers/aws/... ./services/...; golangci-lint --new-from-rev origin/development reports 0 issues; no coverage, compat or tidy drift.

[NitinKumar004]

All the earlier findings are fixed, and I reproduced each one on serve and in Terraform. Approving.

Earlier findings

1. Resource-specific NotFound codes: fixed.
   • create-tags and delete-tags on eipalloc-nope return InvalidAllocationID.NotFound.
   • vpce-0nope returns InvalidVpcEndpointId.NotFound.
   • vpce-svc-0nope returns InvalidVpcEndpointServiceId.NotFound.
   • rtb-0nope still falls back to InvalidID.NotFound.
2. DeleteTags with no --tags: fixed. delete-tags --resources <vpce> clears every user tag, and the same holds for vpce-svc- and instances. aws: tags are kept.
3. Mixed batch not atomic: fixed. create-tags --resources <eip> vpce-0bogus <vpce> fails and leaves no tag anywhere. The same batch through delete-tags leaves the EIP's tag in place.
4. ModifyVPCEndpoint without m.mu: fixed. It now takes the lock. With the lock commented out, TestAddressingTagWriteDoesNotRaceDescribe reports DATA RACE, so the test pins the fix.
5. Tests: fixed. The concurrency test covers the lock. The vpc-endpoint-service resource type and the resource-type filter have wire tests.

Follow-ups:

• 50-tag limit: fixed. An overwrite at 50 is accepted. Adding keys past 50 returns TagLimitExceeded, and the count stays at 50.
• Key and value length: fixed. A 129-character key or a 257-character value returns InvalidParameterValue. A 128-character key is accepted.
• DeleteTags value match: fixed. Key=env,Value=wrong keeps the tag. Key=env,Value=prod removes it.
• aws_vpc_endpoint refresh: still open, and it predates this PR. Gateway and Interface endpoints both fail on DescribePrefixLists.

New findings

Nothing blocking. Two small notes:

• providers/aws/ec2/tags.go (UntagResource): with an empty key list, the compute tagger clears every tag, aws: ones included. The VPC tagger's RemoveResourceTags now keeps them. The wire path is consistent, because deleteTagKeys resolves explicit user keys first. The typed Go API, though, behaves differently for i-/vol- than for eipalloc-/vpce-. Keeping aws: keys in the compute remove closure would line the two up.
• providers/aws/vpc/tags.go (ResourceTags): the vpc-/subnet-/sg- branches go through Store.Update and assign Tags back, which is a field write on a read path. Readers are no worse off than they already are against UpdateVPCTags. Still, the doc comment's "never races" claims more than the code backs up. Soften the wording, or read under the store's read lock instead.

Checked

• aws CLI on serve:
  • create-tags and delete-tags on eipalloc-, vpce- and vpce-svc-.
  • tag: and tag-key filters on describe-addresses and describe-vpc-endpoints.
  • describe-tags by resource-id and by resource-type.
  • aws: prefix rejected with InvalidTagKey.Malformed.
  • Tagging on instances, volumes, VPCs, subnets and route tables is unchanged.
• Persist: tags added after creation survive a --persist restart.
• Terraform (aws 6.66.0):
  • aws_eip with tags: apply, a clean plan, a tag update, a clean plan, then destroy.
  • aws_ec2_tag on a VPC endpoint: the same cycle.
• Merges: this PR merges cleanly with #1354 and with current development. go test -race on server/aws/ec2 and providers/aws/vpc passes on the merged tree.
• Build and lint:
  • go build ./... and go vet pass.
  • go test -race passes on providers/aws/vpc, providers/aws/ec2, server/aws/ec2, services/networking/... and persist.
  • golangci-lint --new-from-rev=origin/development reports 0 issues.