fix(credential): forward [id] positional as credential id via PUT

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/<u>/credentials/<id>` (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 <id> --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 |
 

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 <user> -g <gateway-group>`
+
+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/<user>/credentials/<id>` 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.

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,41 +136,69 @@ 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
 	}
 	if len(labels) > 0 {
 		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
 }