fix: match AWS key pair and image response fields

spinifex/gateway/ec2.go

@@ -105,13 +105,20 @@ func ec2Handler[In any](handler func(ctx context.Context, input *In, gw *Gateway
 	}
 }
 
+// ec2ListsAsSet names the list fields AWS omits when empty instead of
+// rendering them. Each is rendered as its handler set it, so nil omits it.
+var ec2ListsAsSet = map[reflect.Type][]string{
+	reflect.TypeFor[ec2.CreateKeyPairOutput](): {"Tags"},
+	reflect.TypeFor[ec2.Image]():               {"Tags", "ProductCodes"},
+}
+
 // marshalEC2Response renders an EC2 handler's output into the action's XML
 // envelope. Every EC2 action funnels through here (via ec2Handler and
 // ec2HandlerWithReq), so wire-format fixes belong here, not in each handler.
 func marshalEC2Response(action string, output any) ([]byte, error) {
 	// BuildXML omits a nil slice's container element entirely but renders an
-	// empty one for a non-nil empty slice; AWS always renders the latter.
-	normalized := utils.NormalizeXMLOutput(output)
+	// empty one for a non-nil empty slice; AWS mostly renders the latter.
+	normalized := utils.NormalizeXMLOutput(output, ec2ListsAsSet)
 	// The SDK's generated output structs never carry a RequestId field.
 	withRequestID := utils.WithRequestID(normalized, uuid.NewV4().String())
 	payload := utils.GenerateXMLPayload(action+"Response", withRequestID)

spinifex/gateway/ec2_emptyresult_test.go

@@ -116,3 +116,23 @@ func TestEC2Describe_NonEmptyResult_Unchanged(t *testing.T) {
 	assert.Contains(t, body, "<keySet>")
 	assert.NotContains(t, body, "<keySet></keySet>", "non-empty KeyPairs must not render as an empty container")
 }
+
+// AWS omits some empty lists rather than rendering them: an untagged
+// CreateKeyPair's tagSet, and an image's tagSet and productCodes. Other empty
+// lists in the same responses are still rendered.
+func TestEC2_ListsAWSOmitsWhenEmpty(t *testing.T) {
+	key := render[ec2.CreateKeyPairInput](t, "CreateKeyPair", ec2.CreateKeyPairOutput{KeyName: aws.String("k")})
+	assert.NotContains(t, key, "tagSet")
+
+	images := render[ec2.DescribeImagesInput](t, "DescribeImages", ec2.DescribeImagesOutput{
+		Images: []*ec2.Image{{ImageId: aws.String("ami-1")}},
+	})
+	assert.NotContains(t, images, "tagSet")
+	assert.NotContains(t, images, "productCodes")
+	assert.Contains(t, images, "<blockDeviceMapping></blockDeviceMapping>")
+
+	tagged := render[ec2.DescribeImagesInput](t, "DescribeImages", ec2.DescribeImagesOutput{
+		Images: []*ec2.Image{{ImageId: aws.String("ami-1"), Tags: []*ec2.Tag{{Key: aws.String("k"), Value: aws.String("v")}}}},
+	})
+	assert.Contains(t, tagged, "<tagSet><item><key>k</key><value>v</value></item></tagSet>")
+}

spinifex/handlers/ec2/image/describe_byid_test.go

@@ -160,6 +160,41 @@ func TestDescribeImagesByID_SystemAMIIsVisibleToEveryAccount(t *testing.T) {
 	assert.Equal(t, "amazon", aws.StringValue(out.Images[0].ImageOwnerAlias))
 }
 
+// A system image renders as AWS's public, Amazon-owned images do, and the
+// is-public filter reads the rendered value; a private image stays private.
+func TestDescribeImagesByID_SystemAMIRendersPublicAmazon(t *testing.T) {
+	tests := []struct {
+		name       string
+		ownerAlias string
+		wantPublic bool
+		wantAlias  string
+	}{
+		{name: "system image", ownerAlias: "system", wantPublic: true, wantAlias: "amazon"},
+		{name: "own image", ownerAlias: byIDAccount, wantPublic: false, wantAlias: byIDAccount},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			store := recordingstore.New()
+			svc := handlers_ec2_image.NewImageServiceImplWithStore(store, byIDBucket)
+			putByIDAMI(t, store, byIDAMIDoc(byIDImage, tt.ownerAlias))
+
+			out, err := svc.DescribeImages(t.Context(), &ec2.DescribeImagesInput{
+				ImageIds: aws.StringSlice([]string{byIDImage}),
+				Filters: []*ec2.Filter{{
+					Name: aws.String("is-public"), Values: aws.StringSlice([]string{strconv.FormatBool(tt.wantPublic)}),
+				}},
+			}, byIDAccount)
+
+			require.NoError(t, err)
+			require.Len(t, out.Images, 1)
+			img := out.Images[0]
+			assert.Equal(t, tt.wantPublic, aws.BoolValue(img.Public))
+			assert.Equal(t, tt.wantAlias, aws.StringValue(img.ImageOwnerAlias))
+			assert.Equal(t, "RunInstances", aws.StringValue(img.UsageOperation))
+		})
+	}
+}
+
 // The Owners parameter is matched against the resolved owner, and a named image
 // that no owner selects is not-found rather than an empty success.
 func TestDescribeImagesByID_OwnerFilters(t *testing.T) {

spinifex/handlers/ec2/image/service_impl.go

@@ -295,6 +295,7 @@ func renderVisibleImage(amiMeta ebsmetadata.AMI, input *ec2.DescribeImagesInput,
 		Architecture:       aws.String(amiMeta.Architecture),
 		PlatformDetails:    aws.String(amiMeta.PlatformDetails),
 		Platform:           utils.PlatformFromDetails(amiMeta.PlatformDetails),
+		UsageOperation:     utils.UsageOperationFromDetails(amiMeta.PlatformDetails),
 		CreationDate:       aws.String(amiMeta.CreationDate.UTC().Format("2006-01-02T15:04:05.000Z")),
 		RootDeviceType:     aws.String(amiMeta.RootDeviceType),
 		VirtualizationType: aws.String(amiMeta.Virtualization),
@@ -306,6 +307,13 @@ func renderVisibleImage(amiMeta ebsmetadata.AMI, input *ec2.DescribeImagesInput,
 		Hypervisor:         aws.String("xen"),
 	}
 
+	// A system image stands where AWS's public, Amazon-owned images do. Only the
+	// rendered values change: the stored alias still identifies it as system.
+	if isSystemAMI {
+		image.Public = aws.Bool(true)
+		image.ImageOwnerAlias = aws.String("amazon")
+	}
+
 	// RegisterImage takes BootMode as an optional input, so an AMI can
 	// record none. The enum has no empty member: omit it, don't emit "".
 	if amiMeta.BootMode != "" {

spinifex/handlers/ec2/key/service_impl.go

@@ -211,9 +211,15 @@ func (s *KeyServiceImpl) CreateKeyPair(ctx context.Context, input *ec2.CreateKey
 	keyPairID := utils.GenerateResourceID("key")
 	tagMap := utils.ExtractTags(input.TagSpecifications, "key-pair")
 	tags := utils.MapToEC2Tags(tagMap)
+	// AWS ends RSA material at its END line, while OpenSSH-format Ed25519
+	// material keeps the trailing newline.
+	keyMaterial := string(privateKeyData)
+	if keyType == "rsa" {
+		keyMaterial = strings.TrimSuffix(keyMaterial, "\n")
+	}
 	output := &ec2.CreateKeyPairOutput{
 		KeyFingerprint: aws.String(fingerprint),
-		KeyMaterial:    aws.String(string(privateKeyData)),
+		KeyMaterial:    aws.String(keyMaterial),
 		KeyName:        aws.String(keyName),
 		KeyPairId:      aws.String(keyPairID),
 		Tags:           tags,
@@ -608,7 +614,7 @@ func (s *KeyServiceImpl) DeleteKeyPair(ctx context.Context, input *ec2.DeleteKey
 			// AWS DeleteKeyPair is idempotent — return success for non-existent keys
 			if err.Error() == awserrors.ErrorInvalidKeyPairNotFound {
 				slog.DebugContext(ctx, "DeleteKeyPair: key pair not found, returning success (idempotent)", "keyPairId", keyPairID)
-				return &ec2.DeleteKeyPairOutput{}, nil
+				return &ec2.DeleteKeyPairOutput{Return: aws.Bool(true)}, nil
 			}
 			slog.ErrorContext(ctx, "Failed to get keyName from keyPairId", "keyPairId", keyPairID, "err", err)
 			return nil, err
@@ -628,7 +634,7 @@ func (s *KeyServiceImpl) DeleteKeyPair(ctx context.Context, input *ec2.DeleteKey
 			// AWS DeleteKeyPair is idempotent — return success for non-existent keys
 			if err.Error() == awserrors.ErrorInvalidKeyPairNotFound {
 				slog.DebugContext(ctx, "DeleteKeyPair: key pair not found, returning success (idempotent)", "keyName", keyName)
-				return &ec2.DeleteKeyPairOutput{}, nil
+				return &ec2.DeleteKeyPairOutput{Return: aws.Bool(true)}, nil
 			}
 			slog.ErrorContext(ctx, "Failed to find keyPairId from keyName", "keyName", keyName, "err", err)
 			return nil, err
@@ -665,7 +671,8 @@ func (s *KeyServiceImpl) DeleteKeyPair(ctx context.Context, input *ec2.DeleteKey
 
 	slog.InfoContext(ctx, "Key pair deleted successfully", "keyName", keyName, "keyPairId", keyPairID)
 
-	return &ec2.DeleteKeyPairOutput{}, nil
+	// AWS names the key only when one was actually deleted.
+	return &ec2.DeleteKeyPairOutput{Return: aws.Bool(true), KeyPairId: aws.String(keyPairID)}, nil
 }
 
 // DescribeKeyPairs lists available key pairs by reading metadata files from S3

spinifex/handlers/ec2/key/service_impl_test.go

@@ -179,6 +179,7 @@ func TestCreateKeyPair_ED25519(t *testing.T) {
 
 	// ED25519 has no PEM representation, so it stays in the OpenSSH container.
 	assert.Contains(t, *out.KeyMaterial, "BEGIN OPENSSH PRIVATE KEY")
+	assert.True(t, strings.HasSuffix(*out.KeyMaterial, "-----END OPENSSH PRIVATE KEY-----\n"))
 }
 
 func TestCreateKeyPair_RSA(t *testing.T) {
@@ -196,6 +197,7 @@ func TestCreateKeyPair_RSA(t *testing.T) {
 	// PKCS#1 PEM, as AWS returns it: get-password-data --priv-launch-key cannot
 	// read the OpenSSH container ssh-keygen writes by default.
 	assert.Contains(t, *out.KeyMaterial, "BEGIN RSA PRIVATE KEY")
+	assert.True(t, strings.HasSuffix(*out.KeyMaterial, "-----END RSA PRIVATE KEY-----"), "AWS ends RSA material without a newline")
 
 	// The key is generated per-run, so the digest is not knowable in advance.
 	// Recompute it from the private key the caller was handed: that is what pins
@@ -488,7 +490,8 @@ func TestDeleteKeyPair_ByKeyName(t *testing.T) {
 		KeyName: aws.String("to-delete-by-name"),
 	}, testAccountID)
 	require.NoError(t, err)
-	assert.NotNil(t, result)
+	assert.True(t, aws.BoolValue(result.Return))
+	assert.Equal(t, aws.StringValue(imported.KeyPairId), aws.StringValue(result.KeyPairId))
 
 	// Verify public key removed from S3
 	keyPath := "keys/" + testAccountID + "/to-delete-by-name"
@@ -516,7 +519,8 @@ func TestDeleteKeyPair_ByKeyPairId(t *testing.T) {
 		KeyPairId: imported.KeyPairId,
 	}, testAccountID)
 	require.NoError(t, err)
-	assert.NotNil(t, result)
+	assert.True(t, aws.BoolValue(result.Return))
+	assert.Equal(t, aws.StringValue(imported.KeyPairId), aws.StringValue(result.KeyPairId))
 
 	// Verify public key removed from S3
 	keyPath := "keys/" + testAccountID + "/to-delete-by-id"
@@ -543,7 +547,8 @@ func TestDeleteKeyPairIdempotent(t *testing.T) {
 			KeyName: aws.String("no-such-key"),
 		}, testAccountID)
 		require.NoError(t, err)
-		assert.NotNil(t, result)
+		assert.True(t, aws.BoolValue(result.Return))
+		assert.Nil(t, result.KeyPairId, "no key was deleted, so none is named")
 	})
 
 	t.Run("NonExistentKeyPairId", func(t *testing.T) {
@@ -551,7 +556,8 @@ func TestDeleteKeyPairIdempotent(t *testing.T) {
 			KeyPairId: aws.String("key-0123456789abcdef0"),
 		}, testAccountID)
 		require.NoError(t, err)
-		assert.NotNil(t, result)
+		assert.True(t, aws.BoolValue(result.Return))
+		assert.Nil(t, result.KeyPairId, "no key was deleted, so none is named")
 	})
 }
 

spinifex/utils/platform.go

@@ -20,3 +20,23 @@ func PlatformFromDetails(platformDetails string) *string {
 	p := PlatformWindows
 	return &p
 }
+
+// usageOperations maps PlatformDetails to the billing code AWS reports as an
+// image's UsageOperation, for the platforms checked against real AWS images.
+var usageOperations = map[string]string{
+	"Linux/UNIX":               "RunInstances",
+	"Red Hat Enterprise Linux": "RunInstances:0010",
+	"SUSE Linux":               "RunInstances:000g",
+	"Ubuntu Pro Linux":         "RunInstances:0g00",
+	"Windows":                  "RunInstances:0002",
+}
+
+// UsageOperationFromDetails returns the UsageOperation for platformDetails, or
+// nil for a platform whose code has not been confirmed.
+func UsageOperationFromDetails(platformDetails string) *string {
+	op, ok := usageOperations[platformDetails]
+	if !ok {
+		return nil
+	}
+	return &op
+}

spinifex/utils/platform_test.go

@@ -35,3 +35,28 @@ func TestPlatformFromDetails(t *testing.T) {
 		})
 	}
 }
