diff --git a/docs/ga-test-plan.md b/docs/ga-test-plan.md new file mode 100644 index 0000000..447c3f2 --- /dev/null +++ b/docs/ga-test-plan.md @@ -0,0 +1,213 @@ +# a7 GA Test Plan — Local API7 EE 3.9.12 + +Execution plan for **Task #3 (real CLI smoke tests)** and **Task #4 (complete E2E coverage)** +from the GA Readiness Handoff. Run against a locally deployed API7 EE. + +## Scope + +- **In scope**: management-plane validation of the `a7` CLI — CRUD round-trips, declarative + config, output formats, error handling, and confirmation that unsupported resources are + removed/blocked. +- **Out of scope**: real gateway data-plane traffic forwarding. Per the handoff, traffic + tests stay in the gateway repository. The separately-running gateway container is only + needed to keep the EE deployment healthy; we do **not** assert on proxied traffic here. + +## Local environment (assumed already running) + +| Component | How it runs | Notes | +|---|---|---| +| API7 EE dashboard / control-plane + dp-manager + PostgreSQL | docker compose | the management API `a7` talks to | +| API7 gateway | standalone docker container | connected to the dp-manager; health only | + +### 0. Pre-flight — capture connection facts + +Fill these in before doing anything else: + +``` +A7_ADMIN_URL = https://localhost:7443 # control-plane HTTPS port — confirm +A7_TOKEN = a7ee- # access token from the dashboard UI / API +A7_GATEWAY_GROUP = default # resolved to a UUID at runtime +``` + +Confirm before proceeding: + +1. **Reachability** — `curl -k -H "X-API-KEY: $A7_TOKEN" $A7_ADMIN_URL/api/gateway_groups` + returns `200` with a non-empty `list`. +2. **Version is 3.9.12** — check the dashboard image tag in `docker compose ps` / + `docker inspect`, or the dashboard `/api/version` endpoint. Record the exact version. + If it is **not** 3.9.12, note the deviation in the report — the handoff specifically + wants 3.9.12 validated. +3. **Gateway is registered & healthy** — the standalone gateway container shows up under + the gateway group (dashboard UI or `GET /api/gateway_groups/{id}/instances`). This only + needs to be healthy; we are not testing traffic through it. + +## Build + +```bash +cd ~/Desktop/repos/xisipa/a7 +make build # -> ./bin/a7 +./bin/a7 version # record the version string in the report +``` + +All manual steps below use `./bin/a7`. The automated suite (Phase A) builds its own binary. + +--- + +## Phase A — Run the existing automated E2E suite + +Establishes a known-good baseline against the local 3.9.12 instance before manual work. + +```bash +cd ~/Desktop/repos/xisipa/a7 +export A7_ADMIN_URL="https://localhost:7443" +export A7_TOKEN="a7ee.xxxxxxxx" +export A7_GATEWAY_GROUP="default" +# A7_GATEWAY_URL and HTTPBIN_URL intentionally left UNSET — data-plane tests are skipped. +make test-e2e +``` + +**Expected**: suite passes, or fails only in ways explained by known gaps. For every +failure, decide: real bug, missing/incorrect test, or environment issue. Record each one. + +> The standard `test/e2e/docker-compose.yml` is **not** used here — you already have a +> local deployment. Only the env vars above are needed. + +--- + +## Phase B — Manual CRUD smoke tests (per resource) + +For **each** resource below, run the handoff's round-trip pattern with `./bin/a7`: + +1. `create` (both flag-based and `-f file` where supported) +2. `get` — assert the returned config matches what was sent +3. `list` — assert the new resource appears +4. `update` — change a field +5. `get` again — assert the update took effect +6. `export` (where supported) — assert valid YAML/JSON +7. `delete` (try with and without `--force`) +8. `get` / `list` — assert it is gone + +Also exercise, at least once per resource: `-o json`, `-o yaml`, and default table output; +plus one deliberate error case (bad ID, missing `--gateway-group`, malformed `-f` file) and +confirm the error message is clear and actionable. + +**Use unique resource IDs** (`a7-ga--`) and clean up everything, even +on a local instance — it keeps reruns deterministic. + +### Resource checklist + +| Resource | Command | create/get/list/update/delete | export | Notes | +|---|---|:--:|:--:|---| +| context | `a7 context` | ☐ | — | create/use/list/current/delete; no gateway group needed | +| gateway-group | `a7 gateway-group` | ☐ | — | control-plane `/api/*` | +| service | `a7 service` | ☐ | ☐ | inline upstream — **node array format only** | +| route | `a7 route` | ☐ | ☐ | requires `--service-id`; route is service-centered | +| consumer | `a7 consumer` | ☐ | ☐ | | +| credential | `a7 credential` | ☐ | — | nested under a consumer | +| ssl | `a7 ssl` | ☐ | ☐ | **shared/global** — unique IDs, careful cleanup | +| secret | `a7 secret` | ☐ | — | **shared/global** — unique IDs, careful cleanup | +| global-rule | `a7 global-rule` | ☐ | ☐ | **shared/global** — affects whole gateway group | +| plugin | `a7 plugin` | ☐ (list/get only) | — | read-only: list plugins, get schema | +| plugin-metadata | `a7 plugin-metadata` | ☐ (no list) | — | **shared/global**; keyed by plugin name | +| proto | `a7 proto` | ☐ | ☐ | protobuf definitions | +| stream-route | `a7 stream-route` | ☐ | ☐ | **confirmed exposed** by the CP — must work | + +> **Shared/global resources** (ssl, secret, global-rule, plugin-metadata) are visible +> gateway-group-wide. On a shared environment they collide between CI runs; even locally, +> assert you only touch IDs you created and never delete pre-existing resources. + +### Inline upstream format (API7 EE) + +`nodes` must be an **array of objects**. The APISIX map form is rejected. + +```yaml +upstream: + type: roundrobin + nodes: + - host: 127.0.0.1 + port: 8080 + weight: 1 +``` + +--- + +## Phase C — Verify unsupported resources are blocked + +Confirm the resources removed in PR #21 (and the one pending in Task #2) are gone or clearly +rejected — not present as broken commands. + +| Resource | Expected behavior | +|---|---| +| `a7 upstream ...` | command does not exist (unknown command error) | +| `a7 consumer-group ...` | command does not exist | +| `a7 service-template ...` | command does not exist | +| `a7 plugin-config ...` | **after Task #2**: command does not exist | +| `a7 stream-route ...` | **works** — full CRUD (verified exposed by the control plane) | + +Declarative config — these top-level sections must be **rejected with a clear error**: + +```yaml +upstreams: [...] # -> validation error +consumer_groups: [...] # -> validation error +service_templates: [...] # -> validation error +plugin_configs: [...] # -> validation error (after Task #2) +``` + +> Caveat: the control plane still accepts `plugin_configs` *inside batch config-validation +> payloads*. The rejection applies to `a7`'s standalone command and top-level declarative +> section, not to that internal validation path. + +--- + +## Phase D — Declarative config + +Run the full declarative workflow end to end: + +1. `a7 config dump` — export the live gateway-group config to a YAML file. Assert it is + valid YAML and contains only supported resource sections. +2. `a7 config validate -f dump.yaml` — assert it passes for a known-good file, and fails + with a clear message for a file containing an unsupported section (see Phase C). +3. `a7 config diff -f dump.yaml` — assert "no diff" against the just-dumped state; then + edit a field and assert the diff is reported correctly. +4. `a7 config sync -f dump.yaml` — apply changes. Verify `--dry-run` first if available, + then a real sync, then `diff` again to confirm convergence. + +Confirm all four work with **service-centered** API7 EE resources (no standalone upstreams). + +--- + +## Bug-handling protocol + +Per the handoff: **when a bug is found, add or update E2E coverage before fixing it.** + +1. Reproduce with the real binary; capture exact command, output, and HTTP status. +2. Write or update a `test/e2e/_test.go` case that fails for the same reason. +3. Fix the code. +4. Re-run that test plus `make test-e2e` to confirm green. +5. Do **not** change code purely to satisfy a test if it contradicts real API7 EE behavior. + +--- + +## Reporting + +Record results in a table and attach it to the GA tracking issue / PR: + +| Phase | Resource / area | Result | Bug? | E2E test added | Notes | +|---|---|---|---|---|---| +| A | automated suite | | | | | +| B | service | | | | | +| B | route | | | | | +| ... | ... | | | | | +| C | unsupported resources | | | | | +| D | config dump/validate/diff/sync | | | | | + +Also capture: exact API7 EE version tested, `a7 version` string, date, and any environment +deviations from "API7 EE 3.9.12". + +## Exit criteria + +- Phase A suite green against the local instance. +- Phase B round-trip passes for every resource in the checklist. +- Phase C confirms all unsupported resources are absent/blocked and `stream-route` works. +- Phase D confirms declarative config works with service-centered resources. +- Every bug found has a corresponding E2E test and a fix (or a tracked follow-up). diff --git a/docs/ga-test-report.md b/docs/ga-test-report.md new file mode 100644 index 0000000..f7d1b93 --- /dev/null +++ b/docs/ga-test-report.md @@ -0,0 +1,89 @@ +# a7 GA Test Report + +Execution of [`docs/ga-test-plan.md`](./ga-test-plan.md) against a locally deployed API7 EE. + +## Environment + +| Item | Value | +|---|---| +| Date | 2026-05-14 | +| API7 EE version | **3.9.12** (image `api7/api7-ee-3-integrated:v3.9.12`; `/api/version` → `v3.9.12`) | +| `a7 version` | `7b67da5` (then `7b67da5-dirty` after the fixes in this report) | +| Admin URL | `https://localhost:7443` | +| Gateway group | `default` | +| Gateway instance | `api7-ee-gateway-1` — `status: Healthy`, `compatibility: Compatible` | +| Deviations | (1) `a7 plugin-config` command still present — Task #2 not yet done. (2) Data-plane traffic tests not run (out of scope per plan; `A7_GATEWAY_URL`/`HTTPBIN_URL` unset). | + +## Summary + +All four phases executed. **5 real bugs found, all fixed with E2E + unit coverage.** Final automated suite: **128 passed / 10 skipped / 0 failed** (skips are the intentionally-unset data-plane and gateway-group-CRUD tests). + +## Results + +| Phase | Resource / area | Result | Bug? | E2E test added | Notes | +|---|---|---|---|---|---| +| A | automated suite | PASS | — | — | 122→128 pass, 10 skip, 0 fail. Skips = data-plane traffic + gateway-group CRUD (intentionally disabled). | +| B | context | PASS | — | — | create/list/use/current/delete, json output, error case. | +| B | gateway-group | PASS | — | — | full CRUD; non-interactive `delete` needs `--force`. | +| B | service | PASS | — | — | create (file+flags), get, list, update, export. Map-form upstream nodes correctly rejected (400). | +| B | route | PASS | — | — | create (file+flags), get, list, update, export, delete. `get`/`delete` take id only; `update` uses `--uri`. | +| B | consumer | PASS | — | — | CRUD + export. MINOR: file `description:` key silently dropped (accepted key is `desc`). | +| B | credential | PASS | — | — | CRUD via server-returned id. MINOR: `create [id]` positional is treated as `name` (id is server-generated). | +| B | ssl | **FIXED** | **BUG-1** | `TestSSL_UpdateFlagsRequireCertAndKey` | flag-based `ssl update` silently lost partial updates. | +| B | secret | PASS | — | — | CRUD (file+flags); id format is `provider/id`. | +| B | global-rule | PASS | — | — | CRUD (file+flags), export; id must equal the plugin name. MINOR: flag `--id` is required but its value is ignored by EE. | +| B | plugin | **FIXED** | **BUG-3** | `TestPlugin_GetYAML` | `plugin get -o yaml` emitted JSON. list/get otherwise fine. | +| B | plugin-metadata | **FIXED** | **BUG-2** | `TestPluginMetadata_GetYAML` | `plugin-metadata get -o yaml` emitted a byte-array. CRUD otherwise fine. | +| B | proto | PASS | — | — | CRUD (file+flags), export. MINOR: `--desc` / `desc:` silently dropped. | +| B | stream-route | **FIXED** | **BUG-4** | `TestStreamRoute_CreateWithFlags`, `TestStreamRoute_CreateRequiresName` | `create` had no `--name` flag; EE requires `name`. CRUD via `-f` worked. | +| C | unsupported commands | PASS | — | — | `upstream` / `consumer-group` / `service-template` removed. `plugin-config` still present (Task #2 pending) — gives a clear runtime error. `stream-route` works. | +| C | declarative unsupported sections | **FIXED** | **BUG-5** | `TestConfigValidate_RejectsUnsupportedServiceTemplates` | `upstreams`/`consumer_groups` rejected; `service_templates` was silently accepted. `plugin_configs` rejection is expected-pending per plan. | +| D | config dump/validate/diff/sync | PASS | — | — | dump → valid YAML, supported sections only. validate/diff/sync round-trip converges; `--dry-run` applies nothing; post-sync diff clean; route update verified persisted. | + +## Bugs found & fixed + +Per the bug-handling protocol, each bug was reproduced with the real binary, given failing E2E coverage, fixed, and re-verified. + +### BUG-1 — `ssl update` silently lost partial flag-based updates +`a7 ssl update --sni new` (without `--cert`/`--key`) exited 0 and echoed the new SNI, but the server was never updated. Root cause: the command does a GET-merge-PUT, but API7 EE never returns `cert`/`key` on GET, so the merged PUT dropped the certificate material and the EE ignored it. +**Fix:** `pkg/cmd/ssl/update/update.go` now requires `--cert` and `--key` for flag-based updates (consistent with `ssl create`), with a clear error pointing to the `-f` path. `-f` updates with a full definition are unaffected. + +### BUG-2 — `plugin-metadata get -o yaml` emitted a byte-array +The YAML output was a list of integers. Root cause: `pkg/cmd/plugin-metadata/get/get.go` passed `json.RawMessage` (a `[]byte`) to the YAML encoder. +**Fix:** pass the decoded `map` to the exporter. + +### BUG-3 — `plugin get -o yaml` emitted JSON +Root cause: `pkg/cmd/plugin/get/get.go` only special-cased `-o json`; every other value fell through to a hardcoded JSON encoder. +**Fix:** honor any explicit `--output` value via the exporter. + +### BUG-4 — `stream-route create` had no `--name` flag +API7 EE 3.9.12 requires `name` on stream routes, but the create command exposed no `--name` flag, making flag-based creation impossible. +**Fix:** added a `Name` field to `api.StreamRoute`, a `--name` flag, and required-field validation on both the flag and `-f` paths. + +### BUG-5 — `config validate` silently accepted `service_templates` +A declarative file with a top-level `service_templates:` section validated as "Config is valid". Root cause: `api.ConfigFile` had no field for it, so the section was dropped on unmarshal. +**Fix:** added a `ServiceTemplates` field and an explicit rejection in both `config validate` and `configutil.ValidateSupportedSections` (used by `diff`/`sync`), mirroring `upstreams`/`consumer_groups`. + +## Minor observations (not fixed — low severity / by-design) + +- **consumer**: `-f` file with a `description:` key is silently ignored; the accepted key is `desc`. +- **credential**: `create [id]` help text is misleading — the positional arg becomes the `name`; the `id` is server-generated. This is codified by `TestCredential_CreateWithPositionalID`, so it is current intended behavior. +- **global-rule**: flag-based `create` requires `--id`, but API7 EE forces the id to equal the single plugin key, so the `--id` value is effectively ignored. File-based create errors clearly on a mismatch. +- **proto**: `--desc` and a `desc:` file field are silently dropped. +- **stream-route / EE behavior**: API7 EE rejects stream routes bound to a service it has classified as HTTP. Binding to a `type: stream` service works reliably. `a7` surfaces the EE error cleanly; this is EE-side behavior, not an `a7` bug. +- **service**: the EE's schema-mismatch error (e.g. map-form upstream nodes) is correct and actionable but very verbose (dumps the full JSON schema). +- **tooling**: `golangci-lint run ./...` reports spurious `undefined: yaml (typecheck)` on files that correctly import `gopkg.in/yaml.v3` (including untouched files). `go build`, `go vet`, and `go test` are all clean — this is a pre-existing lint-environment issue, unrelated to these changes. + +## Exit criteria + +| Criterion | Status | +|---|---| +| Phase A suite green against the local instance | ✅ 128 pass / 10 skip / 0 fail | +| Phase B round-trip passes for every resource | ✅ all 13 resources pass (ssl/plugin/plugin-metadata/stream-route after fixes) | +| Phase C confirms unsupported resources absent/blocked and `stream-route` works | ✅ — except `plugin-config` command still present (Task #2 pending, tracked) | +| Phase D declarative config works with service-centered resources | ✅ dump/validate/diff/sync all verified | +| Every bug has a corresponding E2E test and a fix | ✅ 5/5 fixed with E2E + unit coverage | + +## Follow-ups + +- **Task #2** (remove the `plugin-config` standalone command) is still outstanding. Once done, Phase C also expects the declarative `plugin_configs` top-level section to be rejected — the `service_templates` rejection added here is the template for that change. diff --git a/pkg/api/types_config.go b/pkg/api/types_config.go index 959209f..58feb71 100644 --- a/pkg/api/types_config.go +++ b/pkg/api/types_config.go @@ -3,19 +3,22 @@ package api // ConfigFile is the declarative configuration file format for a7. // It holds all runtime resources that can be dumped/synced. type ConfigFile struct { - Version string `json:"version" yaml:"version"` - Routes []Route `json:"routes,omitempty" yaml:"routes,omitempty"` - Services []Service `json:"services,omitempty" yaml:"services,omitempty"` - Upstreams []Upstream `json:"upstreams,omitempty" yaml:"upstreams,omitempty"` - Consumers []Consumer `json:"consumers,omitempty" yaml:"consumers,omitempty"` - SSL []SSL `json:"ssl,omitempty" yaml:"ssl,omitempty"` - GlobalRules []GlobalRule `json:"global_rules,omitempty" yaml:"global_rules,omitempty"` - PluginConfigs []PluginConfig `json:"plugin_configs,omitempty" yaml:"plugin_configs,omitempty"` - ConsumerGroups []ConsumerGroup `json:"consumer_groups,omitempty" yaml:"consumer_groups,omitempty"` - StreamRoutes []StreamRoute `json:"stream_routes,omitempty" yaml:"stream_routes,omitempty"` - Protos []Proto `json:"protos,omitempty" yaml:"protos,omitempty"` - Secrets []Secret `json:"secrets,omitempty" yaml:"secrets,omitempty"` - PluginMetadata []PluginMetadataEntry `json:"plugin_metadata,omitempty" yaml:"plugin_metadata,omitempty"` + Version string `json:"version" yaml:"version"` + Routes []Route `json:"routes,omitempty" yaml:"routes,omitempty"` + Services []Service `json:"services,omitempty" yaml:"services,omitempty"` + Upstreams []Upstream `json:"upstreams,omitempty" yaml:"upstreams,omitempty"` + Consumers []Consumer `json:"consumers,omitempty" yaml:"consumers,omitempty"` + SSL []SSL `json:"ssl,omitempty" yaml:"ssl,omitempty"` + GlobalRules []GlobalRule `json:"global_rules,omitempty" yaml:"global_rules,omitempty"` + PluginConfigs []PluginConfig `json:"plugin_configs,omitempty" yaml:"plugin_configs,omitempty"` + ConsumerGroups []ConsumerGroup `json:"consumer_groups,omitempty" yaml:"consumer_groups,omitempty"` + // ServiceTemplates is captured only so the section can be explicitly + // rejected; API7 EE does not support it as a top-level resource. + ServiceTemplates []interface{} `json:"service_templates,omitempty" yaml:"service_templates,omitempty"` + StreamRoutes []StreamRoute `json:"stream_routes,omitempty" yaml:"stream_routes,omitempty"` + Protos []Proto `json:"protos,omitempty" yaml:"protos,omitempty"` + Secrets []Secret `json:"secrets,omitempty" yaml:"secrets,omitempty"` + PluginMetadata []PluginMetadataEntry `json:"plugin_metadata,omitempty" yaml:"plugin_metadata,omitempty"` } // PluginMetadataEntry is a freeform map representing a plugin's metadata. diff --git a/pkg/api/types_stream_route.go b/pkg/api/types_stream_route.go index a9727b2..f3a7ef3 100644 --- a/pkg/api/types_stream_route.go +++ b/pkg/api/types_stream_route.go @@ -3,6 +3,7 @@ package api // StreamRoute represents a stream (L4) route in API7 EE (runtime). type StreamRoute struct { ID string `json:"id,omitempty" yaml:"id,omitempty"` + Name string `json:"name,omitempty" yaml:"name,omitempty"` Desc string `json:"desc,omitempty" yaml:"desc,omitempty"` RemoteAddr string `json:"remote_addr,omitempty" yaml:"remote_addr,omitempty"` ServerAddr string `json:"server_addr,omitempty" yaml:"server_addr,omitempty"` diff --git a/pkg/cmd/config/configutil/configutil.go b/pkg/cmd/config/configutil/configutil.go index 73f746d..fddec02 100644 --- a/pkg/cmd/config/configutil/configutil.go +++ b/pkg/cmd/config/configutil/configutil.go @@ -242,6 +242,9 @@ func ValidateSupportedSections(cfg api.ConfigFile) error { if len(cfg.ConsumerGroups) > 0 { unsupported = append(unsupported, "consumer_groups") } + if len(cfg.ServiceTemplates) > 0 { + unsupported = append(unsupported, "service_templates") + } if len(unsupported) > 0 { return fmt.Errorf("unsupported declarative config sections: %s; define upstreams inline on services and omit API7 EE unsupported resources", strings.Join(unsupported, ", ")) } diff --git a/pkg/cmd/config/validate/validate.go b/pkg/cmd/config/validate/validate.go index 4fe843e..a957da5 100644 --- a/pkg/cmd/config/validate/validate.go +++ b/pkg/cmd/config/validate/validate.go @@ -103,6 +103,9 @@ func ValidateConfigFile(cfg api.ConfigFile) []string { if len(cfg.ConsumerGroups) > 0 { errs = append(errs, "consumer_groups are not supported by current API7 EE") } + if len(cfg.ServiceTemplates) > 0 { + errs = append(errs, "service_templates are not supported by current API7 EE") + } seenRouteIDs := map[string]struct{}{} for i, r := range cfg.Routes { diff --git a/pkg/cmd/plugin-metadata/get/get.go b/pkg/cmd/plugin-metadata/get/get.go index cfffed1..408a10c 100644 --- a/pkg/cmd/plugin-metadata/get/get.go +++ b/pkg/cmd/plugin-metadata/get/get.go @@ -74,8 +74,10 @@ func actionRun(opts *Options) error { } if opts.Output != "" { + // Write the decoded map, not json.RawMessage: the YAML encoder would + // otherwise serialize the raw []byte as a list of integers. exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out) - return exporter.Write(json.RawMessage(body)) + return exporter.Write(metadata) } metaJSON, _ := json.MarshalIndent(metadata, "", " ") diff --git a/pkg/cmd/plugin/get/get.go b/pkg/cmd/plugin/get/get.go index 49cf6fb..e86b691 100644 --- a/pkg/cmd/plugin/get/get.go +++ b/pkg/cmd/plugin/get/get.go @@ -70,8 +70,10 @@ func actionRun(opts *Options) error { return fmt.Errorf("failed to parse plugin response: %w", err) } - if opts.Output == "json" { - return cmdutil.NewExporter("json", opts.IO.Out).Write(item) + if opts.Output != "" { + // Honor the requested format (json or yaml); previously any value + // other than "json" fell through to a hardcoded JSON encoder. + return cmdutil.NewExporter(opts.Output, opts.IO.Out).Write(item) } enc := json.NewEncoder(opts.IO.Out) diff --git a/pkg/cmd/ssl/update/update.go b/pkg/cmd/ssl/update/update.go index 98ddf16..8e5536c 100644 --- a/pkg/cmd/ssl/update/update.go +++ b/pkg/cmd/ssl/update/update.go @@ -107,6 +107,16 @@ func actionRun(opts *Options) error { return writeSSLResponse(format, opts.IO.Out, body) } + // API7 EE never returns the existing cert/key on GET, so a flag-based + // update cannot reconstruct the full object. Without both, the merged + // PUT silently drops the certificate material and the update is lost. + // Require them explicitly, or direct the user to the -f path. + if opts.Cert == "" || opts.Key == "" { + return fmt.Errorf("--cert and --key are both required for flag-based ssl update; " + + "API7 EE does not return existing certificate material, so a partial update would be lost. " + + "Pass both flags, or use -f with a full SSL definition") + } + cert, err := maybeReadFile(opts.Cert) if err != nil { return err diff --git a/pkg/cmd/ssl/update/update_test.go b/pkg/cmd/ssl/update/update_test.go index d4bc267..cc48715 100644 --- a/pkg/cmd/ssl/update/update_test.go +++ b/pkg/cmd/ssl/update/update_test.go @@ -76,22 +76,24 @@ func TestMaybeReadFileTreatsMissingBareFilenameAsPath(t *testing.T) { } } -func TestUpdateSSL_PreservesCertificateWhenUpdatingSNI(t *testing.T) { +func TestUpdateSSL_WithCertKeyAndSNI(t *testing.T) { ios, _, out, _ := iostreams.Test() registry := &httpmock.Registry{} - registry.Register(http.MethodGet, "/apisix/admin/ssls/ssl1", httpmock.JSONResponse(`{"value":{"id":"ssl1","cert":"old-cert","key":"old-key","snis":["old.example.com"],"type":"server","status":1}}`)) + // API7 EE never returns the existing cert/key on GET, so a flag-based + // update must supply them explicitly. + registry.Register(http.MethodGet, "/apisix/admin/ssls/ssl1", httpmock.JSONResponse(`{"value":{"id":"ssl1","snis":["old.example.com"],"type":"server","status":1}}`)) registry.RegisterResponder(http.MethodPut, "/apisix/admin/ssls/ssl1", func(req *http.Request) (httpmock.Response, error) { var body api.SSL if err := json.NewDecoder(req.Body).Decode(&body); err != nil { return httpmock.Response{}, fmt.Errorf("decode request: %w", err) } - if body.Cert != "old-cert" || body.Key != "old-key" { - return httpmock.Response{}, fmt.Errorf("expected cert/key to be preserved, got %#v", body) + if !strings.Contains(body.Cert, "NEWCERT") || !strings.Contains(body.Key, "NEWKEY") { + return httpmock.Response{}, fmt.Errorf("expected cert/key from flags in payload, got %#v", body) } if len(body.SNIs) != 1 || body.SNIs[0] != "new.example.com" { return httpmock.Response{}, fmt.Errorf("expected updated sni, got %#v", body.SNIs) } - return httpmock.JSONResponse(`{"value":{"id":"ssl1","cert":"old-cert","key":"old-key","snis":["new.example.com"],"type":"server","status":1}}`), nil + return httpmock.JSONResponse(`{"value":{"id":"ssl1","cert":"new-cert","key":"new-key","snis":["new.example.com"],"type":"server","status":1}}`), nil }) err := actionRun(&Options{ @@ -99,6 +101,8 @@ func TestUpdateSSL_PreservesCertificateWhenUpdatingSNI(t *testing.T) { Client: func() (*http.Client, error) { return registry.GetClient(), nil }, GatewayGroup: "gg1", ID: "ssl1", + Cert: "-----BEGIN CERTIFICATE-----\nNEWCERT\n-----END CERTIFICATE-----", + Key: "-----BEGIN PRIVATE KEY-----\nNEWKEY\n-----END PRIVATE KEY-----", SNIs: []string{"new.example.com"}, Config: func() (config.Config, error) { return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil @@ -114,12 +118,34 @@ func TestUpdateSSL_PreservesCertificateWhenUpdatingSNI(t *testing.T) { if err := json.Unmarshal(out.Bytes(), &output); err != nil { t.Fatalf("failed to parse output: %v", err) } - if strings.Contains(out.String(), "old-key") || output.Key != api.RedactedSSLKey { + if strings.Contains(out.String(), "new-key") || output.Key != api.RedactedSSLKey { t.Fatalf("expected ssl key to be redacted in output, got %+v", output) } registry.Verify(t) } +// TestUpdateSSL_RequiresCertAndKey guards against a regression where a +// flag-based update without --cert/--key reported success but silently +// dropped the change (the merged PUT lost the certificate material). +func TestUpdateSSL_RequiresCertAndKey(t *testing.T) { + ios, _, _, _ := iostreams.Test() + registry := &httpmock.Registry{} + + err := actionRun(&Options{ + IO: ios, + Client: func() (*http.Client, error) { return registry.GetClient(), nil }, + GatewayGroup: "gg1", + ID: "ssl1", + SNIs: []string{"new.example.com"}, + Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "--cert and --key") { + t.Fatalf("expected error requiring --cert and --key, got %v", err) + } +} + func TestUpdateSSL_SendsExplicitStatusZero(t *testing.T) { ios, _, _, _ := iostreams.Test() registry := &httpmock.Registry{} @@ -140,6 +166,8 @@ func TestUpdateSSL_SendsExplicitStatusZero(t *testing.T) { Client: func() (*http.Client, error) { return registry.GetClient(), nil }, GatewayGroup: "gg1", ID: "ssl1", + Cert: "-----BEGIN CERTIFICATE-----\nNEWCERT\n-----END CERTIFICATE-----", + Key: "-----BEGIN PRIVATE KEY-----\nNEWKEY\n-----END PRIVATE KEY-----", Status: 0, StatusSet: true, Config: func() (config.Config, error) { @@ -172,6 +200,8 @@ func TestUpdateSSL_PreservesExistingStatusZero(t *testing.T) { Client: func() (*http.Client, error) { return registry.GetClient(), nil }, GatewayGroup: "gg1", ID: "ssl1", + Cert: "-----BEGIN CERTIFICATE-----\nNEWCERT\n-----END CERTIFICATE-----", + Key: "-----BEGIN PRIVATE KEY-----\nNEWKEY\n-----END PRIVATE KEY-----", SNIs: []string{"new.example.com"}, Config: func() (config.Config, error) { return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil diff --git a/pkg/cmd/stream-route/create/create.go b/pkg/cmd/stream-route/create/create.go index 9e656a7..79b66ba 100644 --- a/pkg/cmd/stream-route/create/create.go +++ b/pkg/cmd/stream-route/create/create.go @@ -23,6 +23,7 @@ type Options struct { GatewayGroup string File string + Name string Desc string RemoteAddr string ServerAddr string @@ -45,6 +46,7 @@ func NewCmd(f *cmd.Factory) *cobra.Command { }, } + c.Flags().StringVar(&opts.Name, "name", "", "Stream route name (required by API7 EE)") c.Flags().StringVar(&opts.Desc, "desc", "", "Stream route description") c.Flags().StringVarP(&opts.File, "file", "f", "", "Path to JSON/YAML file with resource definition") c.Flags().StringVar(&opts.RemoteAddr, "remote-addr", "", "Remote address") @@ -78,6 +80,9 @@ func actionRun(opts *Options) error { if err := cmdutil.EnsureRequiredStringField(payload, "service_id", opts.ServiceID, "--service-id is required for current API7 EE"); err != nil { return err } + if err := cmdutil.EnsureRequiredStringField(payload, "name", opts.Name, "name is required by API7 EE; set it in the file or pass --name"); err != nil { + return err + } httpClient, err := opts.Client() if err != nil { @@ -104,6 +109,9 @@ func actionRun(opts *Options) error { if opts.ServiceID == "" { return fmt.Errorf("--service-id is required for current API7 EE") } + if opts.Name == "" { + return fmt.Errorf("--name is required by API7 EE") + } httpClient, err := opts.Client() if err != nil { @@ -120,6 +128,7 @@ func actionRun(opts *Options) error { } bodyReq := api.StreamRoute{ + Name: opts.Name, Desc: opts.Desc, RemoteAddr: opts.RemoteAddr, ServerAddr: opts.ServerAddr, diff --git a/pkg/cmd/stream-route/create/create_test.go b/pkg/cmd/stream-route/create/create_test.go index 775dcc1..5e295be 100644 --- a/pkg/cmd/stream-route/create/create_test.go +++ b/pkg/cmd/stream-route/create/create_test.go @@ -43,6 +43,7 @@ func TestCreateStreamRoute_Success(t *testing.T) { IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, GatewayGroup: "gg1", + Name: "sr1", Desc: "mysql", ServiceID: "svc1", Config: func() (config.Config, error) { @@ -146,6 +147,7 @@ func TestCreateStreamRoute_FileServiceIDFlag(t *testing.T) { GatewayGroup: "gg1", File: path, ServiceID: "svc-flag", + Name: "sr-flag", Config: func() (config.Config, error) { return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil }, @@ -187,6 +189,7 @@ func TestCreateStreamRoute_APIError(t *testing.T) { IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, GatewayGroup: "gg1", + Name: "sr1", ServiceID: "svc1", Config: func() (config.Config, error) { return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil @@ -198,3 +201,44 @@ func TestCreateStreamRoute_APIError(t *testing.T) { registry.Verify(t) } + +// TestCreateStreamRoute_MissingName guards the flag-based create path: API7 EE +// requires a name, and the command must reject the request before calling the API. +func TestCreateStreamRoute_MissingName(t *testing.T) { + ios, _, _, _ := iostreams.Test() + err := actionRun(&Options{ + IO: ios, + Client: func() (*http.Client, error) { return (&httpmock.Registry{}).GetClient(), nil }, + GatewayGroup: "gg1", + ServiceID: "svc1", + Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "--name is required") { + t.Fatalf("expected missing name error, got: %v", err) + } +} + +// TestCreateStreamRoute_FileMissingName guards the -f path: name must be +// present in the file or supplied via --name. +func TestCreateStreamRoute_FileMissingName(t *testing.T) { + ios, _, _, _ := iostreams.Test() + path := filepath.Join(t.TempDir(), "stream-route.json") + if err := os.WriteFile(path, []byte(`{"desc":"mysql","service_id":"svc1"}`), 0o600); err != nil { + t.Fatalf("write file: %v", err) + } + + err := actionRun(&Options{ + IO: ios, + Client: func() (*http.Client, error) { return (&httpmock.Registry{}).GetClient(), nil }, + GatewayGroup: "gg1", + File: path, + Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", token: "test", gatewayGroup: "gg1"}, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "name is required") { + t.Fatalf("expected missing name error, got: %v", err) + } +} diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index f453e39..2400feb 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -194,6 +194,22 @@ consumer_groups: assert.Contains(t, stderr, "consumer_groups are not supported by current API7 EE") } +func TestConfigValidate_RejectsUnsupportedServiceTemplates(t *testing.T) { + env := setupEnv(t) + + invalidYAML := `version: "1" +service_templates: + - id: removed-template + name: removed-template +` + tmpFile := filepath.Join(t.TempDir(), "unsupported-service-templates.yaml") + require.NoError(t, os.WriteFile(tmpFile, []byte(invalidYAML), 0644)) + + _, stderr, err := runA7WithEnv(env, "config", "validate", "-f", tmpFile) + assert.Error(t, err) + assert.Contains(t, stderr, "service_templates are not supported by current API7 EE") +} + func TestConfigValidate_MissingRouteURI(t *testing.T) { env := setupEnv(t) diff --git a/test/e2e/plugin_metadata_test.go b/test/e2e/plugin_metadata_test.go index b8b875f..ddc8fb6 100644 --- a/test/e2e/plugin_metadata_test.go +++ b/test/e2e/plugin_metadata_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) // deletePluginMetadataViaAdmin deletes plugin metadata via the Admin API. @@ -70,3 +71,27 @@ func TestPluginMetadata_CRUD(t *testing.T) { _, _, err = runA7WithEnv(env, "plugin-metadata", "get", pluginName, "-g", gatewayGroup) assert.Error(t, err) } + +// TestPluginMetadata_GetYAML guards against a regression where +// `plugin-metadata get -o yaml` serialized the raw response bytes as a YAML +// list of integers instead of the actual metadata map. +func TestPluginMetadata_GetYAML(t *testing.T) { + env := setupEnv(t) + pluginName := "http-logger" + t.Cleanup(func() { deletePluginMetadataViaAdmin(t, pluginName) }) + + pmJSON := `{"log_format":{"host":"$host"}}` + tmpFile := filepath.Join(t.TempDir(), "plugin-metadata.json") + require.NoError(t, os.WriteFile(tmpFile, []byte(pmJSON), 0644)) + _, stderr, err := runA7WithEnv(env, "plugin-metadata", "create", pluginName, "-f", tmpFile, "-g", gatewayGroup) + require.NoError(t, err, stderr) + + stdout, stderr, err := runA7WithEnv(env, "plugin-metadata", "get", pluginName, "-g", gatewayGroup, "-o", "yaml") + require.NoError(t, err, stderr) + + var meta map[string]interface{} + require.NoError(t, yaml.Unmarshal([]byte(stdout), &meta), "output should be a YAML map, got: %s", stdout) + logFormat, ok := meta["log_format"].(map[string]interface{}) + require.True(t, ok, "expected log_format map in: %s", stdout) + assert.Equal(t, "$host", logFormat["host"]) +} diff --git a/test/e2e/plugin_test.go b/test/e2e/plugin_test.go index fd29073..b6c8441 100644 --- a/test/e2e/plugin_test.go +++ b/test/e2e/plugin_test.go @@ -4,10 +4,12 @@ package e2e import ( "encoding/json" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) func TestPlugin_List(t *testing.T) { @@ -53,3 +55,16 @@ func TestPlugin_GetNonexistent(t *testing.T) { _, _, err := runA7WithEnv(env, "plugin", "get", "nonexistent-plugin-12345", "-g", gatewayGroup) assert.Error(t, err) } + +// TestPlugin_GetYAML guards against a regression where `plugin get -o yaml` +// ignored the requested format and emitted JSON instead. +func TestPlugin_GetYAML(t *testing.T) { + env := setupEnv(t) + + stdout, stderr, err := runA7WithEnv(env, "plugin", "get", "proxy-rewrite", "-g", gatewayGroup, "-o", "yaml") + require.NoError(t, err, stderr) + assert.False(t, strings.HasPrefix(strings.TrimSpace(stdout), "{"), "yaml output must not be JSON: %s", stdout) + var schema map[string]interface{} + require.NoError(t, yaml.Unmarshal([]byte(stdout), &schema), "output should be valid YAML") + assert.NotEmpty(t, schema) +} diff --git a/test/e2e/ssl_test.go b/test/e2e/ssl_test.go index ec2fc11..80e5650 100644 --- a/test/e2e/ssl_test.go +++ b/test/e2e/ssl_test.go @@ -148,3 +148,46 @@ func TestSSL_DeleteNonexistent(t *testing.T) { _, _, err := runA7WithEnv(env, "ssl", "delete", "nonexistent-ssl-12345", "--force", "-g", gatewayGroup) assert.Error(t, err) } + +// TestSSL_UpdateFlagsRequireCertAndKey guards against a regression where a +// flag-based `ssl update` without --cert/--key reported success but silently +// dropped the change: API7 EE never returns the existing cert/key on GET, so +// the merged PUT payload lost the certificate material. +func TestSSL_UpdateFlagsRequireCertAndKey(t *testing.T) { + env := setupEnv(t) + sslID := "e2e-ssl-update-requires-cert" + t.Cleanup(func() { deleteSSLViaAdmin(t, sslID) }) + + cert, key := readTestCert(t) + sslJSON := fmt.Sprintf(`{"id":%q,"cert":%q,"key":%q,"snis":["require-cert.example.com"]}`, sslID, cert, key) + tmpFile := filepath.Join(t.TempDir(), "ssl.json") + require.NoError(t, os.WriteFile(tmpFile, []byte(sslJSON), 0644)) + + stdout, stderr, err := runA7WithEnv(env, "ssl", "create", "-f", tmpFile, "-g", gatewayGroup) + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + + // A flag-based update with only --sni must fail loudly rather than + // pretend to succeed while dropping the cert/key. + _, stderr, err = runA7WithEnv(env, "ssl", "update", sslID, + "--sni", "require-cert-updated.example.com", "-g", gatewayGroup) + require.Error(t, err, "ssl update without --cert/--key must error") + assert.Contains(t, stderr, "--cert and --key") + + // The SNI on the server must be unchanged. + var ssl map[string]interface{} + runA7JSON(t, env, &ssl, "ssl", "get", sslID, "-g", gatewayGroup, "-o", "json") + snis := requireJSONArray(t, ssl["snis"], "ssl.snis") + assert.Contains(t, snis, "require-cert.example.com") + + // Supplying --cert and --key makes the update succeed and persist. + modRoot, err := resolveModuleRoot() + require.NoError(t, err) + stdout, stderr, err = runA7WithEnv(env, "ssl", "update", sslID, + "--cert", filepath.Join(modRoot, "test/e2e/testdata/test.crt"), + "--key", filepath.Join(modRoot, "test/e2e/testdata/test.key"), + "--sni", "require-cert-updated.example.com", "-g", gatewayGroup) + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + runA7JSON(t, env, &ssl, "ssl", "get", sslID, "-g", gatewayGroup, "-o", "json") + snis = requireJSONArray(t, ssl["snis"], "ssl.snis") + assert.Contains(t, snis, "require-cert-updated.example.com") +} diff --git a/test/e2e/stream_route_test.go b/test/e2e/stream_route_test.go index 75f63d3..a93d3bb 100644 --- a/test/e2e/stream_route_test.go +++ b/test/e2e/stream_route_test.go @@ -3,6 +3,7 @@ package e2e import ( + "encoding/json" "fmt" "io" "os" @@ -140,3 +141,61 @@ func TestStreamRoute_DeleteNonexistent(t *testing.T) { _, _, err := runA7WithEnv(env, "stream-route", "delete", "nonexistent-sr-12345", "--force", "-g", gatewayGroup) assert.Error(t, err) } + +// TestStreamRoute_CreateRequiresName guards against the gap where flag-based +// `stream-route create` had no --name flag, while API7 EE requires `name`. +func TestStreamRoute_CreateRequiresName(t *testing.T) { + env := setupEnv(t) + + _, stderr, err := runA7WithEnv(env, "stream-route", "create", + "--service-id", "any-service-id", "--server-port", "19099", "-g", gatewayGroup) + require.Error(t, err, "stream-route create without --name must error") + assert.Contains(t, stderr, "name is required") +} + +// createStreamServiceViaCLI creates a `type: stream` service; stream routes +// can only be attached to stream-typed services in API7 EE. +func createStreamServiceViaCLI(t testTB, env []string, id string) { + t.Helper() + svcJSON := fmt.Sprintf(`{ + "id": %q, + "name": "e2e-stream-svc-%s", + "type": "stream", + "upstream": { + "type": "roundrobin", + "nodes": [{"host": "127.0.0.1", "port": 3306, "weight": 1}] + } + }`, id, id) + tmpFile := filepath.Join(t.TempDir(), "stream-service.json") + require.NoError(t, os.WriteFile(tmpFile, []byte(svcJSON), 0644)) + stdout, stderr, err := runA7WithEnv(env, "service", "create", "-f", tmpFile, "-g", gatewayGroup) + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) +} + +// TestStreamRoute_CreateWithFlags exercises the flag-based create path, +// including the --name flag added to satisfy API7 EE's required field. +func TestStreamRoute_CreateWithFlags(t *testing.T) { + env := setupEnv(t) + svcID := "e2e-stream-route-flags-svc" + t.Cleanup(func() { deleteServiceViaAdmin(t, svcID) }) + createStreamServiceViaCLI(t, env, svcID) + + srName := "e2e-stream-route-flags" + stdout, stderr, err := runA7WithEnv(env, "stream-route", "create", + "--name", srName, "--service-id", svcID, "--server-port", "19098", "-g", gatewayGroup) + if err != nil { + t.Skipf("stream-route create failed (may not be enabled): %s %s", stdout, stderr) + } + + var created map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(stdout), &created), "create should return JSON: %s", stdout) + assert.Equal(t, srName, created["name"]) + srID, ok := created["id"].(string) + require.True(t, ok && srID != "", "create response should contain an id: %v", created) + t.Cleanup(func() { deleteStreamRouteViaAdmin(t, srID) }) + + var got map[string]interface{} + runA7JSON(t, env, &got, "stream-route", "get", srID, "-g", gatewayGroup, "-o", "json") + assert.Equal(t, srName, got["name"]) + assert.Equal(t, float64(19098), got["server_port"]) +}