fix(aws): refuse purchase counts that don't fit int32 before any AWS call

CHANGELOG.md

@@ -286,6 +286,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
   actually bought. Before, 2.5 became 3 with the upfront for 2.5, and the
   approved-upfront check refused the purchase. The on-demand baseline and
   break-even months are unchanged (#635).
+- **AWS:** purchase counts outside [1, MaxInt32] are refused before any API call in EC2, ElastiCache, MemoryDB, OpenSearch, RDS and Redshift, with `ErrRefusedBeforePurchase` and `offeringprice.ErrPurchaseCountOutOfRange` (#683).
 - **`azure`:** an error from a reservation purchase retry (attempt 2 or later) now also wraps `common.ErrOutcomeUnknown`. That covers a currency refusal from the retry's calculatePrice, a failed idempotency re-check, a non-retryable purchase failure, a cancelled retry wait and exhausted attempts. A retry runs only after a purchase that failed retryably and may have committed, and the re-check can miss that order, so none of these errors may read as "nothing was bought". The cause stays in the chain, so `errors.Is(err, common.ErrCurrencyNotAccepted)` still matches. The `ErrCurrencyNotAccepted` doc now says it is pre-purchase only without `ErrOutcomeUnknown` (#671).
 - **Azure purchases check the recommendation currency.** Azure reservation purchases
   compare the calculatePrice billing currency with the recommendation currency

pkg/common/types.go

@@ -39,8 +39,9 @@ var ErrCurrencyNotAccepted = errors.New("currency not accepted")
 // contract until the platform persists a typed flag (platform #778).
 var ErrOutcomeUnknown = errors.New("purchase outcome unknown")
 
-// ErrRefusedBeforePurchase wraps any error from the approved-upfront check,
-// which runs before the purchase request is built or sent. Its presence is
+// ErrRefusedBeforePurchase wraps any error from the approved-upfront check or
+// the purchase-count bounds check, both of which run before the purchase
+// request is built or sent. Its presence is
 // sufficient to know that this call sent no purchase request, but its absence
 // proves nothing: other refusals do not carry it. It describes only the call
 // that returned it and says nothing about earlier calls with the same

providers/aws/internal/offeringprice/offeringprice.go

@@ -5,6 +5,7 @@
 package offeringprice
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"strings"
@@ -14,6 +15,19 @@ import (
 
 const hourlyFrequency = "Hourly"
 
+// ErrPurchaseCountOutOfRange marks a purchase count PurchaseCount refused.
+var ErrPurchaseCountOutOfRange = errors.New("purchase count out of range")
+
+// PurchaseCount returns n as the int32 the AWS purchase inputs take. It refuses
+// a count outside [1, math.MaxInt32] instead of letting int32(n) wrap into a
+// wrong, possibly positive, quantity. Call it before any AWS call.
+func PurchaseCount(n int) (int32, error) {
+	if n < 1 || n > math.MaxInt32 {
+		return 0, fmt.Errorf("%w: purchase count %d is out of valid range [1, %d]", ErrPurchaseCountOutOfRange, n, math.MaxInt32)
+	}
+	return int32(n), nil
+}
+
 // Charge is one recurring charge of an offering.
 type Charge struct {
 	Frequency string

providers/aws/internal/offeringprice/offeringprice_test.go

@@ -87,3 +87,16 @@ func TestNoUpfrontApproved(t *testing.T) {
 	assert.False(t, NoUpfrontApproved(math.Inf(1)))
 	assert.False(t, NoUpfrontApproved(math.Inf(-1)))
 }
+
+func TestPurchaseCount(t *testing.T) {
+	t.Parallel()
+	for _, n := range []int{1, 7, math.MaxInt32} {
+		got, err := PurchaseCount(n)
+		require.NoError(t, err)
+		assert.EqualValues(t, n, got)
+	}
+	for _, n := range []int{math.MaxInt32 + 1, 1 << 40, 0, -1} {
+		_, err := PurchaseCount(n)
+		require.ErrorIs(t, err, ErrPurchaseCountOutOfRange, "count %d", n)
+	}
+}

providers/aws/services/ec2/client.go

@@ -128,6 +128,17 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen
 
 // PurchaseCommitment purchases an EC2 Reserved Instance.
 func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) {
+	// Refuse before any lookup or idempotency short-circuit so a bad count
+	// never reaches AWS and a re-drive cannot mask it.
+	count, err := offeringprice.PurchaseCount(rec.Count)
+	if err != nil {
+		err = fmt.Errorf("%w: %w", common.ErrRefusedBeforePurchase, err)
+		return common.PurchaseResult{Recommendation: rec, Timestamp: time.Now(), Error: err}, err
+	}
+	return c.purchase(ctx, rec, count, opts)
+}
+
+func (c *Client) purchase(ctx context.Context, rec common.Recommendation, count int32, opts common.PurchaseOptions) (common.PurchaseResult, error) {
 	result := common.PurchaseResult{
 		Recommendation: rec,
 		DryRun:         false,
@@ -175,7 +186,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	// Create the purchase request
 	input := &ec2.PurchaseReservedInstancesOfferingInput{
 		ReservedInstancesOfferingId: offering.ReservedInstancesOfferingId,
-		InstanceCount:               aws.Int32(int32(rec.Count)), // #nosec G115 -- Count from CE recommendation; AWS RI purchase limits keep this far below math.MaxInt32
+		InstanceCount:               aws.Int32(count),
 	}
 
 	// Execute the purchase. A single attempt: the API has no ClientToken, so an

providers/aws/services/ec2/purchase_count_test.go

@@ -0,0 +1,62 @@
+package ec2
+
+import (
+	"context"
+	"fmt"
+	"math"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/offeringprice"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPurchaseCommitment_Count(t *testing.T) {
+	t.Parallel()
+	rec := func(n int) common.Recommendation {
+		return common.Recommendation{ResourceType: "t3.micro", Count: n, PaymentOption: "partial-upfront", Term: "3yr",
+			Details: &common.ComputeDetails{Platform: "Linux/UNIX", Tenancy: "default", Scope: "Region"}}
+	}
+
+	t.Run("max int32 is sent exactly", func(t *testing.T) {
+		t.Parallel()
+		m := &MockEC2Client{}
+		m.Test(t)
+		m.On("DescribeReservedInstancesOfferings", mock.Anything, mock.Anything).
+			Return(&ec2.DescribeReservedInstancesOfferingsOutput{ReservedInstancesOfferings: []types.ReservedInstancesOffering{{
+				ReservedInstancesOfferingId: aws.String("offering-123"), InstanceType: types.InstanceTypeT3Micro,
+				Duration: aws.Int64(94608000), OfferingType: types.OfferingTypeValuesPartialUpfront,
+				ProductDescription: types.RIProductDescriptionLinuxUnix, InstanceTenancy: types.TenancyDefault, OfferingClass: types.OfferingClassTypeConvertible,
+			}}}, nil)
+		m.On("PurchaseReservedInstancesOffering", mock.Anything, mock.MatchedBy(func(in *ec2.PurchaseReservedInstancesOfferingInput) bool {
+			return aws.ToInt32(in.InstanceCount) == math.MaxInt32
+		})).Return(&ec2.PurchaseReservedInstancesOfferingOutput{ReservedInstancesId: aws.String("ri-1")}, nil).Once()
+		m.On("CreateTags", mock.Anything, mock.Anything).Return(&ec2.CreateTagsOutput{}, nil)
+
+		result, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec(math.MaxInt32), common.PurchaseOptions{})
+		require.NoError(t, err)
+		assert.True(t, result.Success)
+		m.AssertNumberOfCalls(t, "PurchaseReservedInstancesOffering", 1)
+	})
+
+	for _, n := range []int{math.MaxInt32 + 1, 0, -1} {
+		t.Run(fmt.Sprintf("refused before any API call, count %d", n), func(t *testing.T) {
+			t.Parallel()
+			m := &MockEC2Client{}
+			m.Test(t)
+			opts := common.PurchaseOptions{IdempotencyToken: common.DeriveIdempotencyToken("exec-count", 0)}
+
+			result, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec(n), opts)
+			require.Error(t, err)
+			assert.False(t, result.Success)
+			assert.ErrorIs(t, err, offeringprice.ErrPurchaseCountOutOfRange)
+			assert.ErrorIs(t, err, common.ErrRefusedBeforePurchase)
+			assert.Empty(t, m.Calls, "a bad count must make no describe and no purchase call")
+		})
+	}
+}

providers/aws/services/elasticache/client.go

@@ -128,6 +128,17 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen
 
 // PurchaseCommitment purchases an ElastiCache Reserved Cache Node.
 func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) {
+	// Refuse before any lookup or idempotency short-circuit so a bad count
+	// never reaches AWS and a re-drive cannot mask it.
+	count, err := offeringprice.PurchaseCount(rec.Count)
+	if err != nil {
+		err = fmt.Errorf("%w: %w", common.ErrRefusedBeforePurchase, err)
+		return common.PurchaseResult{Recommendation: rec, Timestamp: time.Now(), Error: err}, err
+	}
+	return c.purchase(ctx, rec, count, opts)
+}
+
+func (c *Client) purchase(ctx context.Context, rec common.Recommendation, count int32, opts common.PurchaseOptions) (common.PurchaseResult, error) {
 	result := common.PurchaseResult{
 		Recommendation: rec,
 		DryRun:         false,
@@ -175,7 +186,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 	input := &elasticache.PurchaseReservedCacheNodesOfferingInput{
 		ReservedCacheNodesOfferingId: aws.String(offeringID),
-		CacheNodeCount:               aws.Int32(int32(rec.Count)), // #nosec G115 -- Count from CE recommendation; AWS RI purchase limits keep this far below math.MaxInt32
+		CacheNodeCount:               aws.Int32(count),
 		ReservedCacheNodeId:          aws.String(reservationID),
 		Tags:                         c.createPurchaseTags(rec, opts.Source),
 	}

providers/aws/services/elasticache/purchase_count_test.go

@@ -0,0 +1,68 @@
+package elasticache
+
+import (
+	"context"
+	"fmt"
+	"math"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/offeringprice"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/elasticache"
+	"github.com/aws/aws-sdk-go-v2/service/elasticache/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPurchaseCommitment_Count(t *testing.T) {
+	t.Parallel()
+	rec := func(n int) common.Recommendation {
+		return common.Recommendation{
+			Service: common.ServiceCache, ResourceType: "cache.m5.large", Count: n,
+			PaymentOption: "all-upfront", Term: "1yr",
+			Details: &common.CacheDetails{Engine: "redis", NodeType: "cache.m5.large"},
+		}
+	}
+
+	t.Run("max int32 is sent exactly", func(t *testing.T) {
+		t.Parallel()
+		m := &MockElastiCacheClient{}
+		m.Test(t)
+		m.On("DescribeReservedCacheNodesOfferings", mock.Anything, mock.Anything).
+			Return(&elasticache.DescribeReservedCacheNodesOfferingsOutput{
+				ReservedCacheNodesOfferings: []types.ReservedCacheNodesOffering{{
+					ReservedCacheNodesOfferingId: aws.String("offering-123"),
+					CacheNodeType:                aws.String("cache.m5.large"),
+					ProductDescription:           aws.String("redis"),
+					OfferingType:                 aws.String("All Upfront"),
+					Duration:                     aws.Int32(31536000),
+				}},
+			}, nil)
+		m.On("PurchaseReservedCacheNodesOffering", mock.Anything, mock.MatchedBy(func(in *elasticache.PurchaseReservedCacheNodesOfferingInput) bool {
+			return aws.ToInt32(in.CacheNodeCount) == math.MaxInt32
+		})).Return(&elasticache.PurchaseReservedCacheNodesOfferingOutput{ReservedCacheNode: &types.ReservedCacheNode{ReservedCacheNodeId: aws.String("rn-1")}}, nil).Once()
+
+		result, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec(math.MaxInt32), common.PurchaseOptions{})
+		require.NoError(t, err)
+		assert.True(t, result.Success)
+		m.AssertNumberOfCalls(t, "PurchaseReservedCacheNodesOffering", 1)
+	})
+
+	for _, n := range []int{math.MaxInt32 + 1, 0, -1} {
+		t.Run(fmt.Sprintf("refused before any API call, count %d", n), func(t *testing.T) {
+			t.Parallel()
+			m := &MockElastiCacheClient{}
+			m.Test(t)
+			opts := common.PurchaseOptions{IdempotencyToken: common.DeriveIdempotencyToken("exec-count", 0)}
+
+			result, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec(n), opts)
+			require.Error(t, err)
+			assert.False(t, result.Success)
+			assert.ErrorIs(t, err, offeringprice.ErrPurchaseCountOutOfRange)
+			assert.ErrorIs(t, err, common.ErrRefusedBeforePurchase)
+			assert.Empty(t, m.Calls, "a bad count must make no describe and no purchase call")
+		})
+	}
+}

providers/aws/services/memorydb/client.go

@@ -122,6 +122,17 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen
 
 // PurchaseCommitment purchases a MemoryDB Reserved Node.
 func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) {
+	// Refuse before any lookup or idempotency short-circuit so a bad count
+	// never reaches AWS and a re-drive cannot mask it.
+	count, err := offeringprice.PurchaseCount(rec.Count)
+	if err != nil {
+		err = fmt.Errorf("%w: %w", common.ErrRefusedBeforePurchase, err)
+		return common.PurchaseResult{Recommendation: rec, Timestamp: time.Now(), Error: err}, err
+	}
+	return c.purchase(ctx, rec, count, opts)
+}
+
+func (c *Client) purchase(ctx context.Context, rec common.Recommendation, count int32, opts common.PurchaseOptions) (common.PurchaseResult, error) {
 	result := common.PurchaseResult{
 		Recommendation: rec,
 		DryRun:         false,
@@ -177,7 +188,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	input := &memorydb.PurchaseReservedNodesOfferingInput{
 		ReservedNodesOfferingId: aws.String(offeringID),
 		ReservationId:           aws.String(reservationID),
-		NodeCount:               aws.Int32(int32(rec.Count)), // #nosec G115 -- Count from CE recommendation; AWS RI purchase limits keep this far below math.MaxInt32
+		NodeCount:               aws.Int32(count),
 		Tags:                    c.createPurchaseTags(rec, opts.Source),
 	}
 

providers/aws/services/memorydb/client_test.go

@@ -704,6 +704,7 @@ func TestClient_PurchaseCommitment_OfferingNotFound(t *testing.T) {
 	rec := common.Recommendation{
 		Service:       common.ServiceCache,
 		ResourceType:  "db.r6gd.xlarge",
+		Count:         1,
 		PaymentOption: "partial-upfront",
 		Term:          "1yr",
 		Details: &common.CacheDetails{

providers/aws/services/memorydb/purchase_count_test.go

@@ -0,0 +1,57 @@
+package memorydb
+
+import (
+	"context"
+	"fmt"
+	"math"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/offeringprice"
+	"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"
+)
+
+func TestPurchaseCommitment_Count(t *testing.T) {
+	t.Parallel()
+	rec := func(n int) common.Recommendation {
+		r := mdbIdemRec()
+		r.Count = n
+		return r
+	}
+
+	t.Run("max int32 is sent exactly", func(t *testing.T) {
+		t.Parallel()
+		m := &MockMemoryDBClient{}
+		m.Test(t)
+		expectMDBOffering(m)
+		m.On("PurchaseReservedNodesOffering", mock.Anything, mock.MatchedBy(func(in *memorydb.PurchaseReservedNodesOfferingInput) bool {
+			return aws.ToInt32(in.NodeCount) == math.MaxInt32
+		})).Return(&memorydb.PurchaseReservedNodesOfferingOutput{ReservedNode: &types.ReservedNode{ReservationId: aws.String("rn-1")}}, nil).Once()
+
+		result, err := (&Client{client: m, region: "eu-west-1"}).PurchaseCommitment(context.Background(), rec(math.MaxInt32), common.PurchaseOptions{})
+		require.NoError(t, err)
+		assert.True(t, result.Success)
+		m.AssertNumberOfCalls(t, "PurchaseReservedNodesOffering", 1)
+	})
+
+	for _, n := range []int{math.MaxInt32 + 1, 0, -1} {
+		t.Run(fmt.Sprintf("refused before any API call, count %d", n), func(t *testing.T) {
+			t.Parallel()
+			m := &MockMemoryDBClient{}
+			m.Test(t)
+			opts := common.PurchaseOptions{IdempotencyToken: common.DeriveIdempotencyToken("exec-count", 0)}
+
+			result, err := (&Client{client: m, region: "eu-west-1"}).PurchaseCommitment(context.Background(), rec(n), opts)
+			require.Error(t, err)
+			assert.False(t, result.Success)
+			assert.ErrorIs(t, err, offeringprice.ErrPurchaseCountOutOfRange)
+			assert.ErrorIs(t, err, common.ErrRefusedBeforePurchase)
+			assert.Empty(t, m.Calls, "a bad count must make no describe and no purchase call")
+		})
+	}
+}

providers/aws/services/opensearch/client.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"math"
 	"strings"
 	"sync"
 	"time"
@@ -163,9 +162,9 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	// a re-drive carrying an invalid count (negative / int32-overflow) against
 	// an already-existing reservation would short-circuit to Success and never
 	// surface the bad input. Fail loud up front instead.
-	instanceCount, countErr := safeInt32Count(rec.Count)
+	instanceCount, countErr := offeringprice.PurchaseCount(rec.Count)
 	if countErr != nil {
-		result.Error = countErr
+		result.Error = fmt.Errorf("%w: %w", common.ErrRefusedBeforePurchase, countErr)
 		return result, result.Error
 	}
 
@@ -661,13 +660,3 @@ func (c *Client) GetValidResourceTypes(ctx context.Context) ([]string, error) {
 		"i3.16xlarge.search",
 	}, nil
 }
-
-// safeInt32Count validates that n is a positive value that fits in int32 and
-// returns it as int32. This prevents a gosec G115 integer-overflow conversion
-// on the PurchaseReservedInstanceOffering InstanceCount field.
-func safeInt32Count(n int) (int32, error) {
-	if n < 1 || n > math.MaxInt32 {
-		return 0, fmt.Errorf("instance count %d is out of valid range [1, %d]", n, math.MaxInt32)
-	}
-	return int32(n), nil
-}