fix: return Placement and Spot lineage for stopped and terminated instances (#558)

spinifex/daemon/daemon_handlers_instance.go

@@ -13,7 +13,6 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mulgadc/spinifex/spinifex/awserrors"
 	"github.com/mulgadc/spinifex/spinifex/filterutil"
-	handlers_dns "github.com/mulgadc/spinifex/spinifex/handlers/dns"
 	handlers_ec2_instance "github.com/mulgadc/spinifex/spinifex/handlers/ec2/instance"
 	"github.com/mulgadc/spinifex/spinifex/types"
 	"github.com/mulgadc/spinifex/spinifex/utils"
@@ -313,18 +312,12 @@ func (d *Daemon) handleEC2DescribeInstances(msg *nats.Msg) {
 
 	slog.Info("Processing DescribeInstances request from this node", "accountID", accountID)
 
-	// Validate and filter instances if specific instance IDs were requested
-	instanceIDFilter := make(map[string]bool)
-	if len(describeInstancesInput.InstanceIds) > 0 {
-		for _, id := range describeInstancesInput.InstanceIds {
-			if id != nil && *id != "" {
-				if !strings.HasPrefix(*id, "i-") {
-					respondWithError(msg, awserrors.ErrorInvalidInstanceIDMalformed)
-					return
-				}
-				instanceIDFilter[*id] = true
-			}
-		}
+	// Validate the requested instance IDs, rejecting malformed ones so this path
+	// and the stopped/terminated path below enforce the same rule.
+	instanceIDFilter, err := handlers_ec2_instance.ParseInstanceIDFilter(describeInstancesInput.InstanceIds)
+	if err != nil {
+		respondWithError(msg, awserrors.ErrorInvalidInstanceIDMalformed)
+		return
 	}
 
 	// Parse filters (returns error for unknown filter names)
@@ -373,92 +366,32 @@ func (d *Daemon) handleEC2DescribeInstances(msg *nats.Msg) {
 					reservationMap[resID] = reservation
 				}
 
-				// Update the instance state to current state
-				instanceCopy := *instance.Instance
-				instanceCopy.State = &ec2.InstanceState{}
-
-				// Populate PublicIpAddress from VM if stored
-				if instance.PublicIP != "" && instanceCopy.PublicIpAddress == nil {
-					instanceCopy.PublicIpAddress = aws.String(instance.PublicIP)
-				}
-
-				// Project the AWS-shaped DNS names from the current IPs, the same
-				// way PublicIpAddress/State are derived here. Mirrors the records
-				// the control-plane writer publishes to northstar.
-				publicDNS, privateDNS := handlers_dns.EC2DNSNames(
-					d.config.Region, d.dnsBaseDomain, d.dnsInternalDomain,
-					aws.StringValue(instanceCopy.PublicIpAddress), aws.StringValue(instanceCopy.PrivateIpAddress),
-				)
-				if publicDNS != "" {
-					instanceCopy.PublicDnsName = aws.String(publicDNS)
-				}
-				if privateDNS != "" {
-					instanceCopy.PrivateDnsName = aws.String(privateDNS)
-				}
-
-				// Map internal status to AWS state, projecting Spinifex-only states
-				// (e.g. error -> stopped) so SDK/UI clients see a valid label.
-				if info, ok := vm.EC2APIState(instance.Status); ok {
-					instanceCopy.State.SetCode(info.Code)
-					instanceCopy.State.SetName(info.Name)
-				} else {
+				// Project every vm.VM-sourced field onto an API-shaped copy via the
+				// shared projection, so this path and the stopped/terminated path
+				// below define the field set exactly once and cannot drift. Running
+				// instances carry their runtime network (public IP, DNS) and
+				// capacity reservation; an unmapped status falls back to pending/0.
+				projected, stateMapped := handlers_ec2_instance.ProjectInstance(instance, handlers_ec2_instance.InstanceProjection{
+					Region:                d.config.Region,
+					DNSBaseDomain:         d.dnsBaseDomain,
+					DNSInternalDomain:     d.dnsInternalDomain,
+					AZ:                    d.config.AZ,
+					IncludeRuntimeNetwork: true,
+					FallbackStateCode:     0,
+					FallbackStateName:     "pending",
+				})
+				if !stateMapped {
 					slog.Warn("Instance has unmapped status, reporting as pending",
 						"instanceId", instance.ID, "status", string(instance.Status))
-					instanceCopy.State.SetCode(0)
-					instanceCopy.State.SetName("pending")
-				}
-
-				// Project IamInstanceProfile from vm.VM (single source of truth
-				// across Associate/Disassociate/Replace lifecycle). Id is left
-				// nil — the gateway resolves it via IAMService post-aggregation
-				// since daemons have no IAM access. Empty Arn clears any stale
-				// reference left on stored instance.Instance (e.g. after
-				// Disassociate or auto-clear on terminate).
-				if instance.IamInstanceProfileArn != "" {
-					instanceCopy.IamInstanceProfile = &ec2.IamInstanceProfile{
-						Arn: aws.String(instance.IamInstanceProfileArn),
-					}
-				} else {
-					instanceCopy.IamInstanceProfile = nil
-				}
-
-				// Populate Placement if instance belongs to a placement group
-				if instance.PlacementGroupName != "" {
-					instanceCopy.Placement = &ec2.Placement{
-						GroupName:        aws.String(instance.PlacementGroupName),
-						AvailabilityZone: aws.String(d.config.AZ),
-					}
-				}
-
-				// Echo the consumed capacity reservation so targeted-launch
-				// Terraform converges — without it the instance reports no
-				// reservation and the plan never settles.
-				if instance.CapacityReservationId != "" {
-					instanceCopy.CapacityReservationId = aws.String(instance.CapacityReservationId)
-					instanceCopy.CapacityReservationSpecification = &ec2.CapacityReservationSpecificationResponse{
-						CapacityReservationPreference: aws.String(ec2.CapacityReservationPreferenceOpen),
-						CapacityReservationTarget: &ec2.CapacityReservationTargetResponse{
-							CapacityReservationId: aws.String(instance.CapacityReservationId),
-						},
-					}
-				}
-
-				// Project spot lineage stamped by the post-launch write-back.
-				// Both empty for on-demand, so the fields stay absent there.
-				if instance.InstanceLifecycle != "" {
-					instanceCopy.InstanceLifecycle = aws.String(instance.InstanceLifecycle)
-				}
-				if instance.SpotInstanceRequestId != "" {
-					instanceCopy.SpotInstanceRequestId = aws.String(instance.SpotInstanceRequestId)
 				}
 
 				// Apply filters against the fully-built instance copy
-				if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, &instanceCopy, parsedFilters) {
+				if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, projected, parsedFilters) {
 					continue
 				}
 
 				// Add instance to its reservation
-				reservationMap[resID].Instances = append(reservationMap[resID].Instances, &instanceCopy)
+				reservationMap[resID].Instances = append(reservationMap[resID].Instances, projected)
 			}
 		}
 	})
@@ -576,11 +509,12 @@ func (d *Daemon) describeInstancesFromKV(msg *nats.Msg, listFn func() ([]*vm.VM,
 		}
 	}
 
-	instanceIDFilter := make(map[string]bool)
-	for _, id := range describeInput.InstanceIds {
-		if id != nil {
-			instanceIDFilter[*id] = true
-		}
+	// Validate instance IDs identically to the running path — the gateway fans
+	// out to both, so a malformed ID must fail the same way whichever responds.
+	instanceIDFilter, err := handlers_ec2_instance.ParseInstanceIDFilter(describeInput.InstanceIds)
+	if err != nil {
+		respondWithError(msg, awserrors.ErrorInvalidInstanceIDMalformed)
+		return
 	}
 
 	parsedFilters, filterErr := filterutil.ParseFilters(describeInput.Filters, describeInstancesValidFilters)
@@ -627,31 +561,23 @@ func (d *Daemon) describeInstancesFromKV(msg *nats.Msg, listFn func() ([]*vm.VM,
 			reservationMap[resID] = reservation
 		}
 
-		instanceCopy := *instance.Instance
-		instanceCopy.State = &ec2.InstanceState{}
-		if info, ok := vm.EC2APIState(instance.Status); ok {
-			instanceCopy.State.SetCode(info.Code)
-			instanceCopy.State.SetName(info.Name)
-		} else {
-			instanceCopy.State.SetCode(fallbackCode)
-			instanceCopy.State.SetName(fallbackName)
-		}
+		// Reuse the shared projection so stopped/terminated instances carry the
+		// same vm.VM-sourced fields as the running path — this is the fix for the
+		// dropped Placement/Spot lineage. Runtime network (public IP, DNS) and the
+		// capacity reservation are released on stop, so IncludeRuntimeNetwork stays
+		// false and they remain absent. The caller's fallback labels the state when
+		// a stored status has no EC2 mapping.
+		projected, _ := handlers_ec2_instance.ProjectInstance(instance, handlers_ec2_instance.InstanceProjection{
+			AZ:                d.config.AZ,
+			FallbackStateCode: fallbackCode,
+			FallbackStateName: fallbackName,
+		})
 
-		// Project IamInstanceProfile from vm.VM (cleared on terminate;
-		// preserved across stop/start). Mirrors handleEC2DescribeInstances.
-		if instance.IamInstanceProfileArn != "" {
-			instanceCopy.IamInstanceProfile = &ec2.IamInstanceProfile{
-				Arn: aws.String(instance.IamInstanceProfileArn),
-			}
-		} else {
-			instanceCopy.IamInstanceProfile = nil
-		}
-
-		if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, &instanceCopy, parsedFilters) {
+		if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, projected, parsedFilters) {
 			continue
 		}
 
-		reservationMap[resID].Instances = append(reservationMap[resID].Instances, &instanceCopy)
+		reservationMap[resID].Instances = append(reservationMap[resID].Instances, projected)
 	}
 
 	reservations := make([]*ec2.Reservation, 0, len(reservationMap))

