From 81212fe91b9473c62d4badeb749e02e69725a491 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:56:41 +1000 Subject: [PATCH] feat: EC2 central-record resource tagging (#469) --- docs/COMMANDS.md | 14 +- spinifex/daemon/daemon_handlers_tag.go | 72 ++++++--- .../handlers/ec2/eigw/record_tags_test.go | 64 ++++++++ spinifex/handlers/ec2/eigw/service_impl.go | 23 +++ spinifex/handlers/ec2/eip/record_tags_test.go | 66 ++++++++ spinifex/handlers/ec2/eip/service_impl.go | 23 +++ spinifex/handlers/ec2/igw/record_tags_test.go | 64 ++++++++ spinifex/handlers/ec2/igw/service_impl.go | 23 +++ spinifex/handlers/ec2/image/service_impl.go | 27 +--- .../ec2/key/create_tags_intake_test.go | 94 +++++++++++ spinifex/handlers/ec2/key/record_tags_test.go | 85 ++++++++++ spinifex/handlers/ec2/key/service_impl.go | 70 +++++++- .../handlers/ec2/natgw/record_tags_test.go | 64 ++++++++ spinifex/handlers/ec2/natgw/service_impl.go | 23 +++ .../placementgroup/create_tags_intake_test.go | 96 +++++++++++ .../ec2/placementgroup/service_impl.go | 29 +++- .../ec2/routetable/record_tags_test.go | 66 ++++++++ .../handlers/ec2/routetable/service_impl.go | 23 +++ .../handlers/ec2/snapshot/record_tags_test.go | 66 ++++++++ .../handlers/ec2/snapshot/service_impl.go | 49 ++++++ spinifex/handlers/ec2/tags/service_impl.go | 15 ++ .../handlers/ec2/tags/service_impl_test.go | 5 + .../handlers/ec2/volume/record_tags_test.go | 70 ++++++++ spinifex/handlers/ec2/volume/service_impl.go | 53 ++++++ spinifex/handlers/ec2/vpc/service_impl.go | 75 +++------ .../handlers/ec2/vpc/service_impl_test.go | 75 +++++++++ spinifex/utils/tagmirror.go | 101 ++++++++++++ spinifex/utils/tagmirror_test.go | 152 ++++++++++++++++++ 28 files changed, 1478 insertions(+), 109 deletions(-) create mode 100644 spinifex/handlers/ec2/eigw/record_tags_test.go create mode 100644 spinifex/handlers/ec2/eip/record_tags_test.go create mode 100644 spinifex/handlers/ec2/igw/record_tags_test.go create mode 100644 spinifex/handlers/ec2/key/create_tags_intake_test.go create mode 100644 spinifex/handlers/ec2/key/record_tags_test.go create mode 100644 spinifex/handlers/ec2/natgw/record_tags_test.go create mode 100644 spinifex/handlers/ec2/placementgroup/create_tags_intake_test.go create mode 100644 spinifex/handlers/ec2/routetable/record_tags_test.go create mode 100644 spinifex/handlers/ec2/snapshot/record_tags_test.go create mode 100644 spinifex/handlers/ec2/volume/record_tags_test.go create mode 100644 spinifex/utils/tagmirror.go create mode 100644 spinifex/utils/tagmirror_test.go diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 580422238..af50ba7f2 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -155,10 +155,10 @@ Spot Instance Requests (SIRs) are a **mock** over the on-demand `run-instances` | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-key-pair` | `--key-name`, `--key-type` (rsa/ed25519) | `--key-format`, `--tag-specifications`, `--dry-run` | **DONE** | +| `create-key-pair` | `--key-name`, `--key-type` (rsa/ed25519), `--tag-specifications` | `--key-format`, `--dry-run` | **DONE** | | `describe-key-pairs` | `--key-names`, `--key-pair-ids`, `--filters` (key-pair-id, key-name, fingerprint, tag:*) | `--max-results`, `--dry-run` | **DONE** | | `delete-key-pair` | `--key-name`, `--key-pair-id` | `--dry-run` | **DONE** | -| `import-key-pair` | `--key-name`, `--public-key-material` | `--tag-specifications`, `--dry-run` | **DONE** | +| `import-key-pair` | `--key-name`, `--public-key-material`, `--tag-specifications` | `--dry-run` | **DONE** | ### EC2 — AMI Images @@ -181,7 +181,7 @@ Spot Instance Requests (SIRs) are a **mock** over the on-demand `run-instances` | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| | `describe-volumes` | `--volume-ids`, `--filters` (volume-id, status, size, volume-type, attachment.instance-id, attachment.status, attachment.device, availability-zone, tag:*), persisted `DeleteOnTermination` | `--max-results`, `--next-token`, `--dry-run` | **DONE** | -| `create-volume` | `--size`, `--availability-zone`, `--volume-type` (gp3), `--snapshot-id` | `--iops` (hardcoded 3000), `--encrypted` (hardcoded false), `--throughput`, `--tag-specifications` | **DONE** | +| `create-volume` | `--size`, `--availability-zone`, `--volume-type` (gp3), `--snapshot-id`, `--tag-specifications` | `--iops` (hardcoded 3000), `--encrypted` (hardcoded false), `--throughput` | **DONE** | | `delete-volume` | `--volume-id` | `--dry-run` | **DONE** | | `modify-volume` | `--volume-id`, `--size`, `--volume-type`, `--iops` | `--throughput`, `--dry-run`, `--multi-attach-enabled` | **DONE** | | `attach-volume` | `--volume-id`, `--instance-id`, `--device` (auto-assigns `/dev/sd[f-p]`) | `--dry-run` | **DONE** | @@ -293,7 +293,7 @@ KV CRUD only — no OVN/OVS integration. EIGWs are stored but have no effect on | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-route-table` | `--vpc-id` | `--tag-specifications`, `--dry-run` | **DONE** | +| `create-route-table` | `--vpc-id`, `--tag-specifications` | `--dry-run` | **DONE** | | `delete-route-table` | `--route-table-id` | `--dry-run` | **DONE** | | `describe-route-tables` | `--route-table-ids`, `--filters` (vpc-id, route-table-id, association.main, association.route-table-association-id, association.subnet-id, route.destination-cidr-block, route.gateway-id) | `--max-results`, `--next-token`, `--dry-run` | **DONE** | | `create-route` | `--route-table-id`, `--destination-cidr-block`, `--gateway-id`, `--nat-gateway-id` | `--egress-only-internet-gateway-id`, `--vpc-peering-connection-id`, `--dry-run` | **DONE** | @@ -336,7 +336,7 @@ EIP commands are only registered when an external IPAM pool is configured. | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-nat-gateway` | `--subnet-id`, `--allocation-id` | `--connectivity-type`, `--tag-specifications`, `--dry-run` | **DONE** | +| `create-nat-gateway` | `--subnet-id`, `--allocation-id`, `--tag-specifications` | `--connectivity-type`, `--dry-run` | **DONE** | | `delete-nat-gateway` | `--nat-gateway-id` | `--dry-run` | **DONE** | | `describe-nat-gateways` | `--nat-gateway-ids`, `--filters` (vpc-id, state) | `--max-results`, `--next-token`, `--dry-run` | **DONE** | | `assign-private-nat-gateway-address` | — | `--nat-gateway-id`, `--private-ip-addresses` | **NOT STARTED** | @@ -346,9 +346,9 @@ EIP commands are only registered when an external IPAM pool is configured. | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| -| `create-placement-group` | `--group-name`, `--strategy` (spread/cluster) | `--partition-count`, `--spread-level`, `--tag-specifications`, `--dry-run` | **DONE** | +| `create-placement-group` | `--group-name`, `--strategy` (spread/cluster), `--tag-specifications` | `--partition-count`, `--spread-level`, `--dry-run` | **DONE** | | `delete-placement-group` | `--group-name` | `--dry-run` | **DONE** | -| `describe-placement-groups` | `--group-names`, `--group-ids`, `--filters` (strategy, state, spread-level, group-name) | `--dry-run` | **DONE** | +| `describe-placement-groups` | `--group-names`, `--group-ids`, `--filters` (strategy, state, spread-level, group-name, tag:*, tag-key, tag-value) | `--dry-run` | **DONE** | ### EC2 — VPC Peering diff --git a/spinifex/daemon/daemon_handlers_tag.go b/spinifex/daemon/daemon_handlers_tag.go index 372c5e64b..9845a931a 100644 --- a/spinifex/daemon/daemon_handlers_tag.go +++ b/spinifex/daemon/daemon_handlers_tag.go @@ -17,41 +17,75 @@ func (d *Daemon) handleEC2DescribeTags(msg *nats.Msg) { handleNATSRequest(msg, d.tagsService.DescribeTags) } -// createTags writes the generic tag store, then mirrors tags into owning records -// so tag-filtered describes observe them: subnet/vpc (e.g. LBC subnet -// auto-discovery via DescribeSubnets) and AMIs (DescribeImages). +// recordTagMirror is implemented by every service whose resource records +// mirror the central tag store, so tag-filtered describes observe tags +// written through create-tags/delete-tags. +type recordTagMirror interface { + ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error + RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error +} + +// recordTagMirrors collects the initialized services owning centrally-stored +// records: vpc covers vpc/subnet/sg/eni, the rest own their prefix. +func (d *Daemon) recordTagMirrors() []recordTagMirror { + var mirrors []recordTagMirror + if d.vpcService != nil { + mirrors = append(mirrors, d.vpcService) + } + if d.imageService != nil { + mirrors = append(mirrors, d.imageService) + } + if d.volumeService != nil { + mirrors = append(mirrors, d.volumeService) + } + if d.snapshotService != nil { + mirrors = append(mirrors, d.snapshotService) + } + if d.routeTableService != nil { + mirrors = append(mirrors, d.routeTableService) + } + if d.igwService != nil { + mirrors = append(mirrors, d.igwService) + } + if d.eigwService != nil { + mirrors = append(mirrors, d.eigwService) + } + if d.eipService != nil { + mirrors = append(mirrors, d.eipService) + } + if d.natGatewayService != nil { + mirrors = append(mirrors, d.natGatewayService) + } + if d.keyService != nil { + mirrors = append(mirrors, d.keyService) + } + return mirrors +} + +// createTags writes the generic tag store, then mirrors tags into the owning +// records so tag-filtered describes observe them. func (d *Daemon) createTags(input *ec2.CreateTagsInput, accountID string) (*ec2.CreateTagsOutput, error) { out, err := d.tagsService.CreateTags(input, accountID) if err != nil { return nil, err } - if d.vpcService != nil { - if err := d.vpcService.ApplyRecordTags(input, accountID); err != nil { - return nil, err - } - } - if d.imageService != nil { - if err := d.imageService.ApplyRecordTags(input, accountID); err != nil { + for _, m := range d.recordTagMirrors() { + if err := m.ApplyRecordTags(input, accountID); err != nil { return nil, err } } return out, nil } -// deleteTags removes from the generic tag store, then mirrors the removal into -// the owning subnet/vpc records and AMI configs. +// deleteTags removes from the generic tag store, then mirrors the removal +// into the owning records. func (d *Daemon) deleteTags(input *ec2.DeleteTagsInput, accountID string) (*ec2.DeleteTagsOutput, error) { out, err := d.tagsService.DeleteTags(input, accountID) if err != nil { return nil, err } - if d.vpcService != nil { - if err := d.vpcService.RemoveRecordTags(input, accountID); err != nil { - return nil, err - } - } - if d.imageService != nil { - if err := d.imageService.RemoveRecordTags(input, accountID); err != nil { + for _, m := range d.recordTagMirrors() { + if err := m.RemoveRecordTags(input, accountID); err != nil { return nil, err } } diff --git a/spinifex/handlers/ec2/eigw/record_tags_test.go b/spinifex/handlers/ec2/eigw/record_tags_test.go new file mode 100644 index 000000000..366fe65e9 --- /dev/null +++ b/spinifex/handlers/ec2/eigw/record_tags_test.go @@ -0,0 +1,64 @@ +package handlers_ec2_eigw + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func eigwTagVal(tags []*ec2.Tag, key string) string { + for _, tg := range tags { + if tg.Key != nil && *tg.Key == key { + return aws.StringValue(tg.Value) + } + } + return "" +} + +func TestEIGWRecordTagsMirror(t *testing.T) { + svc := setupTestEIGWService(t) + eigwID := createTestEIGW(t, svc) + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(eigwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeEgressOnlyInternetGateways(&ec2.DescribeEgressOnlyInternetGatewaysInput{ + EgressOnlyInternetGatewayIds: []*string{aws.String(eigwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.EgressOnlyInternetGateways, 1) + assert.Equal(t, "yes", eigwTagVal(out.EgressOnlyInternetGateways[0].Tags, "keep")) + assert.Equal(t, "v", eigwTagVal(out.EgressOnlyInternetGateways[0].Tags, "drop")) + + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(eigwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err = svc.DescribeEgressOnlyInternetGateways(&ec2.DescribeEgressOnlyInternetGatewaysInput{ + EgressOnlyInternetGatewayIds: []*string{aws.String(eigwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.EgressOnlyInternetGateways, 1) + assert.Equal(t, "yes", eigwTagVal(out.EgressOnlyInternetGateways[0].Tags, "keep")) + assert.Equal(t, "", eigwTagVal(out.EgressOnlyInternetGateways[0].Tags, "drop")) +} + +func TestEIGWRecordTagsMirror_UnownedNoError(t *testing.T) { + svc := setupTestEIGWService(t) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("eigw-missing"), aws.String("igw-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/eigw/service_impl.go b/spinifex/handlers/ec2/eigw/service_impl.go index 857e998d8..04fb28a93 100644 --- a/spinifex/handlers/ec2/eigw/service_impl.go +++ b/spinifex/handlers/ec2/eigw/service_impl.go @@ -245,3 +245,26 @@ func (s *EgressOnlyIGWServiceImpl) recordToEC2(record *EgressOnlyIGWRecord) *ec2 return eigw } + +// ApplyRecordTags mirrors CreateTags into the owning egress-only IGW KV +// record so tag-filtered describes observe tags added after create. Resource +// ids this service does not own are skipped; absent records are a no-op. +func (s *EgressOnlyIGWServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.eigwKV, accountID, "eigw-", input.Resources, + func(r *EgressOnlyIGWRecord) *map[string]string { return &r.Tags }, + utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning egress-only IGW KV +// record with AWS-faithful delete semantics. +func (s *EgressOnlyIGWServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.eigwKV, accountID, "eigw-", input.Resources, + func(r *EgressOnlyIGWRecord) *map[string]string { return &r.Tags }, + utils.RemoveTagsMut(input)) +} diff --git a/spinifex/handlers/ec2/eip/record_tags_test.go b/spinifex/handlers/ec2/eip/record_tags_test.go new file mode 100644 index 000000000..e9d384078 --- /dev/null +++ b/spinifex/handlers/ec2/eip/record_tags_test.go @@ -0,0 +1,66 @@ +package handlers_ec2_eip + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func eipTagVal(tags []*ec2.Tag, key string) string { + for _, tg := range tags { + if tg.Key != nil && *tg.Key == key { + return aws.StringValue(tg.Value) + } + } + return "" +} + +func TestEIPRecordTagsMirror(t *testing.T) { + svc, _ := setupTestEIP(t) + alloc, err := svc.AllocateAddress(&ec2.AllocateAddressInput{}, testAccountID) + require.NoError(t, err) + allocID := *alloc.AllocationId + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(allocID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeAddresses(&ec2.DescribeAddressesInput{ + AllocationIds: []*string{aws.String(allocID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Addresses, 1) + assert.Equal(t, "yes", eipTagVal(out.Addresses[0].Tags, "keep")) + assert.Equal(t, "v", eipTagVal(out.Addresses[0].Tags, "drop")) + + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(allocID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err = svc.DescribeAddresses(&ec2.DescribeAddressesInput{ + AllocationIds: []*string{aws.String(allocID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.Addresses, 1) + assert.Equal(t, "yes", eipTagVal(out.Addresses[0].Tags, "keep")) + assert.Equal(t, "", eipTagVal(out.Addresses[0].Tags, "drop")) +} + +func TestEIPRecordTagsMirror_UnownedNoError(t *testing.T) { + svc, _ := setupTestEIP(t) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("eipalloc-missing"), aws.String("nat-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/eip/service_impl.go b/spinifex/handlers/ec2/eip/service_impl.go index a4299bd32..d1614ebcf 100644 --- a/spinifex/handlers/ec2/eip/service_impl.go +++ b/spinifex/handlers/ec2/eip/service_impl.go @@ -692,3 +692,26 @@ func (s *EIPServiceImpl) eipRecordToEC2(record *EIPRecord) *ec2.Address { return addr } + +// ApplyRecordTags mirrors CreateTags into the owning EIP KV record so +// tag-filtered describes observe tags added after create. Resource ids this +// service does not own are skipped; absent records are a no-op. +func (s *EIPServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.eipKV, accountID, "eipalloc-", input.Resources, + func(r *EIPRecord) *map[string]string { return &r.Tags }, + utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning EIP KV record with +// AWS-faithful delete semantics. +func (s *EIPServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.eipKV, accountID, "eipalloc-", input.Resources, + func(r *EIPRecord) *map[string]string { return &r.Tags }, + utils.RemoveTagsMut(input)) +} diff --git a/spinifex/handlers/ec2/igw/record_tags_test.go b/spinifex/handlers/ec2/igw/record_tags_test.go new file mode 100644 index 000000000..1c296194a --- /dev/null +++ b/spinifex/handlers/ec2/igw/record_tags_test.go @@ -0,0 +1,64 @@ +package handlers_ec2_igw + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func igwTagVal(tags []*ec2.Tag, key string) string { + for _, tg := range tags { + if tg.Key != nil && *tg.Key == key { + return aws.StringValue(tg.Value) + } + } + return "" +} + +func TestIGWRecordTagsMirror(t *testing.T) { + svc, _ := setupTestIGWService(t) + igwID := createTestIGW(t, svc) + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(igwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ + InternetGatewayIds: []*string{aws.String(igwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.InternetGateways, 1) + assert.Equal(t, "yes", igwTagVal(out.InternetGateways[0].Tags, "keep")) + assert.Equal(t, "v", igwTagVal(out.InternetGateways[0].Tags, "drop")) + + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(igwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err = svc.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ + InternetGatewayIds: []*string{aws.String(igwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.InternetGateways, 1) + assert.Equal(t, "yes", igwTagVal(out.InternetGateways[0].Tags, "keep")) + assert.Equal(t, "", igwTagVal(out.InternetGateways[0].Tags, "drop")) +} + +func TestIGWRecordTagsMirror_UnownedNoError(t *testing.T) { + svc, _ := setupTestIGWService(t) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("igw-missing"), aws.String("eigw-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/igw/service_impl.go b/spinifex/handlers/ec2/igw/service_impl.go index 99da029bd..f4732c722 100644 --- a/spinifex/handlers/ec2/igw/service_impl.go +++ b/spinifex/handlers/ec2/igw/service_impl.go @@ -427,3 +427,26 @@ func (s *IGWServiceImpl) recordToEC2(record *IGWRecord) *ec2.InternetGateway { return igw } + +// ApplyRecordTags mirrors CreateTags into the owning IGW KV record so +// tag-filtered describes observe tags added after create. Resource ids this +// service does not own are skipped; absent records are a no-op. +func (s *IGWServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.igwKV, accountID, "igw-", input.Resources, + func(r *IGWRecord) *map[string]string { return &r.Tags }, + utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning IGW KV record with +// AWS-faithful delete semantics. +func (s *IGWServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.igwKV, accountID, "igw-", input.Resources, + func(r *IGWRecord) *map[string]string { return &r.Tags }, + utils.RemoveTagsMut(input)) +} diff --git a/spinifex/handlers/ec2/image/service_impl.go b/spinifex/handlers/ec2/image/service_impl.go index c8c1847f7..10cc0e972 100644 --- a/spinifex/handlers/ec2/image/service_impl.go +++ b/spinifex/handlers/ec2/image/service_impl.go @@ -701,13 +701,7 @@ func (s *ImageServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID if input == nil { return nil } - merge := func(tags map[string]string) { - for _, t := range input.Tags { - if t.Key != nil && t.Value != nil { - tags[*t.Key] = *t.Value - } - } - } + merge := utils.MergeTagsMut(input) for _, res := range input.Resources { if res == nil || !strings.HasPrefix(*res, "ami-") { continue @@ -726,24 +720,7 @@ func (s *ImageServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountI if input == nil { return nil } - remove := func(tags map[string]string) { - if len(input.Tags) == 0 { - for k := range tags { - delete(tags, k) - } - return - } - for _, t := range input.Tags { - if t.Key == nil { - continue - } - if t.Value == nil { - delete(tags, *t.Key) - } else if cur, ok := tags[*t.Key]; ok && cur == *t.Value { - delete(tags, *t.Key) - } - } - } + remove := utils.RemoveTagsMut(input) for _, res := range input.Resources { if res == nil || !strings.HasPrefix(*res, "ami-") { continue diff --git a/spinifex/handlers/ec2/key/create_tags_intake_test.go b/spinifex/handlers/ec2/key/create_tags_intake_test.go new file mode 100644 index 000000000..78cf5c328 --- /dev/null +++ b/spinifex/handlers/ec2/key/create_tags_intake_test.go @@ -0,0 +1,94 @@ +package handlers_ec2_key + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/filterutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func keyPairTagSpecs(resourceType string) []*ec2.TagSpecification { + return []*ec2.TagSpecification{{ + ResourceType: aws.String(resourceType), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("build-key")}, + {Key: aws.String("env"), Value: aws.String("dev")}, + }, + }} +} + +func TestCreateKeyPair_TagSpecifications(t *testing.T) { + requireSSHKeygen(t) + svc, _ := newTestKeyService() + + out, err := svc.CreateKeyPair(&ec2.CreateKeyPairInput{ + KeyName: aws.String("tagged-create-key"), + TagSpecifications: keyPairTagSpecs("key-pair"), + }, testAccountID) + require.NoError(t, err) + + tags := filterutil.EC2TagsToMap(out.Tags) + assert.Equal(t, "build-key", tags["Name"]) + assert.Equal(t, "dev", tags["env"]) + + // Persisted metadata round-trips through DescribeKeyPairs. + desc, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + KeyPairIds: []*string{out.KeyPairId}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.KeyPairs, 1) + descTags := filterutil.EC2TagsToMap(desc.KeyPairs[0].Tags) + assert.Equal(t, "build-key", descTags["Name"]) + assert.Equal(t, "dev", descTags["env"]) +} + +func TestImportKeyPair_TagSpecifications(t *testing.T) { + svc, _ := newTestKeyService() + + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("tagged-import-key"), + PublicKeyMaterial: []byte(testED25519PubKey), + TagSpecifications: keyPairTagSpecs("key-pair"), + }, testAccountID) + require.NoError(t, err) + + tags := filterutil.EC2TagsToMap(out.Tags) + assert.Equal(t, "build-key", tags["Name"]) + + // Tag filter matches the imported key. + desc, err := svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + Filters: []*ec2.Filter{{ + Name: aws.String("tag:env"), + Values: []*string{aws.String("dev")}, + }}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, desc.KeyPairs, 1) + assert.Equal(t, "tagged-import-key", *desc.KeyPairs[0].KeyName) + + // Non-matching tag filter excludes it. + desc, err = svc.DescribeKeyPairs(&ec2.DescribeKeyPairsInput{ + Filters: []*ec2.Filter{{ + Name: aws.String("tag:env"), + Values: []*string{aws.String("prod")}, + }}, + }, testAccountID) + require.NoError(t, err) + assert.Empty(t, desc.KeyPairs) +} + +func TestImportKeyPair_TagSpecificationsWrongType(t *testing.T) { + svc, _ := newTestKeyService() + + // Tag specs for a different resource type are ignored. + out, err := svc.ImportKeyPair(&ec2.ImportKeyPairInput{ + KeyName: aws.String("untagged-import-key"), + PublicKeyMaterial: []byte(testED25519PubKey), + TagSpecifications: keyPairTagSpecs("volume"), + }, testAccountID) + require.NoError(t, err) + assert.Empty(t, out.Tags) +} diff --git a/spinifex/handlers/ec2/key/record_tags_test.go b/spinifex/handlers/ec2/key/record_tags_test.go new file mode 100644 index 000000000..49bb0ecd6 --- /dev/null +++ b/spinifex/handlers/ec2/key/record_tags_test.go @@ -0,0 +1,85 @@ +package handlers_ec2_key + +import ( + "encoding/json" + "fmt" + "io" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/mulgadc/spinifex/spinifex/filterutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func readKeyPairTags(t *testing.T, svc *KeyServiceImpl, accountID, keyPairID string) map[string]string { + t.Helper() + result, err := svc.store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(testBucket), + Key: aws.String(fmt.Sprintf("keys/%s/%s.json", accountID, keyPairID)), + }) + require.NoError(t, err) + defer result.Body.Close() + body, err := io.ReadAll(result.Body) + require.NoError(t, err) + var metadata ec2.CreateKeyPairOutput + require.NoError(t, json.Unmarshal(body, &metadata)) + return filterutil.EC2TagsToMap(metadata.Tags) +} + +func TestKeyPairRecordTagsMirror(t *testing.T) { + svc, _ := newTestKeyService() + out := importTestKey(t, svc, "tag-mirror-key") + keyPairID := *out.KeyPairId + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(keyPairID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + tags := readKeyPairTags(t, svc, testAccountID, keyPairID) + assert.Equal(t, "yes", tags["keep"]) + assert.Equal(t, "v", tags["drop"]) + + // Value-mismatched delete is a no-op; matched delete removes. + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(keyPairID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + tags = readKeyPairTags(t, svc, testAccountID, keyPairID) + assert.Equal(t, "yes", tags["keep"]) + _, ok := tags["drop"] + assert.False(t, ok) +} + +func TestKeyPairRecordTagsMirror_CrossAccountNoMutation(t *testing.T) { + svc, _ := newTestKeyService() + out := importTestKey(t, svc, "tag-guard-key") + keyPairID := *out.KeyPairId + + // Another account's mirror misses under its own prefix and no-ops. + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(keyPairID)}, + Tags: []*ec2.Tag{{Key: aws.String("evil"), Value: aws.String("1")}}, + }, "999999999999")) + + tags := readKeyPairTags(t, svc, testAccountID, keyPairID) + assert.Empty(t, tags) +} + +func TestKeyPairRecordTagsMirror_UnownedNoError(t *testing.T) { + svc, _ := newTestKeyService() + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("key-missing"), aws.String("pg-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/key/service_impl.go b/spinifex/handlers/ec2/key/service_impl.go index 76d0af2ab..34461741a 100644 --- a/spinifex/handlers/ec2/key/service_impl.go +++ b/spinifex/handlers/ec2/key/service_impl.go @@ -164,11 +164,13 @@ func (s *KeyServiceImpl) CreateKeyPair(input *ec2.CreateKeyPairInput, accountID // Build response (similar to AWS EC2) keyPairID := utils.GenerateResourceID("key") + tags := utils.MapToEC2Tags(utils.ExtractTags(input.TagSpecifications, "key-pair")) output := &ec2.CreateKeyPairOutput{ KeyFingerprint: aws.String(fingerprint), KeyMaterial: aws.String(string(privateKeyData)), KeyName: aws.String(keyName), KeyPairId: aws.String(keyPairID), + Tags: tags, } // Store metadata file (CreateKeyPairOutput without KeyMaterial) for keyPairId lookups @@ -176,6 +178,7 @@ func (s *KeyServiceImpl) CreateKeyPair(input *ec2.CreateKeyPairInput, accountID KeyFingerprint: aws.String(fingerprint), KeyName: aws.String(keyName), KeyPairId: aws.String(keyPairID), + Tags: tags, }) if err != nil { slog.Error("Failed to store key pair metadata", "err", err, "keyPairId", keyPairID) @@ -616,7 +619,7 @@ func (s *KeyServiceImpl) DescribeKeyPairs(input *ec2.DescribeKeyPairsInput, acco KeyFingerprint: metadata.KeyFingerprint, KeyName: metadata.KeyName, KeyType: aws.String(keyType), - Tags: []*ec2.Tag{}, + Tags: metadata.Tags, } // Use S3 object LastModified as CreateTime @@ -761,11 +764,12 @@ func (s *KeyServiceImpl) ImportKeyPair(input *ec2.ImportKeyPairInput, accountID keyPairID := utils.GenerateResourceID("key") // Build response output + tags := utils.MapToEC2Tags(utils.ExtractTags(input.TagSpecifications, "key-pair")) output := &ec2.ImportKeyPairOutput{ KeyFingerprint: aws.String(fingerprint), KeyName: aws.String(keyName), KeyPairId: aws.String(keyPairID), - Tags: []*ec2.Tag{}, // TODO: Implement tag support from input.TagSpecifications + Tags: tags, } // Store metadata file (without public key material) @@ -773,6 +777,7 @@ func (s *KeyServiceImpl) ImportKeyPair(input *ec2.ImportKeyPairInput, accountID KeyFingerprint: aws.String(fingerprint), KeyName: aws.String(keyName), KeyPairId: aws.String(keyPairID), + Tags: tags, } err = s.storeKeyPairMetadata(accountID, keyPairID, metadataOutput) @@ -792,3 +797,64 @@ func (s *KeyServiceImpl) ImportKeyPair(input *ec2.ImportKeyPairInput, accountID return output, nil } + +// ApplyRecordTags mirrors CreateTags into the owning key-pair metadata so +// DescribeKeyPairs observes tags added after create. Non-key ids and key pairs +// absent from the caller's account prefix are skipped. +func (s *KeyServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorKeyPairTags(input.Resources, accountID, utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning key-pair metadata with +// AWS-faithful delete semantics. +func (s *KeyServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorKeyPairTags(input.Resources, accountID, utils.RemoveTagsMut(input)) +} + +// mirrorKeyPairTags read-modify-writes the metadata Tags slice for each key- +// id, converting through a map to reuse the shared merge/remove semantics. +// Metadata is stored under the caller's account prefix, so a cross-account or +// absent key pair simply misses and no-ops. +func (s *KeyServiceImpl) mirrorKeyPairTags(resources []*string, accountID string, mut func(map[string]string)) error { + for _, res := range resources { + if res == nil || !strings.HasPrefix(*res, "key-") { + continue + } + metadataPath := fmt.Sprintf("keys/%s/%s.json", accountID, *res) + result, err := s.store.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(s.bucketName), + Key: aws.String(metadataPath), + }) + if err != nil { + if objectstore.IsNoSuchKeyError(err) { + continue + } + return fmt.Errorf("failed to get key pair metadata: %w", err) + } + body, err := io.ReadAll(result.Body) + result.Body.Close() + if err != nil { + return fmt.Errorf("failed to read key pair metadata: %w", err) + } + var metadata ec2.CreateKeyPairOutput + if err := json.Unmarshal(body, &metadata); err != nil { + return fmt.Errorf("failed to unmarshal key pair metadata: %w", err) + } + tags := filterutil.EC2TagsToMap(metadata.Tags) + if tags == nil { + tags = map[string]string{} + } + mut(tags) + metadata.Tags = utils.MapToEC2Tags(tags) + if err := s.storeKeyPairMetadata(accountID, *res, &metadata); err != nil { + return err + } + } + return nil +} diff --git a/spinifex/handlers/ec2/natgw/record_tags_test.go b/spinifex/handlers/ec2/natgw/record_tags_test.go new file mode 100644 index 000000000..84e4c0722 --- /dev/null +++ b/spinifex/handlers/ec2/natgw/record_tags_test.go @@ -0,0 +1,64 @@ +package handlers_ec2_natgw + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func natgwTagVal(tags []*ec2.Tag, key string) string { + for _, tg := range tags { + if tg.Key != nil && *tg.Key == key { + return aws.StringValue(tg.Value) + } + } + return "" +} + +func TestNatGatewayRecordTagsMirror(t *testing.T) { + svc := setupTestService(t) + natgwID := createTestNatGateway(t, svc) + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(natgwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{ + NatGatewayIds: []*string{aws.String(natgwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.NatGateways, 1) + assert.Equal(t, "yes", natgwTagVal(out.NatGateways[0].Tags, "keep")) + assert.Equal(t, "v", natgwTagVal(out.NatGateways[0].Tags, "drop")) + + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(natgwID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err = svc.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{ + NatGatewayIds: []*string{aws.String(natgwID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.NatGateways, 1) + assert.Equal(t, "yes", natgwTagVal(out.NatGateways[0].Tags, "keep")) + assert.Equal(t, "", natgwTagVal(out.NatGateways[0].Tags, "drop")) +} + +func TestNatGatewayRecordTagsMirror_UnownedNoError(t *testing.T) { + svc := setupTestService(t) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("nat-missing"), aws.String("igw-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/natgw/service_impl.go b/spinifex/handlers/ec2/natgw/service_impl.go index de6ecd5d5..c2dd2f680 100644 --- a/spinifex/handlers/ec2/natgw/service_impl.go +++ b/spinifex/handlers/ec2/natgw/service_impl.go @@ -529,3 +529,26 @@ func recordToEC2(record *NatGatewayRecord) *ec2.NatGateway { return ngw } + +// ApplyRecordTags mirrors CreateTags into the owning NAT gateway KV record so +// tag-filtered describes observe tags added after create. Resource ids this +// service does not own are skipped; absent records are a no-op. +func (s *NatGatewayServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.natgwKV, accountID, "nat-", input.Resources, + func(r *NatGatewayRecord) *map[string]string { return &r.Tags }, + utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning NAT gateway KV record +// with AWS-faithful delete semantics. +func (s *NatGatewayServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.natgwKV, accountID, "nat-", input.Resources, + func(r *NatGatewayRecord) *map[string]string { return &r.Tags }, + utils.RemoveTagsMut(input)) +} diff --git a/spinifex/handlers/ec2/placementgroup/create_tags_intake_test.go b/spinifex/handlers/ec2/placementgroup/create_tags_intake_test.go new file mode 100644 index 000000000..249d1b35f --- /dev/null +++ b/spinifex/handlers/ec2/placementgroup/create_tags_intake_test.go @@ -0,0 +1,96 @@ +package handlers_ec2_placementgroup + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/filterutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createTaggedGroup(t *testing.T, svc *PlacementGroupServiceImpl, name string, tags map[string]string) *ec2.PlacementGroup { + t.Helper() + var ec2Tags []*ec2.Tag + for k, v := range tags { + ec2Tags = append(ec2Tags, &ec2.Tag{Key: aws.String(k), Value: aws.String(v)}) + } + out, err := svc.CreatePlacementGroup(&ec2.CreatePlacementGroupInput{ + GroupName: aws.String(name), + Strategy: aws.String("spread"), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("placement-group"), + Tags: ec2Tags, + }}, + }, testAccountID) + require.NoError(t, err) + return out.PlacementGroup +} + +func describeWithFilter(t *testing.T, svc *PlacementGroupServiceImpl, name, value string) []*ec2.PlacementGroup { + t.Helper() + out, err := svc.DescribePlacementGroups(&ec2.DescribePlacementGroupsInput{ + Filters: []*ec2.Filter{{ + Name: aws.String(name), + Values: []*string{aws.String(value)}, + }}, + }, testAccountID) + require.NoError(t, err) + return out.PlacementGroups +} + +func TestCreatePlacementGroup_TagSpecifications(t *testing.T) { + svc := setupTestService(t) + pg := createTaggedGroup(t, svc, "tagged-group", map[string]string{"Name": "web", "env": "dev"}) + + tags := filterutil.EC2TagsToMap(pg.Tags) + assert.Equal(t, "web", tags["Name"]) + assert.Equal(t, "dev", tags["env"]) + + out, err := svc.DescribePlacementGroups(&ec2.DescribePlacementGroupsInput{ + GroupNames: []*string{aws.String("tagged-group")}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.PlacementGroups, 1) + descTags := filterutil.EC2TagsToMap(out.PlacementGroups[0].Tags) + assert.Equal(t, "web", descTags["Name"]) +} + +func TestCreatePlacementGroup_TagSpecificationsWrongType(t *testing.T) { + svc := setupTestService(t) + + out, err := svc.CreatePlacementGroup(&ec2.CreatePlacementGroupInput{ + GroupName: aws.String("untagged-group"), + Strategy: aws.String("cluster"), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("volume"), + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }}, + }, testAccountID) + require.NoError(t, err) + assert.Empty(t, out.PlacementGroup.Tags) +} + +func TestDescribePlacementGroups_TagFilters(t *testing.T) { + svc := setupTestService(t) + createTaggedGroup(t, svc, "pg-dev", map[string]string{"env": "dev"}) + createTaggedGroup(t, svc, "pg-prod", map[string]string{"env": "prod"}) + + groups := describeWithFilter(t, svc, "tag:env", "dev") + require.Len(t, groups, 1) + assert.Equal(t, "pg-dev", *groups[0].GroupName) + + groups = describeWithFilter(t, svc, "tag-key", "env") + assert.Len(t, groups, 2) + + groups = describeWithFilter(t, svc, "tag-value", "prod") + require.Len(t, groups, 1) + assert.Equal(t, "pg-prod", *groups[0].GroupName) + + groups = describeWithFilter(t, svc, "tag:env", "staging") + assert.Empty(t, groups) + + groups = describeWithFilter(t, svc, "tag-key", "missing") + assert.Empty(t, groups) +} diff --git a/spinifex/handlers/ec2/placementgroup/service_impl.go b/spinifex/handlers/ec2/placementgroup/service_impl.go index ad25badae..5b40aa6d9 100644 --- a/spinifex/handlers/ec2/placementgroup/service_impl.go +++ b/spinifex/handlers/ec2/placementgroup/service_impl.go @@ -38,6 +38,7 @@ type PlacementGroupRecord struct { // NodeInstances tracks which node hosts which instances in this group. // Key = node name, Value = list of instance IDs on that node. NodeInstances map[string][]string `json:"node_instances"` + Tags map[string]string `json:"tags,omitempty"` } // PlacementGroupServiceImpl implements placement group operations with NATS JetStream persistence. @@ -102,6 +103,7 @@ func (s *PlacementGroupServiceImpl) CreatePlacementGroup(input *ec2.CreatePlacem SpreadLevel: ec2.SpreadLevelHost, AccountID: accountID, NodeInstances: make(map[string][]string), + Tags: utils.ExtractTags(input.TagSpecifications, "placement-group"), } data, err := json.Marshal(record) @@ -164,6 +166,8 @@ var describePlacementGroupsValidFilters = map[string]bool{ "state": true, "spread-level": true, "group-name": true, + "tag-key": true, + "tag-value": true, } // DescribePlacementGroups lists placement groups with optional filters. @@ -255,6 +259,9 @@ func (s *PlacementGroupServiceImpl) DescribePlacementGroups(input *ec2.DescribeP // pgMatchesFilters checks whether a placement group record matches all parsed filters. func pgMatchesFilters(record *PlacementGroupRecord, filters map[string][]string) bool { for name, values := range filters { + if strings.HasPrefix(name, "tag:") { + continue + } switch name { case "group-id": if !filterutil.MatchesAny(values, record.GroupId) { @@ -276,11 +283,30 @@ func pgMatchesFilters(record *PlacementGroupRecord, filters map[string][]string) if !filterutil.MatchesAny(values, record.GroupName) { return false } + case "tag-key": + if !pgMatchesAnyTag(record.Tags, values, func(k, _ string) string { return k }) { + return false + } + case "tag-value": + if !pgMatchesAnyTag(record.Tags, values, func(_, v string) string { return v }) { + return false + } default: return false } } - return true + return filterutil.MatchesTags(filters, record.Tags) +} + +// pgMatchesAnyTag reports whether any tag's selected field (key or value) +// matches any of the filter values. +func pgMatchesAnyTag(tags map[string]string, values []string, field func(k, v string) string) bool { + for k, v := range tags { + if filterutil.MatchesAny(values, field(k, v)) { + return true + } + } + return false } // GetPlacementGroupRecord reads a placement group record from KV with its revision for CAS operations. @@ -578,6 +604,7 @@ func (s *PlacementGroupServiceImpl) recordToEC2(record *PlacementGroupRecord) *e GroupName: aws.String(record.GroupName), Strategy: aws.String(record.Strategy), State: aws.String(record.State), + Tags: utils.MapToEC2Tags(record.Tags), } if record.Strategy == ec2.PlacementStrategySpread { pg.SpreadLevel = aws.String(record.SpreadLevel) diff --git a/spinifex/handlers/ec2/routetable/record_tags_test.go b/spinifex/handlers/ec2/routetable/record_tags_test.go new file mode 100644 index 000000000..34c1b69a1 --- /dev/null +++ b/spinifex/handlers/ec2/routetable/record_tags_test.go @@ -0,0 +1,66 @@ +package handlers_ec2_routetable + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func rtbTagVal(tags []*ec2.Tag, key string) string { + for _, tg := range tags { + if tg.Key != nil && *tg.Key == key { + return aws.StringValue(tg.Value) + } + } + return "" +} + +func TestRouteTableRecordTagsMirror(t *testing.T) { + svc := setupTestService(t) + rtbID := createTestRtb(t, svc) + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(rtbID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ + RouteTableIds: []*string{aws.String(rtbID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.RouteTables, 1) + assert.Equal(t, "yes", rtbTagVal(out.RouteTables[0].Tags, "keep")) + assert.Equal(t, "v", rtbTagVal(out.RouteTables[0].Tags, "drop")) + + // Value-mismatched delete is a no-op; matched delete removes. + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(rtbID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err = svc.DescribeRouteTables(&ec2.DescribeRouteTablesInput{ + RouteTableIds: []*string{aws.String(rtbID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.RouteTables, 1) + assert.Equal(t, "yes", rtbTagVal(out.RouteTables[0].Tags, "keep")) + assert.Equal(t, "", rtbTagVal(out.RouteTables[0].Tags, "drop")) +} + +func TestRouteTableRecordTagsMirror_UnownedNoError(t *testing.T) { + svc := setupTestService(t) + // Absent rtb + non-rtb id: both skipped without error. + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("rtb-missing"), aws.String("vpc-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/routetable/service_impl.go b/spinifex/handlers/ec2/routetable/service_impl.go index 238bc6183..5771811d1 100644 --- a/spinifex/handlers/ec2/routetable/service_impl.go +++ b/spinifex/handlers/ec2/routetable/service_impl.go @@ -1566,3 +1566,26 @@ func recordToEC2(record *RouteTableRecord) *ec2.RouteTable { return rtb } + +// ApplyRecordTags mirrors CreateTags into the owning route-table KV record so +// tag-filtered describes observe tags added after create. Resource ids this +// service does not own are skipped; absent records are a no-op. +func (s *RouteTableServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.rtbKV, accountID, "rtb-", input.Resources, + func(r *RouteTableRecord) *map[string]string { return &r.Tags }, + utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning route-table KV record +// with AWS-faithful delete semantics. +func (s *RouteTableServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return utils.MirrorKVRecordTags(s.rtbKV, accountID, "rtb-", input.Resources, + func(r *RouteTableRecord) *map[string]string { return &r.Tags }, + utils.RemoveTagsMut(input)) +} diff --git a/spinifex/handlers/ec2/snapshot/record_tags_test.go b/spinifex/handlers/ec2/snapshot/record_tags_test.go new file mode 100644 index 000000000..4a587c06e --- /dev/null +++ b/spinifex/handlers/ec2/snapshot/record_tags_test.go @@ -0,0 +1,66 @@ +package handlers_ec2_snapshot + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSnapshotRecordTagsMirror(t *testing.T) { + svc, store := setupTestSnapshotService(t) + snapID := createTestSnapshot(t, svc, store, "vol-tagmirror", 10, nil) + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(snapID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + cfg, err := svc.getSnapshotConfig(snapID) + require.NoError(t, err) + assert.Equal(t, "yes", cfg.Tags["keep"]) + assert.Equal(t, "v", cfg.Tags["drop"]) + + // Value-mismatched delete is a no-op; matched delete removes. + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(snapID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + cfg, err = svc.getSnapshotConfig(snapID) + require.NoError(t, err) + assert.Equal(t, "yes", cfg.Tags["keep"]) + _, ok := cfg.Tags["drop"] + assert.False(t, ok) +} + +func TestSnapshotRecordTagsMirror_CrossTenantNoMutation(t *testing.T) { + svc, store := setupTestSnapshotService(t) + snapID := createTestSnapshot(t, svc, store, "vol-tenantguard", 10, map[string]string{"orig": "1"}) + + // A different account tagging this snapshot must not mutate the record. + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(snapID)}, + Tags: []*ec2.Tag{{Key: aws.String("evil"), Value: aws.String("1")}}, + }, "999999999999")) + + cfg, err := svc.getSnapshotConfig(snapID) + require.NoError(t, err) + assert.Equal(t, map[string]string{"orig": "1"}, cfg.Tags) +} + +func TestSnapshotRecordTagsMirror_UnownedNoError(t *testing.T) { + svc, _ := setupTestSnapshotService(t) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("snap-missing"), aws.String("vol-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testAccountID)) +} diff --git a/spinifex/handlers/ec2/snapshot/service_impl.go b/spinifex/handlers/ec2/snapshot/service_impl.go index c6c7a4a31..6aedad6b2 100644 --- a/spinifex/handlers/ec2/snapshot/service_impl.go +++ b/spinifex/handlers/ec2/snapshot/service_impl.go @@ -740,3 +740,52 @@ func (s *SnapshotServiceImpl) volumeHasSnapshots(volumeID string) (bool, error) return len(snapshots) > 0, nil } + +// ApplyRecordTags mirrors CreateTags into the owning snapshot metadata so +// DescribeSnapshots observes tags added after create. Non-snap ids, snapshots +// absent from this store, and snapshots the caller does not own are skipped. +func (s *SnapshotServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorSnapshotTags(input.Resources, accountID, utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning snapshot metadata with +// AWS-faithful delete semantics. +func (s *SnapshotServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorSnapshotTags(input.Resources, accountID, utils.RemoveTagsMut(input)) +} + +// mirrorSnapshotTags read-modify-writes SnapshotConfig.Tags for each snap- id. +// metadata.json lives at a global ID-keyed path, so the mutation is gated on +// the caller owning the snapshot (OwnerID match); mismatch or absence no-ops. +func (s *SnapshotServiceImpl) mirrorSnapshotTags(resources []*string, accountID string, mut func(map[string]string)) error { + for _, res := range resources { + if res == nil || !strings.HasPrefix(*res, "snap-") { + continue + } + cfg, err := ReadSnapshotConfig(s.store, s.config.Predastore.Bucket, *res) + if err != nil { + if objectstore.IsNoSuchKeyError(err) { + continue + } + return err + } + if cfg.OwnerID != accountID { + slog.Debug("mirrorSnapshotTags: skipping snapshot not owned by caller", "snapshotId", *res) + continue + } + if cfg.Tags == nil { + cfg.Tags = map[string]string{} + } + mut(cfg.Tags) + if err := s.putSnapshotConfig(*res, cfg); err != nil { + return err + } + } + return nil +} diff --git a/spinifex/handlers/ec2/tags/service_impl.go b/spinifex/handlers/ec2/tags/service_impl.go index b67f0bf12..867b45c5c 100644 --- a/spinifex/handlers/ec2/tags/service_impl.go +++ b/spinifex/handlers/ec2/tags/service_impl.go @@ -84,6 +84,21 @@ func getResourceType(resourceID string) string { if strings.HasPrefix(resourceID, "eigw-") { return "egress-only-internet-gateway" } + if strings.HasPrefix(resourceID, "eni-") { + return "network-interface" + } + if strings.HasPrefix(resourceID, "eipalloc-") { + return "elastic-ip" + } + if strings.HasPrefix(resourceID, "nat-") { + return "natgateway" + } + if strings.HasPrefix(resourceID, "key-") { + return "key-pair" + } + if strings.HasPrefix(resourceID, "pg-") { + return "placement-group" + } return "unknown" } diff --git a/spinifex/handlers/ec2/tags/service_impl_test.go b/spinifex/handlers/ec2/tags/service_impl_test.go index ee476282f..39061666a 100644 --- a/spinifex/handlers/ec2/tags/service_impl_test.go +++ b/spinifex/handlers/ec2/tags/service_impl_test.go @@ -439,6 +439,11 @@ func TestGetResourceType(t *testing.T) { {"rtb-abc123", "route-table"}, {"igw-abc123", "internet-gateway"}, {"eigw-abc123", "egress-only-internet-gateway"}, + {"eni-abc123", "network-interface"}, + {"eipalloc-abc123", "elastic-ip"}, + {"nat-abc123", "natgateway"}, + {"key-abc123", "key-pair"}, + {"pg-abc123", "placement-group"}, {"unknown-abc123", "unknown"}, } diff --git a/spinifex/handlers/ec2/volume/record_tags_test.go b/spinifex/handlers/ec2/volume/record_tags_test.go new file mode 100644 index 000000000..c340275ea --- /dev/null +++ b/spinifex/handlers/ec2/volume/record_tags_test.go @@ -0,0 +1,70 @@ +package handlers_ec2_volume + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/objectstore" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestVolumeRecordTagsMirror(t *testing.T) { + store := objectstore.NewMemoryObjectStore() + svc := newTestVolumeServiceWithStore("ap-southeast-2a", store) + seedVolume(t, svc, "vol-tagmirror0001", "available", "") + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("vol-tagmirror0001")}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testVolAccountID)) + + cfg, err := svc.GetVolumeConfig("vol-tagmirror0001") + require.NoError(t, err) + assert.Equal(t, "yes", cfg.VolumeMetadata.Tags["keep"]) + assert.Equal(t, "v", cfg.VolumeMetadata.Tags["drop"]) + + // Value-mismatched delete is a no-op; matched delete removes. + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String("vol-tagmirror0001")}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testVolAccountID)) + + cfg, err = svc.GetVolumeConfig("vol-tagmirror0001") + require.NoError(t, err) + assert.Equal(t, "yes", cfg.VolumeMetadata.Tags["keep"]) + _, ok := cfg.VolumeMetadata.Tags["drop"] + assert.False(t, ok) +} + +func TestVolumeRecordTagsMirror_CrossTenantNoMutation(t *testing.T) { + store := objectstore.NewMemoryObjectStore() + svc := newTestVolumeServiceWithStore("ap-southeast-2a", store) + seedVolume(t, svc, "vol-tenantguard01", "available", "") + + // A different account tagging this volume must not mutate the record. + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("vol-tenantguard01")}, + Tags: []*ec2.Tag{{Key: aws.String("evil"), Value: aws.String("1")}}, + }, "999999999999")) + + cfg, err := svc.GetVolumeConfig("vol-tenantguard01") + require.NoError(t, err) + assert.Empty(t, cfg.VolumeMetadata.Tags) +} + +func TestVolumeRecordTagsMirror_UnownedNoError(t *testing.T) { + store := objectstore.NewMemoryObjectStore() + svc := newTestVolumeServiceWithStore("ap-southeast-2a", store) + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("vol-missing000001"), aws.String("snap-other")}, + Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}, + }, testVolAccountID)) +} diff --git a/spinifex/handlers/ec2/volume/service_impl.go b/spinifex/handlers/ec2/volume/service_impl.go index 4a159f040..3a241bb6e 100644 --- a/spinifex/handlers/ec2/volume/service_impl.go +++ b/spinifex/handlers/ec2/volume/service_impl.go @@ -162,6 +162,8 @@ func (s *VolumeServiceImpl) CreateVolume(input *ec2.CreateVolumeInput, accountID slog.Info("CreateVolume", "volumeId", volumeID, "size", size, "type", volumeType, "az", *input.AvailabilityZone, "snapshotId", snapshotID) + tags := utils.ExtractTags(input.TagSpecifications, "volume") + // Volume size in bytes for viperblock sizeGiB := utils.SafeInt64ToUint64(size) volumeSizeBytes := sizeGiB * 1024 * 1024 * 1024 @@ -178,6 +180,7 @@ func (s *VolumeServiceImpl) CreateVolume(input *ec2.CreateVolumeInput, accountID VolumeType: volumeType, IOPS: iops, SnapshotID: snapshotID, + Tags: tags, }, } @@ -246,6 +249,7 @@ func (s *VolumeServiceImpl) CreateVolume(input *ec2.CreateVolumeInput, accountID CreateTime: aws.Time(now), Iops: aws.Int64(int64(iops)), Encrypted: aws.Bool(mkey != nil), + Tags: utils.MapToEC2Tags(tags), } if snapshotID != "" { @@ -1603,3 +1607,52 @@ func (s *VolumeServiceImpl) volumeHasSnapshotsKV(volumeID string) (bool, error) return len(snapshots) > 0, nil } + +// ApplyRecordTags mirrors CreateTags into the owning volume config so +// DescribeVolumes observes tags added after create. Non-vol ids, volumes +// absent from this store, and volumes the caller does not own are skipped. +func (s *VolumeServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorVolumeTags(input.Resources, accountID, utils.MergeTagsMut(input)) +} + +// RemoveRecordTags mirrors DeleteTags into the owning volume config with +// AWS-faithful delete semantics. +func (s *VolumeServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID string) error { + if input == nil { + return nil + } + return s.mirrorVolumeTags(input.Resources, accountID, utils.RemoveTagsMut(input)) +} + +// mirrorVolumeTags read-modify-writes VolumeMetadata.Tags for each vol- id. +// config.json lives at a global ID-keyed path, so the mutation is gated on the +// caller owning the volume (TenantID match); mismatch or absence is a no-op. +func (s *VolumeServiceImpl) mirrorVolumeTags(resources []*string, accountID string, mut func(map[string]string)) error { + for _, res := range resources { + if res == nil || !strings.HasPrefix(*res, "vol-") { + continue + } + cfg, err := s.GetVolumeConfig(*res) + if err != nil { + if err.Error() == awserrors.ErrorInvalidVolumeNotFound { + continue + } + return err + } + if cfg.VolumeMetadata.TenantID != accountID { + slog.Debug("mirrorVolumeTags: skipping volume not owned by caller", "volumeId", *res) + continue + } + if cfg.VolumeMetadata.Tags == nil { + cfg.VolumeMetadata.Tags = map[string]string{} + } + mut(cfg.VolumeMetadata.Tags) + if err := s.putVolumeConfig(*res, cfg); err != nil { + return err + } + } + return nil +} diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index cb8966ca0..8d132f9e9 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -798,13 +798,7 @@ func (s *VPCServiceImpl) ApplyRecordTags(input *ec2.CreateTagsInput, accountID s if input == nil { return nil } - merge := func(tags map[string]string) { - for _, t := range input.Tags { - if t.Key != nil && t.Value != nil { - tags[*t.Key] = *t.Value - } - } - } + merge := utils.MergeTagsMut(input) for _, res := range input.Resources { if res == nil { continue @@ -823,24 +817,7 @@ func (s *VPCServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID if input == nil { return nil } - remove := func(tags map[string]string) { - if len(input.Tags) == 0 { - for k := range tags { - delete(tags, k) - } - return - } - for _, t := range input.Tags { - if t.Key == nil { - continue - } - if t.Value == nil { - delete(tags, *t.Key) - } else if cur, ok := tags[*t.Key]; ok && cur == *t.Value { - delete(tags, *t.Key) - } - } - } + remove := utils.RemoveTagsMut(input) for _, res := range input.Resources { if res == nil { continue @@ -852,50 +829,38 @@ func (s *VPCServiceImpl) RemoveRecordTags(input *ec2.DeleteTagsInput, accountID return nil } -// updateRecordTags applies mut to the tag map of the subnet- or vpc-scoped -// record identified by resourceID. Other resource ids are a no-op. +// updateRecordTags applies mut to the tag map of the subnet-, vpc-, sg-, or +// eni-scoped record identified by resourceID. Other resource ids are a no-op. func (s *VPCServiceImpl) updateRecordTags(accountID, resourceID string, mut func(map[string]string)) error { switch { case strings.HasPrefix(resourceID, "subnet-"): - return updateKVRecordTags(s.subnetKV, accountID, resourceID, func(r *SubnetRecord) { + return utils.UpdateKVRecordTags(s.subnetKV, accountID, resourceID, func(r *SubnetRecord) { if r.Tags == nil { r.Tags = map[string]string{} } mut(r.Tags) }) case strings.HasPrefix(resourceID, "vpc-"): - return updateKVRecordTags(s.vpcKV, accountID, resourceID, func(r *VPCRecord) { + return utils.UpdateKVRecordTags(s.vpcKV, accountID, resourceID, func(r *VPCRecord) { + if r.Tags == nil { + r.Tags = map[string]string{} + } + mut(r.Tags) + }) + case strings.HasPrefix(resourceID, "sg-"): + return utils.UpdateKVRecordTags(s.sgKV, accountID, resourceID, func(r *SecurityGroupRecord) { + if r.Tags == nil { + r.Tags = map[string]string{} + } + mut(r.Tags) + }) + case strings.HasPrefix(resourceID, "eni-"): + return utils.UpdateKVRecordTags(s.eniKV, accountID, resourceID, func(r *ENIRecord) { if r.Tags == nil { r.Tags = map[string]string{} } mut(r.Tags) }) - } - return nil -} - -// updateKVRecordTags read-modify-writes a typed KV record, applying mut. A -// resource absent from this store is skipped (its tags live elsewhere). -func updateKVRecordTags[R any](kv nats.KeyValue, accountID, resourceID string, mut func(*R)) error { - key := utils.AccountKey(accountID, resourceID) - entry, err := kv.Get(key) - if err != nil { - if errors.Is(err, nats.ErrKeyNotFound) { - return nil - } - return errors.New(awserrors.ErrorServerInternal) - } - var rec R - if err := json.Unmarshal(entry.Value(), &rec); err != nil { - return errors.New(awserrors.ErrorServerInternal) - } - mut(&rec) - data, err := json.Marshal(&rec) - if err != nil { - return errors.New(awserrors.ErrorServerInternal) - } - if _, err := kv.Put(key, data); err != nil { - return errors.New(awserrors.ErrorServerInternal) } return nil } diff --git a/spinifex/handlers/ec2/vpc/service_impl_test.go b/spinifex/handlers/ec2/vpc/service_impl_test.go index 9ee0eb6df..811446351 100644 --- a/spinifex/handlers/ec2/vpc/service_impl_test.go +++ b/spinifex/handlers/ec2/vpc/service_impl_test.go @@ -1828,6 +1828,81 @@ func TestRemoveRecordTags_Subnet(t *testing.T) { assert.Equal(t, "", findTag(out.Subnets[0].Tags, "drop")) } +func TestApplyRecordTags_SecurityGroupTagFilteredDescribe(t *testing.T) { + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.3.0.0/16") + sgID := createTestSG(t, svc, vpcID, "mirror-sg") + + err := svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(sgID)}, + Tags: []*ec2.Tag{{Key: aws.String("env"), Value: aws.String("prod")}}, + }, testAccountID) + require.NoError(t, err) + + out, err := svc.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{ + {Name: aws.String("tag:env"), Values: []*string{aws.String("prod")}}, + }, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.SecurityGroups, 1) + assert.Equal(t, sgID, *out.SecurityGroups[0].GroupId) + assert.Equal(t, "prod", findTag(out.SecurityGroups[0].Tags, "env")) +} + +func TestRemoveRecordTags_SecurityGroup(t *testing.T) { + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.4.0.0/16") + sgID := createTestSG(t, svc, vpcID, "mirror-sg-del") + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(sgID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("yes")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + require.NoError(t, svc.RemoveRecordTags(&ec2.DeleteTagsInput{ + Resources: []*string{aws.String(sgID)}, + Tags: []*ec2.Tag{ + {Key: aws.String("keep"), Value: aws.String("wrong")}, + {Key: aws.String("drop"), Value: aws.String("v")}, + }, + }, testAccountID)) + + out, err := svc.DescribeSecurityGroups(&ec2.DescribeSecurityGroupsInput{ + GroupIds: []*string{aws.String(sgID)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.SecurityGroups, 1) + assert.Equal(t, "yes", findTag(out.SecurityGroups[0].Tags, "keep")) + assert.Equal(t, "", findTag(out.SecurityGroups[0].Tags, "drop")) +} + +func TestApplyRecordTags_ENITagFilteredDescribe(t *testing.T) { + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.5.0.0/16") + subnetID := createTestSubnet(t, svc, vpcID, "10.5.1.0/24") + eniID := createTestENI(t, svc, subnetID) + + err := svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(eniID)}, + Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("primary")}}, + }, testAccountID) + require.NoError(t, err) + + out, err := svc.DescribeNetworkInterfaces(&ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + {Name: aws.String("tag:Name"), Values: []*string{aws.String("primary")}}, + }, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.NetworkInterfaces, 1) + assert.Equal(t, eniID, *out.NetworkInterfaces[0].NetworkInterfaceId) + assert.Equal(t, "primary", findTag(out.NetworkInterfaces[0].TagSet, "Name")) +} + func TestApplyRecordTags_UnknownResourceNoError(t *testing.T) { svc := setupTestVPCService(t) // Absent subnet + non-VPC-owned resource id: both skipped without error. diff --git a/spinifex/utils/tagmirror.go b/spinifex/utils/tagmirror.go new file mode 100644 index 000000000..e3ee2815c --- /dev/null +++ b/spinifex/utils/tagmirror.go @@ -0,0 +1,101 @@ +package utils + +import ( + "encoding/json" + "errors" + "log/slog" + "strings" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/nats-io/nats.go" +) + +// MergeTagsMut returns a mutator that merges the CreateTagsInput tags into a +// record's tag map. Tags with a nil key or value are skipped. +func MergeTagsMut(input *ec2.CreateTagsInput) func(map[string]string) { + return func(tags map[string]string) { + for _, t := range input.Tags { + if t.Key != nil && t.Value != nil { + tags[*t.Key] = *t.Value + } + } + } +} + +// RemoveTagsMut returns a mutator with AWS-faithful DeleteTags semantics: +// empty input.Tags clears all tags; a tag with a value deletes only on a +// value match, a nil value deletes unconditionally. +func RemoveTagsMut(input *ec2.DeleteTagsInput) func(map[string]string) { + return func(tags map[string]string) { + if len(input.Tags) == 0 { + for k := range tags { + delete(tags, k) + } + return + } + for _, t := range input.Tags { + if t.Key == nil { + continue + } + if t.Value == nil { + delete(tags, *t.Key) + } else if cur, ok := tags[*t.Key]; ok && cur == *t.Value { + delete(tags, *t.Key) + } + } + } +} + +// UpdateKVRecordTags read-modify-writes a typed account-scoped KV record, +// applying mut. A resource absent from this store is skipped (its tags live +// elsewhere). +func UpdateKVRecordTags[R any](kv nats.KeyValue, accountID, resourceID string, mut func(*R)) error { + key := AccountKey(accountID, resourceID) + entry, err := kv.Get(key) + if err != nil { + if errors.Is(err, nats.ErrKeyNotFound) { + return nil + } + slog.Error("UpdateKVRecordTags: KV read failed", "key", key, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + var rec R + if err := json.Unmarshal(entry.Value(), &rec); err != nil { + slog.Error("UpdateKVRecordTags: record unmarshal failed", "key", key, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + mut(&rec) + data, err := json.Marshal(&rec) + if err != nil { + slog.Error("UpdateKVRecordTags: record marshal failed", "key", key, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + if _, err := kv.Put(key, data); err != nil { + slog.Error("UpdateKVRecordTags: KV write failed", "key", key, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + return nil +} + +// MirrorKVRecordTags applies mut to the tag map of every resource in +// resources carrying prefix, via UpdateKVRecordTags on the service's own KV +// handle. tagsOf returns the address of the record's tag map so a nil map can +// be initialized in place. Resource ids without prefix are skipped. +func MirrorKVRecordTags[R any](kv nats.KeyValue, accountID, prefix string, resources []*string, tagsOf func(*R) *map[string]string, mut func(map[string]string)) error { + for _, res := range resources { + if res == nil || !strings.HasPrefix(*res, prefix) { + continue + } + if err := UpdateKVRecordTags(kv, accountID, *res, func(r *R) { + tp := tagsOf(r) + if *tp == nil { + *tp = map[string]string{} + } + mut(*tp) + }); err != nil { + return err + } + } + return nil +} diff --git a/spinifex/utils/tagmirror_test.go b/spinifex/utils/tagmirror_test.go new file mode 100644 index 000000000..672127568 --- /dev/null +++ b/spinifex/utils/tagmirror_test.go @@ -0,0 +1,152 @@ +package utils + +import ( + "encoding/json" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/mulgadc/spinifex/spinifex/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type tagMirrorRecord struct { + Name string `json:"name"` + Tags map[string]string `json:"tags"` +} + +func TestMergeTagsMut(t *testing.T) { + mut := MergeTagsMut(&ec2.CreateTagsInput{ + Tags: []*ec2.Tag{ + {Key: aws.String("a"), Value: aws.String("1")}, + {Key: aws.String("b"), Value: aws.String("2")}, + {Key: nil, Value: aws.String("skip")}, + {Key: aws.String("skip"), Value: nil}, + }, + }) + tags := map[string]string{"a": "old", "keep": "x"} + mut(tags) + assert.Equal(t, map[string]string{"a": "1", "b": "2", "keep": "x"}, tags) +} + +func TestRemoveTagsMut(t *testing.T) { + base := func() map[string]string { + return map[string]string{"a": "1", "b": "2", "c": "3"} + } + + t.Run("empty input clears all", func(t *testing.T) { + tags := base() + RemoveTagsMut(&ec2.DeleteTagsInput{})(tags) + assert.Empty(t, tags) + }) + + t.Run("value match deletes, mismatch keeps", func(t *testing.T) { + tags := base() + RemoveTagsMut(&ec2.DeleteTagsInput{Tags: []*ec2.Tag{ + {Key: aws.String("a"), Value: aws.String("1")}, + {Key: aws.String("b"), Value: aws.String("wrong")}, + }})(tags) + assert.Equal(t, map[string]string{"b": "2", "c": "3"}, tags) + }) + + t.Run("nil value deletes unconditionally, nil key skipped", func(t *testing.T) { + tags := base() + RemoveTagsMut(&ec2.DeleteTagsInput{Tags: []*ec2.Tag{ + {Key: aws.String("c"), Value: nil}, + {Key: nil, Value: aws.String("1")}, + }})(tags) + assert.Equal(t, map[string]string{"a": "1", "b": "2"}, tags) + }) +} + +func TestUpdateKVRecordTags(t *testing.T) { + _, _, js := testutil.StartTestJetStream(t) + rec, err := json.Marshal(&tagMirrorRecord{Name: "r1", Tags: map[string]string{"a": "1"}}) + require.NoError(t, err) + kv := testutil.SeedKV(t, js, "tagmirror-test", map[string][]byte{ + AccountKey("acct", "res-1"): rec, + AccountKey("acct", "res-bad"): []byte("not-json"), + }) + + t.Run("mutates and persists", func(t *testing.T) { + require.NoError(t, UpdateKVRecordTags(kv, "acct", "res-1", func(r *tagMirrorRecord) { + r.Tags["b"] = "2" + })) + entry, err := kv.Get(AccountKey("acct", "res-1")) + require.NoError(t, err) + var got tagMirrorRecord + require.NoError(t, json.Unmarshal(entry.Value(), &got)) + assert.Equal(t, "r1", got.Name) + assert.Equal(t, map[string]string{"a": "1", "b": "2"}, got.Tags) + }) + + t.Run("absent key is a no-op", func(t *testing.T) { + require.NoError(t, UpdateKVRecordTags(kv, "acct", "res-missing", func(r *tagMirrorRecord) { + t.Fatal("mutator must not run for absent record") + })) + }) + + t.Run("corrupt record returns internal error", func(t *testing.T) { + err := UpdateKVRecordTags(kv, "acct", "res-bad", func(r *tagMirrorRecord) {}) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) + }) +} + +func TestMirrorKVRecordTags(t *testing.T) { + _, _, js := testutil.StartTestJetStream(t) + withTags, err := json.Marshal(&tagMirrorRecord{Tags: map[string]string{"drop": "v", "keep": "yes"}}) + require.NoError(t, err) + nilTags, err := json.Marshal(&tagMirrorRecord{}) + require.NoError(t, err) + kv := testutil.SeedKV(t, js, "tagmirror-mirror-test", map[string][]byte{ + AccountKey("acct", "vol-1"): withTags, + AccountKey("acct", "vol-2"): nilTags, + }) + + tagsOf := func(r *tagMirrorRecord) *map[string]string { return &r.Tags } + + readTags := func(t *testing.T, id string) map[string]string { + entry, err := kv.Get(AccountKey("acct", id)) + require.NoError(t, err) + var got tagMirrorRecord + require.NoError(t, json.Unmarshal(entry.Value(), &got)) + return got.Tags + } + + resources := []*string{ + aws.String("vol-1"), + aws.String("vol-2"), + aws.String("snap-other"), + aws.String("vol-absent"), + nil, + } + + require.NoError(t, MirrorKVRecordTags(kv, "acct", "vol-", resources, tagsOf, + MergeTagsMut(&ec2.CreateTagsInput{Tags: []*ec2.Tag{ + {Key: aws.String("added"), Value: aws.String("1")}, + }}))) + assert.Equal(t, map[string]string{"drop": "v", "keep": "yes", "added": "1"}, readTags(t, "vol-1")) + assert.Equal(t, map[string]string{"added": "1"}, readTags(t, "vol-2"), "nil tag map initialized in place") + + require.NoError(t, MirrorKVRecordTags(kv, "acct", "vol-", resources, tagsOf, + RemoveTagsMut(&ec2.DeleteTagsInput{Tags: []*ec2.Tag{ + {Key: aws.String("drop"), Value: aws.String("v")}, + {Key: aws.String("keep"), Value: aws.String("wrong")}, + }}))) + assert.Equal(t, map[string]string{"keep": "yes", "added": "1"}, readTags(t, "vol-1")) + + // snap-other carries the wrong prefix, so a record stored under that id + // must never be touched even though it exists in this bucket. + otherRec, err := json.Marshal(&tagMirrorRecord{Tags: map[string]string{"x": "y"}}) + require.NoError(t, err) + _, err = kv.Put(AccountKey("acct", "snap-other"), otherRec) + require.NoError(t, err) + require.NoError(t, MirrorKVRecordTags(kv, "acct", "vol-", resources, tagsOf, + MergeTagsMut(&ec2.CreateTagsInput{Tags: []*ec2.Tag{ + {Key: aws.String("added"), Value: aws.String("2")}, + }}))) + assert.Equal(t, map[string]string{"x": "y"}, readTags(t, "snap-other")) +}