diff --git a/go.mod b/go.mod index 5fb3a2f6..abcf4fb6 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( go.opentelemetry.io/otel/metric v1.45.0 // indirect go.opentelemetry.io/otel/trace v1.45.0 // indirect golang.org/x/crypto v0.58.0 - golang.org/x/net v0.61.0 // indirect + golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/sys v0.49.0 // indirect diff --git a/go.sum b/go.sum index 06deac0b..83df52e6 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= -golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= -golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= +golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= diff --git a/internal/api/handler_marketplace.go b/internal/api/handler_marketplace.go index cded31ab..50f2f91a 100644 --- a/internal/api/handler_marketplace.go +++ b/internal/api/handler_marketplace.go @@ -423,7 +423,8 @@ const awsMarketplaceMinPriceFloorFraction = 0.05 // awsMarketplaceClientFaultCodes is the set of AWS error codes that represent // client-side faults for Marketplace listing operations. These map to 4xx // responses so the caller receives an actionable message. Server-side AWS -// errors remain 5xx. +// errors remain 5xx. SellerNotRegistered is a client fault too, but +// mapAWSMarketplaceError answers it before consulting this set. var awsMarketplaceClientFaultCodes = map[string]bool{ "InvalidReservedInstancesId": true, "InvalidReservedInstancesId.NotFound": true, @@ -432,13 +433,21 @@ var awsMarketplaceClientFaultCodes = map[string]bool{ "IncorrectState": true, "InvalidReservedInstancesListingId": true, "ReservedInstancesListingAlreadyExists": true, - "SellerNotRegistered": true, "AuthFailure": true, "UnauthorizedOperation": true, } const marketplaceRoleAssumptionMessage = "could not assume the role configured for this account" +// marketplaceSellerNotRegisteredMessage replaces AWS's SellerNotRegistered text +// (issue #27), which says only that the account is not a registered seller. +// Registration is a one-time step in the AWS console that this app cannot do: +// the account's root user registers it with a bank account that has a US +// address and completes a tax interview. +const marketplaceSellerNotRegisteredMessage = "this AWS account is not registered as a Reserved Instance Marketplace seller. " + + "Its root user must register it as a seller, with a bank account that has a US address, before it can list Reserved Instances: " + + "https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ri-market-general.html#ri-market-seller-profile" + // isRoleAssumptionFailure reports whether err came from the STS AssumeRole call // that the credential provider makes lazily inside an EC2 request. Its text // names the host account and role ARNs, so it must never reach the caller. @@ -458,10 +467,17 @@ func roleAssumptionOperationError(err error) *smithy.OperationError { return nil } +// marketplaceSellerNotRegisteredReason is the stable details.reason (flattened +// into the JSON body as "reason") that marks the SellerNotRegistered 400, so a +// client can branch on it instead of matching the message. +const marketplaceSellerNotRegisteredReason = "seller_not_registered" + // mapAWSMarketplaceError maps an AWS SDK error to an appropriate ClientError. // AWS client-fault errors (4xx-category codes) produce a 4xx response with the -// original AWS message so the caller gets actionable feedback. All other errors -// produce a 502 (AWS-side failure). STS AssumeRole failures are classified by +// original AWS message so the caller gets actionable feedback. SellerNotRegistered +// is the exception: its 400 carries marketplaceSellerNotRegisteredMessage and +// reason seller_not_registered instead. All other errors produce a 502 +// (AWS-side failure). STS AssumeRole failures are classified by // roleAssumptionClientError and never echo the AWS text. func mapAWSMarketplaceError(opMsg string, err error) error { if opErr := roleAssumptionOperationError(err); opErr != nil { @@ -469,6 +485,10 @@ func mapAWSMarketplaceError(opMsg string, err error) error { } var apiErr smithy.APIError if errors.As(err, &apiErr) { + if apiErr.ErrorCode() == "SellerNotRegistered" { + return NewClientErrorWithDetails(400, marketplaceSellerNotRegisteredMessage, + map[string]any{"reason": marketplaceSellerNotRegisteredReason}) + } if awsMarketplaceClientFaultCodes[apiErr.ErrorCode()] || apiErr.ErrorFault() == smithy.FaultClient { return NewClientError(400, apiErr.ErrorMessage()) } diff --git a/internal/api/handler_marketplace_attempt_test.go b/internal/api/handler_marketplace_attempt_test.go index 297fd31d..5f73f078 100644 --- a/internal/api/handler_marketplace_attempt_test.go +++ b/internal/api/handler_marketplace_attempt_test.go @@ -332,6 +332,31 @@ func TestMarketplaceListHTTP_ClientFaultDropsTheAttempt(t *testing.T) { assert.Empty(t, f.db.row.ListingState) } +// Issue #27: an unregistered seller gets the registration explanation and +// reason "seller_not_registered" in the JSON body. AWS created nothing, so the +// attempt is dropped like any other client fault. +func TestMarketplaceListHTTP_SellerNotRegisteredExplainsRegistration(t *testing.T) { + f := newMarketplaceFixture(t) + f.h.marketplaceEC2Factory = func(aws.Config) marketplaceEC2Client { + return &stubMarketplaceEC2{createFn: func(context.Context, ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) { + return ec2svc.MarketplaceListingResult{}, &marketplaceTestAPIError{code: "SellerNotRegistered", message: "raw aws text", fault: smithy.FaultClient} + }} + } + + resp, err := f.h.HandleRequest(context.Background(), marketplaceListHTTPRequest()) + + require.NoError(t, err) + assert.Equal(t, 400, resp.StatusCode) + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(resp.Body), &body)) + assert.Equal(t, map[string]any{ + "error": marketplaceSellerNotRegisteredMessage, + "reason": "seller_not_registered", + }, body) + assert.Empty(t, f.db.token) + assert.Empty(t, f.db.row.ListingState) +} + // The row was read before another request claimed, listed and canceled it, so // the stale copy knows nothing of the canceled listing. The token must not be // derived from that copy: with a persisted random token the stale request gets diff --git a/internal/api/handler_marketplace_credentials_test.go b/internal/api/handler_marketplace_credentials_test.go index 05f42929..62762431 100644 --- a/internal/api/handler_marketplace_credentials_test.go +++ b/internal/api/handler_marketplace_credentials_test.go @@ -521,3 +521,42 @@ func TestLoadMarketplaceAWSConfig_ResolverErrorNeverFallsBackToHost(t *testing.T }) } } + +// Issue #27: AWS's SellerNotRegistered text does not say what to do, so the +// caller gets a fixed explanation (register as a seller with a US bank account, +// linking the AWS guide) and a stable reason to branch on. The classification +// keys on the code alone, whatever fault AWS reports. +func TestMapAWSMarketplaceError_SellerNotRegistered(t *testing.T) { + for _, fault := range []smithy.ErrorFault{smithy.FaultClient, smithy.FaultUnknown} { + t.Run(fault.String(), func(t *testing.T) { + err := &smithy.OperationError{ + ServiceID: "EC2", + OperationName: "CreateReservedInstancesListing", + Err: &smithy.GenericAPIError{Code: "SellerNotRegistered", Message: "raw aws text", Fault: fault}, + } + + ce, ok := IsClientError(mapAWSMarketplaceError("create listing", err)) + + require.True(t, ok) + assert.Equal(t, 400, ce.code) + assert.Equal(t, marketplaceSellerNotRegisteredMessage, ce.message) + assert.NotContains(t, ce.message, "raw aws text") + assert.Contains(t, ce.message, "US") + assert.Contains(t, ce.message, "https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ri-market-general.html#ri-market-seller-profile") + assert.Equal(t, map[string]any{"reason": "seller_not_registered"}, ce.Details()) + }) + } +} + +// Other AWS client faults keep their own text and carry no reason, so a client +// branching on reason never mistakes them for the seller precondition. +func TestMapAWSMarketplaceError_OtherClientFaultsHaveNoReason(t *testing.T) { + err := &smithy.GenericAPIError{Code: "InvalidParameterValue", Message: "bad price", Fault: smithy.FaultClient} + + ce, ok := IsClientError(mapAWSMarketplaceError("create listing", err)) + + require.True(t, ok) + assert.Equal(t, 400, ce.code) + assert.Equal(t, "bad price", ce.message) + assert.Nil(t, ce.Details()) +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index a7782101..41d7d8b4 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -1667,11 +1667,15 @@ paths: floor, or no default price computable), the RI is not Standard, the purchase's cloud account is missing, not AWS or cannot be signed for, the account role could not be assumed, or AWS rejected - the request as a client error. + the request as a client error. When AWS answers + SellerNotRegistered, reason is seller_not_registered and error + explains that the account's root user must register it as an RI + Marketplace seller, with a bank account that has a US address, + linking the AWS guide; no other 400 carries a reason. content: application/json: schema: - $ref: '#/components/schemas/Error' + $ref: '#/components/schemas/MarketplaceClientError' '401': description: No valid bearer session. content: @@ -1766,11 +1770,13 @@ paths: description: > Invalid ID, the purchase's cloud account is missing, not AWS or cannot be signed for, the account role could not be assumed, or - AWS rejected the cancel as a client error. + AWS rejected the cancel as a client error. AWS errors are mapped + as for marketplace-list, so a SellerNotRegistered answer carries + reason seller_not_registered here too. content: application/json: schema: - $ref: '#/components/schemas/Error' + $ref: '#/components/schemas/MarketplaceClientError' '401': description: No valid bearer session. content: @@ -8482,6 +8488,19 @@ components: One-time USD sale price per instance when term_months remain. Must be positive. + MarketplaceClientError: + type: object + required: [error] + properties: + error: + type: string + reason: + type: string + enum: [seller_not_registered] + description: > + Present only when AWS reported that the account is not a + registered RI Marketplace seller (SellerNotRegistered). + MarketplaceListRequest: type: object properties: