From 410faceb2912d47a4df3ab5af3e799c103dc6a20 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 20:51:18 +0100 Subject: [PATCH 1/2] chore(deps): pin cloud-commitments-go at c83aef8 and require a cap for Compute SP ladders Moves all four cloud-commitments-go modules from 790f369 to c83aef8, which brings in #690 (PurchaseReconciler), #696 (purchase attempt journal contract), #698 (Compute SP Cost Explorer commitment rate), #689 (Commitment.CostKnown) and #701 (Redshift attempt journal). The platform needs no code change for #690, #696, #689 or #701: the new interface is optional, the new options field is nil by default and the journal runs only when one is set. With #698 the ladder sizes the Compute Savings Plan layer from Cost Explorer's account-wide blended rate. That bias is accepted only because max_hourly_commit_per_run bounds it, so executeLadderRun now refuses, before any provider read, a run for a config without that cap whose layers include Compute SP. The run is counted as errored and the error names the field. Saving such a config is still accepted; #1815 tracks the save-time check and the form field. Co-Authored-By: Claude Opus 5.5 --- go.mod | 8 +- go.sum | 16 +-- internal/config/validation.go | 2 +- internal/server/handler_ladder.go | 7 +- internal/server/handler_ladder_rates_test.go | 2 + internal/server/ladder_compute_cap.go | 23 ++++ internal/server/ladder_compute_cap_test.go | 130 +++++++++++++++++++ 7 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 internal/server/ladder_compute_cap.go create mode 100644 internal/server/ladder_compute_cap_test.go diff --git a/go.mod b/go.mod index ab4721d37..e8945c471 100644 --- a/go.mod +++ b/go.mod @@ -88,10 +88,10 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261105183508-790f369f4822 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261105183508-790f369f4822 - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261105183508-790f369f4822 - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261105183508-790f369f4822 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9 github.com/aws/aws-lambda-go v1.55.1 github.com/aws/aws-sdk-go-v2/service/kms v1.62.1 github.com/aws/aws-sdk-go-v2/service/lambda v1.113.4 diff --git a/go.sum b/go.sum index 0adbe8de3..60277f83d 100644 --- a/go.sum +++ b/go.sum @@ -90,14 +90,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.57.0/go.mod h1:dzcEjy1WJ0Q4u9twNR3LcLhNoYMRCrMCMafpxa0TjPQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0 h1:RoO5+d7uCmDqovLrHCr2/BuViUXvdcrNxyNM1pN9dDQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0/go.mod h1:YqwkQPrWSC7+byyc1VlKbWLBF5JsW5IoL6xUkemYSXk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261105183508-790f369f4822 h1:BMVY/iAVbsrxYMLBh8VYCb4qywH+ROp+6lFa9y1NBBs= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261105183508-790f369f4822/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261105183508-790f369f4822 h1:uBRU062BP6FWe+72aexmy+Md+UDUZ0vodQDW1clE58k= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261105183508-790f369f4822/go.mod h1:tsjVv5Rj/QGuyVQ6w+sZ0w8fn/imBTNL5l6okmEBJn8= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261105183508-790f369f4822 h1:Lqf1GTF3g1gUPeCsDpW1cAZ0e1NyHQAw4N1sdb/qCUM= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261105183508-790f369f4822/go.mod h1:X+73EdStvYV6pqEVZZFyMfLG1frORlO9B1IsuK0LfOs= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261105183508-790f369f4822 h1:eKmwzGVdQZ9aowC5ab7OoHyPc7gdKzoNeXdzJjPPUA4= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261105183508-790f369f4822/go.mod h1:kfbQ5CsDr1958iSuZq6edcuB0es62XRCXGn/hFFYBW0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9 h1:omDAY4B4wshoedBYE368TW05d0PR2HTnebPgu8WqIFw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9 h1:z4LNOkM5qTNq7dbzfdh0VdsSW4BTxRuVaXjOZfRADe0= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:uVh1sdSIwKbHCWZ/TA66m2cR9Odb/36sxDHNKLyuMKY= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9 h1:OISjPdyg7xcy8kCJelHJle4YnTMQycAWzglVi6L15uw= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:X+73EdStvYV6pqEVZZFyMfLG1frORlO9B1IsuK0LfOs= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9 h1:7Vz6QVVlwe0GG7TT2+4tQp9HBBCZ0YkfEDwFZOF+Ir8= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:kfbQ5CsDr1958iSuZq6edcuB0es62XRCXGn/hFFYBW0= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= diff --git a/internal/config/validation.go b/internal/config/validation.go index 9ba687625..2f8c74eef 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -888,7 +888,7 @@ func (c *LadderConfigDB) validateLadderBaselineBounds() error { // baseline can use reliably. A window of N days returns at most N daily points // and Cost Explorer lags 24-48h, so AWS needs the 7-point floor // (providers/aws ladder/baseline.go:17 minBaselineSeriesDays, pinned -// 9962786e0695, unexported) plus its 3-day missing-day tolerance +// c83aef8b18a9, unexported) plus its 3-day missing-day tolerance // (maxMissingDays): 10. Recheck when the aws provider module is bumped. // A provider without an entry has no minimum beyond lookback_days > 0. var ladderMinLookbackDays = map[string]int{"aws": 10} diff --git a/internal/server/handler_ladder.go b/internal/server/handler_ladder.go index dd00e21ac..42e2e1f7d 100644 --- a/internal/server/handler_ladder.go +++ b/internal/server/handler_ladder.go @@ -319,6 +319,11 @@ func (app *Application) executeLadderRun( } scope := engineCfg.Scope + layers := capability.SupportedLayers() + if err := ladderComputeCapRefusal(dbCfg, layers); err != nil { + return err + } + // Collect read-side data. All three calls are independent; errors are // fail-loud (not silently defaulted to empty). baseline, err := capability.GetUsageBaseline(ctx, scope, engineCfg.LookbackDays, engineCfg.BaselinePercentile) @@ -371,7 +376,7 @@ func (app *Application) executeLadderRun( // network I/O. in := &ladderRunPlanner{ dbCfg: dbCfg, engineCfg: engineCfg, baseline: baseline, layerStates: layerStates, rates: rates, - layers: capability.SupportedLayers(), term: term, paymentOpt: paymentOpt, now: now, missing: missing, + layers: layers, term: term, paymentOpt: paymentOpt, now: now, missing: missing, } savedRun, err := app.Config.PlanAndSaveLadderRun(ctx, dbCfg.ID, now, in.build) if err != nil { diff --git a/internal/server/handler_ladder_rates_test.go b/internal/server/handler_ladder_rates_test.go index f084ab026..a909d834b 100644 --- a/internal/server/handler_ladder_rates_test.go +++ b/internal/server/handler_ladder_rates_test.go @@ -21,6 +21,8 @@ var ratesTestNow = time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC) func runWithRates(t *testing.T, store *ladderTestStore, cap pkgladder.LadderCapability) error { t.Helper() dbCfg := validTestDBConfig("cfg-rates") + // Capped so the fail-closed Compute SP refusal does not fire for fakes that list it. + dbCfg.MaxHourlyCommitPerRun = f64(100) app := &Application{Config: store} return app.executeLadderRun(testutil.TestContext(t), &dbCfg, cap, "123456789012", pkgladder.Term1Year, pkgladder.PaymentNoUpfront, ratesTestNow) } diff --git a/internal/server/ladder_compute_cap.go b/internal/server/ladder_compute_cap.go new file mode 100644 index 000000000..b9f9d748d --- /dev/null +++ b/internal/server/ladder_compute_cap.go @@ -0,0 +1,23 @@ +package server + +import ( + "errors" + + pkgladder "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// ladderComputeCapRefusal fails closed for an uncapped Compute SP ladder: its +// rate is an account-wide blend, and only max_hourly_commit_per_run bounds that bias. +func ladderComputeCapRefusal(dbCfg *config.LadderConfigDB, layers []pkgladder.LayerSpec) error { + if dbCfg.MaxHourlyCommitPerRun != nil { + return nil + } + for _, l := range layers { + if l.Type == pkgladder.LayerComputeSP { + return errors.New("max_hourly_commit_per_run is required for a ladder config that sizes the Compute Savings Plan layer: its rate is an account-wide blend, so an uncapped run is refused") + } + } + return nil +} diff --git a/internal/server/ladder_compute_cap_test.go b/internal/server/ladder_compute_cap_test.go new file mode 100644 index 000000000..049c7857b --- /dev/null +++ b/internal/server/ladder_compute_cap_test.go @@ -0,0 +1,130 @@ +package server + +import ( + "context" + "testing" + "time" + + pkgladder "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/testutil" +) + +func TestLadderComputeCapRefusal(t *testing.T) { + computeSP := []pkgladder.LayerSpec{{Type: pkgladder.LayerComputeSP, Roles: []pkgladder.LayerRole{pkgladder.RoleFlex}}} + noCompute := []pkgladder.LayerSpec{{Type: pkgladder.LayerEC2InstanceSP, Roles: []pkgladder.LayerRole{pkgladder.RoleBase}}} + for _, tc := range []struct { + name string + layers []pkgladder.LayerSpec + cap *float64 + wantErr bool + }{ + {"compute sp without a cap", computeSP, nil, true}, + {"compute sp among others without a cap", awsLadderLayers, nil, true}, + {"compute sp with a cap", computeSP, f64(100), false}, + {"no compute sp without a cap", noCompute, nil, false}, + {"empty layers without a cap", nil, nil, false}, + } { + t.Run(tc.name, func(t *testing.T) { + dbCfg := validTestDBConfig("cfg-cap") + dbCfg.MaxHourlyCommitPerRun = tc.cap + err := ladderComputeCapRefusal(&dbCfg, tc.layers) + if tc.wantErr { + require.ErrorContains(t, err, "max_hourly_commit_per_run") + return + } + require.NoError(t, err) + }) + } +} + +// providerReadCounter counts the read-side provider calls of one run. +type providerReadCounter struct { + *fakeLadderCapability + baselines, layerStates, rates int +} + +func (c *providerReadCounter) GetUsageBaseline(ctx context.Context, s pkgladder.Scope, days int, pct float64) (pkgladder.UsageBaseline, error) { + c.baselines++ + return c.fakeLadderCapability.GetUsageBaseline(ctx, s, days, pct) +} + +func (c *providerReadCounter) GetLayerStates(ctx context.Context, s pkgladder.Scope) (map[pkgladder.LayerType]pkgladder.LayerState, error) { + c.layerStates++ + return c.fakeLadderCapability.GetLayerStates(ctx, s) +} + +func (c *providerReadCounter) GetCommitmentRates(ctx context.Context, s pkgladder.Scope, term pkgladder.Term, p pkgladder.PaymentOption) (map[pkgladder.LayerType]float64, error) { + c.rates++ + return c.fakeLadderCapability.GetCommitmentRates(ctx, s, term, p) +} + +// persistCountingStore counts PlanAndSaveLadderRun calls. +type persistCountingStore struct { + *ladderTestStore + persists int +} + +func (s *persistCountingStore) PlanAndSaveLadderRun(ctx context.Context, id string, now time.Time, build config.LadderPlanBuilder) (*config.LadderRunDB, error) { + s.persists++ + return s.ladderTestStore.PlanAndSaveLadderRun(ctx, id, now, build) +} + +func runComputeCapConfig(t *testing.T, cap *float64) (*LadderRunResult, *persistCountingStore, *providerReadCounter) { + t.Helper() + states := map[pkgladder.LayerType]pkgladder.LayerState{} + rates := map[pkgladder.LayerType]float64{} + for _, spec := range awsPlanningLayers { + states[spec.Type] = emptyLayerState(spec.Type) + rates[spec.Type] = 0.5 + } + reads := &providerReadCounter{fakeLadderCapability: &fakeLadderCapability{ + t: t, baseline: testBaselineLowWater(12.5), layerStates: states, rates: rates, supportedLayers: awsPlanningLayers, + }} + store := &persistCountingStore{ladderTestStore: &ladderTestStore{cloudAcct: validTestCloudAccount("123456789012")}} + app := &Application{ + Config: store, + LadderCapabilityFactory: func(_ context.Context, _, _ string) (pkgladder.LadderCapability, error) { + return reads, nil + }, + } + dbCfg := validTestDBConfig("cfg-compute-cap") + dbCfg.MaxHourlyCommitPerRun = cap + result := app.runLadderConfigs(testutil.TestContext(t), []config.LadderConfigDB{dbCfg}, "123456789012", "us-east-1", + pkgladder.Term1Year, pkgladder.PaymentNoUpfront, ratesTestNow, false) + return result, store, reads +} + +// awsPlanningLayers mirrors the AWS capability's SupportedLayers, one base layer +// only, so a capped run can allocate. +var awsPlanningLayers = []pkgladder.LayerSpec{ + {Type: pkgladder.LayerEC2InstanceSP, Roles: []pkgladder.LayerRole{pkgladder.RoleBase}}, + {Type: pkgladder.LayerComputeSP, Roles: []pkgladder.LayerRole{pkgladder.RoleFlex}}, + {Type: pkgladder.LayerConvertibleRI, Roles: []pkgladder.LayerRole{pkgladder.RoleBuffer}}, +} + +func TestRunLadderConfigs_ComputeSPWithoutCapIsErroredBeforeAnyRead(t *testing.T) { + result, store, reads := runComputeCapConfig(t, nil) + + assert.Equal(t, LadderRunResult{Errored: 1}, *result) + assert.Zero(t, store.persists, "PlanAndSaveLadderRun must not be called") + assert.Nil(t, store.savedRun) + assert.Empty(t, store.savedRuns) + assert.Zero(t, reads.baselines, "no baseline read") + assert.Zero(t, reads.layerStates, "no layer-state read") + assert.Zero(t, reads.rates, "no rate read") +} + +func TestRunLadderConfigs_ComputeSPWithCapStillPlans(t *testing.T) { + result, store, reads := runComputeCapConfig(t, f64(100)) + + assert.Equal(t, LadderRunResult{Planned: 1}, *result) + assert.Equal(t, 1, store.persists) + require.NotNil(t, store.savedRun) + assert.Equal(t, 1, reads.baselines) + assert.Equal(t, 1, reads.layerStates) + assert.Equal(t, 1, reads.rates) +} From 6c1a0a26c3e0b88a9703bf76233b76fa5233e69c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 21:50:15 +0100 Subject: [PATCH 2/2] fix(server): satisfy govet shadow and unparam in the Compute SP cap change The cap refusal gets its own variable instead of shadowing the err from the config conversion, and the cap tests use their own account ID so validTestCloudAccount keeps a varying argument. Co-Authored-By: Claude Opus 5.5 --- internal/server/handler_ladder.go | 4 ++-- internal/server/ladder_compute_cap_test.go | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/server/handler_ladder.go b/internal/server/handler_ladder.go index 42e2e1f7d..5b86b13c8 100644 --- a/internal/server/handler_ladder.go +++ b/internal/server/handler_ladder.go @@ -320,8 +320,8 @@ func (app *Application) executeLadderRun( scope := engineCfg.Scope layers := capability.SupportedLayers() - if err := ladderComputeCapRefusal(dbCfg, layers); err != nil { - return err + if refusal := ladderComputeCapRefusal(dbCfg, layers); refusal != nil { + return refusal } // Collect read-side data. All three calls are independent; errors are diff --git a/internal/server/ladder_compute_cap_test.go b/internal/server/ladder_compute_cap_test.go index 049c7857b..8db44c6c4 100644 --- a/internal/server/ladder_compute_cap_test.go +++ b/internal/server/ladder_compute_cap_test.go @@ -84,7 +84,7 @@ func runComputeCapConfig(t *testing.T, cap *float64) (*LadderRunResult, *persist reads := &providerReadCounter{fakeLadderCapability: &fakeLadderCapability{ t: t, baseline: testBaselineLowWater(12.5), layerStates: states, rates: rates, supportedLayers: awsPlanningLayers, }} - store := &persistCountingStore{ladderTestStore: &ladderTestStore{cloudAcct: validTestCloudAccount("123456789012")}} + store := &persistCountingStore{ladderTestStore: &ladderTestStore{cloudAcct: validTestCloudAccount(computeCapAccount)}} app := &Application{ Config: store, LadderCapabilityFactory: func(_ context.Context, _, _ string) (pkgladder.LadderCapability, error) { @@ -93,11 +93,14 @@ func runComputeCapConfig(t *testing.T, cap *float64) (*LadderRunResult, *persist } dbCfg := validTestDBConfig("cfg-compute-cap") dbCfg.MaxHourlyCommitPerRun = cap - result := app.runLadderConfigs(testutil.TestContext(t), []config.LadderConfigDB{dbCfg}, "123456789012", "us-east-1", + result := app.runLadderConfigs(testutil.TestContext(t), []config.LadderConfigDB{dbCfg}, computeCapAccount, "us-east-1", pkgladder.Term1Year, pkgladder.PaymentNoUpfront, ratesTestNow, false) return result, store, reads } +// computeCapAccount is the lambda and cloud account of the cap tests. +const computeCapAccount = "210987654321" + // awsPlanningLayers mirrors the AWS capability's SupportedLayers, one base layer // only, so a capped run can allocate. var awsPlanningLayers = []pkgladder.LayerSpec{