Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnforces API7 EE service-scoped queries (routes/stream-routes), implements read-modify-write updates for several commands, renames secret provider token flag to --provider-token, maps credential positional ID to API name, adds APISIX-compatibility 404 formatting, improves SSL path detection, and updates docs/tests. ChangesAPI7 EE Resource Scoping and Update Semantics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the a7 CLI to better match observed API7 EE Admin API behavior and to avoid unsafe full-replacement updates that can accidentally drop fields, while improving error messages and docs around API7/APISIX compatibility boundaries.
Changes:
- Persist
A7_TOKENinto newly created contexts when--tokenis omitted, without inheriting a previous context’s token. - Require
--service-idfor API7 EE route/stream-route list & export commands, with matching e2e + unit test updates. - Make several “flag-mode” updates safer by GET’ing the current resource and merging changes before PUT; improve SSL cert/key path handling and clarify plugin-config 404 compatibility errors.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/stream_route_test.go | Updates stream-route list e2e tests to pass --service-id by creating a service first. |
| pkg/cmdutil/errors.go | Adds a compatibility-oriented formatter for 404s on APISIX-only resources (e.g., plugin-config). |
| pkg/cmd/stream-route/list/list.go | Adds --service-id flag and enforces it for API7 EE stream-route listing. |
| pkg/cmd/stream-route/list/list_test.go | Updates tests to include service-id and adds a missing-service-id test case. |
| pkg/cmd/stream-route/export/export.go | Adds --service-id flag and requires it for export; threads it into pagination queries. |
| pkg/cmd/stream-route/export/export_test.go | Updates export tests to include service-id and adds a missing-service-id test case. |
| pkg/cmd/ssl/update/update.go | Makes SSL flag-based updates merge with current SSL and improves cert/key path detection; supports explicit status: 0. |
| pkg/cmd/ssl/update/update_test.go | Adds unit tests for relative path cert/key reads, merge-preserving behavior, and explicit status=0 payloads. |
| pkg/cmd/ssl/create/create.go | Improves cert/key path detection to support bare relative paths while keeping inline PEM literals intact. |
| pkg/cmd/ssl/create/create_test.go | Adds unit tests covering relative path reads and inline PEM behavior. |
| pkg/cmd/service/update/update.go | Makes service flag-based updates merge with current service to prevent dropping omitted fields. |
| pkg/cmd/service/update/update_test.go | Extends tests to cover GET+PUT flow and “preserve name when omitted” behavior. |
| pkg/cmd/secret/update/update.go | Renames provider token flag and merges updates with current secret provider before PUT. |
| pkg/cmd/secret/update/update_test.go | Updates tests for GET+PUT flow and validates the new --provider-token flag name. |
| pkg/cmd/secret/create/create.go | Renames provider token flag and changes create semantics to idempotent PUT to the resource path. |
| pkg/cmd/secret/create/create_test.go | Updates tests for PUT-based create and validates the new --provider-token flag name. |
| pkg/cmd/route/update/update.go | Makes route flag-based updates merge with current route; maps --uri to paths; supports explicit 0 values. |
| pkg/cmd/route/update/update_test.go | Extends tests for GET+PUT merge behavior, --uri→paths, and explicit status=0 payload. |
| pkg/cmd/route/list/list.go | Enforces --service-id for API7 EE route listing and always includes it in the query. |
| pkg/cmd/route/list/list_test.go | Updates list tests for service-id requirement and adds missing-service-id coverage. |
| pkg/cmd/route/export/export.go | Adds --service-id and requires it for export; includes it in paginated queries. |
| pkg/cmd/route/export/export_test.go | Updates export tests for service-id requirement and adds missing-service-id coverage. |
| pkg/cmd/plugin-config/update/update.go | Improves file-mode validation and wraps 404s with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/update/update_test.go | Adds coverage ensuring file mode does not require --plugins-json. |
| pkg/cmd/plugin-config/list/list.go | Wraps 404 errors with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/list/list_test.go | Adds coverage for 404 compatibility error messaging. |
| pkg/cmd/plugin-config/get/get.go | Wraps 404 errors with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/export/export.go | Wraps 404 errors with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/delete/delete.go | Wraps 404 errors with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/create/create.go | Wraps 404 errors with APISIX-compatibility messaging. |
| pkg/cmd/plugin-config/create/create_test.go | Adds coverage for 404 compatibility error messaging. |
| pkg/cmd/credential/create/create.go | Maps positional credential ID to name and uses PUT-by-name in file mode when name is present. |
| pkg/cmd/credential/create/create_test.go | Adds coverage asserting positional ID maps to name. |
| pkg/cmd/context/create/create.go | Persists A7_TOKEN into the created context when --token is omitted. |
| pkg/cmd/context/create/create_test.go | Adds coverage for env-token persistence and non-inheritance from the current context. |
| docs/user-guide/stream-route.md | Documents API7 EE requirement for --service-id on list/export and updates examples. |
| docs/user-guide/secret.md | Documents --provider-token and clarifies global --token meaning; adds flag-mode example. |
| docs/user-guide/route.md | Documents API7 EE requirement for --service-id on export and updates example. |
| docs/user-guide/configuration.md | Documents A7_TOKEN persistence behavior for context create. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
82
to
96
| @@ -91,8 +91,8 @@ func actionRun(opts *Options) error { | |||
| path := "/apisix/admin/consumers/" + opts.Consumer + "/credentials?gateway_group_id=" + ggID | |||
| client := api.NewClient(httpClient, cfg.BaseURL()) | |||
| var body []byte | |||
| if id, ok := payload["id"]; ok { | |||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, id, ggID), payload) | |||
| if name, ok := payload["name"]; ok { | |||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, name, ggID), payload) | |||
| } else { | |||
| bodyReq.Name = opts.Name | ||
| } | ||
| if opts.URI != "" { | ||
| bodyReq.URI = "" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/user-guide/route.md (1)
11-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocumentation understates the
--service-idrequirement forroute list.Lines 11–13 say
--service-id"may" be required for listing routes, butpkg/cmd/route/list/list.gounconditionally returns an error when it is absent (--service-id is required by API7 EE). The wording should be updated to match the hard requirement, as the current docs will mislead users who try omitting the flag.📝 Proposed fix
-Lists routes in the specified gateway group. API7 EE may require -`--service-id` to list routes; omit it only if your environment supports -unscoped route listing. +Lists routes in the specified gateway group. `--service-id` is **required** +by API7 EE; the command will fail if it is omitted.🤖 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 `@docs/user-guide/route.md` around lines 11 - 13, The docs currently say `--service-id` "may" be required, but pkg/cmd/route/list/list.go unconditionally errors if the flag is missing; update the route list documentation to state that `--service-id` is required (e.g., "The --service-id flag is required for route list in API7 EE; omit only if your environment explicitly supports unscoped listing") so the text matches the behavior implemented in the list command (refer to the check/error in list.go that returns "`--service-id is required by API7 EE`").pkg/cmd/plugin-config/update/update_test.go (1)
97-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate error message to mention both
--fileand--plugins-jsonalternatives.The error message in
update.go(line 90) currently returns"--plugins-json is required", which is what the test correctly expects (line 108). However, since--fileis now a valid alternative path (line 73), the error message should mention both options when neither is provided. Update the error message to"--plugins-json or --file is required"and update the test assertion on line 108 accordingly.🤖 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 `@pkg/cmd/plugin-config/update/update_test.go` around lines 97 - 111, Update the validation error text used when neither plugins JSON nor a file is provided: in update.go locate the validation in actionRun (or where Options is validated) that currently returns "--plugins-json is required" and change it to "--plugins-json or --file is required"; then update the test assertion in pkg/cmd/plugin-config/update/update_test.go (TestUpdatePluginConfig_ValidationError) to expect the new message ("--plugins-json or --file is required") instead of the old one. Ensure the check still uses strings.Contains on err.Error() so the test remains robust.pkg/cmd/secret/update/update.go (1)
124-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
api.Secret.Tokenis serialized unredacted in JSON output.Per coding guidelines, tokens must not be serialized without redaction, and secret-bearing structs must implement
MarshalJSON()to redact sensitive fields. Theapi.Secretstruct lacks a customMarshalJSON()method, so the Token field is included in full when the exporter writes JSON output:
- Line 139:
exporter.Write(updated)serializes the fullapi.Secretto stdout- Line 89:
exporter.Write(json.RawMessage(body))writes the raw server response (includes Token)Add a
MarshalJSON()method toapi.Secretthat redacts or omits the Token field.🤖 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 `@pkg/cmd/secret/update/update.go` around lines 124 - 139, The api.Secret struct is being serialized with its Token unredacted (seen when exporter.Write(updated) and exporter.Write(json.RawMessage(body)) are called); fix by adding a MarshalJSON method on api.Secret that returns JSON with the Token field redacted or omitted (implement api.Secret.MarshalJSON() to copy the struct into a safe representation or a helper struct without Token or with Token replaced by "<redacted>"), ensure the method is used by the exporter (standard encoding/json will pick up MarshalJSON) and keep other fields unchanged.
🧹 Nitpick comments (10)
pkg/cmd/route/list/list.go (1)
63-71: 💤 Low valueInput validation fires after client construction.
opts.ServiceIDis checked on line 69 but the HTTP client is already built on lines 63–66. For a cheap construct this is harmless, but the guard is more readable and slightly more efficient when placed beforeopts.Client():♻️ Proposed fix
+ if opts.ServiceID == "" { + return fmt.Errorf("--service-id is required by API7 EE") + } + httpClient, err := opts.Client() if err != nil { return err } client := api.NewClient(httpClient, cfg.BaseURL()) - if opts.ServiceID == "" { - return fmt.Errorf("--service-id is required by API7 EE") - } query := map[string]string{"gateway_group_id": ggID} query["service_id"] = opts.ServiceID🤖 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 `@pkg/cmd/route/list/list.go` around lines 63 - 71, Move the required-service validation before constructing the HTTP client: check opts.ServiceID and return the fmt.Errorf("--service-id is required by API7 EE") immediately if empty, then call opts.Client() and api.NewClient(...) only after that check; update the block around opts.Client(), api.NewClient, and the ServiceID guard so validation occurs first (referencing opts.Client(), api.NewClient, and opts.ServiceID).test/e2e/stream_route_test.go (1)
35-46: 💤 Low value
assert.NotEmpty(t, stdout)will pass with just table headers.
TestStreamRoute_List(andTestStreamRoute_ListJSON) create the scoping service but no stream routes, so the list command may only emit a header row. The assertion passes regardless of whether the command actually returned data.This is fine for a smoke test, but consider asserting the command exits cleanly as the primary signal and skip
NotEmptyor assert something structural (e.g.,[]in JSON output). No change is strictly required.🤖 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/stream_route_test.go` around lines 35 - 46, TestStreamRoute_List (and its companion TestStreamRoute_ListJSON) currently uses assert.NotEmpty(t, stdout) which can succeed with only table headers; instead, assert the command succeeded by checking err==nil (already handled) and either remove the NotEmpty assertion or replace it with a structural check: for the plain text test verify stdout does not equal just the header string returned by runA7WithEnv, and for the JSON test assert stdout == "[]" (or parse JSON and assert len==0). Update assertions in TestStreamRoute_List and TestStreamRoute_ListJSON referencing runA7WithEnv, stdout, stderr, and err accordingly.pkg/cmd/plugin-config/list/list.go (1)
73-73: ⚡ Quick win
fmt.Errorf("%s", ...)discards the error chain.Using
%sinstead of%wdrops the original error context, breakingerrors.Is/errors.Asfor callers. The same pattern appears inpkg/cmd/plugin-config/get/get.goand likely in all otherplugin-configsubcommands changed in this PR. IfFormatAPISIXCompatibilityResourceErrorreturns anerror, prefer:♻️ Proposed fix
- return fmt.Errorf("%s", cmdutil.FormatAPISIXCompatibilityResourceError(err, "plugin-config")) + return fmt.Errorf("%w", cmdutil.FormatAPISIXCompatibilityResourceError(err, "plugin-config"))If it returns a
string, the wrapping is fine and the same fix applies to the analogous line inget.go.🤖 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 `@pkg/cmd/plugin-config/list/list.go` at line 73, The current return in list.go wraps FormatAPISIXCompatibilityResourceError with fmt.Errorf("%s", ...) which discards the original error chain; change the return to preserve the wrapped error (use fmt.Errorf(..., %w, ...) or simply return the error directly) so callers can use errors.Is/errors.As; locate the call to FormatAPISIXCompatibilityResourceError in pkg/cmd/plugin-config/list/list.go (and the analogous call in pkg/cmd/plugin-config/get/get.go) and if FormatAPISIXCompatibilityResourceError returns an error, return or wrap it with %w, and if it returns a string, keep wrapping but ensure any underlying error is propagated where applicable.pkg/cmd/ssl/create/create_test.go (1)
8-34: ⚡ Quick winEliminate
os.Chdirin test; use absolute path instead.
os.Chdirmutates the working directory for the entire process. While this test doesn't uset.Parallel(), the call remains unsafe if the test binary later enables parallelization or if other test packages also manipulate the working directory. Use an absolute path directly:♻️ Proposed alternative
func TestMaybeReadFileReadsBareRelativePath(t *testing.T) { - cwd, err := os.Getwd() - if err != nil { - t.Fatalf("get cwd: %v", err) - } tmp := t.TempDir() + certPath := filepath.Join(tmp, "cert.pem") - if err := os.Chdir(tmp); err != nil { - t.Fatalf("chdir temp dir: %v", err) - } - t.Cleanup(func() { - if err := os.Chdir(cwd); err != nil { - t.Fatalf("restore cwd: %v", err) - } - }) - - if err := os.WriteFile("cert.pem", []byte("file-cert"), 0o644); err != nil { + if err := os.WriteFile(certPath, []byte("file-cert"), 0o644); err != nil { t.Fatalf("write cert: %v", err) } - - got, err := maybeReadFile("cert.pem") + got, err := maybeReadFile(certPath)Note:
looksLikePathalready handles absolute paths correctly (line 178 increate.goreturnstruefor strings starting with/), so this refactor is safe. The test will exercise the absolute-path branch instead of theos.Statfallback, but both correctly identify a file path.🤖 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 `@pkg/cmd/ssl/create/create_test.go` around lines 8 - 34, The test TestMaybeReadFileReadsBareRelativePath should avoid mutating process cwd; change it to create the cert file in the temp dir and call maybeReadFile with its absolute path instead of calling os.Chdir/restoring cwd. Specifically, drop the os.Chdir/cwd/t.Cleanup block, write the file to filepath.Join(tmp, "cert.pem"), and pass that absolute path into maybeReadFile so the test exercises the path-handling logic (see maybeReadFile and looksLikePath in create.go).pkg/cmd/route/export/export.go (1)
62-70: ⚡ Quick win
--service-idvalidation runs after the HTTP client is constructed.The service-ID guard at lines 68–70 fires after
opts.Client()is called (line 63), which may establish a TLS connection. Moving the check above the client construction (alongside the gateway-group check) fails fast and avoids the unnecessary connection setup.🔧 Suggested fix
ggID := opts.GatewayGroup if ggID == "" { ggID = cfg.GatewayGroup() } if ggID == "" { return fmt.Errorf("gateway group is required; use --gateway-group flag or set a default in context config") } + + if opts.ServiceID == "" { + return fmt.Errorf("--service-id is required by API7 EE") + } httpClient, err := opts.Client() if err != nil { return err } client := api.NewClient(httpClient, cfg.BaseURL()) - if opts.ServiceID == "" { - return fmt.Errorf("--service-id is required by API7 EE") - } items, err := fetchAll(client, ggID, opts.ServiceID, opts.Label)🤖 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 `@pkg/cmd/route/export/export.go` around lines 62 - 70, Move the --service-id validation to run before establishing the HTTP client so we fail fast and avoid opening a TLS connection; specifically, check opts.ServiceID (the same guard that currently returns fmt.Errorf("--service-id is required by API7 EE")) prior to calling opts.Client() and before creating client := api.NewClient(...). Keep the existing error message and behavior, and place the validation alongside the existing gateway-group check so the client is only constructed after required flags are validated.pkg/cmd/ssl/update/update_test.go (2)
35-47: ⚡ Quick winPrefer the project's
httpmockpackage over a hand-rolledRoundTripper.Per the repo testing guideline for
pkg/cmd/**/*_test.go, HTTP calls should be stubbed withpkg/httpmock. The route update test in this same PR mixes both styles, which leaves two parallel mocking utilities in the codebase. Ifhttpmock.Registrydoesn't yet expose request-body inspection, consider extending it once rather than duplicatingroundTripFunc/jsonHTTPResponsehere and inpkg/cmd/route/update/update_test.go.As per coding guidelines: "Use httpmock for testing: stub HTTP calls in unit tests with httpmock and test IOStreams."
🤖 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 `@pkg/cmd/ssl/update/update_test.go` around lines 35 - 47, The test defines a custom roundTripFunc, its RoundTrip method, and jsonHTTPResponse helpers (roundTripFunc, RoundTrip, jsonHTTPResponse); replace these with the project's pkg/httpmock utilities: register stubs via httpmock.Registry to return the desired status/body and remove the hand-rolled RoundTripper; if you need request-body inspection, extend httpmock.Registry (used also by pkg/cmd/route/update/update_test.go) to support inspecting request bodies and use that new capability instead of duplicating logic here so all tests reuse the same httpmock-based stubbing.
49-75: 💤 Low value
os.Chdirmakes this test process-global; flag for future parallelization.
os.Chdirmutates process-wide state; this test is safe today only because nothing in the package usest.Parallel(). A lightweight alternative is to pass an absolute path (e.g.,filepath.Join(tmp, "key.pem")) intomaybeReadFile, which still exercises the bare-relative branch via a relative form derived fromtmpif that is the goal. Optional, but it removes a footgun.🤖 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 `@pkg/cmd/ssl/update/update_test.go` around lines 49 - 75, Test uses os.Chdir which mutates process-global state; update TestMaybeReadFileReadsBareRelativePath to avoid changing cwd by writing the test file to tmp and passing its path to maybeReadFile instead: create path := filepath.Join(tmp, "key.pem"), write the file there, and call maybeReadFile(path); remove the os.Chdir, cwd capture/restore, and t.Cleanup code so the test no longer mutates process-wide working directory while still asserting file contents via maybeReadFile.pkg/cmd/ssl/update/update.go (1)
151-161: 💤 Low valueOutput writes local
body, not the server response.Unlike
pkg/cmd/route/update/update.gowhich decodes the PUT response and writes that back, this command discardsclient.Put's body and prints the locally-mutatedapi.SSL. If the server normalizes/echoes additional fields (timestamps, computed status, etc.), users won't see them, and the two update commands will behave inconsistently. Consider unmarshaling the PUT response and exporting that.♻️ Suggested change
- _, err = client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, payload) + respBody, err := client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, payload) if err != nil { return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) } + var updated api.SSL + if err := json.Unmarshal(respBody, &updated); err != nil { + return fmt.Errorf("failed to decode response: %w", err) + } @@ - return cmdutil.NewExporter(output, opts.IO.Out).Write(body) + return cmdutil.NewExporter(output, opts.IO.Out).Write(updated)🤖 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 `@pkg/cmd/ssl/update/update.go` around lines 151 - 161, The update command currently ignores the HTTP PUT response and writes the local `body` (the modified `api.SSL`), causing inconsistency with `pkg/cmd/route/update/update.go`; change the code that calls `client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, payload)` so you capture the response body, unmarshal/deserialize it into an `api.SSL` (or appropriate response struct) instead of using the local `body`, and pass that unmarshaled object to `cmdutil.NewExporter(output, opts.IO.Out).Write(...)`; keep the existing error handling (use `cmdutil.FormatAPIError(err)` for the PUT error) and ensure `opts.Output` fallbacks to "json" as before.pkg/cmd/route/update/update_test.go (1)
53-110: ⚡ Quick winMixing
httpmockwith a hand-rolledRoundTripperin the same test file.
TestUpdateRoute_SuccessandTestUpdateRoute_FromFileusehttpmock.Registry, while the newTestUpdateRoute_URIMapsToPathsAndPreservesCurrentRouteswitches to a customroundTripFuncwith helpers that are also duplicated inpkg/cmd/ssl/update/update_test.go. Either extendhttpmock.Registryto support payload assertions (preferred per testing guideline) or extract these helpers into a sharedinternal/testutilso both new tests reuse them.As per coding guidelines: "Use httpmock for testing: stub HTTP calls in unit tests with httpmock and test IOStreams."
🤖 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 `@pkg/cmd/route/update/update_test.go` around lines 53 - 110, The new test TestUpdateRoute_URIMapsToPathsAndPreservesCurrentRoute uses a hand-rolled RoundTripper (roundTripFunc and jsonHTTPResponse) instead of the project-standard httpmock.Registry and duplicates helpers from pkg/cmd/ssl/update/update_test.go; refactor by replacing the custom transport with httpmock.Registry-based stubs that assert request payloads (or move roundTripFunc/jsonHTTPResponse into a shared internal/testutil and reuse them), update TestUpdateRoute_URIMapsToPathsAndPreservesCurrentRoute to register GET and PUT handlers via httpmock.Registry (or import the shared helpers) so assertions on payload, paths mapping and status are performed the same way as TestUpdateRoute_Success/TestUpdateRoute_FromFile, and ensure actionRun is exercised using the project IO/client setup (Options{IO, Client, Config, ID, URI, Status, StatusSet, GatewayGroup}) without introducing duplicate test helpers.pkg/cmd/route/update/update.go (1)
176-196: 💤 Low value
routePayloadbranch logic is harder to follow than necessary.The early-return predicate
(!StatusSet || Status != 0) && (!PrioritySet || Priority != 0)reads as "not (StatusSet && Status==0) AND not (PrioritySet && Priority==0)". A simpler equivalent is to enter map mode whenever either flag is explicitly set, and unconditionally write the explicit values:♻️ Simpler payload builder
func routePayload(route api.Route, opts *Options) (interface{}, error) { - if (!opts.StatusSet || opts.Status != 0) && (!opts.PrioritySet || opts.Priority != 0) { - return route, nil - } - - b, err := json.Marshal(route) - if err != nil { - return nil, fmt.Errorf("failed to encode route payload: %w", err) - } - var payload map[string]interface{} - if err := json.Unmarshal(b, &payload); err != nil { - return nil, fmt.Errorf("failed to prepare route payload: %w", err) - } - if opts.StatusSet { - payload["status"] = opts.Status - } - if opts.PrioritySet { - payload["priority"] = opts.Priority - } - return payload, nil + // Only re-marshal when an explicit zero needs to override an `omitempty` tag. + needMap := (opts.StatusSet && opts.Status == 0) || (opts.PrioritySet && opts.Priority == 0) + if !needMap { + return route, nil + } + b, err := json.Marshal(route) + if err != nil { + return nil, fmt.Errorf("failed to encode route payload: %w", err) + } + var payload map[string]interface{} + if err := json.Unmarshal(b, &payload); err != nil { + return nil, fmt.Errorf("failed to prepare route payload: %w", err) + } + if opts.StatusSet { + payload["status"] = opts.Status + } + if opts.PrioritySet { + payload["priority"] = opts.Priority + } + return payload, nil }🤖 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 `@pkg/cmd/route/update/update.go` around lines 176 - 196, The branch logic in routePayload is confusing and incorrectly tied to non-zero values; change it to check whether either flag is explicitly set: if both opts.StatusSet and opts.PrioritySet are false, return route unchanged; otherwise marshal route into a map and, for each flag (opts.StatusSet and opts.PrioritySet), assign the corresponding value (opts.Status, opts.Priority) into the payload even when the value is zero. Update the predicate that currently reads `(!opts.StatusSet || opts.Status != 0) && (!opts.PrioritySet || opts.Priority != 0)` to the clearer `!opts.StatusSet && !opts.PrioritySet` check and then set payload["status"] and payload["priority"] only when their respective Set flags are true.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/user-guide/secret.md`:
- Line 64: The tables incorrectly label `--file` as "(required)" even though the
PR added a flag mode alternative; update both occurrences of the `--file` entry
so the requirement is conditional (for example: change "(required)" to "Required
unless using flag mode" or "Required unless --flag is set"), and ensure the same
wording is applied to the second table's `--file` entry to keep the docs
consistent with the new flag mode behavior.
In `@pkg/cmd/credential/create/create_test.go`:
- Around line 61-99: Replace the custom roundTripFunc transport in
TestCreateCredential_PositionalIDSetsName with an httpmock.Registry: create a
registry via httpmock.NewRegistry(), register a responder for POST requests to
"/apisix/admin/consumers/alice/credentials" that reads and decodes the request
body, asserts payload["name"] == "cred1", and returns the JSON response
`{"id":"generated","name":"cred1"}`; set Options.Client to return
registry.Client(); keep the existing mockConfig/Options fields and assertions on
actionRun output unchanged so the test follows the same httpmock pattern used by
TestCreateCredential_JSONOutput and TestCreateCredential_APIError.
In `@pkg/cmd/credential/create/create.go`:
- Around line 94-95: payload["name"] is used as an interface{} and interpolated
into the PUT URL without validation, which can produce malformed paths; before
calling client.Put (the call that builds
"/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s"), assert that
payload["name"] exists, is a non-empty string (e.g., cast to string and check
len>0) and return an error or handle the case if it is missing/empty or not a
string; only then call client.Put with the validated name along with
opts.Consumer and ggID.
In `@pkg/cmd/secret/update/update_test.go`:
- Around line 69-99: TestUpdateSecret_PreservesCurrentFieldsWhenOmitted
currently only asserts the mocked PUT response instead of verifying the outgoing
PUT request body; change the test to use a custom RoundTripper (e.g.
roundTripFunc) and jsonHTTPResponse helper instead of httpmock.Registry so you
can create an *http.Client that inspects the PUT request body sent by actionRun,
decode/assert that the request contains the merged fields (preserved
prefix/token when omitted), and return an appropriate JSON response; replace
registry.GetClient() with the constructed client and keep references to
TestUpdateSecret_PreservesCurrentFieldsWhenOmitted and actionRun so the test
validates the read-modify-write behavior rather than the mock response.
In `@pkg/cmd/ssl/update/update_test.go`:
- Around line 100-113: Mocked HTTP response for the route update test is missing
the APISIX envelope; update the test handler that returns the route (the case
matching the route GET used by api.Client.Get()) to return the raw API response
wrapped as {"value": {...}} so that unwrapValueEnvelope() sees the same shape as
in the SSL test; locate the route update test's HTTP mock case and change its
JSON response to include the top-level "value" object containing the route (id
r1, etc.).
In `@pkg/cmd/stream-route/list/list.go`:
- Line 44: The flag description for the ServiceID flag is misleading because the
code treats it as required; update the StringVar call that sets the flag
description for opts.ServiceID (the c.Flags().StringVar(..., &opts.ServiceID,
"service-id", ...)) to remove "Filter by" and clearly mark it as required, e.g.
change the description to "Service ID (required by API7 EE)"; ensure the new
text appears in the c.Flags().StringVar invocation that defines the flag so help
output matches the runtime validation.
---
Outside diff comments:
In `@docs/user-guide/route.md`:
- Around line 11-13: The docs currently say `--service-id` "may" be required,
but pkg/cmd/route/list/list.go unconditionally errors if the flag is missing;
update the route list documentation to state that `--service-id` is required
(e.g., "The --service-id flag is required for route list in API7 EE; omit only
if your environment explicitly supports unscoped listing") so the text matches
the behavior implemented in the list command (refer to the check/error in
list.go that returns "`--service-id is required by API7 EE`").
In `@pkg/cmd/plugin-config/update/update_test.go`:
- Around line 97-111: Update the validation error text used when neither plugins
JSON nor a file is provided: in update.go locate the validation in actionRun (or
where Options is validated) that currently returns "--plugins-json is required"
and change it to "--plugins-json or --file is required"; then update the test
assertion in pkg/cmd/plugin-config/update/update_test.go
(TestUpdatePluginConfig_ValidationError) to expect the new message
("--plugins-json or --file is required") instead of the old one. Ensure the
check still uses strings.Contains on err.Error() so the test remains robust.
In `@pkg/cmd/secret/update/update.go`:
- Around line 124-139: The api.Secret struct is being serialized with its Token
unredacted (seen when exporter.Write(updated) and
exporter.Write(json.RawMessage(body)) are called); fix by adding a MarshalJSON
method on api.Secret that returns JSON with the Token field redacted or omitted
(implement api.Secret.MarshalJSON() to copy the struct into a safe
representation or a helper struct without Token or with Token replaced by
"<redacted>"), ensure the method is used by the exporter (standard encoding/json
will pick up MarshalJSON) and keep other fields unchanged.
---
Nitpick comments:
In `@pkg/cmd/plugin-config/list/list.go`:
- Line 73: The current return in list.go wraps
FormatAPISIXCompatibilityResourceError with fmt.Errorf("%s", ...) which discards
the original error chain; change the return to preserve the wrapped error (use
fmt.Errorf(..., %w, ...) or simply return the error directly) so callers can use
errors.Is/errors.As; locate the call to FormatAPISIXCompatibilityResourceError
in pkg/cmd/plugin-config/list/list.go (and the analogous call in
pkg/cmd/plugin-config/get/get.go) and if FormatAPISIXCompatibilityResourceError
returns an error, return or wrap it with %w, and if it returns a string, keep
wrapping but ensure any underlying error is propagated where applicable.
In `@pkg/cmd/route/export/export.go`:
- Around line 62-70: Move the --service-id validation to run before establishing
the HTTP client so we fail fast and avoid opening a TLS connection;
specifically, check opts.ServiceID (the same guard that currently returns
fmt.Errorf("--service-id is required by API7 EE")) prior to calling
opts.Client() and before creating client := api.NewClient(...). Keep the
existing error message and behavior, and place the validation alongside the
existing gateway-group check so the client is only constructed after required
flags are validated.
In `@pkg/cmd/route/list/list.go`:
- Around line 63-71: Move the required-service validation before constructing
the HTTP client: check opts.ServiceID and return the fmt.Errorf("--service-id is
required by API7 EE") immediately if empty, then call opts.Client() and
api.NewClient(...) only after that check; update the block around opts.Client(),
api.NewClient, and the ServiceID guard so validation occurs first (referencing
opts.Client(), api.NewClient, and opts.ServiceID).
In `@pkg/cmd/route/update/update_test.go`:
- Around line 53-110: The new test
TestUpdateRoute_URIMapsToPathsAndPreservesCurrentRoute uses a hand-rolled
RoundTripper (roundTripFunc and jsonHTTPResponse) instead of the
project-standard httpmock.Registry and duplicates helpers from
pkg/cmd/ssl/update/update_test.go; refactor by replacing the custom transport
with httpmock.Registry-based stubs that assert request payloads (or move
roundTripFunc/jsonHTTPResponse into a shared internal/testutil and reuse them),
update TestUpdateRoute_URIMapsToPathsAndPreservesCurrentRoute to register GET
and PUT handlers via httpmock.Registry (or import the shared helpers) so
assertions on payload, paths mapping and status are performed the same way as
TestUpdateRoute_Success/TestUpdateRoute_FromFile, and ensure actionRun is
exercised using the project IO/client setup (Options{IO, Client, Config, ID,
URI, Status, StatusSet, GatewayGroup}) without introducing duplicate test
helpers.
In `@pkg/cmd/route/update/update.go`:
- Around line 176-196: The branch logic in routePayload is confusing and
incorrectly tied to non-zero values; change it to check whether either flag is
explicitly set: if both opts.StatusSet and opts.PrioritySet are false, return
route unchanged; otherwise marshal route into a map and, for each flag
(opts.StatusSet and opts.PrioritySet), assign the corresponding value
(opts.Status, opts.Priority) into the payload even when the value is zero.
Update the predicate that currently reads `(!opts.StatusSet || opts.Status != 0)
&& (!opts.PrioritySet || opts.Priority != 0)` to the clearer `!opts.StatusSet &&
!opts.PrioritySet` check and then set payload["status"] and payload["priority"]
only when their respective Set flags are true.
In `@pkg/cmd/ssl/create/create_test.go`:
- Around line 8-34: The test TestMaybeReadFileReadsBareRelativePath should avoid
mutating process cwd; change it to create the cert file in the temp dir and call
maybeReadFile with its absolute path instead of calling os.Chdir/restoring cwd.
Specifically, drop the os.Chdir/cwd/t.Cleanup block, write the file to
filepath.Join(tmp, "cert.pem"), and pass that absolute path into maybeReadFile
so the test exercises the path-handling logic (see maybeReadFile and
looksLikePath in create.go).
In `@pkg/cmd/ssl/update/update_test.go`:
- Around line 35-47: The test defines a custom roundTripFunc, its RoundTrip
method, and jsonHTTPResponse helpers (roundTripFunc, RoundTrip,
jsonHTTPResponse); replace these with the project's pkg/httpmock utilities:
register stubs via httpmock.Registry to return the desired status/body and
remove the hand-rolled RoundTripper; if you need request-body inspection, extend
httpmock.Registry (used also by pkg/cmd/route/update/update_test.go) to support
inspecting request bodies and use that new capability instead of duplicating
logic here so all tests reuse the same httpmock-based stubbing.
- Around line 49-75: Test uses os.Chdir which mutates process-global state;
update TestMaybeReadFileReadsBareRelativePath to avoid changing cwd by writing
the test file to tmp and passing its path to maybeReadFile instead: create path
:= filepath.Join(tmp, "key.pem"), write the file there, and call
maybeReadFile(path); remove the os.Chdir, cwd capture/restore, and t.Cleanup
code so the test no longer mutates process-wide working directory while still
asserting file contents via maybeReadFile.
In `@pkg/cmd/ssl/update/update.go`:
- Around line 151-161: The update command currently ignores the HTTP PUT
response and writes the local `body` (the modified `api.SSL`), causing
inconsistency with `pkg/cmd/route/update/update.go`; change the code that calls
`client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, payload)`
so you capture the response body, unmarshal/deserialize it into an `api.SSL` (or
appropriate response struct) instead of using the local `body`, and pass that
unmarshaled object to `cmdutil.NewExporter(output, opts.IO.Out).Write(...)`;
keep the existing error handling (use `cmdutil.FormatAPIError(err)` for the PUT
error) and ensure `opts.Output` fallbacks to "json" as before.
In `@test/e2e/stream_route_test.go`:
- Around line 35-46: TestStreamRoute_List (and its companion
TestStreamRoute_ListJSON) currently uses assert.NotEmpty(t, stdout) which can
succeed with only table headers; instead, assert the command succeeded by
checking err==nil (already handled) and either remove the NotEmpty assertion or
replace it with a structural check: for the plain text test verify stdout does
not equal just the header string returned by runA7WithEnv, and for the JSON test
assert stdout == "[]" (or parse JSON and assert len==0). Update assertions in
TestStreamRoute_List and TestStreamRoute_ListJSON referencing runA7WithEnv,
stdout, stderr, and err accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9e84e05-ee92-44fc-8d96-0ff1b5b320b4
📒 Files selected for processing (39)
docs/user-guide/configuration.mddocs/user-guide/route.mddocs/user-guide/secret.mddocs/user-guide/stream-route.mdpkg/cmd/context/create/create.gopkg/cmd/context/create/create_test.gopkg/cmd/credential/create/create.gopkg/cmd/credential/create/create_test.gopkg/cmd/plugin-config/create/create.gopkg/cmd/plugin-config/create/create_test.gopkg/cmd/plugin-config/delete/delete.gopkg/cmd/plugin-config/export/export.gopkg/cmd/plugin-config/get/get.gopkg/cmd/plugin-config/list/list.gopkg/cmd/plugin-config/list/list_test.gopkg/cmd/plugin-config/update/update.gopkg/cmd/plugin-config/update/update_test.gopkg/cmd/route/export/export.gopkg/cmd/route/export/export_test.gopkg/cmd/route/list/list.gopkg/cmd/route/list/list_test.gopkg/cmd/route/update/update.gopkg/cmd/route/update/update_test.gopkg/cmd/secret/create/create.gopkg/cmd/secret/create/create_test.gopkg/cmd/secret/update/update.gopkg/cmd/secret/update/update_test.gopkg/cmd/service/update/update.gopkg/cmd/service/update/update_test.gopkg/cmd/ssl/create/create.gopkg/cmd/ssl/create/create_test.gopkg/cmd/ssl/update/update.gopkg/cmd/ssl/update/update_test.gopkg/cmd/stream-route/export/export.gopkg/cmd/stream-route/export/export_test.gopkg/cmd/stream-route/list/list.gopkg/cmd/stream-route/list/list_test.gopkg/cmdutil/errors.gotest/e2e/stream_route_test.go
Comment on lines
+164
to
+167
| func sslPayload(ssl api.SSL, opts *Options) (interface{}, error) { | ||
| if !opts.StatusSet || opts.Status != 0 { | ||
| return ssl, nil | ||
| } |
Comment on lines
+176
to
+179
| func routePayload(route api.Route, opts *Options) (interface{}, error) { | ||
| if (!opts.StatusSet || opts.Status != 0) && (!opts.PrioritySet || opts.Priority != 0) { | ||
| return route, nil | ||
| } |
Comment on lines
56
to
64
| Creates a new secret manager from a JSON or YAML file using a compound ID. | ||
| Flag mode is also supported for Vault-style provider configuration. Use | ||
| `--provider-token` for the secret backend token; the global `--token` flag is | ||
| reserved for the API7 EE API token. | ||
|
|
||
| | Flag | Short | Default | Description | | ||
| |------|-------|---------|-------------| | ||
| | `--gateway-group` | `-g` | | Target gateway group name (required) | | ||
| | `--file` | `-f` | | Path to the secret configuration file (required) | |
Comment on lines
+85
to
93
| Updates an existing secret manager by compound ID. As with create, use | ||
| `--provider-token` for the secret backend token in flag mode. | ||
|
|
||
| | Flag | Short | Default | Description | | ||
| |------|-------|---------|-------------| | ||
| | `--gateway-group` | `-g` | | Target gateway group name (required) | | ||
| | `--file` | `-f` | | Path to the secret configuration file (required) | | ||
| | `--provider-token` | | | Secret provider token for flag mode | | ||
| | `--output` | `-o` | `yaml` | Output format (json, yaml) | |
| @@ -134,7 +137,7 @@ Exports routes from a gateway group to a file or stdout. | |||
|
|
|||
| Export all routes to a YAML file: | |||
Comment on lines
18
to
19
| | `--page` | | `1` | Page number for pagination | | ||
| | `--page-size` | | `20` | Number of items per page | |
|
|
||
| **Examples:** | ||
|
|
||
| Export all stream routes to a YAML file: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
test/e2e/ssl_test.go (1)
132-133: ⚡ Quick winPost-delete verification is missing.
TestSSL_CRUDfollowsssl deletewith assl getcall to confirm the resource is gone (lines 94–95). This test omits that step. While not a blocker, it leaves a gap in the delete-command coverage of this test path.✅ Proposed addition
stdout, stderr, err = runA7WithEnv(env, "ssl", "delete", sslID, "--force", "-g", gatewayGroup) require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + _, _, err = runA7WithEnv(env, "ssl", "get", sslID, "-g", gatewayGroup) + assert.Error(t, err)🤖 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/ssl_test.go` around lines 132 - 133, After running the delete command with runA7WithEnv("ssl", "delete", sslID, "--force", "-g", gatewayGroup), add a post-delete verification step that calls runA7WithEnv("ssl", "get", sslID, "-g", gatewayGroup) and assert that the command returns a not-found error (or non-zero exit) and/or that stdout/stderr contains an expected "not found" message; locate the existing delete invocation around runA7WithEnv and use the same sslID and gatewayGroup variables, then use require.Error or require.Contains on stdout/stderr (or require.Equal for exit code) to confirm the resource was removed.test/e2e/service_template_test.go (1)
17-31: ⚡ Quick winNarrow capability-gap detection to avoid false skips.
Current needles are broad enough to skip on unrelated 404-style failures, which can mask real regressions in the CLI path.
Suggested tightening
func isKnownServiceTemplateCapabilityGap(stdout, stderr string) bool { combined := strings.ToLower(stdout + "\n" + stderr) - for _, needle := range []string{ - "resource not found", - "api error (status 404)", - "/404", - "api7 enterprise", - "not exposed", - } { - if strings.Contains(combined, needle) { - return true - } - } - return false + has404 := strings.Contains(combined, "api error (status 404)") || + strings.Contains(combined, "resource not found") || + strings.Contains(combined, "/404") + isServiceTemplatePath := strings.Contains(combined, "service-template") || + strings.Contains(combined, "/api/services/template") + return has404 && isServiceTemplatePath }🤖 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/service_template_test.go` around lines 17 - 31, The isKnownServiceTemplateCapabilityGap helper currently uses very broad needles (e.g., "resource not found", "/404") that cause unrelated 404s to be treated as capability gaps; tighten detection in isKnownServiceTemplateCapabilityGap by replacing those generic strings with more specific, contextual markers (for example, match error text that includes the service-template resource name or CLI subcommand, specific API path segments like "/service-templates" or "service template not found", or include the CLI tool prefix seen in stderr), remove generic patterns like "/404" and "resource not found", and/or use anchored regexes that require the templates endpoint or command name to appear alongside the 404 text so only genuine service-template capability gaps are skipped.test/e2e/route_test.go (1)
197-198: 💤 Low valueTest assertion insufficient to verify
--urireplaces (not appends to) paths.The helper
createTestRouteWithServiceViaCLIinitializes the route with a single path (["/test-<routeID>"]). When updated via--uri /test-update-flags-new,assert.Containspasses regardless of whether the operation replaces the original path or appends to it. To catch replace-vs-append regressions, use exact equality:♻️ Suggested assertion
- paths := requireJSONArray(t, route["paths"], "route.paths") - assert.Contains(t, paths, "/test-update-flags-new") + paths := requireJSONArray(t, route["paths"], "route.paths") + assert.Equal(t, []interface{}{"/test-update-flags-new"}, paths)🤖 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/route_test.go` around lines 197 - 198, The test currently uses assert.Contains(t, paths, "/test-update-flags-new") which can't distinguish replacing vs appending; update the assertion after calling requireJSONArray(t, route["paths"], "route.paths") to assert exact equality instead of containment (e.g. use require.Equal or assert.Equal/EqualValues) and compare the paths value to a single-element slice containing "/test-update-flags-new" so the test fails if the original path was preserved (refer to createTestRouteWithServiceViaCLI, requireJSONArray, and the local variable paths/route["paths"]).
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/e2e/service_template_test.go`:
- Around line 72-74: The test prints raw CLI stdout/stderr in t.Skipf and
require.NoError messages; update the calls that use t.Skipf("service-template...
stdout=%s stderr=%s", stdout, stderr) and require.NoError(t, err, "stdout=%s
stderr=%s", stdout, stderr) (and the other occurrences around lines referenced)
to avoid leaking secrets by either omitting stdout/stderr entirely or replacing
them with a sanitized/redacted summary (e.g., "stdout=<redacted>" and
"stderr=<redacted>" or a truncated/sanitized prefix), and ensure the same
redaction is applied to the other similar calls (the occurrences you noted at
108-109, 119-120, 131-132, 190-191, 217-219) so tests assert messages never
include raw CLI output.
In `@test/e2e/ssl_test.go`:
- Around line 117-118: The CLI subprocess in ssl_test.go currently passes
relative paths "--cert", "testdata/test.crt" and "--key", "testdata/test.key"
which can break if the test CWD is not test/e2e/; update the test to build
absolute paths using the same resolveModuleRoot() logic (as used by
readTestCert) and pass those resolved absolute paths to the "--cert" and "--key"
arguments so the subprocess always receives correct file locations regardless of
working directory.
---
Nitpick comments:
In `@test/e2e/route_test.go`:
- Around line 197-198: The test currently uses assert.Contains(t, paths,
"/test-update-flags-new") which can't distinguish replacing vs appending; update
the assertion after calling requireJSONArray(t, route["paths"], "route.paths")
to assert exact equality instead of containment (e.g. use require.Equal or
assert.Equal/EqualValues) and compare the paths value to a single-element slice
containing "/test-update-flags-new" so the test fails if the original path was
preserved (refer to createTestRouteWithServiceViaCLI, requireJSONArray, and the
local variable paths/route["paths"]).
In `@test/e2e/service_template_test.go`:
- Around line 17-31: The isKnownServiceTemplateCapabilityGap helper currently
uses very broad needles (e.g., "resource not found", "/404") that cause
unrelated 404s to be treated as capability gaps; tighten detection in
isKnownServiceTemplateCapabilityGap by replacing those generic strings with more
specific, contextual markers (for example, match error text that includes the
service-template resource name or CLI subcommand, specific API path segments
like "/service-templates" or "service template not found", or include the CLI
tool prefix seen in stderr), remove generic patterns like "/404" and "resource
not found", and/or use anchored regexes that require the templates endpoint or
command name to appear alongside the 404 text so only genuine service-template
capability gaps are skipped.
In `@test/e2e/ssl_test.go`:
- Around line 132-133: After running the delete command with runA7WithEnv("ssl",
"delete", sslID, "--force", "-g", gatewayGroup), add a post-delete verification
step that calls runA7WithEnv("ssl", "get", sslID, "-g", gatewayGroup) and assert
that the command returns a not-found error (or non-zero exit) and/or that
stdout/stderr contains an expected "not found" message; locate the existing
delete invocation around runA7WithEnv and use the same sslID and gatewayGroup
variables, then use require.Error or require.Contains on stdout/stderr (or
require.Equal for exit code) to confirm the resource was removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c61b509-e96e-4354-aaf6-a82517c97664
📒 Files selected for processing (8)
test/e2e/credential_test.gotest/e2e/plugin_config_test.gotest/e2e/route_test.gotest/e2e/secret_test.gotest/e2e/service_template_test.gotest/e2e/service_test.gotest/e2e/ssl_test.gotest/e2e/stream_route_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/stream_route_test.go
Comment on lines
76
to
99
| if opts.File != "" { | ||
| payload, err := cmdutil.ReadResourceFile(opts.File, opts.IO.In) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if opts.ID != "" { | ||
| payload["id"] = opts.ID | ||
| payload["name"] = opts.ID | ||
| delete(payload, "id") | ||
| } | ||
|
|
||
| httpClient, err := opts.Client() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| path := "/apisix/admin/consumers/" + opts.Consumer + "/credentials?gateway_group_id=" + ggID | ||
| client := api.NewClient(httpClient, cfg.BaseURL()) | ||
| var body []byte | ||
| if id, ok := payload["id"]; ok { | ||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, id, ggID), payload) | ||
| if name, ok := payload["name"]; ok { | ||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, name, ggID), payload) | ||
| } else { | ||
| body, err = client.Post(path, payload) | ||
| } |
Comment on lines
155
to
+180
| @@ -140,3 +173,25 @@ func actionRun(opts *Options) error { | |||
| exporter := cmdutil.NewExporter(format, opts.IO.Out) | |||
| return exporter.Write(updated) | |||
| } | |||
|
|
|||
| func routePayload(route api.Route, opts *Options) (interface{}, error) { | |||
| if !opts.StatusSet && !opts.PrioritySet { | |||
| return route, nil | |||
| } | |||
Comment on lines
+168
to
+171
| func sslPayload(ssl api.SSL, opts *Options) (interface{}, error) { | ||
| if !opts.StatusSet || opts.Status != 0 { | ||
| return ssl, nil | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/api/types_secret.go (1)
6-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Secretmust implementMarshalJSON()(andString()) to enforce redaction at the type level.The coding guideline requires secret-bearing structs to implement
MarshalJSON()andString()for redaction, not rely on caller-side helper functions. Any directjson.Marshalof aSecretvalue that bypassesRedactSecret— in future commands, in dump/diff/sync paths, or in error-handling code — will emit the raw token. The helper functions alone provide no compile-time or runtime safety guarantee.🛡️ Suggested approach
+// MarshalJSON always redacts Token before serializing. +func (s Secret) MarshalJSON() ([]byte, error) { + type Alias Secret + a := Alias(s) + if a.Token != "" { + a.Token = RedactedSecretToken + } + return json.Marshal(a) +} + +// String redacts Token in any string representation. +func (s Secret) String() string { + if s.Token != "" { + s.Token = RedactedSecretToken + } + return fmt.Sprintf("%+v", struct{ ID, URI, Prefix, Token string }{s.ID, s.URI, s.Prefix, s.Token}) +}With
MarshalJSONon the struct itself,RedactSecret/RedactSecretscan delegate to it or be removed; all callers are protected automatically.As per coding guidelines: "All secret-bearing structs must implement proper redaction in String(), MarshalJSON(), and MarshalLogObject() methods."
🤖 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 `@pkg/api/types_secret.go` around lines 6 - 31, The Secret type currently exposes raw Token unless callers use RedactSecret/RedactSecrets; implement type-level redaction by adding Secret.MarshalJSON(), Secret.String(), and Secret.MarshalLogObject() that replace Token with RedactedSecretToken when serializing or formatting, and update or remove RedactSecret/RedactSecrets to delegate to these methods as needed so any json.Marshal or fmt.Stringer/zerolog logging automatically emits the redacted token; ensure MarshalJSON returns a JSON representation identical to current fields but with Token replaced by RedactedSecretToken when non-empty.pkg/httpmock/httpmock.go (1)
50-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDeadlock: user-supplied responder is called while the registry mutex is held.
Any responder that calls back into the same
Registry(e.g.,registry.CallCount,registry.Verify, or evenregistry.Registerin a chained test) will deadlock becausesync.Mutexis not re-entrant. SinceRegisterResponderis a public API, this is a latent footgun that is hard to diagnose.Capture the fields you need under the lock, release it before invoking the responder:
🔒 Proposed fix
func (r *Registry) RoundTrip(req *http.Request) (*http.Response, error) { r.mu.Lock() - defer r.mu.Unlock() for i, m := range r.mocks { if m.method == req.Method && m.path == req.URL.Path { r.mocks[i].called++ - resp := m.resp - if m.responder != nil { - var err error - resp, err = m.responder(req) - if err != nil { - return nil, err - } - } + resp := m.resp + responder := m.responder + r.mu.Unlock() + if responder != nil { + var err error + resp, err = responder(req) + if err != nil { + return nil, err + } + } header := make(http.Header) header.Set("Content-Type", "application/json") for k, v := range resp.Header { header[k] = v } return &http.Response{ StatusCode: resp.StatusCode, Body: io.NopCloser(bytes.NewBuffer(resp.Body)), Header: header, }, nil } } + r.mu.Unlock() return nil, fmt.Errorf("no mock registered for %s %s", req.Method, req.URL.Path) }🤖 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 `@pkg/httpmock/httpmock.go` around lines 50 - 64, The RoundTrip function currently calls the user-supplied responder while holding r.mu which can deadlock if the responder calls back into Registry (e.g., RegisterResponder, CallCount, Verify, Register); fix by capturing the minimal state under the lock (the mock entry, its current resp and the responder function, and increment r.mocks[i].called there), then release r.mu before invoking m.responder(req); after unlocking, call the responder (handle returned resp and error) and return — do not access r.mocks while the responder runs.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@pkg/cmd/credential/create/create_test.go`:
- Around line 83-84: The test's assertion uses a fragile string match on
out.String(); instead parse the CLI output JSON into a map or struct using
json.Unmarshal (like TestCreateCredential_JSONOutput does) and assert that the
"name" field equals "cred1". Locate the assertion that checks
strings.Contains(out.String(), `"name": "cred1"`) and replace it with
unmarshalling out.Bytes()/out.String() into a map[string]interface{} (or a
credential struct) and then validate the name value, keeping existing variable
names like out and the test function intact.
In `@pkg/cmd/ssl/update/update.go`:
- Around line 118-151: The GET→modify→PUT sequence in update.go (using
client.Get to fetch api.SSL into body, mutating it, then calling client.Put with
payload via sslPayload) is susceptible to TOCTOU races; update the flow to
perform an optimistic concurrency check: capture the resource version/ETag or
revision metadata from the client.Get response (or response headers) and send it
with the client.Put (e.g., If-Match header or include the version field) so the
server rejects conflicting updates, and handle the rejection by returning a
clear error or retrying (re-fetch, merge, and attempt update again). Ensure you
reference client.Get, client.Put, api.SSL/body, sslPayload, opts.ID and ggID
when implementing the conditional update and error handling.
- Around line 207-215: The looksLikePath function currently treats non-existent
bare filenames as inline content; update it so missing files are still
considered paths: in looksLikePath, after calling os.Stat(v) treat
os.IsNotExist(err) the same as a successful stat (return true) so typos like
"--cert server.pem" produce a local file-read error instead of being
misinterpreted as inline PEM; keep existing checks for "-----BEGIN " and
newlines and for path prefixes unchanged, and only change the os.Stat error
handling to return true when err == nil OR os.IsNotExist(err).
- Around line 155-165: The api.SSL struct exposes a secret Key field that will
be serialized by cmdutil.NewExporter(...).Write(updated) and thus must implement
redaction; add implementations on api.SSL for MarshalJSON, MarshalLogObject (for
zap/log), and String (or fmt.Stringer) that redact or omit the Key value, and
ensure any JSON output used by Exporter returns the redacted form (not the raw
Key). Specifically, locate the api.SSL type and implement MarshalJSON to produce
a JSON object with Key replaced by a redacted placeholder, implement
MarshalLogObject to write fields without exposing Key, and implement String() to
return a safe representation; verify Exporter.Write(updated) now serializes the
redacted output.
---
Outside diff comments:
In `@pkg/api/types_secret.go`:
- Around line 6-31: The Secret type currently exposes raw Token unless callers
use RedactSecret/RedactSecrets; implement type-level redaction by adding
Secret.MarshalJSON(), Secret.String(), and Secret.MarshalLogObject() that
replace Token with RedactedSecretToken when serializing or formatting, and
update or remove RedactSecret/RedactSecrets to delegate to these methods as
needed so any json.Marshal or fmt.Stringer/zerolog logging automatically emits
the redacted token; ensure MarshalJSON returns a JSON representation identical
to current fields but with Token replaced by RedactedSecretToken when non-empty.
In `@pkg/httpmock/httpmock.go`:
- Around line 50-64: The RoundTrip function currently calls the user-supplied
responder while holding r.mu which can deadlock if the responder calls back into
Registry (e.g., RegisterResponder, CallCount, Verify, Register); fix by
capturing the minimal state under the lock (the mock entry, its current resp and
the responder function, and increment r.mocks[i].called there), then release
r.mu before invoking m.responder(req); after unlocking, call the responder
(handle returned resp and error) and return — do not access r.mocks while the
responder runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc0ca909-2aec-4ca0-adce-4fa1c2ad1d1b
📒 Files selected for processing (24)
docs/user-guide/route.mddocs/user-guide/secret.mddocs/user-guide/stream-route.mdinternal/config/config.gopkg/api/types_secret.gopkg/cmd/credential/create/create.gopkg/cmd/credential/create/create_test.gopkg/cmd/route/update/update.gopkg/cmd/route/update/update_test.gopkg/cmd/secret/create/create.gopkg/cmd/secret/create/create_test.gopkg/cmd/secret/get/get.gopkg/cmd/secret/get/get_test.gopkg/cmd/secret/list/list.gopkg/cmd/secret/update/update.gopkg/cmd/secret/update/update_test.gopkg/cmd/ssl/create/create_test.gopkg/cmd/ssl/update/update.gopkg/cmd/ssl/update/update_test.gopkg/httpmock/httpmock.goscripts/validate-skills.shtest/e2e/route_test.gotest/e2e/service_template_test.gotest/e2e/ssl_test.go
✅ Files skipped from review due to trivial changes (2)
- internal/config/config.go
- docs/user-guide/secret.md
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/user-guide/stream-route.md
- pkg/cmd/credential/create/create.go
- test/e2e/ssl_test.go
- pkg/cmd/secret/create/create.go
- pkg/cmd/secret/update/update_test.go
- pkg/cmd/secret/create/create_test.go
- pkg/cmd/ssl/update/update_test.go
- docs/user-guide/route.md
- pkg/cmd/secret/update/update.go
- pkg/cmd/ssl/create/create_test.go
- test/e2e/service_template_test.go
- test/e2e/route_test.go
- pkg/cmd/route/update/update.go
Comment on lines
+118
to
+151
| client := api.NewClient(httpClient, cfg.BaseURL()) | ||
| currentBody, err := client.Get("/apisix/admin/ssls/"+opts.ID, map[string]string{"gateway_group_id": ggID}) | ||
| if err != nil { | ||
| return fmt.Errorf("%s", cmdutil.FormatAPIError(err)) | ||
| } | ||
| var body api.SSL | ||
| if err := json.Unmarshal(currentBody, &body); err != nil { | ||
| return fmt.Errorf("failed to decode current ssl: %w", err) | ||
| } | ||
|
|
||
| client := api.NewClient(httpClient, cfg.BaseURL()) | ||
| _, err = client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, body) | ||
| if cert != "" { | ||
| body.Cert = cert | ||
| } | ||
| if key != "" { | ||
| body.Key = key | ||
| } | ||
| if len(opts.SNIs) > 0 { | ||
| body.SNIs = opts.SNIs | ||
| } | ||
| if len(opts.Labels) > 0 { | ||
| body.Labels = parseLabels(opts.Labels) | ||
| } | ||
| if opts.TypeSet { | ||
| body.Type = opts.Type | ||
| } | ||
| if opts.StatusSet { | ||
| body.Status = opts.Status | ||
| } | ||
|
|
||
| payload, err := sslPayload(body, opts) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| updatedBody, err := client.Put("/apisix/admin/ssls/"+opts.ID+"?gateway_group_id="+ggID, payload) |
There was a problem hiding this comment.
Guard the merge flow against concurrent writers.
This GET→modify→PUT sequence is still a TOCTOU update. If another admin changes the same SSL between Line 119 and Line 151, this command will overwrite those intervening changes with the stale snapshot it fetched.
🤖 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 `@pkg/cmd/ssl/update/update.go` around lines 118 - 151, The GET→modify→PUT
sequence in update.go (using client.Get to fetch api.SSL into body, mutating it,
then calling client.Put with payload via sslPayload) is susceptible to TOCTOU
races; update the flow to perform an optimistic concurrency check: capture the
resource version/ETag or revision metadata from the client.Get response (or
response headers) and send it with the client.Put (e.g., If-Match header or
include the version field) so the server rejects conflicting updates, and handle
the rejection by returning a clear error or retrying (re-fetch, merge, and
attempt update again). Ensure you reference client.Get, client.Put,
api.SSL/body, sslPayload, opts.ID and ggID when implementing the conditional
update and error handling.
Comment on lines
+17
to
+28
| func isKnownServiceTemplateCapabilityGap(stdout, stderr string) bool { | ||
| combined := strings.ToLower(stdout + "\n" + stderr) | ||
| has404 := strings.Contains(combined, "api error (status 404)") || | ||
| strings.Contains(combined, "resource not found") || | ||
| strings.Contains(combined, "/404") | ||
| isServiceTemplate := strings.Contains(combined, "service-template") || | ||
| strings.Contains(combined, "/api/services/template") || | ||
| strings.Contains(combined, "service template") | ||
| // This helper is only called for service-template commands. Newer API7 EE | ||
| // builds can return a generic 404 body without echoing the removed endpoint. | ||
| return has404 || isServiceTemplate | ||
| } |
Comment on lines
76
to
99
| if opts.File != "" { | ||
| payload, err := cmdutil.ReadResourceFile(opts.File, opts.IO.In) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if opts.ID != "" { | ||
| payload["id"] = opts.ID | ||
| payload["name"] = opts.ID | ||
| delete(payload, "id") | ||
| } | ||
|
|
||
| httpClient, err := opts.Client() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| path := "/apisix/admin/consumers/" + opts.Consumer + "/credentials?gateway_group_id=" + ggID | ||
| client := api.NewClient(httpClient, cfg.BaseURL()) | ||
| var body []byte | ||
| if id, ok := payload["id"]; ok { | ||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, id, ggID), payload) | ||
| if name, ok := payload["name"]; ok { | ||
| body, err = client.Put(fmt.Sprintf("/apisix/admin/consumers/%s/credentials/%v?gateway_group_id=%s", opts.Consumer, name, ggID), payload) | ||
| } else { | ||
| body, err = client.Post(path, payload) | ||
| } |
Contributor
Author
|
Follow-up on latest review feedback:
I intentionally did not implement type-level |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@pkg/api/types_ssl.go`:
- Around line 14-31: The SSL.Key field is secret and standalone helpers
(RedactSSL/RedactSSLs) are insufficient; modify the types so
display/JSON/logging always redact: introduce a separate API-write type (e.g.,
SSLRequest) that holds the real Key, keep or create a display type (e.g., SSL or
SSLResponse) used for CLI/logs and implement String(), MarshalJSON(), and
MarshalLogObject() on that display type to replace Key with RedactedSSLKey;
update code paths to use SSLRequest for inbound API writes and the redacting
display type for any output/serialization, ensuring method receiver choices
match the guidelines (pointer vs value) and removing reliance on callers to call
RedactSSL/RedactSSLs.
In `@pkg/cmd/ssl/create/create.go`:
- Around line 194-203: The function looksLikePath incorrectly treats non-ENOENT
os.Stat errors as "not a path" so maybeReadFile then misinterprets a bare
filename as literal PEM; change looksLikePath to return true for any os.Stat
error so permission/other stat errors get propagated: specifically, update
looksLikePath (after calling os.Stat) to return !info.IsDir() when err == nil,
and otherwise return true (so ENOENT and other errors are treated as a path and
let maybeReadFile attempt to read and surface the OS error).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee294ee1-98d2-40de-9fe7-ade8fc068330
📒 Files selected for processing (10)
pkg/api/types_ssl.gopkg/cmd/credential/create/create.gopkg/cmd/credential/create/create_test.gopkg/cmd/ssl/create/create.gopkg/cmd/ssl/get/get.gopkg/cmd/ssl/list/list.gopkg/cmd/ssl/update/update.gopkg/cmd/ssl/update/update_test.gopkg/httpmock/httpmock.gotest/e2e/service_template_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/httpmock/httpmock.go
- pkg/cmd/ssl/update/update_test.go
- test/e2e/service_template_test.go
- pkg/cmd/credential/create/create.go
- pkg/cmd/ssl/update/update.go
Comment on lines
+14
to
+31
| const RedactedSSLKey = "<redacted>" | ||
|
|
||
| // RedactSSL returns a copy safe for CLI output. | ||
| func RedactSSL(ssl SSL) SSL { | ||
| if ssl.Key != "" { | ||
| ssl.Key = RedactedSSLKey | ||
| } | ||
| return ssl | ||
| } | ||
|
|
||
| // RedactSSLs returns copies safe for CLI output. | ||
| func RedactSSLs(ssls []SSL) []SSL { | ||
| redacted := make([]SSL, len(ssls)) | ||
| for i, ssl := range ssls { | ||
| redacted[i] = RedactSSL(ssl) | ||
| } | ||
| return redacted | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
SSL struct must implement MarshalJSON()/String() redaction per coding guidelines — standalone helpers are opt-in only.
SSL.Key is a secret-bearing field. The standalone RedactSSL/RedactSSLs helpers require every output site to explicitly opt in; any direct json.Marshal(ssl) call (e.g., in a future command, a logger, or a fmt.Sprintf("%+v", ...)) silently leaks the private key.
The coding guidelines mandate that the struct itself enforce redaction via String(), MarshalJSON(), and MarshalLogObject() methods. The typical solution where the same struct is used for API writes (which need the real key) is to split into separate types — e.g. an SSLRequest for API writes and an SSL/SSLResponse for read/display — so that MarshalJSON() on the display type always redacts.
As per coding guidelines: "All secret-bearing structs must implement proper redaction in String(), MarshalJSON(), and MarshalLogObject() methods; verify pointer receiver vs value receiver matches."
🤖 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 `@pkg/api/types_ssl.go` around lines 14 - 31, The SSL.Key field is secret and
standalone helpers (RedactSSL/RedactSSLs) are insufficient; modify the types so
display/JSON/logging always redact: introduce a separate API-write type (e.g.,
SSLRequest) that holds the real Key, keep or create a display type (e.g., SSL or
SSLResponse) used for CLI/logs and implement String(), MarshalJSON(), and
MarshalLogObject() on that display type to replace Key with RedactedSSLKey;
update code paths to use SSLRequest for inbound API writes and the redacting
display type for any output/serialization, ensuring method receiver choices
match the guidelines (pointer vs value) and removing reliance on callers to call
RedactSSL/RedactSSLs.
Comment on lines
+177
to
+180
| func routePayload(route api.Route, opts *Options) (interface{}, error) { | ||
| if !opts.StatusSet && !opts.PrioritySet { | ||
| return route, nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A7_TOKENincontext createwhen--tokenis omitted, without copying tokens from the previously active context.name, secret provider flag mode uses--provider-tokenand idempotentPUT, route/stream-route list/export require--service-id.status: 0.Tests
go test ./...Closes #15
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Documentation