feat(chart): 添加 Helm Chart 部署方案#445
Conversation
新增 Helm Chart 支持完整的 SkillHub 私有化部署,包括: - PostgreSQL/Redis 内置 StatefulSet 及外部模式切换 - 零依赖设计,无需 Bitnami 子 Chart - 支持 standalone/cluster 数据库架构 - NodePort/LoadBalancer/ClusterIP 多种服务类型 - HPA、PDB、ServiceMonitor 完整运维支持 - cert-manager 证书自动签发 - initContainer 等待数据库和 Redis 就绪 - PVC 卸载保护 (helm.sh/resource-policy: keep) - GitHub Actions: PR 校验 + 发布到 GHCR OCI
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Helm chart for the SkillHub application, covering backend, frontend, and scanner components along with internal PostgreSQL and Redis statefulsets. The review identified several critical security and stability issues: sensitive credentials for Redis and S3 are exposed as plain text environment variables, and the use of randAlphaNum for secrets without persistence checks will cause authentication failures during Helm upgrades. Furthermore, the PostgreSQL configuration for multiple replicas is invalid for the base image used and risks data corruption. Other feedback includes the need for configuration checksums to trigger rolling updates, fixing hardcoded usernames in health probes, and avoiding the latest image tag for production stability.
| spring-datasource-password: {{ if eq .Values.database.mode "internal" }}{{ default (randAlphaNum 16) .Values.secrets.springDatasourcePassword }}{{ else }}{{ .Values.database.external.password }}{{ end }} | ||
|
|
||
| bootstrap-admin-password: {{ .Values.secrets.bootstrapAdminPassword | default .Values.bootstrapAdmin.password | default (randAlphaNum 16) }} |
There was a problem hiding this comment.
已修复。使用 lookup 函数检查集群中是否已存在该 Secret,若存在则保留原值,仅在首次安装时生成随机密码。避免了 helm upgrade 因密码重新生成导致的数据库连接中断问题。
| - name: SPRING_DATA_REDIS_PASSWORD | ||
| value: {{ .Values.redis.external.password }} | ||
| {{- end }} |
There was a problem hiding this comment.
已修复。Redis 密码已从明文环境变量移除,改为通过 secretKeyRef 从 Secret 中引用。对应的 redis-password 字段已添加至 Secret 模板。
| - name: SKILLHUB_S3_ACCESS_KEY | ||
| value: {{ .Values.storage.s3.accessKey }} | ||
| {{- end }} | ||
| {{- if .Values.storage.s3.secretKey }} | ||
| - name: SKILLHUB_S3_SECRET_KEY | ||
| value: {{ .Values.storage.s3.secretKey }} | ||
| {{- end }} |
There was a problem hiding this comment.
已修复。S3 的 AccessKey 和 SecretKey 已从 Deployment 明文配置移除,改为通过 secretKeyRef 从 Secret 中引用。对应的 s3-access-key 和 s3-secret-key 字段已在 Secret 模板中定义。
| labels: | ||
| {{- include "skillhub.server.selectorLabels" . | nindent 8 }} | ||
| annotations: | ||
| {{- toYaml .Values.server.podAnnotations | nindent 8 }} |
There was a problem hiding this comment.
建议在 Pod 模板的注解中添加 ConfigMap 和 Secret 的校验和(checksum)。这样当配置发生变化时,Deployment 会自动触发 Pod 的滚动更新。如果没有这些注解,用户修改配置后必须手动重启 Pod 才能生效。
checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}
checksum/secret: {{ include (print $.Template.BasePath "/secret.yaml") . | sha256sum }}
{{- toYaml .Values.server.podAnnotations | nindent 8 }}There was a problem hiding this comment.
已修复。已在 server/web/scanner 三个 Deployment 的 Pod 注解中添加 checksum/config 和 checksum/secret,基于 ConfigMap 和 Secret 相关的 values 子集计算 SHA256。配置变更后 Deployment 将自动触发滚动更新。
| echo "Redis is ready!" | ||
| containers: | ||
| - name: server | ||
| image: {{ .Values.images.registry }}/skillhub-server:{{ .Values.images.tag }} |
There was a problem hiding this comment.
已修复。server/web/scanner 三个 Deployment 的镜像地址均已改为 {{ .Values.global.imageRegistry | default .Values.images.registry }},优先使用全局 registry 覆盖。
| app.kubernetes.io/component: database | ||
| spec: | ||
| serviceName: {{ include "skillhub.fullname" . }}-postgres | ||
| replicas: {{ if eq .Values.database.architecture "cluster" }}3{{ else }}1{{ end }} |
There was a problem hiding this comment.
已修复。internal 模式下 PostgreSQL StatefulSet 的 replicas 强制为 1,移除了 architecture: cluster 时设为 3 的逻辑。内置 postgres:16-alpine 镜像不支持自动集群化,如需高可用请使用 external 模式连接外部 PostgreSQL 集群(如 Patroni)。
| - skillhub | ||
| - -h | ||
| - localhost | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 10 | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - pg_isready | ||
| - -U | ||
| - skillhub |
There was a problem hiding this comment.
已修复。探针命令已改用 sh -c "pg_isready -U \"${POSTGRES_USER}\" -h localhost",通过环境变量动态获取数据库用户名,不再硬编码。
| # ============================================================================ | ||
| images: | ||
| registry: ghcr.io/iflytek | ||
| tag: latest |
There was a problem hiding this comment.
已修复。默认镜像标签已从 latest 改为 v0.2.8,支持可追溯的版本回滚。
- secret.yaml: lookup 检查现有 Secret 避免 upgrade 重新生成密码 - secret.yaml: Redis/S3 凭据通过 Secret 引用,移除明文环境变量 - backend/frontend/scanner: 新增 checksum 注解,配置变更自动触发滚动更新 - backend/frontend/scanner: 镜像地址支持 global.imageRegistry 覆盖 - postgres: internal 模式仅支持单副本,移除伪集群配置 - postgres: 探针用户名改用 POSTGRES_USER 环境变量 - values.yaml: accessMode 默认 ReadWriteMany,tag 指定 v0.2.8
values.yaml 中 images.tag 留空时自动取 Chart.yaml 的 appVersion,
格式为 v{appVersion}(如 0.2.8 → v0.2.8)。
用户仍可通过 --set images.tag=xxx 显式覆盖。
|
您好!这个 PR 添加了 Helm Chart 部署方案与 CI/CD 工作流,包括:
当前有 workflows 需要您的批准才能运行,烦请批准,谢谢! |
done |
- 修复 S3 存储模式下卷挂载条件渲染 - CI 多行参数不再被 YAML 尾随换行符截断 - 移除未使用的 database.architecture 字段 - 简化 Helm Chart 发布工作流
|
您好!已修复 PR 中的 CI 问题并提交新 commit,包括:
当前需要您再次批准 workflows 运行,烦请批准,谢谢! |
XiaoSeS
left a comment
There was a problem hiding this comment.
整体看下来完成度很高,架构设计合理,CI 的 9 场景矩阵覆盖也很全面。命名规范、资源结构、values 层级都很专业,没什么可挑的。下面是我读完代码后的一些反馈:
一个必须修的问题
Secret lookup 的 key 对不上(helm upgrade 会清空密码)
secret.yaml 里 stringData 用的是 kebab-case:
stringData:
spring-datasource-password: ...
bootstrap-admin-password: ...
redis-password: ...但 lookup 已有 Secret 时用了 camelCase 点号访问:
{{- $dsPwd = $existingSecret.data.springDatasourcePassword | b64dec }}
{{- $baPwd = $existingSecret.data.bootstrapAdminPassword | b64dec }}Go template 的点号语法访问不了带 - 的 map key,所以这些表达式永远返回 nil。效果是:第一次 install 正常(走 randAlphaNum),但第二次 upgrade 时 $existingSecret 非空、走 lookup 分支、读到 nil、b64dec 得到空字符串 → 密码被覆盖为空 → 服务挂掉。
修复方式:
{{- $dsPwd = index $existingSecret.data "spring-datasource-password" | b64dec }}
{{- $baPwd = index $existingSecret.data "bootstrap-admin-password" | b64dec }}
{{- $redisPwd = index $existingSecret.data "redis-password" | b64dec }}
{{- $s3Key = index $existingSecret.data "s3-access-key" | b64dec }}
{{- $s3Secret = index $existingSecret.data "s3-secret-key" | b64dec }}几个建议改进的点
1. PG/Redis 的 VolumeClaimTemplate accessMode
目前引用的是 storage.local.accessMode(默认 ReadWriteMany),但 PG 和 Redis 都是单实例 StatefulSet,只需要 ReadWriteOnce。大多数云厂商默认 StorageClass 不支持 RWX,这样 PVC 会创建失败。
建议硬编码 ReadWriteOnce,或者在 database.internal / redis.internal 下各加一个独立的 accessMode 字段。
2. HPA 开启时 replicas 字段会打架
server.autoscaling.enabled: true 时,Deployment 仍然渲染了 replicas: {{ .Values.replicaCount }}。每次 helm upgrade 都会把副本数重置回去,和 HPA 冲突。
建议:
{{- if not .Values.server.autoscaling.enabled }}
replicas: {{ .Values.replicaCount }}
{{- end }}web 和 scanner 同理。
3. publish-chart workflow 缺 GHCR 登录
helm push 到 OCI registry 需要先登录,当前 workflow 里没有这步。建议加:
- name: Login to GHCR
run: echo "${{ secrets.GITHUB_TOKEN }}" | helm registry login ghcr.io -u ${{ github.actor }} --password-stdin4. busybox:latest 不可追溯
initContainer 用的 busybox:latest 在生产环境不好追溯版本,离线部署也拉不到。建议固定版本(比如 busybox:1.37)并走 registry 覆盖逻辑。
不阻塞但值得留意的
- certificate.yaml + Ingress annotation 重复签发:cert-manager 的 annotation 方式和 Certificate 资源方式二选一就行,同时用会重复签发证书
- 内置 Redis 没有密码保护:
redis-server --appendonly yes没加--requirepass,集群内任何 Pod 都能直连。如果是有意为之建议在文档里说明 - scanner Service 不需要外部暴露:它只被 server 内部调用,强制 ClusterIP 就够了,跟着全局
service.type走会意外暴露到 NodePort/LB - web/scanner 的 checksum annotation 范围过大:包含了 database/redis 等与它们无关的配置,会导致改数据库密码时 web 也跟着重启
总的来说这个 chart 写得挺扎实的,把 Secret lookup 那个 key 问题修了就可以合了 👍
XiaoSeS
left a comment
There was a problem hiding this comment.
补充一个架构层面的建议(这次比较大,但因为 chart 是新引入、还没人在生产用,调整成本最低):
核心建议:拆成 Umbrella Chart + Subchart 结构,PG/Redis 用 Bitnami 依赖
当前实现的隐患
postgres-statefulset.yaml 和 redis-statefulset.yaml 自己写的版本只能算"能跑起来"的级别,缺很多生产级能力:
- 没有主从复制 / HA
- 没有自动备份
- 没有 Prometheus exporter
- 没有 TLS、连接池、慢查询日志
- 主版本升级没有数据迁移工具
短期看是少写了几十行 YAML,长期看每一个生产部署的运维都要自己补这些能力。
推荐的目录结构
charts/skillhub/
├── Chart.yaml ← 声明 5 个 dependency
├── Chart.lock ← 自动生成
├── values.yaml ← 全局 + 各 subchart 配置覆盖
├── templates/ ← 只剩"胶水"层
│ ├── _helpers.tpl
│ ├── secret.yaml ← 共享 Secret
│ ├── configmap.yaml ← 共享 ConfigMap
│ ├── ingress.yaml
│ └── certificate.yaml
└── charts/
├── server/ ← 自写 subchart
│ ├── Chart.yaml
│ ├── values.yaml
│ └── templates/
│ ├── deployment.yaml
│ ├── service.yaml
│ ├── hpa.yaml
│ └── pdb.yaml
├── web/ ← 自写 subchart
├── scanner/ ← 自写 subchart
├── postgresql-16.x.x.tgz ← Bitnami 依赖(helm dependency update 自动下载)
└── redis-20.x.x.tgz ← Bitnami 依赖
Chart.yaml
apiVersion: v2
name: skillhub
version: 0.3.0
appVersion: 0.3.0
dependencies:
- name: server
version: "0.3.0"
repository: "file://./charts/server"
- name: web
version: "0.3.0"
repository: "file://./charts/web"
- name: scanner
version: "0.3.0"
repository: "file://./charts/scanner"
condition: scanner.enabled
- name: postgresql
version: "16.4.x"
repository: "oci://registry-1.docker.io/bitnamicharts"
condition: postgresql.enabled
- name: redis
version: "20.x.x"
repository: "oci://registry-1.docker.io/bitnamicharts"
condition: redis.enabledvalues.yaml 关键改动
postgresql:
enabled: true
auth:
database: skillhub
username: skillhub
existingSecret: "" # 复用父 chart 的 Secret
secretKeys:
userPasswordKey: spring-datasource-password
primary:
persistence:
size: 10Gi
redis:
enabled: true
architecture: standalone
auth:
enabled: true
existingSecret: ""
existingSecretPasswordKey: redis-password
master:
persistence:
size: 5Gi
# postgresql.enabled=false 时使用
externalDatabase:
host: ""
port: 5432
database: skillhub收益
| 维度 | 当前 | 重构后 |
|---|---|---|
| PG/Redis 维护负担 | 自己写、自己修 | Bitnami 维护,零成本 |
| HA 能力 | 无 | architecture: replication 一行开启 |
| 监控/备份/TLS | 无 | values 配置即可启用 |
| 升级粒度 | 整体升级 | 子 chart 独立版本 |
| 新增组件 | 加文件到同一个 templates/ | 新建子 chart 目录 |
| 代码量 | +1666 | 估计 +1200,且 80% 是子 chart 隔离的 |
CI 适配
publish workflow 加一步:
- name: Update dependencies
run: helm dependency update ./charts/skillhubPR workflow 同理,所有 helm template / helm lint 之前都要先 update。
如果觉得这个方向 OK,我可以帮忙起一份完整的重构草稿。如果觉得改动太大、想保持当前架构,那至少建议把之前那个 Secret lookup key 的 bug 修了,然后内置 PG/Redis 在文档里明确标注"仅适合开发/小规模场景,生产环境推荐 external 模式"。
- 使用 Bitnami PostgreSQL/Redis subchart 替代内置 StatefulSet - 新增 sentinel 模式密码分离(redis-sentinel-password) - 修复证书 secretName 与 Ingress 动态一致性 - 清理 ConfigMap 未引用字段,Service 模板去重 - CI 矩阵修复 sentinel 参数并扩展至 9 场景 - 命名空间硬编码替换为动态 $.Release.Namespace
- RedissonConfig 根据 spring.profiles.active 切换普通/Sentinel密码 - 新增 application-redis-sentinel.yml 专属 Spring profile - 新增 RedissonConfigTest 覆盖普通和 Sentinel 两种模式用例
|
@XiaoSeS,感谢 review。以下是对各项建议的处理情况: 架构层面已按建议采用 Bitnami PostgreSQL/Redis 作为 OCI 依赖。代码层面的问题已逐项处理:
补充说明一个适配问题——Redis Sentinel 模式下的密码兼容: Bitnami sentinel 节点的 requirepass 实际使用
|
概述
添加 Helm Chart 部署支持。
核心特性
安装示例
CI/CD