+
+func TestUsageOperationFromDetails(t *testing.T) {
+	tests := []struct {
+		platformDetails string
+		want            string
+	}{
+		{platformDetails: "Linux/UNIX", want: "RunInstances"},
+		{platformDetails: "Windows", want: "RunInstances:0002"},
+		{platformDetails: "Ubuntu Pro Linux", want: "RunInstances:0g00"},
+		{platformDetails: "Windows BYOL", want: ""},
+		{platformDetails: "", want: ""},
+	}
+	for _, tt := range tests {
+		got := UsageOperationFromDetails(tt.platformDetails)
+		if tt.want == "" {
+			if got != nil {
+				t.Errorf("UsageOperationFromDetails(%q) = %q, want nil", tt.platformDetails, *got)
+			}
+			continue
+		}
+		if got == nil || *got != tt.want {
+			t.Errorf("UsageOperationFromDetails(%q) = %v, want %q", tt.platformDetails, got, tt.want)
+		}
+	}
+}

spinifex/utils/utils.go

@@ -18,6 +18,7 @@ import (
 	"os"
 	"os/signal"
 	"reflect"
+	"slices"
 	"strings"
 	"time"
 
@@ -91,8 +92,10 @@ func GenerateXMLPayload(locationName string, payload any) any {
 // NormalizeXMLOutput returns a copy of output with every nil slice field
 // (recursively) replaced by a non-nil empty slice, since aws-sdk-go's
 // xmlutil.BuildXML omits a nil slice's container element entirely but
-// renders an empty one for a non-nil empty slice, unlike real AWS.
-func NormalizeXMLOutput(output any) any {
+// renders an empty one for a non-nil empty slice. AWS renders most empty lists
+// but omits some; asSet names those fields, per struct type, and they are
+// left as the handler set them so a nil one stays omitted.
+func NormalizeXMLOutput(output any, asSet map[reflect.Type][]string) any {
 	v := reflect.ValueOf(output)
 	if !v.IsValid() {
 		return output
@@ -101,33 +104,34 @@ func NormalizeXMLOutput(output any) any {
 	// can only Set fields through an addressable Value.
 	ptr := reflect.New(v.Type())
 	ptr.Elem().Set(v)
-	normalizeNilSlices(ptr.Elem())
+	normalizeNilSlices(ptr.Elem(), asSet)
 	return ptr.Elem().Interface()
 }
 
 // normalizeNilSlices walks v in place, turning nil slice fields into empty
 // ones and recursing into structs, pointers, and existing slice elements.
-func normalizeNilSlices(v reflect.Value) {
+func normalizeNilSlices(v reflect.Value, asSet map[reflect.Type][]string) {
 	switch v.Kind() {
 	case reflect.Pointer:
 		if !v.IsNil() {
-			normalizeNilSlices(v.Elem())
+			normalizeNilSlices(v.Elem(), asSet)
 		}
 	case reflect.Struct:
-		for _, field := range v.Fields() {
+		keep := asSet[v.Type()]
+		for sf, field := range v.Fields() {
 			switch field.Kind() {
 			case reflect.Slice:
 				if field.IsNil() {
-					if field.CanSet() {
+					if field.CanSet() && !slices.Contains(keep, sf.Name) {
 						field.Set(reflect.MakeSlice(field.Type(), 0, 0))
 					}
 				} else {
 					for j := 0; j < field.Len(); j++ {
-						normalizeNilSlices(field.Index(j))
+						normalizeNilSlices(field.Index(j), asSet)
 					}
 				}
 			case reflect.Pointer, reflect.Struct:
-				normalizeNilSlices(field)
+				normalizeNilSlices(field, asSet)
 			}
 		}
 	}

spinifex/utils/utils_test.go

@@ -1891,11 +1891,11 @@ type normalizeOuter struct {
 
 func TestNormalizeXMLOutput_InvalidValue(t *testing.T) {
 	var output any
-	assert.Nil(t, NormalizeXMLOutput(output))
+	assert.Nil(t, NormalizeXMLOutput(output, nil))
 }
 
 func TestNormalizeXMLOutput_TopLevelNilSlice(t *testing.T) {
-	out := NormalizeXMLOutput(normalizeOuter{}).(normalizeOuter)
+	out := NormalizeXMLOutput(normalizeOuter{}, nil).(normalizeOuter)
 	require.NotNil(t, out.Names)
 	assert.Empty(t, out.Names)
 }
@@ -1903,28 +1903,39 @@ func TestNormalizeXMLOutput_TopLevelNilSlice(t *testing.T) {
 func TestNormalizeXMLOutput_NestedPointerStruct(t *testing.T) {
 	out := NormalizeXMLOutput(normalizeOuter{
 		Nested: &normalizeInner{},
-	}).(normalizeOuter)
+	}, nil).(normalizeOuter)
 	require.NotNil(t, out.Nested)
 	require.NotNil(t, out.Nested.Tags, "nil slice inside a pointer field must be normalized")
 	assert.Empty(t, out.Nested.Tags)
 }
 
 func TestNormalizeXMLOutput_NestedValueStruct(t *testing.T) {
-	out := NormalizeXMLOutput(normalizeOuter{}).(normalizeOuter)
+	out := NormalizeXMLOutput(normalizeOuter{}, nil).(normalizeOuter)
 	require.NotNil(t, out.Value.Tags, "nil slice inside a nested struct field must be normalized")
 }
 
 func TestNormalizeXMLOutput_RecursesIntoSliceElements(t *testing.T) {
 	out := NormalizeXMLOutput(normalizeOuter{
 		Children: []*normalizeInner{{}, {Tags: []string{"a"}}},
-	}).(normalizeOuter)
+	}, nil).(normalizeOuter)
 	require.Len(t, out.Children, 2)
 	require.NotNil(t, out.Children[0].Tags, "nil slice inside a slice element must be normalized")
 	assert.Equal(t, []string{"a"}, out.Children[1].Tags, "an already-populated slice element must be left untouched")
 }
 
+func TestNormalizeXMLOutput_AsSetFieldsKeepNil(t *testing.T) {
+	asSet := map[reflect.Type][]string{reflect.TypeFor[normalizeInner](): {"Tags"}}
+	out := NormalizeXMLOutput(normalizeOuter{
+		Nested:   &normalizeInner{},
+		Children: []*normalizeInner{{Tags: []string{}}},
+	}, asSet).(normalizeOuter)
+	assert.Nil(t, out.Nested.Tags, "a nil as-set field must stay nil so BuildXML omits it")
+	require.NotNil(t, out.Children[0].Tags, "an empty as-set field must stay empty so BuildXML renders it")
+	require.NotNil(t, out.Names, "fields outside asSet are still normalized")
+}
+
 func TestNormalizeXMLOutput_NilPointerFieldUntouched(t *testing.T) {
-	out := NormalizeXMLOutput(normalizeOuter{}).(normalizeOuter)
+	out := NormalizeXMLOutput(normalizeOuter{}, nil).(normalizeOuter)
 	assert.Nil(t, out.Nested, "a nil pointer field must not be allocated")
 }