mirror of https://github.com/coralogix/cx-cli.git synced 2026-09-14 16:15:45 +08:00

Files

T

Vitaly Lavrov 465742234a feat(dashboards): add 'cx dashboards check' subcommand (CX-47295) (#157 )

## Context
Adds `cx dashboards check`, a read-only subcommand that validates a
dashboard definition against the Coralogix Dashboard Service's strict
validator (`CheckDashboard`) without persisting it. This lets users and
AI agents lint a dashboard JSON before `create`/`replace`, or
re-validate an existing stored dashboard by id. Backed by
[CX-47295](https://coralogix.atlassian.net/browse/CX-47295); the backend
RPC already exists in eng-svc-dashboards-api (cx-api-dashboards v2.9.1).

## Linked Issues
- Jira: [CX-47295](https://coralogix.atlassian.net/browse/CX-47295) —
cx-cli: add `dashboards check` command to validate dashboard definitions
via CheckDashboard
- Related: CX-47080 (CheckDashboard wiring for cx-extensions — used as
precedent for the proto/gateway REST path)

## Design
Standard REST archetype (Archetype B), mirroring
`DataArchiveMetricsCmd::Validate` as the only existing validate-style
subcommand in the repo.

- **Endpoint (confirmed from `cx-management-apis` proto):** `POST
/mgmt/openapi/5/dashboards/check/v1`. The proto `google.api.http`
annotation is `POST /dashboards/check/v1`; the management gateway maps
the `/dashboards` prefix to `/mgmt/openapi/5/dashboards` (same mapping
as the existing `DASHBOARDS_BASE`). Permission: `team-dashboards:Read`
(read-only).
- **Request oneof:** `CheckDashboardRequest` has a `source` oneof — `{
"dashboard": {...} }` for an inline definition or `{ "dashboardId":
"<id>" }` for a stored dashboard. Reuses the existing
`read_dashboard_body()` helper for the `--from-file` path (normalizes
bare docs and `{ "dashboard": {...} }` wrappers, validates `layout` is
present).
- **Response:** `CheckDashboardResponse { issues: Vec<Issue> }` where
`Issue { severity, message, location }`. `severity` is an enum
(`SEVERITY_ERROR` / `SEVERITY_WARNING` / `SEVERITY_UNSPECIFIED`)
serialized as SCREAMING_SNAKE_CASE per protobuf-JSON convention.
`location` is an RFC 6901 JSON Pointer. Empty `issues` == valid.
- **Three layers as per contributing/adding-a-command.md:**
1. `api.rs` — response types + `DashboardsApi::check()` +
deserialization unit tests
  2. `mod.rs` — `run_check()` with fan-out/merge/render + helpers
  3. `main.rs` — `DashboardsCmd::Check` variant + dispatch arm
- **Skill integration:** `cx-dashboards` skill updated with a new Phase
7 (server-side validation) inserted between Phase 6 (self-verify) and
the deploy phase. Agents run `cx dashboards check --from-file <draft>`
as a pre-flight gate; on issues, loop back to Phase 4 (regenerate JSON)
→ Phase 5 (re-verify queries) → Phase 7 again.

## Key Decisions
- **Exit-code semantics: non-zero only on `SEVERITY_ERROR`.** The server
today only emits `SEVERITY_ERROR`, but the proto reserves
`SEVERITY_WARNING`. `SeverityUnspecified` is conservatively treated as a
failure (it's the proto default and should never appear in real output).
Warnings print but exit 0. Implemented via
`IssueSeverity::is_failure()`.
- **Multi-profile carve-out:** `check` exits non-zero if **any** profile
returns error-severity issues, even if other profiles are clean. This
deliberately breaks from the repo's "exit 0 if any profile succeeds"
convention (`docs/multi-profile.md`) because `check` is a validation
gate first. Per scope decision, the carve-out is documented **locally**
(command `--help`, `run_check` doc comment, and the skill) rather than
in `docs/multi-profile.md`, keeping the PR within domain-team + cxai
review (no tech-writers needed).
- **`from_file` has no default** (unlike `Create`/`Replace` which
default to `-`). With a `-` default, `cx dashboards check` with no args
would hang reading stdin; the explicit `Option<String>` + runtime guard
produces a clear "specify either `--from-file` or a `<dashboard_id>`"
error instead.
- **Loop-back target on issues is Phase 4** (regenerate JSON), not Phase
5. Some strict issues are structural (missing variable/filter ids), not
query-syntax — Phase 4 is the conservative default; after regenerating,
Phase 5 re-verifies changed queries, then back to Phase 7.
- **`--strict-check` flag on `create`/`replace` deferred.** Part 2 of
the ticket (opt-in pre-flight flag on writes) was scoped out of this PR
per explicit decision; the standalone `check` subcommand covers the
lint-before-create use case on its own.

## Changes
- **New subcommand:** `cx dashboards check`
- `cx dashboards check --from-file <path|->` — validate an inline
definition (reuses `read_dashboard_body` normalization; accepts bare doc
or `{ "dashboard": {...} }` wrapper).
- `cx dashboards check <dashboard-id>` — validate a stored dashboard by
id.
- Mutually exclusive sources via clap `conflicts_with` + runtime guard
for the neither-supplied case.
  - Read-only — no `confirm_destructive`, not blocked by read-only mode.
- **New output rendering for issues:** text (table `Severity | Location
| Message` with colored severities; green `Dashboard is valid (no
issues)` on empty), json (flat issue array with `profile` key when
multi-profile), agents (TOON-encoded same shape).
- **New API client method:** `DashboardsApi::check()` → `POST
/mgmt/openapi/5/dashboards/check/v1`.
- **Skill workflow change:** `cx-dashboards` skill phases renumbered —
inserted Phase 7 (server-side validation), existing Phase 7 → 8
(Deploy), Phase 8 → 9 (Share link). Stale phase references in
`references/deploy.md` and `references/verification.md` updated to
match.

## Testing
All run on this branch:
- `cargo fmt --check` — clean
- `cargo clippy --all-targets` — no warnings (fixed one
`bool_comparison` in a unit test)
- `cargo test --lib` — **404 passed**, 0 failed, 9 ignored (includes 5
new unit tests in `dashboards/api.rs`)
- `cargo test --test dashboards` — **14 passed** (8 existing + 6 new
wiremock integration tests)
- `cargo build` — succeeds
- `cx schema` — verified `check` present with `from_file` +
`dashboard_id` args
- `cx dashboards check --help` — renders correctly with examples +
carve-out note
- `cx dashboards --help` — `check` listed between `replace` and `delete`
- `scripts/verify-skills.sh` — 14/14 passed
- `cargo test --test e2e -- --ignored dashboards_check` — 2 passed
(graceful skip without `CX_API_KEY`); not run against the real test team
in this PR (CI workflow will run them)
- Manual: `cx dashboards check` (no args) → clear error exit 1; `cx
dashboards check --from-file foo <id>` (both args) → clap conflict error

## Risks & Rollout
- **Backwards compatible.** New subcommand only; no changes to existing
`catalog`/`get`/`create`/`replace`/`delete`/`search`/`query-search`/`folders`
behavior. Default behavior of `create`/`replace` is unchanged (the
deferred `--strict-check` flag would have been the only write-path
change, and it's not in this PR).
- **Endpoint path inferred from proto annotation.** `POST
/mgmt/openapi/5/dashboards/check/v1` is derived from the proto
`google.api.http` option + the established gateway prefix mapping. The
e2e tests against the real test team will confirm the path resolves
correctly; if it 404s, only the e2e tests fail (the subcommand is
read-only and the integration tests mock the path, so there's no risk to
existing flows).
- **Skill workflow change is low-risk** — adds one checkpoint between
self-verify and deploy; agents that skip it just go straight to deploy
(no regression, just a missed pre-flight).
- **Rollback:** revert the 7 commits; no migration, no config change, no
data change.

## Out of Scope / Follow-ups
- **`--strict-check` flag on `cx dashboards create`/`replace`** — Part 2
of CX-47295. Opt-in pre-flight validation before writes; off by default
to preserve current behavior. Will be a separate PR.
- **`--request-id` flag on `check** — the proto supports an optional
idempotency key; not exposed in this PR. Can be added if needed.
- **E2E against real test team** — the 2 `#[ignore]`d e2e tests skip
gracefully without `CX_API_KEY`; CI's e2e workflow
(`.github/workflows/e2e.yml`) will run them on merge to master.

[CX-47295]:
https://coralogix.atlassian.net/browse/CX-47295?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[CX-47295]:
https://coralogix.atlassian.net/browse/CX-47295?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Yonatan Beker <yonbek@users.noreply.github.com>

2026-07-16 15:01:01 +02:00

2.0 KiB

Raw Permalink Blame History

Development

Build and test

cargo build                         # Dev build
cargo build --release               # Release build (stripped, LTO)
cargo fmt                           # Format code
cargo clippy                        # Lint
cargo test                          # Run all tests
cargo test -- --ignored             # Run integration tests (system keyring required)
cargo run -- <args>                 # Run CLI in dev mode

Rust toolchain is pinned to 1.96.1 via rust-toolchain.toml.

Before submitting a PR, verify:

cargo fmt --check is clean
cargo clippy produces no warnings
cargo test passes

End-to-end tests

The tests/e2e/ suite invokes the compiled cx binary against a real Coralogix test team, sanity-checking every command and subcommand. All e2e tests are #[ignore]d, so the default cargo test run skips them.

Set credentials one of two ways:

# Option A - environment
export CX_API_KEY=cxtp_...
export CX_REGION=stg1

# Option B - .env file in the repo root (gitignored)
cp .env.example .env
# edit values

Then run:

cargo test --test e2e -- --ignored --test-threads=1

Tests skip gracefully (with a [e2e] skipping ... log line) when credentials are absent, or when the test team has no data for a discovery step (e.g. no alerts to fetch). The suite is read-only against the test team - mutating commands (alerts create, alerts enable/disable) are intentionally not covered.

CI runs the suite on every push to master and via manual workflow_dispatch (see .github/workflows/e2e.yml).

DataPrime documentation bundle

The cx dataprime subcommands read command and function help from assets/dataprime_docs.yaml, which is compiled into the binary at build time via include_str! in src/commands/dataprime/mod.rs. Nothing is loaded from disk at runtime.

Architecture

See architecture.md for the execution flow, command archetypes, error handling, and module map.

2.0 KiB Raw Permalink Blame History