From 0d9885a3499b791965a46a35b655aa77afe99f0d Mon Sep 17 00:00:00 2001 From: Episkey Date: Fri, 17 Jul 2026 09:53:30 -0700 Subject: [PATCH] feat(config): validate remotely only when validation-relevant fields change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config update 与主命令 config 在改存量 profile 时无条件远程校验 region+project， 导致离线/网络抖动下仅改元数据(--active/--timeout-sec 等)也被拦。该守卫在 0.1.18 (2019-07-04)拆 add/update 时丢失，属7年回归：2018 单一 config 命令曾有 if cfg.Region!=""||cfg.Zone!="" 守卫。fail-closed(#142)放大了其痛。 改为按需校验（仅改「何时校验」，不改「校验失败怎么办」）： credsOrEndpointChanged = 改了 public-key/private-key/base-url/channel-key validateRegion = 传了 region/zone || credsOrEndpointChanged (主命令再 || !ok) validateProject = 传了 project-id || credsOrEndpointChanged (主命令再 || !ok) 只改元数据则两者皆跳过，离线可改；凭据/接入点变更须同时重校验 region 与 project (换 key/网关后存量值可能失效或新凭据无权访问，配置时当场抓住才是校验价值)。 if 体内的 fail-closed return 与 errNoDefaultProject 放行原样不动。 config add 不改：新建 profile 无「未改动」基线，region/project 须从零建立(R2)。 主命令新建路径(!ok)恒校验(R3)。 测试：6 个新用例（毒网关证明跳过零远程调用、凭据变更触发双校验、新建仍校验）， 2 个 #142 既有用例适配（纯元数据不再触发校验，改用 --region/--public-key 触发）。 已用变异测试验证 AC4 守护「凭据→project 校验」这一关键分支非空转。 CI 不含 ./cmd/...，用例须本地跑。堆叠在 #142 之上，PR 暂以 fix 分支为 base。 --- cmd/configure.go | 137 ++++++++++++++++++++------------ cmd/configure_test.go | 179 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 255 insertions(+), 61 deletions(-) diff --git a/cmd/configure.go b/cmd/configure.go index 439ce4b1ba..f8402cf447 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -279,48 +279,65 @@ func NewCmdConfig() *cobra.Command { cacheConfig.Zone = cfg.Zone } + //按需远程校验：改存量 profile(ok==true)时，仅当改动涉及校验相关字段才发起 + //对应校验；新建(!ok)恒校验（新 profile 的 region/project 须从零建立）。 + //凭据/接入点(public-key/private-key/base-url/channel-key)变更须同时重校验 + //region 与 project——换 key/网关后存量 region/project 可能失效或新凭据无权访问。 + //只改元数据(active/timeout-sec/max-retry-times/agree-upload-log)且改存量则皆跳过， + //离线可改。用 Changed() 而非空值判断：空值是合法输入（清除语义）。 + //本任务只改「何时校验」，if 体内的 fail-closed return 与 errNoDefaultProject + //放行原样不动（prd R4）；跳过时不碰 cacheConfig 的 region/zone/project，保留存量值。 + credsOrEndpointChanged := c.Flags().Changed("public-key") || c.Flags().Changed("private-key") || + c.Flags().Changed("base-url") || c.Flags().Changed("channel-key") + validateRegion := !ok || c.Flags().Changed("region") || c.Flags().Changed("zone") || credsOrEndpointChanged + validateProject := !ok || c.Flags().Changed("project-id") || credsOrEndpointChanged + //确保设置的Region和Zone真实存在。校验失败即整体放弃、不落盘： //此前用 else 保留原值后仍照常写盘，与 add/update 的 fail-closed 口径不一致 - region, zone, err := getReasonableRegionZone(cacheConfig) - if err != nil { - platform.HandleError(fmt.Errorf("verify region failed: %v", err)) - return + if validateRegion { + region, zone, err := getReasonableRegionZone(cacheConfig) + if err != nil { + platform.HandleError(fmt.Errorf("verify region failed: %v", err)) + return + } + cacheConfig.Region = region + cacheConfig.Zone = zone } - cacheConfig.Region = region - cacheConfig.Zone = zone //如果用户填写的project和配置文件中该配置的project均为空，则调接口拉取默认project //如果用户填写的project不为空，则校验其是否真实存在; - if cfg.ProjectID == "" { - if cacheConfig.ProjectID == "" { - //此处直接调用、未经 %v 包装，sentinel 链完整：errNoDefaultProject - //属良性缺失，放行并留空 ProjectID，口径与 ucloud init 一致 - id, _, err := getDefaultProjectWithConfig(cacheConfig) - if err != nil && !errors.Is(err, errNoDefaultProject) { - platform.HandleError(fmt.Errorf("fetch default project failed: %v", err)) - return + if validateProject { + if cfg.ProjectID == "" { + if cacheConfig.ProjectID == "" { + //此处直接调用、未经 %v 包装，sentinel 链完整：errNoDefaultProject + //属良性缺失，放行并留空 ProjectID，口径与 ucloud init 一致 + id, _, err := getDefaultProjectWithConfig(cacheConfig) + if err != nil && !errors.Is(err, errNoDefaultProject) { + platform.HandleError(fmt.Errorf("fetch default project failed: %v", err)) + return + } + if err == nil { + cacheConfig.ProjectID = id + } } - if err == nil { - cacheConfig.ProjectID = id + } else { + cfg.ProjectID = platform.PickResourceID(cfg.ProjectID) + projects, err := fetchProjectWithConfig(cacheConfig) + if err != nil { + //远程不可达时此前直接采信用户输入并落盘，等于写入未经校验的 project； + //现与其余路径一致：拒绝 + platform.HandleError(fmt.Errorf("fetch project failed: %v", err)) + return } - } - } else { - cfg.ProjectID = platform.PickResourceID(cfg.ProjectID) - projects, err := fetchProjectWithConfig(cacheConfig) - if err != nil { - //远程不可达时此前直接采信用户输入并落盘，等于写入未经校验的 project； - //现与其余路径一致：拒绝 - platform.HandleError(fmt.Errorf("fetch project failed: %v", err)) - return - } - if ok := projects[cfg.ProjectID]; !ok { - platform.HandleError(fmt.Errorf("project %s you assigned not exists", cfg.ProjectID)) - if ok := projects[cacheConfig.ProjectID]; !ok { - platform.HandleError(fmt.Errorf("project %s not exists, assign another one please", cacheConfig.ProjectID)) + if ok := projects[cfg.ProjectID]; !ok { + platform.HandleError(fmt.Errorf("project %s you assigned not exists", cfg.ProjectID)) + if ok := projects[cacheConfig.ProjectID]; !ok { + platform.HandleError(fmt.Errorf("project %s not exists, assign another one please", cacheConfig.ProjectID)) + } + return } - return + cacheConfig.ProjectID = cfg.ProjectID } - cacheConfig.ProjectID = cfg.ProjectID } if active != "" { @@ -343,7 +360,7 @@ func NewCmdConfig() *cobra.Command { } } - err = platform.AggConfigListIns.UpdateAggConfig(cacheConfig) + err := platform.AggConfigListIns.UpdateAggConfig(cacheConfig) if err != nil { platform.HandleError(err) } @@ -563,27 +580,43 @@ func NewCmdConfigUpdate() *cobra.Command { draft.Zone = cfg.Zone } - region, zone, err := getReasonableRegionZone(&draft) - if err != nil { - platform.HandleError(err) - return - } - - draft.Region = region - draft.Zone = zone - - if cfg.ProjectID != "" { - draft.ProjectID = platform.PickResourceID(cfg.ProjectID) + //按需远程校验：改存量 profile 时，仅当改动涉及校验相关字段才发起对应校验。 + //凭据/接入点(public-key/private-key/base-url/channel-key)变更须同时重校验 + //region 与 project——换 key/网关后存量 region/project 可能失效或新凭据无权访问， + //配置时当场抓住才是校验的价值；跳过会把「坏 key」延迟到下次真正用命令时才暴露。 + //只改元数据(active/timeout-sec/max-retry-times/agree-upload-log)则两者皆跳过， + //离线可改。用 Changed() 而非空值判断：空值是合法输入（清除语义）。 + //本任务只改「何时校验」，if 体内的 fail-closed return 与 errNoDefaultProject + //放行原样不动（prd R4）；跳过时不碰 draft 的 region/zone/project，保留存量值。 + credsOrEndpointChanged := c.Flags().Changed("public-key") || c.Flags().Changed("private-key") || + c.Flags().Changed("base-url") || c.Flags().Changed("channel-key") + validateRegion := c.Flags().Changed("region") || c.Flags().Changed("zone") || credsOrEndpointChanged + validateProject := c.Flags().Changed("project-id") || credsOrEndpointChanged + + if validateRegion { + region, zone, err := getReasonableRegionZone(&draft) + if err != nil { + platform.HandleError(err) + return + } + draft.Region = region + draft.Zone = zone } - //errNoDefaultProject 是良性缺失（账号有项目但未设默认），放行并留空 ProjectID， - //口径与 ucloud init 一致；其余错误一律拒绝落盘——此处曾漏 return 而抹空 ProjectID - project, err := getReasonableProject(&draft) - if err != nil && !errors.Is(err, errNoDefaultProject) { - platform.HandleError(err) - return + if validateProject { + //归一化只在要校验 project 时才做，跳过时保留存量 ProjectID + if cfg.ProjectID != "" { + draft.ProjectID = platform.PickResourceID(cfg.ProjectID) + } + //errNoDefaultProject 是良性缺失（账号有项目但未设默认），放行并留空 ProjectID， + //口径与 ucloud init 一致；其余错误一律拒绝落盘——此处曾漏 return 而抹空 ProjectID + project, err := getReasonableProject(&draft) + if err != nil && !errors.Is(err, errNoDefaultProject) { + platform.HandleError(err) + return + } + draft.ProjectID = project } - draft.ProjectID = project if active == "true" { draft.Active = true @@ -597,7 +630,7 @@ func NewCmdConfigUpdate() *cobra.Command { draft.AgreeUploadLog = false } - err = platform.AggConfigListIns.UpdateAggConfig(&draft) + err := platform.AggConfigListIns.UpdateAggConfig(&draft) if err != nil { platform.HandleError(err) } diff --git a/cmd/configure_test.go b/cmd/configure_test.go index 49e5d1ca4e..6fc031fac4 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -289,7 +289,7 @@ func setFlags(t *testing.T, cmd *cobra.Command, kv ...string) { // 回归 AC1：config add 远程校验失败时不得创建 profile。 // 现状 getReasonableRegionZone 出错后只 HandleError 不 return，随即把空 region/zone -// 赋回，照常 Append，落盘一个 region='' zone='' project_id='' 的残缺 profile。 +// 赋回，照常 Append，落盘一个 region=” zone=” project_id=” 的残缺 profile。 // // 预置一个 active profile 而非从空配置起步：AggConfigManager.Load 规定「有 profile // 就必须有 active」（config.go:403），否则这里落盘的 bad(active=false) 会让重新读盘 @@ -385,9 +385,11 @@ func TestConfigUpdateWritesNothingWhenValidationFails(t *testing.T) { } } -// 回归 AC4：config update 的 project 校验失败不得清空 ProjectID。 -// 现状同函数内 region 记得 return、project 忘了 return，随即把 "" 赋回并落盘。 -// 网关对 GetRegion 返成功、对 GetProjectList 返失败，精确构造该组合。 +// 回归 AC4（config-cmd-audit）：config update 的 project 校验失败不得清空 ProjectID +// （此前同函数内 region 记得 return、project 忘了 return，把 "" 赋回并落盘）。 +// 本任务改为「按需校验」后，仅传 --profile 不再触发校验；凭据变更(--public-key)会触发 +// 对存量 region+project 的校验。网关 GetRegion 返成功、GetProjectList 返失败，精确 +// 构造「project 校验触发且失败」：fail-closed → 不落盘 → 存量 org-123 不被清空。 func TestConfigUpdateKeepsProjectIDWhenProjectValidationFails(t *testing.T) { t.Setenv("COMP_LINE", "1") gateway := fakeGatewayServerWith(t, gatewayBehavior{projectResp: respSignatureFail}) @@ -398,7 +400,7 @@ func TestConfigUpdateKeepsProjectIDWhenProjectValidationFails(t *testing.T) { cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) cmd := NewCmdConfigUpdate() - setFlags(t, cmd, "profile", "up") + setFlags(t, cmd, "profile", "up", "public-key", "NEWKEY") cmd.Run(cmd, nil) got, ok := reloadProfile(t, cfgPath, credPath, "up") @@ -410,9 +412,10 @@ func TestConfigUpdateKeepsProjectIDWhenProjectValidationFails(t *testing.T) { } } -// 回归 AC6：config 主命令（非 add/update）的校验失败同样不得落盘。 -// 现状它用 else 保留原 region 后照常 UpdateAggConfig，落盘了同一条命令里的其他改动 -// （此处为 timeout-sec），与 add/update 的 fail-closed 口径不一致。 +// 回归 AC6（config-cmd-audit）：config 主命令改存量、确实发起的校验失败时不得落盘。 +// 本任务改为「按需校验」后，纯元数据编辑不再触发校验，故此处用 --region 触发校验并 +// bundle 一个 --timeout-sec：校验失败时，同一条命令里的元数据改动也必须一并不落盘 +// （fail-closed 不因本任务回退，prd R4）。 func TestConfigMainCommandWritesNothingWhenValidationFails(t *testing.T) { t.Setenv("COMP_LINE", "1") gateway := fakeGatewayServerWith(t, gatewayBehavior{regionResp: respSignatureFail}) @@ -423,7 +426,7 @@ func TestConfigMainCommandWritesNothingWhenValidationFails(t *testing.T) { cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) cmd := NewCmdConfig() - setFlags(t, cmd, "profile", "main", "timeout-sec", "30") + setFlags(t, cmd, "profile", "main", "region", "cn-sh2", "timeout-sec", "30") cmd.Run(cmd, nil) got, ok := reloadProfile(t, cfgPath, credPath, "main") @@ -433,6 +436,9 @@ func TestConfigMainCommandWritesNothingWhenValidationFails(t *testing.T) { if got.Timeout != 15 { t.Errorf("a failed 'ucloud config' must write nothing; got timeout_sec=%d, want 15 (unchanged)", got.Timeout) } + if got.Region != "cn-bj2" { + t.Errorf("a failed region validation must not persist the new region; got %q, want cn-bj2 (unchanged)", got.Region) + } } // 回归 D10：config add 的本地 timeout 检查必须先于远程校验。 @@ -538,3 +544,158 @@ func TestConfigAddAllowsAccountWithoutDefaultProject(t *testing.T) { t.Errorf("region/zone must survive; got region=%q zone=%q", got.Region, got.Zone) } } + +// AC1（复现→守卫后转绿）：config update 只改元数据（--active）时跳过远程校验，离线可改。 +// 存量 base_url 指向毒网关：一旦发起校验就会打到它 → t.Errorf 判红。守卫前无条件校验必红 +// （复现成立），守卫后跳过校验、零远程调用、落盘成功转绿。 +// 需第二个 active profile "keep" 作锚点：up 由 active→inactive 后仍须有 active profile， +// 否则重新读盘因「no active config found」失败（config.go:403）。 +func TestConfigUpdateSkipsValidationForMetadataOnly(t *testing.T) { + t.Setenv("COMP_LINE", "1") + gateway := poisonGateway(t) + t.Cleanup(gateway.Close) + + cliJSON := fmt.Sprintf(`[{"profile":"keep","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":"https://api.ucloud.cn/","timeout_sec":15,"max_retry_times":3},{"profile":"up","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":%q,"timeout_sec":15,"max_retry_times":3}]`, gateway.URL) + credJSON := `[{"public_key":"pub","private_key":"pri","profile":"keep"},{"public_key":"pub","private_key":"pri","profile":"up"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfigUpdate() + setFlags(t, cmd, "profile", "up", "active", "false") + cmd.Run(cmd, nil) + + got, ok := reloadProfile(t, cfgPath, credPath, "up") + if !ok { + t.Fatal("profile up missing after reload") + } + if got.Active { + t.Error("only-metadata 'config update --active false' must persist offline without any remote call; got active=true") + } +} + +// AC2（复现→守卫后转绿）：config update 只改 --timeout-sec（元数据）时跳过远程校验， +// 即便 base_url 指向坏地址也能离线改成功。守卫前无条件校验会打坏网关 → 失败 → 硬拦、 +// timeout 改不了（复现红）；守卫后跳过校验、落盘成功转绿。单 active profile 全程不变， +// 重新读盘无「no active config found」之虞。 +func TestConfigUpdateOfflineTimeoutChange(t *testing.T) { + t.Setenv("COMP_LINE", "1") + // 存量 base_url 指向必然连不通的地址：一旦发起校验必失败 + cliJSON := `[{"profile":"up","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":"http://127.0.0.1:1/","timeout_sec":15,"max_retry_times":3}]` + credJSON := `[{"public_key":"pub","private_key":"pri","profile":"up"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfigUpdate() + setFlags(t, cmd, "profile", "up", "timeout-sec", "30") + cmd.Run(cmd, nil) + + got, ok := reloadProfile(t, cfgPath, credPath, "up") + if !ok { + t.Fatal("profile up missing after reload") + } + if got.Timeout != 30 { + t.Errorf("only-metadata 'config update --timeout-sec 30' must persist offline; got timeout_sec=%d, want 30", got.Timeout) + } +} + +// AC5（复现→守卫后转绿）：主命令 config 改存量 profile 且只改元数据（--active）时跳过校验。 +// p1 已存在 → ok==true → validateRegion/Project 皆 false → 毒网关零调用。keep 作 active +// 锚点，使 p1 由 inactive→active 为可见变更，且切换后仍有 active profile 供重新读盘。 +func TestConfigMainSkipsValidationForExistingMetadataOnly(t *testing.T) { + t.Setenv("COMP_LINE", "1") + gateway := poisonGateway(t) + t.Cleanup(gateway.Close) + + cliJSON := fmt.Sprintf(`[{"profile":"keep","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":"https://api.ucloud.cn/","timeout_sec":15,"max_retry_times":3},{"profile":"p1","active":false,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":%q,"timeout_sec":15,"max_retry_times":3}]`, gateway.URL) + credJSON := `[{"public_key":"pub","private_key":"pri","profile":"keep"},{"public_key":"pub","private_key":"pri","profile":"p1"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfig() + setFlags(t, cmd, "profile", "p1", "active", "true") + cmd.Run(cmd, nil) + + got, ok := reloadProfile(t, cfgPath, credPath, "p1") + if !ok { + t.Fatal("profile p1 missing after reload") + } + if !got.Active { + t.Error("main 'config --active true' on an existing profile must persist offline without validation; got active=false") + } +} + +// AC3（反向，守卫前后都应绿）：config update 传了 --region 时仍必须校验——防止把该校验的 +// 也跳了。网关 GetRegion 返 171，传 --region cn-sh2：校验触发且失败 → fail-closed 不落盘， +// 存量 region 不变。 +func TestConfigUpdateStillValidatesOnRegionChange(t *testing.T) { + t.Setenv("COMP_LINE", "1") + gateway := fakeGatewayServerWith(t, gatewayBehavior{regionResp: respSignatureFail}) + t.Cleanup(gateway.Close) + + cliJSON := fmt.Sprintf(`[{"profile":"up","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":%q,"timeout_sec":15,"max_retry_times":3}]`, gateway.URL) + credJSON := `[{"public_key":"pub","private_key":"pri","profile":"up"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfigUpdate() + setFlags(t, cmd, "profile", "up", "region", "cn-sh2") + cmd.Run(cmd, nil) + + got, ok := reloadProfile(t, cfgPath, credPath, "up") + if !ok { + t.Fatal("profile up missing after reload") + } + if got.Region != "cn-bj2" { + t.Errorf("a --region change must still be validated; a failed validation must not persist. got region=%q, want cn-bj2 (unchanged)", got.Region) + } +} + +// AC4（反向，最关键，守卫前后都应绿）：config update 改凭据（--public-key）必须同时触发 +// region 与 project 两个校验。网关 GetRegion 返成功、GetProjectList 返 171：守卫正确时 +// 凭据变更会触发 project 校验并失败 → fail-closed 不落盘，新 public-key 不落盘。 +// 若守卫漏了「凭据变更 → 校验 project」，project 不被校验 → 命令直接落盘新 key → 断言判红。 +func TestConfigUpdateValidatesBothOnCredChange(t *testing.T) { + t.Setenv("COMP_LINE", "1") + gateway := fakeGatewayServerWith(t, gatewayBehavior{projectResp: respSignatureFail}) + t.Cleanup(gateway.Close) + + cliJSON := fmt.Sprintf(`[{"profile":"up","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":%q,"timeout_sec":15,"max_retry_times":3}]`, gateway.URL) + credJSON := `[{"public_key":"OLD_PUB","private_key":"pri","profile":"up"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfigUpdate() + setFlags(t, cmd, "profile", "up", "public-key", "NEW_PUB") + cmd.Run(cmd, nil) + + got, ok := reloadProfile(t, cfgPath, credPath, "up") + if !ok { + t.Fatal("profile up missing after reload") + } + if got.PublicKey != "OLD_PUB" { + t.Errorf("a credential change must trigger project validation; a failed project validation must not persist. got public_key=%q, want OLD_PUB (unchanged)", got.PublicKey) + } +} + +// AC5 补充（反向，守卫前后都应绿）：主命令 config 新建 profile（!ok）时必须无条件校验。 +// 新 profile 的 region/project 须从零建立，不得因「无 Changed 标志」而跳过。 +// 网关 GetRegion 返 171 → 新建被拒、不落盘。 +func TestConfigMainStillValidatesNewProfile(t *testing.T) { + t.Setenv("COMP_LINE", "1") + gateway := fakeGatewayServerWith(t, gatewayBehavior{regionResp: respSignatureFail}) + t.Cleanup(gateway.Close) + + cliJSON := `[{"profile":"keep","active":true,"project_id":"org-123","region":"cn-bj2","zone":"cn-bj2-04","base_url":"https://api.ucloud.cn/","timeout_sec":15,"max_retry_times":3}]` + credJSON := `[{"public_key":"pub","private_key":"pri","profile":"keep"}]` + cfgPath, credPath := newTestConfigFiles(t, cliJSON, credJSON) + + cmd := NewCmdConfig() + setFlags(t, cmd, + "profile", "fresh", + "public-key", "pub", + "private-key", "pri", + "base-url", gateway.URL, + "region", "cn-bj2", + "zone", "cn-bj2-04", + ) + cmd.Run(cmd, nil) + + if got, ok := reloadProfile(t, cfgPath, credPath, "fresh"); ok { + t.Errorf("a new profile must be validated unconditionally; a failed validation must not create it. got region=%q", got.Region) + } +}