Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
atapp_conf.proto 删除字段后未 reserved 旧的 tag/name，存在后续误复用导致协议兼容性风险。
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
该 PR 为 libatapp 的服务发现/直连链路增加“监听地址隔离维度”：引入 metadata.scope 与 namespace_name 参与 gateway 匹配与 listen 回退地址规则，并新增 bus.inherited_labels 用于将 atapp 的部分 labels 下发到 atbus 节点/网关匹配；同时移除 metadata.uid/service_subset 等不常用维度并同步更新测试与配置样例。
Changes:
- 协议与配置：新增
metadata.scope、gateway.match_scope、bus.inherited_labels，并移除metadata.uid/service_subset的使用路径 - 运行时行为：atbus 配置映射下发 scope/namespace/labels；discovery listen 回退地址携带 scope/namespace 隔离规则；discovery 更新时刷新 atbus endpoint 的隔离信息与网关
- 测试与样例：新增/更新多组单测与配置文件，覆盖 gateway 匹配、isolation 映射、listen 回退与直连跳过不可达地址等场景
File summaries
| File | Description |
|---|---|
| test/case/atapp_test_isolation_1.yaml | 新增隔离映射测试用配置：scope/namespace/labels/inherited_labels/gateways 组合 |
| test/case/atapp_test_direct_1.yaml | 增加 metadata.labels 与 bus.inherited_labels 以覆盖 label 继承路径 |
| test/case/atapp_setup_test.cpp | 增加 gateway scope/namespace/labels 匹配与 atbus isolation 映射单测 |
| test/case/atapp_discovery_test.cpp | 将原 service_subset 维度切换为 scope，并新增 listen 回退携带隔离规则测试 |
| test/case/atapp_direct_connect_test.cpp | 新增 scope 不匹配跳过拨号、scope 匹配直连、discovery 更新刷新 bus endpoint 等测试 |
| test/case/atapp_configure_loader_test.cpp | 表达式加载用例从 uid/service_subset 调整为 kind/scope |
| test/case/atapp_configure_expression_test.yaml | 更新表达式配置样例字段到 kind/scope |
| test/case/atapp_configure_expression_test.env.txt | 更新 env 变量映射到 ATAPP_METADATA_KIND/ATAPP_METADATA_SCOPE |
| test/case/atapp_configure_expression_test.conf | 更新 conf 样例字段到 metadata.kind/metadata.scope |
| src/CMakeLists.txt | 为主库源文件追加对生成的 atapp_conf.pb.h 的对象级依赖，避免并行编译竞态 |
| src/atframe/etcdcli/etcd_discovery.cpp | 元数据等价/哈希/过滤维度切换到 scope；listen 回退 gateway 预置 scope/namespace 规则 |
| src/atframe/connectors/atapp_endpoint.cpp | discovery 更新时同步刷新 atbus endpoint 的 scope/namespace/labels/gateways（含 listen 回退规则） |
| src/atframe/atapp.cpp | atbus 配置映射下发 scope/namespace/labels/gateways；gateway 匹配新增 scope；discovery 事件触发 endpoint 刷新 |
| include/atframe/atapp.h | 公共 API 增加 set_metadata_scope()，移除 uid/service_subset setters 声明 |
| include/atframe/atapp_conf.proto | 协议新增 scope/match_scope/inherited_labels，移除 uid/service_subset 字段 |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
518
to
522
| // kubernetes ENV: ATAPP_METADATA_NAME: valueFrom: fieldRef: fieldPath: metadata.name | ||
| string name = 11 [(atframework.atapp.protocol.CONFIGURE) = { enable_expression: true }]; | ||
| // kubernetes ENV: ATAPP_METADATA_NAMESPACE_NAME: valueFrom: fieldRef: fieldPath: metadata.namespace | ||
| string namespace_name = 12 [(atframework.atapp.protocol.CONFIGURE) = { enable_expression: true }]; | ||
| // kubernetes ENV: ATAPP_METADATA_UID: valueFrom: fieldRef: fieldPath: metadata.uid | ||
| string uid = 13 [(atframework.atapp.protocol.CONFIGURE) = { enable_expression: true }]; | ||
|
|
||
| // `service_subset` is similar to Destination in | ||
| // https://istio.io/latest/docs/reference/config/networking/virtual-service/#Destination | ||
| string service_subset = 23 [(atframework.atapp.protocol.CONFIGURE) = { enable_expression: true }]; | ||
|
|
AI Code Review
AI Code Review Summary集群隔离维度改造评审：match_labels 过滤口径不一致等 3 项Target: 增加监听地址scope和namespace，移除部分不常用的维度 评审范围PR #64（f71037b → d75787d）：新增 问题汇总（3 项）
上下文说明
Problems (3)
Code reference:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Gateway rule translation into atbus currently forwards empty host/namespace entries without filtering, which can diverge from the documented “empty means skip” semantics and affect reachability decisions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+319
to
+324
| gw_cfg.address = gw.address(); | ||
| gw_cfg.match_scope = gw.match_scope(); | ||
| gw_cfg.match_hosts.reserve(static_cast<size_t>(gw.match_hosts().size())); | ||
| gw_cfg.match_hosts = {gw.match_hosts().begin(), gw.match_hosts().end()}; | ||
| gw_cfg.match_namespaces.reserve(static_cast<size_t>(gw.match_namespaces().size())); | ||
| gw_cfg.match_namespaces = {gw.match_namespaces().begin(), gw.match_namespaces().end()}; |
Comment on lines
+167
to
+180
| gw.address = gateway.address(); | ||
| gw.match_scope = gateway.match_scope(); | ||
| if (gateway.match_namespaces_size() > 0) { | ||
| gw.match_namespaces.reserve(static_cast<size_t>(gateway.match_namespaces_size())); | ||
| for (const auto &ns : gateway.match_namespaces()) { | ||
| gw.match_namespaces.insert(ns); | ||
| } | ||
| } | ||
| if (gateway.match_hosts_size() > 0) { | ||
| gw.match_hosts.reserve(static_cast<size_t>(gateway.match_hosts_size())); | ||
| for (const auto &host : gateway.match_hosts()) { | ||
| gw.match_hosts.insert(host); | ||
| } | ||
| } |
AI Code Review
AI Code Review SummaryPR #64 集群隔离功能评审：1 个 high 问题Target: 增加监听地址scope和namespace，移除部分不常用的维度 评审范围本 PR 引入的集群隔离能力： 问题（1 个）
不确定性说明
Problems (1)
Code reference:
|
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
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.
No description provided.