From 2d872d05e44fe25b4247b1117e658343090fab87 Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:40:16 +0800 Subject: [PATCH 1/2] Configuration validation refuses an upstream listed twice in a group The control plane already refuses a group that names the same upstream twice (control.group.upstream_twice), but loading the configuration did not, so `providers: [a, a, a, b]` written by hand was accepted. Nothing de-duplicates a group's candidates: failover tried `a` again after it failed, and load-balance handed `a` more turns, an unwritten weight the user cannot see anywhere. Engine::validate now refuses it with its own code, engine.group_upstream_twice, naming the group and the upstream. This covers every group type, since repeated failover attempts are wrong for all of them. The configuration reference says that each upstream appears once in a group. A configuration that already lists an upstream twice in a group no longer loads and starts in safe mode, which reports the group and the upstream. Refs #289 Co-Authored-By: Claude Opus 5.5 --- crates/tw-api/msg-codes.txt | 1 + crates/tw-config/src/validate.rs | 31 ++++++++++++ crates/tw-config/tests/manual/schema.rs | 5 +- crates/tw-engine/src/engine.rs | 64 +++++++++++++++++++++++++ docs/config.md | 2 +- docs/config.zh-CN.md | 2 +- 6 files changed, 102 insertions(+), 3 deletions(-) diff --git a/crates/tw-api/msg-codes.txt b/crates/tw-api/msg-codes.txt index 30f657b..596ff5e 100644 --- a/crates/tw-api/msg-codes.txt +++ b/crates/tw-api/msg-codes.txt @@ -228,6 +228,7 @@ engine.compare.no_operator engine.duplicate_group engine.duplicate_route engine.empty_group +engine.group_upstream_twice engine.no_action engine.no_match engine.phase_two_with_to diff --git a/crates/tw-config/src/validate.rs b/crates/tw-config/src/validate.rs index 20c263a..b427c1c 100644 --- a/crates/tw-config/src/validate.rs +++ b/crates/tw-config/src/validate.rs @@ -778,6 +778,37 @@ mod tests { assert!(e.to_string().contains("__all__"), "{e}"); } + /// 手写的配置里一个组把同一家写了几遍：加载时就拒绝。控制面保存时本来就拦着， + /// 拦不着的是手改的文件 —— 那时候选不去重，故障转移会把同一家再试几遍 + #[test] + fn a_group_that_lists_an_upstream_twice_is_refused_at_load_time() { + let text = "version: 1 +listen: + control: + key: c0ffee00c0ffee00c0ffee00c0ffee00c0ffee00c0ffee00c0ffee00c0ffee00 +clients: + - name: c + key: tw-k +providers: + - name: a + base_url: https://a.example + key: sk-a + - name: b + base_url: https://b.example + key: sk-b +groups: + - name: pool + type: load-balance + providers: [a, a, a, b] +"; + let m = crate::try_parse(text).unwrap_err().msg(); + assert_eq!(m.code, "engine.group_upstream_twice", "{m:?}"); + assert_eq!((m.arg("group"), m.arg("upstream")), ("pool", "a")); + // 每家写一次就收下 + let once = text.replace("[a, a, a, b]", "[a, b]"); + assert!(crate::try_parse(&once).is_ok(), "{once}"); + } + #[test] fn error_messages_say_what_to_do_next() { // 错误信息是降低使用难度最有效的杠杆。判据不是「说清 diff --git a/crates/tw-config/tests/manual/schema.rs b/crates/tw-config/tests/manual/schema.rs index dd0838e..23fd559 100644 --- a/crates/tw-config/tests/manual/schema.rs +++ b/crates/tw-config/tests/manual/schema.rs @@ -1236,7 +1236,10 @@ pub fn sections() -> Vec
{ "providers", Kind::Strs, Def::Required, - t("Member upstreams, by name.", "成员上游的名字。"), + t( + "Member upstreams, by name. Each upstream appears once in a group.", + "成员上游的名字。同一个上游在一个策略组中只出现一次。", + ), ), row( "selected", diff --git a/crates/tw-engine/src/engine.rs b/crates/tw-engine/src/engine.rs index 8880001..e0e5d1e 100644 --- a/crates/tw-engine/src/engine.rs +++ b/crates/tw-engine/src/engine.rs @@ -555,6 +555,8 @@ pub enum RouteError { #[error("{}", self.msg())] DuplicateGroup(String), #[error("{}", self.msg())] + GroupUpstreamTwice { group: String, provider: String }, + #[error("{}", self.msg())] DuplicateRoute(String), #[error("{}", self.msg())] UnknownDefaultRoute(String), @@ -606,6 +608,11 @@ impl RouteError { "there is more than one group named `{group}`. Rules refer to a group by name, so \ names have to be unique" ), + RouteError::GroupUpstreamTwice { group, provider } => msg!( + "engine.group_upstream_twice", group = group, upstream = provider => + "group `{group}` lists upstream `{upstream}` more than once. Each upstream appears \ + once in a group" + ), RouteError::DuplicateRoute(route) => msg!( "engine.duplicate_route", route = route => "there is more than one route named `{route}`. A gateway key binds to a route by \ @@ -876,6 +883,18 @@ impl Engine { if g.providers.is_empty() { return Err(RouteError::EmptyGroup(g.name.clone())); } + // 同一家写两次：候选不去重，那一家失败之后故障转移会再试它一遍；`load-balance` + // 还会多轮到它几次 —— 一个没人写下、也看不出来的权重。控制面保存时就拦着 + // （`control.group.upstream_twice`），手写的配置在这里拦 + let mut members = std::collections::HashSet::new(); + for p in &g.providers { + if !members.insert(p.as_str()) { + return Err(RouteError::GroupUpstreamTwice { + group: g.name.clone(), + provider: p.clone(), + }); + } + } } for set in &self.sets { self.check_rules(&set.rules)?; @@ -2228,6 +2247,47 @@ mod builtin_tests { ); assert_eq!(e.validate(), Err(RouteError::DuplicateGroup("pool".into()))); } + + /// 手写的配置里同一家写了几遍：拒绝，说出是哪个组、哪一家。不拦的话候选里它出现 + /// 几次，故障转移就试它几次，`load-balance` 也多轮到它几次 + #[test] + fn a_group_that_lists_an_upstream_twice_is_rejected() { + let g: Group = + serde_yaml_ng::from_str("name: pool\ntype: load-balance\nproviders: [a, a, a, b]\n") + .unwrap(); + let e = Engine::with_default_rules( + vec!["a".into(), "b".into()], + vec![g], + vec![rule("兜底", "{}", "pool")], + ); + let err = e.validate().unwrap_err(); + assert_eq!( + err, + RouteError::GroupUpstreamTwice { + group: "pool".into(), + provider: "a".into(), + } + ); + let m = err.msg(); + assert_eq!(m.code, "engine.group_upstream_twice"); + assert_eq!((m.arg("group"), m.arg("upstream")), ("pool", "a")); + // 每种类型都一样：不只是 `load-balance` 会多轮到它，故障转移也会再试它 + let g = Group { + name: "pool".into(), + kind: GroupType::Fallback, + providers: vec!["a".into(), "b".into(), "a".into()], + selected: None, + }; + let e = Engine::with_default_rules( + vec!["a".into(), "b".into()], + vec![g], + vec![rule("兜底", "{}", "pool")], + ); + assert!(matches!( + e.validate(), + Err(RouteError::GroupUpstreamTwice { .. }) + )); + } } #[cfg(test)] @@ -2673,6 +2733,10 @@ mod msg_codes { }, RouteError::EmptyGroup("g".into()), RouteError::DuplicateGroup("g".into()), + RouteError::GroupUpstreamTwice { + group: "g".into(), + provider: "p".into(), + }, RouteError::DuplicateRoute("x".into()), RouteError::UnknownDefaultRoute("x".into()), RouteError::UnknownRoute { diff --git a/docs/config.md b/docs/config.md index 9066764..73ef11b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -953,7 +953,7 @@ group with `to`. |---|---|---|---| | `name` | string | **required** | Name of the group; unique, and not the name of an upstream. | | `type` | `fallback` \| `select` \| `load-balance` \| `url-test` \| `cheapest` | `fallback` | `fallback`: the first healthy member, in order. `select`: the member named in `selected`. `load-balance`: take turns between new conversations. `url-test`: the fastest by measured time to first byte. `cheapest`: the lowest input price. | -| `providers` | list of strings | **required** | Member upstreams, by name. | +| `providers` | list of strings | **required** | Member upstreams, by name. Each upstream appears once in a group. | | `selected` | string | — | For `select`: the chosen member. | diff --git a/docs/config.zh-CN.md b/docs/config.zh-CN.md index a0b9ad7..82ae346 100644 --- a/docs/config.zh-CN.md +++ b/docs/config.zh-CN.md @@ -748,7 +748,7 @@ aliases: |---|---|---|---| | `name` | 字符串 | **必填** | 策略组的名字，不能重复，也不能和上游同名。 | | `type` | `fallback` \| `select` \| `load-balance` \| `url-test` \| `cheapest` | `fallback` | `fallback`：按顺序取第一个健康的。`select`：取 `selected` 指定的那个。`load-balance`：新对话轮流。`url-test`：按实测首字节时间取最快的。`cheapest`：取输入单价最低的。 | -| `providers` | 字符串列表 | **必填** | 成员上游的名字。 | +| `providers` | 字符串列表 | **必填** | 成员上游的名字。同一个上游在一个策略组中只出现一次。 | | `selected` | 字符串 | — | `select` 类型选中的成员。 | From 1edb2d79c38ba384462eb526e0f94b0c939519e4 Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:55:39 +0800 Subject: [PATCH 2/2] Configuration validation refuses a group member that is not an upstream A hand-written group could list a name that is no upstream, a typo or the name of another group, and the configuration still loaded: `twcore check` reported it valid. The member matched no upstream when the group was expanded, so the group silently had one member fewer. The control plane already refuses it when a group is saved (control.group.no_such_upstream), and an upstream that a group still lists cannot be deleted, so only a file edited by hand gets here. Engine::validate now refuses it with its own code, engine.group_unknown_upstream, naming the group and the member. The configuration reference says a group's members are upstreams, not groups. Co-Authored-By: Claude Opus 5.5 --- crates/tw-api/msg-codes.txt | 1 + crates/tw-config/src/validate.rs | 7 +++- crates/tw-config/tests/manual/schema.rs | 4 +-- crates/tw-engine/src/engine.rs | 43 +++++++++++++++++++++++++ docs/config.md | 2 +- docs/config.zh-CN.md | 2 +- 6 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/tw-api/msg-codes.txt b/crates/tw-api/msg-codes.txt index 596ff5e..1531c47 100644 --- a/crates/tw-api/msg-codes.txt +++ b/crates/tw-api/msg-codes.txt @@ -228,6 +228,7 @@ engine.compare.no_operator engine.duplicate_group engine.duplicate_route engine.empty_group +engine.group_unknown_upstream engine.group_upstream_twice engine.no_action engine.no_match diff --git a/crates/tw-config/src/validate.rs b/crates/tw-config/src/validate.rs index b427c1c..a4a05ff 100644 --- a/crates/tw-config/src/validate.rs +++ b/crates/tw-config/src/validate.rs @@ -781,7 +781,7 @@ mod tests { /// 手写的配置里一个组把同一家写了几遍：加载时就拒绝。控制面保存时本来就拦着， /// 拦不着的是手改的文件 —— 那时候选不去重，故障转移会把同一家再试几遍 #[test] - fn a_group_that_lists_an_upstream_twice_is_refused_at_load_time() { + fn a_group_member_written_twice_or_misspelled_is_refused_at_load_time() { let text = "version: 1 listen: control: @@ -807,6 +807,11 @@ groups: // 每家写一次就收下 let once = text.replace("[a, a, a, b]", "[a, b]"); assert!(crate::try_parse(&once).is_ok(), "{once}"); + // 写了一个不是上游的名字也拒绝 + let typo = text.replace("[a, a, a, b]", "[a, typo]"); + let m = crate::try_parse(&typo).unwrap_err().msg(); + assert_eq!(m.code, "engine.group_unknown_upstream", "{m:?}"); + assert_eq!((m.arg("group"), m.arg("upstream")), ("pool", "typo")); } #[test] diff --git a/crates/tw-config/tests/manual/schema.rs b/crates/tw-config/tests/manual/schema.rs index 23fd559..51169a9 100644 --- a/crates/tw-config/tests/manual/schema.rs +++ b/crates/tw-config/tests/manual/schema.rs @@ -1237,8 +1237,8 @@ pub fn sections() -> Vec
{ Kind::Strs, Def::Required, t( - "Member upstreams, by name. Each upstream appears once in a group.", - "成员上游的名字。同一个上游在一个策略组中只出现一次。", + "Member upstreams, by name; not groups. Each upstream appears once in a group.", + "成员上游的名字，不能是策略组。同一个上游在一个策略组中只出现一次。", ), ), row( diff --git a/crates/tw-engine/src/engine.rs b/crates/tw-engine/src/engine.rs index e0e5d1e..1bab7be 100644 --- a/crates/tw-engine/src/engine.rs +++ b/crates/tw-engine/src/engine.rs @@ -557,6 +557,8 @@ pub enum RouteError { #[error("{}", self.msg())] GroupUpstreamTwice { group: String, provider: String }, #[error("{}", self.msg())] + GroupUnknownUpstream { group: String, provider: String }, + #[error("{}", self.msg())] DuplicateRoute(String), #[error("{}", self.msg())] UnknownDefaultRoute(String), @@ -613,6 +615,11 @@ impl RouteError { "group `{group}` lists upstream `{upstream}` more than once. Each upstream appears \ once in a group" ), + RouteError::GroupUnknownUpstream { group, provider } => msg!( + "engine.group_unknown_upstream", group = group, upstream = provider => + "group `{group}` lists `{upstream}`, which is not an upstream. A group's members are \ + upstreams, by name" + ), RouteError::DuplicateRoute(route) => msg!( "engine.duplicate_route", route = route => "there is more than one route named `{route}`. A gateway key binds to a route by \ @@ -888,6 +895,14 @@ impl Engine { // （`control.group.upstream_twice`），手写的配置在这里拦 let mut members = std::collections::HashSet::new(); for p in &g.providers { + // 不认识的名字（拼错了、或者写了另一个组）：候选里它对不上任何上游， + // 这一位就静默地没了。控制面保存时拦着（`control.group.no_such_upstream`） + if !self.providers.contains(p) { + return Err(RouteError::GroupUnknownUpstream { + group: g.name.clone(), + provider: p.clone(), + }); + } if !members.insert(p.as_str()) { return Err(RouteError::GroupUpstreamTwice { group: g.name.clone(), @@ -2288,6 +2303,30 @@ mod builtin_tests { Err(RouteError::GroupUpstreamTwice { .. }) )); } + + /// 组里写了一个不是上游的名字（拼错了，或者写了另一个组）：拒绝，说出是哪个组、 + /// 哪个名字。不拦的话候选里对不上它，组静默地少了一位 + #[test] + fn a_group_member_that_is_not_an_upstream_is_rejected() { + let g: Group = + serde_yaml_ng::from_str("name: pool\ntype: fallback\nproviders: [a, typo]\n").unwrap(); + let e = Engine::with_default_rules( + vec!["a".into(), "b".into()], + vec![g], + vec![rule("兜底", "{}", "pool")], + ); + let err = e.validate().unwrap_err(); + assert_eq!( + err, + RouteError::GroupUnknownUpstream { + group: "pool".into(), + provider: "typo".into(), + } + ); + let m = err.msg(); + assert_eq!(m.code, "engine.group_unknown_upstream"); + assert_eq!((m.arg("group"), m.arg("upstream")), ("pool", "typo")); + } } #[cfg(test)] @@ -2737,6 +2776,10 @@ mod msg_codes { group: "g".into(), provider: "p".into(), }, + RouteError::GroupUnknownUpstream { + group: "g".into(), + provider: "p".into(), + }, RouteError::DuplicateRoute("x".into()), RouteError::UnknownDefaultRoute("x".into()), RouteError::UnknownRoute { diff --git a/docs/config.md b/docs/config.md index 73ef11b..ff747ca 100644 --- a/docs/config.md +++ b/docs/config.md @@ -953,7 +953,7 @@ group with `to`. |---|---|---|---| | `name` | string | **required** | Name of the group; unique, and not the name of an upstream. | | `type` | `fallback` \| `select` \| `load-balance` \| `url-test` \| `cheapest` | `fallback` | `fallback`: the first healthy member, in order. `select`: the member named in `selected`. `load-balance`: take turns between new conversations. `url-test`: the fastest by measured time to first byte. `cheapest`: the lowest input price. | -| `providers` | list of strings | **required** | Member upstreams, by name. Each upstream appears once in a group. | +| `providers` | list of strings | **required** | Member upstreams, by name; not groups. Each upstream appears once in a group. | | `selected` | string | — | For `select`: the chosen member. | diff --git a/docs/config.zh-CN.md b/docs/config.zh-CN.md index 82ae346..deac482 100644 --- a/docs/config.zh-CN.md +++ b/docs/config.zh-CN.md @@ -748,7 +748,7 @@ aliases: |---|---|---|---| | `name` | 字符串 | **必填** | 策略组的名字，不能重复，也不能和上游同名。 | | `type` | `fallback` \| `select` \| `load-balance` \| `url-test` \| `cheapest` | `fallback` | `fallback`：按顺序取第一个健康的。`select`：取 `selected` 指定的那个。`load-balance`：新对话轮流。`url-test`：按实测首字节时间取最快的。`cheapest`：取输入单价最低的。 | -| `providers` | 字符串列表 | **必填** | 成员上游的名字。同一个上游在一个策略组中只出现一次。 | +| `providers` | 字符串列表 | **必填** | 成员上游的名字，不能是策略组。同一个上游在一个策略组中只出现一次。 | | `selected` | 字符串 | — | `select` 类型选中的成员。 |