docs(converter-openapi): align README extension table with code#447
Merged
Conversation
|
|
|
test seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR edits the OpenAPI converter README: it removes Path Level from ChangesExtension Fields Documentation
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Two long-standing inaccuracies in the supported-extensions table: 1. x-adc-labels was documented at Root / Path / Operation levels but the Zod schema in src/schema.ts only defines it at the root document and on each operation object. There is no entry for x-adc-labels on the path-item schema, and src/index.ts only reads oas[ExtKey.LABELS] (root, line 44) and operation[ExtKey.LABELS] (operation, line 95). Path-level x-adc-labels passes loose-object validation but is then silently dropped at conversion. Drop the Path row from the table. 2. x-adc-plugins was documented as causing the service to be split when present at Path or Operation level. The actual splitting condition in src/parser.ts parseSeprateService (lines 60-65) only fires on x-adc-service-defaults, x-adc-upstream-defaults, or a sub-level servers: entry. Plugins are merged into route.plugins directly in src/index.ts (lines 82-85, 102) without invoking parseSeprateService. Test fixtures extension-5.yaml and extension-6.yaml both produce a single service in the output despite carrying plugins at every level. Update the description to match. Fixes #446
…the service Third inaccuracy in the same table: x-adc-route-defaults was described as 'This field on sub-levels will cause the service to be split.' The splitting condition in parseSeprateService (src/parser.ts lines 60-65) only checks SERVICE_DEFAULTS, UPSTREAM_DEFAULTS, and sub-level servers: — not ROUTE_DEFAULTS. The field is merged into each route via structuredClone spreads in src/index.ts lines 103-105 (root -> path -> operation order, operation wins). Verified against test/assets/extension-7.yaml: the GET service split in that fixture is caused by an operation-level servers: block, not by x-adc-route-defaults; the PUT operation keeps its merged route-defaults on the main service without splitting.
kayx23
force-pushed
the
fix/converter-openapi-readme-drift
branch
from
June 16, 2026 06:06
d9d806e to
dbe7926
Compare
bzp2010
approved these changes
Jun 16, 2026
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
3 participants
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.
Fixes #446.
Three long-standing inaccuracies in
libs/converter-openapi/README.md's supported-extensions table, identified by cross-checking each row againstsrc/schema.ts,src/parser.ts,src/index.ts,src/extension.ts, and the fixtures undertest/assets/.1.
x-adc-labelswas documented at Path level but is not honored thereThe Zod schema only defines
x-adc-labelson the root document and on each operation object — there is no entry for it in the path-item schema:https://github.com/api7/adc/blob/main/libs/converter-openapi/src/schema.ts#L58-L75
The path-item is a
z.looseObject, so a path-levelx-adc-labels: {...}passes validation, butsrc/index.tsnever readsoperations[ExtKey.LABELS]— onlyoas[ExtKey.LABELS](root) andoperation[ExtKey.LABELS](operation) are mapped through:https://github.com/api7/adc/blob/main/libs/converter-openapi/src/index.ts#L44
https://github.com/api7/adc/blob/main/libs/converter-openapi/src/index.ts#L95
Result: path-level labels are silently dropped at conversion. Dropping the Path row from the table.
2.
x-adc-pluginsat Path/Operation was documented as causing a service split, but does notThe actual splitting condition in
parseSeprateServiceonly fires onx-adc-service-defaults,x-adc-upstream-defaults, or a sub-levelservers:entry:https://github.com/api7/adc/blob/main/libs/converter-openapi/src/parser.ts#L60-L65
Plugins are merged into
route.pluginsdirectly insrc/index.ts(lines 82-85, 102) without invokingparseSeprateService. The test fixturesextension-5.yamlandextension-6.yamlboth produce a single service in the output despite carrying plugins at root, path, and operation levels. Rewriting the description to match.3.
x-adc-route-defaultswas documented as causing a service split, but does notSame root cause as #2 — the field is not on the splitting checklist in
parseSeprateService. It is merged into each route viastructuredClonespreads insrc/index.ts:https://github.com/api7/adc/blob/main/libs/converter-openapi/src/index.ts#L103-L105
The order is root → path → operation, with operation winning. Verified against
test/assets/extension-7.yaml: the GET-only service split in that fixture is caused by an operation-levelservers:block, not byx-adc-route-defaults; the PUT operation keeps its merged route-defaults on the main service without splitting. Updating the description.Note for reviewers
I chose to fix the README rather than the code because the current behavior (no splitting on plugins or route-defaults; no path-level labels) looks intentional and the test suite encodes it. If any of these is actually a wanted feature, this PR can become a starting point for that conversation — happy to take the other route.
Context
Surfaced while writing downstream documentation in api7/docs#1638. Posting the README fix here so the upstream source of truth stops misleading users.
Summary by CodeRabbit
x-adc-labelsis supported only at Root and Operation levels (Path level removed).x-adc-pluginswording to remove the previous claim that Path/Operation-level plugins trigger service splitting.x-adc-route-defaultsto clarify it does not by itself cause service splitting.