feat: Add a new CRD WasmPluginMatchRule for config decoupling - #3217
feat: Add a new CRD WasmPluginMatchRule for config decoupling#3217CH3CHO wants to merge 1 commit into
Conversation
新增 CRD WasmPluginMatchRule 以实现配置解耦变更概述
变更文件
时序图sequenceDiagram
participant WC as WasmPluginController
participant WMR as WasmPluginMatchRuleController
participant IC as IngressConfig
participant K8sAPI as Kubernetes API Server
Note over WC,WMR: 启动时初始化各自控制器
activate WC
activate WMR
WC->>K8sAPI: Watch WasmPlugin events
WMR->>K8sAPI: Watch WasmPluginMatchRule events
Note over IC: 当收到 WasmPlugin 更新请求时
IC->>IC: Load all related WasmPluginMatchRules via parent reference
IC->>IC: Convert combined configuration into Istio WasmPlugin model
IC->>XDS: Push updated config to data plane
Note over IC: 当收到 WasmPluginMatchRule 更新/删除请求时
IC->>IC: Track affected WasmPlugin through parent mapping
IC->>IC: Trigger reprocessing of impacted WasmPlugins
IC->>XDS: Propagate changes downstream
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
![lingma-agents[bot]](https://avatars.githubusercontent.com/in/1198481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 0 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 2 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题，酬情优化。例如：代码格式不规范或注释缺失。 |
总计: 3 个问题
📋 评审意见详情
💡 代码实现建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
📦 api/extensions/v1alpha1/wasmplugin.proto (1 💬)
⚙️ api/kubernetes/customresourcedefinitions.gen.yaml (1 💬)
- 扩展 OpenAPI 规范定义以涵盖新增的 `status` 属性。 (L170-L192)
🔹 client/pkg/apis/extensions/v1alpha1/types.gen.go (1 💬)
- 调整客户端侧结构体定义以反映后端模型的变化。 (L95-L105)
🚀 架构设计建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍1. WasmPluginMatchRule控制器与事件处理机制需加强测试覆盖与错误恢复策略
新增的WasmPluginMatchRule控制器及其事件处理器（AddOrUpdateWasmPluginMatchRule、DeleteWasmPluginMatchRule）在pkg/ingress/config/ingress_config.go中被引入，但缺乏明确的错误恢复和重试逻辑。若WasmPluginMatchRule资源更新失败或依赖的父级WasmPlugin不存在，可能导致配置不一致且无日志追踪。建议增加对异常情况的监控指标，并确保事件驱动流程具备幂等性和容错能力。此外，当前代码未展示针对这些新功能的单元测试或集成测试用例，存在潜在维护风险。
📌 关键代码
func (m *IngressConfig) AddOrUpdateWasmPluginMatchRule(...) {...}系统可能因单个资源更新失败导致整个插件规则链失效；缺少可观测性将使问题排查困难。
🔍2. WasmPluginMatchRule排序逻辑复杂度高，可能影响大规模部署下的性能表现
convertIstioWasmPlugin方法中的matchRuleResources排序使用了自定义比较函数compareWasmPluginMatchRule，该函数涉及多次字符串切片排序和逐项对比，在大量WasmPluginMatchRule资源场景下会显著拖慢转换过程。尤其当服务、入口或域名匹配项较多时，slice sorting操作重复执行会造成CPU开销增大。应评估是否可通过预计算哈希值或其他优化手段降低时间复杂度。
📌 关键代码
slices.SortFunc(matchRuleResources, compareWasmPluginMatchRule)func compareWasmPluginMatchRule(...) {...}在包含大量WasmPluginMatchRule规则的大规模集群中，XDS推送延迟增加，控制面响应变慢。
🔍3. WasmPluginMatchRule Parent字段引用关系缺乏校验，易引发悬挂指针问题
WasmPluginMatchRule通过Parent字段关联到WasmPlugin资源名，但在处理过程中并未验证所指向的WasmPlugin是否存在或有效。如果用户误配或删除了对应的WasmPlugin而保留MatchRule，则会在运行时造成空引用或者无效配置加载。应在AddOrUpdateWasmPluginMatchRule方法内加入对Parent资源存在的前置检查，并提供清晰的状态反馈。
📌 关键代码
string parent = 20;
配置漂移、运行时报错以及难以调试的非预期行为。
🔍4. 多来源MatchRule合并顺序语义不明，业务一致性存在隐患
convertIstioWasmPlugin方法中同时处理嵌入式MatchRules及外部独立WasmPluginMatchRule资源，虽然注释说明后者优先，但实际合并策略没有文档化说明，容易引起使用者混淆。特别是两者混合使用时，最终生效规则取决于内部实现细节而非显式契约。这违反了业务逻辑一致性原则，应当明确定义并公开优先级规则，最好能体现在CRD规范或配套文档中。
📌 关键代码
// Separated WasmPluginMatchRule configurations always come after those embedded in the WasmPlugin resource.规则覆盖不符合预期，引发安全策略绕过或插件行为异常等问题。
🔍5. 新增CRD WasmPluginMatchRule缺少状态子资源初始化逻辑
尽管为WasmPluginMatchRule添加了status子资源支持（如CRD定义和Go结构体），但是在控制器层面没有任何关于Status更新的操作逻辑。这意味着即使发生错误或状态变更也无法反映给用户，降低了可观察性和运维效率。需要补充相应的status管理器来跟踪同步状态、健康状况等关键信息。
📌 关键代码
status:
type: object
x-kubernetes-preserve-unknown-fields: trueStatus metav1alpha1.IstioStatus `json:"status"`无法获知资源配置的实际应用效果，阻碍自动化运维工具构建闭环。
审查详情
📒 文件清单 （17 个文件）
📝 变更: 17 个文件
📝 变更文件:
api/extensions/v1alpha1/wasmplugin.pb.goapi/extensions/v1alpha1/wasmplugin.protoapi/extensions/v1alpha1/wasmplugin_deepcopy.gen.goapi/extensions/v1alpha1/wasmplugin_json.gen.goapi/kubernetes/customresourcedefinitions.gen.yamlclient/pkg/apis/extensions/v1alpha1/register.gen.goclient/pkg/apis/extensions/v1alpha1/types.gen.goclient/pkg/apis/extensions/v1alpha1/zz_generated.deepcopy.gen.goclient/pkg/applyconfiguration/utils.goclient/pkg/clientset/versioned/typed/extensions/v1alpha1/extensions_client.gen.goclient/pkg/clientset/versioned/typed/extensions/v1alpha1/fake/fake_extensions_client.gen.goclient/pkg/clientset/versioned/typed/extensions/v1alpha1/generated_expansion.gen.goclient/pkg/informers/externalversions/extensions/v1alpha1/interface.gen.goclient/pkg/informers/externalversions/generic.gen.goclient/pkg/listers/extensions/v1alpha1/expansion_generated.gen.gopkg/ingress/config/ingress_config.gopkg/ingress/kube/wasmplugin/controller.go
💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@lingma-agents 分析这个方法的性能瓶颈并提供优化建议。
-
@lingma-agents 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@lingma-agents 请总结上述讨论并提出解决方案。
-
@lingma-agents 请根据讨论内容生成优化代码。
| message WasmPluginMatchRule { | ||
| repeated string ingress = 1; | ||
| repeated string domain = 2; | ||
| google.protobuf.Struct config = 3; | ||
| google.protobuf.BoolValue config_disable = 4; | ||
| repeated string service = 5; | ||
| string parent = 20; | ||
| } No newline at end of file |
There was a problem hiding this comment.
为新增的消息类型 WasmPluginMatchRule 添加 Status 字段，以便于跟踪其运行时状态。
🟡 Major | 🧹 Code Smells
📋 问题详情
新引入的 WasmPluginMatchRule 消息没有包含 Status 字段，这对于记录和管理插件匹配规则的实际执行状态至关重要。
💡 解决方案
在 WasmPluginMatchRule 中增加一个可选的 status 字段来表示当前的状态。
+ google.protobuf.Struct status = 21;您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| properties: | ||
| spec: | ||
| properties: | ||
| config: | ||
| type: object | ||
| x-kubernetes-preserve-unknown-fields: true | ||
| configDisable: | ||
| type: boolean | ||
| domain: | ||
| items: | ||
| type: string | ||
| type: array | ||
| ingress: | ||
| items: | ||
| type: string | ||
| type: array | ||
| parent: | ||
| type: string | ||
| service: | ||
| items: | ||
| type: string | ||
| type: array | ||
| type: object |
There was a problem hiding this comment.
扩展 OpenAPI 规范定义以涵盖新增的 status 属性。
🟡 Major | 🧹 Code Smells
📋 问题详情
CRD 的 OpenAPIV3Schema 定义未包括 status 字段，这意味着用户可能无法通过标准工具验证或操作这个重要属性。
💡 解决方案
将 status 字段添加进 CRD 的 schema 中，指定它是一个保留未知字段的对象类型。
+ status:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
| type WasmPluginMatchRule struct { | ||
| v1.TypeMeta `json:",inline"` | ||
| // +optional | ||
| v1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` | ||
|
|
||
| // Spec defines the implementation of this definition. | ||
| // +optional | ||
| Spec extensionsv1alpha1.WasmPluginMatchRule `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` | ||
|
|
||
| Status metav1alpha1.IstioStatus `json:"status"` | ||
| } |
There was a problem hiding this comment.
|
follow follow |
2 participants
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
Ⅵ. AI Coding Tool Usage Checklist (if applicable)
Please check all applicable items:
For new standalone features (e.g., new wasm plugin or golang-filter plugin):
design/directory in the plugin folderdesign/directoryFor regular updates/changes (not new plugins):
AI Coding Prompts (for regular updates)
AI Coding Summary