ADORSYS-GIS / ADORSYS-GIS/ai-helm

[Ticket]: Phase 6a — re-key monthly budget to the append-only x-budget-tier ladder

Aperta
#877 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
governance P0 ticket
Lingua principale
Go Template
Stelle
3
Fork
1
Merge medio
19h 24m
PR unite (30g)
80

Descrizione

### Type

Feature

### Summary

We need to **replace the per-plan monthly budget rules with an append-only `x-budget-tier` ladder, and stamp every account onto its plan's base rung**, because refills (epic ADORSYS-GIS/lightbridge-authz#188) work by moving an account between rungs — and that mechanism has to exist before any refill can take effect.

Expected result:
> Behaviour is **identical to today**. Every account keeps the same effective budget. Only the counter key changes. No user notices anything.

⚠️ **This is the most dangerous ticket on the governance roadmap.** Read all of it before starting.

### Intent

Phase 6a of the budget epic: the mechanical re-key, deliberately separated from phase 6b (where grants actually move rungs) so the risky migration is not chained to the ledger work.

It needs nothing from phases 1–5 and may go first — it simply no longer has to.

### Source of truth (links)

- Epic: ADORSYS-GIS/lightbridge-authz#188
- Roadmap: ADORSYS-GIS/ai-helm#872 §4 Wave 4
- Plan detail: `plans/lightbridge-dynamic-budget.md` §0.2 (the ladder), §0.3–§0.4 (**the amended schedule**)
- **The incident that governs this work: ADR-0084**
- The mechanism being mirrored: ADR-0110 (project quota tiers, append-only)
- The ADRs that changed this ticket's schedule: ADR-0111, ADR-0112
- Needs its own ADR (reserved: **0117**)

### Current Behavior

Monthly budget is a per-plan rate-limit rule. The Lyft ratelimit service keys each counter in redis-ha on the rule's **position** in the rendered `rateLimit.global.rules` list — not on the plan name, because `Exact`-match headers render as masked constants.

ADR-0084 exists because adding an `enterprise` plan to a Helm **map** sorted it to index 0 mid-window, shifted `free`/`pro` to new indices, and **orphaned every account's accumulated monthly spend**. That was a live incident, confirmed by `SCAN`, not a theory.

### Expected Behavior

- A new, separately ordered, append-only `x-budget-tier` rule family, rendered **strictly after** the ADR-0110 quota-tier rules so **no existing rule index shifts**.
- Every account stamped onto its plan's base rung via the Keycloak attribute → protocol mapper → claim → Authorino CEL path.
- Effective budgets unchanged.
- The quota dashboard keeps working.

### ⚠️ Scheduling — the original date was wrong, and why

This ticket was planned to ship **2026-08-01**, four days before a window boundary at 2026-08-05 that would "absorb the reset". **That boundary no longer exists.**

- **ADR-0111** (2026-07-31) folded a calendar `YYYY-MM` `x-billing-period` marker into the key.
- **ADR-0112** (2026-08-01) then set **`unit: Year`**, making that marker the *only* rotation — because ADR-0111 alone left the 30-day epoch in the key as well, so counters rotated twice and every account was silently handed a spurious extra budget ~12×/year. ADR-0112's own Context records the live `SCAN` evidence: one account holding counters under two window epochs.

**Consequence: shipping this on the original rationale would have orphaned every account's August spend with no compensating reset until 2026-09-01.**

> **Revised target: a calendar month boundary — 2026-09-01 00:00 UTC, or any later 1st.**

Landing on a boundary is also no longer a precision deploy: rotation is driven by the header value, not elapsed time, so a deploy in the first hours of the 1st lands in a window whose counters are near zero anyway. Being a few hours late costs a few hours of spend, not a fleet-wide orphaning.

### Acceptance Criteria