spinifex/handlers/ec2/instance/projection.go

@@ -0,0 +1,146 @@
+package handlers_ec2_instance
+
+import (
+	"errors"
+	"strings"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	handlers_dns "github.com/mulgadc/spinifex/spinifex/handlers/dns"
+	"github.com/mulgadc/spinifex/spinifex/vm"
+)
+
+// InstanceProjection carries the caller-scoped values ProjectInstance needs.
+// All fields are plain data so the projection stays pure and table-testable.
+type InstanceProjection struct {
+	Region            string
+	DNSBaseDomain     string
+	DNSInternalDomain string
+	AZ                string
+
+	// IncludeRuntimeNetwork projects the fields that only hold while an instance
+	// runs: the public IP, its derived DNS names, and the consumed capacity
+	// reservation. AWS releases all three when an instance stops, so the
+	// stopped/terminated path leaves IncludeRuntimeNetwork false and they stay
+	// unset. Placement and Spot lineage survive a stop and are projected either
+	// way.
+	IncludeRuntimeNetwork bool
+
+	// FallbackStateCode/Name label the state when vm.EC2APIState has no mapping
+	// for the instance's internal status.
+	FallbackStateCode int64
+	FallbackStateName string
+}
+
+// ProjectInstance returns the API-shaped copy of v.Instance with every
+// vm.VM-sourced field projected onto it. It is pure: it reads only v and cfg and
+// mutates a fresh copy, so both live describe paths share one field set and
+// cannot drift apart. v.Instance must be non-nil.
+//
+// stateMapped is false when v.Status has no EC2 equivalent; the caller's
+// fallback code/name is applied either way, and the flag lets the running path
+// log the gap without the stopped path having to.
+func ProjectInstance(v *vm.VM, cfg InstanceProjection) (inst *ec2.Instance, stateMapped bool) {
+	instanceCopy := *v.Instance
+	instanceCopy.State = &ec2.InstanceState{}
+
+	// Public IP and its derived DNS names exist only while the instance runs; a
+	// stopped instance has released the public IP, so the KV path leaves them
+	// unset. Mirrors the records the control-plane writer publishes to northstar.
+	if cfg.IncludeRuntimeNetwork {
+		if v.PublicIP != "" && instanceCopy.PublicIpAddress == nil {
+			instanceCopy.PublicIpAddress = aws.String(v.PublicIP)
+		}
+
+		publicDNS, privateDNS := handlers_dns.EC2DNSNames(
+			cfg.Region, cfg.DNSBaseDomain, cfg.DNSInternalDomain,
+			aws.StringValue(instanceCopy.PublicIpAddress), aws.StringValue(instanceCopy.PrivateIpAddress),
+		)
+		if publicDNS != "" {
+			instanceCopy.PublicDnsName = aws.String(publicDNS)
+		}
+		if privateDNS != "" {
+			instanceCopy.PrivateDnsName = aws.String(privateDNS)
+		}
+	}
+
+	// Map internal status to AWS state, projecting Spinifex-only states
+	// (e.g. error -> stopped) so SDK/UI clients see a valid label. An unmapped
+	// status falls back to the caller's code/name and reports stateMapped false.
+	if info, ok := vm.EC2APIState(v.Status); ok {
+		instanceCopy.State.SetCode(info.Code)
+		instanceCopy.State.SetName(info.Name)
+		stateMapped = true
+	} else {
+		instanceCopy.State.SetCode(cfg.FallbackStateCode)
+		instanceCopy.State.SetName(cfg.FallbackStateName)
+	}
+
+	// Project IamInstanceProfile from vm.VM (single source of truth across the
+	// Associate/Disassociate/Replace lifecycle). Id is left nil — the gateway
+	// resolves it via IAMService post-aggregation since daemons have no IAM
+	// access. Empty Arn clears any stale reference left on the stored
+	// instance.Instance (e.g. after Disassociate or auto-clear on terminate).
+	if v.IamInstanceProfileArn != "" {
+		instanceCopy.IamInstanceProfile = &ec2.IamInstanceProfile{
+			Arn: aws.String(v.IamInstanceProfileArn),
+		}
+	} else {
+		instanceCopy.IamInstanceProfile = nil
+	}
+
+	// Placement survives a stop in AWS, so project it regardless of runtime state
+	// whenever the instance belongs to a placement group.
+	if v.PlacementGroupName != "" {
+		instanceCopy.Placement = &ec2.Placement{
+			GroupName:        aws.String(v.PlacementGroupName),
+			AvailabilityZone: aws.String(cfg.AZ),
+		}
+	}
+
+	// Echo the consumed capacity reservation so targeted-launch Terraform
+	// converges — without it the instance reports no reservation and the plan
+	// never settles. Gated to running instances: AWS releases the reservation
+	// when an instance stops, so a stopped instance must not echo it.
+	if cfg.IncludeRuntimeNetwork && v.CapacityReservationId != "" {
+		instanceCopy.CapacityReservationId = aws.String(v.CapacityReservationId)
+		instanceCopy.CapacityReservationSpecification = &ec2.CapacityReservationSpecificationResponse{
+			CapacityReservationPreference: aws.String(ec2.CapacityReservationPreferenceOpen),
+			CapacityReservationTarget: &ec2.CapacityReservationTargetResponse{
+				CapacityReservationId: aws.String(v.CapacityReservationId),
+			},
+		}
+	}
+
+	// Spot lineage stamped by the post-launch write-back survives a stop, so
+	// project it regardless of runtime state. Both empty for on-demand, so the
+	// fields stay absent there.
+	if v.InstanceLifecycle != "" {
+		instanceCopy.InstanceLifecycle = aws.String(v.InstanceLifecycle)
+	}
+	if v.SpotInstanceRequestId != "" {
+		instanceCopy.SpotInstanceRequestId = aws.String(v.SpotInstanceRequestId)
+	}
+
+	return &instanceCopy, stateMapped
+}
+
+// ParseInstanceIDFilter builds the set of instance IDs to filter on, rejecting
+// any ID without the "i-" prefix (including the empty string) as malformed. Nil
+// entries carry no constraint and are skipped. Empty input yields an empty set,
+// which callers treat as "no filter". It is pure so both describe paths validate
+// identically.
+func ParseInstanceIDFilter(ids []*string) (map[string]bool, error) {
+	filter := make(map[string]bool)
+	for _, id := range ids {
+		if id == nil {
+			continue
+		}
+		if !strings.HasPrefix(*id, "i-") {
+			return nil, errors.New(awserrors.ErrorInvalidInstanceIDMalformed)
+		}
+		filter[*id] = true
+	}
+	return filter, nil
+}

