feat(data-plane): support APISIX 3.16 for bound data planes#2837
feat(data-plane): support APISIX 3.16 for bound data planes#2837wklken wants to merge 4 commits into
Conversation
Add an apisix_version to DataPlane (default 3.13, optional 3.16) and propagate it through the release pipeline so every emitted APISIX resource carries a gateway.bk.tencent.com/apisix-version label that matches the bound data plane. This lets the operator validate each data plane against the right APISIX schema. Dashboard: - DataPlane.apisix_version model field, admin display/filter, and migration - thread apisix_version through GlobalApisixResourceTransformer and GatewayApisixResourceTransformer into the service/route/bk_release/ plugin_metadata convertors and the __apigw_version detect route Operator: - register an APISIX 3.16 JSON schema for resource and plugin validation, generated from apisix-core 3.16 while carrying over the BlueKing-curated main section and bk-* plugin schemas from 3.13
PR ReviewReview Scope
变更概览
历史评论核对
发现的问题未发现需要阻塞合并的 correctness / security / performance 问题。 Suggestion
本地验证
结论整体实现与 PR 描述一致,默认 3.13 的兼容路径保留,3.16 数据面的 label/schema 选择链路清晰;没有发现阻塞合并的问题。 [from-gpt local repo] |
This comment was marked as outdated.
This comment was marked as outdated.
…d dump Why this change was needed: The initial 3.16 schema was generated offline by replaying APISIX Lua schema assembly. It over-generated official plugins that are not enabled in production and, more importantly, lacked schemas for the scoped bk-* plugin names the dashboard actually writes to etcd at release time (e.g. bk-stage-rate-limit, bk-resource-header-rewrite, bk-permission). Both schemas also carried empty / _meta-only bk-* placeholders that enforce nothing. What changed: - Rebuilt 3.16 plugins and stream_plugins from the production /v1/schema dump, preserving the BlueKing-curated permissive main from 3.13 - Added the existing repo bk-* plugins the dump lacks (bk-header-rewrite, bk-rate-limit) so dashboard plugins still resolve - Dropped empty / _meta-only placeholder bk-* schemas from both 3.13 and 3.16 (bk-status-rewrite, bk-oauth2-protected-resource, bk-oauth2-verify, bk-oauth2-audience-validate, bk-username-required, bk-legacy-invalid-params); kept bk-traffic-label which has a real required:[rules] contract - Added tests asserting dashboard bk-* plugins resolve in every schema version and that bk-traffic-label rejects an empty config Problem solved: The operator now validates 3.16 resources against the plugin set actually deployed in production, while bk-* plugins without real contracts rely on skip-if-absent instead of meaningless empty schemas.
f9b6218 to
14b859c
Compare
Why this change was needed: Data planes can target different APISIX versions, so controller resource generation needs to use the version stored on the selected data plane instead of relying on a shared default. What changed: - Added an APISIX version option to data-plane creation. - Made resource transformers and convertors receive APISIX version explicitly. - Updated distributor call sites and tests to verify version propagation. Problem solved: Publishing and revoking gateway resources now labels generated APISIX resources with the data plane's configured version, keeping 3.13 and 3.16 data planes distinct. Co-authored-by: Cursor <cursoragent@cursor.com>
|
review again |
wklken
left a comment
There was a problem hiding this comment.
PR #2837 Code Review 汇总报告
由 codex-internal (gpt-5.4) + Claude Opus 4.7 双模型 review,主 agent 汇总整理。
仓库:TencentBlueKing/blueking-apigateway
PR 标题:feat(data-plane): support APISIX 3.16 for bound data planes
作者:wklken
目标分支:master
变更概述
本 PR 引入 APISIX 多版本(3.13 / 3.16)数据面支持,主要变更:
-
Dashboard(Python):
DataPlane模型新增apisix_version字段(默认3.13,可选3.16),含管理后台展示/过滤、迁移、create_data_plane命令参数。- 新增枚举
DataPlaneApisixVersionEnum(值为3.13/3.16)。 apisix_version通过GlobalApisixResourceTransformer/GatewayApisixResourceTransformer透传到 service / route /_bk_release/plugin_metadata转换器,并写入gateway.bk.tencent.com/apisix-versionlabel,以及__apigw_version探测路由的 mock 响应、bk_release模型字段。- 新增统一 label key 常量
LABEL_KEY_APISIX_VERSION(取代散落字面值)。
-
Operator(Go):
- 新增 APISIX 3.16 的 JSON Schema(嵌入
3.16/schema.json,~15485 行,自动生成 + 保留蓝鲸bk-*插件 schema);3.13 schema 补充bk-traffic-label。 schemaVersionMap注册 3.16;GetResourceSchema/GetPluginSchema/GetMetadataPluginSchema通过version查询。- 测试用
APISIXVersionList对全部版本做矩阵式覆盖。
- 新增 APISIX 3.16 的 JSON Schema(嵌入
-
测试:补充
test_data_plane_model、test_management_commands、各 convertor / transformer / distributor 的apisix_version透传断言,以及 operator 端 schema_test / validate_test 矩阵。
整体属于"为多版本数据面打通通路 + 注入 3.16 schema"的纵向贯通改动,向后兼容(默认仍是 3.13)。
问题列表
🔴 Blocking
无。
🟠 Major
M1. GetMetadataPluginSchema 包含死分支 [codex] [claude]
src/operator/pkg/utils/schema/schema.go:51-59
func GetMetadataPluginSchema(version constant.APISIXVersion, path string) any {
ret := schemaVersionMap[version].Get(path).Value()
if ret != nil {
return ret
}
return ret // 死代码:与上面分支返回值完全相同
}问题:
- 无意义包装:函数体只有一行
schemaVersionMap[version].Get(path).Value()加上一个永远等价的if ret != nil { return ret } return ret,整体等价于return schemaVersionMap[version].Get(path).Value()。完全可以删除该函数,把GetPluginSchema中的调用直接替换为内联的schemaVersionMap[version].Get("plugins."+name+".metadata_schema").Value()(与同函数中consumer_schema、stream_schema的写法保持一致),消除一处仅有一个调用点的薄包装。 - 死分支:
if ret != nil { return ret }与最后的return ret行为完全相同,属于代码坏味道(容易让读者怀疑作者是否漏写了别的逻辑分支,例如本来想先查metadata_schema不命中再 fallbackschema,但实际并没有 fallback)。请确认是否漏写 fallback;若不需要 fallback,请直接return schemaVersionMap[version].Get(path).Value()。
建议:
- 直接删除
GetMetadataPluginSchema,在GetPluginSchema中用一行内联实现。 - 或者保留函数但写为单行 return,并补上文档说明为何 metadata 不需要 fallback。
M2. Dashboard 与 Operator 版本号取值不一致,依赖隐式 ToXVersion 转换 [claude]
- 位置:
src/dashboard/apigateway/apigateway/apps/data_plane/constants.py:32-33:dashboard 存的是"3.13"/"3.16";src/operator/pkg/constant/apisix.go:102-103:operator 用的是"3.13.X"/"3.16.X";src/operator/pkg/utils/version_test.go新增的{"3.13", "3.13.X"}、{"3.16", "3.16.X"}用例隐含承担了「dashboard 写入的字符串 → operator 内部识别的 schema key」的转换约定。
- 为什么是问题:dashboard 侧把
"3.13"这个原始字符串直接写进 etcd label(labels[gateway.bk.tencent.com/apisix-version] = "3.13"),operator 校验时则用APISIXVersion313 = "3.13.X"作为 key 去查schemaVersionMap。这个「短版本 → X 版本」的转换分散在version.go::ToXVersion中,dashboard 这边并没有任何注释或常量把两端对齐,未来如果改 operator 的常量命名(比如直接用"3.13")或者 dashboard 改写入策略,很容易出现「label 校验通过但 schema 没找到」的不一致。 - 改进方向:在
DataPlaneApisixVersionEnum旁边添加注释,明确「值会作为 label 写入 etcd,operator 通过ToXVersion转成 schemaVersionMap 的 key,二者必须保持一一对应」;或者更彻底地让 dashboard 直接写"3.13.X",由 operator 直接消费,去掉中间的ToXVersion调用。
M3. bk-release 与 __apigw_version detect route 中 apisix_version 的语义混淆 [claude]
- 位置:
src/dashboard/apigateway/apigateway/controller/transformer.py:54:plugin_metadata_convertor = PluginMetadataConvertor(self.apisix_version)src/dashboard/apigateway/apigateway/controller/distributor/etcd.py:66, 139, 222:把data_plane.apisix_version传给 transformer
- 为什么是问题:
apisix_version写入 etcd label 时,其语义是「配置目标版本」(期望数据面运行的 APISIX 版本),但在 operator 消费时,如果数据面实际运行的版本与 label 不一致(例如数据面升级到 3.16 但 label 还是 3.13),会导致 schema 校验用错版本。当前实现没有在 operator 侧做"数据面实际版本"与"label 指定版本"的一致性校验。 - 改进方向:在 operator 侧增加数据面实际版本校验,或明确文档说明"label 指定的版本必须 ≤ 数据面实际版本"。
M4. 发布前缺少跨绑定数据面的版本兼容性收敛 [codex]
当前实现只把"版本感知"下沉到了资源打标和 operator 侧校验,但没有在发布前做"跨绑定数据面"的兼容性收敛,混部场景下仍然会出现部分数据面成功、部分数据面失败的发布结果。
证据:dashboard 这里只在分发时按 data_plane.apisix_version 给资源打标签并写入 etcd(src/dashboard/apigateway/apigateway/controller/distributor/etcd.py:66, 139, 222),真正的 schema 拒绝发生在 operator 消费资源时(src/operator/pkg/core/registry/apigw.go:366, 469)。如果同一 stage 绑定了 3.13 和 3.16 两类 data plane,发布包含 3.16 专属能力的配置时,3.16 数据面会继续推进,3.13 数据面会在 operator 侧才失败,系统层面得到的是一次"部分生效"的发布,而不是一个在发布前被明确拒绝的变更。这和 PR 描述里"for bound data planes" 的目标不完全一致。
更合理的方向是:在 publish 前先聚合所有绑定数据面的最低/允许 APISIX 版本并做一次兼容性校验,或者直接禁止同一 stage 绑定不同 APISIX 大版本的数据面。
🟡 Minor
m1. apisix_version 在 transformer/convertor 层仍然保留默认值,可能导致静默回退 [codex]
apisix_version 现在已经是数据面上下文的一部分,但在 transformer/convertor 层仍然保留了 DEFAULT_APISIX_VERSION = "3.13" 作为默认值,导致遗漏传参时会静默回退到 3.13,而不是尽早暴露错误。
相关位置:
src/dashboard/apigateway/apigateway/controller/transformer.py:49, 78src/dashboard/apigateway/apigateway/controller/convertor/base.py:44, 60src/dashboard/apigateway/apigateway/controller/convertor/service.py:64src/dashboard/apigateway/apigateway/controller/convertor/route.py:45src/dashboard/apigateway/apigateway/controller/convertor/bk_release.py:30
建议:让 dashboard 在 distributor/transformer 边界显式要求 apisix_version,不要在更深层继续保留默认回退。
m2. 迁移文件缺少版权头注释 [claude]
src/dashboard/apigateway/apigateway/apps/data_plane/migrations/0002_dataplane_apisix_version.py
仓库内其他 Python 文件均有标准的 BlueKing 版权头,唯独本迁移文件没有。Django auto-generated migration 通常也是允许带版权头的,建议补上以保持一致。
m3. BkPluginsDataPlaneGrayStageEnum 与本 PR 主题无关 [claude]
src/dashboard/apigateway/apigateway/apps/data_plane/constants.py:36-41
class BkPluginsDataPlaneGrayStageEnum(StructuredEnum):
NOT_START = EnumField("not_start", "not_start")
START = EnumField("start", "start")
DONE = EnumField("done", "done")该枚举似乎与"BK 插件数据面灰度"相关,但 PR 描述只提及 apisix_version,且 diff 中未看到使用该枚举的地方(请作者在 PR 描述里点名一下用途,或考虑拆到独立的 PR 中)。如果是预埋未使用的枚举,属于伪扩展性。建议确认引用方是否在本 PR 范围内;若否,请单独提交。
m4. _validate_apisix_version 在 add_argument 已用 choices 限制后变成重复校验 [claude]
src/dashboard/apigateway/apigateway/apps/data_plane/management/commands/create_data_plane.py:114-117, 167
parser.add_argument(
"--apisix-version", type=str,
choices=DataPlaneApisixVersionEnum.get_values(),
...
)
...
def _validate_apisix_version(self, apisix_version: str):
allowed = set(DataPlaneApisixVersionEnum.get_values())
if apisix_version not in allowed:
raise CommandError(...)argparse 的 choices 已经会在解析阶段拒绝非法值,再加一层 _validate_apisix_version 属于重复校验(同文件中的 _validate_status 也是同样模式,但 --status 没用 choices 限制,所以那一层是必要的)。这里属于过度防御——如果你想保持对称性以方便后续从其他入口调用 handle,建议在注释里写清楚原因;否则直接去掉 _validate_apisix_version。
m5. apisix_version: str 类型而不是 constant.APISixVersion 或枚举 [claude]
dashboard 侧整条链路(transformer → convertor → release model)都使用 apisix_version: str 进行透传。考虑到 DataPlaneApisixVersionEnum 已经存在,使用裸 str 容易让调用方传入任意字符串导致 label 写花。
建议:
- 在 transformer / convertor
__init__中显式校验apisix_version in DataPlaneApisixVersionEnum.get_values(),或 - 把参数类型从
str改成枚举类型(取.value时再变字符串)。
属于轻量加固,避免上游误传。不阻塞合并。
m6. GetPluginSchema 中 consumer_schema 与常规 plugin 的 fallback 顺序略反直觉 [claude]
src/operator/pkg/utils/schema/schema.go:62-85
当 schemaType == "consumer" 时先取 consumer_schema,未命中则继续取 schema;但 ret == nil 这条判断对 metadata/stream 分支已经提前 return,所以没问题。然而:
var ret any在函数顶部声明,仅 consumer 分支会写入;- 函数最后又写了
if ret != nil { return ret } return ret(与 M1 同样的死分支模式)。
建议:把"非 consumer/metadata/stream 走常规 plugin"的逻辑用 early return 写得更直接,例如:
switch schemaType {
case "metadata", "metadata_schema":
return schemaVersionMap[version].Get("plugins." + name + ".metadata_schema").Value()
case "stream", "stream_schema":
return schemaVersionMap[version].Get("stream_plugins." + name + ".schema").Value()
case "consumer", "consumer_schema":
if ret := schemaVersionMap[version].Get("plugins." + name + ".consumer_schema").Value(); ret != nil {
return ret
}
}
return schemaVersionMap[version].Get("plugins." + name + ".schema").Value()字面值("consumer"/"consumer_schema"/"metadata"/"metadata_schema"/"stream" 等)建议沉淀为常量或 enum,避免后续多处使用时拼写漂移;当前这些是从外部 dashboard 推过来的字符串,已经是事实上的协议字段,更应该有显式定义。
💬 Nit
src/operator/pkg/utils/schema/schema.go:31-33IsInnerPlugin与版本切换无关,未在本 PR 修改,OK。但本文件顶部import "strings"和import "operator/pkg/constant"是新加的,请确认goimports已分组(看上去已分组,OK)。src/dashboard/apigateway/apigateway/controller/transformer.py:49GlobalApisixResourceTransformer.__init__接收的apisix_version没有任何校验,且_converted_plugin_metadata在transform()之前是空 list,与Gatewaytransformer 同步骨架对称——OK。src/dashboard/apigateway/apigateway/controller/convertor/route.py:212-225_get_release_version_detect_route中bk-mock的response_example直接写入"apisix_version": self._apisix_version——若客户端依据该字段做行为分支,请确保格式与_bk_release.apisix_version一致(同样是"3.13"/"3.16",目前一致,OK,仅提醒后续不要漂移)。src/dashboard/apigateway/apigateway/controller/convertor/constants.py:22新增LABEL_KEY_APISIX_VERSION但移除了 3 行(变更 stat: +3 -3)——猜测是把原本散落的字面值合并到此常量,方向正确,去重做得不错。src/operator/pkg/utils/schema/3.16/schema.json全量 embed,会增加二进制体积约 150KB(压缩后可能更小),属于可接受范围,但建议注释说明"此文件由 apisix-core 3.16 生成,勿手动编辑"。
优点
- 常量化做得不错:
LABEL_KEY_APISIX_VERSION集中定义,避免 label key 字面值散落;DataPlaneApisixVersionEnum用StructuredEnum而非裸字符串。 - 测试矩阵覆盖到位:operator 侧
APISIXVersionList让TestNewAPISIXJsonSchemaValidator/TestAPISIXSchemaValidatorValidate/TestGetResourceSchema等用例都对每个版本跑一遍,新增 3.16 时不需要逐个改 case;dashboard 侧test_transformer.py、test_etcd.py也补了apisix_version透传断言。 - 向后兼容设计合理:
- 数据库迁移只
AddField、提供 default"3.13",对存量数据无影响。 - 3.13 schema 中保留蓝鲸
bk-*插件不变,仅追加bk-traffic-label,不会让现有 dashboard payload 失效。 - operator schema 注册表是 map 而非 switch,新增版本只需追加一行。
- 数据库迁移只
- 职责划分清楚:dashboard 只负责"给数据面打 label",schema 校验留给 operator;data plane 与 gateway 的 N:N 绑定模型不变,单纯加了一个版本属性,没有改动 binding 语义。
_bk_release中携带apisix_version让 operator 在 watch 时能立刻拿到版本,不需要再回查 dashboard,减少跨系统调用。- 版本信息沿 dashboard 发布链路的传递比较完整,
global和gateway两条路径都覆盖到了,避免了只改局部资源类型导致 label 不一致。 - Operator 侧没有走散落的 if/else 分支,而是继续复用现有
schemaVersionMap机制扩展 3.16,整体接入方式和现有架构保持一致。
总结建议
整体方向正确:用 label 把数据面的 APISIX 版本透传到 operator,让 operator 选择对应的 schema 校验——这是一种简单清晰的扩展机制。代码改动量虽然大(17k 行),但其中 ~15k 是自动生成的 schema JSON,核心逻辑改动不大,且测试比较扎实。
合并前建议处理:
- M1 必修:删除
GetMetadataPluginSchema中的死分支(要么变成单行 return,要么直接内联)。 - M2 建议修:在
DataPlaneApisixVersionEnum旁边添加注释,明确「值会作为 label 写入 etcd,operator 通过ToXVersion转成 schemaVersionMap 的 key,二者必须保持一一对应」;或者更彻底地让 dashboard 直接写"3.13.X",由 operator 直接消费,去掉中间的ToXVersion调用。 - M3 建议修:在 operator 侧增加数据面实际版本校验,或明确文档说明"label 指定的版本必须 ≤ 数据面实际版本"。
- M4 建议修:在 publish 前先聚合所有绑定数据面的最低/允许 APISIX 版本并做一次兼容性校验,或者直接禁止同一 stage 绑定不同 APISIX 大版本的数据面。
可选改进(不阻塞合并,但建议处理):
- m1:让 dashboard 在 distributor/transformer 边界显式要求
apisix_version,不要在更深层继续保留默认回退。 - m2:迁移文件补上版权头。
- m3:确认
BkPluginsDataPlaneGrayStageEnum是否在本 PR 范围内,若否请单独提交。 - m4:直接去掉
_validate_apisix_version(argparse 的choices已足够)。 - m5:给
apisix_version参数加类型/取值校验。 - m6:把
GetPluginSchema改写为更直观的switch,并把"consumer"/"metadata"/"stream"等字面值常量化。
特别提示:建议在 PR 描述里补一句"dashboard 侧的 3.13/3.16 在 operator 中由 ToXVersion 规范化为 3.13.X/3.16.X 后再查 schemaVersionMap",给后续维护者留下显式路径。
由 codex-internal (gpt-5.4) + Claude Opus 4.7 双模型 review,主 agent 汇总 | [from openclaw-internal]
Summary
apisix_versiontoDataPlane(default3.13, optional3.16) and propagate it through the release pipeline, so every emitted APISIX resource carries agateway.bk.tencent.com/apisix-versionlabel that matches the bound data plane. This lets the operator validate each data plane against the correct APISIX schema and unlocks 3.16-only plugins for data planes that opt in.DataPlane.apisix_versionmodel field (+ admin display/filter + migration);apisix_versionthreaded throughGlobalApisixResourceTransformer/GatewayApisixResourceTransformerinto the service / route /_bk_release/plugin_metadataconvertors and the__apigw_versiondetect route.3.16JSON schema for resource and plugin validation, generated fromapisix-core3.16 while carrying over the BlueKing-curatedmainsection andbk-*plugin schemas from 3.13 (keeps existing dashboard payloads valid while adding new/updated 3.16 plugins).Verification
uv run ruff format+ruff check: clean (all checks passed, no reformat)mypy: Success, no issues found in 1488 source fileslint-imports: 6 contracts kept, 0 brokentests/controller/+tests/apps/data_plane/: 408 passedTestDataPlaneApisixVersion2 passedmake fmt+make lint: 0 issuesmake test(ginkgo, 17 suites): Test Suite Passed — 3.16 schema validated across all resource types and plugins via the sharedAPISIXVersionListReview
mainschema carried verbatim so existing payloads keep validating).