From 64e72b399e05c36925368173131b91ace1df524b Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 11:14:28 +0800 Subject: [PATCH 1/7] test: add skills validation coverage --- .github/workflows/e2e.yml | 3 + Makefile | 7 +- docs/skills.md | 93 +++++++++++---- docs/testing-strategy.md | 14 ++- scripts/validate-skills.sh | 84 +++++++++++++ skills/a7-persona-developer/SKILL.md | 20 +--- skills/a7-persona-operator/SKILL.md | 17 ++- skills/a7-recipe-api-versioning/SKILL.md | 3 +- skills/a7-recipe-blue-green/SKILL.md | 4 +- skills/a7-recipe-canary/SKILL.md | 2 +- skills/a7-recipe-graphql-proxy/SKILL.md | 2 +- skills/a7-recipe-health-check/SKILL.md | 18 +-- skills/a7-recipe-mtls/SKILL.md | 4 +- test/e2e/skills/skills_test.go | 146 +++++++++++++++++++++++ 14 files changed, 355 insertions(+), 62 deletions(-) create mode 100755 scripts/validate-skills.sh create mode 100644 test/e2e/skills/skills_test.go diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 1fad288..6ba64be 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -21,6 +21,9 @@ jobs: - name: Download Go modules run: go mod download + - name: Validate skills + run: make validate-skills + - name: Run E2E tests env: A7_ADMIN_URL: ${{ secrets.DEMO_WEBSITE }} diff --git a/Makefile b/Makefile index c9a21b9..bd28899 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ LDFLAGS := -s -w \ -X $(MODULE)/internal/version.Commit=$(COMMIT) \ -X $(MODULE)/internal/version.Date=$(DATE) -.PHONY: build test test-verbose lint fmt vet check install clean docker-up docker-down test-e2e test-e2e-full +.PHONY: build test test-verbose lint fmt vet check install clean docker-up docker-down validate-skills test-e2e test-e2e-full build: go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/a7 @@ -29,7 +29,10 @@ fmt: vet: go vet ./... -check: fmt vet lint test +check: fmt vet lint test validate-skills + +validate-skills: + ./scripts/validate-skills.sh install: build cp bin/$(BINARY) $(GOPATH)/bin/$(BINARY) diff --git a/docs/skills.md b/docs/skills.md index 77e2950..0b23861 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -13,8 +13,8 @@ skills/ ├── a7-shared/SKILL.md # Core a7 conventions (shared skill) ├── a7-plugin-ai-proxy/SKILL.md # AI Gateway plugin skill ├── a7-plugin-key-auth/SKILL.md # key-auth plugin skill -├── a7-recipe-gateway-group/SKILL.md # Gateway group management recipe -├── a7-persona-platform-eng/SKILL.md # Platform Engineer persona +├── a7-recipe-canary/SKILL.md # Canary release recipe +├── a7-persona-operator/SKILL.md # Operator persona └── ... ``` @@ -28,8 +28,8 @@ Skills follow a naming convention with four types: |--------|------|-------------|---------| | `a7-shared` | Shared | Core project conventions and patterns | `a7-shared` | | `a7-plugin-*` | Plugin | One API7 EE plugin — config, examples, gateway group scoping | `a7-plugin-ai-proxy` | -| `a7-recipe-*` | Recipe | Multi-step operational task (e.g., setting up a service template) | `a7-recipe-service-template` | -| `a7-persona-*` | Persona | Role-specific workflow guidance | `a7-persona-platform-eng` | +| `a7-recipe-*` | Recipe | Multi-step operational task | `a7-recipe-canary` | +| `a7-persona-*` | Persona | Role-specific workflow guidance | `a7-persona-operator` | ### Naming Rules @@ -106,13 +106,19 @@ The body content depends on the skill type: ## CI Validation -Every PR that modifies `skills/` is validated by `scripts/validate-skills.sh`. The script checks: +Every PR validates `skills/` with `scripts/validate-skills.sh`. The script checks: 1. Every `skills/*/SKILL.md` has valid YAML frontmatter 2. Required fields `name` and `description` are present 3. `name` matches the directory name 4. `name` follows kebab-case pattern 5. `description` is non-empty +6. skill names are unique + +The E2E suite also contains static skill checks under `test/e2e/skills`. +Those checks keep this document aligned with the actual `skills/` inventory and +prevent references to known removed commands such as the old health and portal +commands. Run locally: @@ -128,18 +134,65 @@ make validate-skills 4. Run validation: `make validate-skills` 5. Update this document if adding a new skill type or category -## Skill Roadmap - -| PR | Skills | Description | -|----|--------|-------------| -| PR-28 | 1 | Infrastructure + `a7-shared` | -| PR-29 | 6 | AI Gateway plugins (ai-proxy, ai-prompt-template, ai-prompt-decorator, ai-content-moderation, ai-rag, ai-token-limiter) | -| PR-30 | 5 | Enterprise core (gateway-group, service-template, rbac, portal, audit-log) | -| PR-31 | 5 | Authentication plugins (key-auth, jwt-auth, basic-auth, openid-connect, wolf-rbac) | -| PR-32 | 4 | Security + rate limiting (ip-restriction, cors, limit-count, limit-req) | -| PR-33 | 5 | Traffic + transformation (proxy-rewrite, response-rewrite, traffic-split, redirect, grpc-transcode) | -| PR-34 | 5 | Operational recipes (blue-green, canary, circuit-breaker, health-check, service-registry) | -| PR-35 | 6 | Observability (prometheus, skywalking, zipkin, http-logger, kafka-logger, datadog) | -| PR-36 | 3 | Advanced recipes + personas | - -**Total**: 40 skills across 9 PRs. \ No newline at end of file +## Current Inventory + +The repository currently contains 40 skills: + +**Shared** + +- `a7-shared` + +**Personas** + +- `a7-persona-developer` +- `a7-persona-operator` + +**Plugin Skills** + +- `a7-plugin-ai-content-moderation` +- `a7-plugin-ai-prompt-decorator` +- `a7-plugin-ai-prompt-template` +- `a7-plugin-ai-proxy` +- `a7-plugin-basic-auth` +- `a7-plugin-consumer-restriction` +- `a7-plugin-cors` +- `a7-plugin-datadog` +- `a7-plugin-ext-plugin` +- `a7-plugin-fault-injection` +- `a7-plugin-grpc-transcode` +- `a7-plugin-hmac-auth` +- `a7-plugin-http-logger` +- `a7-plugin-ip-restriction` +- `a7-plugin-jwt-auth` +- `a7-plugin-kafka-logger` +- `a7-plugin-key-auth` +- `a7-plugin-limit-count` +- `a7-plugin-limit-req` +- `a7-plugin-openid-connect` +- `a7-plugin-prometheus` +- `a7-plugin-proxy-rewrite` +- `a7-plugin-redirect` +- `a7-plugin-response-rewrite` +- `a7-plugin-serverless` +- `a7-plugin-skywalking` +- `a7-plugin-traffic-split` +- `a7-plugin-wolf-rbac` +- `a7-plugin-zipkin` + +**Recipe Skills** + +- `a7-recipe-api-versioning` +- `a7-recipe-blue-green` +- `a7-recipe-canary` +- `a7-recipe-circuit-breaker` +- `a7-recipe-graphql-proxy` +- `a7-recipe-health-check` +- `a7-recipe-mtls` +- `a7-recipe-multi-tenant` + +## Current Compatibility Notes + +- Route examples should use the current API7 EE model: create a service, then create routes with `service_id`. +- Auth examples should use `consumer create` plus `credential create`; do not put auth plugin credentials directly in the consumer body. +- Standalone upstream workflows are not the preferred `a7` path for current API7 EE. Use service inline upstreams and service-backed routes unless you are intentionally documenting APISIX-compatible behavior. +- Gateway/httpbin traffic checks are optional for `a7`; the default CI focuses on CLI-driven control-plane resource CRUD and structured `get/list/dump` assertions. diff --git a/docs/testing-strategy.md b/docs/testing-strategy.md index de25f90..57e80f7 100644 --- a/docs/testing-strategy.md +++ b/docs/testing-strategy.md @@ -6,6 +6,8 @@ - Unit tests are only allowed for self-contained logic that does not need mocked external systems. - Do not add new command-level tests that mock the Admin API, gateway, or control-plane behavior. - Default E2E coverage should prioritize control-plane CLI roundtrips: create/update/delete/sync via `a7`, then read back state with `get`, `list`, or `config dump`. +- GitHub CI uses repository-configured external environment variables and does not bootstrap API7 with Docker Compose. +- The Docker Compose stack under `test/e2e/` is for local testing and debugging. ## Test Pyramid For `a7` @@ -40,6 +42,10 @@ Optional targets outside the default CI path: - auth and forwarding behavior that depends on a live upstream - debug flows that require live gateway requests +Full gateway/data-plane traffic validation belongs in the gateway repository. +For `a7`, prefer validating CLI behavior by creating, updating, reading, and +deleting resources through the CLI itself. + E2E tests live in `test/e2e/` behind the `e2e` build tag and are run through the Ginkgo entrypoint in `make test-e2e`. ## What To Remove @@ -62,7 +68,10 @@ Optional for live gateway/data-plane coverage: - `A7_GATEWAY_GROUP` - `A7_E2E_ENABLE_GATEWAY_GROUP_CRUD=1` when you explicitly want to exercise gateway-group create/delete against an environment that supports it -For local development, you can bring up the repository's Docker-based environment with: +CI provides `A7_ADMIN_URL`, `A7_TOKEN`, and `A7_GATEWAY_GROUP` through repository +secrets. For local development, you can either point these variables at an +existing API7 environment or bring up the repository's Docker-based environment +with: ```bash make docker-up @@ -95,6 +104,9 @@ export HTTPBIN_URL=... make test-e2e-full ``` +`test-e2e-full` is intended for local/debug use. It should not be treated as the +default CI contract for `a7`. + Equivalent direct command: ```bash diff --git a/scripts/validate-skills.sh b/scripts/validate-skills.sh new file mode 100755 index 0000000..a811f42 --- /dev/null +++ b/scripts/validate-skills.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SKILLS_DIR="${ROOT_DIR}/skills" + +if [[ ! -d "${SKILLS_DIR}" ]]; then + echo "skills directory not found: ${SKILLS_DIR}" >&2 + exit 1 +fi + +status=0 +seen_names_file="$(mktemp)" +trap 'rm -f "${seen_names_file}"' EXIT + +for skill_dir in "${SKILLS_DIR}"/*; do + [[ -d "${skill_dir}" ]] || continue + + skill_name="$(basename "${skill_dir}")" + skill_file="${skill_dir}/SKILL.md" + + if [[ ! -f "${skill_file}" ]]; then + echo "${skill_name}: missing SKILL.md" >&2 + status=1 + continue + fi + + if [[ "$(sed -n '1p' "${skill_file}")" != "---" ]]; then + echo "${skill_name}: SKILL.md must start with YAML frontmatter delimiter" >&2 + status=1 + continue + fi + + end_line="$(awk 'NR > 1 && $0 == "---" { print NR; exit }' "${skill_file}")" + if [[ -z "${end_line}" ]]; then + echo "${skill_name}: SKILL.md missing closing YAML frontmatter delimiter" >&2 + status=1 + continue + fi + + frontmatter="$(sed -n "2,$((end_line - 1))p" "${skill_file}")" + name="$(printf '%s\n' "${frontmatter}" | awk -F': *' '$1 == "name" { print $2; exit }' | tr -d '"'"'"'')" + + if [[ -z "${name}" ]]; then + echo "${skill_name}: missing required frontmatter field: name" >&2 + status=1 + elif [[ "${name}" != "${skill_name}" ]]; then + echo "${skill_name}: frontmatter name ${name} must match directory name" >&2 + status=1 + elif [[ ! "${name}" =~ ^[a-z0-9]+(-[a-z0-9]+)*$ ]]; then + echo "${skill_name}: name must be kebab-case" >&2 + status=1 + fi + + if [[ -n "${name}" ]]; then + if grep -Fxq "${name}" "${seen_names_file}"; then + echo "${skill_name}: duplicate skill name ${name}" >&2 + status=1 + fi + printf '%s\n' "${name}" >>"${seen_names_file}" + fi + + if ! printf '%s\n' "${frontmatter}" | grep -q '^description:'; then + echo "${skill_name}: missing required frontmatter field: description" >&2 + status=1 + fi + + if [[ "$(sed -n "$((end_line + 1))p" "${skill_file}")" == "" ]]; then + # Empty separator line is fine, but the body still needs content. + body_start=$((end_line + 2)) + else + body_start=$((end_line + 1)) + fi + if ! tail -n +"${body_start}" "${skill_file}" | grep -q '[^[:space:]]'; then + echo "${skill_name}: SKILL.md body must not be empty" >&2 + status=1 + fi +done + +if [[ ${status} -eq 0 ]]; then + echo "Validated $(wc -l <"${seen_names_file}" | tr -d ' ') skills" +fi + +exit "${status}" diff --git a/skills/a7-persona-developer/SKILL.md b/skills/a7-persona-developer/SKILL.md index eb50cd2..8d2bc7c 100644 --- a/skills/a7-persona-developer/SKILL.md +++ b/skills/a7-persona-developer/SKILL.md @@ -3,7 +3,7 @@ name: a7-persona-developer description: >- Persona skill for API developers building and testing APIs on API7 Enterprise Edition (API7 EE) using the a7 CLI. Provides decision frameworks for API design, Service Template - lifecycle, Portal publishing, plugin configuration, and local-to-cloud development workflows. + lifecycle, route publication, plugin configuration, and local-to-cloud development workflows. version: "1.0.0" author: API7.ai Contributors license: Apache-2.0 @@ -14,9 +14,8 @@ metadata: - a7 service-template create - a7 service-template publish - a7 route create - - a7 upstream create + - a7 service create - a7 consumer create - - a7 portal list - a7 config sync - a7 config validate - a7 debug trace @@ -29,7 +28,7 @@ metadata: You are an **API Developer** responsible for: - Designing API schemas and configuring routes within a **Gateway Group**. - Leveraging **Service Templates** to standardize API deployments across environments. -- Publishing APIs to the **Developer Portal** for internal or external consumption. +- Publishing APIs to gateway groups with service-backed routes. - Configuring advanced enterprise plugins (OIDC, Canary, Request/Response Transformation). - Debugging complex request flows using built-in enterprise tracing tools. @@ -39,7 +38,7 @@ In API7 EE, developers work within a structured lifecycle: 1. **Gateway Groups**: Your assigned workspace (e.g., `ecommerce-dev`). 2. **Service Templates**: Blueprints for services (e.g., `payment-service-v1`). 3. **Publication**: Promoting a Service Template to a live Gateway Group. -4. **Developer Portal**: The consumer-facing documentation and self-service hub. +4. **Service-backed Routes**: Routes should reference a service with `service_id` in current API7 EE. ## Getting Started @@ -115,13 +114,6 @@ a7 route create -g staging-group -f - <<'EOF' EOF ``` -### Step 4: Add to Developer Portal - -```bash -# Link your service to the Portal for documentation -a7 portal publish -g staging-group --service user-service-template --portal public-portal -``` - ## Plugin Selection Guide (Enterprise Edition) ### Identity & Security @@ -199,7 +191,7 @@ Automate your API lifecycle using `a7` in your pipelines. |-----------|--------|---------| | Standardizing multiple APIs | Use a Service Template | `a7 service-template create` | | Promoting to production | Publish Template to Group | `a7 service-template publish` | -| Exposing to external devs | Add to Developer Portal | `a7 portal publish` | +| Exposing an API path | Create or update a service-backed route | `a7 route create -f route.yaml` | | Backend URI mismatch | Use `proxy-rewrite` | `a7 route update ...` | | Testing Canary version | Use `traffic-split` | `a7 route update ...` | | Auth failure (401) | Check Trace & Logs | `a7 debug trace` & `a7 debug logs` | @@ -213,4 +205,4 @@ Automate your API lifecycle using `a7` in your pipelines. 5. **Declarative Sync**: Prefer `a7 config sync` for complex multi-route deployments. 6. **Documentation**: Always provide a description (`--desc`) for routes and templates for colleagues. 7. **Trace Verbosity**: Use `--verbose` in `debug trace` to inspect plugin input/output headers. -8. **Portal Sync**: Keep your Swagger/OpenAPI docs in sync with your Portal publication. +8. **Route Model**: Prefer `service create` plus `route create` with `service_id`; avoid standalone upstream workflows for API7 EE. diff --git a/skills/a7-persona-operator/SKILL.md b/skills/a7-persona-operator/SKILL.md index 5ccbe02..40dcf7d 100644 --- a/skills/a7-persona-operator/SKILL.md +++ b/skills/a7-persona-operator/SKILL.md @@ -14,12 +14,11 @@ metadata: - a7 gateway-group list - a7 gateway-group create - a7 route list - - a7 upstream health + - a7 service list - a7 config sync - a7 config dump - a7 debug logs - a7 debug trace - - a7 health --- # a7-persona-operator @@ -54,9 +53,6 @@ a7 context create prod-ee \ # Switch context a7 context use prod-ee -# Verify connectivity -a7 health - # List available Gateway Groups a7 gateway-group list ``` @@ -67,13 +63,14 @@ a7 gateway-group list ```bash # Check if the Dashboard and CLI are connected -a7 health +a7 gateway-group list # Verify status of a specific Gateway Group a7 gateway-group get internal-apps -# Check upstream health within a group -a7 upstream health -g internal-apps +# Inspect deployed services and routes within a group +a7 service list -g internal-apps +a7 route list -g internal-apps ``` ### 2. Configuration Audit & Drift Detection @@ -199,7 +196,7 @@ EOF |-----------|--------|---------| | New Team Onboarding | Create Gateway Group & Assign RBAC | `a7 gateway-group create ` | | Configuration Drift | Compare local YAML with Live | `a7 config diff -g -f ` | -| Upstream Timeout | Check health & logs | `a7 upstream health -g ` | +| Backend Timeout | Check route/service config and logs | `a7 route get -g ` and `a7 debug logs` | | Security Breach | Block IP via Global Rule | `a7 global-rule create -g -f block.json` | | Compliance Audit | Dump all configs for review | `a7 config dump -g ` | | Version Upgrade | Validate then Sync | `a7 config validate` then `a7 config sync` | @@ -212,5 +209,5 @@ EOF 4. **Audit Logs**: Regularly review the Dashboard audit logs for any CLI-initiated changes. 5. **HTTPS Only**: Always use the HTTPS port (`7443`) for the Control Plane. 6. **Config as Code**: Store all Gateway Group configurations in Git. Treat the Dashboard as a projection of your repository. -7. **Health Checks**: Always enable active health checks for upstreams to allow the gateway to self-heal. +7. **Backend Health**: Manage backend health through service/route upstream configuration and gateway observability. 8. **Context Awareness**: Use descriptive names for contexts (e.g., `hk-region-prod`, `us-west-staging`) to avoid confusion in multi-region setups. diff --git a/skills/a7-recipe-api-versioning/SKILL.md b/skills/a7-recipe-api-versioning/SKILL.md index 5f0fdf9..5c7006b 100644 --- a/skills/a7-recipe-api-versioning/SKILL.md +++ b/skills/a7-recipe-api-versioning/SKILL.md @@ -13,7 +13,8 @@ metadata: a7_commands: - a7 route create - a7 route update - - a7 upstream create + - a7 service create + - a7 service update - a7 config sync - a7 gateway-group get --- diff --git a/skills/a7-recipe-blue-green/SKILL.md b/skills/a7-recipe-blue-green/SKILL.md index 04eaaef..a21713b 100644 --- a/skills/a7-recipe-blue-green/SKILL.md +++ b/skills/a7-recipe-blue-green/SKILL.md @@ -12,8 +12,8 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 upstream create - - a7 upstream update + - a7 service create + - a7 service update - a7 route create - a7 route update - a7 config sync diff --git a/skills/a7-recipe-canary/SKILL.md b/skills/a7-recipe-canary/SKILL.md index 743692b..4c3d718 100644 --- a/skills/a7-recipe-canary/SKILL.md +++ b/skills/a7-recipe-canary/SKILL.md @@ -12,7 +12,7 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 upstream create + - a7 service create - a7 route create - a7 route update - a7 route get diff --git a/skills/a7-recipe-graphql-proxy/SKILL.md b/skills/a7-recipe-graphql-proxy/SKILL.md index 1b3ad5b..a589089 100644 --- a/skills/a7-recipe-graphql-proxy/SKILL.md +++ b/skills/a7-recipe-graphql-proxy/SKILL.md @@ -14,7 +14,7 @@ metadata: - a7 route create - a7 route update - a7 config sync - - a7 consumer-restriction create + - a7 consumer create --- # a7-recipe-graphql-proxy diff --git a/skills/a7-recipe-health-check/SKILL.md b/skills/a7-recipe-health-check/SKILL.md index 790bc08..2da3444 100644 --- a/skills/a7-recipe-health-check/SKILL.md +++ b/skills/a7-recipe-health-check/SKILL.md @@ -12,10 +12,10 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 upstream create - - a7 upstream update - - a7 upstream get - - a7 upstream health + - a7 service create + - a7 service update + - a7 service get + - a7 config sync --- # a7-recipe-health-check @@ -192,11 +192,13 @@ a7 upstream create --gateway-group default -f - <<'EOF' EOF ``` -### 4. Check upstream health status +### 4. Verify the referencing route and service ```bash -# View health status of all nodes in a gateway group -a7 upstream health backend --gateway-group default +# Current a7 does not expose a standalone upstream-health command. +# Verify the service/route wiring and use gateway observability for node state. +a7 service get backend --gateway-group default --output json +a7 route list --gateway-group default --output json ``` ## Common Patterns @@ -321,6 +323,6 @@ routes: | Probe hitting wrong endpoint | Default `http_path` is `/` | Set `http_path` to your actual health endpoint | | TLS probe fails | Certificate verification fails | Set `https_verify_certificate: false` or fix certificates | | Health checks too aggressive | Low thresholds with flaky endpoints | Increase `failures` threshold and `interval` | -| `a7 upstream health` shows no data | API7 EE hasn't started health checks yet | Wait for the first probe interval to complete | +| No standalone health command | Current a7 does not expose upstream health status | Verify service/route config with `a7 service get` and use gateway observability | | Command failed with 401 | Invalid token | Refresh your token using `a7 context create` | | Upstream not found | Different gateway group | Ensure `--gateway-group` matches the group where upstream was created | diff --git a/skills/a7-recipe-mtls/SKILL.md b/skills/a7-recipe-mtls/SKILL.md index 25dc4cf..41d51d5 100644 --- a/skills/a7-recipe-mtls/SKILL.md +++ b/skills/a7-recipe-mtls/SKILL.md @@ -17,8 +17,8 @@ metadata: - a7 ssl list - a7 ssl get - a7 ssl delete - - a7 upstream create - - a7 upstream update + - a7 service create + - a7 service update - a7 route create --- diff --git a/test/e2e/skills/skills_test.go b/test/e2e/skills/skills_test.go new file mode 100644 index 0000000..472c32e --- /dev/null +++ b/test/e2e/skills/skills_test.go @@ -0,0 +1,146 @@ +//go:build e2e + +package skills + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +var skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) + +func repoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("failed to locate repository root") + } + dir = parent + } +} + +func frontmatter(t *testing.T, file string) map[string]string { + t.Helper() + data, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + lines := strings.Split(string(data), "\n") + if len(lines) < 3 || lines[0] != "---" { + t.Fatalf("%s: missing opening frontmatter delimiter", file) + } + end := -1 + for i := 1; i < len(lines); i++ { + if lines[i] == "---" { + end = i + break + } + } + if end == -1 { + t.Fatalf("%s: missing closing frontmatter delimiter", file) + } + fields := map[string]string{} + for _, line := range lines[1:end] { + key, value, ok := strings.Cut(line, ":") + if !ok { + continue + } + key = strings.TrimSpace(key) + value = strings.Trim(strings.TrimSpace(value), `"`) + if key != "" && value != "" { + fields[key] = value + } + } + return fields +} + +func TestSkillFrontmatterMatchesDirectories(t *testing.T) { + root := repoRoot(t) + entries, err := os.ReadDir(filepath.Join(root, "skills")) + if err != nil { + t.Fatal(err) + } + if len(entries) == 0 { + t.Fatal("expected at least one skill") + } + seen := map[string]bool{} + for _, entry := range entries { + if !entry.IsDir() { + continue + } + name := entry.Name() + file := filepath.Join(root, "skills", name, "SKILL.md") + fields := frontmatter(t, file) + if fields["name"] != name { + t.Fatalf("%s: frontmatter name %q must match directory name", file, fields["name"]) + } + if !skillNamePattern.MatchString(fields["name"]) { + t.Fatalf("%s: skill name must be kebab-case", file) + } + if fields["description"] == "" { + t.Fatalf("%s: description is required", file) + } + if seen[fields["name"]] { + t.Fatalf("duplicate skill name %q", fields["name"]) + } + seen[fields["name"]] = true + } +} + +func TestSkillsDoNotReferenceRemovedA7Commands(t *testing.T) { + root := repoRoot(t) + disallowed := []string{ + "a7 health", + "a7 portal", + "a7 upstream health", + "a7 consumer-restriction create", + } + for _, pattern := range disallowed { + matches, err := filepath.Glob(filepath.Join(root, "skills", "*", "SKILL.md")) + if err != nil { + t.Fatal(err) + } + for _, file := range matches { + data, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), pattern) { + t.Fatalf("%s: references removed or unsupported command %q", file, pattern) + } + } + } +} + +func TestSkillsDocumentationReferencesExistingSkills(t *testing.T) { + root := repoRoot(t) + data, err := os.ReadFile(filepath.Join(root, "docs", "skills.md")) + if err != nil { + t.Fatal(err) + } + doc := string(data) + staleSkillNames := []string{ + "a7-recipe-gateway-group", + "a7-persona-platform-eng", + "a7-recipe-service-template", + "a7-plugin-ai-rag", + "a7-plugin-ai-token-limiter", + "a7-recipe-service-registry", + } + for _, name := range staleSkillNames { + if strings.Contains(doc, name) { + t.Fatalf("docs/skills.md references missing skill %q", name) + } + } +} From 9a32de1983864a2e4c3cd428654f059304ac00b3 Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 11:53:55 +0800 Subject: [PATCH 2/7] ci: split skills validation workflow --- .github/workflows/ci.yml | 4 +- .github/workflows/e2e.yml | 3 - .github/workflows/skills.yml | 23 +++++++ Makefile | 7 +- test/e2e/skills/skills_test.go | 116 ++++++++++++++++++++++++++++++--- 5 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/skills.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b491015..002d091 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [main] + branches: [main, master] pull_request: - branches: [main] + branches: [main, master] jobs: test: diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 6ba64be..1fad288 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -21,9 +21,6 @@ jobs: - name: Download Go modules run: go mod download - - name: Validate skills - run: make validate-skills - - name: Run E2E tests env: A7_ADMIN_URL: ${{ secrets.DEMO_WEBSITE }} diff --git a/.github/workflows/skills.yml b/.github/workflows/skills.yml new file mode 100644 index 0000000..9a58675 --- /dev/null +++ b/.github/workflows/skills.yml @@ -0,0 +1,23 @@ +name: Skills Validation +on: + pull_request: + branches: [main, master] + push: + branches: [main, master] + workflow_dispatch: + +jobs: + validate-skills: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version: "1.23.0" + + - name: Validate skills + run: make validate-skills + + - name: Test skills + run: make test-skills diff --git a/Makefile b/Makefile index bd28899..8f58b03 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ LDFLAGS := -s -w \ -X $(MODULE)/internal/version.Commit=$(COMMIT) \ -X $(MODULE)/internal/version.Date=$(DATE) -.PHONY: build test test-verbose lint fmt vet check install clean docker-up docker-down validate-skills test-e2e test-e2e-full +.PHONY: build test test-verbose lint fmt vet check install clean docker-up docker-down validate-skills test-skills test-e2e test-e2e-full build: go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/a7 @@ -29,11 +29,14 @@ fmt: vet: go vet ./... -check: fmt vet lint test validate-skills +check: fmt vet lint test validate-skills test-skills validate-skills: ./scripts/validate-skills.sh +test-skills: + go test ./test/e2e/skills -tags=e2e -count=1 + install: build cp bin/$(BINARY) $(GOPATH)/bin/$(BINARY) diff --git a/test/e2e/skills/skills_test.go b/test/e2e/skills/skills_test.go index 472c32e..bbce067 100644 --- a/test/e2e/skills/skills_test.go +++ b/test/e2e/skills/skills_test.go @@ -4,6 +4,7 @@ package skills import ( "os" + "os/exec" "path/filepath" "regexp" "strings" @@ -11,26 +12,62 @@ import ( ) var skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) +var a7Binary string -func repoRoot(t *testing.T) string { - t.Helper() +func locateRepoRoot() (string, error) { dir, err := os.Getwd() if err != nil { - t.Fatal(err) + return "", err } for { if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { - return dir + return dir, nil } parent := filepath.Dir(dir) if parent == dir { - t.Fatal("failed to locate repository root") + return "", os.ErrNotExist } dir = parent } } -func frontmatter(t *testing.T, file string) map[string]string { +func TestMain(m *testing.M) { + root, err := locateRepoRoot() + if err != nil { + os.Exit(1) + } + tmpDir, err := os.MkdirTemp("", "a7-skills-test-*") + if err != nil { + os.Exit(1) + } + defer os.RemoveAll(tmpDir) + + a7Binary = filepath.Join(tmpDir, "a7") + cmd := exec.Command("go", "build", "-o", a7Binary, "./cmd/a7") + cmd.Dir = root + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + os.Exit(1) + } + os.Exit(m.Run()) +} + +func repoRoot(t *testing.T) string { + t.Helper() + root, err := locateRepoRoot() + if err != nil { + t.Fatal("failed to locate repository root") + } + return root +} + +type skillMetadata struct { + Fields map[string]string + A7Commands []string +} + +func frontmatter(t *testing.T, file string) skillMetadata { t.Helper() data, err := os.ReadFile(file) if err != nil { @@ -50,19 +87,36 @@ func frontmatter(t *testing.T, file string) map[string]string { if end == -1 { t.Fatalf("%s: missing closing frontmatter delimiter", file) } - fields := map[string]string{} + metadata := skillMetadata{Fields: map[string]string{}} + inA7Commands := false for _, line := range lines[1:end] { key, value, ok := strings.Cut(line, ":") + trimmed := strings.TrimSpace(line) + if inA7Commands { + if strings.HasPrefix(trimmed, "- ") { + command := strings.TrimSpace(strings.TrimPrefix(trimmed, "- ")) + if command != "" { + metadata.A7Commands = append(metadata.A7Commands, command) + } + continue + } + if trimmed != "" && !strings.HasPrefix(line, " ") { + inA7Commands = false + } + } if !ok { continue } key = strings.TrimSpace(key) value = strings.Trim(strings.TrimSpace(value), `"`) if key != "" && value != "" { - fields[key] = value + metadata.Fields[key] = value + } + if key == "a7_commands" { + inA7Commands = true } } - return fields + return metadata } func TestSkillFrontmatterMatchesDirectories(t *testing.T) { @@ -81,7 +135,8 @@ func TestSkillFrontmatterMatchesDirectories(t *testing.T) { } name := entry.Name() file := filepath.Join(root, "skills", name, "SKILL.md") - fields := frontmatter(t, file) + metadata := frontmatter(t, file) + fields := metadata.Fields if fields["name"] != name { t.Fatalf("%s: frontmatter name %q must match directory name", file, fields["name"]) } @@ -98,6 +153,47 @@ func TestSkillFrontmatterMatchesDirectories(t *testing.T) { } } +func TestSkillDeclaredA7CommandsExist(t *testing.T) { + root := repoRoot(t) + matches, err := filepath.Glob(filepath.Join(root, "skills", "*", "SKILL.md")) + if err != nil { + t.Fatal(err) + } + for _, file := range matches { + metadata := frontmatter(t, file) + for _, command := range metadata.A7Commands { + fields := strings.Fields(command) + if len(fields) == 0 { + continue + } + if fields[0] != "a7" { + t.Fatalf("%s: a7_commands entry %q must start with a7", file, command) + } + args := append(fields[1:], "--help") + cmd := exec.Command(a7Binary, args...) + cmd.Dir = root + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s: command %q is not supported by current a7 CLI: %v\n%s", file, command, err, string(output)) + } + } + } +} + +func TestPluginSkillsDeclarePluginName(t *testing.T) { + root := repoRoot(t) + matches, err := filepath.Glob(filepath.Join(root, "skills", "a7-plugin-*", "SKILL.md")) + if err != nil { + t.Fatal(err) + } + for _, file := range matches { + metadata := frontmatter(t, file) + if metadata.Fields["plugin_name"] == "" { + t.Fatalf("%s: plugin skills must declare metadata.plugin_name", file) + } + } +} + func TestSkillsDoNotReferenceRemovedA7Commands(t *testing.T) { root := repoRoot(t) disallowed := []string{ From ae43e92290bd7964c7f0148e2b2edfd99e858b8b Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 11:58:44 +0800 Subject: [PATCH 3/7] test: fix lint violations --- pkg/api/client_test.go | 12 +++++++++--- pkg/cmd/consumer-group/create/create_test.go | 2 +- pkg/cmd/consumer-group/get/get_test.go | 2 +- pkg/cmd/consumer-group/list/list_test.go | 2 +- pkg/cmd/consumer/delete/delete.go | 3 ++- pkg/cmd/consumer/get/get.go | 3 ++- pkg/cmd/consumer/update/update.go | 3 ++- 7 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/api/client_test.go b/pkg/api/client_test.go index 1632171..5f31a4a 100644 --- a/pkg/api/client_test.go +++ b/pkg/api/client_test.go @@ -21,7 +21,9 @@ func TestClient_Get(t *testing.T) { t.Errorf("expected path /routes, got %s", r.URL.Path) } w.WriteHeader(http.StatusOK) - w.Write([]byte(expectedBody)) + if _, err := w.Write([]byte(expectedBody)); err != nil { + t.Errorf("failed to write response: %v", err) + } })) defer server.Close() @@ -63,7 +65,9 @@ func TestClient_Post(t *testing.T) { } w.WriteHeader(http.StatusCreated) - w.Write([]byte(expectedBody)) + if _, err := w.Write([]byte(expectedBody)); err != nil { + t.Errorf("failed to write response: %v", err) + } })) defer server.Close() @@ -97,7 +101,9 @@ func TestClient_Put(t *testing.T) { } w.WriteHeader(http.StatusOK) - w.Write([]byte(expectedBody)) + if _, err := w.Write([]byte(expectedBody)); err != nil { + t.Errorf("failed to write response: %v", err) + } })) defer server.Close() diff --git a/pkg/cmd/consumer-group/create/create_test.go b/pkg/cmd/consumer-group/create/create_test.go index 974e49e..e801488 100644 --- a/pkg/cmd/consumer-group/create/create_test.go +++ b/pkg/cmd/consumer-group/create/create_test.go @@ -45,7 +45,7 @@ func TestCreateConsumerGroup_JSONOutput(t *testing.T) { } var item api.ConsumerGroup - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "cg1" || item.Desc != "group-1" { diff --git a/pkg/cmd/consumer-group/get/get_test.go b/pkg/cmd/consumer-group/get/get_test.go index 9d3ff6f..4b66548 100644 --- a/pkg/cmd/consumer-group/get/get_test.go +++ b/pkg/cmd/consumer-group/get/get_test.go @@ -63,7 +63,7 @@ func TestGetConsumerGroup_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.ConsumerGroup - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse json output: %v", err) } if item.ID != "cg1" { diff --git a/pkg/cmd/consumer-group/list/list_test.go b/pkg/cmd/consumer-group/list/list_test.go index dee7db5..ab28cb1 100644 --- a/pkg/cmd/consumer-group/list/list_test.go +++ b/pkg/cmd/consumer-group/list/list_test.go @@ -72,7 +72,7 @@ func TestListConsumerGroups_JSON(t *testing.T) { } var items []api.ConsumerGroup - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse json output: %v", err) } if len(items) != 1 || items[0].ID != "cg1" { diff --git a/pkg/cmd/consumer/delete/delete.go b/pkg/cmd/consumer/delete/delete.go index 3fde238..8f501f3 100644 --- a/pkg/cmd/consumer/delete/delete.go +++ b/pkg/cmd/consumer/delete/delete.go @@ -2,6 +2,7 @@ package delete import ( "bufio" + "errors" "fmt" "net/http" "strings" @@ -82,7 +83,7 @@ func actionRun(opts *Options) error { } _, err = client.Delete("/apisix/admin/consumers/"+opts.Username, map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return errors.New(cmdutil.FormatAPIError(err)) } _, err = fmt.Fprintf(opts.IO.Out, "consumer %q deleted\n", opts.Username) diff --git a/pkg/cmd/consumer/get/get.go b/pkg/cmd/consumer/get/get.go index d95dadb..5ebb8f9 100644 --- a/pkg/cmd/consumer/get/get.go +++ b/pkg/cmd/consumer/get/get.go @@ -2,6 +2,7 @@ package get import ( "encoding/json" + "errors" "fmt" "net/http" @@ -67,7 +68,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) body, err := client.Get("/apisix/admin/consumers/"+opts.Username, map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return errors.New(cmdutil.FormatAPIError(err)) } var item api.Consumer diff --git a/pkg/cmd/consumer/update/update.go b/pkg/cmd/consumer/update/update.go index 1b9f8a5..280e75c 100644 --- a/pkg/cmd/consumer/update/update.go +++ b/pkg/cmd/consumer/update/update.go @@ -2,6 +2,7 @@ package update import ( "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -99,7 +100,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) _, err = client.Put("/apisix/admin/consumers/"+opts.Username+"?gateway_group_id="+ggID, body) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return errors.New(cmdutil.FormatAPIError(err)) } output := opts.Output From 7b9bac87fc52a617210e79fb8181f9853c2f7e49 Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 12:05:42 +0800 Subject: [PATCH 4/7] ci: cancel stale workflow runs --- .github/workflows/ci.yml | 4 ++ .github/workflows/e2e.yml | 4 ++ .github/workflows/skills.yml | 4 ++ pkg/api/client_test.go | 67 ++++++++++--------- pkg/cmd/consumer-group/update/update_test.go | 2 +- pkg/cmd/consumer/create/create.go | 2 +- pkg/cmd/consumer/list/list.go | 2 +- pkg/cmd/credential/create/create_test.go | 2 +- pkg/cmd/credential/get/get_test.go | 2 +- pkg/cmd/credential/list/list_test.go | 2 +- pkg/cmd/credential/update/update_test.go | 2 +- pkg/cmd/global-rule/get/get_test.go | 2 +- pkg/cmd/global-rule/list/list_test.go | 2 +- pkg/cmd/plugin-config/create/create_test.go | 2 +- pkg/cmd/plugin-config/get/get_test.go | 2 +- pkg/cmd/plugin-config/list/list_test.go | 2 +- pkg/cmd/plugin-config/update/update_test.go | 2 +- pkg/cmd/plugin-metadata/create/create_test.go | 2 +- pkg/cmd/plugin-metadata/get/get_test.go | 2 +- pkg/cmd/plugin-metadata/update/update_test.go | 2 +- pkg/cmd/plugin/get/get.go | 2 +- pkg/cmd/plugin/get/get_test.go | 2 +- pkg/cmd/plugin/list/list.go | 2 +- pkg/cmd/proto/create/create_test.go | 2 +- pkg/cmd/proto/get/get_test.go | 2 +- pkg/cmd/proto/list/list_test.go | 2 +- pkg/cmd/proto/update/update_test.go | 2 +- pkg/cmd/secret/create/create_test.go | 2 +- pkg/cmd/secret/get/get_test.go | 2 +- pkg/cmd/secret/list/list_test.go | 2 +- pkg/cmd/secret/update/update_test.go | 2 +- pkg/cmd/service/create/create_test.go | 2 +- pkg/cmd/service/get/get_test.go | 2 +- pkg/cmd/service/list/list_test.go | 2 +- pkg/cmd/service/update/update_test.go | 2 +- pkg/cmd/ssl/create/create.go | 2 +- pkg/cmd/ssl/delete/delete.go | 2 +- pkg/cmd/ssl/get/get.go | 2 +- pkg/cmd/ssl/list/list.go | 2 +- pkg/cmd/ssl/update/update.go | 2 +- pkg/cmd/stream-route/create/create_test.go | 2 +- pkg/cmd/stream-route/get/get_test.go | 2 +- pkg/cmd/stream-route/list/list_test.go | 2 +- pkg/cmd/stream-route/update/update_test.go | 2 +- 44 files changed, 86 insertions(+), 73 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 002d091..643ac8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,10 @@ on: pull_request: branches: [main, master] +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: test: runs-on: ubuntu-latest diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 1fad288..29c52e2 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -6,6 +6,10 @@ on: branches: [main, master] workflow_dispatch: +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: e2e: runs-on: ubuntu-latest diff --git a/.github/workflows/skills.yml b/.github/workflows/skills.yml index 9a58675..3609397 100644 --- a/.github/workflows/skills.yml +++ b/.github/workflows/skills.yml @@ -6,6 +6,10 @@ on: branches: [main, master] workflow_dispatch: +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: validate-skills: runs-on: ubuntu-latest diff --git a/pkg/api/client_test.go b/pkg/api/client_test.go index 5f31a4a..b94be0d 100644 --- a/pkg/api/client_test.go +++ b/pkg/api/client_test.go @@ -9,6 +9,13 @@ import ( "testing" ) +func writeResponse(t *testing.T, w http.ResponseWriter, body []byte) { + t.Helper() + if _, err := w.Write(body); err != nil { + t.Errorf("failed to write response: %v", err) + } +} + // TestClient_Get tests the Get method with a successful response. func TestClient_Get(t *testing.T) { expectedBody := `{"id": 1, "name": "test"}` @@ -21,9 +28,7 @@ func TestClient_Get(t *testing.T) { t.Errorf("expected path /routes, got %s", r.URL.Path) } w.WriteHeader(http.StatusOK) - if _, err := w.Write([]byte(expectedBody)); err != nil { - t.Errorf("failed to write response: %v", err) - } + writeResponse(t, w, []byte(expectedBody)) })) defer server.Close() @@ -65,9 +70,7 @@ func TestClient_Post(t *testing.T) { } w.WriteHeader(http.StatusCreated) - if _, err := w.Write([]byte(expectedBody)); err != nil { - t.Errorf("failed to write response: %v", err) - } + writeResponse(t, w, []byte(expectedBody)) })) defer server.Close() @@ -101,9 +104,7 @@ func TestClient_Put(t *testing.T) { } w.WriteHeader(http.StatusOK) - if _, err := w.Write([]byte(expectedBody)); err != nil { - t.Errorf("failed to write response: %v", err) - } + writeResponse(t, w, []byte(expectedBody)) })) defer server.Close() @@ -161,7 +162,7 @@ func TestClient_GetWithQuery(t *testing.T) { } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"list": []}`)) + writeResponse(t, w, []byte(`{"list": []}`)) })) defer server.Close() @@ -185,7 +186,7 @@ func TestClient_APIError(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"error_msg":"bad request"}`)) + writeResponse(t, w, []byte(`{"error_msg":"bad request"}`)) })) defer server.Close() @@ -219,7 +220,7 @@ func TestClient_APIError_401(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"error_msg":"invalid api key"}`)) + writeResponse(t, w, []byte(`{"error_msg":"invalid api key"}`)) })) defer server.Close() @@ -294,7 +295,7 @@ func TestApiKeyTransport(t *testing.T) { } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"status": "ok"}`)) + writeResponse(t, w, []byte(`{"status": "ok"}`)) })) defer server.Close() @@ -341,7 +342,7 @@ func TestClient_Patch(t *testing.T) { } w.WriteHeader(http.StatusOK) - w.Write([]byte(expectedBody)) + writeResponse(t, w, []byte(expectedBody)) })) defer server.Close() @@ -465,7 +466,7 @@ func TestApiKeyTransport_MultipleRequests(t *testing.T) { t.Errorf("request %d: expected X-API-KEY header %q, got %q", requestCount, testAPIKey, apiKey) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() @@ -492,7 +493,7 @@ func TestClient_EmptyQuery(t *testing.T) { t.Errorf("expected empty query string, got %s", r.URL.RawQuery) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() @@ -522,7 +523,7 @@ func TestClient_MultipleQueryParams(t *testing.T) { } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"list": []}`)) + writeResponse(t, w, []byte(`{"list": []}`)) })) defer server.Close() @@ -552,7 +553,7 @@ func TestClient_JSONResponseParsing(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - w.Write(respJSON) + writeResponse(t, w, respJSON) })) defer server.Close() @@ -584,7 +585,7 @@ func TestClient_LargeResponseBody(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - w.Write(largeData) + writeResponse(t, w, largeData) })) defer server.Close() @@ -611,7 +612,7 @@ func TestClient_SpecialCharactersInQuery(t *testing.T) { t.Errorf("expected query param 'foo=bar&baz=qux', got %q", actual) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() @@ -634,7 +635,7 @@ func TestClient_NonJSONErrorResponse(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) w.Header().Set("Content-Type", "text/plain") - w.Write([]byte(plainTextError)) + writeResponse(t, w, []byte(plainTextError)) })) defer server.Close() @@ -700,7 +701,7 @@ func TestClient_Get_URLEncodingPreservation(t *testing.T) { t.Errorf("expected path /routes/test, got %s", r.URL.Path) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() @@ -720,7 +721,7 @@ func TestClient_PostWithNilBody(t *testing.T) { t.Errorf("expected Content-Type application/json, got %s", ct) } w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{"status": "created"}`)) + writeResponse(t, w, []byte(`{"status": "created"}`)) })) defer server.Close() @@ -744,7 +745,7 @@ func TestClient_ErrorResponse_WithPartialJSON(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(partialJSON)) + writeResponse(t, w, []byte(partialJSON)) })) defer server.Close() @@ -811,7 +812,7 @@ func TestClient_QueryMapWithEmptyValues(t *testing.T) { t.Errorf("expected empty filter value, got %q", actual) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() @@ -830,7 +831,7 @@ func TestClient_QueryMapWithEmptyValues(t *testing.T) { func TestClient_StatusCode_200(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"success": true}`)) + writeResponse(t, w, []byte(`{"success": true}`)) })) defer server.Close() @@ -850,7 +851,7 @@ func TestClient_StatusCode_200(t *testing.T) { func TestClient_StatusCode_201(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{"id": 1}`)) + writeResponse(t, w, []byte(`{"id": 1}`)) })) defer server.Close() @@ -890,7 +891,7 @@ func TestClient_StatusCode_204(t *testing.T) { func TestClient_StatusCode_400(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`{"error_msg": "bad request"}`)) + writeResponse(t, w, []byte(`{"error_msg": "bad request"}`)) })) defer server.Close() @@ -910,7 +911,7 @@ func TestClient_StatusCode_400(t *testing.T) { func TestClient_StatusCode_403(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusForbidden) - w.Write([]byte(`{"error_msg": "forbidden"}`)) + writeResponse(t, w, []byte(`{"error_msg": "forbidden"}`)) })) defer server.Close() @@ -939,7 +940,7 @@ func TestClient_StatusCode_403(t *testing.T) { func TestClient_StatusCode_404(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) - w.Write([]byte(`{"error_msg": "not found"}`)) + writeResponse(t, w, []byte(`{"error_msg": "not found"}`)) })) defer server.Close() @@ -968,7 +969,7 @@ func TestClient_StatusCode_404(t *testing.T) { func TestClient_StatusCode_500(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(`{"error_msg": "internal error"}`)) + writeResponse(t, w, []byte(`{"error_msg": "internal error"}`)) })) defer server.Close() @@ -997,7 +998,7 @@ func TestClient_StatusCode_500(t *testing.T) { func TestClient_StatusCode_502(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadGateway) - w.Write([]byte(`{"error_msg": "bad gateway"}`)) + writeResponse(t, w, []byte(`{"error_msg": "bad gateway"}`)) })) defer server.Close() @@ -1025,7 +1026,7 @@ func TestClient_BaseURLWithTrailingSlash(t *testing.T) { t.Errorf("expected path /api/routes, got %s", r.URL.Path) } w.WriteHeader(http.StatusOK) - w.Write([]byte(`{}`)) + writeResponse(t, w, []byte(`{}`)) })) defer server.Close() diff --git a/pkg/cmd/consumer-group/update/update_test.go b/pkg/cmd/consumer-group/update/update_test.go index 996eba7..d47c84a 100644 --- a/pkg/cmd/consumer-group/update/update_test.go +++ b/pkg/cmd/consumer-group/update/update_test.go @@ -45,7 +45,7 @@ func TestUpdateConsumerGroup_JSONOutput(t *testing.T) { } var item api.ConsumerGroup - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "cg1" || item.Desc != "updated" { diff --git a/pkg/cmd/consumer/create/create.go b/pkg/cmd/consumer/create/create.go index 0b7d028..d96b41c 100644 --- a/pkg/cmd/consumer/create/create.go +++ b/pkg/cmd/consumer/create/create.go @@ -113,7 +113,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) _, err = client.Post("/apisix/admin/consumers?gateway_group_id="+ggID, body) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } output := opts.Output diff --git a/pkg/cmd/consumer/list/list.go b/pkg/cmd/consumer/list/list.go index 6b24f89..75b40c3 100644 --- a/pkg/cmd/consumer/list/list.go +++ b/pkg/cmd/consumer/list/list.go @@ -77,7 +77,7 @@ func actionRun(opts *Options) error { } body, err := client.Get("/apisix/admin/consumers", query) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } var resp api.ListResponse[api.Consumer] diff --git a/pkg/cmd/credential/create/create_test.go b/pkg/cmd/credential/create/create_test.go index 6f14380..35228c4 100644 --- a/pkg/cmd/credential/create/create_test.go +++ b/pkg/cmd/credential/create/create_test.go @@ -45,7 +45,7 @@ func TestCreateCredential_JSONOutput(t *testing.T) { } var item api.Credential - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "cred1" || item.Desc != "first" { diff --git a/pkg/cmd/credential/get/get_test.go b/pkg/cmd/credential/get/get_test.go index 276506e..0c46d2a 100644 --- a/pkg/cmd/credential/get/get_test.go +++ b/pkg/cmd/credential/get/get_test.go @@ -64,7 +64,7 @@ func TestGetCredential_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.Credential - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "cred1" { diff --git a/pkg/cmd/credential/list/list_test.go b/pkg/cmd/credential/list/list_test.go index 691693d..815d1a0 100644 --- a/pkg/cmd/credential/list/list_test.go +++ b/pkg/cmd/credential/list/list_test.go @@ -67,7 +67,7 @@ func TestListCredentials_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var items []api.Credential - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse output: %v", err) } if len(items) != 1 || items[0].ID != "cred1" { diff --git a/pkg/cmd/credential/update/update_test.go b/pkg/cmd/credential/update/update_test.go index 9672ef1..af84580 100644 --- a/pkg/cmd/credential/update/update_test.go +++ b/pkg/cmd/credential/update/update_test.go @@ -45,7 +45,7 @@ func TestUpdateCredential_JSONOutput(t *testing.T) { } var item api.Credential - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "cred1" || item.Desc != "updated" { diff --git a/pkg/cmd/global-rule/get/get_test.go b/pkg/cmd/global-rule/get/get_test.go index e137345..307343b 100644 --- a/pkg/cmd/global-rule/get/get_test.go +++ b/pkg/cmd/global-rule/get/get_test.go @@ -73,7 +73,7 @@ func TestGetGlobalRule_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.GlobalRule - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "1" { diff --git a/pkg/cmd/global-rule/list/list_test.go b/pkg/cmd/global-rule/list/list_test.go index e16f829..044eb23 100644 --- a/pkg/cmd/global-rule/list/list_test.go +++ b/pkg/cmd/global-rule/list/list_test.go @@ -77,7 +77,7 @@ func TestListGlobalRules_JSON(t *testing.T) { t.Fatalf("listRun failed: %v", err) } var items []api.GlobalRule - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if len(items) != 1 || items[0].ID != "1" { diff --git a/pkg/cmd/plugin-config/create/create_test.go b/pkg/cmd/plugin-config/create/create_test.go index 6b87273..d01fb80 100644 --- a/pkg/cmd/plugin-config/create/create_test.go +++ b/pkg/cmd/plugin-config/create/create_test.go @@ -51,7 +51,7 @@ func TestCreatePluginConfig_Success(t *testing.T) { } var item api.PluginConfig - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "pc1" { diff --git a/pkg/cmd/plugin-config/get/get_test.go b/pkg/cmd/plugin-config/get/get_test.go index ca5dbe0..1aac5e2 100644 --- a/pkg/cmd/plugin-config/get/get_test.go +++ b/pkg/cmd/plugin-config/get/get_test.go @@ -79,7 +79,7 @@ func TestGetPluginConfig_JSON(t *testing.T) { } var item api.PluginConfig - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "pc1" { diff --git a/pkg/cmd/plugin-config/list/list_test.go b/pkg/cmd/plugin-config/list/list_test.go index 8aa3d4a..68645e4 100644 --- a/pkg/cmd/plugin-config/list/list_test.go +++ b/pkg/cmd/plugin-config/list/list_test.go @@ -83,7 +83,7 @@ func TestListPluginConfigs_JSON(t *testing.T) { } var items []api.PluginConfig - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if len(items) != 1 || items[0].ID != "pc1" { diff --git a/pkg/cmd/plugin-config/update/update_test.go b/pkg/cmd/plugin-config/update/update_test.go index 9f0601a..bcb511b 100644 --- a/pkg/cmd/plugin-config/update/update_test.go +++ b/pkg/cmd/plugin-config/update/update_test.go @@ -52,7 +52,7 @@ func TestUpdatePluginConfig_Success(t *testing.T) { } var item api.PluginConfig - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "pc1" { diff --git a/pkg/cmd/plugin-metadata/create/create_test.go b/pkg/cmd/plugin-metadata/create/create_test.go index 8f02bf7..e2972f4 100644 --- a/pkg/cmd/plugin-metadata/create/create_test.go +++ b/pkg/cmd/plugin-metadata/create/create_test.go @@ -49,7 +49,7 @@ func TestCreatePluginMetadata_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.PluginMetadata - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "key-auth" { diff --git a/pkg/cmd/plugin-metadata/get/get_test.go b/pkg/cmd/plugin-metadata/get/get_test.go index dcba6a4..3cc5ee4 100644 --- a/pkg/cmd/plugin-metadata/get/get_test.go +++ b/pkg/cmd/plugin-metadata/get/get_test.go @@ -74,7 +74,7 @@ func TestGetPluginMetadata_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.PluginMetadata - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "key-auth" { diff --git a/pkg/cmd/plugin-metadata/update/update_test.go b/pkg/cmd/plugin-metadata/update/update_test.go index 5607713..bf18371 100644 --- a/pkg/cmd/plugin-metadata/update/update_test.go +++ b/pkg/cmd/plugin-metadata/update/update_test.go @@ -49,7 +49,7 @@ func TestUpdatePluginMetadata_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.PluginMetadata - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "key-auth" { diff --git a/pkg/cmd/plugin/get/get.go b/pkg/cmd/plugin/get/get.go index 93c2140..49cf6fb 100644 --- a/pkg/cmd/plugin/get/get.go +++ b/pkg/cmd/plugin/get/get.go @@ -62,7 +62,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) body, err := client.Get("/apisix/admin/plugins/"+opts.Name, map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } var item map[string]interface{} diff --git a/pkg/cmd/plugin/get/get_test.go b/pkg/cmd/plugin/get/get_test.go index 649056d..a63e1f3 100644 --- a/pkg/cmd/plugin/get/get_test.go +++ b/pkg/cmd/plugin/get/get_test.go @@ -51,7 +51,7 @@ func TestPluginGet_JSONOutput(t *testing.T) { } var got map[string]interface{} - if err := json.Unmarshal([]byte(out.String()), &got); err != nil { + if err := json.Unmarshal(out.Bytes(), &got); err != nil { t.Fatalf("failed to parse output: %v", err) } if got["name"] != "key-auth" { diff --git a/pkg/cmd/plugin/list/list.go b/pkg/cmd/plugin/list/list.go index cf2b3fd..f1c344b 100644 --- a/pkg/cmd/plugin/list/list.go +++ b/pkg/cmd/plugin/list/list.go @@ -66,7 +66,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) body, err := client.Get("/apisix/admin/plugins/list", map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } var plugins []string diff --git a/pkg/cmd/proto/create/create_test.go b/pkg/cmd/proto/create/create_test.go index 5685aca..b3b8e9a 100644 --- a/pkg/cmd/proto/create/create_test.go +++ b/pkg/cmd/proto/create/create_test.go @@ -50,7 +50,7 @@ func TestCreateProto_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.Proto - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "p1" || item.Desc != "d1" { diff --git a/pkg/cmd/proto/get/get_test.go b/pkg/cmd/proto/get/get_test.go index 685730f..266a586 100644 --- a/pkg/cmd/proto/get/get_test.go +++ b/pkg/cmd/proto/get/get_test.go @@ -78,7 +78,7 @@ func TestGetProto_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.Proto - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "p1" || item.Desc != "d1" { diff --git a/pkg/cmd/proto/list/list_test.go b/pkg/cmd/proto/list/list_test.go index c756dfb..9b42dc0 100644 --- a/pkg/cmd/proto/list/list_test.go +++ b/pkg/cmd/proto/list/list_test.go @@ -72,7 +72,7 @@ func TestListProtos_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var items []api.Proto - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse output: %v", err) } if len(items) != 1 || items[0].ID != "p1" { diff --git a/pkg/cmd/proto/update/update_test.go b/pkg/cmd/proto/update/update_test.go index 89d7a30..e3453ec 100644 --- a/pkg/cmd/proto/update/update_test.go +++ b/pkg/cmd/proto/update/update_test.go @@ -51,7 +51,7 @@ func TestUpdateProto_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.Proto - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "p1" || item.Desc != "d2" { diff --git a/pkg/cmd/secret/create/create_test.go b/pkg/cmd/secret/create/create_test.go index e348a0e..68afed5 100644 --- a/pkg/cmd/secret/create/create_test.go +++ b/pkg/cmd/secret/create/create_test.go @@ -54,7 +54,7 @@ func TestCreateSecret_JSON(t *testing.T) { } var item api.Secret - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "vault/s1" || item.URI != "http://vault" { diff --git a/pkg/cmd/secret/get/get_test.go b/pkg/cmd/secret/get/get_test.go index e52f9f7..681f8ac 100644 --- a/pkg/cmd/secret/get/get_test.go +++ b/pkg/cmd/secret/get/get_test.go @@ -82,7 +82,7 @@ func TestGetSecret_JSON(t *testing.T) { } var item api.Secret - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "vault/s1" || item.Prefix != "kv" { diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 72893f3..8e8fc95 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -88,7 +88,7 @@ func TestListSecrets_JSON(t *testing.T) { } var items []api.Secret - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if len(items) != 1 || items[0].ID != "vault/s1" { diff --git a/pkg/cmd/secret/update/update_test.go b/pkg/cmd/secret/update/update_test.go index 0e42981..0942044 100644 --- a/pkg/cmd/secret/update/update_test.go +++ b/pkg/cmd/secret/update/update_test.go @@ -54,7 +54,7 @@ func TestUpdateSecret_JSON(t *testing.T) { } var item api.Secret - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } if item.ID != "vault/s1" || item.Prefix != "kv2" { diff --git a/pkg/cmd/service/create/create_test.go b/pkg/cmd/service/create/create_test.go index abf6cf7..7960dfa 100644 --- a/pkg/cmd/service/create/create_test.go +++ b/pkg/cmd/service/create/create_test.go @@ -55,7 +55,7 @@ func TestCreateService_Success(t *testing.T) { } var item api.Service - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "s1" { diff --git a/pkg/cmd/service/get/get_test.go b/pkg/cmd/service/get/get_test.go index 290072a..846a57d 100644 --- a/pkg/cmd/service/get/get_test.go +++ b/pkg/cmd/service/get/get_test.go @@ -75,7 +75,7 @@ func TestGetService_JSON(t *testing.T) { t.Fatalf("actionRun failed: %v", err) } var item api.Service - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "s1" { diff --git a/pkg/cmd/service/list/list_test.go b/pkg/cmd/service/list/list_test.go index 67eff34..3d04b0f 100644 --- a/pkg/cmd/service/list/list_test.go +++ b/pkg/cmd/service/list/list_test.go @@ -95,7 +95,7 @@ func TestListServices_JSON(t *testing.T) { } var items []api.Service - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if len(items) != 1 || items[0].ID != "s1" { diff --git a/pkg/cmd/service/update/update_test.go b/pkg/cmd/service/update/update_test.go index 08a18bf..1a0f7d7 100644 --- a/pkg/cmd/service/update/update_test.go +++ b/pkg/cmd/service/update/update_test.go @@ -55,7 +55,7 @@ func TestUpdateService_Success(t *testing.T) { } var item api.Service - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "s1" { diff --git a/pkg/cmd/ssl/create/create.go b/pkg/cmd/ssl/create/create.go index 769b997..c2627cb 100644 --- a/pkg/cmd/ssl/create/create.go +++ b/pkg/cmd/ssl/create/create.go @@ -141,7 +141,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) _, err = client.Post("/apisix/admin/ssls?gateway_group_id="+ggID, body) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } output := opts.Output diff --git a/pkg/cmd/ssl/delete/delete.go b/pkg/cmd/ssl/delete/delete.go index bb3af88..a5fad27 100644 --- a/pkg/cmd/ssl/delete/delete.go +++ b/pkg/cmd/ssl/delete/delete.go @@ -82,7 +82,7 @@ func actionRun(opts *Options) error { } _, err = client.Delete("/apisix/admin/ssls/"+opts.ID, map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } _, err = fmt.Fprintf(opts.IO.Out, "ssl %q deleted\n", opts.ID) diff --git a/pkg/cmd/ssl/get/get.go b/pkg/cmd/ssl/get/get.go index 066028c..e1f89b9 100644 --- a/pkg/cmd/ssl/get/get.go +++ b/pkg/cmd/ssl/get/get.go @@ -67,7 +67,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) body, err := client.Get("/apisix/admin/ssls/"+opts.ID, map[string]string{"gateway_group_id": ggID}) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } var item api.SSL diff --git a/pkg/cmd/ssl/list/list.go b/pkg/cmd/ssl/list/list.go index 33a176c..7d57c09 100644 --- a/pkg/cmd/ssl/list/list.go +++ b/pkg/cmd/ssl/list/list.go @@ -77,7 +77,7 @@ func actionRun(opts *Options) error { } body, err := client.Get("/apisix/admin/ssls", query) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } var resp api.ListResponse[api.SSL] diff --git a/pkg/cmd/ssl/update/update.go b/pkg/cmd/ssl/update/update.go index 5f3cf17..e143440 100644 --- a/pkg/cmd/ssl/update/update.go +++ b/pkg/cmd/ssl/update/update.go @@ -124,7 +124,7 @@ func actionRun(opts *Options) error { client := api.NewClient(httpClient, cfg.BaseURL()) _, err = client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, body) if err != nil { - return fmt.Errorf(cmdutil.FormatAPIError(err)) + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } output := opts.Output diff --git a/pkg/cmd/stream-route/create/create_test.go b/pkg/cmd/stream-route/create/create_test.go index a870823..b2f18eb 100644 --- a/pkg/cmd/stream-route/create/create_test.go +++ b/pkg/cmd/stream-route/create/create_test.go @@ -51,7 +51,7 @@ func TestCreateStreamRoute_Success(t *testing.T) { } var item api.StreamRoute - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "sr1" { diff --git a/pkg/cmd/stream-route/get/get_test.go b/pkg/cmd/stream-route/get/get_test.go index d16f02c..68a55e6 100644 --- a/pkg/cmd/stream-route/get/get_test.go +++ b/pkg/cmd/stream-route/get/get_test.go @@ -89,7 +89,7 @@ func TestGetStreamRoute_JSON(t *testing.T) { } var item api.StreamRoute - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "sr1" { diff --git a/pkg/cmd/stream-route/list/list_test.go b/pkg/cmd/stream-route/list/list_test.go index da1a8e8..ac3e920 100644 --- a/pkg/cmd/stream-route/list/list_test.go +++ b/pkg/cmd/stream-route/list/list_test.go @@ -91,7 +91,7 @@ func TestListStreamRoutes_JSON(t *testing.T) { } var items []api.StreamRoute - if err := json.Unmarshal([]byte(out.String()), &items); err != nil { + if err := json.Unmarshal(out.Bytes(), &items); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if len(items) != 1 || items[0].ID != "sr1" { diff --git a/pkg/cmd/stream-route/update/update_test.go b/pkg/cmd/stream-route/update/update_test.go index a66e8d5..689ffae 100644 --- a/pkg/cmd/stream-route/update/update_test.go +++ b/pkg/cmd/stream-route/update/update_test.go @@ -52,7 +52,7 @@ func TestUpdateStreamRoute_Success(t *testing.T) { } var item api.StreamRoute - if err := json.Unmarshal([]byte(out.String()), &item); err != nil { + if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse JSON output: %v", err) } if item.ID != "sr1" { From 6048f80318b98cae2d53fb97f63e9e13b9edb658 Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 12:23:13 +0800 Subject: [PATCH 5/7] ci: clarify workflow check names --- .github/workflows/ci.yml | 2 ++ .github/workflows/skills.yml | 1 + 2 files changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 643ac8c..cd87a6b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,6 +12,7 @@ concurrency: jobs: test: + name: Go Test (${{ matrix.go-version }}) runs-on: ubuntu-latest strategy: matrix: @@ -33,6 +34,7 @@ jobs: run: go vet ./... lint: + name: Go Lint runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/skills.yml b/.github/workflows/skills.yml index 3609397..17175c5 100644 --- a/.github/workflows/skills.yml +++ b/.github/workflows/skills.yml @@ -12,6 +12,7 @@ concurrency: jobs: validate-skills: + name: Skills Metadata & Command Validation runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 From 769d45394be8f2b29f0945765573d9686a517661 Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 12:26:16 +0800 Subject: [PATCH 6/7] ci: use go mod version for tests --- .github/workflows/ci.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd87a6b..3d463de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,17 +12,14 @@ concurrency: jobs: test: - name: Go Test (${{ matrix.go-version }}) + name: Go Test runs-on: ubuntu-latest - strategy: - matrix: - go-version: ["1.22", "1.23"] steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} + go-version-file: go.mod - name: Build run: go build ./... @@ -41,8 +38,8 @@ jobs: - uses: actions/setup-go@v5 with: - go-version: "1.22" + go-version-file: go.mod - uses: golangci/golangci-lint-action@v6 with: - version: latest + version: v1.64.8 From 1672e51e6215d8c17d14d6a90df0911f767d4935 Mon Sep 17 00:00:00 2001 From: Qi Guo <979918879@qq.com> Date: Tue, 28 Apr 2026 15:28:13 +0800 Subject: [PATCH 7/7] test: address skills validation review --- Makefile | 2 +- docs/testing-strategy.md | 8 +- scripts/validate-skills.sh | 31 +++++++- skills/a7-recipe-api-versioning/SKILL.md | 3 +- skills/a7-recipe-blue-green/SKILL.md | 3 +- skills/a7-recipe-canary/SKILL.md | 3 +- skills/a7-recipe-health-check/SKILL.md | 11 ++- skills/a7-recipe-mtls/SKILL.md | 3 +- test/e2e/skills/skills_test.go | 98 +++++++++++++++++++----- 9 files changed, 122 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 8f58b03..00e8114 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ vet: check: fmt vet lint test validate-skills test-skills validate-skills: - ./scripts/validate-skills.sh + bash ./scripts/validate-skills.sh test-skills: go test ./test/e2e/skills -tags=e2e -count=1 diff --git a/docs/testing-strategy.md b/docs/testing-strategy.md index 57e80f7..a30d446 100644 --- a/docs/testing-strategy.md +++ b/docs/testing-strategy.md @@ -68,10 +68,10 @@ Optional for live gateway/data-plane coverage: - `A7_GATEWAY_GROUP` - `A7_E2E_ENABLE_GATEWAY_GROUP_CRUD=1` when you explicitly want to exercise gateway-group create/delete against an environment that supports it -CI provides `A7_ADMIN_URL`, `A7_TOKEN`, and `A7_GATEWAY_GROUP` through repository -secrets. For local development, you can either point these variables at an -existing API7 environment or bring up the repository's Docker-based environment -with: +CI provides `A7_ADMIN_URL` and `A7_TOKEN` through repository secrets, and sets +`A7_GATEWAY_GROUP` to the fixed value `default` in the workflow. For local +development, you can either point these variables at an existing API7 +environment or bring up the repository's Docker-based environment with: ```bash make docker-up diff --git a/scripts/validate-skills.sh b/scripts/validate-skills.sh index a811f42..0c36061 100755 --- a/scripts/validate-skills.sh +++ b/scripts/validate-skills.sh @@ -60,7 +60,36 @@ for skill_dir in "${SKILLS_DIR}"/*; do printf '%s\n' "${name}" >>"${seen_names_file}" fi - if ! printf '%s\n' "${frontmatter}" | grep -q '^description:'; then + description="$(printf '%s\n' "${frontmatter}" | awk ' + function ltrim(s) { sub(/^[[:space:]]+/, "", s); return s } + function has_text(s) { + s = ltrim(s) + return s !~ /^[>|]-?$/ && s ~ /[^[:space:]]/ + } + /^[A-Za-z0-9_-]+:/ { + if (in_description && $0 !~ /^description:/) { + exit + } + } + /^description:[[:space:]]*/ { + in_description = 1 + sub(/^description:[[:space:]]*/, "") + if (has_text($0)) { + print + } + next + } + in_description { + if ($0 ~ /^[[:space:]]+/) { + if (has_text($0)) { + print + } + next + } + exit + } + ')" + if [[ -z "${description}" ]]; then echo "${skill_name}: missing required frontmatter field: description" >&2 status=1 fi diff --git a/skills/a7-recipe-api-versioning/SKILL.md b/skills/a7-recipe-api-versioning/SKILL.md index 5c7006b..5f0fdf9 100644 --- a/skills/a7-recipe-api-versioning/SKILL.md +++ b/skills/a7-recipe-api-versioning/SKILL.md @@ -13,8 +13,7 @@ metadata: a7_commands: - a7 route create - a7 route update - - a7 service create - - a7 service update + - a7 upstream create - a7 config sync - a7 gateway-group get --- diff --git a/skills/a7-recipe-blue-green/SKILL.md b/skills/a7-recipe-blue-green/SKILL.md index a21713b..83238dc 100644 --- a/skills/a7-recipe-blue-green/SKILL.md +++ b/skills/a7-recipe-blue-green/SKILL.md @@ -12,8 +12,7 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 service create - - a7 service update + - a7 upstream create - a7 route create - a7 route update - a7 config sync diff --git a/skills/a7-recipe-canary/SKILL.md b/skills/a7-recipe-canary/SKILL.md index 4c3d718..3d5c0e9 100644 --- a/skills/a7-recipe-canary/SKILL.md +++ b/skills/a7-recipe-canary/SKILL.md @@ -12,7 +12,8 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 service create + - a7 upstream create + - a7 upstream update - a7 route create - a7 route update - a7 route get diff --git a/skills/a7-recipe-health-check/SKILL.md b/skills/a7-recipe-health-check/SKILL.md index 2da3444..60c02da 100644 --- a/skills/a7-recipe-health-check/SKILL.md +++ b/skills/a7-recipe-health-check/SKILL.md @@ -12,9 +12,8 @@ metadata: category: recipe apisix_version: ">=3.0.0" a7_commands: - - a7 service create - - a7 service update - - a7 service get + - a7 upstream create + - a7 upstream get - a7 config sync --- @@ -192,12 +191,12 @@ a7 upstream create --gateway-group default -f - <<'EOF' EOF ``` -### 4. Verify the referencing route and service +### 4. Verify the referencing route and upstream ```bash # Current a7 does not expose a standalone upstream-health command. -# Verify the service/route wiring and use gateway observability for node state. -a7 service get backend --gateway-group default --output json +# Verify the upstream/route wiring and use gateway observability for node state. +a7 upstream get backend --gateway-group default --output json a7 route list --gateway-group default --output json ``` diff --git a/skills/a7-recipe-mtls/SKILL.md b/skills/a7-recipe-mtls/SKILL.md index 41d51d5..24f995f 100644 --- a/skills/a7-recipe-mtls/SKILL.md +++ b/skills/a7-recipe-mtls/SKILL.md @@ -17,8 +17,7 @@ metadata: - a7 ssl list - a7 ssl get - a7 ssl delete - - a7 service create - - a7 service update + - a7 upstream create - a7 route create --- diff --git a/test/e2e/skills/skills_test.go b/test/e2e/skills/skills_test.go index bbce067..f5c42de 100644 --- a/test/e2e/skills/skills_test.go +++ b/test/e2e/skills/skills_test.go @@ -3,10 +3,13 @@ package skills import ( + "fmt" "os" "os/exec" "path/filepath" "regexp" + "sort" + "strconv" "strings" "testing" ) @@ -40,7 +43,6 @@ func TestMain(m *testing.M) { if err != nil { os.Exit(1) } - defer os.RemoveAll(tmpDir) a7Binary = filepath.Join(tmpDir, "a7") cmd := exec.Command("go", "build", "-o", a7Binary, "./cmd/a7") @@ -48,9 +50,16 @@ func TestMain(m *testing.M) { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { + _ = os.RemoveAll(tmpDir) os.Exit(1) } - os.Exit(m.Run()) + + exitCode := m.Run() + if err := os.RemoveAll(tmpDir); err != nil && exitCode == 0 { + fmt.Fprintf(os.Stderr, "failed to remove temp dir %s: %v\n", tmpDir, err) + exitCode = 1 + } + os.Exit(exitCode) } func repoRoot(t *testing.T) string { @@ -63,8 +72,9 @@ func repoRoot(t *testing.T) string { } type skillMetadata struct { - Fields map[string]string - A7Commands []string + Fields map[string]string + A7Commands []string + HasDescriptionText bool } func frontmatter(t *testing.T, file string) skillMetadata { @@ -89,7 +99,8 @@ func frontmatter(t *testing.T, file string) skillMetadata { } metadata := skillMetadata{Fields: map[string]string{}} inA7Commands := false - for _, line := range lines[1:end] { + frontmatterLines := lines[1:end] + for i, line := range frontmatterLines { key, value, ok := strings.Cut(line, ":") trimmed := strings.TrimSpace(line) if inA7Commands { @@ -112,6 +123,9 @@ func frontmatter(t *testing.T, file string) skillMetadata { if key != "" && value != "" { metadata.Fields[key] = value } + if key == "description" { + metadata.HasDescriptionText = hasNonEmptyDescription(frontmatterLines, i, value) + } if key == "a7_commands" { inA7Commands = true } @@ -119,6 +133,26 @@ func frontmatter(t *testing.T, file string) skillMetadata { return metadata } +func hasNonEmptyDescription(lines []string, startIdx int, value string) bool { + value = strings.Trim(strings.TrimSpace(value), `"`) + if value != "" && !strings.HasPrefix(value, ">") && !strings.HasPrefix(value, "|") { + return true + } + for _, line := range lines[startIdx+1:] { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + if !strings.HasPrefix(line, " ") && strings.Contains(trimmed, ":") { + return false + } + if trimmed != ">" && trimmed != ">-" && trimmed != "|" && trimmed != "|-" { + return true + } + } + return false +} + func TestSkillFrontmatterMatchesDirectories(t *testing.T) { root := repoRoot(t) entries, err := os.ReadDir(filepath.Join(root, "skills")) @@ -143,7 +177,7 @@ func TestSkillFrontmatterMatchesDirectories(t *testing.T) { if !skillNamePattern.MatchString(fields["name"]) { t.Fatalf("%s: skill name must be kebab-case", file) } - if fields["description"] == "" { + if !metadata.HasDescriptionText { t.Fatalf("%s: description is required", file) } if seen[fields["name"]] { @@ -162,15 +196,15 @@ func TestSkillDeclaredA7CommandsExist(t *testing.T) { for _, file := range matches { metadata := frontmatter(t, file) for _, command := range metadata.A7Commands { - fields := strings.Fields(command) - if len(fields) == 0 { + command = strings.TrimSpace(command) + if command == "" { continue } - if fields[0] != "a7" { + if command != "a7" && !strings.HasPrefix(command, "a7 ") { t.Fatalf("%s: a7_commands entry %q must start with a7", file, command) } - args := append(fields[1:], "--help") - cmd := exec.Command(a7Binary, args...) + helpCommand := strconv.Quote(a7Binary) + strings.TrimPrefix(command, "a7") + " --help" + cmd := exec.Command("sh", "-c", helpCommand) cmd.Dir = root output, err := cmd.CombinedOutput() if err != nil { @@ -221,22 +255,44 @@ func TestSkillsDoNotReferenceRemovedA7Commands(t *testing.T) { func TestSkillsDocumentationReferencesExistingSkills(t *testing.T) { root := repoRoot(t) + entries, err := os.ReadDir(filepath.Join(root, "skills")) + if err != nil { + t.Fatal(err) + } + existing := map[string]bool{} + for _, entry := range entries { + if entry.IsDir() { + existing[entry.Name()] = true + } + } + data, err := os.ReadFile(filepath.Join(root, "docs", "skills.md")) if err != nil { t.Fatal(err) } doc := string(data) - staleSkillNames := []string{ - "a7-recipe-gateway-group", - "a7-persona-platform-eng", - "a7-recipe-service-template", - "a7-plugin-ai-rag", - "a7-plugin-ai-token-limiter", - "a7-recipe-service-registry", + referencedSkills := regexp.MustCompile(`\ba7-[a-z0-9]+(?:-[a-z0-9]+)*\b`).FindAllString(doc, -1) + categoryNames := map[string]bool{ + "a7-persona": true, + "a7-plugin": true, + "a7-recipe": true, } - for _, name := range staleSkillNames { - if strings.Contains(doc, name) { - t.Fatalf("docs/skills.md references missing skill %q", name) + missing := map[string]bool{} + for _, name := range referencedSkills { + if categoryNames[name] { + continue } + if !existing[name] { + missing[name] = true + } + } + if len(missing) == 0 { + return + } + names := make([]string, 0, len(missing)) + for name := range missing { + names = append(names, name) } + sort.Strings(names) + t.Fatalf("docs/skills.md references missing skills: %s", strings.Join(names, ", ")) }