Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/cmd/route/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Options struct {
File string

Name string
Desc string
URI string
Paths []string
Methods []string
Expand All @@ -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)")
Expand Down Expand Up @@ -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,
Expand Down
39 changes: 39 additions & 0 deletions pkg/cmd/route/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 8 additions & 0 deletions pkg/cmd/route/update/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type Options struct {
ID string

Name string
Desc string
DescSet bool
URI string
Methods []string
Host string
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions pkg/cmd/route/update/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
55 changes: 55 additions & 0 deletions test/e2e/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package e2e

import (
"encoding/json"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -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)
}
52 changes: 40 additions & 12 deletions test/e2e/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines +137 to +144
if err != nil {
Comment on lines +140 to +145
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the default/unset case on the fallback path.

Lines 141-143 currently normalize both "" and "default" to "default", so resolveGatewayGroupID never reaches its wanted == "" fallback branch. That breaks the documented behavior for the unset/default case and can make the mutating E2E tests target a nonexistent or ingress-only group.

Suggested fix
  wanted := gatewayGroup
  if wanted == "default" || wanted == "" {
-     wanted = "default"
+     wanted = ""
  }
  ggID, err := resolveGatewayGroupID(wanted)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/setup_test.go` around lines 140 - 145, The current normalization
coerces both "" and "default" into "default", preventing resolveGatewayGroupID
from exercising its wanted == "" fallback; change the logic around the
gatewayGroup/wanted variable so an empty gatewayGroup remains an empty string
(do not set wanted = "default" when gatewayGroup == ""), i.e. remove or adjust
the branch that normalizes "" into "default" and pass the original gatewayGroup
value into resolveGatewayGroupID (refer to the local variable wanted and the
call to resolveGatewayGroupID).

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())
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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")
}
Loading