diff --git a/docs/ga-test-report.md b/docs/ga-test-report.md index a9df5a7..1cc8eea 100644 --- a/docs/ga-test-report.md +++ b/docs/ga-test-report.md @@ -28,7 +28,7 @@ All four phases executed. **5 real bugs found, all fixed with E2E + unit coverag | 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 | credential | PASS | — | — | CRUD via server-returned id. `create [id]` positional is now forwarded as the credential id via `PUT` (issue #36); a separate `--name` flag sets the display name. | | 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. | @@ -67,7 +67,7 @@ A declarative file with a top-level `service_templates:` section validated as "C ## 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. +- **credential**: `create [id]` now forwards the positional as the credential id via `PUT /apisix/admin/consumers//credentials/` (issue #36). The display name has moved to a dedicated `--name` flag. - **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. @@ -124,7 +124,7 @@ Re-run after the Run 1 fixes (PR #31) and the `plugin-config` removal (PR #34) m |---|---|---|---|---| | R2-1 | 🟡 Bug | route | README documents `a7 route update --desc "..."` but neither `route create` nor `route update` exposes a `--desc` flag. Description is only settable through `-f`. | ✅ Fixed in PR #39 / closes #35 | | R2-2 | 🟡 UX | route | `a7 route list -g default` errors with `--service-id is required by API7 EE`. The e2e helper iterates services to aggregate routes; the CLI doesn't. | #42 (post-GA candidate) | -| R2-3 | 🟡 Bug | credential | `a7 credential create smoke-cred-X --consumer Y ...` returned a server-assigned UUID, ignoring the positional id. (Run 1 noted this as "intended" via `TestCredential_CreateWithPositionalID` — needs re-confirmation; if intended, drop the misleading `[id]` from the help.) | #36 | +| R2-3 | 🟡 Bug | credential | `a7 credential create smoke-cred-X --consumer Y ...` returned a server-assigned UUID, ignoring the positional id. | ✅ Fixed: positional now forwarded as id via `PUT`, `--name` added for display name (closes #36) | | R2-4 | 🟡 Bug | global-rule | `a7 global-rule create --id X --plugins-json '{"cors":...}'` ignores `--id`; resource is created at `id=cors`. Run 1 noted "value is ignored by EE" as a minor; treat as a real bug: the CLI shouldn't accept a flag it will silently override. | #37 | | R2-5 | 🟢 Cosmetic | stream-route | `-o yaml` renders `created_at: 1.779521636e+09` in scientific notation. Should be plain integer. | Note only, untracked | diff --git a/docs/user-guide/credential.md b/docs/user-guide/credential.md index 6f3ed3a..3e454b5 100644 --- a/docs/user-guide/credential.md +++ b/docs/user-guide/credential.md @@ -56,22 +56,42 @@ a7 credential get key-1 --consumer jack -g default ### `a7 credential create` -Creates a new credential for a consumer from a JSON or YAML file. +Creates a new credential for a consumer. + +**Usage:** `a7 credential create [id] --consumer -g ` + +The positional `[id]` is optional. When supplied (either as the positional or as `id:` inside a `--file` payload), the CLI sends `PUT /apisix/admin/consumers//credentials/` and the credential is created with that exact id. When omitted, the CLI sends `POST` and the server assigns a UUID. If both the positional and the file specify an id, the positional wins. | Flag | Short | Default | Description | |------|-------|---------|-------------| | `--consumer` | | | Consumer username (required) | | `--gateway-group` | `-g` | | Target gateway group (required) | -| `--file` | `-f` | | Path to credential config file (required) | -| `--output` | `-o` | `yaml` | Output format (`json`, `yaml`) | +| `--file` | `-f` | | Path to credential config file (JSON or YAML) | +| `--name` | | | Credential display name (separate from the id) | +| `--desc` | | | Credential description | +| `--plugins-json` | | | Plugins as a JSON string | +| `--labels` | | | Labels in `key=value` format (repeatable) | +| `--output` | `-o` | `json` | Output format (`json`, `yaml`) | **Examples:** -Create a credential from YAML: +Create a credential from YAML (id taken from the file's `id:` field, or auto-assigned if absent): ```bash a7 credential create --consumer jack -g default -f key-auth.yaml ``` +Create a credential with a client-chosen id and a display name: +```bash +a7 credential create my-key --consumer jack -g default --name "Jack's API key" \ + --plugins-json '{"key-auth":{"key":"secret-token-val"}}' +``` + +Let the server assign the id: +```bash +a7 credential create --consumer jack -g default --name "Jack's API key" \ + --plugins-json '{"key-auth":{"key":"secret-token-val"}}' +``` + ### `a7 credential update` Updates an existing credential by ID using a JSON or YAML file. API7 EE uses JSON Patch (RFC 6902) for partial updates. diff --git a/pkg/cmd/credential/create/create.go b/pkg/cmd/credential/create/create.go index 78a3e91..de8971c 100644 --- a/pkg/cmd/credential/create/create.go +++ b/pkg/cmd/credential/create/create.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strings" "github.com/spf13/cobra" @@ -25,6 +26,7 @@ type Options struct { File string ID string + Name string Desc string PluginsJSON string Labels []string @@ -34,7 +36,7 @@ func NewCmd(f *cmd.Factory) *cobra.Command { opts := &Options{IO: f.IOStreams, Client: f.HttpClient, Config: f.Config} c := &cobra.Command{ Use: "create [id]", - Short: "Create a credential", + Short: "Create a credential (id is server-generated if omitted)", Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.Output, _ = c.Flags().GetString("output") @@ -49,6 +51,7 @@ func NewCmd(f *cmd.Factory) *cobra.Command { c.Flags().StringVar(&opts.Consumer, "consumer", "", "Consumer username") c.Flags().StringVarP(&opts.File, "file", "f", "", "Path to JSON/YAML file with resource definition") + c.Flags().StringVar(&opts.Name, "name", "", "Credential display name") c.Flags().StringVar(&opts.Desc, "desc", "", "Credential description") c.Flags().StringVar(&opts.PluginsJSON, "plugins-json", "", "Plugins JSON string") c.Flags().StringSliceVar(&opts.Labels, "labels", nil, "Labels in key=value format") @@ -73,54 +76,48 @@ func actionRun(opts *Options) error { if ggID == "" { return fmt.Errorf("gateway group is required; use --gateway-group flag or set a default in context config") } + if opts.File != "" { payload, err := cmdutil.ReadResourceFile(opts.File, opts.IO.In) if err != nil { return err } - if opts.ID != "" { - payload["name"] = opts.ID - delete(payload, "id") - } else if _, ok := payload["name"]; ok { - delete(payload, "id") - } else if id, ok := payload["id"]; ok { - payload["name"] = id - delete(payload, "id") - } - - name, hasName, err := credentialNameFromPayload(payload) + // Validate the file's id (if present) regardless of whether the + // positional overrides it: a bogus value in the file is a user error + // worth surfacing. + fileID, hasFileID, err := stringField(payload, "id") if err != nil { return err } - httpClient, err := opts.Client() - if err != nil { - return err + // Positional [id] overrides any id in the file payload. + id := strings.TrimSpace(opts.ID) + if id == "" && hasFileID { + id = fileID } + // The id, if any, is carried in the URL path on PUT. Drop it from the body + // so the request body stays clean. + delete(payload, "id") - path := "/apisix/admin/consumers/" + opts.Consumer + "/credentials?gateway_group_id=" + ggID - client := api.NewClient(httpClient, cfg.BaseURL()) - var body []byte - if hasName { - body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%s?gateway_group_id=%s", opts.Consumer, name, ggID), payload) - } else { - body, err = client.Post(path, payload) + if opts.Name != "" { + payload["name"] = strings.TrimSpace(opts.Name) } - if err != nil { - return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) + // API7 EE requires a non-empty `name` on credentials. When the caller + // gave us an id but no explicit name, mirror the id into the name. + if _, hasName := payload["name"]; !hasName && id != "" { + payload["name"] = id } - - format := opts.Output - if format == "" { - format = "json" + if _, _, err := stringField(payload, "name"); err != nil { + return err } - return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(body) - } - httpClient, err := opts.Client() - if err != nil { - return err + httpClient, err := opts.Client() + if err != nil { + return err + } + client := api.NewClient(httpClient, cfg.BaseURL()) + return submit(client, opts, ggID, id, payload) } pl := make(map[string]interface{}) @@ -139,7 +136,14 @@ func actionRun(opts *Options) error { labels[parts[0]] = parts[1] } - bodyReq := api.Credential{Name: opts.ID, Desc: opts.Desc} + id := strings.TrimSpace(opts.ID) + name := strings.TrimSpace(opts.Name) + if name == "" && id != "" { + // API7 EE requires a non-empty `name`. Mirror the id when the caller + // did not pass --name explicitly. + name = id + } + bodyReq := api.Credential{Name: name, Desc: opts.Desc} if len(pl) > 0 { bodyReq.Plugins = pl } @@ -147,33 +151,54 @@ func actionRun(opts *Options) error { bodyReq.Labels = labels } - path := "/apisix/admin/consumers/" + opts.Consumer + "/credentials?gateway_group_id=" + ggID - client := api.NewClient(httpClient, cfg.BaseURL()) - body, err := client.Post(path, bodyReq) + httpClient, err := opts.Client() if err != nil { - return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) + return err } + client := api.NewClient(httpClient, cfg.BaseURL()) + return submit(client, opts, ggID, id, bodyReq) +} - var created api.Credential - if err := json.Unmarshal(body, &created); err != nil { - return fmt.Errorf("failed to decode response: %w", err) +func submit(client *api.Client, opts *Options, ggID, id string, body interface{}) error { + var ( + raw []byte + err error + ) + consumer := url.PathEscape(opts.Consumer) + group := url.QueryEscape(ggID) + if id != "" { + raw, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%s?gateway_group_id=%s", consumer, url.PathEscape(id), group), body) + } else { + raw, err = client.Post(fmt.Sprintf("/apisix/admin/consumers/%s/credentials?gateway_group_id=%s", consumer, group), body) + } + if err != nil { + return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } format := opts.Output if format == "" { format = "json" } - return cmdutil.NewExporter(format, opts.IO.Out).Write(created) + return cmdutil.NewExporter(format, opts.IO.Out).WriteAPIResponse(raw) } -func credentialNameFromPayload(payload map[string]interface{}) (string, bool, error) { - rawName, ok := payload["name"] +// stringField pulls a string field out of a parsed file payload. It returns +// the trimmed value, a bool indicating whether the key was present, or an +// error if the key was present but not a non-empty string (after trimming). +// The trimmed value is written back to the payload when present. +func stringField(payload map[string]interface{}, key string) (string, bool, error) { + raw, ok := payload[key] if !ok { return "", false, nil } - name, ok := rawName.(string) - if !ok || strings.TrimSpace(name) == "" { - return "", false, fmt.Errorf("credential name must be a non-empty string") + str, ok := raw.(string) + if !ok { + return "", true, fmt.Errorf("credential %s must be a non-empty string", key) + } + trimmed := strings.TrimSpace(str) + if trimmed == "" { + return "", true, fmt.Errorf("credential %s must be a non-empty string", key) } - return name, true, nil + payload[key] = trimmed + return trimmed, true, nil } diff --git a/pkg/cmd/credential/create/create_test.go b/pkg/cmd/credential/create/create_test.go index 0763990..e3c53da 100644 --- a/pkg/cmd/credential/create/create_test.go +++ b/pkg/cmd/credential/create/create_test.go @@ -57,26 +57,23 @@ func TestCreateCredential_JSONOutput(t *testing.T) { registry.Verify(t) } -func TestCreateCredential_PositionalIDSetsName(t *testing.T) { +func TestCreateCredential_PositionalForwardsAsIDViaPUT(t *testing.T) { ios, _, out, _ := iostreams.Test() registry := &httpmock.Registry{} - registry.RegisterResponder(http.MethodPost, "/apisix/admin/consumers/alice/credentials", func(req *http.Request) (httpmock.Response, error) { - var payload map[string]interface{} + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/cred1", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]any if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) } - if payload["name"] != "cred1" { - return httpmock.Response{}, fmt.Errorf("expected positional id to map to name, got payload: %#v", payload) - } if _, ok := payload["id"]; ok { - return httpmock.Response{}, fmt.Errorf("expected positional id to be normalized to name only, got payload: %#v", payload) + return httpmock.Response{}, fmt.Errorf("expected id to be carried in URL, not body, got: %#v", payload) } - return httpmock.JSONResponse(`{"id":"generated","name":"cred1"}`), nil + return httpmock.JSONResponse(`{"id":"cred1","name":"key-auth"}`), nil }) opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil - }, Consumer: "alice", GatewayGroup: "gg1", ID: "cred1", PluginsJSON: `{"key-auth":{"key":"k"}}`} + }, Consumer: "alice", GatewayGroup: "gg1", ID: "cred1", Name: "key-auth", PluginsJSON: `{"key-auth":{"key":"k"}}`} if err := actionRun(opts); err != nil { t.Fatalf("actionRun failed: %v", err) @@ -85,14 +82,38 @@ func TestCreateCredential_PositionalIDSetsName(t *testing.T) { if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } - if item.Name != "cred1" { - t.Fatalf("expected credential name in output, got %+v", item) + if item.ID != "cred1" { + t.Fatalf("expected credential id from server response, got %+v", item) } registry.Verify(t) } -func TestCreateCredential_FileNormalizesLegacyID(t *testing.T) { - ios, _, out, _ := iostreams.Test() +func TestCreateCredential_PositionalAutoDerivesName(t *testing.T) { + ios, _, _, _ := iostreams.Test() + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/cred1", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]any + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) + } + if payload["name"] != "cred1" { + return httpmock.Response{}, fmt.Errorf("expected name to default to id when --name omitted, got: %#v", payload) + } + return httpmock.JSONResponse(`{"id":"cred1","name":"cred1"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", ID: "cred1", PluginsJSON: `{"key-auth":{"key":"k"}}`} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + registry.Verify(t) +} + +func TestCreateCredential_FileIDAutoDerivesName(t *testing.T) { + ios, _, _, _ := iostreams.Test() tmpFile := t.TempDir() + "/credential.json" if err := os.WriteFile(tmpFile, []byte(`{"id":"cred-file","plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { t.Fatalf("write credential file: %v", err) @@ -100,17 +121,74 @@ func TestCreateCredential_FileNormalizesLegacyID(t *testing.T) { registry := &httpmock.Registry{} registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/cred-file", func(req *http.Request) (httpmock.Response, error) { - var payload map[string]interface{} + var payload map[string]any if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) } if payload["name"] != "cred-file" { - return httpmock.Response{}, fmt.Errorf("expected legacy id to map to name, got payload: %#v", payload) + return httpmock.Response{}, fmt.Errorf("expected name to default to id when file omits name, got: %#v", payload) + } + return httpmock.JSONResponse(`{"id":"cred-file","name":"cred-file"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", File: tmpFile} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + registry.Verify(t) +} + +func TestCreateCredential_NameFlagPOSTs(t *testing.T) { + ios, _, out, _ := iostreams.Test() + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPost, "/apisix/admin/consumers/alice/credentials", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]any + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) + } + if payload["name"] != "display-name" { + return httpmock.Response{}, fmt.Errorf("expected --name to populate name, got: %#v", payload) + } + return httpmock.JSONResponse(`{"id":"generated-uuid","name":"display-name"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", Name: "display-name", PluginsJSON: `{"key-auth":{"key":"k"}}`} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + var item api.Credential + if err := json.Unmarshal(out.Bytes(), &item); err != nil { + t.Fatalf("failed to parse output: %v", err) + } + if item.ID != "generated-uuid" || item.Name != "display-name" { + t.Fatalf("expected server-generated id and explicit name, got %+v", item) + } + registry.Verify(t) +} + +func TestCreateCredential_FileIDForwardedViaPUT(t *testing.T) { + ios, _, out, _ := iostreams.Test() + tmpFile := t.TempDir() + "/credential.json" + if err := os.WriteFile(tmpFile, []byte(`{"id":"cred-file","plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + t.Fatalf("write credential file: %v", err) + } + + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/cred-file", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]any + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) } if _, ok := payload["id"]; ok { - return httpmock.Response{}, fmt.Errorf("expected legacy id to be removed, got payload: %#v", payload) + return httpmock.Response{}, fmt.Errorf("expected id to be stripped from body when carried in URL, got: %#v", payload) } - return httpmock.JSONResponse(`{"id":"generated","name":"cred-file"}`), nil + return httpmock.JSONResponse(`{"id":"cred-file"}`), nil }) opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { @@ -124,8 +202,92 @@ func TestCreateCredential_FileNormalizesLegacyID(t *testing.T) { if err := json.Unmarshal(out.Bytes(), &item); err != nil { t.Fatalf("failed to parse output: %v", err) } - if item.Name != "cred-file" { - t.Fatalf("expected credential name in output, got %+v", item) + if item.ID != "cred-file" { + t.Fatalf("expected credential id from file, got %+v", item) + } + registry.Verify(t) +} + +func TestCreateCredential_PositionalOverridesFileID(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tmpFile := t.TempDir() + "/credential.json" + if err := os.WriteFile(tmpFile, []byte(`{"id":"from-file","plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + t.Fatalf("write credential file: %v", err) + } + + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/from-positional", func(req *http.Request) (httpmock.Response, error) { + return httpmock.JSONResponse(`{"id":"from-positional"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", ID: "from-positional", File: tmpFile} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + registry.Verify(t) +} + +func TestCreateCredential_FileRejectsInvalidID(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tmpFile := t.TempDir() + "/credential.json" + if err := os.WriteFile(tmpFile, []byte(`{"id":123,"plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + t.Fatalf("write credential file: %v", err) + } + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { + t.Fatal("unexpected HTTP client call") + return nil, nil + }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", File: tmpFile} + + err := actionRun(opts) + if err == nil || !strings.Contains(err.Error(), "credential id must be a non-empty string") { + t.Fatalf("expected invalid credential id error, got: %v", err) + } +} + +func TestCreateCredential_FileRejectsInvalidIDEvenWhenPositionalOverrides(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tmpFile := t.TempDir() + "/credential.json" + if err := os.WriteFile(tmpFile, []byte(`{"id":123,"plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + t.Fatalf("write credential file: %v", err) + } + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { + t.Fatal("unexpected HTTP client call") + return nil, nil + }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", ID: "override", File: tmpFile} + + err := actionRun(opts) + if err == nil || !strings.Contains(err.Error(), "credential id must be a non-empty string") { + t.Fatalf("expected invalid credential id error even when positional overrides, got: %v", err) + } +} + +func TestCreateCredential_FileIDIsTrimmed(t *testing.T) { + ios, _, _, _ := iostreams.Test() + tmpFile := t.TempDir() + "/credential.json" + if err := os.WriteFile(tmpFile, []byte(`{"id":" cred-spaced ","plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + t.Fatalf("write credential file: %v", err) + } + + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/alice/credentials/cred-spaced", func(req *http.Request) (httpmock.Response, error) { + return httpmock.JSONResponse(`{"id":"cred-spaced","name":"cred-spaced"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg1"}, nil + }, Consumer: "alice", GatewayGroup: "gg1", File: tmpFile} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) } registry.Verify(t) } @@ -133,7 +295,7 @@ func TestCreateCredential_FileNormalizesLegacyID(t *testing.T) { func TestCreateCredential_FileRejectsInvalidName(t *testing.T) { ios, _, _, _ := iostreams.Test() tmpFile := t.TempDir() + "/credential.json" - if err := os.WriteFile(tmpFile, []byte(`{"name":123,"plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { + if err := os.WriteFile(tmpFile, []byte(`{"id":"ok","name":123,"plugins":{"key-auth":{"key":"k"}}}`), 0o644); err != nil { t.Fatalf("write credential file: %v", err) } @@ -150,6 +312,33 @@ func TestCreateCredential_FileRejectsInvalidName(t *testing.T) { } } +func TestCreateCredential_EscapesConsumerAndIDInURL(t *testing.T) { + ios, _, _, _ := iostreams.Test() + registry := &httpmock.Registry{} + // httpmock matches on `req.URL.Path` (the decoded form). So both encoded + // "%2F" and a literal "/" decode to the same path. To prove escaping + // actually happened, the responder checks `EscapedPath()` and the raw + // query — they must contain the percent-encoded forms. + registry.RegisterResponder(http.MethodPut, "/apisix/admin/consumers/a/b/credentials/c d", func(req *http.Request) (httpmock.Response, error) { + if got, want := req.URL.EscapedPath(), "/apisix/admin/consumers/a%2Fb/credentials/c%20d"; got != want { + return httpmock.Response{}, fmt.Errorf("escaped path mismatch: got %q want %q", got, want) + } + if got, want := req.URL.RawQuery, "gateway_group_id=gg%26x"; got != want { + return httpmock.Response{}, fmt.Errorf("raw query mismatch: got %q want %q", got, want) + } + return httpmock.JSONResponse(`{"id":"c d","name":"c d"}`), nil + }) + + opts := &Options{IO: ios, Client: func() (*http.Client, error) { return registry.GetClient(), nil }, Config: func() (config.Config, error) { + return &mockConfig{baseURL: "http://api.local", gatewayGroup: "gg&x"}, nil + }, Consumer: "a/b", GatewayGroup: "gg&x", ID: "c d", PluginsJSON: `{"key-auth":{"key":"k"}}`} + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + registry.Verify(t) +} + func TestCreateCredential_MissingGatewayGroup(t *testing.T) { ios, _, _, _ := iostreams.Test() opts := &Options{IO: ios, Client: func() (*http.Client, error) { return (&httpmock.Registry{}).GetClient(), nil }, Config: func() (config.Config, error) {