Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
28 changes: 24 additions & 4 deletions internal/api/handler_marketplace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -458,17 +467,28 @@ 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 {
return roleAssumptionClientError(opMsg, opErr)
}
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())
}
Expand Down
25 changes: 25 additions & 0 deletions internal/api/handler_marketplace_attempt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions internal/api/handler_marketplace_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
27 changes: 23 additions & 4 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Loading