diff --git a/AGENTS.md b/AGENTS.md index a0a8bb0..c43324a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,10 @@ List of project documents and their specific purpose: | `docs/documentation-maintenance.md` | Doc update rules | After any code change | | `docs/skills.md` | AI agent skill format, taxonomy, authoring guide | When adding or modifying skills | | `docs/user-guide/getting-started.md` | Installation, first context, quick start | New users, onboarding | -| `docs/user-guide/configuration.md` | Config file, env vars, context commands | When working with config/context | +| `docs/user-guide/configuration.md` | Config file, env vars, override precedence | When working with config files / env vars | +| `docs/user-guide/context.md` | `a6 context` command reference | When working with the context command | +| `docs/user-guide/version.md` | `a6 version` output and use cases | When filing bug reports / verifying installs | +| `docs/user-guide/conditional-plugins.md` | Plugins that need APISIX-side enabling (skywalking et al) and test-coverage states | When a plugin fails with "unknown plugin" or "not enabled" | | `.github/workflows/ci.yml` | Unit test + lint CI workflow | When modifying CI | | `.github/workflows/e2e.yml` | E2E test CI with real APISIX | When modifying e2e infrastructure | @@ -54,7 +57,7 @@ a6/ ├── pkg/iostreams/ # I/O abstraction (TTY detection) ├── pkg/cmdutil/ # Shared command utilities ├── pkg/tableprinter/ # Table output rendering -├── pkg/httpmock/ # Legacy HTTP mocking helpers still under migration +├── pkg/httpmock/ # HTTP mocking helpers for unit tests ├── internal/config/ # Configuration/context management ├── internal/version/ # Build version info ├── docs/ # All documentation @@ -161,6 +164,23 @@ Example: feat(route): add route list command with table output 6. Run `make check` before committing. All checks must pass. 7. Do not add new mock-Admin-API command tests. Use real e2e coverage for command behavior. +### Bug-Class Audit Protocol +When a live-validation bug is found (e.g. surfaced by the GA smoke walkthrough or a real user report), don't just fix the single occurrence. Identify the **bug class** — the underlying shape that produced it — and audit the rest of the codebase for the same shape before closing. + +Trigger: you reproduce a bug in production code. Before opening the fix PR: + +1. **Name the class.** What is the *shape* of this bug? Examples: + - "JSON unmarshal of an APISIX response that changed shape across versions" (e.g. `nodes` becoming an object in 3.x). + - "Success message echoes a server-returned field that doesn't actually carry the resource id." + - "`validate` accepts unknown top-level keys because the typed struct has no field for them." + - "Command path assembles `/` and silently strips one half when the file payload disagrees." +2. **Grep for the same shape.** Use the file/symbol patterns that match the class, not just the resource that surfaced it. For an unmarshal-shape bug, grep every `*_response.go` or `Unmarshal` call site, not only the one you hit. For a success-message bug, grep every `fmt.Fprintf.*deleted\|created\|updated` line. +3. **Decide per hit.** For each match: confirm it's affected, confirm it's exempt, or convert it to a separate sub-issue. Don't bundle unrelated fixes into the same PR — list the matches in the PR description even if you defer them. +4. **Generalise the test.** When possible, add the test at the *class* level (e.g. unmarshal-shape test across two response variants) rather than only the single resource that surfaced the bug. +5. **Note the class in the PR.** Even if only one fix lands, the description must say "audited X other call sites; Y were unaffected, Z tracked as #N." Future readers should be able to tell the audit happened. + +Reference: the a7 protocol that this mirrors — a7 PR #32 audited every `*/create.go` after `BUG-2/BUG-3` turned out to share a `json.RawMessage`→YAML byte-array root cause; a7 PR #33 generalised "reject empty `upstreams: []`" to "reject bare and null forms too." These are the shape and granularity expected here. + ### Environment Variables | Variable | Purpose | Default | |---|---|---| diff --git a/docs/ga-ux-sweep.md b/docs/ga-ux-sweep.md new file mode 100644 index 0000000..a3df17c --- /dev/null +++ b/docs/ga-ux-sweep.md @@ -0,0 +1,100 @@ +# a6 GA UX Consistency Sweep + +Tracks the CLI UX consistency review requested in [#41](https://github.com/api7/a6/issues/41). Mirrors a7's review pattern (a7 issues #35, #36, #37, #42, #49): for every command, scan for **doc-vs-code flag mismatches**, **silently-dropped positional args**, **inconsistent `--output` defaults**, and **confusing `--id` semantics**. + +Most findings here were surfaced during the [GA Run 1 manual walkthrough](./ga-test-report.md); a few were added by widening the sweep across `--help` output for every ` ` combination. Each finding lists its current disposition: ✅ fixed in PR, 🟡 deferred follow-up, 🟢 minor/cosmetic, 📘 PRD-side fix. + +--- + +## Class 1 — Doc-vs-code mismatches (PRD drift) + +The PRD lists commands or flags that don't exist in the binary, or that have moved. + +| # | What PRD says | What the binary does | Disposition | +|---|---|---|---| +| UX-1 | `a6 consumer credential ...` (nested under consumer) | `a6 credential ...` (top-level command) | 📘 PRD update (or rename the top-level command — pick one) | +| UX-2 | `a6 ssl upload` (in §"Resource Commands") | No `upload` subcommand exists — `ssl` has only `create/delete/export/get/list/update` | 📘 PRD update or implement | +| UX-3 | `a6 schema get` (in §"Resource Commands") | No `schema` top-level command exists | 📘 PRD update or implement | +| UX-4 | `--verbose / -v` (in §"Common Flags") | The `--verbose` flag exists but no `-v` shorthand is registered | 📘 PRD update or wire shorthand | + +These are doc/spec follow-ups, not code regressions, but they bite first-time users who reach for the documented command. The fix is one of: update PRD, implement the command, or rename/alias the implementation. + +--- + +## Class 2 — Silently-dropped positional args / unknown subcommands + +| # | Reproducer | Expected | Actual | +|---|---|---|---| +| UX-5 | `a6 route bogus` (typo'd subcommand) | non-zero exit, error message | exit=0, prints `route --help`. Same shape for any ` ` | +| UX-6 | `a6 route deletex --force` (typo'd subcommand, plausible-looking flag) | error: unknown subcommand `deletex` | exit=0, prints `route --help`; `--force` silently swallowed | + +🟡 **Tracked as a follow-up.** Cobra has `SilenceErrors` defaults that can be tightened (use `cobra.OnlyValidArgs` or set `SilenceUsage: false` + return an error from the parent's `RunE`). Affects every parent command — fix is centralised in `pkg/cmd/root/root.go` and the per-resource parent files. + +In the manual walkthrough, no resource was found to silently drop positionals on **valid** subcommands (Cobra's `ExactArgs` / `MaximumNArgs` reject extras correctly). The silent-success failure mode is specific to **unknown** subcommands. + +--- + +## Class 3 — Inconsistent `--output` defaults + +Verified by sweeping `--help` text across every ` ` combination: + +| Action group | `--help` advertises | Actual behaviour | +|---|---|---| +| ` list`, ` delete` | `json, yaml, table` | All three formats work | +| ` get`, ` create`, ` update` | `json, yaml` | `-o table` returns `unsupported output format: table` | + +🟡 **BUG-6** in the [Run 1 report](./ga-test-report.md). Deferred from PR #51 — fix needs a shared per-resource table renderer lifted out of each `list.go`. Touches 12 `get.go` files plus the matching `create.go` / `update.go` if we want table for those too. + +Additional symptom of the same class: **TTY default differs between `get` and `list`.** `get` defaults to `yaml` in TTY (because table is unsupported); `list` defaults to `table`. A user piping `a6 route get | grep` and `a6 route list | grep` gets different formats — not wrong per se, but unexpected. + +--- + +## Class 4 — `--id` / positional-id semantics + +The sweep across every resource confirms there is **no `--id` flag** anywhere — ids are always positional, which is consistent with Cobra conventions. The convention split that matters is the **bracket style** in `Use:` strings: + +| Resource group | Pattern | Convention | Required? | +|---|---|---|---| +| `route`, `service`, `upstream`, `ssl`, `plugin-config`, `global-rule`, `stream-route`, `proto`, `consumer-group` | ` get [id]` | Square brackets | Optional — falls back to interactive selector when omitted in TTY | +| `consumer` | `consumer get [username]` | Square brackets | Optional (username is the id, just renamed) | +| `secret` | `secret get [manager/id]` | Square brackets | Optional; positional must include the `manager/` prefix | +| `plugin-metadata` | `plugin-metadata get ` | Angle brackets | Required | +| `credential` | `credential get ` | Angle brackets, **but also needs `--consumer`** flag | Required | + +🟢 The square-vs-angle distinction is semantically correct (Cobra: `` required, `[x]` optional). No bug here, but the cognitive load is real — readers have to remember that `plugin-metadata` and `credential` require the positional while everyone else doesn't. + +Real `--id`-shape bug surfaced during Run 1 and fixed: **secret create** rejected mismatched ids in body vs. positional. See [Run 1 report — BUG-4](./ga-test-report.md#bug-4--secret-create-vaultid-rejected-by-apisix-when-file-has-a-bare-id). + +One related observation that survived Run 1: **global-rule `--id`** (where the resource id is forced to equal the single plugin key inside `plugins:`). a7 logged the same finding and treated it as a real bug. We didn't separately fix it on a6 — but `a6 global-rule create -f file.yaml` with a mismatched `id:` should clearly reject rather than silently letting APISIX compute the id from the plugin name. 🟡 Tracked. + +--- + +## Class 5 — Other UX inconsistencies surfaced during Run 1 + +| # | Symptom | Notes | +|---|---|---| +| UX-7 | ` delete ` (no `--force`) **silently deletes in non-TTY** | PRD says destructive ops "must require confirmation unless `--force`." In non-TTY there's no way to confirm interactively, so a6 just deletes. Either the PRD needs to allow this carve-out or the CLI should bail out asking for `--force` when stdin isn't a TTY. | +| UX-8 | `a6 config sync --dry-run` and `a6 config diff` produce **identical output** | A `Dry-run — no changes applied.` banner on the sync side would help operators in shared shells distinguish them at a glance. | +| UX-9 | `a6 config diff` emits a line per known section even when empty | `consumer_groups: create=0 update=0 delete=0 unchanged=0` etc. on an empty diff is verbose; collapse zero-change sections by default with a `--verbose` opt-in. | +| UX-10 | `credential delete` output read `Credential 1 deleted` (literal "1") before PR #51 | ✅ Fixed in PR #51 as BUG-3. | +| UX-11 | `upstream health` couldn't parse APISIX 3.x healthcheck response shape | ✅ Fixed in PR #51 as BUG-1. | +| UX-12 | `upstream health` ignored `A6_CONTROL_URL` env var | ✅ Fixed in PR #51 as BUG-2. | +| UX-13 | `config validate` accepted unknown top-level sections silently | ✅ Fixed in PR #51 as BUG-5. | + +--- + +## Summary + +| Class | Total findings | Already fixed (PR #51) | Open / deferred | +|---|---|---|---| +| 1 — PRD drift | 4 | 0 | 4 (📘 PRD-side) | +| 2 — Silent unknown subcommand | 2 | 0 | 1 (centralised fix) | +| 3 — `--output` inconsistency | 1 class spanning ~36 commands | 0 | 1 (BUG-6 in #51 report) | +| 4 — `--id` semantics | 1 cross-resource + 1 global-rule case | 1 (BUG-4) | 1 (global-rule body-id) | +| 5 — Other UX | 7 items | 4 (BUG-1/2/3/5) | 3 | + +**Net: 10 open UX findings.** All non-blocking for GA from a correctness standpoint; together they're the polish layer for the imperative CLI surface. + +## Next step for #41 + +Per the issue body, each finding becomes a discrete sub-issue. After this PR lands, file 10 GitHub issues under #33 — one per open row — and reference this document so the audit trail stays linked. Closed-by-#51 items don't need new issues. diff --git a/docs/roadmap.md b/docs/roadmap.md index 6c6a51f..02e5e2a 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -4,7 +4,7 @@ This document defines the per-PR development plan for the a6 CLI. Each PR is sel **Audience**: AI coding agents and human developers. Each PR section contains enough detail to be implemented autonomously. -> **Status: ✅ Phase 1-3 COMPLETE (27 PRs)** — Phase 4 (AI Agent Skills, PR-28 through PR-36) is in progress. +> **Status: ✅ Phase 1-4 COMPLETE (36 PRs)** — All phases shipped. Phase 4 (AI Agent Skills, PR-28 through PR-36) merged as GitHub PRs #1 to #9 in March 2026. Post-merge work tracked under the GA-readiness epic ([#33](https://github.com/api7/a6/issues/33)). --- @@ -1369,12 +1369,12 @@ Phase 4 adds AI agent skill files (`SKILL.md`) that enable AI coding agents to w | PR-25 | Bulk Operations | 3 | ✅ | Bulk delete/export by label | | PR-26 | Auto-Update | 3 | ✅ | Version + update check | | PR-27 | CLI Extensions | 3 | ✅ | Extension management lifecycle | -| PR-28 | Skills Infra + Shared Skill | 4 | 🚧 | CI validation | -| PR-29 | Auth Plugin Skills | 4 | ⬚ | CI validation | -| PR-30 | Security + Rate Limiting Skills | 4 | ⬚ | CI validation | -| PR-31 | Traffic + Transformation Skills | 4 | ⬚ | CI validation | -| PR-32 | Operational Recipe Skills | 4 | ⬚ | CI validation | -| PR-33 | AI Gateway Skills | 4 | ⬚ | CI validation | -| PR-34 | Observability Skills | 4 | ⬚ | CI validation | -| PR-35 | Advanced Plugin Skills | 4 | ⬚ | CI validation | -| PR-36 | Advanced Recipes + Personas | 4 | ⬚ | CI validation | +| PR-28 | Skills Infra + Shared Skill | 4 | ✅ | CI validation (GitHub PR #1) | +| PR-29 | Auth Plugin Skills | 4 | ✅ | CI validation (GitHub PR #2) | +| PR-30 | Security + Rate Limiting Skills | 4 | ✅ | CI validation (GitHub PR #3) | +| PR-31 | Traffic + Transformation Skills | 4 | ✅ | CI validation (GitHub PR #4) | +| PR-32 | Operational Recipe Skills | 4 | ✅ | CI validation (GitHub PR #5) | +| PR-33 | AI Gateway Skills | 4 | ✅ | CI validation (GitHub PR #6) | +| PR-34 | Observability Skills | 4 | ✅ | CI validation (GitHub PR #7) | +| PR-35 | Advanced Plugin Skills | 4 | ✅ | CI validation (GitHub PR #8) | +| PR-36 | Advanced Recipes + Personas | 4 | ✅ | CI validation (GitHub PR #9) | diff --git a/docs/user-guide/conditional-plugins.md b/docs/user-guide/conditional-plugins.md new file mode 100644 index 0000000..8db3801 --- /dev/null +++ b/docs/user-guide/conditional-plugins.md @@ -0,0 +1,61 @@ +# Conditional Plugins + +Most APISIX plugins are enabled by default and "just work" once you reference them from a route, service, consumer, or global rule. A few plugins are **conditional**: they ship with APISIX but are commented out in the stock `config.yaml`, and APISIX rejects requests that reference them until you enable them in the `plugins:` list. + +If a plugin isn't enabled, you'll see one of these errors when you try to use it through a6: + +```text +API error (status 400): unknown plugin [] +API error (status 400): plugin is not enabled +``` + +This page lists the conditional plugins a6 has surfaced during real-environment validation, how to enable them, and how that affects the test/skills coverage you'll see in this repo. + +## Plugins that need enabling + +| Plugin | APISIX docs | Why it's off by default | Notes | +|---|---|---|---| +| `skywalking` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking/) | Requires an external SkyWalking OAP collector; off so APISIX doesn't ship a noisy default. | Enabled in this repo's e2e config (`test/e2e/apisix_conf/config.yaml`, `config-docker.yaml`). | +| `skywalking-logger` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking-logger/) | Same external dependency as `skywalking`. | Enabled in this repo's e2e config. | +| `ai-content-moderation` | [docs](https://apisix.apache.org/docs/apisix/plugins/ai-content-moderation/) | Requires an external moderation backend (e.g. AWS Comprehend). | Not enabled in the e2e config; the matching skill case is `Skip()`-gated. | +| `chaitin-waf` | [docs](https://apisix.apache.org/docs/apisix/plugins/chaitin-waf/) | Requires the Chaitin SafeLine WAF sidecar. | Enabled in plugin list but no upstream to talk to in tests. | +| `splunk-hec-logging`, `kafka-logger`, `tcp-logger`, etc. | various | Require a real external sink. | Enabled in plugin list but harmless without a configured destination. | + +This list reflects what a6's own e2e walkthrough has run into. APISIX's full conditional set is broader — when in doubt, check your APISIX deployment's `config.yaml` against the [stock APISIX config](https://github.com/apache/apisix/blob/master/conf/config.yaml.example) to see which plugins are commented out. + +## Enabling a conditional plugin + +Edit your APISIX deployment's `config.yaml` and add the plugin name to the `plugins:` list, then restart or hot-reload APISIX: + +```yaml +plugins: + - real-ip + - cors + - key-auth + - skywalking # <-- add the line +``` + +```bash +apisix reload +# or, with Docker: +docker restart +``` + +After the reload, the previously-failing a6 command should succeed. + +## How this maps to the test suite + +The a6 e2e suite distinguishes three states for each plugin scenario: + +| State | What you'll see | +|---|---| +| **Verified in real env** | Tests run against a live APISIX with the plugin enabled. These cases run on every CI pass — no skip. | +| **Conditional / skipped** | Tests are gated behind a `Skip(...)` call (see `test/e2e/`). They run if the plugin is enabled in the test APISIX, otherwise they skip with an explanatory message. Currently this covers `ai-content-moderation`, `chaitin-waf` flows where the dependency isn't installed, and a small number of license-gated scenarios. | +| **Not covered** | Plugins for which a6 ships no skill or test. They may still work — the gateway accepts them — but a6 makes no guarantees beyond the generic CRUD path. | + +The full list of currently-skipped scenarios is tracked in [issue #36](https://github.com/api7/a6/issues/36). Each skip will be either un-skipped (by enabling the dependency) or removed (by dropping the scenario from the supported set) before GA. + +## Related + +- [Plugin commands](./plugin.md) — `a6 plugin list` / `a6 plugin get` for plugin introspection. +- [Skills](../skills.md) — the AI-agent skill format, including how plugin skills declare their conditional dependencies. diff --git a/docs/user-guide/context.md b/docs/user-guide/context.md new file mode 100644 index 0000000..579401e --- /dev/null +++ b/docs/user-guide/context.md @@ -0,0 +1,83 @@ +# Context + +A **context** is a named connection profile that tells a6 which APISIX instance to talk to. You can keep multiple contexts (e.g. `local`, `staging`, `prod`) in the same config file and switch between them at any time — similar to `kubectl config` or `gcloud config configurations`. + +This page covers the `a6 context` command surface. For the underlying config file format, precedence rules, and environment-variable overrides, see [configuration.md](./configuration.md). + +## Commands + +```text +a6 context create Create a new context +a6 context use Switch the active context +a6 context list List all contexts +a6 context current Print the active context name +a6 context delete Delete a context +``` + +### Create + +```bash +a6 context create local \ + --server http://localhost:9180 \ + --api-key edd1c9f034335f136f87ad84b625c8f1 +``` + +The context is saved and immediately set as the current context. + +| Flag | Required | Description | +|---|---|---| +| `--server` | yes | Admin API URL (`http://:`). | +| `--api-key` | no | Admin API key. Omit if the server doesn't require auth. | + +### Use + +```bash +a6 context use staging +``` + +All subsequent commands hit the `staging` context's server until you switch again. You can override at the command level with `--context staging` or with the `A6_SERVER` / `A6_API_KEY` env vars on a single invocation. + +### List + +```bash +a6 context list +``` + +```text +NAME SERVER CURRENT +local http://localhost:9180 * +staging https://stg.example:9180 +``` + +The `*` marker shows the currently active context. + +### Current + +```bash +a6 context current +``` + +Prints just the name (one line). Handy for shell prompts: + +```bash +PS1='[$(a6 context current)] $ ' +``` + +### Delete + +```bash +a6 context delete staging +``` + +You cannot delete the currently active context. Switch to a different one with `a6 context use ` first. + +## Tips + +- Contexts are stored in `~/.config/a6/config.yaml` (or `$A6_CONFIG_DIR/config.yaml` / `$XDG_CONFIG_HOME/a6/config.yaml` if set). The file is created lazily on the first `a6 context create`. +- API keys are stored in plain text. If that's a concern, omit `--api-key` and use the `A6_API_KEY` env var on each invocation instead. +- Use `--context ` on any command to target a different context for that one invocation without switching. + +## Related + +- [Configuration](./configuration.md) — config file format, env vars, override precedence. +- [Getting Started](./getting-started.md) — the first-context walkthrough. diff --git a/docs/user-guide/getting-started.md b/docs/user-guide/getting-started.md index b3ce6f4..66d7d78 100644 --- a/docs/user-guide/getting-started.md +++ b/docs/user-guide/getting-started.md @@ -181,6 +181,8 @@ a6 debug logs --container apisix --tail 50 --follow ## Managing Multiple Contexts +The summary below is enough to get going. For the full command reference (`create` / `use` / `list` / `current` / `delete`) plus tips on using `--context` for one-off overrides, see the [Context Guide](context.md). + You can create multiple contexts for different environments like staging or production. ```bash @@ -238,8 +240,13 @@ For full usage details across resource types, see the [Bulk Operations Guide](bu ## What's Next -- Check the [Configuration Guide](configuration.md) for detailed configuration options. -- See the [Route Management Guide](route.md) for comprehensive route CRUD operations. +- [Configuration](configuration.md) — config file format, env vars, override precedence. +- [Context](context.md) — direct reference for `a6 context` commands. +- [Route Management](route.md) — comprehensive route CRUD. +- [Declarative Config](declarative-config.md) — `a6 config dump / diff / sync / validate` workflow. +- [Conditional Plugins](conditional-plugins.md) — plugins that need APISIX-side enabling (skywalking et al) and how the test suite handles them. +- [Auto-Update](auto-update.md) — keeping a6 itself up to date. +- [Version](version.md) — what `a6 version` shows and when to use it. ## Keeping a6 Up To Date diff --git a/docs/user-guide/version.md b/docs/user-guide/version.md new file mode 100644 index 0000000..9c702e5 --- /dev/null +++ b/docs/user-guide/version.md @@ -0,0 +1,34 @@ +# Version + +Print the a6 build information. + +```bash +a6 version +``` + +```text +a6 version v1.2.3 + commit: abc1234 + built: 2026-05-27T03:23:51Z + go: go1.22.x + platform: darwin/arm64 +``` + +| Field | Source | +|---|---| +| `version` | Tagged release if built from a tag (e.g. `v0.1.0-rc1`); otherwise the short commit. `dev` when built without ldflags. | +| `commit` | Short Git commit the binary was built from. | +| `built` | UTC timestamp of the build. | +| `go` | Go toolchain used. | +| `platform` | `/` of the binary. | + +## When to use it + +- **Filing a bug report.** Paste the full `a6 version` output so maintainers know exactly which build is affected. +- **Verifying an update.** After `a6 update`, re-run `a6 version` to confirm the new version is in place. +- **CI logs.** Run `a6 version` as the first step of any pipeline that uses a6, so the build is recorded against the run. + +## Related + +- [Auto-Update](./auto-update.md) — keep a6 itself up to date. +- [Getting Started](./getting-started.md) — installation and first run.