diff --git a/providers/aws/ec2/tags.go b/providers/aws/ec2/tags.go index 4365a6272..e6be7ab41 100644 --- a/providers/aws/ec2/tags.go +++ b/providers/aws/ec2/tags.go @@ -131,3 +131,39 @@ func (m *Mock) UntagResource(_ context.Context, id string, keys []string) error return nil } + +// ResourceTags returns a copy of the current tags on an EC2 instance, volume, +// snapshot, or image, or NotFound for an unknown ID. The EC2 CreateTags and +// DeleteTags handler reads it for every resource in a batch before writing, so +// an unknown id, a tag-limit breach, or a DeleteTags value mismatch is decided +// before any resource changes. It takes the same locks as TagResource. +func (m *Mock) ResourceTags(_ context.Context, id string) (map[string]string, error) { + var out map[string]string + + snapshot := func(src map[string]string) { + out = make(map[string]string, len(src)) + for k, v := range src { + out[k] = v + } + } + + if strings.HasPrefix(id, "i-") { + if !m.mutateInstanceTags(id, snapshot) { + return nil, cerrors.Newf(cerrors.NotFound, "resource %q not found", id) + } + + return out, nil + } + + m.mu.Lock() + defer m.mu.Unlock() + + src, ok := m.tagsOf(id) + if !ok { + return nil, cerrors.Newf(cerrors.NotFound, "resource %q not found", id) + } + + snapshot(src) + + return out, nil +} diff --git a/providers/aws/ec2/tags_test.go b/providers/aws/ec2/tags_test.go new file mode 100644 index 000000000..c4c83c450 --- /dev/null +++ b/providers/aws/ec2/tags_test.go @@ -0,0 +1,51 @@ +package ec2 + +import ( + "context" + "testing" + + cerrors "github.com/stackshy/cloudemu/v2/errors" + "github.com/stackshy/cloudemu/v2/services/compute/driver" +) + +// TestResourceTagsReturnsACopy covers ResourceTags on an instance and a volume +// (the two lock paths): it reports the current tags, the returned map is a copy, +// and an unknown id is NotFound. The EC2 CreateTags/DeleteTags handler reads it +// to check a batch before writing. +func TestResourceTagsReturnsACopy(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + insts, err := m.RunInstances(ctx, driver.InstanceConfig{ImageID: "ami-123", InstanceType: "t2.micro"}, 1) + if err != nil { + t.Fatalf("RunInstances: %v", err) + } + + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{Size: 8, AvailabilityZone: "us-east-1a"}) + if err != nil { + t.Fatalf("CreateVolume: %v", err) + } + + for _, id := range []string{insts[0].ID, vol.ID} { + if err := m.TagResource(ctx, id, map[string]string{"env": "prod"}); err != nil { + t.Fatalf("TagResource(%s): %v", id, err) + } + + got, err := m.ResourceTags(ctx, id) + if err != nil || got["env"] != "prod" { + t.Fatalf("ResourceTags(%s) = %v, %v; want env=prod", id, got, err) + } + + got["env"] = "mutated" + + if again, _ := m.ResourceTags(ctx, id); again["env"] != "prod" { + t.Fatalf("ResourceTags(%s) returned the live map: %v", id, again) + } + } + + for _, id := range []string{"i-missing", "vol-missing", "x-unknown"} { + if _, err := m.ResourceTags(ctx, id); !cerrors.IsNotFound(err) { + t.Fatalf("ResourceTags(%s) = %v, want NotFound", id, err) + } + } +} diff --git a/providers/aws/vpc/endpoint.go b/providers/aws/vpc/endpoint.go index 8ee0c5bbc..96cdb252d 100644 --- a/providers/aws/vpc/endpoint.go +++ b/providers/aws/vpc/endpoint.go @@ -110,6 +110,9 @@ func (m *Mock) DeleteVPCEndpoint( func (m *Mock) DescribeVPCEndpoints( _ context.Context, ids []string, ) ([]driver.VPCEndpoint, error) { + m.mu.RLock() + defer m.mu.RUnlock() + for _, id := range ids { if !m.endpoints.Has(id) { return nil, errors.Newf( @@ -129,6 +132,11 @@ func (m *Mock) DescribeVPCEndpoints( func (m *Mock) ModifyVPCEndpoint( _ context.Context, id string, cfg driver.VPCEndpointConfig, ) (*driver.VPCEndpoint, error) { + // The field writes below go through the stored pointer, so they need m.mu: + // DescribeVPCEndpoints and the EC2 tag writer touch the same record under it. + m.mu.Lock() + defer m.mu.Unlock() + ep, ok := m.endpoints.Get(id) if !ok { return nil, errors.Newf( diff --git a/providers/aws/vpc/tags.go b/providers/aws/vpc/tags.go index be9d1d12b..c0287532b 100644 --- a/providers/aws/vpc/tags.go +++ b/providers/aws/vpc/tags.go @@ -11,9 +11,13 @@ import ( // UpdateResourceTags merges tags onto a VPC-family resource that has no // dedicated Update*Tags method: route tables, internet gateways, NAT // gateways, network ACLs, DHCP option sets, peering connections, managed -// prefix lists, and egress-only internet gateways. An unknown or missing id -// is NotFound, so the wire layer can map it to the InvalidID.NotFound code -// real EC2 returns for CreateTags on a non-existent resource. +// prefix lists, egress-only internet gateways, security-group rules, Elastic IP +// allocations, VPC endpoints and VPC endpoint services. An unknown or missing id +// is NotFound; the wire layer maps it to the resource-specific code real EC2 +// defines for it (InvalidAllocationID.NotFound for eipalloc-, +// InvalidVpcEndpointId.NotFound for vpce-, InvalidVpcEndpointServiceId.NotFound +// for vpce-svc-, InvalidSecurityGroupRuleId.NotFound for sgr-), and to the +// generic InvalidID.NotFound for the rest. func (m *Mock) UpdateResourceTags(_ context.Context, id string, tags map[string]string) error { if !m.mutateResourceTags(id, func(existing map[string]string) map[string]string { return mergeTagMap(existing, tags) @@ -24,10 +28,20 @@ func (m *Mock) UpdateResourceTags(_ context.Context, id string, tags map[string] return nil } +// reservedTagPrefix is the namespace of AWS-generated tags, which EC2 DeleteTags +// never removes. +const reservedTagPrefix = "aws:" + // RemoveResourceTags drops the given tag keys from a VPC-family resource, the -// DeleteTags counterpart to UpdateResourceTags. +// DeleteTags counterpart to UpdateResourceTags. An empty key list deletes every +// user-defined tag and keeps the AWS-generated "aws:" ones, which is what EC2 +// DeleteTags does when the Tag parameter is omitted. func (m *Mock) RemoveResourceTags(_ context.Context, id string, keys []string) error { if !m.mutateResourceTags(id, func(existing map[string]string) map[string]string { + if len(keys) == 0 { + return keepReservedTags(existing) + } + return removeTagMapKeys(existing, keys) }) { return errors.Newf(errors.NotFound, "resource %q not found", id) @@ -36,6 +50,54 @@ func (m *Mock) RemoveResourceTags(_ context.Context, id string, keys []string) e return nil } +// ResourceTags returns a copy of the current tags on any resource the EC2 tag +// API addresses through this provider: VPCs, subnets and security groups plus +// every id UpdateResourceTags accepts. It is NotFound for an unknown id, so the +// EC2 CreateTags/DeleteTags handler can check every resource in a batch +// (existence, the per-resource tag limit, DeleteTags value matching) before it +// writes to any of them. The read goes through the same store/m.mu path as the +// writers, so it never races a concurrent tag write. +func (m *Mock) ResourceTags(_ context.Context, id string) (map[string]string, error) { + var out map[string]string + + snapshot := func(existing map[string]string) map[string]string { + out = copyTags(existing) + return existing + } + + var found bool + + switch { + case strings.HasPrefix(id, "vpc-"): + found = m.vpcs.Update(id, func(v *vpcData) *vpcData { v.Tags = snapshot(v.Tags); return v }) + case strings.HasPrefix(id, "subnet-"): + found = m.subnets.Update(id, func(s *subnetData) *subnetData { s.Tags = snapshot(s.Tags); return s }) + case strings.HasPrefix(id, "sg-"): + found = m.securityGroups.Update(id, func(sg *sgData) *sgData { sg.Tags = snapshot(sg.Tags); return sg }) + default: + found = m.mutateResourceTags(id, snapshot) + } + + if !found { + return nil, errors.Newf(errors.NotFound, "resource %q not found", id) + } + + return out, nil +} + +// keepReservedTags returns a fresh map holding only the "aws:" tags of existing. +func keepReservedTags(existing map[string]string) map[string]string { + out := make(map[string]string) + + for k, v := range existing { + if strings.HasPrefix(k, reservedTagPrefix) { + out[k] = v + } + } + + return out +} + // mutateResourceTags routes an id to the store that owns it and applies // transform to that record's tag map inside memstore.Update, so the store lock // covers the read-modify-write. It reports whether the id matched a known @@ -63,6 +125,32 @@ func (m *Mock) mutateResourceTags(id string, transform func(map[string]string) m }) case strings.HasPrefix(id, "sgr-"): return m.mutateRuleTags(id, transform) + default: + return m.mutateAddressingTags(id, transform) + } +} + +// mutateAddressingTags is the mutateResourceTags continuation for Elastic IP +// allocations (eipalloc-), VPC endpoint services (vpce-svc-) and VPC endpoints +// (vpce-). These are the same records AllocateAddress / CreateVpcEndpoint +// TagSpecifications write, so tags added after creation show up on the +// Describe calls. vpce-svc- is checked before vpce- because it shares the prefix. +// m.mu is held because the Describe readers for these records read their fields +// under m.mu.RLock (see the Mock.mu comment). +func (m *Mock) mutateAddressingTags(id string, transform func(map[string]string) map[string]string) bool { + m.mu.Lock() + defer m.mu.Unlock() + + switch { + case strings.HasPrefix(id, "eipalloc-"): + return m.eips.Update(id, func(v *eipData) *eipData { v.Tags = transform(v.Tags); return v }) + case strings.HasPrefix(id, "vpce-svc-"): + return m.endpointServices.Update(id, func(v *driver.EndpointService) *driver.EndpointService { + v.Tags = transform(v.Tags) + return v + }) + case strings.HasPrefix(id, "vpce-"): + return m.endpoints.Update(id, func(v *driver.VPCEndpoint) *driver.VPCEndpoint { v.Tags = transform(v.Tags); return v }) default: return false } diff --git a/providers/aws/vpc/tags_test.go b/providers/aws/vpc/tags_test.go index 45cd4985e..b902c4143 100644 --- a/providers/aws/vpc/tags_test.go +++ b/providers/aws/vpc/tags_test.go @@ -2,6 +2,7 @@ package vpc import ( "context" + "sync" "testing" "github.com/stackshy/cloudemu/v2/errors" @@ -100,3 +101,196 @@ func routeTableTag(t *testing.T, m *Mock, id, key string) string { func isNotFound(err error) bool { return err != nil && errors.IsNotFound(err) } + +// TestAddressingResourceTagger covers tagging Elastic IP allocations, VPC +// endpoints and VPC endpoint services after creation: the tag merges with the +// ones set at create time on the same record the Describe calls read, delete +// removes only the named key, and an unknown id of each prefix is NotFound. +func TestAddressingResourceTagger(t *testing.T) { + ctx := context.Background() + m := newTestMock() + v := createTestVPC(m) + + eip, err := m.AllocateAddress(ctx, driver.ElasticIPConfig{Tags: map[string]string{"Name": "eip"}}) + requireNoError(t, err) + + ep, err := m.CreateVPCEndpoint(ctx, driver.VPCEndpointConfig{ + VPCID: v.ID, ServiceName: "com.amazonaws.us-east-1.s3", EndpointType: "Gateway", + Tags: map[string]string{"Name": "ep"}, + }) + requireNoError(t, err) + + svc, err := m.CreateVPCEndpointServiceConfiguration(ctx, driver.EndpointServiceConfig{ + NetworkLoadBalancerARNs: []string{"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/net/n/1"}, + }) + requireNoError(t, err) + + for _, id := range []string{eip.AllocationID, ep.ID, svc.ID} { + requireNoError(t, m.UpdateResourceTags(ctx, id, map[string]string{"env": "prod"})) + } + + eips, err := m.DescribeAddresses(ctx, []string{eip.AllocationID}) + requireNoError(t, err) + assertEqual(t, "prod", eips[0].Tags["env"]) + assertEqual(t, "eip", eips[0].Tags["Name"]) + + eps, err := m.DescribeVPCEndpoints(ctx, []string{ep.ID}) + requireNoError(t, err) + assertEqual(t, "prod", eps[0].Tags["env"]) + assertEqual(t, "ep", eps[0].Tags["Name"]) + + svcs, err := m.DescribeVPCEndpointServiceConfigurations(ctx, []string{svc.ID}) + requireNoError(t, err) + assertEqual(t, "prod", svcs[0].Tags["env"]) + + for _, id := range []string{eip.AllocationID, ep.ID, svc.ID} { + requireNoError(t, m.RemoveResourceTags(ctx, id, []string{"env"})) + } + + eips, err = m.DescribeAddresses(ctx, []string{eip.AllocationID}) + requireNoError(t, err) + + if _, ok := eips[0].Tags["env"]; ok { + t.Fatalf("eip still carries env after RemoveResourceTags: %v", eips[0].Tags) + } + + assertEqual(t, "eip", eips[0].Tags["Name"]) + + eps, err = m.DescribeVPCEndpoints(ctx, []string{ep.ID}) + requireNoError(t, err) + + if _, ok := eps[0].Tags["env"]; ok { + t.Fatalf("endpoint still carries env after RemoveResourceTags: %v", eps[0].Tags) + } + + for _, id := range []string{"eipalloc-missing", "vpce-missing", "vpce-svc-missing"} { + if err := m.UpdateResourceTags(ctx, id, map[string]string{"k": "v"}); !isNotFound(err) { + t.Fatalf("UpdateResourceTags(%s) = %v, want NotFound", id, err) + } + } +} + +// TestRemoveResourceTagsWithoutKeysKeepsReservedTags pins EC2 DeleteTags with no +// Tag parameter: every user tag goes, the AWS-generated "aws:" tags stay. Before +// the fix an empty key list removed nothing. +func TestRemoveResourceTagsWithoutKeysKeepsReservedTags(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + eip, err := m.AllocateAddress(ctx, driver.ElasticIPConfig{Tags: map[string]string{ + "Name": "eip", "env": "prod", "aws:cloudformation:stack-name": "s", + }}) + requireNoError(t, err) + + requireNoError(t, m.RemoveResourceTags(ctx, eip.AllocationID, nil)) + + got, err := m.ResourceTags(ctx, eip.AllocationID) + requireNoError(t, err) + + if len(got) != 1 || got["aws:cloudformation:stack-name"] != "s" { + t.Fatalf("tags after RemoveResourceTags(nil) = %v, want only the aws: tag", got) + } +} + +// TestResourceTagsReadsEveryTaggableID covers ResourceTags on each id family the +// EC2 tag handler routes to this provider, that it returns a copy, and that an +// unknown id is NotFound. +func TestResourceTagsReadsEveryTaggableID(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + v, err := m.CreateVPC(ctx, driver.VPCConfig{CIDRBlock: "10.0.0.0/16", Tags: map[string]string{"k": "vpc"}}) + requireNoError(t, err) + + s, err := m.CreateSubnet(ctx, driver.SubnetConfig{VPCID: v.ID, CIDRBlock: "10.0.1.0/24", Tags: map[string]string{"k": "subnet"}}) + requireNoError(t, err) + + sg, err := m.CreateSecurityGroup(ctx, driver.SecurityGroupConfig{VPCID: v.ID, Name: "sg", Description: "d", Tags: map[string]string{"k": "sg"}}) + requireNoError(t, err) + + eip, err := m.AllocateAddress(ctx, driver.ElasticIPConfig{Tags: map[string]string{"k": "eip"}}) + requireNoError(t, err) + + rt, err := m.CreateRouteTable(ctx, driver.RouteTableConfig{VPCID: v.ID}) + requireNoError(t, err) + requireNoError(t, m.UpdateResourceTags(ctx, rt.ID, map[string]string{"k": "rtb"})) + + for id, want := range map[string]string{v.ID: "vpc", s.ID: "subnet", sg.ID: "sg", eip.AllocationID: "eip", rt.ID: "rtb"} { + got, err := m.ResourceTags(ctx, id) + requireNoError(t, err) + assertEqual(t, want, got["k"]) + + got["k"] = "mutated" + + again, err := m.ResourceTags(ctx, id) + requireNoError(t, err) + assertEqual(t, want, again["k"]) + } + + for _, id := range []string{"vpc-missing", "subnet-missing", "sg-missing", "eipalloc-missing", "rtb-missing", "vol-other"} { + if _, err := m.ResourceTags(ctx, id); !isNotFound(err) { + t.Fatalf("ResourceTags(%s) = %v, want NotFound", id, err) + } + } +} + +// TestAddressingTagWriteDoesNotRaceDescribe pins the m.mu the tag writer holds +// for Elastic IPs and VPC endpoints: DescribeAddresses / DescribeVPCEndpoints +// read the record's Tags field under m.mu.RLock, so without the writer's lock +// this trips -race. ModifyVPCEndpoint writes the same record and is run too. +func TestAddressingTagWriteDoesNotRaceDescribe(t *testing.T) { + ctx := context.Background() + m := newTestMock() + v := createTestVPC(m) + + eip, err := m.AllocateAddress(ctx, driver.ElasticIPConfig{}) + requireNoError(t, err) + + ep, err := m.CreateVPCEndpoint(ctx, driver.VPCEndpointConfig{ + VPCID: v.ID, ServiceName: "com.amazonaws.us-east-1.s3", EndpointType: "Gateway", + }) + requireNoError(t, err) + + const iters = 200 + + var wg sync.WaitGroup + + wg.Add(4) + + go func() { + defer wg.Done() + + for i := 0; i < iters; i++ { + _ = m.UpdateResourceTags(ctx, eip.AllocationID, map[string]string{"k": "v"}) + _ = m.UpdateResourceTags(ctx, ep.ID, map[string]string{"k": "v"}) + } + }() + + go func() { + defer wg.Done() + + for i := 0; i < iters; i++ { + _, _ = m.ModifyVPCEndpoint(ctx, ep.ID, driver.VPCEndpointConfig{ + RouteTableIDs: []string{"rtb-1"}, Tags: map[string]string{"m": "v"}, + }) + } + }() + + go func() { + defer wg.Done() + + for i := 0; i < iters; i++ { + _, _ = m.DescribeAddresses(ctx, []string{eip.AllocationID}) + } + }() + + go func() { + defer wg.Done() + + for i := 0; i < iters; i++ { + _, _ = m.DescribeVPCEndpoints(ctx, []string{ep.ID}) + } + }() + + wg.Wait() +} diff --git a/server/aws/ec2/endpoint.go b/server/aws/ec2/endpoint.go index e9f9416ca..cf983b56f 100644 --- a/server/aws/ec2/endpoint.go +++ b/server/aws/ec2/endpoint.go @@ -81,7 +81,7 @@ func (h *Handler) deleteVPCEndpoints(w http.ResponseWriter, r *http.Request) { for _, id := range awsquery.ListStrings(r.Form, "VpcEndpointId") { if err := h.vpc.DeleteVPCEndpoint(r.Context(), id); err != nil { item := unsuccessfulItemXML{ResourceID: id} - item.Error.Code = "InvalidVpcEndpointId.NotFound" + item.Error.Code = codeInvalidVpcEndpointID item.Error.Message = cerrors.Message(err) unsuccessful = append(unsuccessful, item) } @@ -231,5 +231,5 @@ func toVPCEndpointXML(ep *netdriver.VPCEndpoint) vpcEndpointXML { } func writeVPCEndpointErr(w http.ResponseWriter, err error) { - writeErrWithNotFound(w, err, "InvalidVpcEndpointId.NotFound", "DependencyViolation") + writeErrWithNotFound(w, err, codeInvalidVpcEndpointID, "DependencyViolation") } diff --git a/server/aws/ec2/tags.go b/server/aws/ec2/tags.go index 84127147d..b85ea5bfb 100644 --- a/server/aws/ec2/tags.go +++ b/server/aws/ec2/tags.go @@ -3,8 +3,12 @@ package ec2 import ( "context" "encoding/xml" + "fmt" "net/http" + "net/url" + "strconv" "strings" + "unicode/utf8" "github.com/stackshy/cloudemu/v2/server/wire/awsquery" netdriver "github.com/stackshy/cloudemu/v2/services/networking/driver" @@ -95,6 +99,7 @@ func (h *Handler) describeTags(w http.ResponseWriter, r *http.Request) { var recs []tagRecord recs = h.collectComputeTags(r.Context(), recs) recs = h.collectNetworkTags(r.Context(), recs) + recs = h.collectAddressingTags(r.Context(), recs) items := make([]describeTagItemXML, 0, len(recs)) @@ -185,6 +190,37 @@ func (h *Handler) collectNetworkTags(ctx context.Context, recs []tagRecord) []ta return recs } +// collectAddressingTags appends tag records for Elastic IP allocations, VPC +// endpoints and VPC endpoint services, using the resource-type names real EC2 +// DescribeTags reports for them. +func (h *Handler) collectAddressingTags(ctx context.Context, recs []tagRecord) []tagRecord { + if h.vpc == nil { + return recs + } + + if eips, err := h.vpc.DescribeAddresses(ctx, nil); err == nil { + for i := range eips { + recs = appendTagRecords(recs, eips[i].AllocationID, "elastic-ip", eips[i].Tags) + } + } + + if eps, err := h.vpc.DescribeVPCEndpoints(ctx, nil); err == nil { + for i := range eps { + recs = appendTagRecords(recs, eps[i].ID, "vpc-endpoint", eps[i].Tags) + } + } + + if svcs, ok := h.vpc.(netdriver.VPCEndpointServices); ok { + if list, err := svcs.DescribeVPCEndpointServiceConfigurations(ctx, nil); err == nil { + for i := range list { + recs = appendTagRecords(recs, list[i].ID, "vpc-endpoint-service", list[i].Tags) + } + } + } + + return recs +} + // appendSGRuleTagRecords appends tag records for each security-group rule that // carries tags, keyed by the rule's sgr- id. func appendSGRuleTagRecords(recs []tagRecord, rules []netdriver.SecurityRule) []tagRecord { @@ -240,9 +276,41 @@ func tagMatchesFilter(rec tagRecord, f awsquery.Filter) bool { return false } +// tagReader is the optional read side of the EC2 taggers: the current tags of +// one resource, NotFound when it does not exist. The AWS compute and VPC +// providers implement it; the handler uses it to check a whole CreateTags / +// DeleteTags batch before writing any of it. +type tagReader interface { + ResourceTags(ctx context.Context, id string) (map[string]string, error) +} + +// currentTags returns the tags on id from the provider that owns it. A provider +// that cannot report tags yields a nil map after an existence probe (a no-op +// CreateTags), so the batch is still checked for unknown ids. +func (h *Handler) currentTags(ctx context.Context, id string) (map[string]string, error) { + var owner any = h.compute + if networkOwnedTagID(id) { + owner = h.vpc + } + + if rd, ok := owner.(tagReader); ok { + return rd.ResourceTags(ctx, id) + } + + return nil, h.tagResource(ctx, id, nil) +} + +// networkOwnedTagID reports whether the networking provider owns id's tags. +func networkOwnedTagID(id string) bool { + return strings.HasPrefix(id, "vpc-") || strings.HasPrefix(id, "subnet-") || + strings.HasPrefix(id, "sg-") || networkTaggableID(id) +} + // createTags applies tags to one or more resources, dispatching each resource // ID by prefix to the owning provider (VPC-family IDs to the networking -// provider, compute IDs to the compute tagger). +// provider, compute IDs to the compute tagger). Real EC2 checks the whole batch +// before it writes: an unknown id or a resource that would pass the tag limit +// fails the call with nothing tagged, so every id is checked first. func (h *Handler) createTags(w http.ResponseWriter, r *http.Request) { ids := awsquery.ListStrings(r.Form, "ResourceId") tags := awsquery.FlatTags(r.Form, "Tag") @@ -252,6 +320,19 @@ func (h *Handler) createTags(w http.ResponseWriter, r *http.Request) { return } + for _, id := range ids { + existing, err := h.currentTags(r.Context(), id) + if err != nil { + writeErrWithNotFound(w, err, tagNotFoundCode(id), "IncorrectState") + return + } + + if userTagCountAfter(existing, tags) > maxUserTagsPerResource { + awsquery.WriteXMLError(w, http.StatusBadRequest, codeTagLimitExceeded, msgTagLimitExceeded) + return + } + } + for _, id := range ids { if err := h.tagResource(r.Context(), id, tags); err != nil { writeErrWithNotFound(w, err, tagNotFoundCode(id), "IncorrectState") @@ -262,65 +343,196 @@ func (h *Handler) createTags(w http.ResponseWriter, r *http.Request) { awsquery.WriteXMLResponse(w, tagsResponseXML{Return: true, RequestID: "cloudemu"}) } -// maxUserTagsPerResource is the ceiling EC2 enforces on user tags per resource; -// reservedTagPrefix is the "aws:" namespace reserved for AWS-managed tags that a -// CreateTags call may not write. +// The CreateTags limits from the EC2 tag restrictions: at most 50 user tags per +// resource, keys up to 128 and values up to 256 Unicode characters, and the +// "aws:" key namespace reserved for AWS-generated tags (which do not count +// toward the 50). const ( maxUserTagsPerResource = 50 + maxTagKeyLen = 128 + maxTagValueLen = 256 reservedTagPrefix = "aws:" + + codeTagLimitExceeded = "TagLimitExceeded" + codeTagLengthExceeded = "InvalidParameterValue" + codeInvalidVpcEndpointID = "InvalidVpcEndpointId.NotFound" + msgTagLimitExceeded = "The maximum number of tags per resource is 50" ) -// validateUserTags enforces the CreateTags restrictions real EC2 applies before -// any tag is written: at most 50 user tags per resource (TagLimitExceeded), and -// no key in the reserved "aws:" namespace (InvalidTagKey.Malformed). Only the -// key is checked; real EC2 permits a value that starts with "aws:". It returns -// the wire error code and message plus ok=false when a rule is violated. +// userTagCountAfter is how many user (non-"aws:") tags a resource holds once +// tags are merged onto existing: a key already present is overwritten, not +// added. +func userTagCountAfter(existing, tags map[string]string) int { + n := 0 + + for k := range existing { + if _, overwritten := tags[k]; !overwritten && !strings.HasPrefix(k, reservedTagPrefix) { + n++ + } + } + + return n + len(tags) +} + +// validateUserTags enforces the per-request CreateTags restrictions real EC2 +// applies before any tag is written: no more than 50 tags in the request +// (TagLimitExceeded), no key in the reserved "aws:" namespace +// (InvalidTagKey.Malformed), keys of at most 128 and values of at most 256 +// characters (InvalidParameterValue). A value that starts with "aws:" is +// permitted. The per-resource limit, which also counts the tags a resource +// already has, is checked by createTags. It returns the wire error code and +// message plus ok=false when a rule is violated. func validateUserTags(tags map[string]string) (code, msg string, ok bool) { if len(tags) > maxUserTagsPerResource { - return "TagLimitExceeded", - "The maximum number of tags per resource is 50", false + return codeTagLimitExceeded, msgTagLimitExceeded, false } - for k := range tags { + for k, v := range tags { if strings.HasPrefix(k, reservedTagPrefix) { return "InvalidTagKey.Malformed", "The specified tag key is not valid. Tag keys cannot be empty or null, and cannot start with aws:", false } + + if utf8.RuneCountInString(k) > maxTagKeyLen { + return codeTagLengthExceeded, + fmt.Sprintf("Tag key exceeds the maximum length of %d characters", maxTagKeyLen), false + } + + if utf8.RuneCountInString(v) > maxTagValueLen { + return codeTagLengthExceeded, + fmt.Sprintf("Tag value exceeds the maximum length of %d characters", maxTagValueLen), false + } } return "", "", true } -// deleteTags removes tags (by key) from one or more resources. +// deleteTagSpec is one Tag.N entry of a DeleteTags request. hasValue is false +// when the request sent no Tag.N.Value at all, which deletes the key whatever +// its value; an explicit Value (even "") deletes it only on an exact match. +type deleteTagSpec struct { + key string + value string + hasValue bool +} + +// parseDeleteTags reads the Tag.N.Key / Tag.N.Value pairs of a DeleteTags +// request, keeping whether each Value was present. Entries without a key are +// skipped, as FlatTags does. +func parseDeleteTags(form url.Values) []deleteTagSpec { + idxs := awsquery.CollectIndices(form, "Tag") + specs := make([]deleteTagSpec, 0, len(idxs)) + + for _, idx := range idxs { + base := "Tag." + strconv.Itoa(idx) + + k := form.Get(base + ".Key") + if k == "" { + continue + } + + _, hasValue := form[base+".Value"] + specs = append(specs, deleteTagSpec{key: k, value: form.Get(base + ".Value"), hasValue: hasValue}) + } + + return specs +} + +// deleteTagKeys resolves a DeleteTags request against one resource's current +// tags into the keys to remove. With no Tag entries it is every user tag (EC2 +// never deletes "aws:" tags that way); otherwise it is each named key that is +// present and, when the entry carries a value, holds exactly that value. +// write is false when nothing on the resource matches, so the caller skips a +// removal whose empty key list the provider would read as "delete all". +// +// existing is nil when the provider cannot report tags; then the named keys are +// passed through unchecked (and an empty list is the provider's delete-all). +func deleteTagKeys(existing map[string]string, specs []deleteTagSpec) (keys []string, write bool) { + if existing == nil { + keys = make([]string, 0, len(specs)) + for _, s := range specs { + keys = append(keys, s.key) + } + + return keys, true + } + + if len(specs) == 0 { + for k := range existing { + if !strings.HasPrefix(k, reservedTagPrefix) { + keys = append(keys, k) + } + } + + return keys, len(keys) > 0 + } + + for _, s := range specs { + v, ok := existing[s.key] + if !ok || (s.hasValue && v != s.value) { + continue + } + + keys = append(keys, s.key) + } + + return keys, len(keys) > 0 +} + +// deleteTags removes tags from one or more resources. Like createTags it checks +// every id (and resolves which keys match) before it removes anything, so an +// unknown id fails the batch with nothing deleted. func (h *Handler) deleteTags(w http.ResponseWriter, r *http.Request) { ids := awsquery.ListStrings(r.Form, "ResourceId") - tags := awsquery.FlatTags(r.Form, "Tag") + specs := parseDeleteTags(r.Form) - keys := make([]string, 0, len(tags)) - for k := range tags { - keys = append(keys, k) + type removal struct { + id string + keys []string } + plan := make([]removal, 0, len(ids)) + for _, id := range ids { - if err := h.untagResource(r.Context(), id, keys); err != nil { + existing, err := h.currentTags(r.Context(), id) + if err != nil { writeErrWithNotFound(w, err, tagNotFoundCode(id), "IncorrectState") return } + + if keys, write := deleteTagKeys(existing, specs); write { + plan = append(plan, removal{id: id, keys: keys}) + } + } + + for _, p := range plan { + if err := h.untagResource(r.Context(), p.id, p.keys); err != nil { + writeErrWithNotFound(w, err, tagNotFoundCode(p.id), "IncorrectState") + return + } } awsquery.WriteXMLResponse(w, deleteTagsResponseXML{Return: true, RequestID: "cloudemu"}) } // tagNotFoundCode returns the "…NotFound" error code real EC2 emits when -// CreateTags/DeleteTags names a non-existent resource. An instance id yields the -// resource-specific InvalidInstanceID.NotFound; other resource types fall back -// to the generic InvalidID.NotFound. +// CreateTags/DeleteTags names a non-existent resource: the resource-specific +// code the EC2 error reference defines (and this package already returns from +// the resource's own actions) where there is one, and the generic +// InvalidID.NotFound for the rest. vpce-svc- is matched before vpce-, whose +// prefix it shares. func tagNotFoundCode(id string) string { switch { case strings.HasPrefix(id, "i-"): return codeInvalidInstanceID case strings.HasPrefix(id, "sgr-"): return "InvalidSecurityGroupRuleId.NotFound" + case strings.HasPrefix(id, "eipalloc-"): + return "InvalidAllocationID.NotFound" + case strings.HasPrefix(id, "vpce-svc-"): + return "InvalidVpcEndpointServiceId.NotFound" + case strings.HasPrefix(id, "vpce-"): + return codeInvalidVpcEndpointID default: return "InvalidID.NotFound" } @@ -332,7 +544,10 @@ func tagNotFoundCode(id string) string { // their own methods and are handled separately. // //nolint:gochecknoglobals // static id-prefix routing table -var networkResourceTagPrefixes = []string{"rtb-", "igw-", "nat-", "acl-", "dopt-", "pcx-", "pl-", "eigw-", "sgr-"} +var networkResourceTagPrefixes = []string{ + "rtb-", "igw-", "nat-", "acl-", "dopt-", "pcx-", "pl-", "eigw-", "sgr-", + "eipalloc-", "vpce-", // vpce- also covers vpce-svc- endpoint services +} // networkTaggableID reports whether id belongs to a resource tagged via the // NetworkResourceTagger optional interface. diff --git a/server/aws/ec2/tags_eip_vpce_test.go b/server/aws/ec2/tags_eip_vpce_test.go new file mode 100644 index 000000000..1121d5928 --- /dev/null +++ b/server/aws/ec2/tags_eip_vpce_test.go @@ -0,0 +1,583 @@ +package ec2_test + +import ( + "context" + "errors" + "strconv" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/ec2" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + "github.com/aws/smithy-go" +) + +// tagOnlyFilters are the two ways a caller finds its own resource by tag. +func tagOnlyFilters(key, value string) [][]ec2types.Filter { + return [][]ec2types.Filter{ + {{Name: aws.String("tag:" + key), Values: []string{value}}}, + {{Name: aws.String("tag-key"), Values: []string{key}}}, + } +} + +func addressTagged(ctx context.Context, t *testing.T, c *ec2.Client, id string, fs []ec2types.Filter) (map[string]string, bool) { + t.Helper() + + out, err := c.DescribeAddresses(ctx, &ec2.DescribeAddressesInput{Filters: fs}) + if err != nil { + t.Fatalf("DescribeAddresses: %v", err) + } + + for i := range out.Addresses { + if aws.ToString(out.Addresses[i].AllocationId) == id { + return sdkTagMap(out.Addresses[i].Tags), true + } + } + + return nil, false +} + +func endpointTagged(ctx context.Context, t *testing.T, c *ec2.Client, id string, fs []ec2types.Filter) (map[string]string, bool) { + t.Helper() + + out, err := c.DescribeVpcEndpoints(ctx, &ec2.DescribeVpcEndpointsInput{Filters: fs}) + if err != nil { + t.Fatalf("DescribeVpcEndpoints: %v", err) + } + + for i := range out.VpcEndpoints { + if aws.ToString(out.VpcEndpoints[i].VpcEndpointId) == id { + return sdkTagMap(out.VpcEndpoints[i].Tags), true + } + } + + return nil, false +} + +func sdkTagMap(tags []ec2types.Tag) map[string]string { + m := make(map[string]string, len(tags)) + for _, tg := range tags { + m[aws.ToString(tg.Key)] = aws.ToString(tg.Value) + } + + return m +} + +func describeTagsFor(ctx context.Context, t *testing.T, c *ec2.Client, id string) map[string]string { + t.Helper() + + out, err := c.DescribeTags(ctx, &ec2.DescribeTagsInput{ + Filters: []ec2types.Filter{{Name: aws.String("resource-id"), Values: []string{id}}}, + }) + if err != nil { + t.Fatalf("DescribeTags: %v", err) + } + + m := map[string]string{} + for _, td := range out.Tags { + m[aws.ToString(td.Key)] = aws.ToString(td.Value) + "|" + string(td.ResourceType) + } + + return m +} + +// assertTagRoundTrip drives CreateTags -> Describe (by tag filter and via +// DescribeTags) -> DeleteTags -> Describe for one resource id through the real +// SDK client. describe returns the resource's tags and whether the filtered +// Describe call returned it at all. +func assertTagRoundTrip( + ctx context.Context, t *testing.T, c *ec2.Client, id, resourceType string, + describe func(fs []ec2types.Filter) (map[string]string, bool), +) { + t.Helper() + + if _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: []string{id}, + Tags: []ec2types.Tag{{Key: aws.String("owner"), Value: aws.String("team-a")}}, + }); err != nil { + t.Fatalf("CreateTags(%s): %v", id, err) + } + + for _, fs := range tagOnlyFilters("owner", "team-a") { + tags, found := describe(fs) + if !found { + t.Fatalf("%s not returned for filter %s after CreateTags", id, aws.ToString(fs[0].Name)) + } + + if tags["owner"] != "team-a" || tags["Name"] != "created" { + t.Fatalf("%s tags = %v, want owner=team-a merged with create-time Name=created", id, tags) + } + } + + if got := describeTagsFor(ctx, t, c, id); got["owner"] != "team-a|"+resourceType { + t.Fatalf("DescribeTags(%s) = %v, want owner=team-a|%s", id, got, resourceType) + } + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{ + Resources: []string{id}, + Tags: []ec2types.Tag{{Key: aws.String("owner")}}, + }); err != nil { + t.Fatalf("DeleteTags(%s): %v", id, err) + } + + if _, found := describe(tagOnlyFilters("owner", "team-a")[0]); found { + t.Fatalf("%s still matches tag:owner after DeleteTags", id) + } + + tags, found := describe(nil) + if !found || tags["Name"] != "created" { + t.Fatalf("%s after DeleteTags: found=%v tags=%v, want Name=created kept", id, found, tags) + } + + if _, ok := tags["owner"]; ok { + t.Fatalf("%s still carries owner after DeleteTags: %v", id, tags) + } +} + +// TestCreateTagsOnElasticIP pins that an Elastic IP allocation id can be +// tagged and untagged after creation, and that the tags land on the same record +// AllocateAddress TagSpecifications populate. Before the fix, CreateTags on an +// eipalloc- id fell through to the compute tagger and answered InvalidID.NotFound. +func TestCreateTagsOnElasticIP(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + alloc, err := c.AllocateAddress(ctx, &ec2.AllocateAddressInput{ + Domain: ec2types.DomainTypeVpc, + TagSpecifications: []ec2types.TagSpecification{{ + ResourceType: ec2types.ResourceTypeElasticIp, + Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("created")}}, + }}, + }) + if err != nil { + t.Fatalf("AllocateAddress: %v", err) + } + + id := aws.ToString(alloc.AllocationId) + + assertTagRoundTrip(ctx, t, c, id, "elastic-ip", func(fs []ec2types.Filter) (map[string]string, bool) { + return addressTagged(ctx, t, c, id, fs) + }) +} + +// TestCreateTagsOnVPCEndpoint is the vpce- counterpart of +// TestCreateTagsOnElasticIP. +func TestCreateTagsOnVPCEndpoint(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + vpc, err := c.CreateVpc(ctx, &ec2.CreateVpcInput{CidrBlock: aws.String("10.0.0.0/16")}) + if err != nil { + t.Fatalf("CreateVpc: %v", err) + } + + ep, err := c.CreateVpcEndpoint(ctx, &ec2.CreateVpcEndpointInput{ + VpcId: vpc.Vpc.VpcId, + ServiceName: aws.String("com.amazonaws.us-east-1.s3"), + VpcEndpointType: ec2types.VpcEndpointTypeGateway, + TagSpecifications: []ec2types.TagSpecification{{ + ResourceType: ec2types.ResourceTypeVpcEndpoint, + Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("created")}}, + }}, + }) + if err != nil { + t.Fatalf("CreateVpcEndpoint: %v", err) + } + + id := aws.ToString(ep.VpcEndpoint.VpcEndpointId) + + assertTagRoundTrip(ctx, t, c, id, "vpc-endpoint", func(fs []ec2types.Filter) (map[string]string, bool) { + return endpointTagged(ctx, t, c, id, fs) + }) +} + +func assertTagAPIErr(t *testing.T, err error, code, what string) { + t.Helper() + + var apiErr smithy.APIError + if !errors.As(err, &apiErr) || apiErr.ErrorCode() != code { + t.Fatalf("%s err = %v, want %s", what, err, code) + } +} + +// TestCreateTagsOnMissingAddressingIDs pins that tagging or untagging an Elastic +// IP, VPC endpoint or VPC endpoint service id that does not exist fails with the +// resource-specific NotFound code the EC2 error reference defines for it (and +// that this package already returns from the resource's own actions), not a +// silent success and not the generic InvalidID.NotFound. +func TestCreateTagsOnMissingAddressingIDs(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + for id, code := range map[string]string{ + "eipalloc-0000000000000000": "InvalidAllocationID.NotFound", + "vpce-0000000000000000": "InvalidVpcEndpointId.NotFound", + "vpce-svc-0000000000000000": "InvalidVpcEndpointServiceId.NotFound", + } { + _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: []string{id}, + Tags: []ec2types.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }) + assertTagAPIErr(t, err, code, "CreateTags("+id+")") + + _, err = c.DeleteTags(ctx, &ec2.DeleteTagsInput{Resources: []string{id}}) + assertTagAPIErr(t, err, code, "DeleteTags("+id+")") + } +} + +// newTaggedEIP allocates an Elastic IP carrying tags from create time. +func newTaggedEIP(ctx context.Context, t *testing.T, c *ec2.Client, tags map[string]string) string { + t.Helper() + + spec := make([]ec2types.Tag, 0, len(tags)) + for k, v := range tags { + spec = append(spec, ec2types.Tag{Key: aws.String(k), Value: aws.String(v)}) + } + + alloc, err := c.AllocateAddress(ctx, &ec2.AllocateAddressInput{ + Domain: ec2types.DomainTypeVpc, + TagSpecifications: []ec2types.TagSpecification{{ + ResourceType: ec2types.ResourceTypeElasticIp, Tags: spec, + }}, + }) + if err != nil { + t.Fatalf("AllocateAddress: %v", err) + } + + return aws.ToString(alloc.AllocationId) +} + +// newTaggedEndpoint creates a Gateway VPC endpoint carrying tags from create time. +func newTaggedEndpoint(ctx context.Context, t *testing.T, c *ec2.Client, tags map[string]string) string { + t.Helper() + + vpc, err := c.CreateVpc(ctx, &ec2.CreateVpcInput{CidrBlock: aws.String("10.0.0.0/16")}) + if err != nil { + t.Fatalf("CreateVpc: %v", err) + } + + spec := make([]ec2types.Tag, 0, len(tags)) + for k, v := range tags { + spec = append(spec, ec2types.Tag{Key: aws.String(k), Value: aws.String(v)}) + } + + ep, err := c.CreateVpcEndpoint(ctx, &ec2.CreateVpcEndpointInput{ + VpcId: vpc.Vpc.VpcId, + ServiceName: aws.String("com.amazonaws.us-east-1.s3"), + VpcEndpointType: ec2types.VpcEndpointTypeGateway, + TagSpecifications: []ec2types.TagSpecification{{ + ResourceType: ec2types.ResourceTypeVpcEndpoint, Tags: spec, + }}, + }) + if err != nil { + t.Fatalf("CreateVpcEndpoint: %v", err) + } + + return aws.ToString(ep.VpcEndpoint.VpcEndpointId) +} + +// TestDeleteTagsWithoutTagsClearsUserTags pins EC2 DeleteTags with the Tag +// parameter omitted: "we delete all user-defined tags for the specified +// resources". Before the fix it answered success and left every tag in place on +// eipalloc- and vpce- ids, while the same call on an i- id cleared them. +func TestDeleteTagsWithoutTagsClearsUserTags(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + eip := newTaggedEIP(ctx, t, c, map[string]string{"Name": "created", "env": "prod"}) + ep := newTaggedEndpoint(ctx, t, c, map[string]string{"Name": "created", "env": "prod"}) + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{Resources: []string{eip, ep}}); err != nil { + t.Fatalf("DeleteTags: %v", err) + } + + if tags, _ := addressTagged(ctx, t, c, eip, nil); len(tags) != 0 { + t.Fatalf("eip tags after DeleteTags with no Tag = %v, want none", tags) + } + + if tags, _ := endpointTagged(ctx, t, c, ep, nil); len(tags) != 0 { + t.Fatalf("endpoint tags after DeleteTags with no Tag = %v, want none", tags) + } +} + +// TestDeleteTagsMatchesValue pins the DeleteTags Tag.N.Value rule: a key sent +// without a value deletes the tag whatever its value, a key sent with a value +// deletes it only when the value matches, and an explicit empty value matches +// only an empty-valued tag. Before the fix Key=env,Value=wrong deleted env. +func TestDeleteTagsMatchesValue(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + eip := newTaggedEIP(ctx, t, c, map[string]string{"env": "prod", "blank": "", "owner": "a"}) + + del := func(tags ...ec2types.Tag) { + t.Helper() + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{Resources: []string{eip}, Tags: tags}); err != nil { + t.Fatalf("DeleteTags: %v", err) + } + } + + del(ec2types.Tag{Key: aws.String("env"), Value: aws.String("wrong")}, + ec2types.Tag{Key: aws.String("owner"), Value: aws.String("")}) + + if tags, _ := addressTagged(ctx, t, c, eip, nil); tags["env"] != "prod" || tags["owner"] != "a" { + t.Fatalf("tags after value-mismatched DeleteTags = %v, want env=prod and owner=a kept", tags) + } + + del(ec2types.Tag{Key: aws.String("env"), Value: aws.String("prod")}, + ec2types.Tag{Key: aws.String("blank"), Value: aws.String("")}) + + tags, _ := addressTagged(ctx, t, c, eip, nil) + if _, ok := tags["env"]; ok { + t.Fatalf("env kept after DeleteTags Key=env,Value=prod: %v", tags) + } + + if _, ok := tags["blank"]; ok { + t.Fatalf("blank kept after DeleteTags Key=blank,Value=\"\": %v", tags) + } + + del(ec2types.Tag{Key: aws.String("owner")}) + + if tags, _ := addressTagged(ctx, t, c, eip, nil); len(tags) != 0 { + t.Fatalf("tags after key-only DeleteTags = %v, want none", tags) + } +} + +// TestTagBatchIsAtomic pins that CreateTags and DeleteTags check every +// ResourceId before writing: a batch naming one unknown id fails with nothing +// changed on the ids that do exist. Before the fix the EIP listed ahead of the +// bogus id was tagged (and untagged) anyway. +func TestTagBatchIsAtomic(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + eip := newTaggedEIP(ctx, t, c, map[string]string{"Name": "created"}) + ep := newTaggedEndpoint(ctx, t, c, map[string]string{"Name": "created"}) + batch := []string{eip, "vpce-0000000000000bad", ep} + + _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: batch, + Tags: []ec2types.Tag{{Key: aws.String("owner"), Value: aws.String("a")}}, + }) + assertTagAPIErr(t, err, "InvalidVpcEndpointId.NotFound", "CreateTags(mixed batch)") + + if tags, _ := addressTagged(ctx, t, c, eip, nil); tags["owner"] != "" { + t.Fatalf("eip tagged by a failed CreateTags batch: %v", tags) + } + + _, err = c.DeleteTags(ctx, &ec2.DeleteTagsInput{ + Resources: batch, + Tags: []ec2types.Tag{{Key: aws.String("Name")}}, + }) + assertTagAPIErr(t, err, "InvalidVpcEndpointId.NotFound", "DeleteTags(mixed batch)") + + if tags, _ := addressTagged(ctx, t, c, eip, nil); tags["Name"] != "created" { + t.Fatalf("eip untagged by a failed DeleteTags batch: %v", tags) + } +} + +// numberedTags returns n tags k..k. +func numberedTags(start, n int) []ec2types.Tag { + out := make([]ec2types.Tag, 0, n) + for i := start; i < start+n; i++ { + out = append(out, ec2types.Tag{Key: aws.String("k" + strconv.Itoa(i)), Value: aws.String("v")}) + } + + return out +} + +// TestCreateTagsCountsExistingTags pins the 50-tag limit per resource: the tags +// a resource already carries count toward it (an overwritten key counts once, +// and "aws:" tags do not count). Before the fix only the request was counted, so +// two CreateTags calls could leave a resource with more than 50 tags. +func TestCreateTagsCountsExistingTags(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + eip := newTaggedEIP(ctx, t, c, map[string]string{"Name": "created"}) + + create := func(tags []ec2types.Tag) error { + _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{Resources: []string{eip}, Tags: tags}) + return err + } + + if err := create(numberedTags(0, 40)); err != nil { + t.Fatalf("CreateTags(40): %v", err) + } + + // 41 on the resource + 10 new = 51. + assertTagAPIErr(t, create(numberedTags(40, 10)), "TagLimitExceeded", "CreateTags(41+10)") + + // 41 + 9 new + overwrite of k0 = 50, the limit itself. + if err := create(append(numberedTags(40, 9), numberedTags(0, 1)...)); err != nil { + t.Fatalf("CreateTags to exactly 50: %v", err) + } + + if tags, _ := addressTagged(ctx, t, c, eip, nil); len(tags) != 50 { + t.Fatalf("eip has %d tags, want 50", len(tags)) + } + + assertTagAPIErr(t, create(numberedTags(100, 1)), "TagLimitExceeded", "CreateTags(51st)") +} + +// TestCreateTagsRejectsOversizedKeyAndValue pins the EC2 tag restrictions of +// 128 characters per key and 256 per value, counted in Unicode characters, not +// bytes. Before the fix any length was accepted. +func TestCreateTagsRejectsOversizedKeyAndValue(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + eip := newTaggedEIP(ctx, t, c, nil) + + create := func(k, v string) error { + _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: []string{eip}, + Tags: []ec2types.Tag{{Key: aws.String(k), Value: aws.String(v)}}, + }) + + return err + } + + if err := create(strings.Repeat("é", 128), strings.Repeat("é", 256)); err != nil { + t.Fatalf("CreateTags at the 128/256-character limits: %v", err) + } + + assertTagAPIErr(t, create(strings.Repeat("k", 129), "v"), "InvalidParameterValue", "CreateTags(129-char key)") + assertTagAPIErr(t, create("k", strings.Repeat("v", 257)), "InvalidParameterValue", "CreateTags(257-char value)") +} + +// TestTagsOnVPCEndpointServiceOverTheWire covers the vpce-svc- id on the wire: +// CreateTags/DeleteTags reach the endpoint service record, and DescribeTags +// reports it as resource-type vpc-endpoint-service. It also pins the +// resource-type filter: each type returns only its own resources. +func TestTagsOnVPCEndpointServiceOverTheWire(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + svcOut, err := c.CreateVpcEndpointServiceConfiguration(ctx, &ec2.CreateVpcEndpointServiceConfigurationInput{ + NetworkLoadBalancerArns: []string{"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/net/n/1"}, + }) + if err != nil { + t.Fatalf("CreateVpcEndpointServiceConfiguration: %v", err) + } + + svc := aws.ToString(svcOut.ServiceConfiguration.ServiceId) + eip := newTaggedEIP(ctx, t, c, nil) + ep := newTaggedEndpoint(ctx, t, c, nil) + + if _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: []string{svc, eip, ep}, + Tags: []ec2types.Tag{{Key: aws.String("owner"), Value: aws.String("a")}}, + }); err != nil { + t.Fatalf("CreateTags: %v", err) + } + + byType := func(types ...string) map[string]string { + t.Helper() + + out, err := c.DescribeTags(ctx, &ec2.DescribeTagsInput{Filters: []ec2types.Filter{ + {Name: aws.String("resource-type"), Values: types}, + {Name: aws.String("key"), Values: []string{"owner"}}, + }}) + if err != nil { + t.Fatalf("DescribeTags(%v): %v", types, err) + } + + got := map[string]string{} + for _, td := range out.Tags { + got[aws.ToString(td.ResourceId)] = string(td.ResourceType) + } + + return got + } + + for typ, id := range map[string]string{"vpc-endpoint-service": svc, "elastic-ip": eip, "vpc-endpoint": ep} { + if got := byType(typ); len(got) != 1 || got[id] != typ { + t.Fatalf("DescribeTags(resource-type=%s) = %v, want only %s", typ, got, id) + } + } + + if got := byType("vpc-endpoint-service", "elastic-ip"); len(got) != 2 || got[svc] == "" || got[eip] == "" { + t.Fatalf("DescribeTags(resource-type=vpc-endpoint-service,elastic-ip) = %v, want %s and %s", got, svc, eip) + } + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{ + Resources: []string{svc}, Tags: []ec2types.Tag{{Key: aws.String("owner")}}, + }); err != nil { + t.Fatalf("DeleteTags(%s): %v", svc, err) + } + + cfgs, err := c.DescribeVpcEndpointServiceConfigurations(ctx, &ec2.DescribeVpcEndpointServiceConfigurationsInput{ + ServiceIds: []string{svc}, + }) + if err != nil || len(cfgs.ServiceConfigurations) != 1 { + t.Fatalf("DescribeVpcEndpointServiceConfigurations: %v %v", cfgs, err) + } + + if tags := sdkTagMap(cfgs.ServiceConfigurations[0].Tags); len(tags) != 0 { + t.Fatalf("endpoint service tags after DeleteTags = %v, want none", tags) + } +} + +// TestDeleteTagsSemanticsOnEveryTagger drives the same DeleteTags rules through +// the other taggers the handler dispatches to (compute for i-, the networking +// provider's own methods for vpc- and subnet-): a value mismatch keeps the tag, +// and omitting Tag clears every user tag. +func TestDeleteTagsSemanticsOnEveryTagger(t *testing.T) { + ctx := context.Background() + c, _ := newTagServer(t) + + run, err := c.RunInstances(ctx, &ec2.RunInstancesInput{ + ImageId: aws.String("ami-123"), InstanceType: ec2types.InstanceTypeT2Micro, + MinCount: aws.Int32(1), MaxCount: aws.Int32(1), + }) + if err != nil { + t.Fatalf("RunInstances: %v", err) + } + + vpc, err := c.CreateVpc(ctx, &ec2.CreateVpcInput{CidrBlock: aws.String("10.1.0.0/16")}) + if err != nil { + t.Fatalf("CreateVpc: %v", err) + } + + subnet, err := c.CreateSubnet(ctx, &ec2.CreateSubnetInput{VpcId: vpc.Vpc.VpcId, CidrBlock: aws.String("10.1.1.0/24")}) + if err != nil { + t.Fatalf("CreateSubnet: %v", err) + } + + ids := []string{aws.ToString(run.Instances[0].InstanceId), aws.ToString(vpc.Vpc.VpcId), aws.ToString(subnet.Subnet.SubnetId)} + + if _, err := c.CreateTags(ctx, &ec2.CreateTagsInput{ + Resources: ids, + Tags: []ec2types.Tag{ + {Key: aws.String("env"), Value: aws.String("prod")}, + {Key: aws.String("owner"), Value: aws.String("a")}, + }, + }); err != nil { + t.Fatalf("CreateTags: %v", err) + } + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{ + Resources: ids, Tags: []ec2types.Tag{{Key: aws.String("env"), Value: aws.String("wrong")}}, + }); err != nil { + t.Fatalf("DeleteTags(value mismatch): %v", err) + } + + for _, id := range ids { + if got := describeTagsFor(ctx, t, c, id); len(got) != 2 { + t.Fatalf("%s tags after value-mismatched DeleteTags = %v, want env and owner kept", id, got) + } + } + + if _, err := c.DeleteTags(ctx, &ec2.DeleteTagsInput{Resources: ids}); err != nil { + t.Fatalf("DeleteTags(no Tag): %v", err) + } + + for _, id := range ids { + if got := describeTagsFor(ctx, t, c, id); len(got) != 0 { + t.Fatalf("%s tags after DeleteTags with no Tag = %v, want none", id, got) + } + } +} diff --git a/server/aws/ec2/tags_internal_test.go b/server/aws/ec2/tags_internal_test.go new file mode 100644 index 000000000..8e6a7b617 --- /dev/null +++ b/server/aws/ec2/tags_internal_test.go @@ -0,0 +1,42 @@ +package ec2 + +import ( + "slices" + "testing" +) + +// TestDeleteTagKeys covers how a DeleteTags request resolves against one +// resource's current tags, including the pass-through used when the owning +// provider cannot report tags (existing == nil). +func TestDeleteTagKeys(t *testing.T) { + existing := map[string]string{"env": "prod", "blank": "", "aws:managed": "x"} + + cases := []struct { + name string + existing map[string]string + specs []deleteTagSpec + want []string + write bool + }{ + {"no Tag keeps aws: tags", existing, nil, []string{"blank", "env"}, true}, + {"key only", existing, []deleteTagSpec{{key: "env"}}, []string{"env"}, true}, + {"value match", existing, []deleteTagSpec{{key: "env", value: "prod", hasValue: true}}, []string{"env"}, true}, + {"value mismatch", existing, []deleteTagSpec{{key: "env", value: "dev", hasValue: true}}, nil, false}, + {"empty value matches only empty", existing, []deleteTagSpec{ + {key: "blank", hasValue: true}, {key: "env", hasValue: true}, + }, []string{"blank"}, true}, + {"absent key", existing, []deleteTagSpec{{key: "missing"}}, nil, false}, + {"no user tags left", map[string]string{"aws:managed": "x"}, nil, nil, false}, + {"unknown tags pass keys through", nil, []deleteTagSpec{{key: "env", value: "v", hasValue: true}}, []string{"env"}, true}, + {"unknown tags, no Tag", nil, nil, []string{}, true}, + } + + for _, tc := range cases { + keys, write := deleteTagKeys(tc.existing, tc.specs) + slices.Sort(keys) + + if write != tc.write || !slices.Equal(keys, tc.want) { + t.Errorf("%s: deleteTagKeys = %v, %v; want %v, %v", tc.name, keys, write, tc.want, tc.write) + } + } +} diff --git a/services/networking/driver/aws_capabilities.go b/services/networking/driver/aws_capabilities.go index 323c338d6..f9ce4a2e5 100644 --- a/services/networking/driver/aws_capabilities.go +++ b/services/networking/driver/aws_capabilities.go @@ -670,7 +670,9 @@ type VPCBlockPublicAccess interface { // It tags VPC-family resources that have no dedicated Update*Tags method on the // portable Networking interface: route tables, internet gateways, NAT gateways, // network ACLs, DHCP option sets, peering connections, managed prefix lists, and -// egress-only internet gateways. The EC2 tag handler routes those id prefixes here. +// egress-only internet gateways, security-group rules, Elastic IP allocations, +// VPC endpoints and VPC endpoint services. The EC2 tag handler routes those id +// prefixes here. type NetworkResourceTagger interface { UpdateResourceTags(ctx context.Context, id string, tags map[string]string) error RemoveResourceTags(ctx context.Context, id string, keys []string) error