fix(marketplace): resolve 3 blocking defects in #808 RI sell flow

Defect 1 -- migration number collision:
Renumber marketplace migration from 000068 (already deployed, skipped by
golang-migrate on prod) to 000084 (next free after main's 000083).
Verified: main tops at 000083; #1277 uses 000082; no open PR holds 000084.
All "000068" references in SQL comments and types.go updated to 000084.

Defect 2 -- offering_class never written:
CUDly's EC2 client hardwires OfferingClassTypeConvertible; savePurchaseHistory
never stamped the field, so every row landed with offering_class=NULL and the
Sell button never rendered.
Fix: (a) stamp offering_class='convertible' in savePurchaseHistory for all
AWS EC2 purchases so future CUDly-bought rows are correctly classified at write
time; (b) add FetchOfferingClass to the marketplaceEC2Client interface and
implement it via DescribeReservedInstances so the marketplace-list handler can
lazily populate offering_class for pre-migration rows and for
externally-created Standard RIs (purchased before CUDly existed); (c) add
StampOfferingClass to ConfigStore + PostgresStore + mock to persist the
fetched class so subsequent requests skip the extra AWS call; (d) a new
populateOfferingClass helper in the handler ties it together -- it runs when
offering_class is empty after validateMarketplaceListRequest and before the
definitive "standard only" gate.

How a 'standard' row comes to exist: an externally-created Standard RI will
get its offering_class populated from AWS DescribeReservedInstances on the
first POST .../marketplace-list call and the value persisted for future calls.
Rows purchased by CUDly are stamped 'convertible' at write time (CUDly only
ever buys Convertible EC2 RIs).

Defect 3 -- $0 price guardrail gap:
resolveMarketplacePriceSchedule accepted Price >= 0, allowing zero-dollar
listings. Fix: reject Price <= 0 with a clear error; add a named constant
awsMarketplaceMinPriceFloorFraction (5%) and a total-schedule floor check so
a schedule that sums to less than 5% of the prorated residual value is also
rejected with an explicit message. Extract floor check into
checkSuppliedScheduleFloor and the class-check predicate into
isKnownNonStandardOfferingClass to keep all touched functions below gocyclo 10.

Regression tests:
- TestMigration084_MarketplaceColumns: integration test proves the three new
  columns exist and round-trip after migrating a fresh DB through 000084.
- TestMarketplaceList_EmptyOfferingClassFetchedStandard: proves an
  externally-created Standard RI (offering_class="") is listable after the
  lazy-populate path fetches and stamps 'standard' from AWS.
- TestMarketplaceList_EmptyOfferingClassFetchedConvertible: proves the gate
  still rejects when AWS reports 'convertible' even if DB had no class.
- TestResolveMarketplacePriceSchedule_ZeroPriceRejected: Price=0 rejected.
- TestResolveMarketplacePriceSchedule_BelowFloorRejected: sub-floor rejected.

Author: cristim

internal/analytics/collector_test.go

@@ -434,6 +434,10 @@ func (m *mockConfigStore) ClaimMarketplaceListingSlot(_ context.Context, _ strin
 	return true, nil
 }
 
+func (m *mockConfigStore) StampOfferingClass(_ context.Context, _, _ string) error {
+	return nil
+}
+
 // strPtr is a test helper for *string fields.
 func strPtr(s string) *string { return &s }
 

internal/api/handler_marketplace.go

@@ -38,6 +38,11 @@ type marketplaceEC2Client interface {
 	CreateMarketplaceListing(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error)
 	DescribeMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error)
 	CancelMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error)
+	// FetchOfferingClass calls AWS DescribeReservedInstances to determine
+	// whether a given Reserved Instance is 'standard' or 'convertible'. Used
+	// when the purchase_history row has no offering_class stored (pre-migration
+	// 000084 rows and externally-created Standard RIs).
+	FetchOfferingClass(ctx context.Context, reservedInstancesID string) (string, error)
 }
 
 // buildMarketplaceEC2Client honors the injected factory for tests, falling
@@ -100,8 +105,10 @@ func (h *Handler) validateMarketplaceListRequest(ctx context.Context, req *event
 		return nil, body, NewClientError(404, "purchase not found")
 	}
 
