Repository navigation
Fix review findings in the v0.1 controller #18
Description
Review findings on the first Go version of the controller (#2, at ca59be5). Each one was reproduced with the aisix v1.9.0 validator, envtest or Helm, and each sub-issue carries the scenario, impact, proposed fix and tests to add.
Findings
Scores are out of 10. Higher is worse for likelihood and severity. Higher is better for mitigations, detectability, test coverage and cost vs benefit (10 = cheap fix, high value). Every finding reproduced deterministically (reproducibility 10).
| Issue | Finding | Likely | Severity | Mitig. | Detect | Tests | Cost/benefit |
|---|---|---|---|---|---|---|---|
| #4 | Model served on another ProviderKey's credential after a rename | 3 | 6 | 2 | 3 | 2 | 6 |
| #5 | Deleting or renaming a Model removes every APIKey that names it | 6 | 6 | 2 | 5 | 1 | 7 |
| #6 | Deleting an APIKey's Secret does not revoke the key | 4 | 7 | 3 | 4 | 2 | 8 |
| #7 | Gateway rolls onto an empty resources file | 4 | 9 | 3 | 3 | 2 | 8 |
| #8 | Newline in a display_name forges validator report lines | 2 | 6 | 3 | 4 | 1 | 8 |
| #9 | Duplicate key_hash swaps caller identity | 2 | 6 | 2 | 4 | 1 | 8 |
| #10 | Edit bursts over 1 s roll the gateway once per second | 5 | 3 | 2 | 5 | 1 | 6 |
| #11 | Terminating resources keep being served | 2 | 6 | 3 | 3 | 2 | 9 |
| #12 | Semantic routers with a classifier can never be accepted | 4 | 3 | 1 | 8 | 1 | 8 |
| #13 | One deeply nested resource freezes the whole file | 2 | 6 | 1 | 6 | 2 | 8 |
| #14 | Quick re-create inherits the deleted resource's entry | 2 | 5 | 4 | 4 | 1 | 6 |
| #15 | Failed Secret write or stamp leaves statuses stale | 3 | 4 | 3 | 3 | 1 | 8 |
| #16 | Second WithPredicates call drops GenerationChangedPredicate | 8 | 1 | 7 | 2 | 1 | 9 |
| #17 | make test passes when the envtest asset fetch fails | 2 | 4 | 3 | 3 | 0 | 9 |
Suggested order
- 1. Outage and revocation gaps, each small: Gateway can roll onto an empty resources file (deleted Secret, chart Secret adoption) #7, Deleting an APIKey's Secret does not revoke the key #6, Terminating resources held by a finalizer keep being served #11
- 2. A few lines each, bundle them: Second WithPredicates call drops GenerationChangedPredicate #16, make test passes when the envtest asset fetch fails #17
- 3. One ProviderKey reference resolver for both refs: Model can be served on another ProviderKey's credential after a rename #4 with Semantic routers with a classifier can never be accepted #12
- 4. Reference pruning and the key_hash pre-check: Deleting or renaming a Model removes every APIKey that names it #5, Duplicate key_hash demotes the unchanged APIKey and swaps caller identity #9
- 5. Render-time input limits (control characters, nesting depth): Newline in a display_name forges validator report lines #8, One deeply nested resource freezes the whole resources file #13
- 6. Status, debounce, UID-keyed index: A failed Secret write or Deployment stamp leaves statuses stale #15, Edit bursts longer than 1s roll the gateway once per second #10, Delete and quick re-create inherits the deleted resource's served entry #14
Activity