From 50b7e3bca251974dd59b1c2dffa7b4fe970e5757 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Tue, 26 May 2026 12:08:19 +0800 Subject: [PATCH 1/3] fix: decode API responses before exporting to avoid yaml byte-array bug Writing a json.RawMessage directly to the yaml.v3 encoder serializes the bytes as a list of integers, not a map. PR #31 fixed this for `plugin-metadata get` and PR #39's review fixup did it for one stream-route create path, but the same anti-pattern lived in 21 other sites across 12 resources (every flag/file create and update path for route, service, consumer, credential, ssl, secret, global-rule, proto, plugin-metadata, stream-route, gateway-group). - Add cmdutil.Exporter.WriteAPIResponse(body []byte) that decodes once via encoding/json and writes through the existing Write() formatter. - Replace all 21 `.Write(json.RawMessage(body))` call sites with `.WriteAPIResponse(body)`. - Drop now-unused encoding/json imports from consumer create/update. - Unit tests for the helper cover YAML emits a map (the regression guard), JSON round-trips an object, and invalid JSON returns a clear error. Verified live against API7 EE 3.9.12: service/route file-based create `-o yaml` now emit proper maps instead of integer sequences. Closes #32. --- pkg/cmd/consumer/create/create.go | 3 +- pkg/cmd/consumer/update/update.go | 3 +- pkg/cmd/credential/create/create.go | 2 +- pkg/cmd/credential/update/update.go | 2 +- pkg/cmd/gateway-group/create/create.go | 2 +- pkg/cmd/gateway-group/update/update.go | 2 +- pkg/cmd/global-rule/create/create.go | 2 +- pkg/cmd/global-rule/update/update.go | 2 +- pkg/cmd/plugin-metadata/create/create.go | 4 +- pkg/cmd/plugin-metadata/update/update.go | 4 +- pkg/cmd/proto/create/create.go | 2 +- pkg/cmd/proto/update/update.go | 2 +- pkg/cmd/route/create/create.go | 2 +- pkg/cmd/route/update/update.go | 2 +- pkg/cmd/service/create/create.go | 2 +- pkg/cmd/service/update/update.go | 2 +- pkg/cmd/ssl/create/create.go | 2 +- pkg/cmd/ssl/update/update.go | 2 +- pkg/cmd/stream-route/update/update.go | 2 +- pkg/cmdutil/exporter.go | 12 +++++ pkg/cmdutil/exporter_test.go | 63 ++++++++++++++++++++++++ 21 files changed, 96 insertions(+), 23 deletions(-) create mode 100644 pkg/cmdutil/exporter_test.go diff --git a/pkg/cmd/consumer/create/create.go b/pkg/cmd/consumer/create/create.go index d96b41c..637944c 100644 --- a/pkg/cmd/consumer/create/create.go +++ b/pkg/cmd/consumer/create/create.go @@ -1,7 +1,6 @@ package create import ( - "encoding/json" "fmt" "net/http" "strings" @@ -93,7 +92,7 @@ func actionRun(opts *Options) error { if output == "" { output = "json" } - return cmdutil.NewExporter(output, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(output, opts.IO.Out).WriteAPIResponse(body) } if opts.Username == "" { return fmt.Errorf("--username is required") diff --git a/pkg/cmd/consumer/update/update.go b/pkg/cmd/consumer/update/update.go index 280e75c..702706d 100644 --- a/pkg/cmd/consumer/update/update.go +++ b/pkg/cmd/consumer/update/update.go @@ -1,7 +1,6 @@ package update import ( - "encoding/json" "errors" "fmt" "net/http" @@ -88,7 +87,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } body := api.Consumer{ diff --git a/pkg/cmd/credential/create/create.go b/pkg/cmd/credential/create/create.go index a5e9185..78a3e91 100644 --- a/pkg/cmd/credential/create/create.go +++ b/pkg/cmd/credential/create/create.go @@ -115,7 +115,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } httpClient, err := opts.Client() diff --git a/pkg/cmd/credential/update/update.go b/pkg/cmd/credential/update/update.go index 28d7b90..c492051 100644 --- a/pkg/cmd/credential/update/update.go +++ b/pkg/cmd/credential/update/update.go @@ -99,7 +99,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } pl := make(map[string]interface{}) diff --git a/pkg/cmd/gateway-group/create/create.go b/pkg/cmd/gateway-group/create/create.go index fc4349d..42c16c9 100644 --- a/pkg/cmd/gateway-group/create/create.go +++ b/pkg/cmd/gateway-group/create/create.go @@ -85,7 +85,7 @@ func createRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.Name == "" { diff --git a/pkg/cmd/gateway-group/update/update.go b/pkg/cmd/gateway-group/update/update.go index db4eec2..4874497 100644 --- a/pkg/cmd/gateway-group/update/update.go +++ b/pkg/cmd/gateway-group/update/update.go @@ -88,7 +88,7 @@ func updateRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } labels := map[string]string{} diff --git a/pkg/cmd/global-rule/create/create.go b/pkg/cmd/global-rule/create/create.go index 7647d06..70bc407 100644 --- a/pkg/cmd/global-rule/create/create.go +++ b/pkg/cmd/global-rule/create/create.go @@ -85,7 +85,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.ID == "" { return fmt.Errorf("--id is required") diff --git a/pkg/cmd/global-rule/update/update.go b/pkg/cmd/global-rule/update/update.go index 26fe719..82ba990 100644 --- a/pkg/cmd/global-rule/update/update.go +++ b/pkg/cmd/global-rule/update/update.go @@ -82,7 +82,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } plugins := make(map[string]interface{}) diff --git a/pkg/cmd/plugin-metadata/create/create.go b/pkg/cmd/plugin-metadata/create/create.go index b1302b2..2132920 100644 --- a/pkg/cmd/plugin-metadata/create/create.go +++ b/pkg/cmd/plugin-metadata/create/create.go @@ -92,7 +92,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.PluginName == "" { return fmt.Errorf("--plugin-name is required") @@ -121,5 +121,5 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } diff --git a/pkg/cmd/plugin-metadata/update/update.go b/pkg/cmd/plugin-metadata/update/update.go index 1bda0ba..d7ee1e1 100644 --- a/pkg/cmd/plugin-metadata/update/update.go +++ b/pkg/cmd/plugin-metadata/update/update.go @@ -78,7 +78,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } metadata := map[string]interface{}{} @@ -98,5 +98,5 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } diff --git a/pkg/cmd/proto/create/create.go b/pkg/cmd/proto/create/create.go index 87e15f9..f0c54bc 100644 --- a/pkg/cmd/proto/create/create.go +++ b/pkg/cmd/proto/create/create.go @@ -88,7 +88,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } httpClient, err := opts.Client() diff --git a/pkg/cmd/proto/update/update.go b/pkg/cmd/proto/update/update.go index a753573..735f86f 100644 --- a/pkg/cmd/proto/update/update.go +++ b/pkg/cmd/proto/update/update.go @@ -90,7 +90,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } labels := make(map[string]string) diff --git a/pkg/cmd/route/create/create.go b/pkg/cmd/route/create/create.go index 6f7944a..b25f24d 100644 --- a/pkg/cmd/route/create/create.go +++ b/pkg/cmd/route/create/create.go @@ -103,7 +103,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.URI == "" && len(opts.Paths) == 0 { return fmt.Errorf("--path or --uri is required") diff --git a/pkg/cmd/route/update/update.go b/pkg/cmd/route/update/update.go index 2079ffb..d02aef8 100644 --- a/pkg/cmd/route/update/update.go +++ b/pkg/cmd/route/update/update.go @@ -102,7 +102,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } labels := make(map[string]string) diff --git a/pkg/cmd/service/create/create.go b/pkg/cmd/service/create/create.go index 895b1c8..bc0da9e 100644 --- a/pkg/cmd/service/create/create.go +++ b/pkg/cmd/service/create/create.go @@ -89,7 +89,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.Name == "" { return fmt.Errorf("--name is required") diff --git a/pkg/cmd/service/update/update.go b/pkg/cmd/service/update/update.go index ba79a85..873db2a 100644 --- a/pkg/cmd/service/update/update.go +++ b/pkg/cmd/service/update/update.go @@ -85,7 +85,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } labels := make(map[string]string) diff --git a/pkg/cmd/ssl/create/create.go b/pkg/cmd/ssl/create/create.go index 5d133fc..53b758d 100644 --- a/pkg/cmd/ssl/create/create.go +++ b/pkg/cmd/ssl/create/create.go @@ -156,7 +156,7 @@ func actionRun(opts *Options) error { func writeSSLResponse(format string, out io.Writer, body []byte) error { var item api.SSL if err := json.Unmarshal(body, &item); err != nil { - return cmdutil.NewExporter(format, out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, out).WriteAPIResponse(body) } return cmdutil.NewExporter(format, out).Write(api.RedactSSL(item)) } diff --git a/pkg/cmd/ssl/update/update.go b/pkg/cmd/ssl/update/update.go index 8e5536c..658b14e 100644 --- a/pkg/cmd/ssl/update/update.go +++ b/pkg/cmd/ssl/update/update.go @@ -179,7 +179,7 @@ func actionRun(opts *Options) error { func writeSSLResponse(format string, out io.Writer, body []byte) error { var item api.SSL if err := json.Unmarshal(body, &item); err != nil { - return cmdutil.NewExporter(format, out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, out).WriteAPIResponse(body) } return cmdutil.NewExporter(format, out).Write(api.RedactSSL(item)) } diff --git a/pkg/cmd/stream-route/update/update.go b/pkg/cmd/stream-route/update/update.go index c1f285e..507a702 100644 --- a/pkg/cmd/stream-route/update/update.go +++ b/pkg/cmd/stream-route/update/update.go @@ -94,7 +94,7 @@ func actionRun(opts *Options) error { if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body)) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) } if opts.ServiceID == "" { return fmt.Errorf("--service-id is required for current API7 EE") diff --git a/pkg/cmdutil/exporter.go b/pkg/cmdutil/exporter.go index ef05388..9b6d895 100644 --- a/pkg/cmdutil/exporter.go +++ b/pkg/cmdutil/exporter.go @@ -35,6 +35,18 @@ func (e *Exporter) Write(data interface{}) error { } } +// WriteAPIResponse decodes a JSON API response body and writes it via the +// exporter. Writing a raw []byte (e.g. json.RawMessage) directly would cause +// yaml.v3 to emit the bytes as a sequence of integers; decoding first ensures +// both -o yaml and -o json render a proper object. +func (e *Exporter) WriteAPIResponse(body []byte) error { + var decoded interface{} + if err := json.Unmarshal(body, &decoded); err != nil { + return fmt.Errorf("failed to decode response: %w", err) + } + return e.Write(decoded) +} + func (e *Exporter) writeJSON(data interface{}) error { enc := json.NewEncoder(e.writer) enc.SetIndent("", " ") diff --git a/pkg/cmdutil/exporter_test.go b/pkg/cmdutil/exporter_test.go new file mode 100644 index 0000000..eef8a99 --- /dev/null +++ b/pkg/cmdutil/exporter_test.go @@ -0,0 +1,63 @@ +package cmdutil + +import ( + "bytes" + "encoding/json" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestExporter_WriteAPIResponse_YAML_DecodesObject(t *testing.T) { + // The original bug: writing json.RawMessage([]byte) directly to the YAML + // encoder serialized the bytes as a list of integers instead of a map. + // WriteAPIResponse must decode first so YAML emits a proper object. + body := []byte(`{"id":"r1","name":"demo","status":1}`) + + var buf bytes.Buffer + if err := NewExporter("yaml", &buf).WriteAPIResponse(body); err != nil { + t.Fatalf("WriteAPIResponse failed: %v", err) + } + + out := buf.String() + if strings.HasPrefix(strings.TrimSpace(out), "- ") { + t.Fatalf("yaml output looks like a byte sequence, not an object: %q", out) + } + + var decoded map[string]interface{} + if err := yaml.Unmarshal(buf.Bytes(), &decoded); err != nil { + t.Fatalf("yaml output is not a valid map: %v (output: %q)", err, out) + } + if decoded["id"] != "r1" || decoded["name"] != "demo" { + t.Fatalf("yaml output missing expected fields: got %v", decoded) + } +} + +func TestExporter_WriteAPIResponse_JSON_RoundTripsObject(t *testing.T) { + body := []byte(`{"id":"r1","name":"demo"}`) + + var buf bytes.Buffer + if err := NewExporter("json", &buf).WriteAPIResponse(body); err != nil { + t.Fatalf("WriteAPIResponse failed: %v", err) + } + + var decoded map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil { + t.Fatalf("json output is not a valid object: %v (output: %q)", err, buf.String()) + } + if decoded["id"] != "r1" || decoded["name"] != "demo" { + t.Fatalf("json output missing expected fields: got %v", decoded) + } +} + +func TestExporter_WriteAPIResponse_InvalidJSON_ReturnsError(t *testing.T) { + body := []byte(`not json`) + err := NewExporter("yaml", &bytes.Buffer{}).WriteAPIResponse(body) + if err == nil { + t.Fatal("expected error decoding invalid JSON") + } + if !strings.Contains(err.Error(), "failed to decode response") { + t.Fatalf("error did not mention decoding: %v", err) + } +} From e0b4badaf48e7a97f3e9acbbceed7cc195b45eb3 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Tue, 26 May 2026 12:08:19 +0800 Subject: [PATCH 2/3] ci(e2e): make the suite verbose so it isn't dark for minutes Without -v, `go test` prints nothing between tests; combined with a silent TestMain (binary build + dashboard health-wait + group resolve) the CI log shows nothing for the first 2+ minutes of a healthy run. - Add -v to both `make test-e2e` and `make test-e2e-full` so each test emits === RUN / --- PASS lines as it runs. - Print three progress markers to stderr in TestMain so the dark setup phase has visible breadcrumbs: building binary, waiting for dashboard, resolving gateway group. Closes #41. --- Makefile | 4 ++-- test/e2e/setup_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 00e8114..9316b76 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ docker-down: # The default CI stays narrower via runtime guards such as requireGatewayURL, # requireHTTPBin, and A7_E2E_ENABLE_GATEWAY_GROUP_CRUD. test-e2e: - go run github.com/onsi/ginkgo/v2/ginkgo -r --procs=1 --tags=e2e --timeout=45m ./test/e2e/... + go run github.com/onsi/ginkgo/v2/ginkgo -r --procs=1 --tags=e2e --timeout=45m -v ./test/e2e/... test-e2e-full: - go run github.com/onsi/ginkgo/v2/ginkgo -r --procs=1 --tags=e2e --timeout=45m ./test/e2e/... + go run github.com/onsi/ginkgo/v2/ginkgo -r --procs=1 --tags=e2e --timeout=45m -v ./test/e2e/... diff --git a/test/e2e/setup_test.go b/test/e2e/setup_test.go index 43f239e..39ebb69 100644 --- a/test/e2e/setup_test.go +++ b/test/e2e/setup_test.go @@ -114,6 +114,7 @@ func TestMain(m *testing.M) { os.Exit(1) } + fmt.Fprintln(os.Stderr, "Building a7 binary ...") buildCmd := exec.Command("go", "build", "-o", binaryPath, "./cmd/a7") buildCmd.Dir = modRoot buildCmd.Stdout = os.Stdout @@ -126,6 +127,7 @@ func TestMain(m *testing.M) { // Wait for API7 EE Dashboard API to become healthy. // Try the /api/status endpoint first, fall back to /api/gateway_groups. healthURL := adminURL + "/api/gateway_groups" + fmt.Fprintln(os.Stderr, "Waiting for API7 EE dashboard at "+adminURL+" ...") if err := waitForHealthy(healthURL, 120*time.Second); err != nil { fmt.Fprintf(os.Stderr, "API7 EE not ready: %v\n", err) os.Exit(1) @@ -134,6 +136,7 @@ func TestMain(m *testing.M) { // API7 EE uses UUID-style ids for runtime API calls. Resolve name -> id. // An explicit name (incl. "default") is honored literally; only an // unset/empty A7_GATEWAY_GROUP falls back to "first non-ingress group". + fmt.Fprintln(os.Stderr, "Resolving gateway group ...") wanted := os.Getenv("A7_GATEWAY_GROUP") ggID, err := resolveGatewayGroupID(wanted) if err != nil { From 34a72ae04a2a2cb84976c96914671645e51e6747 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Tue, 26 May 2026 14:28:20 +0800 Subject: [PATCH 3/3] fix(ssl): fail closed when ssl response can't be decoded The previous fallback dumped the raw response body via WriteAPIResponse, bypassing api.RedactSSL and exporting the private key in plaintext when the body didn't decode into api.SSL. Return an error in that path so the key is never serialized unredacted. --- pkg/cmd/ssl/create/create.go | 2 +- pkg/cmd/ssl/update/update.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/ssl/create/create.go b/pkg/cmd/ssl/create/create.go index 53b758d..313da43 100644 --- a/pkg/cmd/ssl/create/create.go +++ b/pkg/cmd/ssl/create/create.go @@ -156,7 +156,7 @@ func actionRun(opts *Options) error { func writeSSLResponse(format string, out io.Writer, body []byte) error { var item api.SSL if err := json.Unmarshal(body, &item); err != nil { - return cmdutil.NewExporter(format, out).WriteAPIResponse(body) + return fmt.Errorf("failed to decode ssl response for safe export: %w", err) } return cmdutil.NewExporter(format, out).Write(api.RedactSSL(item)) } diff --git a/pkg/cmd/ssl/update/update.go b/pkg/cmd/ssl/update/update.go index 658b14e..42a0c66 100644 --- a/pkg/cmd/ssl/update/update.go +++ b/pkg/cmd/ssl/update/update.go @@ -179,7 +179,7 @@ func actionRun(opts *Options) error { func writeSSLResponse(format string, out io.Writer, body []byte) error { var item api.SSL if err := json.Unmarshal(body, &item); err != nil { - return cmdutil.NewExporter(format, out).WriteAPIResponse(body) + return fmt.Errorf("failed to decode ssl response for safe export: %w", err) } return cmdutil.NewExporter(format, out).Write(api.RedactSSL(item)) }