fix(aws): floor Savings Plan commitments to API precision

providers/aws/internal/spcommitment/commitment.go

@@ -0,0 +1,48 @@
+package spcommitment
+
+import (
+	"fmt"
+	"math"
+	"strconv"
+	"strings"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+const (
+	minimumHourlyCommitment = 0.001
+	maximumHourlyCommitment = 1000000.0
+	maximumDecimalPlaces    = 5
+)
+
+// Normalize copies details so provider normalization cannot change caller authorization data.
+func Normalize(rec *common.Recommendation) (string, error) {
+	details, ok := rec.Details.(*common.SavingsPlanDetails)
+	if !ok || details == nil {
+		return "", fmt.Errorf("invalid service details for Savings Plans: recommendation Details must be *common.SavingsPlanDetails, got %T", rec.Details)
+	}
+	amount := details.HourlyCommitment
+	if math.IsNaN(amount) || math.IsInf(amount, 0) || amount <= 0 {
+		return "", fmt.Errorf("SavingsPlanDetails.HourlyCommitment must be a positive finite value, got %g", amount)
+	}
+	if amount < minimumHourlyCommitment || amount > maximumHourlyCommitment {
+		return "", fmt.Errorf("SavingsPlanDetails.HourlyCommitment must be between %g and %g, got %g", minimumHourlyCommitment, maximumHourlyCommitment, amount)
+	}
+	// Truncate decimal intent, rather than multiplying a binary float that can cross a boundary.
+	wire := strconv.FormatFloat(amount, 'f', -1, 64)
+	whole, fraction, _ := strings.Cut(wire, ".")
+	if len(fraction) > maximumDecimalPlaces {
+		fraction = fraction[:maximumDecimalPlaces]
+	}
+	fraction = strings.TrimRight(fraction, "0")
+	fraction += strings.Repeat("0", max(0, 2-len(fraction)))
+	wire = whole + "." + fraction
+	normalized, err := strconv.ParseFloat(wire, 64)
+	if err != nil {
+		return "", fmt.Errorf("parse normalized Savings Plans commitment: %w", err)
+	}
+	copyDetails := *details
+	copyDetails.HourlyCommitment = normalized
+	rec.Details = &copyDetails
+	return wire, nil
+}

providers/aws/ladder/precision_test.go

@@ -0,0 +1,67 @@
+package ladder
+
+import (
+	"context"
+	"math"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/ladder"
+)
+
+func TestPurchaseLayer_CommitmentPrecision(t *testing.T) {
+	t.Parallel()
+	for _, layer := range []ladder.LayerType{ladder.LayerComputeSP, ladder.LayerEC2InstanceSP} {
+		for _, tc := range []struct{ amount, want float64 }{{.001, .001}, {.00499, .00499}, {.005, .005}, {1.234567, 1.23456}, {.29, .29}, {1000000, 1000000}, {math.Nextafter(1000000, 0), 999999.99999}} {
+			t.Run(string(layer)+strconv.FormatFloat(tc.amount, 'g', -1, 64), func(t *testing.T) {
+				planType := spPlanTypeCompute
+				if layer == ladder.LayerEC2InstanceSP {
+					planType = spPlanTypeEC2Instance
+				}
+				p := &fakePurchaser{}
+				a := newWiredLadder(t, &fakePurchaser{}, p, &fakeExchangeRunner{})
+				rec := validSPRec(planType)
+				original := rec.Details.(*common.SavingsPlanDetails)
+				original.HourlyCommitment = tc.amount
+				_, err := a.PurchaseLayer(context.Background(), layer, rec, validPurchaseOpts())
+				require.NoError(t, err)
+				require.Equal(t, 1, p.calls)
+				details := p.gotRec.Details.(*common.SavingsPlanDetails)
+				assert.Equal(t, tc.want, details.HourlyCommitment)
+				assert.NotSame(t, original, details)
+				assert.Equal(t, tc.amount, original.HourlyCommitment)
+				assert.Equal(t, "ladder-tok-1", p.gotOpts.IdempotencyToken)
+			})
+		}
+	}
+}
+
+func TestPurchaseLayer_CommitmentPrecisionRefusesInvalid(t *testing.T) {
+	t.Parallel()
+	for _, amount := range []float64{.00099, math.Nextafter(.001, 0), math.Nextafter(1000000, math.Inf(1)), 1000001, math.Inf(-1)} {
+		t.Run(strconv.FormatFloat(amount, 'g', -1, 64), func(t *testing.T) {
+			p := &fakePurchaser{}
+			a := newWiredLadder(t, &fakePurchaser{}, p, &fakeExchangeRunner{})
+			rec := validSPRec(spPlanTypeCompute)
+			rec.Details.(*common.SavingsPlanDetails).HourlyCommitment = amount
+			_, err := a.PurchaseLayer(context.Background(), ladder.LayerComputeSP, rec, validPurchaseOpts())
+			require.Error(t, err)
+			assert.Zero(t, p.calls)
+		})
+	}
+	for _, details := range []common.ServiceDetails{nil, (*common.SavingsPlanDetails)(nil)} {
+		p := &fakePurchaser{}
+		a := newWiredLadder(t, &fakePurchaser{}, p, &fakeExchangeRunner{})
+		rec := validSPRec(spPlanTypeCompute)
+		rec.Details = details
+		require.NotPanics(t, func() {
+			_, err := a.PurchaseLayer(context.Background(), ladder.LayerComputeSP, rec, validPurchaseOpts())
+			require.Error(t, err)
+		})
+		assert.Zero(t, p.calls)
+	}
+}

providers/aws/ladder/purchase.go

@@ -3,10 +3,10 @@ package ladder
 import (
 	"context"
 	"fmt"
-	"math"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/ladder"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/spcommitment"
 )
 
 // PurchaseLayer buys a commitment for the given layer by dispatching to the
@@ -136,8 +136,8 @@ func validateSPPurchaseRec(rec *common.Recommendation, wantPlanType string) erro
 	if details.PlanType != wantPlanType {
 		return fmt.Errorf("recommendation plan type %q does not match the dispatched layer's plan type %q", details.PlanType, wantPlanType)
 	}
-	if math.IsNaN(details.HourlyCommitment) || math.IsInf(details.HourlyCommitment, 0) || details.HourlyCommitment <= 0 {
-		return fmt.Errorf("SavingsPlanDetails.HourlyCommitment must be a positive finite value, got %g", details.HourlyCommitment)
+	if _, err := spcommitment.Normalize(rec); err != nil {
+		return err
 	}
 	return validateTermAndPayment(rec)
 }

providers/aws/services/savingsplans/client.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/purchasecfg"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/internal/spcommitment"
 )
 
 // API defines the interface for Savings Plans operations (enables mocking).
