mirror of
https://github.com/coralogix/cx-cli.git
synced 2026-09-14 16:15:45 +08:00
Tristan Watson
a394c6f206
## Context
`cx iam api-keys list` has been broken since the command landed: it
requests `GET /mgmt/openapi/5/aaa/api-keys/v3/list`, which the AAA
service routes to `GET /aaa/api-keys/v3/{key_id}`. The literal string
`list` is parsed as a key ID and the request fails with `400 Bad
Request: key_id invalid character: found `l` at 1`. Reported against cx
0.1.14 in #182; the sibling api-keys subcommands are unaffected.
## Linked Issues
Fixes #182.
## Design
Single-line endpoint correction in `src/commands/api_keys/api.rs`. The
documented operation `ApiKeysService_GetApiKeys` lives at `GET
/aaa/api-keys/v3/list/all`, not `/list`. No change to the response type
— `ListApiKeysResponse { keys: Vec<KeyInfo> }` already matches the
spec's `GetApiKeysResponse`, so `run_list`, the fan-out, and all three
renderers are untouched.
I checked every other path in the module against the published OpenAPI
v5 spec (`docs.coralogix.com/llms.txt` → `api-keys-service` /
`api-keys-admin-service`). `POST /v3`, `GET|PUT|DELETE /v3/{key_id}`,
`GET /send-data-keys/v3`, `GET /v3` (admin list), `POST /v3/all/delete`,
and `POST /v3/all/status` all match. `list` was the only wrong one.
## Key Decisions
- **Kept the two-segment `/list/all` literal rather than inventing a
constant.** It's used once, and a comment on `list()` explains *why* the
suffix matters — the failure mode is non-obvious (a wrong path here
doesn't 404, it gets misrouted into the by-ID handler).
- **Made the regression test reproduce the real server behaviour.** The
existing wiremock test mounted the *buggy* path, so it passed against
the bug and would have passed against any future regression to a
different wrong path only by 404-ing. The test now also mounts
`^/mgmt/openapi/5/aaa/api-keys/v3/[^/]+$` returning the real 400
payload. Reverting the fix produces the exact error from the issue,
verified locally.
- **Fixed the mock response body too.** It was
`{"keys":[{"keyInfo":{"keyId":…,"hashedKey":…}}]}` — a shape the API
never returns. Every `KeyInfo` field is `Option`, so it deserialized to
all-`None` and asserted nothing. Now uses the real
`id`/`name`/`owner`/`active`/`hashed` shape.
## Changes
- `cx iam api-keys list` now calls `GET /aaa/api-keys/v3/list/all` and
returns the team's API keys instead of failing with a 400.
- No CLI surface change: no new/changed/removed commands, flags, output
fields, or config.
## Testing
- `cargo test --test api_keys` — 2 passed.
- Reverted the path to `/list` and re-ran: `list_api_keys_from_mock`
fails with `API request failed (400): Bad Request: key_id invalid
character`, matching the issue verbatim. Restored the fix and confirmed
green.
- Full CI sequence: `cargo fmt --check && cargo clippy --locked -- -D
warnings && cargo test --locked` — clean, all suites passing.
- **Not run:** the e2e suite (`cargo test --test e2e -- --ignored`),
which covers `api_keys_list` against the Coralogix test team. No
`CX_API_KEY` or `~/.cx/` config on this machine. Worth a run by someone
with credentials before merge — that test would have caught this
originally.
## Risks & Rollout
Low. One endpoint path on a command that is currently 100% broken, so
there is no working behaviour to regress. No migration, no flag, no
schema change. Roll back by reverting the commit.
## Out of Scope / Follow-ups
- The text-mode table for `api-keys list` renders a `Created` column
that is always blank — `KeyInfo` has no `createdAt` field in the v5
spec, and `skills/cx-platform-admin/SKILL.md` documents a `jq` recipe
sorting on `created_at`. Both are pre-existing and unrelated to the 400;
worth a separate cleanup.
- No audit of `/list`-style paths in other command modules beyond
confirming they differ (`slos` and `scopes` use `/all/list`,
`dashboards` uses `/catalog/list`). A broader sweep of every module
against the OpenAPI spec would be a good follow-up.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: amitcoralogix <amit.segal@coralogix.com>