fix(aws/rds): refuse an offering priced above the approved upfront

CHANGELOG.md

@@ -215,6 +215,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- **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 ElastiCache purchases refuse a recommendation with no engine instead of
   sending an empty `ProductDescription` filter, send the engine lowercased as

providers/aws/services/ec2/client.go

@@ -226,7 +226,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 // approved recommendation's upfront by more than offeringprice.UpfrontTolerance,
 // or whose price cannot be compared with it. The API's LimitPrice is not used:
 // AWS documents it for Marketplace offerings only, and findOffering excludes
-// them.
+// them. rds.Client.checkApprovedUpfront mirrors this (keep them in step).
 func checkApprovedUpfront(rec common.Recommendation, offering *types.ReservedInstancesOffering) error {
 	approved := rec.CommitmentCost
 	// Zero means no upfront or no figure (the MCP EC2 tool never sets it), so

providers/aws/services/rds/client.go

@@ -164,6 +164,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
+	}
+
 	// Create the purchase request
 	input := &rds.PurchaseReservedDBInstancesOfferingInput{
 		ReservedDBInstancesOfferingId: aws.String(offeringID),
@@ -204,6 +211,44 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	return result, nil
 }
 
+// checkApprovedUpfront refuses an offering whose total upfront (FixedPrice x
+// count) exceeds the approved recommendation's upfront by more than
+// offeringprice.UpfrontTolerance, or whose price cannot be compared with it.
+// Mirrors ec2.checkApprovedUpfront (same semantics; keep them in step). The
+// shared comparison is offeringprice.CheckUpfront. RDS 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
+// (the MCP never sets it), so there is nothing to compare.
+func (c *Client) checkApprovedUpfront(ctx context.Context, rec common.Recommendation, offeringID string) error {
+	approved := rec.CommitmentCost
+	if approved <= 0 {
+		return nil
+	}
+	if cur := rec.CommitmentCurrency; cur != "" && !strings.EqualFold(cur, "USD") {
+		return fmt.Errorf("RDS recommendation upfront is in %s; the approved upfront check supports USD only", cur)
+	}
+	out, err := c.client.DescribeReservedDBInstancesOfferings(ctx, &rds.DescribeReservedDBInstancesOfferingsInput{
+		ReservedDBInstancesOfferingId: aws.String(offeringID),
+	})
+	if err != nil {
+		return fmt.Errorf("failed to read RDS offering %s for the approved upfront check: %w", offeringID, err)
+	}
+	if len(out.ReservedDBInstancesOfferings) != 1 {
+		return fmt.Errorf("RDS offering %s: expected 1 offering for the approved upfront check, got %d", offeringID, len(out.ReservedDBInstancesOfferings))
+	}
+	offering := out.ReservedDBInstancesOfferings[0]
+	if offering.FixedPrice == nil {
+		return fmt.Errorf("RDS offering %s has no FixedPrice; cannot compare it with the approved upfront %.2f", offeringID, approved)
+	}
+	if cc := aws.ToString(offering.CurrencyCode); cc != "" && !strings.EqualFold(cc, "USD") {
+		return fmt.Errorf("RDS offering %s is priced in %s; the approved upfront check supports USD only", offeringID, cc)
+	}
+	if err := offeringprice.CheckUpfront("RDS", *offering.FixedPrice*float64(rec.Count), approved); err != nil {
+		return fmt.Errorf("RDS offering %s: %w", offeringID, err)
+	}
+	return nil
+}
+
 // deriveReservationID returns the ReservedDBInstanceId to use. When an
 // idempotency token is supplied (issue #641) the ID is derived deterministically
 // from it, so a re-drive sends the identical ID and RDS rejects the duplicate

providers/aws/services/rds/purchase_price_guard_test.go

