Skip to content

Commit ccabf7a

Browse files
committed
fix: round 7 — plugin-metadata raw map format, config-sync service_id, validate paths, test timeout
- Plugin metadata: send/receive raw map instead of wrapped struct (API7 EE format) - Config sync tests: create service first, include service_id in routes - Validator: accept 'paths' field (API7 EE) in addition to uri/uris - Add createTestRouteWithServiceViaCLI helper for config sync tests - Increase e2e test timeout from 15m to 25m to prevent panic
1 parent 81241dc commit ccabf7a

7 files changed

Lines changed: 70 additions & 48 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,4 @@ docker-down:
4444
docker compose -f test/e2e/docker-compose.yml down -v
4545

4646
test-e2e:
47-
go test ./test/e2e/... -count=1 -v -tags=e2e -timeout 15m
47+
go test ./test/e2e/... -count=1 -v -tags=e2e -timeout 25m

pkg/cmd/config/validate/validate.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ func hasRouteURI(r api.Route) bool {
208208
return true
209209
}
210210
}
211+
// API7 EE uses "paths" instead of "uri"/"uris".
212+
for _, p := range r.Paths {
213+
if strings.TrimSpace(p) != "" {
214+
return true
215+
}
216+
}
211217
return false
212218
}
213219

pkg/cmd/plugin-metadata/create/create.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,30 +103,23 @@ func actionRun(opts *Options) error {
103103
return err
104104
}
105105

106+
// API7 EE expects raw metadata map as request body (e.g., {"log_format": {...}}).
106107
metadata := map[string]interface{}{}
107108
if opts.MetadataJSON != "" {
108109
if err := json.Unmarshal([]byte(opts.MetadataJSON), &metadata); err != nil {
109110
return fmt.Errorf("invalid --metadata-json: %w", err)
110111
}
111112
}
112113

113-
bodyReq := api.PluginMetadata{Metadata: metadata}
114-
115114
client := api.NewClient(httpClient, cfg.BaseURL())
116-
body, err := client.Put("/apisix/admin/plugin_metadata/"+opts.PluginName+"?gateway_group_id="+ggID, bodyReq)
115+
body, err := client.Put("/apisix/admin/plugin_metadata/"+opts.PluginName+"?gateway_group_id="+ggID, metadata)
117116
if err != nil {
118117
return fmt.Errorf("%s", cmdutil.FormatAPIError(err))
119118
}
120119

121-
var created api.PluginMetadata
122-
if err := json.Unmarshal(body, &created); err != nil {
123-
return fmt.Errorf("failed to decode response: %w", err)
124-
}
125-
126120
format := opts.Output
127121
if format == "" {
128122
format = "json"
129123
}
130-
exporter := cmdutil.NewExporter(format, opts.IO.Out)
131-
return exporter.Write(created)
124+
return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body))
132125
}

pkg/cmd/plugin-metadata/get/get.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,22 @@ func actionRun(opts *Options) error {
6666
return fmt.Errorf("%s", cmdutil.FormatAPIError(err))
6767
}
6868

69-
var item api.PluginMetadata
70-
if err := json.Unmarshal(body, &item); err != nil {
69+
// API7 EE returns plugin metadata as a raw map (e.g., {"log_format": {...}}).
70+
// After unwrapValueEnvelope, `body` is the raw metadata map directly.
71+
var metadata map[string]interface{}
72+
if err := json.Unmarshal(body, &metadata); err != nil {
7173
return fmt.Errorf("failed to decode response: %w", err)
7274
}
7375

7476
if opts.Output != "" {
7577
exporter := cmdutil.NewExporter(opts.Output, opts.IO.Out)
76-
return exporter.Write(item)
78+
return exporter.Write(json.RawMessage(body))
7779
}
7880

79-
metaJSON, _ := json.Marshal(item.Metadata)
81+
metaJSON, _ := json.MarshalIndent(metadata, "", " ")
8082
tp := tableprinter.New(opts.IO.Out)
8183
tp.SetHeaders("FIELD", "VALUE")
82-
tp.AddRow("id", item.ID)
84+
tp.AddRow("plugin", opts.PluginName)
8385
tp.AddRow("metadata", string(metaJSON))
8486
return tp.Render()
8587
}

pkg/cmd/plugin-metadata/update/update.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,23 +88,15 @@ func actionRun(opts *Options) error {
8888
}
8989
}
9090

91-
bodyReq := api.PluginMetadata{Metadata: metadata}
92-
9391
client := api.NewClient(httpClient, cfg.BaseURL())
94-
body, err := client.Put("/apisix/admin/plugin_metadata/"+opts.PluginName+"?gateway_group_id="+ggID, bodyReq)
92+
body, err := client.Put("/apisix/admin/plugin_metadata/"+opts.PluginName+"?gateway_group_id="+ggID, metadata)
9593
if err != nil {
9694
return fmt.Errorf("%s", cmdutil.FormatAPIError(err))
9795
}
9896

99-
var updated api.PluginMetadata
100-
if err := json.Unmarshal(body, &updated); err != nil {
101-
return fmt.Errorf("failed to decode response: %w", err)
102-
}
103-
10497
format := opts.Output
10598
if format == "" {
10699
format = "json"
107100
}
108-
exporter := cmdutil.NewExporter(format, opts.IO.Out)
109-
return exporter.Write(updated)
101+
return cmdutil.NewExporter(format, opts.IO.Out).Write(json.RawMessage(body))
110102
}

test/e2e/config_sync_test.go

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@ import (
1515
func TestConfigSync_DryRun(t *testing.T) {
1616
env := setupEnv(t)
1717

18-
// Create a config with a new route to sync.
19-
syncYAML := `version: "1"
18+
svcID := "e2e-sync-dryrun-svc"
19+
createTestServiceViaCLI(t, env, svcID)
20+
t.Cleanup(func() { deleteServiceViaAdmin(t, svcID) })
21+
22+
syncYAML := fmt.Sprintf(`version: "1"
2023
routes:
2124
- id: e2e-sync-dryrun-route
2225
name: e2e-sync-dryrun-route
26+
service_id: %s
2327
paths:
2428
- /sync-dryrun
2529
upstream:
@@ -28,30 +32,32 @@ routes:
2832
- host: "127.0.0.1"
2933
port: 8080
3034
weight: 1
31-
`
35+
`, svcID)
3236
tmpFile := filepath.Join(t.TempDir(), "sync-dryrun.yaml")
3337
require.NoError(t, os.WriteFile(tmpFile, []byte(syncYAML), 0644))
3438

35-
// --dry-run should show what would change without applying.
3639
stdout, stderr, err := runA7WithEnv(env, "config", "sync", "-f", tmpFile, "--dry-run", "-g", gatewayGroup)
3740
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
3841

39-
// The route should NOT exist after dry-run.
4042
_, _, getErr := runA7WithEnv(env, "route", "get", "e2e-sync-dryrun-route", "-g", gatewayGroup)
4143
assert.Error(t, getErr, "dry-run should not create resources")
4244
}
4345

