Skip to content

[WIP]fix enum vars in image filed for deploy - #4833

Open
PetrusZ wants to merge 1 commit into
koderover:mainfrom
PetrusZ:fix/deploy_enum_var_main
Open

[WIP]fix enum vars in image filed for deploy#4833
PetrusZ wants to merge 1 commit into
koderover:mainfrom
PetrusZ:fix/deploy_enum_var_main

Conversation

@PetrusZ

@PetrusZ PetrusZ commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What this PR does / Why we need it:

fix enum vars in image filed for deploy

What is changed and how it works?

fix enum vars in image filed for deploy

Does this PR introduce a user-facing change?

  • API change
  • database schema change
  • upgrade assistant change
  • change in non-functional attributes such as efficiency or availability
  • fix of a previous issue

This change is Reviewable

Signed-off-by: Patrick Zhao <zhaoyu@koderover.com>
@lilianzhu

lilianzhu commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Zadig AI Review

✅ 审查通过

  • 审查范围:origin/mainpr4833
  • 变更文件:3
  • Findings:1
  • 模型:glm-5-2-260617
  • 严重级别:critical 0 / high 0 / medium 1 / low 0

Findings

1. [MEDIUM] 新服务渲染失败时错误处理与更新路径不一致,丢失日志和错误上下文

pkg/microservice/aslan/core/environment/service/environment.go:3046-3049 · correctness · confidence 0.85

当 prevSvc == nil(新服务)时,RenderEnvServiceNotSetImages 返回的错误被直接 return nil, err 返回,既没有调用 log.Errorf 记录日志,也没有通过 multierror.Append 包装进 errList。对比 prevSvc != nil 分支(第3038-3042行),该分支会先 log.Errorf 再将错误附带服务名信息追加到 errList 后返回。errList 拥有自定义 ErrorFormat(第3005-3021行),能为创建服务失败提供带服务名的格式化错误信息。直接返回原始 err 导致丢失服务名上下文和自定义错误格式,同时下游调用方若依赖 multierror 类型断言或错误格式也会受到影响。

证据

	} else {
		// When prevSvc is nil, it means this is a new service.
		// So we need to render the service yaml without setting the images.
		parsedYaml, err = kube.RenderEnvServiceNotSetImages(env, newService.GetServiceRender(), newService)
		if err != nil {
			return nil, err
		}

建议

与 prevSvc != nil 分支保持一致的错误处理方式:先 log.Errorf 记录日志,再将错误追加到 errList 并返回 errList。例如:

if err != nil {
log.Errorf("Failed to render newService %s, error: %v", newService.ServiceName, err)
errList = multierror.Append(errList, fmt.Errorf("newService template %s error: %v", newService.ServiceName, err))
return nil, errList
}

@lilianzhu

Copy link
Copy Markdown
Collaborator

Zadig AI Review

⚠️ 审查未完整完成

  • 审查范围:origin/mainpr4833
  • 变更文件:3
  • Findings:2
  • 模型:glm-5-2-260617
  • 严重级别:critical 0 / high 1 / medium 0 / low 1

Findings

1. [HIGH]

pkg/microservice/aslan/core/common/service/kube/render.go:1140-3104 · correctness · confidence 0.80

当 prevSvc == nil 时,environment.go:3046 调用 RenderEnvServiceNotSetImages 生成 parsedYaml。此时 YAML 中所有 $<name>-image$ 占位符都未被替换(ReplaceWorkloadImages 和 ParseModuleImageKeys 均被跳过)。该 parsedYaml 随后被作为 UpdateResourceYaml 传入 ResourceApplyParam(line 3090),最终由 kube.CreateOrPatchResource(line 3103)执行集群资源创建/修补操作。如果 CreateOrPatchResource 直接用该 YAML 做 kubectl apply 或 server-side apply,未替换的 $<name>-image$ 占位符会被写入集群,导致 Deployment/StatefulSet 等工作负载引用无效镜像。

证据

environment.go:3046: parsedYaml, err = kube.RenderEnvServiceNotSetImages(env, newService.GetServiceRender(), newService)
...
3086: resourceApplyParam := &kube.ResourceApplyParam{
3088: ServiceName: newService.ServiceName,
3090: UpdateResourceYaml: parsedYaml,
...
3103: return kube.CreateOrPatchResource(resourceApplyParam, log)

建议

确认 kube.CreateOrPatchResource 在使用 UpdateResourceYaml 之前会做镜像替换,或者在 new-service 路径中改用带镜像替换的 RenderEnvService。如果 CreateOrPatchResource 内部有额外的镜像注入逻辑(如 InjectSecrets 路径),请确认该逻辑能正确处理 $<name>-image$ 占位符。

2. [LOW]

pkg/microservice/aslan/core/common/service/kube/render.go:1140-1142 · compatibility · confidence 0.85

新增的公共函数 RenderEnvServiceWithTemplNotSetImages 目前在整个仓库中没有任何调用方。如果该函数是为未来调用预留的,未使用的公共 API 可能会在后续被误用——调用方可能未意识到返回的 YAML 包含未替换的 $<name>-image$ 占位符,直接将其用于 kubectl apply 或 helm install,导致无效镜像被部署到集群。

证据

func RenderEnvServiceWithTemplNotSetImages(prod *commonmodels.Product, serviceRender *template.ServiceRender, service *commonmodels.ProductService, svcTmpl *commonmodels.Service, clusterName string) (yaml string, err error) {
return renderEnvServiceWithTempl(prod, serviceRender, service, svcTmpl, clusterName, false)
}

建议

如果当前没有调用方,考虑移除该函数以避免误用风险;或者添加清晰的文档注释说明返回的 YAML 中镜像占位符未替换,调用方必须自行处理镜像替换后才能用于集群部署。

Errors

Warnings

  • finding_localization_failed: localized content is incomplete for c-0

@landylee007
landylee007 requested review from Cynthia-0203 and removed request for Cynthia-0203 July 31, 2026 09:48
@landylee007 landylee007 changed the title fix enum vars in image filed for deploy [WIP]fix enum vars in image filed for deploy Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants