From 2f609efa14859b034090521a2ab2f9d80f08ddf8 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:03:18 +1100 Subject: [PATCH] fix: hide platform-managed instances from DescribeInstanceStatus (#1230) --- .../awsapi/instance/status_synthesis_test.go | 40 +++++++++++++++++++ .../domains/ec2/instance/service_impl_test.go | 24 +++++++++++ .../domains/ec2/instance/status_selection.go | 2 +- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/spinifex/domains/ec2/awsapi/instance/status_synthesis_test.go b/spinifex/domains/ec2/awsapi/instance/status_synthesis_test.go index 13d66450d..64283ffdf 100644 --- a/spinifex/domains/ec2/awsapi/instance/status_synthesis_test.go +++ b/spinifex/domains/ec2/awsapi/instance/status_synthesis_test.go @@ -12,6 +12,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + awsidentifiers "github.com/mulgadc/spinifex/spinifex/foundation/aws/identifiers" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/tags" "github.com/mulgadc/spinifex/spinifex/runtime/compute/cache" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/nats-io/nats.go" @@ -237,6 +239,44 @@ func TestSynthesis_HonoursInstanceIDFilter(t *testing.T) { require.Equal(t, "i-asked", aws.StringValue(out.InstanceStatuses[0].InstanceId)) } +// Synthesis shares the node path's selection, so a platform-managed VM on a +// stale node is hidden from the customer account it is recorded under. +func TestSynthesis_HidesManagedSystemVMFromCustomer(t *testing.T) { + t.Parallel() + _, nc := startTestNATSServer(t) + answerWith(t, nc, "node-active") + + managed := cachedVM("i-ekscp", "node-dead", "az-a", vm.StateRunning) + managed.ManagedBy = tags.ManagedByEKS + synth := StatusSynthesis{ + Records: fakeRecords{ready: true, vms: []*vm.VM{managed}}, + Liveness: fakeLiveness{states: map[string]instancecache.NodeState{"node-dead": instancecache.NodeStale}}, + } + + out, err := DescribeInstanceStatus(context.Background(), &ec2.DescribeInstanceStatusInput{}, nc, 1, synthAccount, "az-a", synth) + require.NoError(t, err) + require.Empty(t, out.InstanceStatuses) +} + +func TestSynthesis_RootSeesManagedSystemVM(t *testing.T) { + t.Parallel() + _, nc := startTestNATSServer(t) + answerWith(t, nc, "node-active") + + managed := cachedVM("i-lb", "node-dead", "az-a", vm.StateRunning) + managed.AccountID = awsidentifiers.GlobalAccountID + managed.ManagedBy = tags.ManagedByELBv2 + synth := StatusSynthesis{ + Records: fakeRecords{ready: true, vms: []*vm.VM{managed}}, + Liveness: fakeLiveness{states: map[string]instancecache.NodeState{"node-dead": instancecache.NodeStale}}, + } + + out, err := DescribeInstanceStatus(context.Background(), &ec2.DescribeInstanceStatusInput{}, nc, 1, awsidentifiers.GlobalAccountID, "az-a", synth) + require.NoError(t, err) + require.Len(t, out.InstanceStatuses, 1) + require.Equal(t, "i-lb", aws.StringValue(out.InstanceStatuses[0].InstanceId)) +} + // A cache that has not finished its first whole-set sync cannot support a // claim about what is missing, so it contributes nothing. func TestSynthesis_ColdCacheContributesNothing(t *testing.T) { diff --git a/spinifex/domains/ec2/instance/service_impl_test.go b/spinifex/domains/ec2/instance/service_impl_test.go index 64a72cbe3..c176eecb7 100644 --- a/spinifex/domains/ec2/instance/service_impl_test.go +++ b/spinifex/domains/ec2/instance/service_impl_test.go @@ -4507,6 +4507,30 @@ func TestDescribeInstanceStatus_AccountFilteringHidesOtherTenant(t *testing.T) { assert.Empty(t, out.InstanceStatuses) } +// A platform-managed VM recorded under a customer account stays out of that +// customer's status listing, matching DescribeInstances. +func TestDescribeInstanceStatus_HidesManagedSystemVMFromCustomer(t *testing.T) { + owner := "111122223333" + v := runningVM("i-ekscp", owner) + v.ManagedBy = tags.ManagedByEKS + svc := instanceStatusService(t, "az-a", map[string]*vm.VM{v.ID: v}) + + out, err := svc.DescribeInstanceStatus(context.Background(), &ec2.DescribeInstanceStatusInput{}, owner) + require.NoError(t, err) + assert.Empty(t, out.InstanceStatuses, "managed system VM must not appear in customer status listing") +} + +func TestDescribeInstanceStatus_RootSeesManagedSystemVM(t *testing.T) { + v := runningVM("i-lb", awsidentifiers.GlobalAccountID) + v.ManagedBy = tags.ManagedByELBv2 + svc := instanceStatusService(t, "az-a", map[string]*vm.VM{v.ID: v}) + + out, err := svc.DescribeInstanceStatus(context.Background(), &ec2.DescribeInstanceStatusInput{}, awsidentifiers.GlobalAccountID) + require.NoError(t, err) + require.Len(t, out.InstanceStatuses, 1) + assert.Equal(t, "i-lb", *out.InstanceStatuses[0].InstanceId) +} + func TestDescribeInstanceStatus_StoppedExcludedByDefault(t *testing.T) { owner := "111122223333" stopped := runningVM("i-stop", owner) diff --git a/spinifex/domains/ec2/instance/status_selection.go b/spinifex/domains/ec2/instance/status_selection.go index e8d5e1fa4..5abd89761 100644 --- a/spinifex/domains/ec2/instance/status_selection.go +++ b/spinifex/domains/ec2/instance/status_selection.go @@ -59,7 +59,7 @@ func ParseStatusSelection(input *ec2.DescribeInstanceStatusInput, accountID stri // status is built. Split out so a caller can test membership without paying // for the projection. func (s StatusSelection) Includes(v *vm.VM) bool { - if v == nil || !IsInstanceVisible(s.accountID, v.AccountID) { + if v == nil || !IsInstanceVisibleToCaller(s.accountID, v) { return false } if len(s.instanceIDs) > 0 && !s.instanceIDs[v.ID] {