gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 09:01:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: conform DescribeImages BootMode and IAM list required members

Browse Source 
DescribeImages projected BootMode through aws.String unconditionally, so an
AMI recording none emitted "", which is outside the model enum. RegisterImage
takes BootMode as an optional input, so this is the normal path for an AMI
registered without one, not stale data. Set the member only when present.

ListAccessKeys and ListGroups built their results from a nil slice when
nothing matched. xmlutil drops a nil slice entirely rather than emitting an
empty element, so the response lost a member the model marks required. Start
both slices empty and non-nil.

Integration conformance drops from 18 violations to 15; the remainder are the
EC2 AccessDenied error-code findings, which are unrelated.
This commit is contained in:
Julian Sommer
2026-08-27 11:46:39 +10:00
parent 23f7081a2a
commit fae6b08acc
9 changed files with 65 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/aws-model-operation-coverage.md
+15 -15
View File
@@ -7,7 +7,7 @@ Implemented means a modelled operation is registered to a real handler. Stub and
| Service | API version | Modelled | Registered | Implemented | Stubbed | Unsupported | Missing | Extra |
|---|---:|---:|---:|---:|---:|---:|---:|---:|
| acm | 2015-12-08 | 15 | 9 | 9 | 0 | 0 | 6 | 0 |
| ec2 | 2016-11-15 | 625 | 119 | 119 | 0 | 0 | 506 | 0 |
| ec2 | 2016-11-15 | 625 | 121 | 121 | 0 | 0 | 504 | 0 |
| ecr | 2015-09-21 | 47 | 39 | 19 | 13 | 7 | 11 | 3 |
| ecs | 2014-11-13 | 56 | 39 | 31 | 5 | 0 | 20 | 3 |
| elasticloadbalancingv2 | 2015-12-01 | 46 | 37 | 33 | 0 | 0 | 13 | 4 |
@@ -56,9 +56,9 @@ None.
## ec2
Implements **119 of 625** modelled operations (19.0%).
Implements **121 of 625** modelled operations (19.4%).
<details><summary>Implemented (119)</summary>
<details><summary>Implemented (121)</summary>
`AllocateAddress`, `AssociateAddress`, `AssociateIamInstanceProfile`, `AssociateRouteTable`, `AttachInternetGateway`, `AttachNetworkInterface`, `AttachVolume`, `AuthorizeSecurityGroupEgress`
@@ -86,14 +86,16 @@ Implements **119 of 625** modelled operations (19.0%).
`ImportKeyPair`, `ModifyImageAttribute`, `ModifyInstanceAttribute`, `ModifyInstanceMetadataOptions`, `ModifyLaunchTemplate`, `ModifyNetworkInterfaceAttribute`, `ModifySubnetAttribute`, `ModifyVolume`
`ModifyVpcAttribute`, `RebootInstances`, `RegisterImage`, `ReleaseAddress`, `ReplaceIamInstanceProfileAssociation`, `ReplaceRoute`, `ReplaceRouteTableAssociation`, `RequestSpotInstances`
`ModifyVpcAttribute`, `MonitorInstances`, `RebootInstances`, `RegisterImage`, `ReleaseAddress`, `ReplaceIamInstanceProfileAssociation`, `ReplaceRoute`, `ReplaceRouteTableAssociation`
`ResetImageAttribute`, `RevokeSecurityGroupEgress`, `RevokeSecurityGroupIngress`, `RunInstances`, `StartInstances`, `StopInstances`, `TerminateInstances`
`RequestSpotInstances`, `ResetImageAttribute`, `RevokeSecurityGroupEgress`, `RevokeSecurityGroupIngress`, `RunInstances`, `StartInstances`, `StopInstances`, `TerminateInstances`
`UnmonitorInstances`
</details>
<details><summary>Missing from dispatch (506)</summary>
<details><summary>Missing from dispatch (504)</summary>
`AcceptAddressTransfer`, `AcceptReservedInstancesExchangeQuote`, `AcceptTransitGatewayMulticastDomainAssociations`, `AcceptTransitGatewayPeeringAttachment`, `AcceptTransitGatewayVpcAttachment`, `AcceptVpcEndpointConnections`, `AcceptVpcPeeringConnection`, `AdvertiseByoipCidr`
@@ -207,21 +209,19 @@ Implements **119 of 625** modelled operations (19.0%).
`ModifyVpcEndpointConnectionNotification`, `ModifyVpcEndpointServiceConfiguration`, `ModifyVpcEndpointServicePayerResponsibility`, `ModifyVpcEndpointServicePermissions`, `ModifyVpcPeeringConnectionOptions`, `ModifyVpcTenancy`, `ModifyVpnConnection`, `ModifyVpnConnectionOptions`
`ModifyVpnTunnelCertificate`, `ModifyVpnTunnelOptions`, `MonitorInstances`, `MoveAddressToVpc`, `MoveByoipCidrToIpam`, `ProvisionByoipCidr`, `ProvisionIpamByoasn`, `ProvisionIpamPoolCidr`
`ModifyVpnTunnelCertificate`, `ModifyVpnTunnelOptions`, `MoveAddressToVpc`, `MoveByoipCidrToIpam`, `ProvisionByoipCidr`, `ProvisionIpamByoasn`, `ProvisionIpamPoolCidr`, `ProvisionPublicIpv4PoolCidr`
`ProvisionPublicIpv4PoolCidr`, `PurchaseCapacityBlock`, `PurchaseHostReservation`, `PurchaseReservedInstancesOffering`, `PurchaseScheduledInstances`, `RegisterInstanceEventNotificationAttributes`, `RegisterTransitGatewayMulticastGroupMembers`, `RegisterTransitGatewayMulticastGroupSources`
`PurchaseCapacityBlock`, `PurchaseHostReservation`, `PurchaseReservedInstancesOffering`, `PurchaseScheduledInstances`, `RegisterInstanceEventNotificationAttributes`, `RegisterTransitGatewayMulticastGroupMembers`, `RegisterTransitGatewayMulticastGroupSources`, `RejectTransitGatewayMulticastDomainAssociations`
`RejectTransitGatewayMulticastDomainAssociations`, `RejectTransitGatewayPeeringAttachment`, `RejectTransitGatewayVpcAttachment`, `RejectVpcEndpointConnections`, `RejectVpcPeeringConnection`, `ReleaseHosts`, `ReleaseIpamPoolAllocation`, `ReplaceNetworkAclAssociation`
`RejectTransitGatewayPeeringAttachment`, `RejectTransitGatewayVpcAttachment`, `RejectVpcEndpointConnections`, `RejectVpcPeeringConnection`, `ReleaseHosts`, `ReleaseIpamPoolAllocation`, `ReplaceNetworkAclAssociation`, `ReplaceNetworkAclEntry`
`ReplaceNetworkAclEntry`, `ReplaceTransitGatewayRoute`, `ReplaceVpnTunnel`, `ReportInstanceStatus`, `RequestSpotFleet`, `ResetAddressAttribute`, `ResetEbsDefaultKmsKeyId`, `ResetFpgaImageAttribute`
`ReplaceTransitGatewayRoute`, `ReplaceVpnTunnel`, `ReportInstanceStatus`, `RequestSpotFleet`, `ResetAddressAttribute`, `ResetEbsDefaultKmsKeyId`, `ResetFpgaImageAttribute`, `ResetInstanceAttribute`
`ResetInstanceAttribute`, `ResetNetworkInterfaceAttribute`, `ResetSnapshotAttribute`, `RestoreAddressToClassic`, `RestoreImageFromRecycleBin`, `RestoreManagedPrefixListVersion`, `RestoreSnapshotFromRecycleBin`, `RestoreSnapshotTier`
`ResetNetworkInterfaceAttribute`, `ResetSnapshotAttribute`, `RestoreAddressToClassic`, `RestoreImageFromRecycleBin`, `RestoreManagedPrefixListVersion`, `RestoreSnapshotFromRecycleBin`, `RestoreSnapshotTier`, `RevokeClientVpnIngress`
`RevokeClientVpnIngress`, `RunScheduledInstances`, `SearchLocalGatewayRoutes`, `SearchTransitGatewayMulticastGroups`, `SearchTransitGatewayRoutes`, `SendDiagnosticInterrupt`, `StartNetworkInsightsAccessScopeAnalysis`, `StartNetworkInsightsAnalysis`
`RunScheduledInstances`, `SearchLocalGatewayRoutes`, `SearchTransitGatewayMulticastGroups`, `SearchTransitGatewayRoutes`, `SendDiagnosticInterrupt`, `StartNetworkInsightsAccessScopeAnalysis`, `StartNetworkInsightsAnalysis`, `StartVpcEndpointServicePrivateDnsVerification`
`StartVpcEndpointServicePrivateDnsVerification`, `TerminateClientVpnConnections`, `UnassignIpv6Addresses`, `UnassignPrivateIpAddresses`, `UnassignPrivateNatGatewayAddress`, `UnlockSnapshot`, `UnmonitorInstances`, `UpdateSecurityGroupRuleDescriptionsEgress`
`UpdateSecurityGroupRuleDescriptionsIngress`, `WithdrawByoipCidr`
`TerminateClientVpnConnections`, `UnassignIpv6Addresses`, `UnassignPrivateIpAddresses`, `UnassignPrivateNatGatewayAddress`, `UnlockSnapshot`, `UpdateSecurityGroupRuleDescriptionsEgress`, `UpdateSecurityGroupRuleDescriptionsIngress`, `WithdrawByoipCidr`
</details>
spinifex/ebsprovider/paging_test.go
+10 -9
View File
@@ -1,8 +1,9 @@
package ebsprovider
package ebsprovider_test
import (
"testing"
"github.com/mulgadc/spinifex/spinifex/ebsprovider"
"github.com/stretchr/testify/assert"
)
@@ -15,19 +16,19 @@ func TestPageSizeClamp(t *testing.T) {
maxResults int32
want int32
}{
{"above the cap is clamped", MaxListResults * 10, MaxListResults},
{"one above the cap is clamped", MaxListResults + 1, MaxListResults},
{"at the cap is honoured", MaxListResults, MaxListResults},
{"below the cap is honoured", MaxListResults - 1, MaxListResults - 1},
{"above the cap is clamped", ebsprovider.MaxListResults * 10, ebsprovider.MaxListResults},
{"one above the cap is clamped", ebsprovider.MaxListResults + 1, ebsprovider.MaxListResults},
{"at the cap is honoured", ebsprovider.MaxListResults, ebsprovider.MaxListResults},
{"below the cap is honoured", ebsprovider.MaxListResults - 1, ebsprovider.MaxListResults - 1},
{"one is honoured", 1, 1},
{"zero means no preference", 0, MaxListResults},
{"negative means no preference", -1, MaxListResults},
{"zero means no preference", 0, ebsprovider.MaxListResults},
{"negative means no preference", -1, ebsprovider.MaxListResults},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, ListVolumesRequest{MaxResults: tt.maxResults}.PageSize(),
assert.Equalf(t, tt.want, ebsprovider.ListVolumesRequest{MaxResults: tt.maxResults}.PageSize(),
"ListVolumes MaxResults=%d must page at %d; an unclamped page overflows one NATS message", tt.maxResults, tt.want)
assert.Equalf(t, tt.want, ListSnapshotsRequest{MaxResults: tt.maxResults}.PageSize(),
assert.Equalf(t, tt.want, ebsprovider.ListSnapshotsRequest{MaxResults: tt.maxResults}.PageSize(),
"ListSnapshots MaxResults=%d must page at %d; an unclamped page overflows one NATS message", tt.maxResults, tt.want)
})
}
spinifex/handlers/ec2/image/service_impl.go
+6 -1
View File
@@ -222,7 +222,12 @@ func (s *ImageServiceImpl) describeImages(ctx context.Context, input *ec2.Descri
State: aws.String(amiImageState(amiMeta.State)),
ImageType: aws.String("machine"),
Hypervisor: aws.String("xen"),
BootMode: aws.String(amiMeta.BootMode),
}
// 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 != "" {
image.BootMode = aws.String(amiMeta.BootMode)
}
if bdms := synthesizeRootBlockDeviceMapping(amiMeta, encryptedAtRest); bdms != nil {
spinifex/handlers/ec2/image/service_impl_test.go
+4 -3
View File
@@ -185,7 +185,8 @@ func TestDescribeImages_AfterCreate(t *testing.T) {
}
// TestDescribeImages_BootModeProjection asserts that AMIMetadata.BootMode round-trips
// through DescribeImages; empty BootMode on legacy AMIs must pass through unchanged.
// through DescribeImages. RegisterImage takes BootMode as an optional input, so an
// AMI recording none must omit the member: "" is not one of the enum's values.
func TestDescribeImages_BootModeProjection(t *testing.T) {
svc, store := setupTestImageService(t)
@@ -224,8 +225,8 @@ func TestDescribeImages_BootModeProjection(t *testing.T) {
require.Contains(t, byID, "ami-uefi001")
require.Contains(t, byID, "ami-legacy001")
assert.Equal(t, "uefi", aws.StringValue(byID["ami-uefi001"].BootMode))
assert.Empty(t, aws.StringValue(byID["ami-legacy001"].BootMode),
"legacy AMIs (empty BootMode) must pass through as empty, not be backfilled")
assert.Nil(t, byID["ami-legacy001"].BootMode,
`an AMI with no BootMode must omit the member, not be backfilled and not emit "": BootMode is an enum of [legacy-bios, uefi, uefi-preferred]`)
}
// TestDescribeImages_StateProjection asserts that AMIMetadata.State drives the
spinifex/handlers/iam/groups.go
+3 -1
View File
@@ -125,7 +125,9 @@ func (s *IAMServiceImpl) ListGroups(accountID string, input *iam.ListGroupsInput
}
keyPrefix := accountID + "."
var groups []*iam.Group
// Non-nil: Groups is a required member, and a nil slice marshals to no
// element at all rather than an empty one.
groups := []*iam.Group{}
for _, key := range keys {
if key == utils.VersionKey {
continue
spinifex/handlers/iam/groups_test.go
+23
View File
@@ -12,6 +12,14 @@ import (
"github.com/stretchr/testify/require"
)
// emptyRequiredListMsg explains why a required list member must be empty and
// non-nil: xmlutil drops a nil slice entirely, so the wire response loses a
// member the AWS model marks required and clients decode as absent.
func emptyRequiredListMsg(operation, member string) string {
return operation + " must return an empty non-nil " + member +
"; a nil slice marshals to no element at all, and " + member + " is a required member"
}
func createTestGroup(t *testing.T, svc *IAMServiceImpl, name string) *iam.Group {
t.Helper()
out, err := svc.CreateGroup(testAccountID, &iam.CreateGroupInput{
@@ -152,6 +160,21 @@ func TestListGroups_Empty(t *testing.T) {
out, err := svc.ListGroups(testAccountID, &iam.ListGroupsInput{})
require.NoError(t, err)
assert.Empty(t, out.Groups)
assert.NotNil(t, out.Groups, emptyRequiredListMsg("ListGroups", "Groups"))
}
// TestListGroups_EmptyAfterOtherAccountsExist walks the filtering path rather
// than the no-keys-at-all shortcut: the bucket has keys, none of them ours.
func TestListGroups_EmptyAfterOtherAccountsExist(t *testing.T) {
svc := setupTestIAMService(t)
_, err := svc.CreateGroup("999988887777", &iam.CreateGroupInput{GroupName: aws.String("othergroup")})
require.NoError(t, err)
out, err := svc.ListGroups(testAccountID, &iam.ListGroupsInput{})
require.NoError(t, err)
assert.Empty(t, out.Groups)
assert.NotNil(t, out.Groups, emptyRequiredListMsg("ListGroups", "Groups"))
}
func TestListGroups_PathPrefix(t *testing.T) {
spinifex/handlers/iam/service_impl.go
+3 -1
View File
@@ -579,7 +579,9 @@ func (s *IAMServiceImpl) ListAccessKeys(accountID string, input *iam.ListAccessK
return nil, err
}
var metadata []*iam.AccessKeyMetadata
// Non-nil: AccessKeyMetadata is a required member, and a nil slice marshals
// to no element at all rather than an empty one.
metadata := []*iam.AccessKeyMetadata{}
for _, keyID := range user.AccessKeys {
entry, err := s.accessKeysBucket.Get(ctx, keyID)
if err != nil {
spinifex/handlers/iam/service_impl_test.go
+1
View File
@@ -458,6 +458,7 @@ func TestDeleteAccessKey(t *testing.T) {
})
require.NoError(t, err)
assert.Empty(t, listOut.AccessKeyMetadata)
assert.NotNil(t, listOut.AccessKeyMetadata, emptyRequiredListMsg("ListAccessKeys", "AccessKeyMetadata"))
// User should now be deletable (no access keys)
_, err = svc.DeleteUser(testAccountID, &iam.DeleteUserInput{
tests/integration/conformance-known-findings.md
-17
View File
@@ -9,23 +9,6 @@ make test-integration
```
Use `AWS_MODEL_CONFORMANCE_MODE=warn` only for investigation. The report is written to `.cache/aws-model-conformance-report.txt` by default.
## Promoted services
- `sts` — promoted on 2026-08-05 after the integration baseline completed with no STS violations or decoder errors.
## Triaged warnings
Baseline from 2026-08-05: 370 successful responses checked, three violations, zero decoder errors.
| Service / operation | Finding | Classification | Implementation cause |
|---|---|---|---|
| EC2 `DescribeImages` | `BootMode` is emitted as `""`, outside the model enum | Defect | Legacy AMI metadata is intentionally projected through `aws.String(amiMeta.BootMode)`, which emits an empty element instead of omitting the optional member. |
| IAM `ListAccessKeys` | Required `AccessKeyMetadata` missing | Defect | The empty result starts as a nil slice, so Query XML serialization omits the required list container. |
| IAM `ListGroups` | Required `Groups` missing | Defect | When the KV bucket has keys but none match the account, the result slice remains nil and Query XML serialization omits the required list container. |
These warnings are not suppressed by a finding allowlist. They remain visible on every run. Handler changes belong in focused follow-up work; once a service's warnings are resolved, add that service to the promoted-services file in the same change that proves its full integration baseline is clean.
## Error-code coverage
Phase 4 validates IAM, STS, ECS, and ELBv2 error envelopes against their service models. The 2026-08-05 full integration baseline checked 46 operation-declared errors (45 IAM, one STS) with no error-code violations. ECS and ELBv2 decoding/model comparison have focused tests but their error paths are not exercised by the current integration suite, so neither service is ready for promotion on that evidence.