diff --git a/pkg/cmd/route/create/create.go b/pkg/cmd/route/create/create.go index 3796431..6f7944a 100644 --- a/pkg/cmd/route/create/create.go +++ b/pkg/cmd/route/create/create.go @@ -24,6 +24,7 @@ type Options struct { File string Name string + Desc string URI string Paths []string Methods []string @@ -47,6 +48,7 @@ func NewCmd(f *cmd.Factory) *cobra.Command { } c.Flags().StringVar(&opts.Name, "name", "", "Route name") + c.Flags().StringVar(&opts.Desc, "desc", "", "Route description") c.Flags().StringVarP(&opts.File, "file", "f", "", "Path to JSON/YAML file with resource definition") c.Flags().StringVar(&opts.URI, "uri", "", "Route URI (single path, APISIX compat)") c.Flags().StringSliceVar(&opts.Paths, "path", nil, "Route path (repeatable, API7 EE format)") @@ -136,6 +138,7 @@ func actionRun(opts *Options) error { bodyReq := api.Route{ Name: opts.Name, + Desc: opts.Desc, Paths: paths, Methods: opts.Methods, Host: opts.Host, diff --git a/pkg/cmd/route/create/create_test.go b/pkg/cmd/route/create/create_test.go index 2105218..3f5423e 100644 --- a/pkg/cmd/route/create/create_test.go +++ b/pkg/cmd/route/create/create_test.go @@ -238,3 +238,42 @@ func TestCreateRoute_FromYAMLFile(t *testing.T) { } registry.Verify(t) } + +// TestCreateRoute_DescFlag guards the regression where flag-based `route create` +// had no `--desc` flag despite the README documenting one. The description must +// reach the API request body. +func TestCreateRoute_DescFlag(t *testing.T) { + ios, _, out, _ := iostreams.Test() + registry := &httpmock.Registry{} + registry.RegisterResponder(http.MethodPost, "/apisix/admin/routes", func(r *http.Request) (httpmock.Response, error) { + body, err := io.ReadAll(r.Body) + if err != nil { + return httpmock.Response{}, err + } + if !strings.Contains(string(body), `"desc":"my description"`) { + t.Fatalf("expected --desc to land in request body, got: %s", string(body)) + } + return httpmock.JSONResponse(`{"id":"r-desc","name":"demo","desc":"my description","service_id":"svc1"}`), 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 + }, + GatewayGroup: "gg1", + URI: "/demo", + Name: "demo", + Desc: "my description", + ServiceID: "svc1", + } + + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + if !strings.Contains(out.String(), "my description") { + t.Fatalf("expected desc in output, got: %s", out.String()) + } + registry.Verify(t) +} diff --git a/pkg/cmd/route/update/update.go b/pkg/cmd/route/update/update.go index 307b782..2079ffb 100644 --- a/pkg/cmd/route/update/update.go +++ b/pkg/cmd/route/update/update.go @@ -25,6 +25,8 @@ type Options struct { ID string Name string + Desc string + DescSet bool URI string Methods []string Host string @@ -48,11 +50,13 @@ func NewCmd(f *cmd.Factory) *cobra.Command { opts.GatewayGroup, _ = c.Flags().GetString("gateway-group") opts.StatusSet = c.Flags().Changed("status") opts.PrioritySet = c.Flags().Changed("priority") + opts.DescSet = c.Flags().Changed("desc") return actionRun(opts) }, } c.Flags().StringVar(&opts.Name, "name", "", "Route name") + c.Flags().StringVar(&opts.Desc, "desc", "", "Route description (pass empty string to clear)") c.Flags().StringVar(&opts.URI, "uri", "", "Route URI") c.Flags().StringSliceVar(&opts.Methods, "methods", nil, "Allowed HTTP methods") c.Flags().StringVar(&opts.Host, "host", "", "Route host") @@ -123,6 +127,10 @@ func actionRun(opts *Options) error { if opts.Name != "" { bodyReq.Name = opts.Name } + // DescSet lets the user explicitly clear the description with --desc "". + if opts.DescSet { + bodyReq.Desc = opts.Desc + } if opts.URI != "" { bodyReq.URI = "" bodyReq.URIs = nil diff --git a/pkg/cmd/route/update/update_test.go b/pkg/cmd/route/update/update_test.go index e0d0f49..65f570e 100644 --- a/pkg/cmd/route/update/update_test.go +++ b/pkg/cmd/route/update/update_test.go @@ -130,3 +130,77 @@ func TestUpdateRoute_FromFile(t *testing.T) { } registry.Verify(t) } + +// TestUpdateRoute_DescFlag guards the regression where `route update` had no +// `--desc` flag despite the README documenting one. The new description must +// reach the PUT body. +func TestUpdateRoute_DescFlag(t *testing.T) { + ios, _, out, _ := iostreams.Test() + registry := &httpmock.Registry{} + registry.Register(http.MethodGet, "/apisix/admin/routes/r1", httpmock.JSONResponse(`{"id":"r1","name":"old-name","desc":"old desc","paths":["/demo"],"service_id":"svc1","status":1}`)) + registry.RegisterResponder(http.MethodPut, "/apisix/admin/routes/r1", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]interface{} + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) + } + if payload["desc"] != "updated desc" { + return httpmock.Response{}, fmt.Errorf("expected updated desc in payload, got desc=%v", payload["desc"]) + } + return httpmock.JSONResponse(`{"id":"r1","name":"old-name","desc":"updated desc","paths":["/demo"],"service_id":"svc1","status":1}`), 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 + }, + ID: "r1", + GatewayGroup: "gg1", + Desc: "updated desc", + DescSet: true, + } + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + if !strings.Contains(out.String(), "updated desc") { + t.Fatalf("expected updated desc in output: %s", out.String()) + } + registry.Verify(t) +} + +// TestUpdateRoute_DescFlagCanClear verifies that passing --desc "" explicitly +// clears the existing description (rather than being treated as "unset"). +func TestUpdateRoute_DescFlagCanClear(t *testing.T) { + ios, _, _, _ := iostreams.Test() + registry := &httpmock.Registry{} + registry.Register(http.MethodGet, "/apisix/admin/routes/r1", httpmock.JSONResponse(`{"id":"r1","name":"old-name","desc":"old desc","paths":["/demo"],"service_id":"svc1","status":1}`)) + registry.RegisterResponder(http.MethodPut, "/apisix/admin/routes/r1", func(req *http.Request) (httpmock.Response, error) { + var payload map[string]interface{} + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + return httpmock.Response{}, fmt.Errorf("decode request body: %w", err) + } + // With api.Route's `omitempty` desc tag, clearing should drop the field + // from the payload entirely; an empty string would also be acceptable. + if d, present := payload["desc"]; present && d != "" { + return httpmock.Response{}, fmt.Errorf("expected desc to be cleared, got desc=%v", d) + } + return httpmock.JSONResponse(`{"id":"r1","name":"old-name","paths":["/demo"],"service_id":"svc1","status":1}`), 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 + }, + ID: "r1", + GatewayGroup: "gg1", + Desc: "", + DescSet: true, + } + if err := actionRun(opts); err != nil { + t.Fatalf("actionRun failed: %v", err) + } + registry.Verify(t) +} diff --git a/test/e2e/route_test.go b/test/e2e/route_test.go index 851da54..a87f771 100644 --- a/test/e2e/route_test.go +++ b/test/e2e/route_test.go @@ -3,6 +3,7 @@ package e2e import ( + "encoding/json" "fmt" "io" "net/http" @@ -383,3 +384,57 @@ func TestRoute_TrafficForwarding(t *testing.T) { } assert.Equal(t, 200, status) } + +// TestRoute_DescFlagCRUD guards the regression where `route create` and +// `route update` exposed no `--desc` flag despite the README documenting one. +// Covers create-with-desc, update-with-new-desc, and update-with-empty-desc +// (which clears the field). +func TestRoute_DescFlagCRUD(t *testing.T) { + env := setupEnv(t) + svcID := "e2e-route-desc-svc" + routeID := "e2e-route-desc" + t.Cleanup(func() { deleteServiceViaAdmin(t, svcID) }) + createTestServiceViaCLI(t, env, svcID) + + stdout, stderr, err := runA7WithEnv(env, "route", "create", + "--name", routeID, "--path", "/"+routeID, "--service-id", svcID, + "--desc", "initial desc", "-g", gatewayGroup, "-o", "json") + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + var created map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(stdout), &created)) + assert.Equal(t, "initial desc", created["desc"], "create --desc should land in response") + rtID, ok := created["id"].(string) + require.True(t, ok && rtID != "", "create response should contain an id: %v", created) + t.Cleanup(func() { deleteRouteViaAdmin(t, rtID) }) + + stdout, stderr, err = runA7WithEnv(env, "route", "update", rtID, + "--desc", "updated desc", "-g", gatewayGroup, "-o", "json") + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + var updated map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(stdout), &updated)) + assert.Equal(t, "updated desc", updated["desc"], "update --desc should land in response") + + stdout, stderr, err = runA7WithEnv(env, "route", "update", rtID, + "--desc", "", "-g", gatewayGroup, "-o", "json") + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + var cleared map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(stdout), &cleared)) + assertDescCleared(t, cleared, "update --desc \"\" should clear the description in response") + + // Verify the clear persisted server-side, not just in the update response. + var persisted map[string]interface{} + runA7JSON(t, env, &persisted, "route", "get", rtID, "-g", gatewayGroup, "-o", "json") + assertDescCleared(t, persisted, "cleared desc should not be persisted") +} + +// assertDescCleared treats absent, nil, and "" all as a successfully-cleared +// desc — the API may serialize a cleared field as `null`, omit it entirely +// (json `omitempty`), or echo an empty string. +func assertDescCleared(t testTB, obj map[string]interface{}, msg string) { + t.Helper() + d, present := obj["desc"] + if !present || d == nil { + return + } + assert.Equal(t, "", d, msg) +} diff --git a/test/e2e/setup_test.go b/test/e2e/setup_test.go index 35842e1..9f9c7ba 100644 --- a/test/e2e/setup_test.go +++ b/test/e2e/setup_test.go @@ -134,17 +134,20 @@ func TestMain(m *testing.M) { os.Exit(1) } - // Resolve the actual gateway group UUID. API7 EE uses UUID-style IDs, - // not human-readable names like "default". - if gatewayGroup == "default" || gatewayGroup == "" { - ggID, err := resolveFirstGatewayGroupID() - if err != nil { - fmt.Fprintf(os.Stderr, "failed to resolve gateway group ID: %v\n", err) - os.Exit(1) - } - gatewayGroup = ggID - fmt.Fprintf(os.Stderr, "Resolved gateway group ID: %s\n", gatewayGroup) + // API7 EE uses UUID-style ids for runtime API calls but env vars carry + // names; resolve name -> id. An explicit name is honored literally; only + // the empty/"default" case falls back to "first writable group". + wanted := gatewayGroup + if wanted == "default" || wanted == "" { + wanted = "default" + } + ggID, err := resolveGatewayGroupID(wanted) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to resolve gateway group: %v\n", err) + os.Exit(1) } + gatewayGroup = ggID + fmt.Fprintf(os.Stderr, "Resolved gateway group %q -> %s\n", wanted, gatewayGroup) os.Exit(m.Run()) } @@ -403,7 +406,12 @@ func createTestRouteWithServiceViaCLI(t testTB, env []string, routeID, serviceID require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) } -func resolveFirstGatewayGroupID() (string, error) { +// resolveGatewayGroupID looks up a gateway group by exact name; if wanted is +// empty it falls back to the first non-ingress group. Ingress-controller +// gateway groups (Type == "api7_ingress_controller") reject POST/PUT/PATCH/ +// DELETE for any auth mode other than admin_key, so picking one as the test +// target silently breaks every mutating test in CI. +func resolveGatewayGroupID(wanted string) (string, error) { req, err := http.NewRequest(http.MethodGet, adminURL+"/api/gateway_groups", nil) if err != nil { return "", err @@ -426,6 +434,7 @@ func resolveFirstGatewayGroupID() (string, error) { List []struct { ID string `json:"id"` Name string `json:"name"` + Type string `json:"type"` } `json:"list"` } if err := json.Unmarshal(body, &result); err != nil { @@ -434,5 +443,24 @@ func resolveFirstGatewayGroupID() (string, error) { if len(result.List) == 0 { return "", fmt.Errorf("no gateway groups found") } - return result.List[0].ID, nil + + if wanted != "" { + for _, g := range result.List { + if g.Name == wanted { + return g.ID, nil + } + } + names := make([]string, 0, len(result.List)) + for _, g := range result.List { + names = append(names, g.Name) + } + return "", fmt.Errorf("gateway group %q not found; available: %v", wanted, names) + } + + for _, g := range result.List { + if g.Type != "api7_ingress_controller" { + return g.ID, nil + } + } + return "", fmt.Errorf("no non-ingress gateway group found; ingress-controller groups reject writes without admin_key auth") }