Skip to content

feat(apisix): add validate command support for APISIX and standalone backends#434

Open
jarvis9443 wants to merge 4 commits intomainfrom
feat/apisix-validate
Open

feat(apisix): add validate command support for APISIX and standalone backends#434
jarvis9443 wants to merge 4 commits intomainfrom
feat/apisix-validate

Conversation

@jarvis9443
Copy link
Copy Markdown
Contributor

@jarvis9443 jarvis9443 commented Apr 20, 2026

Add validate() support to both APISIX and APISIX Standalone backends, sharing a common Validator class in backend-apisix.

Changes

  • Create shared Validator class in libs/backend-apisix/src/validator.ts
    • Transforms ADC events to APISIX validate request body
    • Handles upstreams as separate resources (APISIX extracts them from services)
    • Posts to /apisix/admin/configs/validate and processes 200/400 responses
    • Returns user-friendly error when endpoint doesn't exist (404)
  • Add validate() method to BackendAPISIX
  • Add validate() method to BackendAPISIXStandalone (imports Validator from backend-apisix)
  • Add e2e test suites for both backends (gated behind semverCondition(gte, '3.17.0'))
  • Update CI matrix: add APISIX 3.15.0, 3.16.0, and dev to both apisix and standalone jobs

Notes

  • The /apisix/admin/configs/validate endpoint exists on APISIX master but not in any released version (3.2.2–3.16.0). Tests are version-gated and will auto-enable when 3.17.0 releases.
  • The standalone backend reuses the same Validator via @api7/adc-backend-apisix workspace dependency.

Summary by CodeRabbit

  • New Features

    • Added a validate API to run configuration validation for APISIX resources; returns detailed per‑resource error reports and preserves dry‑run semantics.
  • Tests

    • Added comprehensive end‑to‑end tests covering successful validations, multiple failure modes (single/multiple errors, invalid types), mixed resource sets, and dry‑run behavior.
  • Chores

    • CI matrix expanded to test additional APISIX versions; workspace dependency added.

Add server-side configuration validation for both APISIX and APISIX
Standalone backends, reusing the same /apisix/admin/configs/validate
endpoint.

- Create shared Validator class in backend-apisix that handles
  transforming ADC events to APISIX native format and calling the
  validate API
- backend-apisix-standalone imports and reuses the Validator with
  custom request config (baseURL + auth header)
- Handle APISIX-specific transformService tuple return which produces
  separate upstream resources
- Add e2e test suites for both backends
@jarvis9443 jarvis9443 requested a review from bzp2010 as a code owner April 20, 2026 02:39
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

Introduces a Validator that builds APISIX validation requests from ADC events, exposes validate(events) on BackendAPISIX and BackendAPISIXStandalone (returning Observables), adds semver-gated E2E tests for validation scenarios, updates CI matrix versions, and adds a workspace dependency for the standalone backend.

Changes

Cohort / File(s) Summary
Validator Core
libs/backend-apisix/src/validator.ts
New Validator and ValidatorOptions. Builds validate request bodies from CREATE/UPDATE ADC events, POSTs to /apisix/admin/configs/validate, emits debug events, maps validation errors to resource names, and returns structured BackendValidateResult.
Backend APIs
libs/backend-apisix/src/index.ts, libs/backend-apisix-standalone/src/index.ts
Added public validate(events: Array<ADCSDK.Event>) on both backends. Each constructs a Validator (client + subject + optional requestConfig), calls validate, wraps result with from(...) and returns an Observable. Also re-exports Validator from libs/backend-apisix.
E2E Tests
libs/backend-apisix/e2e/validate.e2e-spec.ts, libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts
New semver-gated Jest E2E suites using configToEvents (via DifferV3.diff). Tests cover empty configs, valid services/routes/consumers, invalid plugin/route cases, multiple aggregated errors, mixed-resource validation, and dry-run semantics.
Package
libs/backend-apisix-standalone/package.json
Added workspace dependency: @api7/adc-backend-apisix.
CI Matrix
.github/workflows/e2e.yaml
Expanded APISIX job matrix to include 3.15.0 and 3.16.0; moved BACKEND_APISIX_VERSION/BACKEND_APISIX_IMAGE into a step that writes to GITHUB_ENV, with special-case handling for dev.

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Backend as BackendAPISIX / Standalone
    participant Validator as Validator
    participant APISIX as APISIX Server

    Test->>Backend: validate(events)
    Backend->>Validator: new Validator({ client, eventSubject, requestConfig })
    Backend->>Validator: validate(events)
    Validator->>Validator: buildRequestBody(events)
    Validator->>APISIX: POST /apisix/admin/configs/validate
    APISIX-->>Validator: 200 OK or 400 Validation Error
    Validator->>Validator: map errors to resource_name via nameIndex
    Validator-->>Backend: BackendValidateResult(success, errors, errorMessage)
    Backend->>Test: Observable<BackendValidateResult>
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • nic-6443
  • bzp2010
  • guoqqqi

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Security Check ❓ Inconclusive Unable to access the requested validator.ts file; source code inspection requires direct repository access. Provide the file path or repository details to enable security review of AXIOS_DEBUG event emissions and request header handling.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(apisix): add validate command support for APISIX and standalone backends' clearly and accurately summarizes the main change across the entire changeset, which introduces validate() methods and supporting infrastructure for both APISIX and APISIX Standalone backends.
E2e Test Quality Review ✅ Passed PR introduces well-structured E2E tests for validate() functionality across APISIX and APISIX Standalone backends that comprehensively meet all quality criteria.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/apisix-validate

Comment @coderabbitai help to get the list of available commands and usage tips.

The configs/validate endpoint is not available in released APISIX
versions yet (only on master). Skip validate e2e tests on APISIX
versions below 3.15.0, and handle 404 responses gracefully with a
user-friendly error message.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts (1)

12-17: Consider edge case: configToEvents with undefined/null input.

The helper casts inputs directly without null checks. While the test suite controls all inputs, adding defensive handling would improve robustness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts` around lines 12 -
17, The helper configToEvents currently casts its input without null checks
which can blow up if config is undefined/null; update the function
(configToEvents) to first guard against null/undefined (e.g., if config == null
return an empty Array<ADCSDK.Event>) and only call DifferV3.diff when config is
present, preserving the current cast to ADCSDK.InternalConfiguration for the
non-null path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts`:
- Around line 31-35: The test in validate.e2e-spec.ts fails because
backend.validate([]) calls a nonstandard /apisix/admin/configs/validate
endpoint; update the test or the backend.validate implementation to use the
correct APISIX schema validation endpoint (e.g.,
/apisix/admin/schema/validate/{resource}) or make the path configurable based on
APISIX version/image; specifically, change the call-site in the it block or
adjust the backend.validate method to resolve the endpoint dynamically (or mock
the validation call) so the E2E run uses the supported endpoint for the APISIX
instance under test.

In `@libs/backend-apisix/src/validator.ts`:
- Around line 110-121: The code mutates event.newValue by assigning id directly
(inside the ADCSDK.ResourceType.SERVICE case before calling
this.fromADC.transformService), which can produce unintended side effects; fix
it by creating a shallow copy of event.newValue (e.g., const svc = {
...(event.newValue as ADCSDK.Service), id: event.resourceId }) and pass that
copy to this.fromADC.transformService, then push the returned service/upstream
as before; apply the same pattern in the ROUTE, STREAM_ROUTE, and SSL branches
so you never modify event.newValue in-place.
- Around line 160-166: The pushed global rule is missing the required id
property and is using an unsafe cast; update the object in the
body.global_rules.push call to include id: event.resourceId and remove the "as
unknown as" cast so the object conforms to typing.GlobalRule (mirror how
SERVICE/ROUTE/STREAM_ROUTE/PLUGIN_METADATA set id); ensure
nameIndex.global_rules.push(event.resourceName) remains unchanged.

---

Nitpick comments:
In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts`:
- Around line 12-17: The helper configToEvents currently casts its input without
null checks which can blow up if config is undefined/null; update the function
(configToEvents) to first guard against null/undefined (e.g., if config == null
return an empty Array<ADCSDK.Event>) and only call DifferV3.diff when config is
present, preserving the current cast to ADCSDK.InternalConfiguration for the
non-null path.
🪄 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: af799504-33c1-499d-8ee4-9c87356def22

📥 Commits

Reviewing files that changed from the base of the PR and between f01631a and d81071b.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts
  • libs/backend-apisix-standalone/package.json
  • libs/backend-apisix-standalone/src/index.ts
  • libs/backend-apisix/e2e/validate.e2e-spec.ts
  • libs/backend-apisix/src/index.ts
  • libs/backend-apisix/src/validator.ts

Comment thread libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts
Comment thread libs/backend-apisix/src/validator.ts
Comment thread libs/backend-apisix/src/validator.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
libs/backend-apisix/src/validator.ts (1)

165-168: ⚠️ Potential issue | 🔴 Critical

global_rules entries still miss the required id.

This branch is still building a non-conforming typing.GlobalRule and hiding it behind as unknown as. Add id: event.resourceId and drop the cast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix/src/validator.ts` around lines 165 - 168, The GLOBAL_RULE
case is constructing a typing.GlobalRule without the required id and using an
unsafe cast; update the ADCSDK.ResourceType.GLOBAL_RULE branch so the object
pushed to body.global_rules includes id: event.resourceId and the plugins field
set to { [event.resourceId]: event.newValue }, and remove the "as unknown as
typing.GlobalRule" cast so a properly typed object is added (i.e., push { id:
event.resourceId, plugins: { [event.resourceId]: event.newValue } }).
🧹 Nitpick comments (1)
libs/backend-apisix/e2e/validate.e2e-spec.ts (1)

17-18: The new 404 fallback still needs coverage somewhere.

Because this suite is gated to >= 3.15.0, the “validate not supported” branch can regress silently. Please add a small mocked test around Validator.validate() that asserts the thrown message on a 404.

Based on learnings "E2E tests must cover boundary cases (empty values, min/max), invalid inputs, combination scenarios, and extreme cases (high load, failures)".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix/e2e/validate.e2e-spec.ts` around lines 17 - 18, The suite
is gated to >=3.15.0 so the new 404 fallback path in Validator.validate() can
regress unnoticed; add a unit-style test that mocks/stubs Validator.validate()
to simulate a 404 response and asserts the thrown/rejected error message matches
the expected fallback text. Specifically, inside or alongside the existing
conditionalDescribe block (or in a non-gated sibling test), create a small test
that uses your test framework’s mocking (e.g., jest.spyOn or similar) to make
Validator.validate() throw an error/object with status 404, then call the code
path that invokes Validator.validate() and assert the error message is the
fallback message; reference BackendAPISIX and Validator.validate() so the mock
targets the right symbol and ensure the test runs regardless of semverCondition
to protect the fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/backend-apisix/src/validator.ts`:
- Around line 46-49: The axios debug event currently forwards the raw response
(see subject.next publishing ADCSDK.BackendEventType.AXIOS_DEBUG with event: {
response: resp, ... }), which can leak auth headers and plugin payloads; before
calling this.subject.next in validate (and the similar block around lines
58-63), construct a sanitized payload that strips or masks sensitive fields from
resp (remove Authorization/X-API-KEY and other auth headers, redact
request.data/request.body and any plugin config fields, and keep only safe
primitives such as status, statusText, url/path, and optionally masked headers
and truncated timing info), then publish that sanitized object instead of the
raw resp. Ensure the masking logic is centralized or reused so both publish
sites use the same redact function.

---

Duplicate comments:
In `@libs/backend-apisix/src/validator.ts`:
- Around line 165-168: The GLOBAL_RULE case is constructing a typing.GlobalRule
without the required id and using an unsafe cast; update the
ADCSDK.ResourceType.GLOBAL_RULE branch so the object pushed to body.global_rules
includes id: event.resourceId and the plugins field set to { [event.resourceId]:
event.newValue }, and remove the "as unknown as typing.GlobalRule" cast so a
properly typed object is added (i.e., push { id: event.resourceId, plugins: {
[event.resourceId]: event.newValue } }).

---

Nitpick comments:
In `@libs/backend-apisix/e2e/validate.e2e-spec.ts`:
- Around line 17-18: The suite is gated to >=3.15.0 so the new 404 fallback path
in Validator.validate() can regress unnoticed; add a unit-style test that
mocks/stubs Validator.validate() to simulate a 404 response and asserts the
thrown/rejected error message matches the expected fallback text. Specifically,
inside or alongside the existing conditionalDescribe block (or in a non-gated
sibling test), create a small test that uses your test framework’s mocking
(e.g., jest.spyOn or similar) to make Validator.validate() throw an error/object
with status 404, then call the code path that invokes Validator.validate() and
assert the error message is the fallback message; reference BackendAPISIX and
Validator.validate() so the mock targets the right symbol and ensure the test
runs regardless of semverCondition to protect the fallback behavior.
🪄 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: 03a8bb10-160e-4b13-8dbc-a94a1a110d3d