spinifex/handlers/ec2/instance/projection_test.go

@@ -0,0 +1,207 @@
+package handlers_ec2_instance
+
+import (
+	"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/vm"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// fullVM returns a VM whose every vm.VM-sourced projection field is populated,
+// so a projection either carries each through or deliberately drops it.
+func fullVM(status vm.InstanceState) *vm.VM {
+	return &vm.VM{
+		ID:     "i-abc123",
+		Status: status,
+		Instance: &ec2.Instance{
+			InstanceId:       aws.String("i-abc123"),
+			PrivateIpAddress: aws.String("10.0.0.5"),
+		},
+		PublicIP:              "203.0.113.7",
+		PlacementGroupName:    "pg-1",
+		IamInstanceProfileArn: "arn:aws:iam::123456789012:instance-profile/role",
+		CapacityReservationId: "cr-1",
+		InstanceLifecycle:     "spot",
+		SpotInstanceRequestId: "sir-1",
+	}
+}
+
+// runningCfg is the projection the running describe path passes.
+func runningCfg() InstanceProjection {
+	return InstanceProjection{
+		Region:                "us-east-1",
+		DNSBaseDomain:         "example.com",
+		DNSInternalDomain:     "compute.internal",
+		AZ:                    "us-east-1a",
+		IncludeRuntimeNetwork: true,
+		FallbackStateName:     "pending",
+	}
+}
+
+// stoppedCfg is the projection the stopped/terminated KV path passes.
+func stoppedCfg(code int64, name string) InstanceProjection {
+	return InstanceProjection{
+		AZ:                "us-east-1a",
+		FallbackStateCode: code,
+		FallbackStateName: name,
+	}
+}
+
+func TestProjectInstance_RunningProjectsAllFields(t *testing.T) {
+	got, mapped := ProjectInstance(fullVM(vm.StateRunning), runningCfg())
+
+	require.True(t, mapped)
+	assert.Equal(t, int64(16), aws.Int64Value(got.State.Code))
+	assert.Equal(t, "running", aws.StringValue(got.State.Name))
+
+	// Runtime network is present for a running instance.
+	assert.Equal(t, "203.0.113.7", aws.StringValue(got.PublicIpAddress))
+	assert.NotEmpty(t, aws.StringValue(got.PublicDnsName))
+	assert.NotEmpty(t, aws.StringValue(got.PrivateDnsName))
+
+	require.NotNil(t, got.Placement)
+	assert.Equal(t, "pg-1", aws.StringValue(got.Placement.GroupName))
+	assert.Equal(t, "us-east-1a", aws.StringValue(got.Placement.AvailabilityZone))
+
+	require.NotNil(t, got.IamInstanceProfile)
+	assert.Equal(t, "arn:aws:iam::123456789012:instance-profile/role", aws.StringValue(got.IamInstanceProfile.Arn))
+	// Id is left for the gateway to resolve post-aggregation.
+	assert.Nil(t, got.IamInstanceProfile.Id)
+
+	assert.Equal(t, "cr-1", aws.StringValue(got.CapacityReservationId))
+	require.NotNil(t, got.CapacityReservationSpecification)
+
+	assert.Equal(t, "spot", aws.StringValue(got.InstanceLifecycle))
+	assert.Equal(t, "sir-1", aws.StringValue(got.SpotInstanceRequestId))
+}
+
+// TestProjectInstance_StoppedRetainsPlacementAndSpot is the direct bug fix:
+// Placement and Spot lineage survive a stop, while the runtime network and the
+// capacity reservation — all released by AWS on stop — do not.
+func TestProjectInstance_StoppedRetainsPlacementAndSpot(t *testing.T) {
+	got, mapped := ProjectInstance(fullVM(vm.StateStopped), stoppedCfg(80, "stopped"))
+
+	require.True(t, mapped)
+	assert.Equal(t, "stopped", aws.StringValue(got.State.Name))
+
+	// Retained across the stop.
+	require.NotNil(t, got.Placement)
+	assert.Equal(t, "pg-1", aws.StringValue(got.Placement.GroupName))
+	assert.Equal(t, "spot", aws.StringValue(got.InstanceLifecycle))
+	assert.Equal(t, "sir-1", aws.StringValue(got.SpotInstanceRequestId))
+
+	// Released on stop, so never projected onto a stopped instance.
+	assert.Nil(t, got.PublicIpAddress)
+	assert.Nil(t, got.PublicDnsName)
+	assert.Nil(t, got.PrivateDnsName)
+	assert.Nil(t, got.CapacityReservationId)
+	assert.Nil(t, got.CapacityReservationSpecification)
+}
+
+// TestProjectInstance_EmptyIamArnClearsStaleProfile guards the auto-clear: an
+// empty ARN on the VM record must wipe any profile left on the stored instance.
+func TestProjectInstance_EmptyIamArnClearsStaleProfile(t *testing.T) {
+	v := fullVM(vm.StateRunning)
+	v.IamInstanceProfileArn = ""
+	v.Instance.IamInstanceProfile = &ec2.IamInstanceProfile{Arn: aws.String("arn:aws:iam::123456789012:instance-profile/stale")}
+
+	got, _ := ProjectInstance(v, runningCfg())
+
+	assert.Nil(t, got.IamInstanceProfile)
+}
+
+// TestProjectInstance_UnmappedStatusUsesFallback covers a stored status with no
+// EC2 equivalent: the caller's fallback labels it and stateMapped reports the gap.
+func TestProjectInstance_UnmappedStatusUsesFallback(t *testing.T) {
+	got, mapped := ProjectInstance(fullVM(vm.InstanceState("weird-status")), stoppedCfg(48, "terminated"))
+
+	assert.False(t, mapped)
+	assert.Equal(t, int64(48), aws.Int64Value(got.State.Code))
+	assert.Equal(t, "terminated", aws.StringValue(got.State.Name))
+}
+
+// TestProjectInstance_DoesNotMutateSource confirms the projection writes only to
+// its fresh copy, never back to the stored vm.VM.Instance shared under the lock.
+func TestProjectInstance_DoesNotMutateSource(t *testing.T) {
+	v := fullVM(vm.StateRunning)
+
+	_, _ = ProjectInstance(v, runningCfg())
+
+	assert.Nil(t, v.Instance.PublicIpAddress)
+	assert.Nil(t, v.Instance.State)
+	assert.Nil(t, v.Instance.Placement)
+}
+
+// TestProjectInstance_PathsAgreeOnRetainedFields is the regression guard that
+// would have caught the original drift: for one vm.VM, the fields AWS keeps
+// across a stop must be identical whether projected by the running path or the
+// stopped path. The running-only fields must, conversely, differ.
+func TestProjectInstance_PathsAgreeOnRetainedFields(t *testing.T) {
+	v := fullVM(vm.StateStopped)
+
+	running, _ := ProjectInstance(v, runningCfg())
+	stopped, _ := ProjectInstance(v, stoppedCfg(80, "stopped"))
+
+	// Retained-field set must match across both paths.
+	assert.Equal(t, running.Placement, stopped.Placement)
+	assert.Equal(t, running.InstanceLifecycle, stopped.InstanceLifecycle)
+	assert.Equal(t, running.SpotInstanceRequestId, stopped.SpotInstanceRequestId)
+	assert.Equal(t, running.IamInstanceProfile, stopped.IamInstanceProfile)
+
+	// Runtime-only fields are intentionally path-specific.
+	assert.NotEqual(t, running.PublicIpAddress, stopped.PublicIpAddress)
+	assert.NotEqual(t, running.CapacityReservationId, stopped.CapacityReservationId)
+}
+
+func TestParseInstanceIDFilter(t *testing.T) {
+	tests := []struct {
+		name    string
+		ids     []*string
+		want    map[string]bool
+		wantErr bool
+	}{
+		{
+			name: "valid i- IDs",
+			ids:  []*string{aws.String("i-aaa"), aws.String("i-bbb")},
+			want: map[string]bool{"i-aaa": true, "i-bbb": true},
+		},
+		{
+			name:    "malformed ID rejected",
+			ids:     []*string{aws.String("not-an-id")},
+			wantErr: true,
+		},
+		{
+			name:    "empty string rejected as malformed",
+			ids:     []*string{aws.String("")},
+			wantErr: true,
+		},
+		{
+			name: "nil entries skipped",
+			ids:  []*string{nil, aws.String("i-aaa"), nil},
+			want: map[string]bool{"i-aaa": true},
+		},
+		{
+			name: "empty input yields empty filter",
+			ids:  nil,
+			want: map[string]bool{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseInstanceIDFilter(tt.ids)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error())
+				assert.Nil(t, got)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

spinifex/handlers/ec2/instance/service_impl.go

@@ -1754,14 +1754,9 @@ func matchTagValue(tags []*ec2.Tag, values []string) bool {
 func (s *InstanceServiceImpl) DescribeInstances(ctx context.Context, input *ec2.DescribeInstancesInput, accountID string) (*ec2.DescribeInstancesOutput, error) {
 	slog.InfoContext(ctx, "Processing DescribeInstances request from this node", "accountID", accountID)
 
-	instanceIDFilter := make(map[string]bool)
-	for _, id := range input.InstanceIds {
-		if id != nil && *id != "" {
-			if !strings.HasPrefix(*id, "i-") {
-				return nil, errors.New(awserrors.ErrorInvalidInstanceIDMalformed)
-			}
-			instanceIDFilter[*id] = true
-		}
+	instanceIDFilter, err := ParseInstanceIDFilter(input.InstanceIds)
+	if err != nil {
+		return nil, err
 	}
 
 	parsedFilters, err := filterutil.ParseFilters(input.Filters, DescribeInstancesValidFilters)
@@ -1807,35 +1802,29 @@ func (s *InstanceServiceImpl) DescribeInstances(ctx context.Context, input *ec2.
 				reservationMap[resID] = reservation
 			}
 
-			instanceCopy := *instance.Instance
-			instanceCopy.State = &ec2.InstanceState{}
-
-			if instance.PublicIP != "" && instanceCopy.PublicIpAddress == nil {
-				instanceCopy.PublicIpAddress = aws.String(instance.PublicIP)
-			}
-
-			if info, ok := vm.EC2APIState(instance.Status); ok {
-				instanceCopy.State.SetCode(info.Code)
-				instanceCopy.State.SetName(info.Name)
-			} else {
+			// Project every vm.VM-sourced field via the shared projection, the
+			// same call the daemon's running path makes, so both define the field
+			// set once. Running instances include their runtime network and
+			// capacity reservation; an unmapped status falls back to pending/0.
+			projected, stateMapped := ProjectInstance(instance, InstanceProjection{
+				Region:                s.config.Region,
+				DNSBaseDomain:         s.dnsBaseDomain,
+				DNSInternalDomain:     s.dnsInternalDomain,
+				AZ:                    s.config.AZ,
+				IncludeRuntimeNetwork: true,
+				FallbackStateCode:     0,
+				FallbackStateName:     "pending",
+			})
+			if !stateMapped {
 				slog.WarnContext(ctx, "Instance has unmapped status, reporting as pending",
 					"instanceId", instance.ID, "status", string(instance.Status))
-				instanceCopy.State.SetCode(0)
-				instanceCopy.State.SetName("pending")
 			}
 
-			if instance.PlacementGroupName != "" {
-				instanceCopy.Placement = &ec2.Placement{
-					GroupName:        aws.String(instance.PlacementGroupName),
-					AvailabilityZone: aws.String(s.config.AZ),
-				}
-			}
-
-			if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, &instanceCopy, parsedFilters) {
+			if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, projected, parsedFilters) {
 				continue
 			}
 
-			reservationMap[resID].Instances = append(reservationMap[resID].Instances, &instanceCopy)
+			reservationMap[resID].Instances = append(reservationMap[resID].Instances, projected)
 		}
 	})
 
@@ -1897,11 +1886,9 @@ func (s *InstanceServiceImpl) DescribeTerminatedInstances(ctx context.Context, i
 }
 
 func (s *InstanceServiceImpl) describeInstancesFromKV(ctx context.Context, input *ec2.DescribeInstancesInput, accountID string, listFn func() ([]*vm.VM, error), fallbackCode int64, fallbackName, opName string) (*ec2.DescribeInstancesOutput, error) {
-	instanceIDFilter := make(map[string]bool)
-	for _, id := range input.InstanceIds {
-		if id != nil {
-			instanceIDFilter[*id] = true
-		}
+	instanceIDFilter, err := ParseInstanceIDFilter(input.InstanceIds)
+	if err != nil {
+		return nil, err
 	}
 
 	parsedFilters, filterErr := filterutil.ParseFilters(input.Filters, DescribeInstancesValidFilters)
@@ -1946,21 +1933,22 @@ func (s *InstanceServiceImpl) describeInstancesFromKV(ctx context.Context, input
 			reservationMap[resID] = reservation
 		}
 
-		instanceCopy := *instance.Instance
-		instanceCopy.State = &ec2.InstanceState{}
-		if info, ok := vm.EC2APIState(instance.Status); ok {
-			instanceCopy.State.SetCode(info.Code)
-			instanceCopy.State.SetName(info.Name)
-		} else {
-			instanceCopy.State.SetCode(fallbackCode)
-			instanceCopy.State.SetName(fallbackName)
-		}
+		// Reuse the shared projection so stopped/terminated instances carry the
+		// same fields as the running path. Runtime network and the capacity
+		// reservation are released on stop, so IncludeRuntimeNetwork stays false;
+		// Placement and Spot lineage survive and are projected. The caller's
+		// fallback labels the state when a stored status has no EC2 mapping.
+		projected, _ := ProjectInstance(instance, InstanceProjection{
+			AZ:                s.config.AZ,
+			FallbackStateCode: fallbackCode,
+			FallbackStateName: fallbackName,
+		})
 
-		if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, &instanceCopy, parsedFilters) {
+		if len(parsedFilters) > 0 && !instanceMatchesFilters(instance, projected, parsedFilters) {
 			continue
 		}
 
-		reservationMap[resID].Instances = append(reservationMap[resID].Instances, &instanceCopy)
+		reservationMap[resID].Instances = append(reservationMap[resID].Instances, projected)
 	}
 
 	reservations := make([]*ec2.Reservation, 0, len(reservationMap))
@@ -2814,15 +2802,9 @@ func instanceStatusMatchesFilters(v *vm.VM, is *ec2.InstanceStatus, filters map[
 func (s *InstanceServiceImpl) DescribeInstanceStatus(ctx context.Context, input *ec2.DescribeInstanceStatusInput, accountID string) (*ec2.DescribeInstanceStatusOutput, error) {
 	slog.InfoContext(ctx, "Processing DescribeInstanceStatus request from this node", "accountID", accountID)
 
-	instanceIDFilter := make(map[string]bool)
-	for _, id := range input.InstanceIds {
-		if id == nil || *id == "" {
-			continue
-		}
-		if !strings.HasPrefix(*id, "i-") {
-			return nil, errors.New(awserrors.ErrorInvalidInstanceIDMalformed)
-		}
-		instanceIDFilter[*id] = true
+	instanceIDFilter, err := ParseInstanceIDFilter(input.InstanceIds)
+	if err != nil {
+		return nil, err
 	}
 
 	parsedFilters, err := filterutil.ParseFilters(input.Filters, DescribeInstanceStatusValidFilters)

spinifex/handlers/ec2/instance/service_impl_test.go

@@ -828,7 +828,7 @@ func TestDescribeInstances_OneVisibleInstance(t *testing.T) {
 		},
 		Instance: &ec2.Instance{InstanceId: aws.String(id), InstanceType: aws.String("t3.micro")},
 	}
-	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v})}
+	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{id: v}), config: &config.Config{}}
 
 	out, err := svc.DescribeInstances(context.Background(), &ec2.DescribeInstancesInput{}, owner)
 	require.NoError(t, err)
@@ -889,7 +889,7 @@ func TestDescribeInstances_RootSeesManagedSystemVM(t *testing.T) {
 		},
 		Instance: &ec2.Instance{InstanceId: aws.String("i-lb")},
 	}
-	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{v.ID: v})}
+	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{v.ID: v}), config: &config.Config{}}
 
 	out, err := svc.DescribeInstances(context.Background(), &ec2.DescribeInstancesInput{}, utils.GlobalAccountID)
 	require.NoError(t, err)
