fix(aws/ladder): reject nil reshape payment caps instead of treating them as unlimited

CHANGELOG.md

@@ -214,6 +214,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- **`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
+  the auto-exchange path (#83).
 - **Azure savings plan recommendations use a 730 hour month** (was 730.5),
   matching `common.HoursPerMonth` and the 8760 hour year; monthly recurring
   cost, on-demand cost and savings shift by about 0.07% (#583).

providers/aws/ladder/reshape.go

@@ -10,19 +10,6 @@ import (
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/ladder"
 )
 
-// unlimitedCapUSD is the explicit "no cap" value passed to
-// exchange.RIExchangeConfig when a BufferReshapeConfig cap is nil.
-//
-// Why not something cleaner: RIExchangeConfig has no nil/absent
-// representation for its float64 caps, and 0 there is maximally RESTRICTIVE
-// (RunAutoExchange skips any exchange whose payment exceeds the cap, so a
-// zero cap blocks everything) — mapping nil to 0 would silently invert the
-// caller's intent. +Inf is not usable either: big.Rat.SetFloat64(+Inf)
-// returns nil and the comparison in RunAutoExchange would panic.
-// math.MaxFloat64 is finite (exactly representable in big.Rat) and exceeds
-// any real exchange payment, making it a faithful "no cap".
-const unlimitedCapUSD = math.MaxFloat64
-
 // ReshapeBuffer runs the automated RI exchange flow over the buffer layer
 // (convertible RIs), delegating to the injected exchangeRunner. AWSLadder
 // only maps the configuration and the outcome; the runner owns the exchange
@@ -49,11 +36,12 @@ const unlimitedCapUSD = math.MaxFloat64
 //
 // Config mapping (BufferReshapeConfig -> exchange.RIExchangeConfig):
 //
-//   - MaxPaymentPerExchangeUSD / MaxPaymentDailyUSD: nil means no cap and maps
-//     to unlimitedCapUSD (see that constant for why); non-nil values must be
-//     finite and > 0 — zero is rejected loudly because RunAutoExchange treats
-//     the cap as a skip threshold and a zero cap would silently block every
-//     exchange (almost certainly a config bug, not an intent).
+//   - MaxPaymentPerExchangeUSD / MaxPaymentDailyUSD: both are required. A nil
+//     cap is rejected loudly (Mode is always auto, so an omitted cap must
+//     never mean unlimited spend), and values must be finite and > 0 — zero
+//     is rejected because RunAutoExchange treats the cap as a skip threshold
+//     and a zero cap would silently block every exchange (almost certainly a
+//     config bug, not an intent).
 //   - UtilizationThresholdPct must be in (0, 100]; LookbackDays must be > 0.
 //   - Mode is always exchange.ExchangeModeAuto: exchanges execute immediately,
 //     subject to the per-exchange and daily caps.
@@ -109,11 +97,11 @@ func (a *AWSLadder) ReshapeBuffer(ctx context.Context, scope ladder.Scope, cfg l
 // exchange package's runtime configuration. See ReshapeBuffer's godoc for the
 // full mapping rationale.
 func buildRIExchangeConfig(cfg ladder.BufferReshapeConfig) (exchange.RIExchangeConfig, error) {
-	perExchangeCap, err := capOrUnlimited("MaxPaymentPerExchangeUSD", cfg.MaxPaymentPerExchangeUSD)
+	perExchangeCap, err := requireCap("MaxPaymentPerExchangeUSD", cfg.MaxPaymentPerExchangeUSD)
 	if err != nil {
 		return exchange.RIExchangeConfig{}, err
 	}
-	dailyCap, err := capOrUnlimited("MaxPaymentDailyUSD", cfg.MaxPaymentDailyUSD)
+	dailyCap, err := requireCap("MaxPaymentDailyUSD", cfg.MaxPaymentDailyUSD)
 	if err != nil {
 		return exchange.RIExchangeConfig{}, err
 	}
@@ -138,19 +126,20 @@ func buildRIExchangeConfig(cfg ladder.BufferReshapeConfig) (exchange.RIExchangeC
 	}, nil
 }
 
-// capOrUnlimited maps an optional money cap to the float64 the exchange
-// config requires: nil -> unlimitedCapUSD (no cap); non-nil values must be
-// finite and > 0 (see the unlimitedCapUSD comment for why zero is rejected).
-func capOrUnlimited(name string, capUSD *float64) (float64, error) {
+// requireCap validates a required money cap and returns it as the float64 the
+// exchange config requires. nil is rejected: the caller must set an explicit
+// cap, so an omitted field can never mean unlimited spend. Values must be
+// finite and > 0 (a zero cap would block every exchange).
+func requireCap(name string, capUSD *float64) (float64, error) {
 	if capUSD == nil {
-		return unlimitedCapUSD, nil
+		return 0, fmt.Errorf("%s is required: set an explicit cap (a nil cap is not treated as unlimited)", name)
 	}
 	v := *capUSD
 	if math.IsNaN(v) || math.IsInf(v, 0) {
 		return 0, fmt.Errorf("%s must be finite, got %g", name, v)
 	}
 	if v <= 0 {
-		return 0, fmt.Errorf("%s must be > 0 when set (a zero cap would block every exchange; use nil for no cap), got %g", name, v)
+		return 0, fmt.Errorf("%s must be > 0 (a zero cap would block every exchange), got %g", name, v)
 	}
 	return v, nil
 }

providers/aws/ladder/reshape_test.go

@@ -66,20 +66,32 @@ func TestReshapeBuffer_CapMapping_SetValuesPassedVerbatim(t *testing.T) {
 	assert.Equal(t, 30, ex.gotCfg.LookbackDays)
 }
 
-func TestReshapeBuffer_CapMapping_NilMeansUnlimited(t *testing.T) {
+func TestReshapeBuffer_CapMapping_NilRejected(t *testing.T) {
 	t.Parallel()
-	ex := &fakeExchangeRunner{}
-	a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex)
+	tests := []struct {
+		name    string
+		mutate  func(*ladder.BufferReshapeConfig)
+		wantErr string
+	}{
+		{"nil per-exchange cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentPerExchangeUSD = nil },
+			"MaxPaymentPerExchangeUSD is required"},
+		{"nil daily cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentDailyUSD = nil },
+			"MaxPaymentDailyUSD is required"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			ex := &fakeExchangeRunner{}
+			a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex)
 
-	cfg := validReshapeCfg()
-	cfg.MaxPaymentPerExchangeUSD = nil
-	cfg.MaxPaymentDailyUSD = nil
-	_, err := a.ReshapeBuffer(context.Background(), testScope(), cfg)
-	require.NoError(t, err)
-	assert.Equal(t, unlimitedCapUSD, ex.gotCfg.MaxPaymentPerExchangeUSD,
-		"nil per-exchange cap must map to the explicit unlimited constant, never to 0 (0 blocks every exchange)")
-	assert.Equal(t, unlimitedCapUSD, ex.gotCfg.MaxPaymentDailyUSD,
-		"nil daily cap must map to the explicit unlimited constant, never to 0")
+			cfg := validReshapeCfg()
+			tt.mutate(&cfg)
+			_, err := a.ReshapeBuffer(context.Background(), testScope(), cfg)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), tt.wantErr)
+			assert.Equal(t, 0, ex.calls, "a missing cap must never reach the exchange runner")
+		})
+	}
 }
 
 func TestReshapeBuffer_CapValidation_ZeroAndBadValuesRejected(t *testing.T) {