📥 Commits

Reviewing files that changed from the base of the PR and between d81071b and a07e2cf.

📒 Files selected for processing (3)
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts
  • libs/backend-apisix/e2e/validate.e2e-spec.ts
  • libs/backend-apisix/src/validator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts

Comment thread libs/backend-apisix/src/validator.ts
@nic-6443 nic-6443 added the test/apisix-standalone Trigger the APISIX standalone test on the PR label Apr 20, 2026
Comment thread libs/backend-apisix/e2e/validate.e2e-spec.ts Outdated
…p semver to 3.17.0

- Add missing id field to global_rules in validator (required by APISIX schema)
- Add APISIX 3.15.0, 3.16.0, and dev to apisix backend CI matrix
- Add APISIX 3.15.0, 3.16.0 to standalone CI matrix
- Update validate test semver condition to gte 3.17.0 (validate endpoint
  not in any released version yet)
@jarvis9443 jarvis9443 changed the title feat(apisix,apisix-standalone): add validate command support feat(apisix): add validate command support for APISIX and standalone backends Apr 20, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/backend-apisix/src/validator.ts`:
- Around line 128-148: The code uses unsafe non-null assertions on
event.parentId when calling this.fromADC.transformRoute and transformStreamRoute
for ADCSDK.ResourceType.ROUTE and STREAM_ROUTE; add a defensive check that
event.parentId is defined before using it: if event.parentId is undefined, throw
or return a clear validation error that includes event.resourceName and
event.resourceId (or otherwise handle root-level routes if that is the intended
behavior), and only call this.fromADC.transformRoute(event.newValue as
ADCSDK.Route, event.parentId) / transformStreamRoute(...) after validating
parentId; also consider surfacing this check from validator.validate() or
documenting the required invariant.
🪄 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: a5c8a7aa-ff47-48af-8310-ac6b06df7f79

📥 Commits

Reviewing files that changed from the base of the PR and between a07e2cf and f8d5c67.

📒 Files selected for processing (4)
  • .github/workflows/e2e.yaml
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts
  • libs/backend-apisix/e2e/validate.e2e-spec.ts
  • libs/backend-apisix/src/validator.ts
✅ Files skipped from review due to trivial changes (1)
  • libs/backend-apisix/e2e/validate.e2e-spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts

Comment thread libs/backend-apisix/src/validator.ts
@nic-6443
Copy link
Copy Markdown

QA: #434validate for apisix & apisix-standalone backends

Branch tested: feat/apisix-validate @ f8d5c67
Backends: apisix (admin/etcd mode), apisix-standalone (config_provider: yaml + PUT /apisix/admin/configs)
APISIX images: apache/apisix:dev (has /apisix/admin/configs/validate); apache/apisix:3.16.0-debian / 3.16.0-ubuntu (released, used to verify version‑gate behavior)
ADC binary: built from PR via pnpm exec nx build cli

Scenarios

# Scenario apisix apisix-standalone Notes
01 empty config ✅ success ✅ success no events sent, returns immediately
02 valid service+route+consumer ✅ success ✅ success
03 invalid plugin (limit-count count=-1, time_window=-10) ✅ correctly rejected ✅ correctly rejected error tagged with route name
04 invalid route (uris: [123]) ✅ correctly rejected ✅ correctly rejected error tagged with route name
05 multi-error (3 bad routes) ✅ all 3 returned, named ✅ all 3 returned, named confirms multi-error aggregation works
06 mixed (service + route + consumer + global_rule) ✅ success ✅ success
07 dry-run check (validate then dump) ✅ no resources persisted ✅ no resources persisted confirms validate is side-effect free
08 unsupported version (3.16.0) ✅ user-friendly error: “Validate is not supported by the current APISIX version. Please upgrade to a newer version.” ⚠️ see note below

Sample error output (scenario 05, both backends)

Error: Configuration validation failed:
Configuration validation failed
  - [routes, name="bad-route-a"]: invalid configuration: property "uris" validation failed: failed to validate item 1: wrong type: expected string, got number
  - [routes, name="bad-route-b"]: failed to check the configuration of plugin limit-count err: property "count" validation failed: value should match only one schema, but matches none
  - [routes, name="bad-route-c"]: invalid configuration: property "uris" validation failed: failed to validate item 1: wrong type: expected string, got number

resource_name mapping (nameIndex) works; users get a meaningful pointer back to the offending resource in their adc.yaml rather than just index 0.

Notable finding — version gating is overly conservative for standalone mode

The PR description and e2e gate (semverCondition(gte, '3.17.0')) say the validate endpoint exists only on master. That is true for etcd / admin mode:

  • apache/apisix:3.16.0-debian etcd mode → POST /apisix/admin/configs/validate returns 404 {"error_msg":"Unsupported resource type: configs"} → ADC throws the expected user-friendly upgrade message ✅

But it is not true for standalone mode:

  • apache/apisix:3.16.0-ubuntu standalone mode → POST /apisix/admin/configs/validate returns 200 and actually validates (rejecting uris: [123] correctly).

So the standalone backend validate() is functional starting from APISIX 3.16.0, not 3.17.0. The semver gate in libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts could be relaxed (or split per backend) so 3.16.0 standalone runs the suite in CI.

Test environment caveat

The committed e2e standalone compose (libs/backend-apisix-standalone/e2e/assets/docker-compose.yaml) declares restart: always and points to etcd:2379 but doesn’t define an etcd service. When the host’s docker DNS returns temporary failure in name resolution (instead of NXDOMAIN), init_etcd exits 1 and the container restart-loops. CI happens to work because GitHub runners’ DNS gives a fast NXDOMAIN that doesn’t trip the retry path. For local QA I had to add a dummy etcd service to the compose. Consider adding etcd to the compose or removing the deployment.etcd block from the conf to make local runs deterministic.

Conclusion

Both backends’ validate() work as designed: the shared Validator correctly builds the request body, surfaces multi-error responses with resource names, gracefully degrades with a clear upgrade message on 404, and has no observable side effects. LGTM.

The validate endpoint is available in standalone mode (config_provider: yaml)
starting from APISIX 3.16.0, while etcd mode requires 3.17.0+.
Split the version gating accordingly.
@jarvis9443
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough QA!

Addressed the version gate finding — relaxed standalone e2e to gte 3.16.0 in ed238ce. The apisix (etcd mode) gate stays at 3.17.0.

Re the docker-compose etcd issue — that's pre-existing in the standalone test setup, not introduced here. Happy to fix it in a follow-up if needed.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts (3)

24-31: Consider per-test backend setup to reduce hidden order coupling.

Using one shared backend instance via beforeAll is efficient, but it can couple tests if backend state/caching behavior changes. beforeEach (or explicit reset) would keep cases fully independent.

Based on learnings: Applies to **/*.{test,spec,e2e,integration}.{js,ts,go} : Avoid explicit dependencies between tests and hidden execution order assumptions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts` around lines 24 -
31, The tests create a single shared BackendAPISIXStandalone instance in
beforeAll which can introduce hidden order coupling; change the setup to create
a fresh backend per test (use beforeEach to instantiate BackendAPISIXStandalone
into the backend variable with the same options including cacheKey and
defaultBackendOptions) or add an explicit reset method call on
BackendAPISIXStandalone between tests to clear state/caches; ensure references
to BackendAPISIXStandalone, backend, cacheKey and defaultBackendOptions are
updated so each spec runs with an isolated backend instance.

