From cb11df4982198d1f1686c41afd501f92ff15f930 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 2 Jul 2026 15:01:40 +0200 Subject: [PATCH 1/5] feat(ladder): add core types and capability interfaces for commitment laddering New pkg/ladder package with typed enums (layers, roles, actions, modes, cadence, run/tranche statuses), LadderConfig and RampSchedule validation, LadderPlan with auditable PlannedActions and Explain() email rendering, and provider-neutral LadderCapability and LadderStore interfaces. Part of #1334. --- pkg/ladder/capability.go | 82 +++++ pkg/ladder/plan.go | 170 ++++++++++ pkg/ladder/plan_test.go | 291 +++++++++++++++++ pkg/ladder/store.go | 120 +++++++ pkg/ladder/types.go | 382 ++++++++++++++++++++++ pkg/ladder/types_test.go | 686 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 1731 insertions(+) create mode 100644 pkg/ladder/capability.go create mode 100644 pkg/ladder/plan.go create mode 100644 pkg/ladder/plan_test.go create mode 100644 pkg/ladder/store.go create mode 100644 pkg/ladder/types.go create mode 100644 pkg/ladder/types_test.go diff --git a/pkg/ladder/capability.go b/pkg/ladder/capability.go new file mode 100644 index 000000000..55fa8231a --- /dev/null +++ b/pkg/ladder/capability.go @@ -0,0 +1,82 @@ +package ladder + +import ( + "context" + "math/big" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// LadderCapability is implemented by each cloud provider to give the ladder +// engine a uniform interface for querying commitment state and executing +// purchases. Implementations live in providers/ and are injected into the +// engine at startup. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderCapability interface { + // Provider returns the cloud provider identifier (common.ProviderAWS, + // common.ProviderAzure, or common.ProviderGCP). + Provider() common.ProviderType + + // SupportedLayers returns the commitment layers this provider can fulfill. + // Each LayerSpec declares both the layer type and the roles it covers. + SupportedLayers() []LayerSpec + + // ListCommitments returns all active commitments for the given scope. + ListCommitments(ctx context.Context, scope Scope) ([]common.Commitment, error) + + // GetLayerStates returns a point-in-time snapshot for each supported layer. + // Layers with no active commitments are represented with nil pointer fields + // (never coerced to zero). + GetLayerStates(ctx context.Context, scope Scope) (map[LayerType]LayerState, error) + + // GetUsageBaseline computes a statistical baseline from historical + // on-demand usage over the given lookback window and percentile. + GetUsageBaseline(ctx context.Context, scope Scope, lookbackDays int, percentile float64) (UsageBaseline, error) + + // PurchaseLayer buys commitments for the given layer using the provided + // recommendation. Returns ErrCommitmentPurchaseNotSupported when the + // provider has no programmatic purchase API for this layer. + // + // Precision note: the engine plans amounts as exact *big.Rat values + // (PlannedAction.AmountUSDPerHour), but this boundary converts them to + // the float64 cost fields of common.Recommendation. This is a documented + // precision seam; exact-decimal plumbing through the purchase path is + // addressed in a later PR. + PurchaseLayer(ctx context.Context, layer LayerType, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) + + // ReshapeBuffer exchanges or modifies buffer-layer commitments to improve + // utilization when it falls below the configured threshold. + ReshapeBuffer(ctx context.Context, scope Scope, cfg BufferReshapeConfig) (ReshapeSummary, error) +} + +// BufferReshapeConfig parameterizes a buffer reshape operation. +type BufferReshapeConfig struct { + // MaxPaymentPerExchangeUSD caps the payment for a single exchange. nil + // means no per-exchange cap is applied. + MaxPaymentPerExchangeUSD *big.Rat + // MaxPaymentDailyUSD caps the total exchange payments for the current day. + // nil means no daily cap is applied. + MaxPaymentDailyUSD *big.Rat + // UtilizationThresholdPct triggers a reshape when commitment utilization + // drops below this percentage. + UtilizationThresholdPct float64 + // LookbackDays is the window used to measure utilization when deciding + // whether to trigger a reshape. + LookbackDays int + // DryRun, when true, simulates the reshape without executing any exchanges. + DryRun bool +} + +// ReshapeSummary reports the outcome of a buffer reshape operation. +type ReshapeSummary struct { + // Details holds per-commitment outcome descriptions for logging and audit. + Details []string + // Analyzed is the total number of commitments inspected. + Analyzed int + // Reshaped is the number of commitments exchanged or modified. + Reshaped int + // Skipped is the number of commitments that did not meet the utilization + // threshold or were blocked by a spend cap. + Skipped int +} diff --git a/pkg/ladder/plan.go b/pkg/ladder/plan.go new file mode 100644 index 000000000..d5ae18494 --- /dev/null +++ b/pkg/ladder/plan.go @@ -0,0 +1,170 @@ +package ladder + +import ( + "fmt" + "math/big" + "strings" + "time" +) + +// PlannedAction is a single commitment action proposed by the ladder engine. +// +// Every action must carry a non-empty Rationale for audit and approval-email +// readability -- automated money-path decisions must be explained so the +// human approver (or the audit log) can understand why each action was chosen. +type PlannedAction struct { + Action ActionType + Layer LayerType + // AmountUSDPerHour is the hourly commitment delta. It must be non-nil and + // positive for ActionPurchase, and must be nil for ActionHold and + // ActionReshape (whose financial impact is implicit in the underlying + // exchange operation). + AmountUSDPerHour *big.Rat + // Term is the commitment term (e.g., "1yr", "3yr"). Populated for purchase + // actions; empty for hold and reshape. + Term string + // PaymentOption is the payment structure (e.g., "all-upfront", + // "no-upfront"). Populated for purchase actions; empty for hold and reshape. + PaymentOption string + // Rationale is a human-readable explanation of why this action was chosen. + // Must be non-empty for all action types. + Rationale string + // DataSources lists the data feeds (Cost Explorer, CloudWatch, etc.) used + // to derive this action. Aids auditability. + DataSources []string +} + +// Validate checks that the action is self-consistent: +// - action type and layer type must be recognized +// - rationale must be non-empty +// - purchase actions require a positive AmountUSDPerHour and non-empty +// Term and PaymentOption (money-shaping fields must be present) +// - hold and reshape actions require a nil AmountUSDPerHour +func (a *PlannedAction) Validate() error { + if err := a.Action.Validate(); err != nil { + return fmt.Errorf("action: %w", err) + } + if err := a.Layer.Validate(); err != nil { + return fmt.Errorf("layer: %w", err) + } + if a.Rationale == "" { + return fmt.Errorf("rationale is required (money-path auditability)") + } + switch a.Action { + case ActionPurchase: + return a.validatePurchaseFields() + case ActionHold, ActionReshape: + if a.AmountUSDPerHour != nil { + return fmt.Errorf("%s action requires nil AmountUSDPerHour, got %s", + a.Action, a.AmountUSDPerHour.RatString()) + } + } + return nil +} + +// validatePurchaseFields checks the money-shaping fields required for a +// purchase action. Split out of Validate to keep each function's cyclomatic +// complexity within the repo limit. +func (a *PlannedAction) validatePurchaseFields() error { + if a.AmountUSDPerHour == nil { + return fmt.Errorf("purchase action requires a non-nil AmountUSDPerHour") + } + if a.AmountUSDPerHour.Sign() <= 0 { + return fmt.Errorf("purchase action requires a positive AmountUSDPerHour, got %s", + a.AmountUSDPerHour.RatString()) + } + // Term and PaymentOption shape the money committed; a purchase with + // either missing would silently default at the provider boundary. + // pkg/common has no typed Term/PaymentOption enums yet, so non-empty + // is the strongest check available here. + if a.Term == "" { + return fmt.Errorf("purchase action requires a non-empty Term") + } + if a.PaymentOption == "" { + return fmt.Errorf("purchase action requires a non-empty PaymentOption") + } + return nil +} + +// LadderPlan is a complete, validated commitment plan for one scope and run. +// It captures the baseline, the monetary gap, and the ordered list of actions +// the engine proposes to close that gap. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderPlan struct { + Scope Scope + GeneratedAt time.Time + // TargetUSDPerHour is the hourly commitment target derived from + // Baseline * TargetCoveragePct. nil means it could not be computed. + TargetUSDPerHour *big.Rat + // ExistingUSDPerHour is the sum of hourly amortized costs across all + // active commitment layers. nil means it could not be measured. + ExistingUSDPerHour *big.Rat + // GapUSDPerHour is TargetUSDPerHour - ExistingUSDPerHour. nil when either + // input is nil. + GapUSDPerHour *big.Rat + Actions []PlannedAction + Baseline UsageBaseline +} + +// Validate checks that all PlannedActions in the plan are self-consistent. +func (p *LadderPlan) Validate() error { + for i, a := range p.Actions { + if err := a.Validate(); err != nil { + return fmt.Errorf("action[%d]: %w", i, err) + } + } + return nil +} + +// formatUSDPerHour renders a *big.Rat as a fixed 2-decimal USD/hr string. +// nil is rendered as "unknown" -- never as "$0.00" -- to unambiguously +// distinguish absent data from a genuine zero commitment (project rule: +// absent numbers are nil, not zero). +func formatUSDPerHour(v *big.Rat) string { + if v == nil { + return "unknown" + } + f, _ := v.Float64() + return fmt.Sprintf("$%.2f/hr", f) +} + +// Explain returns a deterministic, human-readable multi-line summary of the +// plan. The output is suitable for use as an approval email body and is +// designed to be read by a non-technical budget owner. +// +// Layout: +// 1. Scope header and generation timestamp +// 2. Baseline parameters (lookback window, percentile) +// 3. Target / existing / gap hourly rates +// 4. Numbered action list (or "none" when all layers are at target) +func (p *LadderPlan) Explain() string { + var b strings.Builder + + fmt.Fprintf(&b, "Scope: provider=%s account=%s\n", p.Scope.Provider, p.Scope.AccountID) + fmt.Fprintf(&b, "Generated: %s\n", p.GeneratedAt.UTC().Format(time.RFC3339)) + fmt.Fprintf(&b, "Baseline: lookback=%dd percentile=%.0f\n", + p.Baseline.LookbackDays, p.Baseline.Percentile) + fmt.Fprintf(&b, "Target: %s\n", formatUSDPerHour(p.TargetUSDPerHour)) + fmt.Fprintf(&b, "Existing: %s\n", formatUSDPerHour(p.ExistingUSDPerHour)) + fmt.Fprintf(&b, "Gap: %s\n", formatUSDPerHour(p.GapUSDPerHour)) + + if len(p.Actions) == 0 { + fmt.Fprintf(&b, "Actions: none (all layers at target)\n") + return b.String() + } + + fmt.Fprintf(&b, "Actions (%d):\n", len(p.Actions)) + for i, a := range p.Actions { + switch a.Action { + case ActionPurchase: + fmt.Fprintf(&b, " %d. %s %s %s term=%s payment=%s -- %s\n", + i+1, a.Action, a.Layer, formatUSDPerHour(a.AmountUSDPerHour), + a.Term, a.PaymentOption, a.Rationale) + default: + fmt.Fprintf(&b, " %d. %s %s -- %s\n", + i+1, a.Action, a.Layer, a.Rationale) + } + } + return b.String() +} diff --git a/pkg/ladder/plan_test.go b/pkg/ladder/plan_test.go new file mode 100644 index 000000000..ef63b28c6 --- /dev/null +++ b/pkg/ladder/plan_test.go @@ -0,0 +1,291 @@ +package ladder + +import ( + "math/big" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +func TestPlannedActionValidate(t *testing.T) { + t.Parallel() + amount := new(big.Rat).SetFrac64(25, 100) // $0.25/hr + + cases := []struct { + name string + action PlannedAction + wantErr bool + }{ + { + name: "valid purchase", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: amount, Term: "1yr", + PaymentOption: "no-upfront", Rationale: "gap fill", + }, + wantErr: false, + }, + { + name: "valid hold", + action: PlannedAction{ + Action: ActionHold, Layer: LayerConvertibleRI, + Rationale: "layer already at target", + }, + wantErr: false, + }, + { + name: "valid reshape", + action: PlannedAction{ + Action: ActionReshape, Layer: LayerConvertibleRI, + Rationale: "low utilization below threshold", + }, + wantErr: false, + }, + { + name: "purchase without amount (nil)", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "purchase with zero amount", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: new(big.Rat), // zero + Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "purchase with negative amount", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: new(big.Rat).SetInt64(-1), + Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "purchase with empty term", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: amount, Term: "", + PaymentOption: "no-upfront", Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "purchase with empty payment option", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: amount, Term: "1yr", + PaymentOption: "", Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "hold with non-nil amount", + action: PlannedAction{ + Action: ActionHold, Layer: LayerConvertibleRI, + AmountUSDPerHour: amount, + Rationale: "at target", + }, + wantErr: true, + }, + { + name: "reshape with non-nil amount", + action: PlannedAction{ + Action: ActionReshape, Layer: LayerConvertibleRI, + AmountUSDPerHour: amount, + Rationale: "low utilization", + }, + wantErr: true, + }, + { + name: "empty rationale", + action: PlannedAction{ + Action: ActionHold, Layer: LayerConvertibleRI, + Rationale: "", + }, + wantErr: true, + }, + { + name: "unknown action type", + action: PlannedAction{ + Action: "unknown-action", Layer: LayerConvertibleRI, + Rationale: "x", + }, + wantErr: true, + }, + { + name: "unknown layer type", + action: PlannedAction{ + Action: ActionHold, Layer: "unknown-layer", + Rationale: "x", + }, + wantErr: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.action.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} + +func TestFormatUSDPerHour(t *testing.T) { + t.Parallel() + cases := []struct { + in *big.Rat + want string + }{ + {nil, "unknown"}, + {new(big.Rat).SetFrac64(25, 100), "$0.25/hr"}, + {new(big.Rat).SetInt64(10), "$10.00/hr"}, + {new(big.Rat), "$0.00/hr"}, + {new(big.Rat).SetFrac64(1, 3), "$0.33/hr"}, // truncated at 2 decimals via Float64 + } + for _, c := range cases { + got := formatUSDPerHour(c.in) + if got != c.want { + t.Errorf("formatUSDPerHour(%v) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestExplain_NilMoneyRendersUnknown verifies that nil *big.Rat fields in +// LadderPlan are rendered as "unknown" and never as "$0.00". +func TestExplain_NilMoneyRendersUnknown(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + TargetUSDPerHour: nil, + ExistingUSDPerHour: nil, + GapUSDPerHour: nil, + } + out := plan.Explain() + if strings.Contains(out, "$0.00") { + t.Errorf("Explain() rendered nil as $0.00; want 'unknown':\n%s", out) + } + if !strings.Contains(out, "unknown") { + t.Errorf("Explain() does not contain 'unknown' for nil money:\n%s", out) + } +} + +// TestExplain_AllHold verifies that an empty Actions slice is handled +// gracefully with a "none" note rather than an empty or malformed section. +func TestExplain_AllHold(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + Actions: nil, + } + out := plan.Explain() + if !strings.Contains(out, "none") { + t.Errorf("Explain() all-HOLD case missing 'none':\n%s", out) + } +} + +// TestExplain_PurchaseAction verifies that a purchase action is rendered with +// all expected fields present in the output. +func TestExplain_PurchaseAction(t *testing.T) { + t.Parallel() + target := new(big.Rat).SetInt64(100) + existing := new(big.Rat).SetInt64(60) + gap := new(big.Rat).SetInt64(40) + amount := new(big.Rat).SetInt64(40) + + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123456789012"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + TargetUSDPerHour: target, + ExistingUSDPerHour: existing, + GapUSDPerHour: gap, + Actions: []PlannedAction{ + { + Action: ActionPurchase, + Layer: LayerComputeSP, + AmountUSDPerHour: amount, + Term: "1yr", + PaymentOption: "no-upfront", + Rationale: "fill coverage gap", + }, + }, + } + out := plan.Explain() + checks := []string{ + "aws", + "123456789012", + "$100.00/hr", + "$60.00/hr", + "$40.00/hr", + string(ActionPurchase), + string(LayerComputeSP), + "1yr", + "no-upfront", + "fill coverage gap", + } + for _, want := range checks { + if !strings.Contains(out, want) { + t.Errorf("Explain() missing %q in output:\n%s", want, out) + } + } +} + +// TestExplain_ReshapeAction verifies that a reshape action does not include +// amount/term/payment fields in the output (they should be absent for +// non-purchase actions). +func TestExplain_ReshapeAction(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + Actions: []PlannedAction{ + { + Action: ActionReshape, + Layer: LayerConvertibleRI, + Rationale: "utilization below threshold", + }, + }, + } + out := plan.Explain() + if !strings.Contains(out, string(ActionReshape)) { + t.Errorf("Explain() missing action type %q:\n%s", ActionReshape, out) + } + if !strings.Contains(out, "utilization below threshold") { + t.Errorf("Explain() missing rationale:\n%s", out) + } +} + +// TestExplain_Deterministic verifies that calling Explain() twice on the same +// plan produces identical output. +func TestExplain_Deterministic(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderGCP, AccountID: "my-project"}, + GeneratedAt: time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 14, Percentile: 10}, + Actions: nil, + } + first := plan.Explain() + second := plan.Explain() + if first != second { + t.Errorf("Explain() is not deterministic:\nfirst:\n%s\nsecond:\n%s", first, second) + } +} diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go new file mode 100644 index 000000000..4565ed1ce --- /dev/null +++ b/pkg/ladder/store.go @@ -0,0 +1,120 @@ +package ladder + +import ( + "context" + "fmt" + "time" +) + +// RunStatus is the lifecycle state of a ladder engine run. +type RunStatus string + +const ( + RunStatusPlanned RunStatus = "planned" + RunStatusAwaitingApproval RunStatus = "awaiting_approval" + RunStatusApproved RunStatus = "approved" + RunStatusExecuting RunStatus = "executing" + RunStatusCompleted RunStatus = "completed" + RunStatusFailed RunStatus = "failed" + RunStatusCancelled RunStatus = "cancelled" //nolint:misspell // matches existing DB status spelling ("cancelled") + RunStatusExpired RunStatus = "expired" +) + +// Validate returns an error when s is not a recognized RunStatus. +func (s RunStatus) Validate() error { + switch s { + case RunStatusPlanned, RunStatusAwaitingApproval, RunStatusApproved, + RunStatusExecuting, RunStatusCompleted, RunStatusFailed, + RunStatusCancelled, RunStatusExpired: + return nil + } + return fmt.Errorf("unknown run status %q", s) +} + +// ParseRunStatus converts s into a RunStatus, returning a descriptive error +// when s is not a recognized value. +func ParseRunStatus(s string) (RunStatus, error) { + st := RunStatus(s) + if err := st.Validate(); err != nil { + return "", err + } + return st, nil +} + +// TrancheStatus is the lifecycle state of a single ramp tranche. +type TrancheStatus string + +const ( + TrancheStatusScheduled TrancheStatus = "scheduled" + TrancheStatusFired TrancheStatus = "fired" + TrancheStatusCompleted TrancheStatus = "completed" + TrancheStatusCancelled TrancheStatus = "cancelled" //nolint:misspell // matches existing DB status spelling ("cancelled") + TrancheStatusFailed TrancheStatus = "failed" +) + +// Validate returns an error when s is not a recognized TrancheStatus. +func (s TrancheStatus) Validate() error { + switch s { + case TrancheStatusScheduled, TrancheStatusFired, TrancheStatusCompleted, + TrancheStatusCancelled, TrancheStatusFailed: + return nil + } + return fmt.Errorf("unknown tranche status %q", s) +} + +// ParseTrancheStatus converts s into a TrancheStatus, returning a +// descriptive error when s is not a recognized value. +func ParseTrancheStatus(s string) (TrancheStatus, error) { + st := TrancheStatus(s) + if err := st.Validate(); err != nil { + return "", err + } + return st, nil +} + +// RunRecord is the persistent record of a single ladder engine run. IDs are +// strings to remain agnostic to the backing store's key scheme (UUID, ULID, +// etc.). +type RunRecord struct { + ID string + Scope Scope + Status RunStatus + CreatedAt time.Time + CompletedAt *time.Time + // PlanJSON holds a serialized LadderPlan for audit. Stored as a string + // rather than an embedded struct so the store interface stays + // serialization-format-agnostic. + PlanJSON string +} + +// Tranche is one ramp step that has been persisted for scheduled firing. +type Tranche struct { + // FireAfter is the wall-clock time at or after which this tranche fires. + FireAfter time.Time + // FiredAt is nil until the tranche actually fires. + FiredAt *time.Time + ID string + RunID string + Status TrancheStatus + StepIndex int +} + +// LadderStore is the storage contract for the ladder engine. The concrete +// implementation lives in internal/ (separate Go module) and is injected +// into the engine at startup; pkg/ defines only the interface. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderStore interface { + // SaveRun persists a new run record or updates an existing one (upsert by + // ID). + SaveRun(ctx context.Context, run *RunRecord) error + + // LatestRunStartedAt returns the CreatedAt timestamp of the most recent + // run for the given scope, or nil when no run has been recorded yet. + LatestRunStartedAt(ctx context.Context, scope Scope) (*time.Time, error) + + // SaveTranches persists a batch of tranches for the given run. Callers + // may call this multiple times (e.g., once per ramp step) and + // implementations should upsert by tranche ID. + SaveTranches(ctx context.Context, runID string, tranches []Tranche) error +} diff --git a/pkg/ladder/types.go b/pkg/ladder/types.go new file mode 100644 index 000000000..dcc649f0f --- /dev/null +++ b/pkg/ladder/types.go @@ -0,0 +1,382 @@ +// Package ladder defines the types, interfaces, and plan rendering for the +// commitment-laddering engine. This package contains the public contract only; +// the allocation algorithm and provider implementations live in internal/ and +// providers/ respectively. +package ladder + +import ( + "fmt" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// LayerType identifies a commitment layer in the ladder hierarchy. +type LayerType string + +const ( + LayerEC2InstanceSP LayerType = "ec2-instance-sp" + LayerComputeSP LayerType = "compute-sp" + LayerConvertibleRI LayerType = "convertible-ri" + LayerAzureReservation LayerType = "azure-reservation" + LayerAzureSavingsPlan LayerType = "azure-savings-plan" +) + +// Validate returns an error when l is not a recognized LayerType. +func (l LayerType) Validate() error { + switch l { + case LayerEC2InstanceSP, LayerComputeSP, LayerConvertibleRI, + LayerAzureReservation, LayerAzureSavingsPlan: + return nil + } + return fmt.Errorf("unknown layer type %q", l) +} + +// ParseLayerType converts s into a LayerType, returning a descriptive error +// when s is not a recognized value. Use it to validate external input at the +// boundary instead of casting raw strings. +func ParseLayerType(s string) (LayerType, error) { + l := LayerType(s) + if err := l.Validate(); err != nil { + return "", err + } + return l, nil +} + +// LayerRole describes the role a layer plays in the ladder allocation. +type LayerRole string + +const ( + RoleBase LayerRole = "base" + RoleFlex LayerRole = "flex" + RoleBuffer LayerRole = "buffer" +) + +// Validate returns an error when r is not a recognized LayerRole. +func (r LayerRole) Validate() error { + switch r { + case RoleBase, RoleFlex, RoleBuffer: + return nil + } + return fmt.Errorf("unknown layer role %q", r) +} + +// ParseLayerRole converts s into a LayerRole, returning a descriptive error +// when s is not a recognized value. +func ParseLayerRole(s string) (LayerRole, error) { + r := LayerRole(s) + if err := r.Validate(); err != nil { + return "", err + } + return r, nil +} + +// ActionType describes what a PlannedAction will do. +type ActionType string + +const ( + ActionPurchase ActionType = "purchase" + ActionReshape ActionType = "reshape" + ActionHold ActionType = "hold" +) + +// Validate returns an error when a is not a recognized ActionType. +func (a ActionType) Validate() error { + switch a { + case ActionPurchase, ActionReshape, ActionHold: + return nil + } + return fmt.Errorf("unknown action type %q", a) +} + +// ParseActionType converts s into an ActionType, returning a descriptive +// error when s is not a recognized value. +func ParseActionType(s string) (ActionType, error) { + a := ActionType(s) + if err := a.Validate(); err != nil { + return "", err + } + return a, nil +} + +// LadderMode controls whether ladder runs require human approval before +// executing purchases. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderMode string + +const ( + ModeEmailApproval LadderMode = "email_approval" + ModeAutoApprove LadderMode = "auto_approve" +) + +// Validate returns an error when m is not a recognized LadderMode. +func (m LadderMode) Validate() error { + switch m { + case ModeEmailApproval, ModeAutoApprove: + return nil + } + return fmt.Errorf("unknown ladder mode %q", m) +} + +// ParseLadderMode converts s into a LadderMode, returning a descriptive +// error when s is not a recognized value. +func ParseLadderMode(s string) (LadderMode, error) { + m := LadderMode(s) + if err := m.Validate(); err != nil { + return "", err + } + return m, nil +} + +// LadderCadence controls how often the ladder engine runs. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderCadence string + +const ( + CadenceDaily LadderCadence = "daily" + CadenceWeekly LadderCadence = "weekly" +) + +// Validate returns an error when c is not a recognized LadderCadence. +func (c LadderCadence) Validate() error { + switch c { + case CadenceDaily, CadenceWeekly: + return nil + } + return fmt.Errorf("unknown ladder cadence %q", c) +} + +// ParseLadderCadence converts s into a LadderCadence, returning a +// descriptive error when s is not a recognized value. +func ParseLadderCadence(s string) (LadderCadence, error) { + c := LadderCadence(s) + if err := c.Validate(); err != nil { + return "", err + } + return c, nil +} + +// LayerSpec describes a layer and the roles it fulfills within the ladder. +// Azure reservations carry both base and buffer roles simultaneously. +type LayerSpec struct { + Type LayerType + Roles []LayerRole +} + +// Scope identifies the ladder scope: a specific provider account or +// subscription that the ladder engine operates on. +type Scope struct { + Provider common.ProviderType + AccountID string +} + +// Validate checks that the scope names a known provider and a non-empty +// account identifier. Called first by LadderConfig.Validate so a malformed +// scope fails loud before any other configuration check. +func (s Scope) Validate() error { + switch s.Provider { + case common.ProviderAWS, common.ProviderAzure, common.ProviderGCP: + // known provider + default: + return fmt.Errorf("unknown provider %q (allowed: %s, %s, %s)", + s.Provider, common.ProviderAWS, common.ProviderAzure, common.ProviderGCP) + } + if s.AccountID == "" { + return fmt.Errorf("account ID is required") + } + return nil +} + +// Default configuration constants. Every default is named and documented so +// callers can reference the symbolic name in code rather than bare literals. +const ( + // DefaultTargetCoveragePct is the percentage of on-demand spend to cover + // with commitments when no explicit target is configured. + DefaultTargetCoveragePct = 100.0 + + // DefaultBufferFraction is the share of the base allocation reserved in + // short-term or convertible buffer commitments that can be reshaped as + // usage patterns shift. + DefaultBufferFraction = 0.10 + + // DefaultBaselinePercentile is the usage percentile used to anchor the + // base commitment layer. A low percentile guards against over-committing + // on volatile workloads. + DefaultBaselinePercentile = 5.0 + + // DefaultLookbackDays is the historical window (in days) used to compute + // the usage baseline when none is specified. + DefaultLookbackDays = 30 + + // rampSumEpsilon is the acceptable absolute error when validating that + // RampStep fractions sum to 1.0. Floating-point arithmetic can introduce + // small rounding error, so we allow a small tolerance rather than + // requiring exact equality. + rampSumEpsilon = 1e-9 +) + +// RampStep is a single tranche within a ramp schedule. +type RampStep struct { + // AfterDays is the number of days after the run starts at which this + // tranche fires. Must be strictly greater than the previous step's + // AfterDays (ascending order required by RampSchedule.Validate). + AfterDays int + // Fraction is the share of the total target allocation committed by this + // tranche. Must be in (0, 1]; fractions across all steps must sum to 1.0. + Fraction float64 +} + +// RampSchedule spreads commitment purchases across time-indexed tranches. +// +// This type mirrors the semantic intent of internal/config/types.go +// RampSchedule (which uses a percent-per-step + interval model) but is +// defined independently because pkg/ and internal/ are separate Go modules +// and pkg/ cannot import internal/. See internal/config/types.go for the +// internal variant. +type RampSchedule struct { + Steps []RampStep +} + +// Validate checks that the ramp schedule is well-formed: +// - at least one step +// - AfterDays values are strictly ascending +// - each fraction is in (0, 1] +// - fractions sum to 1.0 within rampSumEpsilon +func (r RampSchedule) Validate() error { + if len(r.Steps) == 0 { + return fmt.Errorf("ramp schedule has no steps") + } + var sum float64 + prev := -1 + for i, s := range r.Steps { + if s.AfterDays <= prev { + return fmt.Errorf("ramp step %d: AfterDays %d must be strictly greater than previous %d", i, s.AfterDays, prev) + } + prev = s.AfterDays + if s.Fraction <= 0 || s.Fraction > 1 { + return fmt.Errorf("ramp step %d: fraction %g must be in (0, 1]", i, s.Fraction) + } + sum += s.Fraction + } + diff := sum - 1.0 + if diff < -rampSumEpsilon || diff > rampSumEpsilon { + return fmt.Errorf("ramp step fractions sum to %g, must equal 1.0", sum) + } + return nil +} + +// LadderConfig holds the full configuration for a single ladder scope run. +// +//nolint:revive // Ladder* prefix is the spec-mandated public name (issue #1334); matches pkg/exchange's Exchange* convention. +type LadderConfig struct { + Scope Scope + Mode LadderMode + Cadence LadderCadence + // MaxHourlyCommitPerRun caps the total hourly commitment delta a single + // run may purchase. nil means no cap is applied; when present the cap + // must be positive (validated by Validate). + MaxHourlyCommitPerRun *float64 + Ramp RampSchedule + TargetCoveragePct float64 + BufferFraction float64 + BaselinePercentile float64 + LookbackDays int + // MaxActionsPerRun limits how many PlannedActions the engine may execute + // per run. Must be > 0. + MaxActionsPerRun int +} + +// Validate checks that all configuration fields are within valid ranges and +// all sub-types are well-formed. It returns a specific, descriptive error on +// any violation -- callers must not silently default away a validation failure. +func (c *LadderConfig) Validate() error { + if err := c.Scope.Validate(); err != nil { + return fmt.Errorf("scope: %w", err) + } + if err := c.validateBaselineBounds(); err != nil { + return err + } + if err := c.Mode.Validate(); err != nil { + return fmt.Errorf("mode: %w", err) + } + if err := c.Cadence.Validate(); err != nil { + return fmt.Errorf("cadence: %w", err) + } + if err := c.Ramp.Validate(); err != nil { + return fmt.Errorf("ramp: %w", err) + } + return c.validateRunLimits() +} + +// validateBaselineBounds checks the coverage target, buffer fraction, and +// baseline measurement parameters. Split out of Validate to keep each +// function's cyclomatic complexity within the repo limit. +func (c *LadderConfig) validateBaselineBounds() error { + if c.TargetCoveragePct <= 0 || c.TargetCoveragePct > 100 { + return fmt.Errorf("target_coverage_pct %g must be in (0, 100]", c.TargetCoveragePct) + } + if c.BufferFraction < 0 || c.BufferFraction >= 1 { + return fmt.Errorf("buffer_fraction %g must be in [0, 1)", c.BufferFraction) + } + if c.BaselinePercentile <= 0 || c.BaselinePercentile > 50 { + return fmt.Errorf("baseline_percentile %g must be in (0, 50]", c.BaselinePercentile) + } + if c.LookbackDays <= 0 { + return fmt.Errorf("lookback_days %d must be > 0", c.LookbackDays) + } + return nil +} + +// validateRunLimits checks the per-run spend and action caps. +func (c *LadderConfig) validateRunLimits() error { + if c.MaxHourlyCommitPerRun != nil && *c.MaxHourlyCommitPerRun <= 0 { + return fmt.Errorf("max_hourly_commit_per_run %g must be > 0 when set (leave nil for no cap)", *c.MaxHourlyCommitPerRun) + } + if c.MaxActionsPerRun <= 0 { + return fmt.Errorf("max_actions_per_run %d must be > 0", c.MaxActionsPerRun) + } + return nil +} + +// UsageBaseline is a statistical summary of recent on-demand usage used to +// size the base commitment layer. Every monetary field is a pointer to +// distinguish "absent" from "genuinely zero" (project rule: absent numbers +// are pointers/nil, never 0). +type UsageBaseline struct { + // LowWaterUSDPerHour is the usage floor derived from the configured + // percentile (e.g., the 5th percentile of hourly spend). + LowWaterUSDPerHour *float64 + // StableUSDPerHour is the estimated stable portion after applying the + // buffer fraction to LowWaterUSDPerHour. + StableUSDPerHour *float64 + // Series holds the raw hourly values used for computation. It is optional + // and may be nil when only the summary statistics are needed (e.g., when + // populating the approval email body). + Series []float64 + // LookbackDays is the window (in days) over which the baseline was + // computed. + LookbackDays int + // Percentile is the statistical percentile used for LowWaterUSDPerHour. + Percentile float64 +} + +// LayerState is a point-in-time snapshot of an existing commitment layer. +// All monetary and percentage fields are pointers to distinguish "not yet +// measured" from "measured zero" (project rule: absent numbers are nil). +type LayerState struct { + // ExistingUSDPerHour is the total hourly amortized cost of active + // commitments in this layer. + ExistingUSDPerHour *float64 + // ExpiringUSDPerHour is the share of ExistingUSDPerHour whose commitments + // expire within the current run cadence window. + ExpiringUSDPerHour *float64 + // CoveragePct is the percentage of eligible on-demand spend currently + // covered by commitments in this layer. + CoveragePct *float64 + // UtilizationPct is the current utilization percentage of commitments in + // this layer. + UtilizationPct *float64 + // Layer identifies which commitment layer this snapshot describes. + Layer LayerType +} diff --git a/pkg/ladder/types_test.go b/pkg/ladder/types_test.go new file mode 100644 index 000000000..fa2970d58 --- /dev/null +++ b/pkg/ladder/types_test.go @@ -0,0 +1,686 @@ +package ladder + +import ( + "testing" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +func TestLayerTypeValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in LayerType + wantErr bool + }{ + {LayerEC2InstanceSP, false}, + {LayerComputeSP, false}, + {LayerConvertibleRI, false}, + {LayerAzureReservation, false}, + {LayerAzureSavingsPlan, false}, + {"unknown-layer", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("LayerType(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("LayerType(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestLayerRoleValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in LayerRole + wantErr bool + }{ + {RoleBase, false}, + {RoleFlex, false}, + {RoleBuffer, false}, + {"unknown-role", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("LayerRole(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("LayerRole(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestActionTypeValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in ActionType + wantErr bool + }{ + {ActionPurchase, false}, + {ActionReshape, false}, + {ActionHold, false}, + {"unknown-action", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("ActionType(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("ActionType(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestLadderModeValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in LadderMode + wantErr bool + }{ + {ModeEmailApproval, false}, + {ModeAutoApprove, false}, + {"unknown-mode", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("LadderMode(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("LadderMode(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestLadderCadenceValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in LadderCadence + wantErr bool + }{ + {CadenceDaily, false}, + {CadenceWeekly, false}, + {"monthly", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("LadderCadence(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("LadderCadence(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestRunStatusValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in RunStatus + wantErr bool + }{ + {RunStatusPlanned, false}, + {RunStatusAwaitingApproval, false}, + {RunStatusApproved, false}, + {RunStatusExecuting, false}, + {RunStatusCompleted, false}, + {RunStatusFailed, false}, + {RunStatusCancelled, false}, + {RunStatusExpired, false}, + {"unknown-status", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("RunStatus(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("RunStatus(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestTrancheStatusValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in TrancheStatus + wantErr bool + }{ + {TrancheStatusScheduled, false}, + {TrancheStatusFired, false}, + {TrancheStatusCompleted, false}, + {TrancheStatusCancelled, false}, + {TrancheStatusFailed, false}, + {"unknown-tranche-status", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("TrancheStatus(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("TrancheStatus(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestParseLayerType(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want LayerType + wantErr bool + }{ + {"ec2-instance-sp", LayerEC2InstanceSP, false}, + {"compute-sp", LayerComputeSP, false}, + {"convertible-ri", LayerConvertibleRI, false}, + {"azure-reservation", LayerAzureReservation, false}, + {"azure-savings-plan", LayerAzureSavingsPlan, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseLayerType(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseLayerType(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseLayerType(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseLayerType(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseLayerRole(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want LayerRole + wantErr bool + }{ + {"base", RoleBase, false}, + {"flex", RoleFlex, false}, + {"buffer", RoleBuffer, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseLayerRole(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseLayerRole(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseLayerRole(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseLayerRole(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseActionType(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want ActionType + wantErr bool + }{ + {"purchase", ActionPurchase, false}, + {"reshape", ActionReshape, false}, + {"hold", ActionHold, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseActionType(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseActionType(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseActionType(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseActionType(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseLadderMode(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want LadderMode + wantErr bool + }{ + {"email_approval", ModeEmailApproval, false}, + {"auto_approve", ModeAutoApprove, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseLadderMode(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseLadderMode(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseLadderMode(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseLadderMode(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseLadderCadence(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want LadderCadence + wantErr bool + }{ + {"daily", CadenceDaily, false}, + {"weekly", CadenceWeekly, false}, + {"monthly", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseLadderCadence(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseLadderCadence(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseLadderCadence(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseLadderCadence(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseRunStatus(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want RunStatus + wantErr bool + }{ + {"planned", RunStatusPlanned, false}, + {"awaiting_approval", RunStatusAwaitingApproval, false}, + {"approved", RunStatusApproved, false}, + {"executing", RunStatusExecuting, false}, + {"completed", RunStatusCompleted, false}, + {"failed", RunStatusFailed, false}, + {"cancelled", RunStatusCancelled, false}, //nolint:misspell // matches existing DB status spelling ("cancelled") + {"expired", RunStatusExpired, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseRunStatus(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseRunStatus(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseRunStatus(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseRunStatus(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestParseTrancheStatus(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want TrancheStatus + wantErr bool + }{ + {"scheduled", TrancheStatusScheduled, false}, + {"fired", TrancheStatusFired, false}, + {"completed", TrancheStatusCompleted, false}, + {"cancelled", TrancheStatusCancelled, false}, //nolint:misspell // matches existing DB status spelling ("cancelled") + {"failed", TrancheStatusFailed, false}, + {"bogus", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseTrancheStatus(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseTrancheStatus(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseTrancheStatus(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseTrancheStatus(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestScopeValidate(t *testing.T) { + t.Parallel() + cases := []struct { + name string + scope Scope + wantErr bool + }{ + {"valid aws", Scope{Provider: common.ProviderAWS, AccountID: "123456789012"}, false}, + {"valid azure", Scope{Provider: common.ProviderAzure, AccountID: "sub-id"}, false}, + {"valid gcp", Scope{Provider: common.ProviderGCP, AccountID: "my-project"}, false}, + {"unknown provider", Scope{Provider: "oracle", AccountID: "x"}, true}, + {"empty provider", Scope{Provider: "", AccountID: "x"}, true}, + {"empty account", Scope{Provider: common.ProviderAWS, AccountID: ""}, true}, + {"both empty", Scope{}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.scope.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} + +// validRamp returns a well-formed two-step RampSchedule for use in table +// tests. +func validRamp() RampSchedule { + return RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.5}, + {AfterDays: 7, Fraction: 0.5}, + }} +} + +func TestRampScheduleValidate(t *testing.T) { + t.Parallel() + cases := []struct { + name string + ramp RampSchedule + wantErr bool + }{ + { + name: "valid two-step", + ramp: validRamp(), + wantErr: false, + }, + { + name: "single step full fraction", + ramp: RampSchedule{Steps: []RampStep{{AfterDays: 0, Fraction: 1.0}}}, + wantErr: false, + }, + { + name: "empty steps", + ramp: RampSchedule{}, + wantErr: true, + }, + { + name: "unsorted steps", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 7, Fraction: 0.5}, + {AfterDays: 3, Fraction: 0.5}, + }}, + wantErr: true, + }, + { + name: "duplicate AfterDays", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.5}, + {AfterDays: 0, Fraction: 0.5}, + }}, + wantErr: true, + }, + { + name: "fraction sum less than 1", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.4}, + {AfterDays: 7, Fraction: 0.4}, + }}, + wantErr: true, + }, + { + name: "fraction sum greater than 1", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.7}, + {AfterDays: 7, Fraction: 0.7}, + }}, + wantErr: true, + }, + { + name: "zero fraction", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0}, + }}, + wantErr: true, + }, + { + name: "negative fraction", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: -0.1}, + }}, + wantErr: true, + }, + { + name: "fraction > 1", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 1.5}, + }}, + wantErr: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.ramp.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} + +func TestLadderConfigValidate(t *testing.T) { + t.Parallel() + // valid is a fully-populated valid config used as the baseline for each + // mutation. + valid := LadderConfig{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123456789012"}, + TargetCoveragePct: 80, + BufferFraction: 0.1, + BaselinePercentile: 5, + LookbackDays: 30, + Mode: ModeEmailApproval, + Cadence: CadenceWeekly, + Ramp: validRamp(), + MaxActionsPerRun: 10, + } + cases := []struct { + mutate func(c *LadderConfig) + name string + wantErr bool + }{ + {name: "valid", mutate: func(*LadderConfig) {}, wantErr: false}, + { + name: "bad scope provider", + mutate: func(c *LadderConfig) { c.Scope.Provider = "oracle" }, + wantErr: true, + }, + { + name: "empty scope account", + mutate: func(c *LadderConfig) { c.Scope.AccountID = "" }, + wantErr: true, + }, + { + name: "coverage zero", + mutate: func(c *LadderConfig) { c.TargetCoveragePct = 0 }, + wantErr: true, + }, + { + name: "coverage negative", + mutate: func(c *LadderConfig) { c.TargetCoveragePct = -1 }, + wantErr: true, + }, + { + name: "coverage above 100", + mutate: func(c *LadderConfig) { c.TargetCoveragePct = 100.01 }, + wantErr: true, + }, + { + name: "coverage exactly 100 is valid", + mutate: func(c *LadderConfig) { c.TargetCoveragePct = 100 }, + wantErr: false, + }, + { + name: "buffer fraction negative", + mutate: func(c *LadderConfig) { c.BufferFraction = -0.1 }, + wantErr: true, + }, + { + name: "buffer fraction exactly 1 is invalid", + mutate: func(c *LadderConfig) { c.BufferFraction = 1.0 }, + wantErr: true, + }, + { + name: "buffer fraction 0 is valid", + mutate: func(c *LadderConfig) { c.BufferFraction = 0 }, + wantErr: false, + }, + { + name: "percentile zero", + mutate: func(c *LadderConfig) { c.BaselinePercentile = 0 }, + wantErr: true, + }, + { + name: "percentile above 50", + mutate: func(c *LadderConfig) { c.BaselinePercentile = 50.01 }, + wantErr: true, + }, + { + name: "percentile exactly 50 is valid", + mutate: func(c *LadderConfig) { c.BaselinePercentile = 50 }, + wantErr: false, + }, + { + name: "lookback zero", + mutate: func(c *LadderConfig) { c.LookbackDays = 0 }, + wantErr: true, + }, + { + name: "lookback negative", + mutate: func(c *LadderConfig) { c.LookbackDays = -1 }, + wantErr: true, + }, + { + name: "bad mode", + mutate: func(c *LadderConfig) { c.Mode = "unknown_mode" }, + wantErr: true, + }, + { + name: "bad cadence", + mutate: func(c *LadderConfig) { c.Cadence = "monthly" }, + wantErr: true, + }, + { + name: "bad ramp (empty steps)", + mutate: func(c *LadderConfig) { c.Ramp = RampSchedule{} }, + wantErr: true, + }, + { + name: "hourly commit cap nil is valid (no cap)", + mutate: func(c *LadderConfig) { c.MaxHourlyCommitPerRun = nil }, + wantErr: false, + }, + { + name: "hourly commit cap positive is valid", + mutate: func(c *LadderConfig) { + hourlyCap := 12.5 + c.MaxHourlyCommitPerRun = &hourlyCap + }, + wantErr: false, + }, + { + name: "hourly commit cap zero is invalid", + mutate: func(c *LadderConfig) { + hourlyCap := 0.0 + c.MaxHourlyCommitPerRun = &hourlyCap + }, + wantErr: true, + }, + { + name: "hourly commit cap negative is invalid", + mutate: func(c *LadderConfig) { + hourlyCap := -1.0 + c.MaxHourlyCommitPerRun = &hourlyCap + }, + wantErr: true, + }, + { + name: "max actions zero", + mutate: func(c *LadderConfig) { c.MaxActionsPerRun = 0 }, + wantErr: true, + }, + { + name: "max actions negative", + mutate: func(c *LadderConfig) { c.MaxActionsPerRun = -1 }, + wantErr: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cfg := valid // shallow copy; all fields are value types or pointers + c.mutate(&cfg) + err := cfg.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} From 9750262925d009b9fa2f0abdb440f21227e8ced8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 2 Jul 2026 15:21:52 +0200 Subject: [PATCH 2/5] fix(ladder): enforce action invariants and address review findings - Enforce empty Term/PaymentOption on hold and reshape actions (doc already claimed the invariant); add negative test cases. - Fix PurchaseLayer godoc to reference the existing common.ErrCommitmentPurchaseNotSupported sentinel (errors.Is). - Drop the redundant runID parameter from LadderStore.SaveTranches; Tranche.RunID is the single source of run linkage. - Add RunRecord.Validate and Tranche.Validate with RunStatus.IsTerminal and FiredAt/CompletedAt lifecycle consistency checks, plus table tests. - Surface deduplicated action DataSources in LadderPlan.Explain output. - Format money via big.Rat FloatString(2) instead of Float64 so halfway cases round half away from zero (199/200 renders $1.00/hr). --- pkg/ladder/capability.go | 6 +- pkg/ladder/plan.go | 36 +++++- pkg/ladder/plan_test.go | 78 ++++++++++++- pkg/ladder/store.go | 74 ++++++++++++- pkg/ladder/store_test.go | 231 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 415 insertions(+), 10 deletions(-) create mode 100644 pkg/ladder/store_test.go diff --git a/pkg/ladder/capability.go b/pkg/ladder/capability.go index 55fa8231a..dcd877a50 100644 --- a/pkg/ladder/capability.go +++ b/pkg/ladder/capability.go @@ -35,8 +35,10 @@ type LadderCapability interface { GetUsageBaseline(ctx context.Context, scope Scope, lookbackDays int, percentile float64) (UsageBaseline, error) // PurchaseLayer buys commitments for the given layer using the provided - // recommendation. Returns ErrCommitmentPurchaseNotSupported when the - // provider has no programmatic purchase API for this layer. + // recommendation. Implementations return an error wrapping + // common.ErrCommitmentPurchaseNotSupported when the layer cannot be + // purchased programmatically; callers detect it with + // errors.Is(err, common.ErrCommitmentPurchaseNotSupported). // // Precision note: the engine plans amounts as exact *big.Rat values // (PlannedAction.AmountUSDPerHour), but this boundary converts them to diff --git a/pkg/ladder/plan.go b/pkg/ladder/plan.go index d5ae18494..1c5cc95f1 100644 --- a/pkg/ladder/plan.go +++ b/pkg/ladder/plan.go @@ -39,7 +39,8 @@ type PlannedAction struct { // - rationale must be non-empty // - purchase actions require a positive AmountUSDPerHour and non-empty // Term and PaymentOption (money-shaping fields must be present) -// - hold and reshape actions require a nil AmountUSDPerHour +// - hold and reshape actions require a nil AmountUSDPerHour and empty +// Term and PaymentOption func (a *PlannedAction) Validate() error { if err := a.Action.Validate(); err != nil { return fmt.Errorf("action: %w", err) @@ -58,6 +59,9 @@ func (a *PlannedAction) Validate() error { return fmt.Errorf("%s action requires nil AmountUSDPerHour, got %s", a.Action, a.AmountUSDPerHour.RatString()) } + if a.Term != "" || a.PaymentOption != "" { + return fmt.Errorf("%s action requires empty Term and PaymentOption", a.Action) + } } return nil } @@ -121,12 +125,15 @@ func (p *LadderPlan) Validate() error { // nil is rendered as "unknown" -- never as "$0.00" -- to unambiguously // distinguish absent data from a genuine zero commitment (project rule: // absent numbers are nil, not zero). +// +// FloatString(2) formats the exact rational with half-away-from-zero +// rounding, avoiding the float64 conversion step (e.g. 199/200 renders as +// $1.00/hr, not $0.99/hr). func formatUSDPerHour(v *big.Rat) string { if v == nil { return "unknown" } - f, _ := v.Float64() - return fmt.Sprintf("$%.2f/hr", f) + return fmt.Sprintf("$%s/hr", v.FloatString(2)) } // Explain returns a deterministic, human-readable multi-line summary of the @@ -138,6 +145,8 @@ func formatUSDPerHour(v *big.Rat) string { // 2. Baseline parameters (lookback window, percentile) // 3. Target / existing / gap hourly rates // 4. Numbered action list (or "none" when all layers are at target) +// 5. "Data sources:" line aggregating the actions' DataSources (omitted +// when no action carries any) func (p *LadderPlan) Explain() string { var b strings.Builder @@ -166,5 +175,26 @@ func (p *LadderPlan) Explain() string { i+1, a.Action, a.Layer, a.Rationale) } } + if sources := collectDataSources(p.Actions); len(sources) > 0 { + fmt.Fprintf(&b, "Data sources: %s\n", strings.Join(sources, ", ")) + } return b.String() } + +// collectDataSources aggregates the DataSources of all actions, +// deduplicated, preserving first-seen stored order so the output stays +// deterministic for a given plan. +func collectDataSources(actions []PlannedAction) []string { + var sources []string + seen := make(map[string]bool) + for _, a := range actions { + for _, s := range a.DataSources { + if seen[s] { + continue + } + seen[s] = true + sources = append(sources, s) + } + } + return sources +} diff --git a/pkg/ladder/plan_test.go b/pkg/ladder/plan_test.go index ef63b28c6..9896b8c6a 100644 --- a/pkg/ladder/plan_test.go +++ b/pkg/ladder/plan_test.go @@ -105,6 +105,42 @@ func TestPlannedActionValidate(t *testing.T) { }, wantErr: true, }, + { + name: "hold with Term set", + action: PlannedAction{ + Action: ActionHold, Layer: LayerConvertibleRI, + Term: "1yr", + Rationale: "at target", + }, + wantErr: true, + }, + { + name: "hold with PaymentOption set", + action: PlannedAction{ + Action: ActionHold, Layer: LayerConvertibleRI, + PaymentOption: "no-upfront", + Rationale: "at target", + }, + wantErr: true, + }, + { + name: "reshape with Term set", + action: PlannedAction{ + Action: ActionReshape, Layer: LayerConvertibleRI, + Term: "3yr", + Rationale: "low utilization", + }, + wantErr: true, + }, + { + name: "reshape with PaymentOption set", + action: PlannedAction{ + Action: ActionReshape, Layer: LayerConvertibleRI, + PaymentOption: "all-upfront", + Rationale: "low utilization", + }, + wantErr: true, + }, { name: "empty rationale", action: PlannedAction{ @@ -153,7 +189,10 @@ func TestFormatUSDPerHour(t *testing.T) { {new(big.Rat).SetFrac64(25, 100), "$0.25/hr"}, {new(big.Rat).SetInt64(10), "$10.00/hr"}, {new(big.Rat), "$0.00/hr"}, - {new(big.Rat).SetFrac64(1, 3), "$0.33/hr"}, // truncated at 2 decimals via Float64 + {new(big.Rat).SetFrac64(1, 3), "$0.33/hr"}, // rounded to 2 decimals via FloatString + // Halfway case: 199/200 = 0.995 must round half away from zero to + // $1.00/hr (FloatString semantics), not truncate to $0.99/hr. + {new(big.Rat).SetFrac64(199, 200), "$1.00/hr"}, } for _, c := range cases { got := formatUSDPerHour(c.in) @@ -224,6 +263,7 @@ func TestExplain_PurchaseAction(t *testing.T) { Term: "1yr", PaymentOption: "no-upfront", Rationale: "fill coverage gap", + DataSources: []string{"cost-explorer", "cloudwatch"}, }, }, } @@ -239,6 +279,7 @@ func TestExplain_PurchaseAction(t *testing.T) { "1yr", "no-upfront", "fill coverage gap", + "Data sources: cost-explorer, cloudwatch", } for _, want := range checks { if !strings.Contains(out, want) { @@ -271,6 +312,41 @@ func TestExplain_ReshapeAction(t *testing.T) { if !strings.Contains(out, "utilization below threshold") { t.Errorf("Explain() missing rationale:\n%s", out) } + if strings.Contains(out, "Data sources:") { + t.Errorf("Explain() rendered a Data sources line with no sources set:\n%s", out) + } +} + +// TestExplain_DataSourcesDeduplicated verifies that the Data sources line +// aggregates sources across actions in first-seen stored order, without +// duplicates. +func TestExplain_DataSourcesDeduplicated(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + Actions: []PlannedAction{ + { + Action: ActionHold, Layer: LayerConvertibleRI, + Rationale: "at target", + DataSources: []string{"cost-explorer", "cloudwatch"}, + }, + { + Action: ActionReshape, Layer: LayerConvertibleRI, + Rationale: "low utilization", + DataSources: []string{"cloudwatch", "pricing-api"}, + }, + }, + } + out := plan.Explain() + want := "Data sources: cost-explorer, cloudwatch, pricing-api\n" + if !strings.Contains(out, want) { + t.Errorf("Explain() missing deduplicated sources line %q:\n%s", want, out) + } + if strings.Count(out, "cloudwatch") != 1 { + t.Errorf("Explain() duplicated a data source:\n%s", out) + } } // TestExplain_Deterministic verifies that calling Explain() twice on the same diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index 4565ed1ce..7eaa8b0e0 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -31,6 +31,18 @@ func (s RunStatus) Validate() error { return fmt.Errorf("unknown run status %q", s) } +// IsTerminal reports whether s is a terminal run status: RunStatusCompleted, +// RunStatusFailed, RunStatusCancelled, or RunStatusExpired. Terminal runs are +// the only ones allowed to carry a CompletedAt timestamp (see +// RunRecord.Validate). +func (s RunStatus) IsTerminal() bool { + switch s { + case RunStatusCompleted, RunStatusFailed, RunStatusCancelled, RunStatusExpired: + return true + } + return false +} + // ParseRunStatus converts s into a RunStatus, returning a descriptive error // when s is not a recognized value. func ParseRunStatus(s string) (RunStatus, error) { @@ -62,6 +74,18 @@ func (s TrancheStatus) Validate() error { return fmt.Errorf("unknown tranche status %q", s) } +// allowsFiredAt reports whether a tranche in this status may carry a +// non-nil FiredAt timestamp: fired itself, or a post-firing outcome +// (completed, failed). Scheduled and canceled tranches never fired, so +// they must not carry one. +func (s TrancheStatus) allowsFiredAt() bool { + switch s { + case TrancheStatusFired, TrancheStatusCompleted, TrancheStatusFailed: + return true + } + return false +} + // ParseTrancheStatus converts s into a TrancheStatus, returning a // descriptive error when s is not a recognized value. func ParseTrancheStatus(s string) (TrancheStatus, error) { @@ -87,6 +111,22 @@ type RunRecord struct { PlanJSON string } +// Validate checks that the run record is self-consistent: non-empty ID, +// recognized status, and a CompletedAt timestamp only when the status is +// terminal (a run still in flight cannot have completed). +func (r *RunRecord) Validate() error { + if r.ID == "" { + return fmt.Errorf("run ID is required") + } + if err := r.Status.Validate(); err != nil { + return fmt.Errorf("status: %w", err) + } + if r.CompletedAt != nil && !r.Status.IsTerminal() { + return fmt.Errorf("completed_at must be nil for non-terminal status %q", r.Status) + } + return nil +} + // Tranche is one ramp step that has been persisted for scheduled firing. type Tranche struct { // FireAfter is the wall-clock time at or after which this tranche fires. @@ -99,6 +139,29 @@ type Tranche struct { StepIndex int } +// Validate checks that the tranche is self-consistent: non-empty ID and +// RunID (RunID is the single source of run linkage, see +// LadderStore.SaveTranches), non-negative step index, recognized status, +// and a FiredAt timestamp only when the status implies the tranche fired. +func (t *Tranche) Validate() error { + if t.ID == "" { + return fmt.Errorf("tranche ID is required") + } + if t.RunID == "" { + return fmt.Errorf("tranche RunID is required") + } + if t.StepIndex < 0 { + return fmt.Errorf("step_index %d must be >= 0", t.StepIndex) + } + if err := t.Status.Validate(); err != nil { + return fmt.Errorf("status: %w", err) + } + if t.FiredAt != nil && !t.Status.allowsFiredAt() { + return fmt.Errorf("fired_at must be nil for status %q (tranche has not fired)", t.Status) + } + return nil +} + // LadderStore is the storage contract for the ladder engine. The concrete // implementation lives in internal/ (separate Go module) and is injected // into the engine at startup; pkg/ defines only the interface. @@ -113,8 +176,11 @@ type LadderStore interface { // run for the given scope, or nil when no run has been recorded yet. LatestRunStartedAt(ctx context.Context, scope Scope) (*time.Time, error) - // SaveTranches persists a batch of tranches for the given run. Callers - // may call this multiple times (e.g., once per ramp step) and - // implementations should upsert by tranche ID. - SaveTranches(ctx context.Context, runID string, tranches []Tranche) error + // SaveTranches persists a batch of tranches. Every tranche must carry a + // non-empty RunID (Tranche.RunID is the single source of truth for run + // linkage); implementations persist tranches exactly as given and must + // not infer linkage from anything else. Callers may call this multiple + // times (e.g., once per ramp step) and implementations should upsert by + // tranche ID. + SaveTranches(ctx context.Context, tranches []Tranche) error } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go new file mode 100644 index 000000000..74eb21834 --- /dev/null +++ b/pkg/ladder/store_test.go @@ -0,0 +1,231 @@ +package ladder + +import ( + "testing" + "time" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +func TestRunStatusIsTerminal(t *testing.T) { + t.Parallel() + cases := []struct { + in RunStatus + want bool + }{ + {RunStatusPlanned, false}, + {RunStatusAwaitingApproval, false}, + {RunStatusApproved, false}, + {RunStatusExecuting, false}, + {RunStatusCompleted, true}, + {RunStatusFailed, true}, + {RunStatusCancelled, true}, + {RunStatusExpired, true}, + } + for _, c := range cases { + if got := c.in.IsTerminal(); got != c.want { + t.Errorf("RunStatus(%q).IsTerminal() = %v, want %v", c.in, got, c.want) + } + } +} + +func TestRunRecordValidate(t *testing.T) { + t.Parallel() + now := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + valid := RunRecord{ + ID: "run-1", + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123456789012"}, + Status: RunStatusPlanned, + CreatedAt: now, + } + cases := []struct { + mutate func(r *RunRecord) + name string + wantErr bool + }{ + {name: "valid planned", mutate: func(*RunRecord) {}, wantErr: false}, + { + name: "empty ID", + mutate: func(r *RunRecord) { r.ID = "" }, + wantErr: true, + }, + { + name: "unknown status", + mutate: func(r *RunRecord) { r.Status = "bogus" }, + wantErr: true, + }, + { + name: "completed with CompletedAt is valid", + mutate: func(r *RunRecord) { + r.Status = RunStatusCompleted + r.CompletedAt = &now + }, + wantErr: false, + }, + { + name: "failed with CompletedAt is valid", + mutate: func(r *RunRecord) { + r.Status = RunStatusFailed + r.CompletedAt = &now + }, + wantErr: false, + }, + { + name: "RunStatusCancelled with CompletedAt is valid", + mutate: func(r *RunRecord) { + r.Status = RunStatusCancelled + r.CompletedAt = &now + }, + wantErr: false, + }, + { + name: "expired with CompletedAt is valid", + mutate: func(r *RunRecord) { + r.Status = RunStatusExpired + r.CompletedAt = &now + }, + wantErr: false, + }, + { + name: "planned with CompletedAt is invalid", + mutate: func(r *RunRecord) { + r.CompletedAt = &now + }, + wantErr: true, + }, + { + name: "executing with CompletedAt is invalid", + mutate: func(r *RunRecord) { + r.Status = RunStatusExecuting + r.CompletedAt = &now + }, + wantErr: true, + }, + { + name: "awaiting_approval with CompletedAt is invalid", + mutate: func(r *RunRecord) { + r.Status = RunStatusAwaitingApproval + r.CompletedAt = &now + }, + wantErr: true, + }, + { + name: "completed with nil CompletedAt is valid (timestamp may lag)", + mutate: func(r *RunRecord) { + r.Status = RunStatusCompleted + }, + wantErr: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + rec := valid + c.mutate(&rec) + err := rec.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} + +func TestTrancheValidate(t *testing.T) { + t.Parallel() + now := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + valid := Tranche{ + ID: "tranche-1", + RunID: "run-1", + StepIndex: 0, + FireAfter: now, + Status: TrancheStatusScheduled, + } + cases := []struct { + mutate func(tr *Tranche) + name string + wantErr bool + }{ + {name: "valid scheduled", mutate: func(*Tranche) {}, wantErr: false}, + { + name: "empty ID", + mutate: func(tr *Tranche) { tr.ID = "" }, + wantErr: true, + }, + { + name: "empty RunID", + mutate: func(tr *Tranche) { tr.RunID = "" }, + wantErr: true, + }, + { + name: "negative StepIndex", + mutate: func(tr *Tranche) { tr.StepIndex = -1 }, + wantErr: true, + }, + { + name: "unknown status", + mutate: func(tr *Tranche) { tr.Status = "bogus" }, + wantErr: true, + }, + { + name: "fired with FiredAt is valid", + mutate: func(tr *Tranche) { + tr.Status = TrancheStatusFired + tr.FiredAt = &now + }, + wantErr: false, + }, + { + name: "completed with FiredAt is valid", + mutate: func(tr *Tranche) { + tr.Status = TrancheStatusCompleted + tr.FiredAt = &now + }, + wantErr: false, + }, + { + name: "failed with FiredAt is valid", + mutate: func(tr *Tranche) { + tr.Status = TrancheStatusFailed + tr.FiredAt = &now + }, + wantErr: false, + }, + { + name: "scheduled with FiredAt is invalid", + mutate: func(tr *Tranche) { + tr.FiredAt = &now + }, + wantErr: true, + }, + { + name: "TrancheStatusCancelled with FiredAt is invalid", + mutate: func(tr *Tranche) { + tr.Status = TrancheStatusCancelled + tr.FiredAt = &now + }, + wantErr: true, + }, + { + name: "fired with nil FiredAt is valid (timestamp may lag)", + mutate: func(tr *Tranche) { + tr.Status = TrancheStatusFired + }, + wantErr: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + tr := valid + c.mutate(&tr) + err := tr.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} From 0754d39245fdc177ab7e83c74c637d7592b2b8b4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 2 Jul 2026 15:31:10 +0200 Subject: [PATCH 3/5] fix(ladder): require FireAfter on tranches Tranche.Validate now rejects a zero FireAfter timestamp: a zero value would make the tranche immediately eligible to fire (year-1 timestamp is always in the past), silently defeating the ramp schedule. Add zero- rejected and set-accepted test cases. --- pkg/ladder/store.go | 11 +++++++++-- pkg/ladder/store_test.go | 10 ++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index 7eaa8b0e0..bb322fdf9 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -141,8 +141,9 @@ type Tranche struct { // Validate checks that the tranche is self-consistent: non-empty ID and // RunID (RunID is the single source of run linkage, see -// LadderStore.SaveTranches), non-negative step index, recognized status, -// and a FiredAt timestamp only when the status implies the tranche fired. +// LadderStore.SaveTranches), non-negative step index, a set FireAfter +// timestamp, recognized status, and a FiredAt timestamp only when the +// status implies the tranche fired. func (t *Tranche) Validate() error { if t.ID == "" { return fmt.Errorf("tranche ID is required") @@ -153,6 +154,12 @@ func (t *Tranche) Validate() error { if t.StepIndex < 0 { return fmt.Errorf("step_index %d must be >= 0", t.StepIndex) } + // A zero FireAfter would make the tranche immediately eligible to fire + // (year-1 timestamp is always in the past), silently defeating the ramp + // schedule. Fail loud instead. + if t.FireAfter.IsZero() { + return fmt.Errorf("fire_after must be set (zero time would fire immediately)") + } if err := t.Status.Validate(); err != nil { return fmt.Errorf("status: %w", err) } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go index 74eb21834..c0f94ee34 100644 --- a/pkg/ladder/store_test.go +++ b/pkg/ladder/store_test.go @@ -163,6 +163,16 @@ func TestTrancheValidate(t *testing.T) { mutate: func(tr *Tranche) { tr.StepIndex = -1 }, wantErr: true, }, + { + name: "zero FireAfter is invalid (would fire immediately)", + mutate: func(tr *Tranche) { tr.FireAfter = time.Time{} }, + wantErr: true, + }, + { + name: "set FireAfter is valid", + mutate: func(tr *Tranche) { tr.FireAfter = now.AddDate(0, 0, 7) }, + wantErr: false, + }, { name: "unknown status", mutate: func(tr *Tranche) { tr.Status = "bogus" }, From 2c21ea3fa969a487355820602230a597c67b795e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 15:38:43 +0200 Subject: [PATCH 4/5] fix(ladder): reject NaN in ramp fractions and config bounds NaN compares false against every bound, so exclusion-style range checks ("v <= 0 || v > 1") silently admitted NaN in RampStep.Fraction, TargetCoveragePct, BufferFraction, BaselinePercentile, and the MaxHourlyCommitPerRun deref. Downstream big.Rat.SetFloat64(NaN) returns nil and panics on first use. - Rewrite all float range checks in NaN-hostile inverted form with an explicit math.IsNaN guard (withinExclIncl helper for the (lo, hi] intervals). - Guard the ramp fraction sum against NaN as defense in depth. - Reject non-finite (NaN and +/-Inf) MaxHourlyCommitPerRun, since SetFloat64 returns nil for Inf as well. - Regression tests: NaN fraction in single- and multi-step schedules (asserting the step index in the error), NaN per config field, and infinite hourly cap. --- pkg/ladder/types.go | 42 ++++++++++++++++++----- pkg/ladder/types_test.go | 74 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 8 deletions(-) diff --git a/pkg/ladder/types.go b/pkg/ladder/types.go index dcc649f0f..4d28ffb08 100644 --- a/pkg/ladder/types.go +++ b/pkg/ladder/types.go @@ -6,6 +6,7 @@ package ladder import ( "fmt" + "math" "github.com/LeanerCloud/CUDly/pkg/common" ) @@ -254,11 +255,21 @@ func (r RampSchedule) Validate() error { return fmt.Errorf("ramp step %d: AfterDays %d must be strictly greater than previous %d", i, s.AfterDays, prev) } prev = s.AfterDays - if s.Fraction <= 0 || s.Fraction > 1 { + // NaN compares false against every bound, so the naive + // "<= 0 || > 1" form silently admits it (and big.Rat.SetFloat64 + // on NaN returns nil downstream, panicking on use). Reject NaN + // explicitly AND use the inverted in-range form, which is also + // NaN-hostile on its own. + if math.IsNaN(s.Fraction) || !(s.Fraction > 0 && s.Fraction <= 1) { return fmt.Errorf("ramp step %d: fraction %g must be in (0, 1]", i, s.Fraction) } sum += s.Fraction } + // Defense in depth: a NaN sum would sail through the epsilon window + // below because NaN comparisons are always false. + if math.IsNaN(sum) { + return fmt.Errorf("ramp step fractions sum to NaN") + } diff := sum - 1.0 if diff < -rampSumEpsilon || diff > rampSumEpsilon { return fmt.Errorf("ramp step fractions sum to %g, must equal 1.0", sum) @@ -309,17 +320,27 @@ func (c *LadderConfig) Validate() error { return c.validateRunLimits() } +// withinExclIncl reports whether v is a real (non-NaN) number in the +// half-open interval (lo, hi]. The positive in-range form rejects NaN by +// construction (every NaN comparison is false); the explicit IsNaN check is +// kept for readability. Exclusion-style checks like "v <= lo || v > hi" are +// NOT NaN-safe because both comparisons are false for NaN. +func withinExclIncl(v, lo, hi float64) bool { + return !math.IsNaN(v) && v > lo && v <= hi +} + // validateBaselineBounds checks the coverage target, buffer fraction, and // baseline measurement parameters. Split out of Validate to keep each -// function's cyclomatic complexity within the repo limit. +// function's cyclomatic complexity within the repo limit. All float checks +// are NaN-hostile (see withinExclIncl). func (c *LadderConfig) validateBaselineBounds() error { - if c.TargetCoveragePct <= 0 || c.TargetCoveragePct > 100 { + if !withinExclIncl(c.TargetCoveragePct, 0, 100) { return fmt.Errorf("target_coverage_pct %g must be in (0, 100]", c.TargetCoveragePct) } - if c.BufferFraction < 0 || c.BufferFraction >= 1 { + if math.IsNaN(c.BufferFraction) || !(c.BufferFraction >= 0 && c.BufferFraction < 1) { return fmt.Errorf("buffer_fraction %g must be in [0, 1)", c.BufferFraction) } - if c.BaselinePercentile <= 0 || c.BaselinePercentile > 50 { + if !withinExclIncl(c.BaselinePercentile, 0, 50) { return fmt.Errorf("baseline_percentile %g must be in (0, 50]", c.BaselinePercentile) } if c.LookbackDays <= 0 { @@ -328,10 +349,15 @@ func (c *LadderConfig) validateBaselineBounds() error { return nil } -// validateRunLimits checks the per-run spend and action caps. +// validateRunLimits checks the per-run spend and action caps. The hourly +// cap must be a positive finite number when set: NaN and +/-Inf are both +// rejected because big.Rat.SetFloat64 returns nil for non-finite values, +// which would panic downstream. func (c *LadderConfig) validateRunLimits() error { - if c.MaxHourlyCommitPerRun != nil && *c.MaxHourlyCommitPerRun <= 0 { - return fmt.Errorf("max_hourly_commit_per_run %g must be > 0 when set (leave nil for no cap)", *c.MaxHourlyCommitPerRun) + if v := c.MaxHourlyCommitPerRun; v != nil { + if math.IsNaN(*v) || math.IsInf(*v, 0) || !(*v > 0) { + return fmt.Errorf("max_hourly_commit_per_run %g must be a positive finite number when set (leave nil for no cap)", *v) + } } if c.MaxActionsPerRun <= 0 { return fmt.Errorf("max_actions_per_run %d must be > 0", c.MaxActionsPerRun) diff --git a/pkg/ladder/types_test.go b/pkg/ladder/types_test.go index fa2970d58..0afc0c417 100644 --- a/pkg/ladder/types_test.go +++ b/pkg/ladder/types_test.go @@ -1,6 +1,8 @@ package ladder import ( + "math" + "strings" "testing" "github.com/LeanerCloud/CUDly/pkg/common" @@ -435,6 +437,47 @@ func validRamp() RampSchedule { }} } +// TestRampScheduleValidate_NaN is the regression test for the NaN hole: +// NaN compares false against every bound, so the pre-fix "<= 0 || > 1" +// range check admitted NaN fractions, and big.Rat.SetFloat64(NaN) returns +// nil downstream, panicking on first use. The error must name the step +// index so a multi-step schedule pinpoints the bad tranche. +func TestRampScheduleValidate_NaN(t *testing.T) { + t.Parallel() + cases := []struct { + name string + wantStep string + ramp RampSchedule + }{ + { + name: "single NaN fraction", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: math.NaN()}, + }}, + wantStep: "ramp step 0", + }, + { + name: "NaN in multi-step schedule", + ramp: RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.5}, + {AfterDays: 7, Fraction: math.NaN()}, + }}, + wantStep: "ramp step 1", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.ramp.Validate() + if err == nil { + t.Fatalf("Validate() = nil, want error for NaN fraction") + } + if !strings.Contains(err.Error(), c.wantStep) { + t.Errorf("Validate() error %q does not name %q", err, c.wantStep) + } + }) + } +} + func TestRampScheduleValidate(t *testing.T) { t.Parallel() cases := []struct { @@ -659,6 +702,37 @@ func TestLadderConfigValidate(t *testing.T) { }, wantErr: true, }, + { + name: "NaN coverage is invalid", + mutate: func(c *LadderConfig) { c.TargetCoveragePct = math.NaN() }, + wantErr: true, + }, + { + name: "NaN buffer fraction is invalid", + mutate: func(c *LadderConfig) { c.BufferFraction = math.NaN() }, + wantErr: true, + }, + { + name: "NaN percentile is invalid", + mutate: func(c *LadderConfig) { c.BaselinePercentile = math.NaN() }, + wantErr: true, + }, + { + name: "NaN hourly commit cap is invalid", + mutate: func(c *LadderConfig) { + hourlyCap := math.NaN() + c.MaxHourlyCommitPerRun = &hourlyCap + }, + wantErr: true, + }, + { + name: "infinite hourly commit cap is invalid", + mutate: func(c *LadderConfig) { + hourlyCap := math.Inf(1) + c.MaxHourlyCommitPerRun = &hourlyCap + }, + wantErr: true, + }, { name: "max actions zero", mutate: func(c *LadderConfig) { c.MaxActionsPerRun = 0 }, From 904f3d14cf83c113972c07b227ac0900c09c311e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 15:46:10 +0200 Subject: [PATCH 5/5] fix(ladder): align capability contracts, add term/payment enums, harden email rendering - GetLayerStates doc now matches the enforced contract: explicit zeros for measured-zero money fields, nil reserved for unmeasured metrics. - SupportedLayers doc states the role-cardinality contract (exactly one flex, at most one base, at most one buffer, base+buffer merge only). - New typed enums Term (1yr/3yr) and PaymentOption (all-upfront/ partial-upfront/no-upfront) with Validate/Parse; PlannedAction adopts them and purchase validation checks enum validity instead of non-empty. - Explain() passes every interpolated free-form field through a new sanitizeLine helper (strips control chars) to prevent line spoofing in the approval email; "Actions: none" drops the speculative reason; percentile renders via %g. - RunRecord.Validate gains Scope validation and a zero-CreatedAt check; LadderPlan.Validate gains Scope validation and a zero-GeneratedAt check. - BufferReshapeConfig money caps switch *big.Rat -> *float64 for consistency with LadderConfig.MaxHourlyCommitPerRun (config-boundary floats, converted where exact math is needed). - Scope doc notes GCP validates but fails loud at layer validation until GCP layers exist. --- pkg/ladder/capability.go | 23 +++++++-- pkg/ladder/plan.go | 73 ++++++++++++++++++--------- pkg/ladder/plan_test.go | 105 ++++++++++++++++++++++++++++++++++++++ pkg/ladder/store.go | 15 ++++-- pkg/ladder/store_test.go | 15 ++++++ pkg/ladder/types.go | 58 +++++++++++++++++++++ pkg/ladder/types_test.go | 106 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 364 insertions(+), 31 deletions(-) diff --git a/pkg/ladder/capability.go b/pkg/ladder/capability.go index dcd877a50..3bbf7b42b 100644 --- a/pkg/ladder/capability.go +++ b/pkg/ladder/capability.go @@ -2,7 +2,6 @@ package ladder import ( "context" - "math/big" "github.com/LeanerCloud/CUDly/pkg/common" ) @@ -20,14 +19,22 @@ type LadderCapability interface { // SupportedLayers returns the commitment layers this provider can fulfill. // Each LayerSpec declares both the layer type and the roles it covers. + // + // Role-cardinality contract (enforced by the engine): the returned set + // must contain exactly one layer carrying RoleFlex, at most one carrying + // RoleBase, and at most one carrying RoleBuffer. The only permitted + // multi-role merge is base+buffer on a single layer (e.g. an Azure + // reservation serving both roles). SupportedLayers() []LayerSpec // ListCommitments returns all active commitments for the given scope. ListCommitments(ctx context.Context, scope Scope) ([]common.Commitment, error) // GetLayerStates returns a point-in-time snapshot for each supported layer. - // Layers with no active commitments are represented with nil pointer fields - // (never coerced to zero). + // Layers with no active commitments carry explicit zeros in + // ExistingUSDPerHour/ExpiringUSDPerHour; nil is reserved for genuinely + // unmeasured metrics (e.g. UtilizationPct on an empty layer) and is + // treated as missing data by the engine. GetLayerStates(ctx context.Context, scope Scope) (map[LayerType]LayerState, error) // GetUsageBaseline computes a statistical baseline from historical @@ -53,13 +60,19 @@ type LadderCapability interface { } // BufferReshapeConfig parameterizes a buffer reshape operation. +// +// The money caps are config-boundary floats, consistent with +// LadderConfig.MaxHourlyCommitPerRun (implementations convert to +// pkg/exchange float64 anyway). Convert via ratFromFloat where exact math +// is needed; NaN/Inf/non-positive values are rejected wherever these caps +// are validated. type BufferReshapeConfig struct { // MaxPaymentPerExchangeUSD caps the payment for a single exchange. nil // means no per-exchange cap is applied. - MaxPaymentPerExchangeUSD *big.Rat + MaxPaymentPerExchangeUSD *float64 // MaxPaymentDailyUSD caps the total exchange payments for the current day. // nil means no daily cap is applied. - MaxPaymentDailyUSD *big.Rat + MaxPaymentDailyUSD *float64 // UtilizationThresholdPct triggers a reshape when commitment utilization // drops below this percentage. UtilizationThresholdPct float64 diff --git a/pkg/ladder/plan.go b/pkg/ladder/plan.go index 1c5cc95f1..ede7b0c13 100644 --- a/pkg/ladder/plan.go +++ b/pkg/ladder/plan.go @@ -5,6 +5,7 @@ import ( "math/big" "strings" "time" + "unicode" ) // PlannedAction is a single commitment action proposed by the ladder engine. @@ -20,12 +21,12 @@ type PlannedAction struct { // ActionReshape (whose financial impact is implicit in the underlying // exchange operation). AmountUSDPerHour *big.Rat - // Term is the commitment term (e.g., "1yr", "3yr"). Populated for purchase + // Term is the commitment term. Must be a valid Term for purchase // actions; empty for hold and reshape. - Term string - // PaymentOption is the payment structure (e.g., "all-upfront", - // "no-upfront"). Populated for purchase actions; empty for hold and reshape. - PaymentOption string + Term Term + // PaymentOption is the payment structure. Must be a valid PaymentOption + // for purchase actions; empty for hold and reshape. + PaymentOption PaymentOption // Rationale is a human-readable explanation of why this action was chosen. // Must be non-empty for all action types. Rationale string @@ -37,8 +38,8 @@ type PlannedAction struct { // Validate checks that the action is self-consistent: // - action type and layer type must be recognized // - rationale must be non-empty -// - purchase actions require a positive AmountUSDPerHour and non-empty -// Term and PaymentOption (money-shaping fields must be present) +// - purchase actions require a positive AmountUSDPerHour and valid Term +// and PaymentOption enum values (money-shaping fields must be present) // - hold and reshape actions require a nil AmountUSDPerHour and empty // Term and PaymentOption func (a *PlannedAction) Validate() error { @@ -78,14 +79,13 @@ func (a *PlannedAction) validatePurchaseFields() error { a.AmountUSDPerHour.RatString()) } // Term and PaymentOption shape the money committed; a purchase with - // either missing would silently default at the provider boundary. - // pkg/common has no typed Term/PaymentOption enums yet, so non-empty - // is the strongest check available here. - if a.Term == "" { - return fmt.Errorf("purchase action requires a non-empty Term") + // either missing or unrecognized would silently default at the provider + // boundary. Validate against the typed enums. + if err := a.Term.Validate(); err != nil { + return fmt.Errorf("purchase action term: %w", err) } - if a.PaymentOption == "" { - return fmt.Errorf("purchase action requires a non-empty PaymentOption") + if err := a.PaymentOption.Validate(); err != nil { + return fmt.Errorf("purchase action payment option: %w", err) } return nil } @@ -111,8 +111,17 @@ type LadderPlan struct { Baseline UsageBaseline } -// Validate checks that all PlannedActions in the plan are self-consistent. +// Validate checks that the plan is self-consistent: a valid scope, a set +// generation timestamp, and all PlannedActions valid. func (p *LadderPlan) Validate() error { + if err := p.Scope.Validate(); err != nil { + return fmt.Errorf("scope: %w", err) + } + // A zero GeneratedAt would corrupt approval-expiry and audit ordering + // downstream; fail loud like Tranche.FireAfter. + if p.GeneratedAt.IsZero() { + return fmt.Errorf("generated_at must be set") + } for i, a := range p.Actions { if err := a.Validate(); err != nil { return fmt.Errorf("action[%d]: %w", i, err) @@ -121,6 +130,19 @@ func (p *LadderPlan) Validate() error { return nil } +// sanitizeLine strips control characters (including \n and \r) from a value +// interpolated into a single Explain line. Without this, a crafted field +// (e.g. a Rationale containing "\n 2. fake action") could spoof extra lines +// in the approval email body. +func sanitizeLine(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return -1 + } + return r + }, s) +} + // formatUSDPerHour renders a *big.Rat as a fixed 2-decimal USD/hr string. // nil is rendered as "unknown" -- never as "$0.00" -- to unambiguously // distinguish absent data from a genuine zero commitment (project rule: @@ -144,22 +166,25 @@ func formatUSDPerHour(v *big.Rat) string { // 1. Scope header and generation timestamp // 2. Baseline parameters (lookback window, percentile) // 3. Target / existing / gap hourly rates -// 4. Numbered action list (or "none" when all layers are at target) +// 4. Numbered action list (or "none") // 5. "Data sources:" line aggregating the actions' DataSources (omitted // when no action carries any) +// +// Every interpolated free-form field is passed through sanitizeLine so a +// crafted value cannot spoof additional lines in the email body. func (p *LadderPlan) Explain() string { var b strings.Builder - fmt.Fprintf(&b, "Scope: provider=%s account=%s\n", p.Scope.Provider, p.Scope.AccountID) + fmt.Fprintf(&b, "Scope: provider=%s account=%s\n", p.Scope.Provider, sanitizeLine(p.Scope.AccountID)) fmt.Fprintf(&b, "Generated: %s\n", p.GeneratedAt.UTC().Format(time.RFC3339)) - fmt.Fprintf(&b, "Baseline: lookback=%dd percentile=%.0f\n", + fmt.Fprintf(&b, "Baseline: lookback=%dd percentile=%g\n", p.Baseline.LookbackDays, p.Baseline.Percentile) fmt.Fprintf(&b, "Target: %s\n", formatUSDPerHour(p.TargetUSDPerHour)) fmt.Fprintf(&b, "Existing: %s\n", formatUSDPerHour(p.ExistingUSDPerHour)) fmt.Fprintf(&b, "Gap: %s\n", formatUSDPerHour(p.GapUSDPerHour)) if len(p.Actions) == 0 { - fmt.Fprintf(&b, "Actions: none (all layers at target)\n") + fmt.Fprintf(&b, "Actions: none\n") return b.String() } @@ -169,10 +194,11 @@ func (p *LadderPlan) Explain() string { case ActionPurchase: fmt.Fprintf(&b, " %d. %s %s %s term=%s payment=%s -- %s\n", i+1, a.Action, a.Layer, formatUSDPerHour(a.AmountUSDPerHour), - a.Term, a.PaymentOption, a.Rationale) + sanitizeLine(string(a.Term)), sanitizeLine(string(a.PaymentOption)), + sanitizeLine(a.Rationale)) default: fmt.Fprintf(&b, " %d. %s %s -- %s\n", - i+1, a.Action, a.Layer, a.Rationale) + i+1, a.Action, a.Layer, sanitizeLine(a.Rationale)) } } if sources := collectDataSources(p.Actions); len(sources) > 0 { @@ -181,14 +207,15 @@ func (p *LadderPlan) Explain() string { return b.String() } -// collectDataSources aggregates the DataSources of all actions, -// deduplicated, preserving first-seen stored order so the output stays +// collectDataSources aggregates the DataSources of all actions, sanitized +// and deduplicated, preserving first-seen stored order so the output stays // deterministic for a given plan. func collectDataSources(actions []PlannedAction) []string { var sources []string seen := make(map[string]bool) for _, a := range actions { for _, s := range a.DataSources { + s = sanitizeLine(s) if seen[s] { continue } diff --git a/pkg/ladder/plan_test.go b/pkg/ladder/plan_test.go index 9896b8c6a..55fd7b4f5 100644 --- a/pkg/ladder/plan_test.go +++ b/pkg/ladder/plan_test.go @@ -87,6 +87,24 @@ func TestPlannedActionValidate(t *testing.T) { }, wantErr: true, }, + { + name: "purchase with unknown term", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: amount, Term: "2yr", + PaymentOption: "no-upfront", Rationale: "gap fill", + }, + wantErr: true, + }, + { + name: "purchase with unknown payment option", + action: PlannedAction{ + Action: ActionPurchase, Layer: LayerComputeSP, + AmountUSDPerHour: amount, Term: "1yr", + PaymentOption: "monthly", Rationale: "gap fill", + }, + wantErr: true, + }, { name: "hold with non-nil amount", action: PlannedAction{ @@ -365,3 +383,90 @@ func TestExplain_Deterministic(t *testing.T) { t.Errorf("Explain() is not deterministic:\nfirst:\n%s\nsecond:\n%s", first, second) } } + +func TestLadderPlanValidate(t *testing.T) { + t.Parallel() + valid := LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123456789012"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Actions: []PlannedAction{ + {Action: ActionHold, Layer: LayerConvertibleRI, Rationale: "at target"}, + }, + } + cases := []struct { + mutate func(p *LadderPlan) + name string + wantErr bool + }{ + {name: "valid", mutate: func(*LadderPlan) {}, wantErr: false}, + { + name: "unknown scope provider", + mutate: func(p *LadderPlan) { p.Scope.Provider = "oracle" }, + wantErr: true, + }, + { + name: "empty scope account", + mutate: func(p *LadderPlan) { p.Scope.AccountID = "" }, + wantErr: true, + }, + { + name: "zero GeneratedAt is invalid", + mutate: func(p *LadderPlan) { p.GeneratedAt = time.Time{} }, + wantErr: true, + }, + { + name: "invalid action propagates", + mutate: func(p *LadderPlan) { + p.Actions = []PlannedAction{ + {Action: ActionHold, Layer: LayerConvertibleRI, Rationale: ""}, + } + }, + wantErr: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + plan := valid + c.mutate(&plan) + err := plan.Validate() + if c.wantErr && err == nil { + t.Errorf("Validate() = nil, want error") + } + if !c.wantErr && err != nil { + t.Errorf("Validate() = %v, want nil", err) + } + }) + } +} + +// TestExplain_SanitizesControlChars verifies that control characters in +// interpolated fields cannot spoof extra lines in the approval email: a +// rationale containing an embedded newline plus a fake action line must +// render on a single line with the control characters stripped. +func TestExplain_SanitizesControlChars(t *testing.T) { + t.Parallel() + plan := &LadderPlan{ + Scope: Scope{Provider: common.ProviderAWS, AccountID: "123\naccount-spoof"}, + GeneratedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Baseline: UsageBaseline{LookbackDays: 30, Percentile: 5}, + Actions: []PlannedAction{ + { + Action: ActionHold, + Layer: LayerConvertibleRI, + Rationale: "legit rationale\n 2. fake action\r", + }, + }, + } + out := plan.Explain() + if strings.Contains(out, "\n 2. fake action") { + t.Errorf("Explain() allowed a spoofed action line:\n%s", out) + } + // The embedded newline and carriage return are stripped, joining the + // injected text onto the legitimate rationale's single line. + if !strings.Contains(out, "legit rationale 2. fake action") { + t.Errorf("Explain() did not render sanitized rationale on one line:\n%s", out) + } + if !strings.Contains(out, "account=123account-spoof\n") { + t.Errorf("Explain() did not sanitize AccountID:\n%s", out) + } +} diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index bb322fdf9..6f2b86cc8 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -111,13 +111,22 @@ type RunRecord struct { PlanJSON string } -// Validate checks that the run record is self-consistent: non-empty ID, -// recognized status, and a CompletedAt timestamp only when the status is -// terminal (a run still in flight cannot have completed). +// Validate checks that the run record is self-consistent: non-empty ID, a +// valid scope, a set CreatedAt timestamp, recognized status, and a +// CompletedAt timestamp only when the status is terminal (a run still in +// flight cannot have completed). func (r *RunRecord) Validate() error { if r.ID == "" { return fmt.Errorf("run ID is required") } + if err := r.Scope.Validate(); err != nil { + return fmt.Errorf("scope: %w", err) + } + // A zero CreatedAt would corrupt the cadence gate (LatestRunStartedAt + // comparisons); fail loud like Tranche.FireAfter. + if r.CreatedAt.IsZero() { + return fmt.Errorf("created_at must be set") + } if err := r.Status.Validate(); err != nil { return fmt.Errorf("status: %w", err) } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go index c0f94ee34..395380372 100644 --- a/pkg/ladder/store_test.go +++ b/pkg/ladder/store_test.go @@ -54,6 +54,21 @@ func TestRunRecordValidate(t *testing.T) { mutate: func(r *RunRecord) { r.Status = "bogus" }, wantErr: true, }, + { + name: "unknown scope provider", + mutate: func(r *RunRecord) { r.Scope.Provider = "oracle" }, + wantErr: true, + }, + { + name: "empty scope account", + mutate: func(r *RunRecord) { r.Scope.AccountID = "" }, + wantErr: true, + }, + { + name: "zero CreatedAt is invalid (cadence-gate integrity)", + mutate: func(r *RunRecord) { r.CreatedAt = time.Time{} }, + wantErr: true, + }, { name: "completed with CompletedAt is valid", mutate: func(r *RunRecord) { diff --git a/pkg/ladder/types.go b/pkg/ladder/types.go index 4d28ffb08..0b2753b51 100644 --- a/pkg/ladder/types.go +++ b/pkg/ladder/types.go @@ -158,6 +158,61 @@ func ParseLadderCadence(s string) (LadderCadence, error) { return c, nil } +// Term is the commitment duration of a purchase action. +type Term string + +const ( + Term1Year Term = "1yr" + Term3Year Term = "3yr" +) + +// Validate returns an error when t is not a recognized Term. +func (t Term) Validate() error { + switch t { + case Term1Year, Term3Year: + return nil + } + return fmt.Errorf("unknown term %q", t) +} + +// ParseTerm converts s into a Term, returning a descriptive error when s is +// not a recognized value. +func ParseTerm(s string) (Term, error) { + t := Term(s) + if err := t.Validate(); err != nil { + return "", err + } + return t, nil +} + +// PaymentOption is the payment structure of a purchase action. +type PaymentOption string + +const ( + PaymentAllUpfront PaymentOption = "all-upfront" + PaymentPartialUpfront PaymentOption = "partial-upfront" + PaymentNoUpfront PaymentOption = "no-upfront" +) + +// Validate returns an error when p is not a recognized PaymentOption. +func (p PaymentOption) Validate() error { + switch p { + case PaymentAllUpfront, PaymentPartialUpfront, PaymentNoUpfront: + return nil + } + return fmt.Errorf("unknown payment option %q", p) +} + +// ParsePaymentOption converts s into a PaymentOption, returning a +// descriptive error when s is not a recognized value. +func ParsePaymentOption(s string) (PaymentOption, error) { + p := PaymentOption(s) + if err := p.Validate(); err != nil { + return "", err + } + return p, nil +} + // LayerSpec describes a layer and the roles it fulfills within the ladder. // Azure reservations carry both base and buffer roles simultaneously. type LayerSpec struct { @@ -167,6 +222,9 @@ type LayerSpec struct { // Scope identifies the ladder scope: a specific provider account or // subscription that the ladder engine operates on. +// +// Note: GCP scopes validate here, but no GCP LayerType exists yet, so every +// GCP run fails loud at layer validation until GCP layers land. type Scope struct { Provider common.ProviderType AccountID string diff --git a/pkg/ladder/types_test.go b/pkg/ladder/types_test.go index 0afc0c417..6d897755c 100644 --- a/pkg/ladder/types_test.go +++ b/pkg/ladder/types_test.go @@ -331,6 +331,112 @@ func TestParseLadderCadence(t *testing.T) { } } +func TestTermValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in Term + wantErr bool + }{ + {Term1Year, false}, + {Term3Year, false}, + {"2yr", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("Term(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("Term(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestParseTerm(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want Term + wantErr bool + }{ + {"1yr", Term1Year, false}, + {"3yr", Term3Year, false}, + {"2yr", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParseTerm(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseTerm(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseTerm(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseTerm(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestPaymentOptionValidate(t *testing.T) { + t.Parallel() + cases := []struct { + in PaymentOption + wantErr bool + }{ + {PaymentAllUpfront, false}, + {PaymentPartialUpfront, false}, + {PaymentNoUpfront, false}, + {"monthly", true}, + {"", true}, + } + for _, c := range cases { + err := c.in.Validate() + if c.wantErr && err == nil { + t.Errorf("PaymentOption(%q).Validate() = nil, want error", c.in) + } + if !c.wantErr && err != nil { + t.Errorf("PaymentOption(%q).Validate() = %v, want nil", c.in, err) + } + } +} + +func TestParsePaymentOption(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want PaymentOption + wantErr bool + }{ + {"all-upfront", PaymentAllUpfront, false}, + {"partial-upfront", PaymentPartialUpfront, false}, + {"no-upfront", PaymentNoUpfront, false}, + {"monthly", "", true}, + {"", "", true}, + } + for _, c := range cases { + got, err := ParsePaymentOption(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParsePaymentOption(%q) = %q, nil; want error", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParsePaymentOption(%q) error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParsePaymentOption(%q) = %q, want %q", c.in, got, c.want) + } + } +} + func TestParseRunStatus(t *testing.T) { t.Parallel() cases := []struct {