@@ -218,11 +219,12 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		Timestamp:      time.Now(),
 	}
 
-	spDetails, ok := rec.Details.(*common.SavingsPlanDetails)
-	if !ok {
-		result.Error = fmt.Errorf("invalid service details for Savings Plans")
+	commitment, err := spcommitment.Normalize(&rec)
+	if err != nil {
+		result.Error = err
 		return result, result.Error
 	}
+	result.Recommendation = rec
 
 	offeringID, err := c.findOfferingID(ctx, rec, opts.ExecutionID)
 	if err != nil {
@@ -232,7 +234,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 
 	input := &savingsplans.CreateSavingsPlanInput{
 		SavingsPlanOfferingId: aws.String(offeringID),
-		Commitment:            aws.String(fmt.Sprintf("%.2f", spDetails.HourlyCommitment)),
+		Commitment:            aws.String(commitment),
 		UpfrontPaymentAmount:  nil, // AWS calculates this based on payment option
 		PurchaseTime:          aws.Time(time.Now()),
 		Tags:                  buildSavingsPlanTags(opts.Source),
@@ -452,7 +454,7 @@ func (c *Client) collectSPOfferings(ctx context.Context, input *savingsplans.Des
 // calling outside of a purchase flow (ValidateOffering, GetOfferingDetails).
 func (c *Client) findOfferingID(ctx context.Context, rec common.Recommendation, execID string) (string, error) {
 	spDetails, ok := rec.Details.(*common.SavingsPlanDetails)
-	if !ok {
+	if !ok || spDetails == nil {
 		return "", fmt.Errorf("invalid service details for Savings Plans")
 	}
 
@@ -647,6 +649,9 @@ func (c *Client) ValidateOffering(ctx context.Context, rec common.Recommendation
 
 // GetOfferingDetails retrieves offering details.
 func (c *Client) GetOfferingDetails(ctx context.Context, rec common.Recommendation) (*common.OfferingDetails, error) {
+	if _, err := spcommitment.Normalize(&rec); err != nil {
+		return nil, err
+	}
 	offeringID, err := c.findOfferingID(ctx, rec, "")
 	if err != nil {
 		return nil, err

providers/aws/services/savingsplans/precision_test.go

@@ -0,0 +1,110 @@
+package savingsplans
+
+import (
+	"context"
+	"math"
+	"strconv"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/savingsplans"
+	"github.com/aws/aws-sdk-go-v2/service/savingsplans/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 TestCommitmentPrecision_PurchaseAndPreview(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct {
+		amount float64
+		wire   string
+	}{
+		{.001, "0.001"}, {.00499, "0.00499"}, {.005, "0.005"},
+		{1.234567, "1.23456"}, {.29, "0.29"}, {10, "10.00"},
+		{1000000, "1000000.00"}, {math.Nextafter(.001, math.Inf(1)), "0.001"},
+		{math.Nextafter(1000000, 0), "999999.99999"},
+		{math.Nextafter(.29, 0), "0.28999"}, {math.Nextafter(.29, math.Inf(1)), "0.29"},
+	} {
+		t.Run(strconv.FormatFloat(tc.amount, 'g', -1, 64), func(t *testing.T) {
+			t.Parallel()
+			m := &MockSavingsPlansClient{}
+			c := NewClient(aws.Config{Region: "us-east-1"}, types.SavingsPlanTypeCompute)
+			c.SetSavingsPlansAPI(m)
+			rec := spRec()
+			original := rec.Details.(*common.SavingsPlanDetails)
+			original.HourlyCommitment = tc.amount
+			original.OfferingID = "offering"
+			want, err := strconv.ParseFloat(tc.wire, 64)
+			require.NoError(t, err)
+			m.On("DescribeSavingsPlansOfferingRates", mock.Anything, mock.Anything).
+				Return(&savingsplans.DescribeSavingsPlansOfferingRatesOutput{}, nil).Once()
+			preview, err := c.GetOfferingDetails(context.Background(), rec)
+			require.NoError(t, err)
+			assert.Equal(t, want, preview.EffectiveHourlyRate)
+			assert.Equal(t, want*8760, preview.TotalCost)
+			assert.Equal(t, (want*8760)/8760, preview.RecurringCost)
+			m.On("CreateSavingsPlan", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+				input := args.Get(1).(*savingsplans.CreateSavingsPlanInput)
+				assert.Equal(t, tc.wire, aws.ToString(input.Commitment))
+				assert.Equal(t, "stable-token", aws.ToString(input.ClientToken))
+			}).Return(&savingsplans.CreateSavingsPlanOutput{SavingsPlanId: aws.String("purchased")}, nil).Twice()
+			m.On("DescribeSavingsPlans", mock.Anything, mock.Anything).
+				Return(&savingsplans.DescribeSavingsPlansOutput{SavingsPlans: []types.SavingsPlan{{SavingsPlanId: aws.String("purchased"), UpfrontPaymentAmount: aws.String("123.45")}}}, nil).Twice()
+			for i := 0; i < 2; i++ {
+				result, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{IdempotencyToken: "stable-token"})
+				require.NoError(t, err)
+				require.True(t, result.Success)
+				normalized := result.Recommendation.Details.(*common.SavingsPlanDetails)
+				assert.Equal(t, want, normalized.HourlyCommitment)
+				assert.NotSame(t, original, normalized)
+				require.NotNil(t, result.Cost)
+				assert.Equal(t, 123.45, *result.Cost)
+				rec = result.Recommendation
+			}
+			assert.Equal(t, tc.amount, original.HourlyCommitment)
+			m.AssertExpectations(t)
+		})
+	}
+}
+
+func TestCommitmentPrecision_InvalidInputMakesNoSDKCalls(t *testing.T) {
+	t.Parallel()
+	amounts := []float64{0, -1, .00099, math.Nextafter(.001, 0), math.Nextafter(1000000, math.Inf(1)), 1000001, math.NaN(), math.Inf(1), math.Inf(-1)}
+	inputs := make([]common.ServiceDetails, 0, len(amounts)+2)
+	inputs = append(inputs, nil, (*common.SavingsPlanDetails)(nil))
+	for _, amount := range amounts {
+		inputs = append(inputs, &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: amount})
+	}
+	for i, details := range inputs {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			m := &MockSavingsPlansClient{}
+			c := NewClient(aws.Config{}, types.SavingsPlanTypeCompute)
+			c.SetSavingsPlansAPI(m)
+			rec := spRec()
+			rec.Details = details
+			require.NotPanics(t, func() {
+				result, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+				require.Error(t, err)
+				assert.False(t, result.Success)
+				_, err = c.GetOfferingDetails(context.Background(), rec)
+				require.Error(t, err)
+			})
+			assert.Empty(t, m.Calls)
+		})
+	}
+}
+
+func TestCommitmentPrecision_ValidateOfferingDoesNotRequireCommitment(t *testing.T) {
+	c := NewClient(aws.Config{}, types.SavingsPlanTypeCompute)
+	m := &MockSavingsPlansClient{}
+	c.SetSavingsPlansAPI(m)
+	rec := spRec()
+	rec.Details = (*common.SavingsPlanDetails)(nil)
+	require.NotPanics(t, func() { require.Error(t, c.ValidateOffering(context.Background(), rec)) })
+	rec.Details = &common.SavingsPlanDetails{PlanType: "Compute", OfferingID: "offering"}
+	require.NoError(t, c.ValidateOffering(context.Background(), rec))
+	assert.Empty(t, m.Calls)
+}