-	// Only Standard RIs can be listed on the Marketplace.
-	if !strings.EqualFold(row.OfferingClass, "standard") {
+	// Reject explicitly non-standard classes before the RBAC call (fast path).
+	// Empty offering_class is allowed here: the listing handler populates it
+	// lazily from AWS before performing the definitive check.
+	if isKnownNonStandardOfferingClass(row.OfferingClass) {
 		return nil, body, NewClientError(400, "only Standard Reserved Instances can be listed on the AWS Marketplace; this purchase has offering_class="+row.OfferingClass)
 	}
 
@@ -158,6 +165,23 @@ func (h *Handler) marketplaceList(ctx context.Context, req *events.LambdaFunctio
 
 	ec2Client := h.buildMarketplaceEC2Client(cfg)
 
+	// Populate offering_class from AWS when the DB row has none. This covers
+	// two cases: (1) pre-migration 000084 rows purchased by CUDly before this
+	// column existed, and (2) externally-created Standard RIs whose
+	// offering_class was never stamped. After population the value is persisted
+	// so subsequent requests do not require an extra AWS API call.
+	offeringClass := row.OfferingClass
+	if offeringClass == "" {
+		offeringClass, err = h.populateOfferingClass(ctx, purchaseID, ec2Client)
+		if err != nil {
+			return nil, err
+		}
+	}
+	// Definitive offering_class gate: only Standard RIs can be listed.
+	if !strings.EqualFold(offeringClass, "standard") {
+		return nil, NewClientError(400, "only Standard Reserved Instances can be listed on the AWS Marketplace; this purchase has offering_class="+offeringClass)
+	}
+
 	awsSchedule := make([]ec2svc.MarketplacePriceTier, 0, len(schedule))
 	for _, t := range schedule {
 		awsSchedule = append(awsSchedule, ec2svc.MarketplacePriceTier{
@@ -244,6 +268,25 @@ func (h *Handler) reserveAndCreateListing(ctx context.Context, purchaseID string
 	return result, nil
 }
 
+// populateOfferingClass calls AWS DescribeReservedInstances to determine the
+// offering class for an RI whose purchase_history row lacks one, then persists
+// the result so subsequent requests are served from the DB. Returns the class
+// string on success; returns an error if AWS reports no such RI.
+//
+// The DB stamp is best-effort: a write failure is logged but does not block the
+// listing request because the in-memory class value is authoritative for this
+// request.
+func (h *Handler) populateOfferingClass(ctx context.Context, purchaseID string, ec2Client marketplaceEC2Client) (string, error) {
+	class, err := ec2Client.FetchOfferingClass(ctx, purchaseID)
+	if err != nil {
+		return "", fmt.Errorf("offering_class not set and could not fetch from AWS: %w", err)
+	}
+	if stampErr := h.config.StampOfferingClass(ctx, purchaseID, class); stampErr != nil {
+		logging.Errorf("marketplace: failed to persist offering_class %q for purchase %s: %v", class, purchaseID, stampErr)
+	}
+	return class, nil
+}
+
 // releaseMarketplaceClaim restores a purchase_history row's listing fields to
 // the state captured before ClaimMarketplaceListingSlot reserved the slot. It
 // runs on every failure path after a successful claim so a failed listing
@@ -419,6 +462,16 @@ const awsMarketplaceNetFactor = 1 - awsMarketplaceFeePercent/100.0
 // seller's remaining cost basis. Applied before AWS deducts its fee.
 const awsMarketplaceBuyerDiscountFactor = 0.95
 
+// awsMarketplaceMinPriceFloorFraction is the minimum ratio of prorated
+// residual value that a caller-supplied price schedule must total across its
+// tiers. At 5%, a schedule totalling less than 1/20 of what the default
+// schedule would offer is rejected with a 400 so a user cannot accidentally
+// (or maliciously) list an RI at $0 or a nominal amount. The default schedule
+// uses awsMarketplaceBuyerDiscountFactor (95%) of residual; this floor is
+// intentionally far below that to give sellers flexibility while preventing
+// zero-price listings.
+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
@@ -462,16 +515,56 @@ func mapAWSMarketplaceError(opMsg string, err error) error {
 // term field, which would overprice older RIs).
 // originalTerm is the full contract term in months, used to prorate the
 // upfront cost to its remaining value.
+// isKnownNonStandardOfferingClass reports true when offering_class is
+// explicitly set to a non-standard value (e.g. "convertible"). An empty string
+// returns false because the class is not yet known and will be resolved lazily.
+func isKnownNonStandardOfferingClass(class string) bool {
+	return class != "" && !strings.EqualFold(class, "standard")
+}
+
+// checkSuppliedScheduleFloor returns a non-nil error when the total value of
+// a caller-supplied price schedule falls below awsMarketplaceMinPriceFloorFraction
+// of the computed residual value. Extracted from resolveMarketplacePriceSchedule
+// to keep that function within the gocyclo budget. The check is skipped when
+// residual is zero (fully-elapsed term or $0 RI) to avoid spurious rejections.
+func checkSuppliedScheduleFloor(tiers []MarketplacePriceTier, remainingMonths, originalTerm int, upfrontCost, monthlyCost float64) error {
+	if remainingMonths <= 0 {
+		return nil
+	}
+	upfrontRemaining := 0.0
+	if originalTerm > 0 {
+		upfrontRemaining = upfrontCost * (float64(remainingMonths) / float64(originalTerm))
+	}
+	residual := upfrontRemaining + monthlyCost*float64(remainingMonths)
+	if residual <= 0 {
+		return nil
+	}
+	floor := residual * awsMarketplaceMinPriceFloorFraction
+	var total float64
+	for _, t := range tiers {
+		total += t.Price * float64(t.TermMonths)
+	}
+	if total < floor {
+		return fmt.Errorf(
+			"total listing price (%.2f) is below the minimum floor (%.2f, %.0f%% of residual value %.2f); raise your price schedule or omit it to use the default",
+			total, floor, awsMarketplaceMinPriceFloorFraction*100, residual)
+	}
+	return nil
+}
+
 func resolveMarketplacePriceSchedule(supplied []MarketplacePriceTier, remainingMonths, originalTerm int, upfrontCost, monthlyCost float64) ([]MarketplacePriceTier, error) {
 	if len(supplied) > 0 {
 		for i, t := range supplied {
 			if t.TermMonths <= 0 {
 				return nil, fmt.Errorf("price_schedule[%d]: term_months must be a positive integer", i)
 			}
-			if t.Price < 0 {
-				return nil, fmt.Errorf("price_schedule[%d]: price must be non-negative", i)
+			if t.Price <= 0 {
+				return nil, fmt.Errorf("price_schedule[%d]: price must be positive (received %.4f); use a non-zero listing price", i, t.Price)
 			}
 		}
+		if err := checkSuppliedScheduleFloor(supplied, remainingMonths, originalTerm, upfrontCost, monthlyCost); err != nil {
+			return nil, err
+		}
 		return supplied, nil
 	}
 

internal/api/handler_marketplace_test.go

@@ -25,8 +25,9 @@ const validMarketplacePurchaseID = "11111111-1111-1111-1111-111111111111"
 // Each field, when set, overrides the corresponding method's behavior and
 // records the last request so tests can assert on what was sent to AWS.
 type stubMarketplaceEC2 struct {
-	createFn func(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error)
-	cancelFn func(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error)
+	createFn     func(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error)
+	cancelFn     func(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error)
+	fetchClassFn func(ctx context.Context, reservedInstancesID string) (string, error)
 
 	lastCreateReq   ec2svc.MarketplaceListingRequest
 	createCallCount int
@@ -56,6 +57,14 @@ func (s *stubMarketplaceEC2) CancelMarketplaceListing(ctx context.Context, listi
 	return ec2svc.MarketplaceListingResult{ListingID: listingID, State: config.ListingStateCancelled}, nil
 }
 
+func (s *stubMarketplaceEC2) FetchOfferingClass(ctx context.Context, reservedInstancesID string) (string, error) {
+	if s.fetchClassFn != nil {
+		return s.fetchClassFn(ctx, reservedInstancesID)
+	}
+	// Default: return "standard" so tests that set offering_class="" still proceed.
+	return "standard", nil
+}
+
 // marketplaceTestAPIError is an smithy.APIError with a controllable fault so
 // the error-mapping tests can exercise both the client- and server-fault paths.
 type marketplaceTestAPIError struct {
@@ -450,3 +459,103 @@ func TestMarketplaceList_ClaimErrorMapsToInternal(t *testing.T) {
 	assert.Equal(t, 0, ec2.createCallCount)
 	cfgStore.AssertExpectations(t)
 }
+
+// --- Regression tests for the three blocking defects fixed in this PR ---
+
+// TestResolveMarketplacePriceSchedule_ZeroPriceRejected verifies Defect 3:
+// Price=0 must be rejected with a 400 so a user cannot list an RI for free.
+// This is the fail-before/pass-after regression test (the old code accepted
+// Price >= 0, so Price=0 was accepted; the fix requires Price > 0).
+func TestResolveMarketplacePriceSchedule_ZeroPriceRejected(t *testing.T) {
+	_, err := resolveMarketplacePriceSchedule([]MarketplacePriceTier{
+		{TermMonths: 6, Price: 0},
+	}, 6, 12, 1200, 0)
+	require.Error(t, err, "Price=0 must be rejected")
+	assert.Contains(t, err.Error(), "positive")
+}
+
+// TestResolveMarketplacePriceSchedule_BelowFloorRejected verifies Defect 3:
+// A schedule whose total value is below awsMarketplaceMinPriceFloorFraction of
+// the residual must be rejected. The fixture has residual=$1200 (upfront $1200,
+// no monthly, remaining=12 of 12 months) so the floor at 5% is $60; a price of
+// $0.01/month for 12 months totals $0.12 and must be rejected.
+func TestResolveMarketplacePriceSchedule_BelowFloorRejected(t *testing.T) {
+	_, err := resolveMarketplacePriceSchedule([]MarketplacePriceTier{
+		{TermMonths: 12, Price: 0.01},
+	}, 12, 12, 1200, 0)
+	require.Error(t, err, "sub-floor schedule must be rejected")
+	assert.Contains(t, err.Error(), "floor")
+	assert.Contains(t, err.Error(), "residual")
+}
+
+// TestMarketplaceList_EmptyOfferingClassFetchedStandard verifies Defect 2:
+// When purchase_history.offering_class is empty (pre-migration rows and
+// externally-created RIs), the handler fetches the class from AWS, persists
+// it, and proceeds when AWS reports "standard". This is the sell path becoming
+// reachable for externally-created Standard RIs.
+func TestMarketplaceList_EmptyOfferingClassFetchedStandard(t *testing.T) {
+	cfgStore := &MockConfigStore{}
+	authSvc := &MockAuthService{}
+	adminSession(authSvc)
+
+	// Row has no offering_class (simulates pre-migration or externally-created RI).
+	row := standardRow()
+	row.OfferingClass = ""
+	cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID).
+		Return(row, nil)
+	// The handler must stamp the fetched class back to the DB.
+	cfgStore.On("StampOfferingClass", mock.Anything, validMarketplacePurchaseID, "standard").
+		Return(nil)
+	cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID).
+		Return(true, nil)
+	cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "ril-default", config.ListingStateActive).
+		Return(nil)
+
+	// AWS reports this RI is "standard".
+	ec2 := &stubMarketplaceEC2{
+		fetchClassFn: func(_ context.Context, id string) (string, error) {
+			assert.Equal(t, validMarketplacePurchaseID, id)
+			return "standard", nil
+		},
+	}
+	h := newMarketplaceHandler(cfgStore, authSvc, ec2)
+	resp, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID)
+
+	require.NoError(t, err, "standard RI with empty offering_class must be listable after lazy-populate")
+	typed, ok := resp.(*MarketplaceListResponse)
+	require.True(t, ok)
+	assert.Equal(t, "ril-default", typed.ListingID)
+	cfgStore.AssertExpectations(t)
+}
+
+// TestMarketplaceList_EmptyOfferingClassFetchedConvertible verifies Defect 2
+// guard: when AWS reports the RI is "convertible", the handler must reject
+// with 400 even if offering_class was empty in the DB (not silently proceed).
+func TestMarketplaceList_EmptyOfferingClassFetchedConvertible(t *testing.T) {
+	cfgStore := &MockConfigStore{}
+	authSvc := &MockAuthService{}
+	adminSession(authSvc)
+
+	row := standardRow()
+	row.OfferingClass = ""
+	cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID).
+		Return(row, nil)
+	// The handler stamps the fetched class back even when it is convertible.
+	cfgStore.On("StampOfferingClass", mock.Anything, validMarketplacePurchaseID, "convertible").
+		Return(nil)
+
+	ec2 := &stubMarketplaceEC2{
+		fetchClassFn: func(_ context.Context, _ string) (string, error) {
+			return "convertible", nil
+		},
+	}
+	h := newMarketplaceHandler(cfgStore, authSvc, ec2)
+	_, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID)
+
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok)
+	assert.Equal(t, 400, ce.code)
+	assert.Contains(t, err.Error(), "Standard")
+	cfgStore.AssertExpectations(t)
+}

internal/config/interfaces.go

@@ -212,6 +212,14 @@ type StoreInterface interface {
 	// on subsequent poll/cancel transitions (issue #292).
 	UpdatePurchaseHistoryListing(ctx context.Context, purchaseID, listingID, listingState string) error
 
+	// StampOfferingClass writes the offering_class value to a purchase_history
+	// row identified by purchase_id. Called by the marketplace-list handler
+	// when offering_class is absent in the DB (pre-migration 000084 rows and
+	// externally-created Standard RIs): after fetching the class from AWS
+	// DescribeReservedInstances it is persisted so subsequent requests do not
+	// incur an extra AWS API call.
+	StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error
+
 	// ClaimMarketplaceListingSlot atomically reserves the marketplace-listing
 	// slot for a purchase_history row so two concurrent marketplace-list
 	// requests cannot both proceed to create a duplicate AWS listing (issue

internal/config/store_postgres.go

@@ -1682,6 +1682,20 @@ func (s *PostgresStore) UpdatePurchaseHistoryListing(ctx context.Context, purcha
 	return nil
 }
 
+// StampOfferingClass writes the offering_class value onto a purchase_history
+// row identified by purchase_id. Used by the marketplace-list handler to
+// lazily persist offering_class fetched from AWS DescribeReservedInstances for
+// rows created before migration 000084 or for externally-created Standard RIs.
+// A no-match (row not found) is treated as a non-fatal warning by callers.
+func (s *PostgresStore) StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error {
+	query := `UPDATE purchase_history SET offering_class = $1 WHERE purchase_id = $2`
+	_, err := s.db.Exec(ctx, query, offeringClass, purchaseID)
+	if err != nil {
+		return fmt.Errorf("failed to stamp offering_class for purchase %s: %w", purchaseID, err)
+	}
+	return nil
+}
+
 // ClaimMarketplaceListingSlot atomically reserves the marketplace-listing slot
 // for a purchase_history row so two concurrent marketplace-list requests cannot
 // both proceed to create a duplicate AWS listing (issue #292). The single

internal/config/types.go

@@ -791,15 +791,15 @@ type PurchaseHistoryRecord struct {
 	// OfferingClass records whether this commitment is a 'standard' or
 	// 'convertible' RI. NULL on pre-migration rows. The Sell-on-Marketplace
 	// button renders only when this equals "standard" (issue #292).
-	// Persisted in purchase_history via migration 000068.
+	// Persisted in purchase_history via migration 000084.
 	OfferingClass string `json:"offering_class,omitempty" dynamodbav:"offering_class,omitempty"`
 	// ListingID is the AWS ReservedInstancesListingId returned by
 	// CreateReservedInstancesListing. Empty when the RI has not been
-	// listed. Persisted in purchase_history via migration 000068.
+	// listed. Persisted in purchase_history via migration 000084.
 	ListingID string `json:"listing_id,omitempty" dynamodbav:"listing_id,omitempty"`
 	// ListingState mirrors the AWS marketplace listing state (see the
 	// ListingState* constants). Empty when not listed. Persisted in
-	// purchase_history via migration 000068.
+	// purchase_history via migration 000084.
 	ListingState string `json:"listing_state,omitempty" dynamodbav:"listing_state,omitempty"`
 }
 

…ase_history_marketplace_listing.down.sql …ase_history_marketplace_listing.down.sqlinternal/database/postgres/migrations/000068_purchase_history_marketplace_listing.down.sql renamed to internal/database/postgres/migrations/000084_purchase_history_marketplace_listing.down.sql internal/database/postgres/migrations/000068_purchase_history_marketplace_listing.down.sql renamed to internal/database/postgres/migrations/000084_purchase_history_marketplace_listing.down.sql

@@ -1,4 +1,4 @@
--- Migration 000068: add RI Marketplace listing columns to purchase_history.
+-- Migration 000084: add RI Marketplace listing columns to purchase_history.
 --
 -- offering_class: 'convertible' or 'standard'; NULL for pre-migration rows.
 -- The Sell button renders only when offering_class = 'standard' and the RI is