109-115: Strengthen invalid-case assertions to verify resource-name mapping and aggregation.

These checks currently assert only count/type. Since this suite validates user-facing error mapping, assert expected resource_name values for single-error and multi-error cases to lock in the intended behavior.

Proposed assertion upgrade
   expect(result.success).toBe(false);
-  expect(result.errors.length).toBeGreaterThan(0);
-  expect(result.errors[0].resource_type).toBe('routes');
+  expect(result.errors).toEqual(
+    expect.arrayContaining([
+      expect.objectContaining({
+        resource_type: 'routes',
+        resource_name: 'validate-bad-plugin-route',
+      }),
+    ]),
+  );
@@
   expect(result.success).toBe(false);
   expect(result.errors.length).toBeGreaterThanOrEqual(2);
+  expect(result.errors.map((e) => e.resource_name)).toEqual(
+    expect.arrayContaining([
+      'validate-multi-err-route1',
+      'validate-multi-err-route2',
+    ]),
+  );

Also applies to: 172-177

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts` around lines 109 -
115, The test only asserts result.success, errors.length and
errors[0].resource_type but must also verify that the validator maps and
aggregates resource names correctly; update the assertions after calling
backend.validate(configToEvents(config)) (variable result) to check expected
result.errors[0].resource_name for the single-error case and, for the
multi-error case (the other test around the same file), assert that the
set/array of result.errors.resource_name values contains the expected resource
names and that aggregation behavior (e.g., combined entries for the same
resource) matches the expected mapping; use the existing result and
result.errors references and add assertions that explicitly compare
resource_name values to the expected strings.

14-18: Consider extracting configToEvents helper to a shared utility or documenting why the ConfigurationInternalConfiguration cast is safe.

The cast at line 16 is used identically across all validate E2E tests (backend-api7, backend-apisix, and backend-apisix-standalone), suggesting it's a deliberate pattern. However, the type cast silently drops consumer_groups (present in Configuration, absent in InternalConfiguration). Since the current test configs don't exercise consumer_groups, the impact is minimal, but making this pattern explicit (via a documented utility or inline comment) would improve clarity and prevent future confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts` around lines 14 -
18, Extract the configToEvents logic into a shared test utility (e.g.,
tests/utils/configToEvents) or add an inline comment above the configToEvents
function explaining why the cast from ADCSDK.Configuration to
ADCSDK.InternalConfiguration is safe in these E2E tests; specifically mention
that consumer_groups is dropped by the cast, that current test fixtures don't
include consumer_groups, and that the helper centralizes this behavior for reuse
across backend-api7, backend-apisix, and backend-apisix-standalone. Update
references to call the shared configToEvents helper (or keep the local function
but include the explanatory comment) and ensure the helper uses DifferV3.diff
with the same config as ADCSDK.InternalConfiguration to preserve existing
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts`:
- Around line 24-31: The tests create a single shared BackendAPISIXStandalone
instance in beforeAll which can introduce hidden order coupling; change the
setup to create a fresh backend per test (use beforeEach to instantiate
BackendAPISIXStandalone into the backend variable with the same options
including cacheKey and defaultBackendOptions) or add an explicit reset method
call on BackendAPISIXStandalone between tests to clear state/caches; ensure
references to BackendAPISIXStandalone, backend, cacheKey and
defaultBackendOptions are updated so each spec runs with an isolated backend
instance.
- Around line 109-115: The test only asserts result.success, errors.length and
errors[0].resource_type but must also verify that the validator maps and
aggregates resource names correctly; update the assertions after calling
backend.validate(configToEvents(config)) (variable result) to check expected
result.errors[0].resource_name for the single-error case and, for the
multi-error case (the other test around the same file), assert that the
set/array of result.errors.resource_name values contains the expected resource
names and that aggregation behavior (e.g., combined entries for the same
resource) matches the expected mapping; use the existing result and
result.errors references and add assertions that explicitly compare
resource_name values to the expected strings.
- Around line 14-18: Extract the configToEvents logic into a shared test utility
(e.g., tests/utils/configToEvents) or add an inline comment above the
configToEvents function explaining why the cast from ADCSDK.Configuration to
ADCSDK.InternalConfiguration is safe in these E2E tests; specifically mention
that consumer_groups is dropped by the cast, that current test fixtures don't
include consumer_groups, and that the helper centralizes this behavior for reuse
across backend-api7, backend-apisix, and backend-apisix-standalone. Update
references to call the shared configToEvents helper (or keep the local function
but include the explanatory comment) and ensure the helper uses DifferV3.diff
with the same config as ADCSDK.InternalConfiguration to preserve existing
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6e4d21a9-e69a-4bfd-b81e-94dc073dadd0

📥 Commits

Reviewing files that changed from the base of the PR and between f8d5c67 and ed238ce.

📒 Files selected for processing (1)
  • libs/backend-apisix-standalone/e2e/validate.e2e-spec.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test/apisix-standalone Trigger the APISIX standalone test on the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants