feat(config-validate): add --remote flag for live backend validation#65
Open
shreemaan-abhishek wants to merge 1 commit into
Open
feat(config-validate): add --remote flag for live backend validation#65shreemaan-abhishek wants to merge 1 commit into
shreemaan-abhishek wants to merge 1 commit into
Conversation
`a7 config validate --remote` posts the parsed config to `/apisix/admin/configs/validate?gateway_group_id=<id>` so API7 EE can report errors caught by the live backend schema (plugin-config shapes, cross-resource references, etc) on top of a7's local checks. Local errors short-circuit before any HTTP call so the remote step never sees a known-bad input. Each remote error is prefixed with [remote] to distinguish it from local validation output. Endpoint shape inferred from adc's backend-api7 validator (request body keys, error array). Implementation issues a raw HTTP request instead of going through api.Client.Post so the 400 body's structured `errors[]` array is preserved instead of being collapsed into a single error string.
📝 WalkthroughWalkthroughThis PR adds optional remote validation to the config validate command. It introduces new HTTP client logic to call API7 EE's dry-run endpoint, integrates the remote validation into the command flow behind a ChangesRemote Configuration Validation
Sequence DiagramsequenceDiagram
participant CLI as CLI / validateRun
participant Local as Local Validator
participant Remote as Remote Client
participant EE as API7 EE Endpoint
CLI->>Local: ValidateConfigFile(config)
Local-->>CLI: nil or local errors
alt local errors
CLI-->>CLI: return early
else local success & opts.Remote
CLI->>Remote: runRemoteValidation(opts)
Remote->>Remote: buildRemoteValidateBody(config)
Remote->>EE: POST /apisix/admin/configs/validate (JSON)
EE-->>Remote: 400 or 2xx with body
alt status 2xx
Remote-->>CLI: nil
else status 400
Remote->>Remote: unmarshal & formatRemoteError for each entry
Remote-->>CLI: aggregated errors with [remote] prefix
end
end
alt all validations pass
CLI-->>CLI: print "Config is valid"
else any validation failed
CLI-->>CLI: return error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/config/validate/remote.go`:
- Around line 85-128: validateRemote mixes transport/serialization failures and
non-400 HTTP errors into the []string validation slice; change its API to return
([]string, error) so operational failures (json.Marshal, http.NewRequest,
httpClient.Do, io.ReadAll, and any non-2xx non-400 status) return a non-nil
error while only a 400 with a parsed remoteValidationResponse returns ([]string,
nil); update validateRemote to return detailed errors (wrap context) for
marshal/build/Do/read and non-400 statuses, keep the existing parsing/unmarshal
path to return the per-resource []string, and then update the caller
runRemoteValidation to handle the new (results, err) tuple and treat err as
"couldn't reach/use remote" and results as schema rejections.
- Around line 88-91: The URL is built by string concatenation which allows
reserved chars in gatewayGroup to break the request; update the construction
around the url variable (the baseURL + "/apisix/admin/configs/validate" block)
to use net/url and url.Values: create values := url.Values{},
values.Set("gateway_group_id", gatewayGroup) when gatewayGroup is non-empty,
then append "?" + values.Encode() (or build a url.URL and set RawQuery) so
gatewayGroup is properly escaped instead of concatenated.
In `@pkg/cmd/config/validate/validate.go`:
- Around line 114-115: The call to validateRemote is not passing the auth token,
so modify the call site in validate.go to forward configReader.Token() along
with BaseURL() and GatewayGroup() (i.e., call validateRemote(httpClient,
configReader.BaseURL(), cfg, configReader.GatewayGroup(),
configReader.Token())). Then update the validateRemote function signature in
pkg/cmd/config/validate/remote.go to accept the token parameter and ensure the
implementation sets the Authorization header (or appropriate auth header) on the
outgoing request before dispatching to the remote admin validate endpoint.
🪄 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: 775bd14b-4a6f-49f0-86b8-5721ffa930c8
📒 Files selected for processing (3)
pkg/cmd/config/validate/remote.gopkg/cmd/config/validate/validate.gopkg/cmd/config/validate/validate_test.go
Comment on lines
+85
to
+128
| func validateRemote(httpClient *http.Client, baseURL string, cfg api.ConfigFile, gatewayGroup string) []string { | ||
| body := buildRemoteValidateBody(cfg) | ||
|
|
||
| url := baseURL + "/apisix/admin/configs/validate" | ||
| if gatewayGroup != "" { | ||
| url += "?gateway_group_id=" + gatewayGroup | ||
| } | ||
|
|
||
| encoded, err := json.Marshal(body) | ||
| if err != nil { | ||
| return []string{fmt.Sprintf("failed to encode remote validate request: %v", err)} | ||
| } | ||
|
|
||
| req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(encoded)) | ||
| if err != nil { | ||
| return []string{fmt.Sprintf("failed to build remote validate request: %v", err)} | ||
| } | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := httpClient.Do(req) | ||
| if err != nil { | ||
| return []string{fmt.Sprintf("remote validation request failed: %v", err)} | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| respBody, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return []string{fmt.Sprintf("failed to read remote validate response: %v", err)} | ||
| } | ||
|
|
||
| if resp.StatusCode >= 200 && resp.StatusCode < 300 { | ||
| return nil | ||
| } | ||
|
|
||
| // Only 400 carries the structured per-resource error list. For other | ||
| // non-2xx statuses surface the raw body so auth or availability issues | ||
| // aren't swallowed. | ||
| if resp.StatusCode != http.StatusBadRequest { | ||
| return []string{fmt.Sprintf("remote validation failed (status %d): %s", resp.StatusCode, string(respBody))} | ||
| } | ||
|
|
||
| var parsed remoteValidationResponse | ||
| if jsonErr := json.Unmarshal(respBody, &parsed); jsonErr != nil { | ||
| return []string{fmt.Sprintf("remote validation failed (status 400): %s", string(respBody))} |
There was a problem hiding this comment.
Propagate transport/setup failures through error, not the validation slice.
validateRemote currently turns marshal/build/Do/read/non-400 failures into []string. That breaks the contract used by runRemoteValidation in pkg/cmd/config/validate/validate.go, Lines 97-115, where error means “couldn’t reach/use the remote” and []string means “remote schema rejected the config.” As written, auth/network/404/5xx paths are reported as [remote] validation failures instead of operational errors.
Suggested direction
-func validateRemote(httpClient *http.Client, baseURL string, cfg api.ConfigFile, gatewayGroup string) []string {
+func validateRemote(httpClient *http.Client, baseURL string, cfg api.ConfigFile, gatewayGroup string) ([]string, error) {
body := buildRemoteValidateBody(cfg)
encoded, err := json.Marshal(body)
if err != nil {
- return []string{fmt.Sprintf("failed to encode remote validate request: %v", err)}
+ return nil, fmt.Errorf("failed to encode remote validate request: %w", err)
}
req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(encoded))
if err != nil {
- return []string{fmt.Sprintf("failed to build remote validate request: %v", err)}
+ return nil, fmt.Errorf("failed to build remote validate request: %w", err)
}
resp, err := httpClient.Do(req)
if err != nil {
- return []string{fmt.Sprintf("remote validation request failed: %v", err)}
+ return nil, fmt.Errorf("remote validation request failed: %w", err)
}
respBody, err := io.ReadAll(resp.Body)
if err != nil {
- return []string{fmt.Sprintf("failed to read remote validate response: %v", err)}
+ return nil, fmt.Errorf("failed to read remote validate response: %w", err)
}
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
- return nil
+ return nil, nil
}
if resp.StatusCode != http.StatusBadRequest {
- return []string{fmt.Sprintf("remote validation failed (status %d): %s", resp.StatusCode, string(respBody))}
+ return nil, fmt.Errorf("remote validation failed (status %d): %s", resp.StatusCode, string(respBody))
}
// keep returning []string only for decoded 400 validation errors
- return out
+ return out, nil
}As per coding guidelines, **/*.go: Never suppress errors. Always handle and propagate errors explicitly.
🤖 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/config/validate/remote.go` around lines 85 - 128, validateRemote
mixes transport/serialization failures and non-400 HTTP errors into the []string
validation slice; change its API to return ([]string, error) so operational
failures (json.Marshal, http.NewRequest, httpClient.Do, io.ReadAll, and any
non-2xx non-400 status) return a non-nil error while only a 400 with a parsed
remoteValidationResponse returns ([]string, nil); update validateRemote to
return detailed errors (wrap context) for marshal/build/Do/read and non-400
statuses, keep the existing parsing/unmarshal path to return the per-resource
[]string, and then update the caller runRemoteValidation to handle the new
(results, err) tuple and treat err as "couldn't reach/use remote" and results as
schema rejections.
Comment on lines
+88
to
+91
| url := baseURL + "/apisix/admin/configs/validate" | ||
| if gatewayGroup != "" { | ||
| url += "?gateway_group_id=" + gatewayGroup | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section of the file with line numbers
sed -n '1,200p' pkg/cmd/config/validate/remote.go | nl -ba | sed -n '1,200p'
# Focus around the cited lines 70-120
sed -n '60,130p' pkg/cmd/config/validate/remote.go | nl -ba
# Find validateRemote signature and any URL parsing/values usage in the file
rg -n "func validateRemote|validateRemote\(" -n pkg/cmd/config/validate/remote.go
rg -n "baseURL|gatewayGroup|gateway_group_id|url\.Values|net/url|Query\(" pkg/cmd/config/validate/remote.goRepository: api7/a7
Length of output: 94
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section of the file with line numbers (cat -n is available)
sed -n '1,220p' pkg/cmd/config/validate/remote.go | cat -n
# Focus around the cited lines 70-120 (with cat -n to keep line numbers)
sed -n '60,130p' pkg/cmd/config/validate/remote.go | cat -n
# Find validateRemote signature and URL parsing usage
rg -n "func validateRemote|validateRemote\(" pkg/cmd/config/validate/remote.go
rg -n "baseURL|gatewayGroup|gateway_group_id|net/url|url\.Values|Query\(" pkg/cmd/config/validate/remote.goRepository: api7/a7
Length of output: 9893
Escape gateway_group_id instead of concatenating it into the URL.
url := baseURL + "/apisix/admin/configs/validate"
if gatewayGroup != "" {
url += "?gateway_group_id=" + gatewayGroup
}
Build the query using net/url + url.Values (e.g., values.Set("gateway_group_id", gatewayGroup); ... + "?" + values.Encode()) so reserved characters in gatewayGroup can’t break or alter the request target.
🤖 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/config/validate/remote.go` around lines 88 - 91, The URL is built by
string concatenation which allows reserved chars in gatewayGroup to break the
request; update the construction around the url variable (the baseURL +
"/apisix/admin/configs/validate" block) to use net/url and url.Values: create
values := url.Values{}, values.Set("gateway_group_id", gatewayGroup) when
gatewayGroup is non-empty, then append "?" + values.Encode() (or build a url.URL
and set RawQuery) so gatewayGroup is properly escaped instead of concatenated.
Comment on lines
+114
to
+115
| return validateRemote(httpClient, configReader.BaseURL(), cfg, configReader.GatewayGroup()), nil | ||
| } |
There was a problem hiding this comment.
Pass auth context into remote validate dispatch.
Line 114 forwards only BaseURL() and GatewayGroup(). The remote admin validate endpoint is authenticated; not threading Token() into the request builder can make --remote fail (401/forbidden) on real EE environments.
Suggested fix
- return validateRemote(httpClient, configReader.BaseURL(), cfg, configReader.GatewayGroup()), nil
+ return validateRemote(
+ httpClient,
+ configReader.BaseURL(),
+ configReader.Token(),
+ cfg,
+ configReader.GatewayGroup(),
+ ), nilThen update validateRemote(...) in pkg/cmd/config/validate/remote.go to accept/use the token when setting request headers.
🤖 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/config/validate/validate.go` around lines 114 - 115, The call to
validateRemote is not passing the auth token, so modify the call site in
validate.go to forward configReader.Token() along with BaseURL() and
GatewayGroup() (i.e., call validateRemote(httpClient, configReader.BaseURL(),
cfg, configReader.GatewayGroup(), configReader.Token())). Then update the
validateRemote function signature in pkg/cmd/config/validate/remote.go to accept
the token parameter and ensure the implementation sets the Authorization header
(or appropriate auth header) on the outgoing request before dispatching to the
remote admin validate endpoint.
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
1 participant
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 config validate --remoteposts the parsed config toPOST /apisix/admin/configs/validate?gateway_group_id=<id>so API7 EE can report errors caught by the live backend schema (plugin-config shapes, cross-resource references, etc) on top of a7's local checks.syncvalidation gating).[remote]to distinguish from local output.api.Client.Postso the 400 body's structurederrors[]array is preserved (api.Client.dowould collapse it into a single error string).Part of the adc → a7 parity gap tracked in
docs/adc-test-parity-plan.md.Endpoint contract
POST /apisix/admin/configs/validate?gateway_group_id=<id>with flat JSON body containingroutes,services,consumers,ssls,global_rules,stream_routes,plugin_metadata. Inferred fromadc/libs/backend-api7/src/validator.ts(request body shape) andadc/.../e2e/validate.e2e-spec.ts(success/failure semantics, confirms dry-run).Test plan
TestConfigValidate_Remote_HappyPathTestConfigValidate_Remote_CollectsErrorsTestConfigValidate_Remote_LocalErrorsSkipRemoteTestConfigValidate_NoRemoteFlag_BehavesAsBeforego test ./pkg/... ./internal/...passesgo vet ./...;gofmt -l .cleanCaveat
Endpoint shape is inferred, not confirmed against API7 EE docs. If the real admin API differs, the contract change is localized to
validate/remote.go. adc gates this feature on API7 EE >= 3.9.10; a7 currently surfaces an older-version 404 as-is rather than checking version first.Summary by CodeRabbit
--remoteflag to the config validate command for validation against API7 EE backend.[remote]prefix.