@@ -911,7 +911,7 @@ func TestDescribeInstances_CustomerWorkloadStaysVisible(t *testing.T) {
 		},
 		Instance: &ec2.Instance{InstanceId: aws.String("i-worker")},
 	}
-	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{v.ID: v})}
+	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{v.ID: v}), config: &config.Config{}}
 
 	out, err := svc.DescribeInstances(context.Background(), &ec2.DescribeInstancesInput{}, owner)
 	require.NoError(t, err)
@@ -947,7 +947,7 @@ func TestDescribeInstances_FilterByInstanceID(t *testing.T) {
 	svc := &InstanceServiceImpl{vmMgr: mgrWith(map[string]*vm.VM{
 		keep.ID: keep,
 		drop.ID: drop,
-	})}
+	}), config: &config.Config{}}
 
 	input := &ec2.DescribeInstancesInput{InstanceIds: []*string{aws.String("i-keep")}}
 	out, err := svc.DescribeInstances(context.Background(), input, utils.GlobalAccountID)
@@ -1191,7 +1191,7 @@ func TestDescribeStoppedInstances_HappyPath(t *testing.T) {
 			},
 		},
 	}
-	svc := &InstanceServiceImpl{stoppedStore: store}
+	svc := &InstanceServiceImpl{stoppedStore: store, config: &config.Config{}}
 
 	out, err := svc.DescribeStoppedInstances(context.Background(), &ec2.DescribeInstancesInput{}, owner)
 	require.NoError(t, err)
@@ -1221,7 +1221,7 @@ func TestDescribeTerminatedInstances_HappyPath(t *testing.T) {
 			},
 		},
 	}
-	svc := &InstanceServiceImpl{stoppedStore: store}
+	svc := &InstanceServiceImpl{stoppedStore: store, config: &config.Config{}}
 
 	out, err := svc.DescribeTerminatedInstances(context.Background(), &ec2.DescribeInstancesInput{}, owner)
 	require.NoError(t, err)