@@ -0,0 +1,192 @@
+package rds
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/rds"
+	"github.com/aws/aws-sdk-go-v2/service/rds/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+// guardMock serves the exact-match page lookup, then the by-ID price read with
+// the given FixedPrice and currency, and accepts the purchase.
+func guardMock(fixed float64, currency string) *MockRDSClient {
+	m := &MockRDSClient{}
+	byID := func(in *rds.DescribeReservedDBInstancesOfferingsInput) bool {
+		return aws.ToString(in.ReservedDBInstancesOfferingId) == "offering-1"
+	}
+	priced := exactOffering("offering-1")
+	priced.FixedPrice = aws.Float64(fixed)
+	if currency != "" {
+		priced.CurrencyCode = aws.String(currency)
+	}
+	m.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.MatchedBy(byID)).
+		Return(&rds.DescribeReservedDBInstancesOfferingsOutput{ReservedDBInstancesOfferings: []types.ReservedDBInstancesOffering{priced}}, nil)
+	m.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.MatchedBy(func(in *rds.DescribeReservedDBInstancesOfferingsInput) bool { return !byID(in) })).
+		Return(&rds.DescribeReservedDBInstancesOfferingsOutput{ReservedDBInstancesOfferings: []types.ReservedDBInstancesOffering{exactOffering("offering-1")}}, nil)
+	m.On("PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything).
+		Return(&rds.PurchaseReservedDBInstancesOfferingOutput{ReservedDBInstance: &types.ReservedDBInstance{ReservedDBInstanceId: aws.String("ri-1")}}, nil)
+	return m
+}
+
+// guardRec is a 2-instance all-upfront rec approved at a $1000 total upfront.
+func guardRec() common.Recommendation {
+	r := idempotencyTestRec()
+	r.Count = 2
+	r.CommitmentCost = 1000
+	return r
+}
+
+func buy(m *MockRDSClient, 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, "USD") // 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, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_BuysAtExactlyTolerance(t *testing.T) {
+	t.Parallel()
+	m := guardMock(525, "USD") // 2 x 525 = 1050 == ceiling
+	res, err := buy(m, guardRec())
+	require.NoError(t, err)
+	assert.True(t, res.Success)
+	m.AssertCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+// The MCP never sets CommitmentCost; zero must not block the buy.
+func TestPurchaseCommitment_ZeroApprovedUpfrontIsNotChecked(t *testing.T) {
+	t.Parallel()
+	m := guardMock(9000, "USD")
+	rec := guardRec()
+	rec.CommitmentCost = 0
+	_, err := buy(m, rec)
+	require.NoError(t, err)
+}
+
+func TestPurchaseCommitment_RefusesNonUSD(t *testing.T) {
+	t.Parallel()
+	m := guardMock(400, "EUR")
+	_, err := buy(m, guardRec())
+	require.ErrorContains(t, err, "EUR")
+	m.AssertNotCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+
+	m = guardMock(400, "USD")
+	rec := guardRec()
+	rec.CommitmentCurrency = "eur"
+	_, err = buy(m, rec)
+	require.ErrorContains(t, err, "eur")
+	m.AssertNotCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_EmptyCurrencyIsUSD(t *testing.T) {
+	t.Parallel()
+	_, err := buy(guardMock(500, ""), guardRec())
+	require.NoError(t, err)
+}
+
+// No-upfront recs carry CommitmentCost 0, so the price is never read back.
+func TestPurchaseCommitment_NoUpfrontUnaffected(t *testing.T) {
+	t.Parallel()
+	m := &MockRDSClient{}
+	noUpfront := exactOffering("offering-1")
+	noUpfront.OfferingType = aws.String("No Upfront")
+	m.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.Anything).
+		Return(&rds.DescribeReservedDBInstancesOfferingsOutput{ReservedDBInstancesOfferings: []types.ReservedDBInstancesOffering{noUpfront}}, nil)
+	m.On("PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything).
+		Return(&rds.PurchaseReservedDBInstancesOfferingOutput{ReservedDBInstance: &types.ReservedDBInstance{ReservedDBInstanceId: aws.String("ri-1")}}, nil)
+	rec := idempotencyTestRec()
+	rec.PaymentOption = "no-upfront"
+	_, err := buy(m, rec)
+	require.NoError(t, err)
+	m.AssertNumberOfCalls(t, "DescribeReservedDBInstancesOfferings", 1)
+}
+
+// A re-drive whose RI already exists must return it even after the offering
+// was repriced above the approved upfront: nothing is bought, so no refusal.
+func TestPurchaseCommitment_ExistingRIWinsOverRepricedOffering(t *testing.T) {
+	t.Parallel()
+	m := guardMock(600, "USD") // 2 x 600 = 1200 against an approved 1000
+	token := common.DeriveIdempotencyToken("exec-53", 0)
+	derivedID := common.IdempotentReservationID("rds-id-", token)
+	m.On("DescribeReservedDBInstances", mock.Anything, mock.Anything).
+		Return(&rds.DescribeReservedDBInstancesOutput{ReservedDBInstances: []types.ReservedDBInstance{
+			{ReservedDBInstanceId: 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, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+// byIDMock serves the exact-match page lookup, then answers the by-ID price
+// read with the given offerings or error.
+func byIDMock(byID []types.ReservedDBInstancesOffering, byIDErr error) *MockRDSClient {
+	m := &MockRDSClient{}
+	isByID := func(in *rds.DescribeReservedDBInstancesOfferingsInput) bool {
+		return aws.ToString(in.ReservedDBInstancesOfferingId) == "offering-1"
+	}
+	m.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.MatchedBy(isByID)).
+		Return(&rds.DescribeReservedDBInstancesOfferingsOutput{ReservedDBInstancesOfferings: byID}, byIDErr)
+	m.On("DescribeReservedDBInstancesOfferings", mock.Anything, mock.MatchedBy(func(in *rds.DescribeReservedDBInstancesOfferingsInput) bool { return !isByID(in) })).
+		Return(&rds.DescribeReservedDBInstancesOfferingsOutput{ReservedDBInstancesOfferings: []types.ReservedDBInstancesOffering{exactOffering("offering-1")}}, nil)
+	m.On("PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything).
+		Return(&rds.PurchaseReservedDBInstancesOfferingOutput{ReservedDBInstance: &types.ReservedDBInstance{ReservedDBInstanceId: aws.String("ri-1")}}, nil)
+	return m
+}
+
+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, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_RefusesWhenPriceReadIsNotOneOffering(t *testing.T) {
+	t.Parallel()
+	two := []types.ReservedDBInstancesOffering{exactOffering("offering-1"), exactOffering("offering-1")}
+	for _, n := range [][]types.ReservedDBInstancesOffering{two, nil} {
+		m := byIDMock(n, nil)
+		_, err := buy(m, guardRec())
+		require.ErrorContains(t, err, "expected 1 offering")
+		m.AssertNotCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+	}
+}
+
+func TestPurchaseCommitment_RefusesOfferingWithoutFixedPrice(t *testing.T) {
+	t.Parallel()
+	m := byIDMock([]types.ReservedDBInstancesOffering{exactOffering("offering-1")}, nil) // FixedPrice nil
+	_, err := buy(m, guardRec())
+	require.ErrorContains(t, err, "no FixedPrice")
+	m.AssertNotCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}
+
+func TestPurchaseCommitment_LowercaseUSDBuys(t *testing.T) {
+	t.Parallel()
+	rec := guardRec()
+	rec.CommitmentCurrency = "usd"
+	m := guardMock(500, "usd")
+	res, err := buy(m, rec)
+	require.NoError(t, err)
+	assert.True(t, res.Success)
+	m.AssertCalled(t, "PurchaseReservedDBInstancesOffering", mock.Anything, mock.Anything)
+}