fix(aws): refuse MemoryDB, OpenSearch and Redshift offerings priced above the approved upfront

CHANGELOG.md

@@ -217,6 +217,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 - **AWS/RDS:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency; a zero approved upfront is not checked (#53).
 - Bump Go to 1.26.10 to pick up the fix for GO-2026-6809 (crypto/x509), which `govulncheck` now reports against 1.26.9.
+- **AWS/MemoryDB, OpenSearch, Redshift:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency (MemoryDB offerings carry no currency, so only the recommendation's currency is checked there), or, for MemoryDB, an upfront offering with a zero price or one bought in a China region (cn-north-1, cn-northwest-1), where the assumed USD price is wrong; a zero approved upfront is not checked. All AWS RI price guards (EC2, RDS, MemoryDB, OpenSearch, Redshift) also refuse a NaN or infinite approved upfront (#53).
 - AWS ElastiCache purchases refuse a recommendation with no engine instead of
   sending an empty `ProductDescription` filter, send the engine lowercased as
   ElastiCache reports it, and verify the chosen offering's node type, duration

providers/aws/internal/offeringprice/offeringprice.go

@@ -149,10 +149,22 @@ func UpfrontCeiling(approved float64) float64 {
 	return approved * (1 + UpfrontTolerance)
 }
 
+// NoUpfrontApproved reports whether approved gives no ceiling to check: zero
+// or negative means a no-upfront option or a missing figure. NaN and +-Inf are
+// not "no figure" (a corrupt one), so they return false and CheckUpfront
+// refuses them.
+func NoUpfrontApproved(approved float64) bool {
+	return approved <= 0 && !math.IsInf(approved, -1)
+}
+
 // CheckUpfront fails when offeredTotal exceeds UpfrontCeiling(approved), both
-// compared in whole cents. approved must be positive: a zero upfront is either
-// a no-upfront option or a missing figure, and neither gives a ceiling.
+// compared in whole cents, and when approved is NaN or infinite (every
+// comparison against those is false, so they would pass). Callers skip the
+// call when NoUpfrontApproved(approved).
 func CheckUpfront(service string, offeredTotal, approved float64) error {
+	if math.IsNaN(approved) || math.IsInf(approved, 0) {
+		return fmt.Errorf("%s approved upfront %v is not a finite amount; refusing to purchase", service, approved)
+	}
 	if math.Round(offeredTotal*100) > math.Round(UpfrontCeiling(approved)*100) {
 		return fmt.Errorf("%s offering upfront %.2f exceeds the approved upfront %.2f by more than %.0f%%; refusing to purchase (re-approve at the current price)",
 			service, offeredTotal, approved, UpfrontTolerance*100)

providers/aws/internal/offeringprice/offeringprice_test.go

@@ -1,6 +1,7 @@
 package offeringprice
 
 import (
+	"math"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -65,3 +66,24 @@ func TestCheckUpfront_BoundaryIsTheCeilingInCents(t *testing.T) {
 	require.ErrorContains(t, CheckUpfront("EC2", 1050.01, 1000), "exceeds the approved upfront")
 	require.NoError(t, CheckUpfront("EC2", 0, 1000))
 }
+
+func TestCheckUpfront_RefusesNonFiniteApproved(t *testing.T) {
+	t.Parallel()
+	for _, svc := range []string{"EC2", "RDS", "MemoryDB", "OpenSearch", "Redshift"} {
+		for _, approved := range []float64{math.NaN(), math.Inf(1), math.Inf(-1)} {
+			err := CheckUpfront(svc, 100, approved)
+			require.ErrorContains(t, err, svc, "%v", approved)
+			assert.ErrorContains(t, err, "not a finite amount", "%v", approved)
+		}
+	}
+}
+
+func TestNoUpfrontApproved(t *testing.T) {
+	t.Parallel()
+	assert.True(t, NoUpfrontApproved(0))
+	assert.True(t, NoUpfrontApproved(-5))
+	assert.False(t, NoUpfrontApproved(1))
+	assert.False(t, NoUpfrontApproved(math.NaN()))
+	assert.False(t, NoUpfrontApproved(math.Inf(1)))
+	assert.False(t, NoUpfrontApproved(math.Inf(-1)))
+}

providers/aws/services/ec2/client.go

@@ -231,7 +231,7 @@ func checkApprovedUpfront(rec common.Recommendation, offering *types.ReservedIns
 	approved := rec.CommitmentCost
 	// Zero means no upfront or no figure (the MCP EC2 tool never sets it), so
 	// there is nothing to compare; refusing would break those purchases.
-	if approved <= 0 {
+	if offeringprice.NoUpfrontApproved(approved) {
 		return nil
 	}
 	id := aws.ToString(offering.ReservedInstancesOfferingId)

providers/aws/services/memorydb/client.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -166,6 +167,13 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, nil
 	}
 
+	// After the dedupe: a re-drive that already bought must not be refused for a
+	// later price rise (ec2/client.go does the same).
+	if err = c.checkApprovedUpfront(ctx, rec, offeringID); err != nil {
+		result.Error = err
+		return result, result.Error
+	}
+
 	input := &memorydb.PurchaseReservedNodesOfferingInput{
 		ReservedNodesOfferingId: aws.String(offeringID),
 		ReservationId:           aws.String(reservationID),
@@ -206,6 +214,60 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	return result, nil
 }
 
+// checkApprovedUpfront refuses an offering whose total upfront (FixedPrice, per
+// node, x count) exceeds the approved recommendation's upfront by more than
+// offeringprice.UpfrontTolerance, or whose price cannot be compared with it.
+// Mirrors rds.checkApprovedUpfront (same semantics; keep them in step). The
+// shared comparison is offeringprice.CheckUpfront. MemoryDB has no server-side
+// price cap, so this is the only guard. The offering is read back by ID, and
+// only when an upfront was approved: CommitmentCost 0 means no upfront or no
+// figure, so there is nothing to compare. The offering carries no currency
+// field, so only the recommendation's currency can be checked.
+func (c *Client) checkApprovedUpfront(ctx context.Context, rec common.Recommendation, offeringID string) error {
+	approved := rec.CommitmentCost
+	if offeringprice.NoUpfrontApproved(approved) {
+		return nil
+	}
+	if cur := rec.CommitmentCurrency; cur != "" && !strings.EqualFold(cur, "USD") {
+		return fmt.Errorf("MemoryDB recommendation upfront is in %s; the approved upfront check supports USD only", cur)
+	}
+	out, err := c.client.DescribeReservedNodesOfferings(ctx, &memorydb.DescribeReservedNodesOfferingsInput{
+		ReservedNodesOfferingId: aws.String(offeringID),
+	})
+	if err != nil {
+		return fmt.Errorf("failed to read MemoryDB offering %s for the approved upfront check: %w", offeringID, err)
+	}
+	if len(out.ReservedNodesOfferings) != 1 {
+		return fmt.Errorf("MemoryDB offering %s: expected 1 offering for the approved upfront check, got %d", offeringID, len(out.ReservedNodesOfferings))
+	}
+	off := out.ReservedNodesOfferings[0]
+	// FixedPrice is a plain float64, so 0 on an upfront offering means the price
+	// is missing (other services refuse a nil price); No Upfront is legitimately 0.
+	if off.FixedPrice == 0 && aws.ToString(off.OfferingType) != "No Upfront" {
+		return fmt.Errorf("MemoryDB offering %s: upfront offering has no FixedPrice; refusing to buy against an approved upfront of %.2f", offeringID, approved)
+	}
+	if err := c.checkAssumedUSD(off, offeringID); err != nil {
+		return err
+	}
+	if err := offeringprice.CheckUpfront("MemoryDB", off.FixedPrice*float64(rec.Count), approved); err != nil {
+		return fmt.Errorf("MemoryDB offering %s: %w", offeringID, err)
+	}
+	return nil
+}
+
+// checkAssumedUSD refuses a priced offering in a region not billed in USD: the
+// offering has no currency field, so the region decides it. No Upfront has no
+// price to compare.
+func (c *Client) checkAssumedUSD(off types.ReservedNodesOffering, offeringID string) error {
+	if aws.ToString(off.OfferingType) == "No Upfront" {
+		return nil
+	}
+	if _, err := offeringprice.AssumedUSD("MemoryDB", c.region); err != nil {
+		return fmt.Errorf("MemoryDB offering %s: %w", offeringID, err)
+	}
+	return nil
+}
+
 // findReservationByID looks for a nonterminal MemoryDB reserved
 // node with the given ReservationId (issue #641), so a re-driven purchase can
 // short-circuit instead of buying a second node. Retired/expired nodes are

providers/aws/services/memorydb/purchase_price_guard_test.go

@@ -0,0 +1,229 @@
+package memorydb
+
+import (
+	"context"
+	"errors"
+	"math"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/memorydb"
+	"github.com/aws/aws-sdk-go-v2/service/memorydb/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+func guardOffering(fixed float64) types.ReservedNodesOffering {
+	return types.ReservedNodesOffering{
+		ReservedNodesOfferingId: aws.String("offering-1"),
+		NodeType:                aws.String("db.r6gd.large"),
+		Duration:                31536000,
+		OfferingType:            aws.String("All Upfront"),
+		FixedPrice:              fixed,
+	}
+}
+
+// byIDMock serves the page lookup, then answers the by-ID price read with the
+// given offerings or error, and accepts the purchase.
+func byIDMock(byID []types.ReservedNodesOffering, byIDErr error) *MockMemoryDBClient {
+	m := &MockMemoryDBClient{}
+	isByID := func(in *memorydb.DescribeReservedNodesOfferingsInput) bool {
+		return aws.ToString(in.ReservedNodesOfferingId) == "offering-1"
+	}
+	m.On("DescribeReservedNodesOfferings", mock.Anything, mock.MatchedBy(isByID)).
+		Return(&memorydb.DescribeReservedNodesOfferingsOutput{ReservedNodesOfferings: byID}, byIDErr)
+	m.On("DescribeReservedNodesOfferings", mock.Anything, mock.MatchedBy(func(in *memorydb.DescribeReservedNodesOfferingsInput) bool { return !isByID(in) })).
+		Return(&memorydb.DescribeReservedNodesOfferingsOutput{ReservedNodesOfferings: []types.ReservedNodesOffering{guardOffering(0)}}, nil)
+	m.On("PurchaseReservedNodesOffering", mock.Anything, mock.Anything).
+		Return(&memorydb.PurchaseReservedNodesOfferingOutput{ReservedNode: &types.ReservedNode{ReservationId: aws.String("rn-1")}}, nil)
+	return m
+}
+
+func guardMock(fixed float64) *MockMemoryDBClient {
+	return byIDMock([]types.ReservedNodesOffering{guardOffering(fixed)}, nil)
+}
+
+// guardRec is a 2-node all-upfront rec approved at a $1000 total upfront.
+func guardRec() common.Recommendation {
+	r := mdbIdemRec()
+	r.Count = 2
+	r.CommitmentCost = 1000
+	return r
+}
+
+func buy(m *MockMemoryDBClient, rec common.Recommendation) (common.PurchaseResult, error) {
+	return (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+}
+
+func TestPurchaseCommitment_RefusesOfferingAboveApprovedUpfront(t *testing.T) {
+	t.Parallel()
+	m := guardMock(525.01) // 2 x 525.01 = 1050.02 > ceiling 1050.00
+	res, err := buy(m, guardRec())
+	require.ErrorContains(t, err, "exceeds the approved upfront")
+	assert.ErrorContains(t, err, "1050.02")
+	assert.ErrorContains(t, err, "1000.00")
+	assert.False(t, res.Success)
+	m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_BuysAtExactlyTolerance(t *testing.T) {
+	t.Parallel()
+	m := guardMock(525) // 2 x 525 = 1050 == ceiling
+	res, err := buy(m, guardRec())
+	require.NoError(t, err)
+	assert.True(t, res.Success)
+	m.AssertCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_ZeroApprovedUpfrontIsNotChecked(t *testing.T) {
+	t.Parallel()
+	m := guardMock(9000)
+	rec := guardRec()
+	rec.CommitmentCost = 0
+	_, err := buy(m, rec)
+	require.NoError(t, err)
+	m.AssertNumberOfCalls(t, "DescribeReservedNodesOfferings", 1)
+}
+
+func TestPurchaseCommitment_RefusesZeroPricedUpfrontOffering(t *testing.T) {
+	t.Parallel()
+	m := guardMock(0)
+	res, err := buy(m, guardRec())
+	require.ErrorContains(t, err, "offering-1")
+	assert.ErrorContains(t, err, "no FixedPrice")
+	assert.False(t, res.Success)
+	m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_RefusesNonUSDRec(t *testing.T) {
+	t.Parallel()
+	m := guardMock(400)
+	rec := guardRec()
+	rec.CommitmentCurrency = "eur"
+	_, err := buy(m, rec)
+	require.ErrorContains(t, err, "eur")
+	m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_EmptyAndLowercaseUSDBuy(t *testing.T) {
+	t.Parallel()
+	for _, cur := range []string{"", "usd"} {
+		rec := guardRec()
+		rec.CommitmentCurrency = cur
+		_, err := buy(guardMock(500), rec)
+		require.NoError(t, err, cur)
+	}
+}
+
+// No-upfront recs carry CommitmentCost 0, so the price is never read back.
+func TestPurchaseCommitment_NoUpfrontUnaffected(t *testing.T) {
+	t.Parallel()
+	m := &MockMemoryDBClient{}
+	noUpfront := guardOffering(0)
+	noUpfront.OfferingType = aws.String("No Upfront")
+	m.On("DescribeReservedNodesOfferings", mock.Anything, mock.Anything).
+		Return(&memorydb.DescribeReservedNodesOfferingsOutput{ReservedNodesOfferings: []types.ReservedNodesOffering{noUpfront}}, nil)
+	m.On("PurchaseReservedNodesOffering", mock.Anything, mock.Anything).
+		Return(&memorydb.PurchaseReservedNodesOfferingOutput{ReservedNode: &types.ReservedNode{ReservationId: aws.String("rn-1")}}, nil)
+	rec := mdbIdemRec()
+	rec.PaymentOption = "no-upfront"
+	_, err := buy(m, rec)
+	require.NoError(t, err)
+	m.AssertNumberOfCalls(t, "DescribeReservedNodesOfferings", 1)
+}
+
+// A re-drive whose node already exists must return it even after the offering
+// was repriced above the approved upfront: nothing is bought, so no refusal.
+func TestPurchaseCommitment_ExistingNodeWinsOverRepricedOffering(t *testing.T) {
+	t.Parallel()
+	m := guardMock(600) // 2 x 600 = 1200 against an approved 1000
+	token := common.DeriveIdempotencyToken("exec-53", 0)
+	derivedID := common.IdempotentReservationID("memorydb-id-", token)
+	m.On("DescribeReservedNodes", mock.Anything, mock.Anything).
+		Return(&memorydb.DescribeReservedNodesOutput{ReservedNodes: []types.ReservedNode{
+			{ReservationId: aws.String(derivedID), State: aws.String("active")},
+		}}, nil)
+	res, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), guardRec(), common.PurchaseOptions{IdempotencyToken: token})
+	require.NoError(t, err)
+	assert.True(t, res.Success)
+	assert.True(t, res.ExistingCommitment)
+	assert.Equal(t, derivedID, res.CommitmentID)
+	m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_RefusesWhenPriceReadFails(t *testing.T) {
+	t.Parallel()
+	m := byIDMock(nil, errors.New("throttled"))
+	res, err := buy(m, guardRec())
+	require.ErrorContains(t, err, "approved upfront check")
+	assert.ErrorContains(t, err, "throttled")
+	assert.False(t, res.Success)
+	m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_RefusesWhenPriceReadIsNotOneOffering(t *testing.T) {
+	t.Parallel()
+	two := []types.ReservedNodesOffering{guardOffering(400), guardOffering(400)}
+	for _, n := range [][]types.ReservedNodesOffering{two, nil} {
+		m := byIDMock(n, nil)
+		_, err := buy(m, guardRec())
+		require.ErrorContains(t, err, "expected 1 offering")
+		m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+	}
+}
+
+func TestPurchaseCommitment_RefusesNonFiniteApprovedUpfront(t *testing.T) {
+	t.Parallel()
+	for _, approved := range []float64{math.NaN(), math.Inf(1), math.Inf(-1)} {
+		m := guardMock(400)
+		rec := guardRec()
+		rec.CommitmentCost = approved
+		res, err := buy(m, rec)
+		require.ErrorContains(t, err, "not a finite amount", "%v", approved)
+		assert.False(t, res.Success)
+		m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+	}
+}
+
+func buyIn(region string, m *MockMemoryDBClient, rec common.Recommendation) (common.PurchaseResult, error) {
+	return (&Client{client: m, region: region}).PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+}
+
+func TestPurchaseCommitment_ChinaRegionRefusesApprovedUpfront(t *testing.T) {
+	t.Parallel()
+	for _, region := range []string{"cn-north-1", "cn-northwest-1"} {
+		m := guardMock(400)
+		res, err := buyIn(region, m, guardRec())
+		require.ErrorContains(t, err, "not billed in USD", region)
+		assert.False(t, res.Success)
+		m.AssertNotCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+	}
+}
+
+func TestPurchaseCommitment_ChinaRegionZeroApprovedStillBuys(t *testing.T) {
+	t.Parallel()
+	m := guardMock(400)
+	rec := guardRec()
+	rec.CommitmentCost = 0
+	_, err := buyIn("cn-north-1", m, rec)
+	require.NoError(t, err)
+	m.AssertCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+}
+
+// A No Upfront offering is exempt from the zero-FixedPrice refusal and the
+// China-region refusal even when the rec carries a positive approved upfront.
+func TestPurchaseCommitment_NoUpfrontOfferingWithApprovedUpfrontBuys(t *testing.T) {
+	t.Parallel()
+	for _, region := range []string{"us-east-1", "cn-north-1"} {
+		off := guardOffering(0)
+		off.OfferingType = aws.String("No Upfront")
+		m := byIDMock([]types.ReservedNodesOffering{off}, nil)
+		res, err := buyIn(region, m, guardRec())
+		require.NoError(t, err, region)
+		assert.True(t, res.Success)
+		m.AssertCalled(t, "PurchaseReservedNodesOffering", mock.Anything, mock.Anything)
+	}
+}