- [ ] Given the rendered `BackendTrafficPolicy`, when it is diffed against the current one, then the **existing** rule indices are **unchanged** — new tier rules are appended only. Verified by diffing `helm template` output, not by reading the values file.
- [ ] Given every account, when the change is deployed, then each lands on its plan's base rung and its **effective budget is unchanged**.
- [ ] Given the Keycloak attribute → protocol mapper → claim → Authorino CEL path, when a token is issued, then `x-budget-tier` is stamped correctly; an account with **no** attribute falls back to its plan's base rung (**fail closed**, never to a higher rung).
- [ ] ⚠️ Given the redis-exporter, when the rules change, then `REDIS_EXPORTER_CHECK_KEYS`, the `ServiceMonitor` `metricRelabelings`, and `tools/dashboards/.../ratelimit_quota.py` are updated **in the same PR**. Missing this makes `gateway_ratelimit_spend_micro_usd` go silent and the `ratelimit-quota` dashboard go blank.
- [ ] ⚠️ That co-change is written against the **`billing_period`** label. The plan originally said "keep `window` as a label" — that predates ADR-0111/0112, and re-introducing a `window` label is now a **regression** against what `ea1c81b`, `3b1c69d`, `1d8743e` and #866 already shipped.
- [ ] Given the ladder, when a rung is added later, then it is **appended**. No rung is ever reordered or removed.
- [ ] Given the deploy, when it completes, then a `SCAN` of ratelimit keys confirms counters landed on the expected new keys and no account's spend was orphaned.
- [ ] ADR-0117 is written and merged with this change.
- [ ] It lands on a **calendar month boundary**.

### Out of Scope

- **Phase 6b** — grants writing the tier attribute. Separate ticket, no schedule constraint.
- Refill workflows, ledger, policy engine — all in `lightbridge-authz`.
- **Touching the ADR-0110 quota-tier rules.** Different axis, different header (`x-quota-tier`), owned by epic #531. Append after them; do not modify them.
- Any change to effective budget amounts. Behaviour is identical; this is a re-key.

### Technical Context

- `charts/ai-model/templates/backendtrafficpolicy.yaml` — the rule list. Also `charts/core-gateway/templates/backendtrafficpolicy.yaml` for the shared plan budget.
- The ladder (plan §0.2): `b-15 … b-1000`, roughly doubling, so a rung is always a meaningful jump. `b-1000` is where enterprise starts.
- Values live in `ai-helm-values` — **values-repo-first**, or `ignoreMissingValueFiles` silently falls back to chart defaults.
- ⚠️ `x-budget-tier` is deliberately a **separate** header from ADR-0110's `x-quota-tier`. Two rule families keyed on the same account is a real cost; collapsing them is the alternative and is worth arguing before ADR-0117 is written.

### Risks

| Risk | Probability | Impact | Mitigation |
|---|---|---|---|
| **Rule reorder orphans fleet-wide spend (ADR-0084 repeat)** | Low with care | 🔴 Critical | Append-only; `fail`-guard on missing `id`; diff rendered output; land on a boundary |
| **Redis-exporter/dashboard co-change forgotten → quota board blank** | **High** | Medium | Explicit acceptance criterion; same PR |
| The co-change written against `window` instead of `billing_period` | **Medium** | Medium | Called out explicitly above; check against `main` first |
| Deployed mid-month | Medium | High | Boundary-scheduled; do not "just ship it" |
| Accounts with no tier attribute get an unintended rung | Medium | High | Fail closed to the plan's base rung; test it |

### Test Plan

1. `helm template` before and after; **diff the rendered rules** and confirm no existing index moved.
2. Render with the `ci/lint-values.yaml` fixture (this chart is `lint-mode: ci-values`).
3. Verify each rule's resolved `rule/N` matches its comment.
4. In a non-production or dry-run context, verify the Authorino CEL stamp for: account with attribute; account without; unknown value.
5. Immediately before deploy: `SCAN` the ratelimit keys and record the current key shape — **do not trust any document for this**.
6. After deploy: `SCAN` again; confirm expected new keys and no orphaning.
7. Confirm the quota dashboard still renders and `gateway_ratelimit_spend_micro_usd` is still being produced.

### Verification evidence

*To be completed on delivery — this ticket must not be closed without it.*

Required:
- The rendered-rules diff, showing existing indices unchanged.
- `SCAN` output from **before and after** the deploy.
- A screenshot of the `ratelimit-quota` dashboard after the change, showing data.
- Evidence of the fail-closed fallback for an account with no attribute.
- The deploy timestamp, showing it landed on a month boundary.
- Link to the merged ADR-0117.

### Human accountable owner

@stephane-segning

### AI Usage Declaration

Drafting the ticket

### Human verification completed

- [x] I understood the intent
- [x] I checked the source of truth
- [x] I documented remaining risks
- [x] I am the accountable owner and accept responsibility for this ticket.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.