Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
sending an empty `ProductDescription` filter, send the engine lowercased as
ElastiCache reports it, and verify the chosen offering's node type, duration
and engine before buying it (#53).
- AWS EC2 RI purchases refuse an offering whose total upfront exceeds the
recommendation's approved upfront by more than 5% (USD only; a
recommendation without an upfront figure is not checked), so a repriced or
differently-priced offering can no longer bypass the approved cap (#53).
- **`ReshapeBuffer` rejects nil payment caps** instead of mapping them to
`math.MaxFloat64`: `MaxPaymentPerExchangeUSD` and `MaxPaymentDailyUSD`
are now required, so an omitted cap can no longer mean unlimited spend on
Expand Down
24 changes: 24 additions & 0 deletions providers/aws/internal/offeringprice/offeringprice.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,27 @@ func AssumedUSD(service, region string) (string, error) {
}
return "USD", nil
}

// UpfrontTolerance is the fraction by which a resolved offering's upfront may
// exceed the approved recommendation's upfront before a purchase is refused.
// It absorbs Cost Explorer rounding; anything beyond it means the offering was
// repriced or a differently-priced variant matched, so the approval no longer
// covers it.
const UpfrontTolerance = 0.05

// UpfrontCeiling is the most a purchase may charge upfront for an approved
// upfront amount.
func UpfrontCeiling(approved float64) float64 {
return approved * (1 + UpfrontTolerance)
}

// 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.
func CheckUpfront(service string, offeredTotal, approved float64) error {
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)
}
return nil
}
7 changes: 7 additions & 0 deletions providers/aws/internal/offeringprice/offeringprice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,10 @@ func TestCurrency(t *testing.T) {
_, err = AssumedUSD("S", "cn-north-1")
require.Error(t, err)
}

func TestCheckUpfront_BoundaryIsTheCeilingInCents(t *testing.T) {
t.Parallel()
require.NoError(t, CheckUpfront("EC2", 1050, 1000))
require.ErrorContains(t, CheckUpfront("EC2", 1050.01, 1000), "exceeds the approved upfront")
require.NoError(t, CheckUpfront("EC2", 0, 1000))
}
34 changes: 34 additions & 0 deletions providers/aws/services/ec2/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
return result, result.Error
}

if err = checkApprovedUpfront(rec, offering); err != nil {
result.Error = err
return result, result.Error
}

// Create the purchase request
input := &ec2.PurchaseReservedInstancesOfferingInput{
ReservedInstancesOfferingId: offering.ReservedInstancesOfferingId,
Expand Down Expand Up @@ -217,6 +222,35 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
return result, nil
}

// checkApprovedUpfront refuses an offering whose total upfront exceeds the
// 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.
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
// there is nothing to compare; refusing would break those purchases.
if approved <= 0 {
return nil
}
id := aws.ToString(offering.ReservedInstancesOfferingId)
if cur := rec.CommitmentCurrency; cur != "" && !strings.EqualFold(cur, "USD") {
Comment thread
cristim marked this conversation as resolved.
return fmt.Errorf("EC2 recommendation upfront is in %s; the approved upfront check supports USD only", cur)
}
if offering.FixedPrice == nil {
return fmt.Errorf("EC2 offering %s has no FixedPrice; cannot compare it with the approved upfront %.2f", id, approved)
}
if cc := offering.CurrencyCode; cc != "" && cc != types.CurrencyCodeValuesUsd {
return fmt.Errorf("EC2 offering %s is priced in %s; the approved upfront check supports USD only", id, cc)
}
total := fixedPriceCents(*offering.FixedPrice) * float64(rec.Count) / 100
if err := offeringprice.CheckUpfront("EC2", total, approved); err != nil {
return fmt.Errorf("EC2 offering %s: %w", id, err)
}
return nil
}

// findRIByIdempotencyToken looks for an active or payment-pending Reserved
// Instance tagged with the given idempotency token (issue #636). It returns the
// RI ID and true when exactly such an RI exists, so a re-driven purchase can
Expand Down
86 changes: 86 additions & 0 deletions providers/aws/services/ec2/purchase_price_guard_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package ec2

import (
"context"
"testing"

"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"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
)

// pricedRec is a 2-instance partial-upfront rec approved at a $1000 total upfront.
func pricedRec() common.Recommendation {
r := matchRec()
r.Count = 2
r.PaymentOption = "partial-upfront"
r.CommitmentCost = 1000
return r
}

func pricedOffering(fixed float32) types.ReservedInstancesOffering {
return fullOffering("priced", "", func(o *types.ReservedInstancesOffering) {
o.OfferingType = types.OfferingTypeValuesPartialUpfront
o.FixedPrice = aws.Float32(fixed)
o.CurrencyCode = types.CurrencyCodeValuesUsd
})
}

func TestPurchaseCommitment_RefusesOfferingAboveApprovedUpfront(t *testing.T) {
t.Parallel()
// 2 x 600 = 1200 against an approved 1000 (ceiling 1050).
m := offeringsMock(pricedOffering(600))
res, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), pricedRec(), common.PurchaseOptions{})
require.ErrorContains(t, err, "exceeds the approved upfront")
assert.False(t, res.Success)
m.AssertNotCalled(t, "PurchaseReservedInstancesOffering", mock.Anything, mock.Anything)
}

func TestPurchaseCommitment_BuysAtApprovedUpfront(t *testing.T) {
t.Parallel()
m := offeringsMock(pricedOffering(500)) // 2 x 500 == approved 1000
res, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), pricedRec(), common.PurchaseOptions{})
require.NoError(t, err)
assert.True(t, res.Success)
m.AssertCalled(t, "PurchaseReservedInstancesOffering", mock.Anything, mock.MatchedBy(func(in *ec2.PurchaseReservedInstancesOfferingInput) bool {
return in.LimitPrice == nil
}), mock.Anything)
}

// The MCP EC2 tool never sets CommitmentCost; zero must not block the buy.
func TestPurchaseCommitment_NoApprovedUpfrontIsNotChecked(t *testing.T) {
t.Parallel()
m := offeringsMock(pricedOffering(9000))
rec := pricedRec()
rec.CommitmentCost = 0
_, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
require.NoError(t, err)
}

func TestCheckApprovedUpfront_Currency(t *testing.T) {
t.Parallel()
off := pricedOffering(500)
for _, cur := range []string{"", "USD", "usd"} {
rec := pricedRec()
rec.CommitmentCurrency = cur
require.NoError(t, checkApprovedUpfront(rec, &off), "currency %q", cur)
}
rec := pricedRec()
rec.CommitmentCurrency = "EUR"
require.ErrorContains(t, checkApprovedUpfront(rec, &off), "EUR")
}

func TestCheckApprovedUpfront_RefusesMissingPriceAndNonUSDOffering(t *testing.T) {
t.Parallel()
noPrice := fullOffering("p", "")
require.ErrorContains(t, checkApprovedUpfront(pricedRec(), &noPrice), "no FixedPrice")

eur := pricedOffering(500)
eur.CurrencyCode = types.CurrencyCodeValues("EUR")
require.ErrorContains(t, checkApprovedUpfront(pricedRec(), &eur), "USD only")
}
Loading