4446
func TestConfigSync_CreateAndCleanup(t *testing.T) {
4547
env := setupEnv(t)
48+
svcID := "e2e-sync-create-svc"
4649
routeID := "e2e-sync-create-route"
47-
t.Cleanup(func() { deleteRouteViaAdmin(t, routeID) })
50+
createTestServiceViaCLI(t, env, svcID)
51+
t.Cleanup(func() {
52+
deleteRouteViaAdmin(t, routeID)
53+
deleteServiceViaAdmin(t, svcID)
54+
})
4855

49-
// Create a minimal config with just the new route
50-
// and use --delete=false to avoid removing existing resources.
5156
minimalYAML := fmt.Sprintf(`version: "1"
5257
routes:
5358
- id: %s
5459
name: %s
60+
service_id: %s
5561
paths:
5662
- /sync-create-test
5763
upstream:
@@ -60,31 +66,32 @@ routes:
6066
- host: "127.0.0.1"
6167
port: 8080
6268
weight: 1
63-
`, routeID, routeID)
69+
`, routeID, routeID, svcID)
6470

6571
syncFile := filepath.Join(t.TempDir(), "sync-create.yaml")
6672
require.NoError(t, os.WriteFile(syncFile, []byte(minimalYAML), 0644))
6773

68-
// Sync with --delete=false so we only create, not remove existing resources.
6974
stdout, stderr, err := runA7WithEnv(env, "config", "sync", "-f", syncFile, "--delete=false", "-g", gatewayGroup)
7075
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
7176
assert.Contains(t, stdout, "Sync completed")
7277

73-
// Verify route was created.
7478
stdout, stderr, err = runA7WithEnv(env, "route", "get", routeID, "-g", gatewayGroup)
7579
require.NoError(t, err, stderr)
7680
assert.Contains(t, stdout, routeID)
7781
}
7882

7983
func TestConfigSync_DeleteFalse(t *testing.T) {
8084
env := setupEnv(t)
85+
svcID := "e2e-sync-nodelete-svc"
8186
routeID := "e2e-sync-nodelete-route"
82-
t.Cleanup(func() { deleteRouteViaAdmin(t, routeID) })
87+
createTestServiceViaCLI(t, env, svcID)
88+
t.Cleanup(func() {
89+
deleteRouteViaAdmin(t, routeID)
90+
deleteServiceViaAdmin(t, svcID)
91+
})
8392

84-
// First create a route via CLI.
85-
createTestRouteViaCLI(t, env, routeID)
93+
createTestRouteWithServiceViaCLI(t, env, routeID, svcID)
8694

87-
// Now sync an empty config with --delete=false.
8895
emptyYAML := `version: "1"
8996
`
9097
syncFile := filepath.Join(t.TempDir(), "sync-empty.yaml")
@@ -93,21 +100,23 @@ func TestConfigSync_DeleteFalse(t *testing.T) {
93100
stdout, stderr, err := runA7WithEnv(env, "config", "sync", "-f", syncFile, "--delete=false", "-g", gatewayGroup)
94101
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
95102

96-
// Route should STILL exist because --delete=false.
97103
stdout, stderr, err = runA7WithEnv(env, "route", "get", routeID, "-g", gatewayGroup)
98104
require.NoError(t, err, stderr)
99105
assert.Contains(t, stdout, routeID)
100106
}
101107

102108
func TestConfigSync_FullRoundtrip(t *testing.T) {
103109
env := setupEnv(t)
110+
svcID := "e2e-sync-roundtrip-svc"
104111
routeID := "e2e-sync-roundtrip-route"
105-
t.Cleanup(func() { deleteRouteViaAdmin(t, routeID) })
112+
createTestServiceViaCLI(t, env, svcID)
113+
t.Cleanup(func() {
114+
deleteRouteViaAdmin(t, routeID)
115+
deleteServiceViaAdmin(t, svcID)
116+
})
106117

107-
// Step 1: Create route via CLI.
108-
createTestRouteViaCLI(t, env, routeID)
118+
createTestRouteWithServiceViaCLI(t, env, routeID, svcID)
109119

110-
// Step 2: Dump current config.
111120
dumpFile := filepath.Join(t.TempDir(), "roundtrip-dump.yaml")
112121
_, stderr, err := runA7WithEnv(env, "config", "dump", "-g", gatewayGroup, "-f", dumpFile)
113122
require.NoError(t, err, stderr)
@@ -116,11 +125,9 @@ func TestConfigSync_FullRoundtrip(t *testing.T) {
116125
require.NoError(t, err)
117126
assert.Contains(t, string(data), routeID)
118127

119-
// Step 3: Diff should show no differences.
120128
stdout, stderr, err := runA7WithEnv(env, "config", "diff", "-f", dumpFile, "-g", gatewayGroup)
121129
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
122130

123-
// Step 4: Sync same config — should be a no-op.
124131
stdout, stderr, err = runA7WithEnv(env, "config", "sync", "-f", dumpFile, "-g", gatewayGroup)
125132
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
126133
assert.Contains(t, stdout, "Sync completed")

test/e2e/setup_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,28 @@ func upstreamNodePort() int {
299299
return 80
300300
}
301301

302+
// createTestRouteWithServiceViaCLI creates a route that belongs to an existing service.
303+
// The service must already exist. This is needed because API7 EE requires service_id for routes.
304+
func createTestRouteWithServiceViaCLI(t *testing.T, env []string, routeID, serviceID string) {
305+
t.Helper()
306+
routeJSON := fmt.Sprintf(`{
307+
"id": %q,
308+
"name": "e2e-route-%s",
309+
"service_id": %q,
310+
"paths": ["/test-%s"],
311+
"upstream": {
312+
"type": "roundrobin",
313+
"nodes": [{"host": %q, "port": %d, "weight": 1}]
314+
}
315+
}`, routeID, routeID, serviceID, routeID, upstreamNodeHost(), upstreamNodePort())
316+
317+
tmpFile := filepath.Join(t.TempDir(), "route.json")
318+
require.NoError(t, os.WriteFile(tmpFile, []byte(routeJSON), 0644))
319+
320+
stdout, stderr, err := runA7WithEnv(env, "route", "create", "-f", tmpFile, "-g", gatewayGroup)
321+
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
322+
}
323+
302324
func resolveFirstGatewayGroupID() (string, error) {
303325
req, err := http.NewRequest(http.MethodGet, adminURL+"/api/gateway_groups", nil)
304326
if err != nil {

0 commit comments

Comments
 (0)