feat(deploy): k3s/k8s one-click deploy script and cch management CLI#1048
Conversation
将本地 k3s/k8s 部署资产迁入仓库,对齐现有 scripts/deploy.sh 的一键体验,
支持 k3s 与标准 Kubernetes (EKS/GKE/AKS) 双兼容。
新增:
- scripts/deploy-k8s.sh: 一键部署引导(CLI 参数 + 交互回退 + 幂等升级),
自动探测 runtime、StorageClass、Ingress 变体 (Traefik IngressRoute /
标准 Ingress / NodePort 兜底),支持 --install-k3s 自动装 k3s
- scripts/cch: 运维管理 CLI,包含 update/restart/rollback/scale/backup/
info/doctor/uninstall 等子命令,支持配置文件与 env 覆盖
- deploy/k8s/**: 15 份 manifest 模板(namespace / app / postgres / redis
/ ingress),使用 {{VAR}} 占位符,部署时由 python 统一渲染
- docs/k8s-deployment.md: 10 节完整部署运维指南,含架构图、参数参考、
云厂商 StorageClass 映射、故障排查
- README.md / README.en.md: 新增 Kubernetes / k3s 部署章节
关键兼容与安全改造(经三轮 Codex 审查):
- BSD/macOS 兼容:去除 cp -RT / date -Iseconds / xargs -r 的 GNU 专属用法;
新增 b64d() 兼容 GNU/BSD base64
- Secret 不再通过 --from-literal 传参(避免进程表泄露),改为 600 权限临时
文件 + --from-file
- 去除独立 migration Job(drizzle-kit 为 devDependency 在运行时不可用),
改由应用 AUTO_MIGRATE=true 启动时自动迁移,避免并发竞态
- App rollout 超时 + 升级模式下自动 rollout undo + HPA/副本数恢复
- 非交互场景 backup 失败直接中止;uninstall 需 CCH_CONFIRM_UNINSTALL=<ns>
显式授权
- cch RUNTIME_OVERRIDE=k3s 且无 kubectl 时自动回退 sudo k3s kubectl
- 新增 --disable-networkpolicy flag,适配 Ingress Controller 在非标准
namespace 的集群
验证:
- bash -n 两个脚本通过
- kubectl apply --dry-run=client 15/15 manifest 通过
- --dry-render 覆盖 NodePort 与 Ingress 两种场景,占位符残留 = 0
- 三轮 Codex 审查最终 10/10,无剩余阻断项
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough新增面向 Kubernetes/k3s 的一键部署与运维套件：包含可渲染的 Kubernetes 清单模板（app、Postgres、Redis、Ingress、Namespace、NetworkPolicy、HPA、PDB 等）、部署脚本 Changes
预期代码审查工作量🎯 4 (Complex) | ⏱️ ~75 分钟 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Kubernetes and k3s deployment solution for Claude Code Hub. It includes production-ready manifests for the application, PostgreSQL, and Redis, along with a one-click deployment script and a management CLI (cch) for operations like rolling updates and backups. Documentation has been updated to cover these new deployment options. Feedback highlighted a non-existent PostgreSQL image version (18) and recommended using environment variables for Redis authentication in probes to avoid exposing credentials in process lists.
| fsGroup: 999 | ||
| containers: | ||
| - name: postgres | ||
| image: postgres:18 |
There was a problem hiding this comment.
PostgreSQL 18 目前尚未发布（当前最新的稳定主版本为 17）。使用不存在的镜像版本会导致 Kubernetes 无法拉取镜像，Pod 将卡在 ImagePullBackOff 状态。建议将其修正为 postgres:17 或 postgres:17-alpine。
image: postgres:17There was a problem hiding this comment.
Checked current upstream release status before changing it: PostgreSQL 18 is already GA, so I kept postgres:18 instead of downgrading. The actionable part in this area was the memory/timeout tuning, which is fixed in e185cc3e.
| command: | ||
| - sh | ||
| - -c | ||
| - redis-cli -a "$REDIS_PASSWORD" ping | grep -q PONG |
There was a problem hiding this comment.
在命令行中直接使用 -a 参数传递密码是不安全的，因为密码会以明文形式出现在容器的进程列表（如 ps 命令输出）中。建议改用 REDISCLI_AUTH 环境变量来安全地传递密码。
- REDISCLI_AUTH="$REDIS_PASSWORD" redis-cli ping | grep -q PONGThere was a problem hiding this comment.
Fixed in e185cc3e: both Redis probes now use REDISCLI_AUTH plus --no-auth-warning, so the password no longer lands in redis-cli -a ... argv.
| command: | ||
| - sh | ||
| - -c | ||
| - redis-cli -a "$REDIS_PASSWORD" ping | grep -q PONG |
There was a problem hiding this comment.
同上，建议在就绪检查（Readiness Probe）中使用 REDISCLI_AUTH 环境变量代替命令行参数 -a，以避免在进程表中泄露敏感凭据。
- REDISCLI_AUTH="$REDIS_PASSWORD" redis-cli ping | grep -q PONGThere was a problem hiding this comment.
Fixed in e185cc3e: both Redis probes now use REDISCLI_AUTH plus --no-auth-warning, so the password no longer lands in redis-cli -a ... argv.
github-actions
Bot
added
enhancement
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - podSelector: | ||
| matchLabels: | ||
| component: migration | ||
| ports: | ||
| - port: 5432 |
There was a problem hiding this comment.
The migration Job was intentionally removed in this PR (deploy/k8s/jobs/ no longer exists), yet this NetworkPolicy rule still permits any pod labeled component: migration to reach Postgres on port 5432. Any future pod (accidental or malicious) in the namespace that carries that label can now bypass the intended network isolation without restriction.
| - podSelector: | |
| matchLabels: | |
| component: migration | |
| ports: | |
| - port: 5432 | |
| # Migration Job has been removed; app uses AUTO_MIGRATE=true on startup. | |
| # If a migration job is re-introduced, add its label here. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: deploy/k8s/postgres/networkpolicy.yaml
Line: 20-24
Comment:
**Stale migration-pod allowlist creates unnecessary access path**
The migration Job was intentionally removed in this PR (`deploy/k8s/jobs/` no longer exists), yet this NetworkPolicy rule still permits any pod labeled `component: migration` to reach Postgres on port 5432. Any future pod (accidental or malicious) in the namespace that carries that label can now bypass the intended network isolation without restriction.
```suggestion
# Migration Job has been removed; app uses AUTO_MIGRATE=true on startup.
# If a migration job is re-introduced, add its label here.
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in e185cc3e: removed the stale component: migration allowlist from the Postgres NetworkPolicy.
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - sh | ||
| - -c | ||
| - redis-cli -a "$REDIS_PASSWORD" ping | grep -q PONG | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - sh | ||
| - -c | ||
| - redis-cli -a "$REDIS_PASSWORD" ping | grep -q PONG | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 5 | ||
| timeoutSeconds: 3 |
There was a problem hiding this comment.
Both liveness and readiness probes execute redis-cli -a "$REDIS_PASSWORD" ping. At runtime the shell expands $REDIS_PASSWORD into the actual password, which then appears in /proc/<pid>/cmdline and is readable by any process that can exec into the pod. A safer alternative is to read the password from a file via a mounted secret volume. At minimum, redis-cli --no-auth-warning suppresses the warning but does not remove the process-table exposure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: deploy/k8s/redis/statefulset.yaml
Line: 64-81
Comment:
**Redis password exposed in probe process arguments**
Both liveness and readiness probes execute `redis-cli -a "$REDIS_PASSWORD" ping`. At runtime the shell expands `$REDIS_PASSWORD` into the actual password, which then appears in `/proc/<pid>/cmdline` and is readable by any process that can exec into the pod. A safer alternative is to read the password from a file via a mounted secret volume. At minimum, `redis-cli --no-auth-warning` suppresses the warning but does not remove the process-table exposure.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in e185cc3e: both Redis probes now use REDISCLI_AUTH plus --no-auth-warning, so the password no longer lands in process arguments.
| if ! curl -fsSL https://get.k3s.io | sh -; then | ||
| log_error "k3s 安装失败"; exit 1 |
There was a problem hiding this comment.
curl | sh)
curl -fsSL https://get.k3s.io | sh - executes whatever the remote server returns without any integrity check. A compromised CDN, MITM, or supply-chain incident would give the remote host root code execution on the target machine. Consider adding a checksum verification step or at minimum documenting that the user should verify the script before piping.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/deploy-k8s.sh
Line: 325-326
Comment:
**Remote script piped to shell (`curl | sh`)**
`curl -fsSL https://get.k3s.io | sh -` executes whatever the remote server returns without any integrity check. A compromised CDN, MITM, or supply-chain incident would give the remote host root code execution on the target machine. Consider adding a checksum verification step or at minimum documenting that the user should verify the script before piping.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressed in e185cc3e: added an explicit warning in install_k3s() and in the deployment guide that --install-k3s executes the official remote installer and should be reviewed before production use.
| - name: AUTO_MIGRATE | ||
| value: "true" |
There was a problem hiding this comment.
On a first-time deploy the deployment starts {{APP_REPLICAS}} (default 2) pods simultaneously, all with AUTO_MIGRATE=true. If Drizzle's migrate() does not acquire a Postgres advisory lock before applying schema changes, two concurrent migration runs can race on DDL statements. The cch update workflow correctly gates migration behind a single-replica scale-down, but the initial deploy-k8s.sh flow does not. Confirm that instrumentation.ts wraps migrate() in a Postgres advisory lock, or reduce spec.replicas to 1 in the template and rely on HPA to scale up post-deploy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: deploy/k8s/app/deployment.yaml
Line: 51-52
Comment:
**Concurrent AUTO_MIGRATE on fresh install with default replicas=2**
On a first-time deploy the deployment starts `{{APP_REPLICAS}}` (default 2) pods simultaneously, all with `AUTO_MIGRATE=true`. If Drizzle's `migrate()` does not acquire a Postgres advisory lock before applying schema changes, two concurrent migration runs can race on DDL statements. The `cch update` workflow correctly gates migration behind a single-replica scale-down, but the initial `deploy-k8s.sh` flow does not. Confirm that `instrumentation.ts` wraps `migrate()` in a Postgres advisory lock, or reduce `spec.replicas` to 1 in the template and rely on HPA to scale up post-deploy.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Checked the current implementation before changing defaults: src/lib/migrate.ts already serializes migrations with a PostgreSQL advisory lock. I kept the 2-replica baseline, documented that lock next to AUTO_MIGRATE, and clarified it in the k8s docs in e185cc3e.
| if [[ -r "$CCH_CONFIG_FILE" ]]; then | ||
| # shellcheck disable=SC1090 | ||
| source "$CCH_CONFIG_FILE" | ||
| fi |
There was a problem hiding this comment.
Sourcing the config file at CCH_CONFIG_FILE executes all shell content within it. An attacker who can write to that path (symlink attack, world-writable parent directory) gains code execution under the user running this script. A safer approach is parsing only assignment lines with a while IFS= read loop rather than unconditional sourcing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/cch
Line: 47-50
Comment:
**Arbitrary code execution via sourced config**
Sourcing the config file at `CCH_CONFIG_FILE` executes all shell content within it. An attacker who can write to that path (symlink attack, world-writable parent directory) gains code execution under the user running this script. A safer approach is parsing only assignment lines with a `while IFS= read` loop rather than unconditional sourcing.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in e185cc3e: scripts/cch now parses a small allowlist of CCH_* assignments instead of source-ing the config file.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e57b99440b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if [[ "$FORCE_NEW" == true ]]; then | ||
| UPDATE_MODE=false | ||
| return |
There was a problem hiding this comment.
In detect_existing_deployment, the --force-new path only flips UPDATE_MODE=false and returns, so no namespace/resources are deleted before re-applying manifests. That means a “force new install” still reuses existing StatefulSets/PVCs (so broken PVC state is not reset) while later logic treats it like a fresh install and regenerates secrets, which can leave existing data-plane resources and new credentials out of sync. This should either perform an explicit teardown/recreate flow or fail fast instead of pretending a reinstall happened.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e185cc3e: --force-new now performs an explicit namespace teardown/recreate flow instead of pretending a fresh install happened while reusing old resources.
| $KUBECTL -n "$NAMESPACE" set image deployment/claude-code-hub app="$IMAGE" >/dev/null | ||
| fi | ||
| fi | ||
| $KUBECTL -n "$NAMESPACE" rollout status deployment/claude-code-hub --timeout=600s |
There was a problem hiding this comment.
This rollout status call is unguarded under set -euo pipefail, so if rollout times out (e.g., image pull error or crashloop), cmd_update exits immediately before the rollback/replica-restore logic below runs. Because step 3 already forced the deployment/HPA down to a single replica, a failed upgrade can leave production stuck in a degraded state with no automatic undo. Wrap this command in an explicit failure branch that restores HPA/replicas and rolls back.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e185cc3e: rollout waits now go through an explicit helper that preserves rollback/replica-restore behavior and surfaces diagnostic commands on failure.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
deploy/k8s/postgres/networkpolicy.yaml (1)
19-24: 清理无效的 migration 来源规则，避免误导。Line 19-24 依赖
component: migration，但当前清单体系中未看到对应工作负载标签；这条规则很可能永不生效，后续排障会被误导。建议删除该段，或改为与实际迁移执行体一致的可配置标签。建议变更
ingress: - from: - podSelector: matchLabels: app: claude-code-hub ports: - port: 5432 - - from: - - podSelector: - matchLabels: - component: migration - ports: - - port: 5432🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/k8s/postgres/networkpolicy.yaml` around lines 19 - 24, 删除或替换 networkpolicy 中对不可达来源的静态规则：移除 from: podSelector: matchLabels: component: migration（以及对应 ports: - port: 5432）这段无效规则，或将其替换为与实际迁移工作负载一致且可配置的标签（例如将 matchLabels 改为可配置的 migrationLabel 变量或与实际 podLabel 对齐），确保引用的标识符（matchLabels -> component: migration）不再出现在 NetworkPolicy 中以避免误导。deploy/k8s/postgres/statefulset.yaml (1)
67-73: PostgreSQLshared_buffers=2GB与 request/limit 的匹配需关注。
shared_buffers=2GB属于确定会被 Postgres 启动时申请的共享内存,requests.memory=4Gi 尚有余量,但如果用户通过文档建议的"单机 4 vCPU / 8GB RAM"起步规格(含 Redis/app/系统开销)调度,pod 实际可用内存有限,limits=8Gi 触发 OOMKill 时 Postgres 会被直接杀掉。建议在文档里显式标注本 StatefulSet 的默认资源画像,或在脚本中提供--pg-shared-buffers覆盖,避免小规格单机 k3s 用户直接被 OOM。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/k8s/postgres/statefulset.yaml` around lines 67 - 73, The StatefulSet currently sets resources.requests.memory: 4Gi and limits.memory: 8Gi while Postgres starts with shared_buffers=2GB which can push small single-node clusters into OOM; update the deployment and docs: document the default resource profile (the resources.requests/limits block and the implied shared_buffers), and add an explicit override mechanism (e.g., a CLI flag or env var such as --pg-shared-buffers or PG_SHARED_BUFFERS read by the Postgres container entrypoint) so operators can reduce shared_buffers on small nodes; alternatively lower the default requests/limits to safer values for single-node k3s or emit a clear runtime warning in the StatefulSet/README about required node size to avoid OOMKills.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/k8s/app/networkpolicy.yaml`:
- Around line 15-26: The NetworkPolicy currently restricts ingress sources to
only specific namespaces (namespaceSelector with kubernetes.io/metadata.name:
kube-system, ingress-nginx, traefik) which breaks NodePort fallback; update the
template that renders this policy to conditionally include those
namespaceSelectors only when exposing via Ingress, and for NodePort mode either
add an ipBlock entry (configurable CIDR) to the ingress from list or
skip/disable the NetworkPolicy entirely; specifically modify the rendering logic
that emits the "from:" block and the ports: - port: 3000 stanza so it branches
on the exposure mode and injects a configurable ipBlock or removes the policy
for NodePort.
In `@deploy/k8s/app/pdb.yaml`:
- Line 7: The PDB uses a fixed minAvailable: 1 which blocks voluntary evictions
when APP_REPLICAS is 1; update the PDB template to be replica-aware: replace the
static minAvailable with a conditional rendering or computed field based on
APP_REPLICAS so that when APP_REPLICAS == 1 you use maxUnavailable: 0 (or omit
the PDB) and when APP_REPLICAS >= 2 set minAvailable appropriately (e.g.,
minAvailable: 1). Locate the PDB spec entry containing minAvailable in pdb.yaml
and implement the conditional logic (or switch to a maxUnavailable-based policy)
so drains/rolling updates are not blocked for single-replica deployments.
In `@deploy/k8s/ingress/ingress.yaml`:
- Line 13: The Traefik middleware annotation value
traefik.ingress.kubernetes.io/router.middlewares currently lacks quotes which
can make the template invalid YAML before {{NAMESPACE}} is rendered; update the
annotation to wrap the value in double quotes (e.g., "
{{NAMESPACE}}-streaming-headers@kubernetescrd ") and likewise wrap
spec.rules[].host ({{INGRESS_HOST}}) and spec.ingressClassName
({{INGRESS_CLASS}}) in quotes so the template is valid YAML for linters/static
checks while still rendering correctly at deploy time.
In `@deploy/k8s/postgres/statefulset.yaml`:
- Around line 63-66: The cluster-level Postgres flags "statement_timeout" and
"idle_in_transaction_session_timeout" in the StatefulSet cause long-running
migrations/backups to be killed; remove these two "-c" entries (the lines
setting statement_timeout=60000 and idle_in_transaction_session_timeout=30000)
from the Postgres startup args so the DB defaults are not applied to the
claude_code_hub role, or alternatively implement role-specific overrides by
running "ALTER ROLE claude_code_hub SET statement_timeout = 0; ALTER ROLE
claude_code_hub SET idle_in_transaction_session_timeout = 0;" in your DB
initialization script used for provisioning so AUTO_MIGRATE (drizzle-kit/drizzle
migrate) and cch backup are not interrupted.
In `@deploy/k8s/redis/statefulset.yaml`:
- Around line 29-63: The Redis config sets maxmemory 4gb which matches the
container limit (limits.memory: 4Gi) and can cause OOMKills; change the
redis.conf value for maxmemory to a safer fraction of the container limit (e.g.,
70–80%, such as ~3.2g) or increase the container memory limit above 4Gi, or make
maxmemory configurable via a template variable (e.g., {{REDIS_MAXMEMORY}}) so
deploy-k8s.sh can render it; update the value referenced in the redis container
spec (container name "redis", command "redis-server /etc/redis/redis.conf") and
ensure the limits.memory and maxmemory stay consistent after the change.
In `@docs/k8s-deployment.md`:
- Around line 340-355: The doc's step to patch HPA minReplicas to 0 is invalid
for CPU/Memory-based HPA and will be rejected; instead update the procedure to
delete the HPA resource before scaling down and restoring, and after the DB
restore recreate the HPA by running your deployment bootstrap (referenced as
deploy-k8s.sh) then scale the Deployment back up; specifically replace the patch
call that sets minReplicas:0 for the HPA named claude-code-hub with an explicit
deletion of the HPA, and add a step after restore to reapply manifests via
deploy-k8s.sh (which will recreate the HPA) before running cch scale.
In `@scripts/cch`:
- Around line 427-509: cmd_doctor currently never invokes detect_runtime, so
KUBECTL and RUNTIME may be unset and checks (e.g. "$KUBECTL cluster-info",
runtime==k3s) are inaccurate; fix by calling detect_runtime at the start of
cmd_doctor but without allowing a full process exit (e.g., run (detect_runtime)
|| true or call a non-fatal variant) so the function populates KUBECTL and
RUNTIME yet continues running diagnostics; update cmd_doctor to call
detect_runtime safely before any use of KUBECTL or RUNTIME (references:
cmd_doctor, detect_runtime, KUBECTL, RUNTIME).
---
Nitpick comments:
In `@deploy/k8s/postgres/networkpolicy.yaml`:
- Around line 19-24: 删除或替换 networkpolicy 中对不可达来源的静态规则：移除 from: podSelector:
matchLabels: component: migration（以及对应 ports: - port:
5432）这段无效规则，或将其替换为与实际迁移工作负载一致且可配置的标签（例如将 matchLabels 改为可配置的 migrationLabel
变量或与实际 podLabel 对齐），确保引用的标识符（matchLabels -> component: migration）不再出现在
NetworkPolicy 中以避免误导。
In `@deploy/k8s/postgres/statefulset.yaml`:
- Around line 67-73: The StatefulSet currently sets resources.requests.memory:
4Gi and limits.memory: 8Gi while Postgres starts with shared_buffers=2GB which
can push small single-node clusters into OOM; update the deployment and docs:
document the default resource profile (the resources.requests/limits block and
the implied shared_buffers), and add an explicit override mechanism (e.g., a CLI
flag or env var such as --pg-shared-buffers or PG_SHARED_BUFFERS read by the
Postgres container entrypoint) so operators can reduce shared_buffers on small
nodes; alternatively lower the default requests/limits to safer values for
single-node k3s or emit a clear runtime warning in the StatefulSet/README about
required node size to avoid OOMKills.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a85e6524-6703-4b6f-be44-34ab489be04f
📒 Files selected for processing (20)
README.en.mdREADME.mddeploy/k8s/README.mddeploy/k8s/app/deployment.yamldeploy/k8s/app/hpa.yamldeploy/k8s/app/networkpolicy.yamldeploy/k8s/app/pdb.yamldeploy/k8s/app/service.yamldeploy/k8s/ingress/ingress.yamldeploy/k8s/ingress/traefik-ingressroute.yamldeploy/k8s/namespace.yamldeploy/k8s/postgres/networkpolicy.yamldeploy/k8s/postgres/service.yamldeploy/k8s/postgres/statefulset.yamldeploy/k8s/redis/networkpolicy.yamldeploy/k8s/redis/service.yamldeploy/k8s/redis/statefulset.yamldocs/k8s-deployment.mdscripts/cchscripts/deploy-k8s.sh
| - from: | ||
| - namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: kube-system | ||
| - namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: ingress-nginx | ||
| - namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: traefik | ||
| ports: | ||
| - port: 3000 |
There was a problem hiding this comment.
当前入站来源限制会破坏 NodePort fallback 可用性。
Line 15-26 只允许 Ingress Controller 命名空间来源；当部署退化到 NodePort 时，客户端流量来源不满足该条件，应用将不可达。建议按暴露模式条件渲染策略：Ingress 模式保留现状，NodePort 模式增加 ipBlock（可配置 CIDR）或关闭该策略。
建议变更方向（示例）
ingress:
- from:
- namespaceSelector:
matchLabels:
kubernetes.io/metadata.name: kube-system
- namespaceSelector:
matchLabels:
kubernetes.io/metadata.name: ingress-nginx
- namespaceSelector:
matchLabels:
kubernetes.io/metadata.name: traefik
+ - ipBlock:
+ cidr: {{NODEPORT_ALLOWED_CIDR}}
ports:
- port: 3000📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - from: | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: kube-system | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: ingress-nginx | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: traefik | |
| ports: | |
| - port: 3000 | |
| - from: | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: kube-system | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: ingress-nginx | |
| - namespaceSelector: | |
| matchLabels: | |
| kubernetes.io/metadata.name: traefik | |
| - ipBlock: | |
| cidr: {{NODEPORT_ALLOWED_CIDR}} | |
| ports: | |
| - port: 3000 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/k8s/app/networkpolicy.yaml` around lines 15 - 26, The NetworkPolicy
currently restricts ingress sources to only specific namespaces
(namespaceSelector with kubernetes.io/metadata.name: kube-system, ingress-nginx,
traefik) which breaks NodePort fallback; update the template that renders this
policy to conditionally include those namespaceSelectors only when exposing via
Ingress, and for NodePort mode either add an ipBlock entry (configurable CIDR)
to the ingress from list or skip/disable the NetworkPolicy entirely;
specifically modify the rendering logic that emits the "from:" block and the
ports: - port: 3000 stanza so it branches on the exposure mode and injects a
configurable ipBlock or removes the policy for NodePort.
There was a problem hiding this comment.
Fixed in e185cc3e: NodePort fallback now skips the app NetworkPolicy, and the template/docs explicitly call out that this policy is only applied in Ingress mode.
There was a problem hiding this comment.
@ding113, thanks for the update! Skipping the NetworkPolicy entirely for NodePort mode is a cleaner solution than adding a configurable ipBlock CIDR — it avoids the extra placeholder and keeps the policy straightforward for Ingress-only use. Glad it's resolved! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| app: claude-code-hub | ||
| ports: | ||
| - port: 5432 | ||
| - from: |
There was a problem hiding this comment.
[CRITICAL] [SECURITY-ORPHANED-RULE] Stale migration-pod allowlist creates unnecessary access path
Why this is a problem: The migration Job was intentionally removed in this PR (PR body states: "去除独立 migration Job"), yet this NetworkPolicy rule still permits any pod labeled component: migration to reach Postgres on port 5432. Any future pod (accidental or malicious) in the namespace that carries that label can bypass the intended network isolation.
Suggested fix: Remove the second ingress rule (lines 19-24) entirely, or add a clarifying comment that this is intentionally disabled:
There was a problem hiding this comment.
Fixed in e185cc3e: removed the stale component: migration allowlist from the Postgres NetworkPolicy.
| command: | ||
| - sh | ||
| - -c | ||
| - redis-cli -a "$REDIS_PASSWORD" ping | grep -q PONG |
There was a problem hiding this comment.
[HIGH] [SECURITY-CREDENTIAL-EXPOSURE] Redis password exposed in probe process arguments
Why this is a problem: Both liveness and readiness probes execute redis-cli -a "$REDIS_PASSWORD" ping. At runtime the shell expands the environment variable into the actual password, which then appears in /proc/<pid>/cmdline readable by any process that can exec into the pod.
Suggested fix: Use a mounted secret file instead of command-line argument, or at minimum add --no-auth-warning and document this as a known limitation. Consider mounting the secret password to a file and using redis-cli -a $(cat /path/to/secret) ping.
There was a problem hiding this comment.
Fixed in e185cc3e: both Redis probes now use REDISCLI_AUTH plus --no-auth-warning, so the password no longer lands in process arguments.
| CCH_CONFIG_FILE="${CCH_CONFIG_FILE:-${XDG_CONFIG_HOME:-$HOME/.config}/cch/config}" | ||
| if [[ -r "$CCH_CONFIG_FILE" ]]; then | ||
| # shellcheck disable=SC1090 | ||
| source "$CCH_CONFIG_FILE" |
There was a problem hiding this comment.
[MEDIUM] [SECURITY-CODE-INJECTION] Arbitrary code execution via sourced config
Why this is a problem: Sourcing the config file executes all shell content within it. If an attacker can write to that path (symlink attack, world-writable parent directory), they gain code execution under the user running this script.
Suggested fix: Add file permission validation before sourcing - check that the file is not world-writable. Or parse only assignment lines with a restricted parser instead of full source.
This is acceptable for a management CLI targeting infrastructure operators, but should be documented as a security consideration in the docs.
There was a problem hiding this comment.
Fixed in e185cc3e: scripts/cch now parses a small allowlist of CCH_* assignments instead of source-ing the config file.
| fi | ||
| fi | ||
| log_info "Installing k3s via official installer..." | ||
| if ! curl -fsSL https://get.k3s.io | sh -; then |
There was a problem hiding this comment.
[LOW] [SECURITY-SUPPLY-CHAIN] Remote script piped to shell (curl | sh)
Why this is a problem: curl -fsSL https://get.k3s.io | sh - executes whatever the remote server returns without integrity verification. A compromised CDN, MITM, or supply-chain incident would give the remote host root code execution.
Suggested fix: Document the security consideration and optionally add a flag for checksum verification. This is acceptable for dev/testing but should be noted for production use.
There was a problem hiding this comment.
Addressed in e185cc3e: added an explicit warning in install_k3s() and in the deployment guide that --install-k3s executes the official remote installer and should be reviewed before production use.
| secretKeyRef: | ||
| name: claude-code-hub-secrets | ||
| key: admin-token | ||
| - name: AUTO_MIGRATE |
There was a problem hiding this comment.
[MEDIUM] [RACE-CONDITION] Concurrent AUTO_MIGRATE on fresh install with default replicas=2
Why this is a problem: On a first-time deploy the deployment starts 2 pods (default APP_REPLICAS), all with AUTO_MIGRATE=true. If Drizzle's migrate() does not acquire a Postgres advisory lock before applying schema changes, two concurrent migration runs can race on DDL statements.
Suggested fix: Add a warning during deployment or reduce default replicas to 1 for initial deployment. The cch update workflow correctly gates migration behind single-replica scale-down, but the initial deploy does not.
Confirm that Drizzle migrate() uses advisory locks - this is acceptable if true.
There was a problem hiding this comment.
Checked the current implementation before changing defaults: src/lib/migrate.ts already serializes migrations with a PostgreSQL advisory lock. I kept the 2-replica baseline, documented that lock next to AUTO_MIGRATE, and clarified it in the k8s docs in e185cc3e.
There was a problem hiding this comment.
Code Review Summary
This PR introduces comprehensive Kubernetes/k3s deployment automation with good cross-platform compatibility and security-conscious design (e.g., using --from-file for secrets instead of --from-literal). However, one critical security issue requires immediate attention before merge.
PR Size: L (Large)
- Lines changed: 2,683 additions
- Files changed: 20 files
- Note: This is a pure-additive feature (no breaking changes to existing Docker Compose path). Due to size, consider future PRs splitting documentation updates from core deployment logic.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 1 | 0 |
| Security | 1 | 1 | 1 | 1 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
1. [SECURITY-ORPHANED-RULE] deploy/k8s/postgres/networkpolicy.yaml:19-24
The migration Job was intentionally removed from this PR (PR body: "去除独立 migration Job"), yet the NetworkPolicy still allows pods labeled component: migration to access Postgres on port 5432. This creates an orphaned access path - any pod (accidental or malicious) in the namespace carrying that label can bypass network isolation.
Fix: Remove the second ingress rule (lines 19-24) entirely, or add a clarifying comment that this is intentionally disabled pending future use.
High Priority Issues (Should Fix)
2. [SECURITY-CREDENTIAL-EXPOSURE] deploy/k8s/redis/statefulset.yaml:69,78
Redis probes execute redis-cli -a "$REDIS_PASSWORD" ping. The password expands in the shell and appears in /proc/<pid>/cmdline, visible to any process that can exec into the pod.
Fix: Mount the secret password to a file and use redis-cli -a $(cat /path/to/secret) or use a Redis ACL file. At minimum, add --no-auth-warning and document this limitation.
Medium Priority Issues
3. [RACE-CONDITION] deploy/k8s/app/deployment.yaml:51
Default replicas: 2 with AUTO_MIGRATE=true means two pods will attempt migration concurrently on first deploy. The cch update workflow correctly gates this behind single-replica scale-down, but initial deploy does not.
Fix: Add a warning in deploy-k8s.sh for first-time deploy with replicas>1, or default to 1 replica and let HPA scale up.
4. [SECURITY-CODE-INJECTION] scripts/cch:49
Unconditional source "$CCH_CONFIG_FILE" executes arbitrary shell code. If the file is world-writable or attacker-controlled, this yields code execution.
Fix: Add file permission validation before sourcing (check not world-writable), or parse assignments with a restricted parser.
Low Priority Issues
5. [SECURITY-SUPPLY-CHAIN] scripts/deploy-k8s.sh:325
curl -fsSL https://get.k3s.io | sh - executes remote content without integrity verification. This is common for k3s installation but should be documented as a security consideration for production environments.
Positive Findings
- Good use of
--from-filefor secrets (avoids command-line credential exposure) - Proper
trapcleanup for temp directories containing secrets - NetworkPolicy ingress rules correctly scoped to specific namespaces
- HPA with stabilization windows and scale-down policies
- Cross-platform compatibility (macOS BSD vs GNU coreutils)
Review Coverage
- Logic and correctness - One race condition identified
- Security (OWASP Top 10) - 4 security findings
- Error handling - Proper error propagation in scripts
- Type safety - N/A (shell scripts)
- Documentation accuracy - Comments match intent
- Test coverage - N/A (deployment scripts, test plan in PR)
- Code clarity - Well-structured with good separation of concerns
Automated review by Claude AI
| else | ||
| kube_apply "$base/postgres/networkpolicy.yaml" || log_warning "postgres networkpolicy 应用失败,忽略" | ||
| kube_apply "$base/redis/networkpolicy.yaml" || log_warning "redis networkpolicy 应用失败,忽略" | ||
| kube_apply "$base/app/networkpolicy.yaml" || log_warning "app networkpolicy 应用失败,忽略" |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] NodePort fallback can be blocked by the default app NetworkPolicy
Why this is a problem: In nodeport mode (--disable-ingress or when --ingress-host is omitted), external traffic to the app does not originate from the Ingress Controller namespaces that deploy/k8s/app/networkpolicy.yaml allows. Because apply_manifests() always applies the app NetworkPolicy, NodePort access can become unreachable on clusters that enforce NetworkPolicy.
Evidence:
scripts/deploy-k8s.sh:659applies the app NetworkPolicy unconditionally:kube_apply "$base/app/networkpolicy.yaml" || log_warning "app networkpolicy 应用失败,忽略"
deploy/k8s/app/networkpolicy.yaml:15only allows ingress from specific controller namespaces.
Suggested fix: Skip applying the app NetworkPolicy when INGRESS_VARIANT=nodeport (or apply a separate NodePort-safe policy). For example:
# scripts/deploy-k8s.sh (apply_manifests)
if [[ "$DISABLE_NETWORKPOLICY" == true ]]; then
log_info "已跳过 NetworkPolicy (--disable-networkpolicy)"
else
kube_apply "$base/postgres/networkpolicy.yaml" || log_warning "postgres networkpolicy 应用失败,忽略"
kube_apply "$base/redis/networkpolicy.yaml" || log_warning "redis networkpolicy 应用失败,忽略"
if [[ "$INGRESS_VARIANT" == "nodeport" ]]; then
log_warning "NodePort 模式下跳过 app NetworkPolicy,避免阻断外部访问"
else
kube_apply "$base/app/networkpolicy.yaml" || log_warning "app networkpolicy 应用失败,忽略"
fi
fiThere was a problem hiding this comment.
Fixed in e185cc3e: NodePort fallback now skips the app NetworkPolicy instead of unconditionally applying the Ingress-only policy.
| $KUBECTL -n "$NAMESPACE" patch hpa claude-code-hub --type merge -p '{"spec":{"minReplicas":1}}' >/dev/null | ||
| fi | ||
| $KUBECTL -n "$NAMESPACE" scale deployment/claude-code-hub --replicas=1 >/dev/null | ||
| $KUBECTL -n "$NAMESPACE" rollout status deployment/claude-code-hub --timeout=180s >/dev/null 2>&1 |
There was a problem hiding this comment.
[HIGH] [ERROR-NO-USER-FEEDBACK] cch update can exit with no error context when rollout status fails
Why this is a problem: scripts/cch runs with set -euo pipefail (see scripts/cch:5). If kubectl rollout status times out or fails, the script will terminate immediately. At scripts/cch:216 and scripts/cch:271 the command output is fully suppressed (>/dev/null 2>&1), so the user gets neither kubectl’s reason nor a custom error message — making the failure hard to diagnose.
Evidence:
scripts/cch:216$KUBECTL -n "$NAMESPACE" rollout status deployment/claude-code-hub --timeout=180s >/dev/null 2>&1
scripts/cch:271$KUBECTL -n "$NAMESPACE" rollout status deployment/claude-code-hub --timeout=300s >/dev/null 2>&1
Suggested fix: Don’t hide stderr on failure. Either remove the redirection or wrap with an explicit error message. For example:
if ! $KUBECTL -n "$NAMESPACE" rollout status deployment/claude-code-hub --timeout=180s; then
err "Rollout did not become ready within 180s"
err "Try: kubectl -n $NAMESPACE describe deploy/claude-code-hub"
exit 1
fiApply the same pattern to the 300s rollout at scripts/cch:271.
There was a problem hiding this comment.
Fixed in e185cc3e: rollout waits now surface explicit diagnostics and no longer silently exit before rollback/replica-restore logic can run.
|
|
||
| cmd_logs() { | ||
| require_cluster | ||
| local TAIL="${1:-100}" |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] cch logs breaks pass-through kubectl flags when the first arg is not a number
Why this is a problem: cmd_logs() treats the first argument as --tail unconditionally:
scripts/cch:301local TAIL="${1:-100}"scripts/cch:302shift 2>/dev/null || true
So cch logs --since=1h becomes kubectl logs ... --tail="--since=1h", which fails. Since the function also forwards "$@", it looks like pass-through flags are intended.
Suggested fix: Only consume $1 as TAIL when it’s numeric.
cmd_logs() {
require_cluster
local TAIL="100"
if [[ "${1:-}" =~ ^[0-9]+$ ]]; then
TAIL="$1"
shift
fi
$KUBECTL -n "$NAMESPACE" logs deploy/claude-code-hub --all-containers --tail="$TAIL" "$@"
}There was a problem hiding this comment.
Fixed in e185cc3e: cmd_logs() now only consumes the first argument as --tail when it is numeric, so pass-through kubectl flags keep working.
There was a problem hiding this comment.
Code Review Summary
This PR adds a large k8s/k3s deployment surface area (manifests + deploy script + management CLI + docs). The main risks are (1) default NetworkPolicy behavior conflicting with the NodePort fallback, and (2) cch update hiding rollout failures, making operational failures hard to diagnose.
PR Size: XL
- Lines changed: 2683
- Files changed: 20
- Split suggestions:
- PR 1:
deploy/k8s/**manifests +deploy/k8s/README.md - PR 2:
scripts/deploy-k8s.sh+scripts/cch - PR 3:
docs/k8s-deployment.md+README.md/README.en.md
- PR 1:
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 1 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- (Confidence 96) NodePort fallback can be blocked by default app NetworkPolicy (
scripts/deploy-k8s.sh:659,deploy/k8s/app/networkpolicy.yaml:15).
High Priority Issues (Should Fix)
- (Confidence 90)
cch updatesuppressesrollout statuserrors and may exit without context (scripts/cch:216,scripts/cch:271). - (Confidence 88)
cch logsconsumes the first arg as--taileven when it is a kubectl flag, breaking pass-through (scripts/cch:301,scripts/cch:302).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Detected PR
#1048and gathered metadata + full diff. - Applied PR size label:
size/XL(2683 lines changed, 20 files). - Posted 3 inline review comments (via
gh api .../pulls/1048/comments, usingpositionsincelinewasn’t accepted by the API schema here):scripts/deploy-k8s.sh:659— NodePort fallback can be blocked by default app NetworkPolicy.scripts/cch:216(also referencesscripts/cch:271) —cch updatesuppressesrollout statuserrors and can exit without context.scripts/cch:301—cch logstail parsing breaks pass-through kubectl flags when first arg isn’t numeric.
- Submitted the required summary review with split suggestions using
gh pr review 1048 --comment.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e185cc3e9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| failureThreshold: 3 | ||
| readinessProbe: | ||
| httpGet: | ||
| path: /api/actions/health |
There was a problem hiding this comment.
The deployment probes target /api/actions/health, but that handler always responds with HTTP 200 in both success and error paths (src/app/api/actions/[...route]/route.ts), so Kubernetes will keep pods Ready even when dependencies are down. The real readiness handler (/api/health/ready) returns 503 when unhealthy (src/lib/health/checker.ts), which is what probes need to gate traffic and detect bad rollouts.
Useful? React with 👍 / 👎.
| case "$key" in | ||
| CCH_NAMESPACE|CCH_IMAGE|CCH_DEPLOY_DIR|CCH_RUNTIME|CCH_INGRESS_HOST|CCH_INGRESS_VARIANT|CCH_BACKUP_DIR|CCH_BACKUP_KEEP) | ||
| value="$(parse_config_value "$raw")" | ||
| printf -v "$key" '%s' "$value" |
There was a problem hiding this comment.
load_config_file unconditionally writes parsed values with printf -v, which overwrites already-exported CCH_* environment variables even though the script documents precedence as env > config > defaults. This makes one-off overrides like CCH_NAMESPACE=staging cch update silently ignore the env value when a config file exists, and can target the wrong namespace for operational commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/deploy-k8s.sh (1)
409-433: 默认 StorageClass 探测只匹配 GA 注解,略建议补上 beta 注解以兼容部分旧集群/老 CSI。Line 423 的 jsonpath 仅匹配
storageclass.kubernetes.io/is-default-class=="true"。虽然 K8s ≥ 1.26 上绝大多数发行版都已使用 GA 注解,但一些长期运行的集群或第三方 CSI 安装脚本仍然只打storageclass.beta.kubernetes.io/is-default-class。这种环境下脚本会落到 "未检测到默认 StorageClass" 的告警分支,PVC 可能无法自动绑定。♻️ 建议的兼容写法(保持优先 GA,退到 beta)
- default_sc=$($KUBECTL get sc -o jsonpath='{range .items[?(@.metadata.annotations.storageclass\.kubernetes\.io/is-default-class=="true")]}{.metadata.name}{"\n"}{end}' 2>/dev/null | head -1) + default_sc=$($KUBECTL get sc -o jsonpath='{range .items[?(@.metadata.annotations.storageclass\.kubernetes\.io/is-default-class=="true")]}{.metadata.name}{"\n"}{end}' 2>/dev/null | head -1) + if [[ -z "$default_sc" ]]; then + default_sc=$($KUBECTL get sc -o jsonpath='{range .items[?(@.metadata.annotations.storageclass\.beta\.kubernetes\.io/is-default-class=="true")]}{.metadata.name}{"\n"}{end}' 2>/dev/null | head -1) + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/deploy-k8s.sh` around lines 409 - 433, The detect_storage_class function currently only checks the GA annotation when computing default_sc; update the $KUBECTL get sc jsonpath invocation (the command that sets default_sc) to also consider the legacy beta annotation as a fallback (prefer GA storageclass.kubernetes.io/is-default-class=="true" first, then check storageclass.beta.kubernetes.io/is-default-class=="true"), so default_sc captures either annotation and STORAGE_CLASS/log_info behave unchanged when a match is found; modify the jsonpath used to populate default_sc accordingly while keeping the rest of the function (and the STORAGE_CLASS_ARG override) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/k8s-deployment.md`:
- Around line 354-368: 文档中恢复步骤使用 scripts/deploy-k8s.sh 的 --dry-render
未传入初次部署时的自定义参数（例如 --replicas、--hpa-min、--hpa-max），会导致重新渲染的 HPA/Deployment
被默认值覆盖并意外缩容；请在该步骤（调用 scripts/deploy-k8s.sh --dry-render --deploy-dir
/tmp/cch-restore）改为明确要求“使用与初次部署一致的 CLI 参数重跑一次整个
scripts/deploy-k8s.sh（幂等升级路径会跳过数据库销毁）”，或者在命令前加注释提示保留并传入原始参数（例如
--replicas/--hpa-min/--hpa-max），以避免在后续 kubectl apply 或 cch scale 时覆盖现有 HPA/副本设置。
In `@scripts/cch`:
- Around line 395-404: The cmd_scale function currently scales the deployment
but ignores any HPA controlling minReplicas/maxReplicas; update cmd_scale to
query the HPA for the claude-code-hub target in namespace $NAMESPACE (using
$KUBECTL get hpa claude-code-hub -n "$NAMESPACE") and if an HPA exists read its
minReplicas and maxReplicas, then: if the requested N is less than the HPA
minReplicas, either patch the HPA to set minReplicas to N (kubectl
patch/replace) before scaling or refuse the scale with a clear err message
showing the HPA min/max and the conflict; if N is greater than HPA.maxReplicas,
warn the user and refuse or scale up the HPA.maxReplicas accordingly; ensure you
reference cmd_scale, $KUBECTL, $NAMESPACE and the HPA minReplicas/maxReplicas
fields when implementing the checks and patches and log informative info/err
messages.
- Around line 326-337: 成功路径当前只把副本缩回为 MIN_REPLICAS，应该恢复到升级前实际副本数（但不低于
MIN_REPLICAS）以避免短暂容量下降；在脚本成功分支替换对 HPA 和 deployment 的操作，计算 desired_replicas=$((
CURRENT_REPLICAS > MIN_REPLICAS ? CURRENT_REPLICAS : MIN_REPLICAS ))（使用已保存的
CURRENT_REPLICAS），并用 desired_replicas 同时去 patch hpa (minReplicas) 和 scale
deployment/claude-code-hub，再调用 wait_for_deployment_rollout，保留现有函数名如
CURRENT_REPLICAS、MIN_REPLICAS、restore_update_scaling、wait_for_deployment_rollout
以便定位修改点。
In `@scripts/deploy-k8s.sh`:
- Around line 486-512: In force_new_reset_existing_namespace, update the
user-facing warning text to mention StorageClass reclaimPolicy behavior: when
warning the user about deleting namespace and that PVCs/volumes will be cleared
(the log_warning calls currently shown before the prompt and the non-interactive
branch message), append a concise sentence like "PV release depends on
StorageClass reclaimPolicy; with reclaimPolicy=Retain old PVs become Released
and must be cleaned up manually" so users are aware orphaned PVs may remain
under Retain policies and require manual cleanup.
---
Nitpick comments:
In `@scripts/deploy-k8s.sh`:
- Around line 409-433: The detect_storage_class function currently only checks
the GA annotation when computing default_sc; update the $KUBECTL get sc jsonpath
invocation (the command that sets default_sc) to also consider the legacy beta
annotation as a fallback (prefer GA
storageclass.kubernetes.io/is-default-class=="true" first, then check
storageclass.beta.kubernetes.io/is-default-class=="true"), so default_sc
captures either annotation and STORAGE_CLASS/log_info behave unchanged when a
match is found; modify the jsonpath used to populate default_sc accordingly
while keeping the rest of the function (and the STORAGE_CLASS_ARG override)
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df17f1d7-cf35-4a7b-a7bf-06c1352cebf2
📒 Files selected for processing (12)
deploy/k8s/README.mddeploy/k8s/app/deployment.yamldeploy/k8s/app/networkpolicy.yamldeploy/k8s/app/pdb.yamldeploy/k8s/ingress/ingress.yamldeploy/k8s/postgres/networkpolicy.yamldeploy/k8s/postgres/statefulset.yamldeploy/k8s/redis/statefulset.yamldocs/k8s-deployment.mdscripts/cchscripts/deploy-k8s.shtests/unit/k8s-deploy-assets-review-fixes.test.ts
✅ Files skipped from review due to trivial changes (3)
- deploy/k8s/app/networkpolicy.yaml
- deploy/k8s/postgres/statefulset.yaml
- deploy/k8s/redis/statefulset.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- deploy/k8s/postgres/networkpolicy.yaml
- deploy/k8s/app/pdb.yaml
- deploy/k8s/ingress/ingress.yaml
- deploy/k8s/app/deployment.yaml
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6231a2a1a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $KUBECTL -n "$NAMESPACE" patch hpa claude-code-hub --type merge \ | ||
| -p "{\"spec\":{\"minReplicas\":$desired_replicas}}" >/dev/null || true |
There was a problem hiding this comment.
restore_update_scaling patches the HPA minimum to desired_replicas, which is derived from the current replica count captured before the upgrade, not the original HPA floor. If an update starts during a traffic spike (for example current=8 while min=2), this permanently raises spec.minReplicas to 8, so autoscaling can no longer scale back down after the rollout and costs remain inflated until someone manually fixes the HPA.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8772f40a: rollback helpers now restore the original HPA floor, and the successful path separately scales the deployment back to max(CURRENT_REPLICAS, MIN_REPLICAS) without permanently inflating minReplicas.
| if $KUBECTL get namespace "$NAMESPACE" &>/dev/null && \ | ||
| $KUBECTL -n "$NAMESPACE" get deployment claude-code-hub &>/dev/null; then |
There was a problem hiding this comment.
Upgrade detection only checks for the app Deployment, so a namespace that still has StatefulSets/PVC data and secrets but a missing Deployment is treated as a fresh install. In that path prepare_secret_values generates new DB/Redis credentials and apply_secret overwrites DSN/password values, while PostgreSQL data still expects the old password, which breaks app/database authentication on the next rollout and can require manual recovery.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8772f40a: existing-install detection now treats the namespace as an upgrade path when any core resource or the shared secret is still present, so credentials are reused instead of regenerated against old data.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
docs/k8s-deployment.md (1)
353-357:⚠️ Potential issue | 🟡 Minor先删除 HPA 再把 app 缩到 0。
当前顺序先 scale 到 0，再删除 HPA；在控制循环窗口内 HPA 可能把副本重新拉起，恢复期间仍可能有写入。应先停掉 HPA 控制器再缩容。
建议修改
-# 1. 停 app (避免写入):直接 kubectl 操作 (cch scale 要求 >=1) -kubectl -n claude-code-hub scale deployment/claude-code-hub --replicas=0 -# CPU/内存 HPA 不支持直接把 minReplicas 改成 0,先删掉 HPA +# 1. 停 app (避免写入):先删掉 HPA,再直接 kubectl 缩到 0 (cch scale 要求 >=1) +# CPU/内存 HPA 不支持直接把 minReplicas 改成 0 kubectl -n claude-code-hub delete hpa claude-code-hub 2>/dev/null || true +kubectl -n claude-code-hub scale deployment/claude-code-hub --replicas=0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/k8s-deployment.md` around lines 353 - 357, The current steps scale the deployment to 0 before removing the HPA, which can let the HPA immediately recreate pods; change the procedure to first remove the HPA controller and then scale the deployment to zero: call kubectl delete hpa claude-code-hub (keeping the existing 2>/dev/null || true behavior if desired) before running kubectl -n claude-code-hub scale deployment/claude-code-hub --replicas=0 so the HPA cannot intervene during the scale-down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/k8s-deployment.md`:
- Around line 11-27: Locate the fenced code blocks in docs/k8s-deployment.md
(the diagram block around lines 11-27 and the other blocks at 107-125, 309-319,
398-410, 454-474) and add appropriate language identifiers to the opening
triple-backticks (e.g., use text for ASCII diagrams, bash for shell snippets,
console where interactive console output is shown) so each fence becomes
```text, ```bash or ```console as appropriate, which will satisfy MD040; keep
the original block contents unchanged.
- Line 318: 文档中“7. 健康检查通过 → 扩回 HPA minReplicas”表述不准确；`cch update`
实际行为是将副本数恢复为“升级前实际副本数”和 HPA 的 `minReplicas` 中的较大值。请在 docs/k8s-deployment.md
中将该步骤文字替换为明确描述：健康检查通过后恢复到升级前实际副本数与 HPA 的 `minReplicas` 的较大值（而非单纯“扩回 HPA
minReplicas”），并保留示例/说明以防用户误判容量行为；定位关键词可搜索 “cch update” 和目前的短语 “扩回 HPA
minReplicas” 进行修改。
In `@scripts/cch`:
- Around line 61-90: The load_config_file function currently unconditionally
writes config keys (e.g., CCH_NAMESPACE, CCH_IMAGE, CCH_DEPLOY_DIR, etc.), which
overwrites environment-provided values; update load_config_file so after
computing value="$(parse_config_value "$raw")" it only assigns into the variable
if the corresponding env var is not already set/non-empty (i.e., check the
current value of the dynamic variable name like "${!key}" or use a test -z
before calling printf -v). Keep parse_config_value and the case keys unchanged,
but gate the printf -v "$key" '%s' "$value" behind this existence check so
environment variables keep priority over the config file.
- Around line 443-447: Validate and sanitize CCH_BACKUP_KEEP before using it:
check the environment variable CCH_BACKUP_KEEP and ensure the local variable
keep is a positive integer (>=1); if it's non-numeric or less than 1 either set
keep to the safe default (30) or abort with an error. Use this validated keep
when computing tail -n +"$((keep+1))" so malformed values like "abc" or 0 cannot
break the pipeline or delete newly created backups; update the logic around the
keep variable and the cleanup loop that reads from ls -t
"$backup_dir"/claude_code_hub_*.sql.gz.
In `@scripts/deploy-k8s.sh`:
- Around line 382-396: The HPA bounds validation can perform arithmetic on
non-numeric input and should mirror APP_REPLICAS validation: first validate
APP_HPA_MIN and APP_HPA_MAX are positive integers (use the same regex check as
for APP_REPLICAS) and reject values that are not digits or less than 1 via
log_error; only after both pass numeric validation perform the numeric
comparison APP_HPA_MIN -gt APP_HPA_MAX and the existing error exit; reference
APP_HPA_MIN, APP_HPA_MAX and log_error when adding these checks.
- Around line 793-797: The get_node_ip function currently pipes kubectl calls
into || fallbacks but kubectl can exit 0 with empty output so the || branches
never run; change get_node_ip to use command substitution to capture the first
kubectl output into a variable (referencing get_node_ip and the KUBECTL
variable), then explicitly check if that variable is non-empty (e.g. if [ -n
"$node_ip" ]); if empty, run the InternalIP kubectl call into the same variable
and check again, and finally echo a default like "<your-node-ip>" when still
empty—this ensures the InternalIP fallback runs when ExternalIP yields an empty
string.
---
Duplicate comments:
In `@docs/k8s-deployment.md`:
- Around line 353-357: The current steps scale the deployment to 0 before
removing the HPA, which can let the HPA immediately recreate pods; change the
procedure to first remove the HPA controller and then scale the deployment to
zero: call kubectl delete hpa claude-code-hub (keeping the existing 2>/dev/null
|| true behavior if desired) before running kubectl -n claude-code-hub scale
deployment/claude-code-hub --replicas=0 so the HPA cannot intervene during the
scale-down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ccf8522-c482-4f52-a27a-f9c046facf1c
📒 Files selected for processing (4)
docs/k8s-deployment.mdscripts/cchscripts/deploy-k8s.shtests/unit/k8s-deploy-assets-review-fixes.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/unit/k8s-deploy-assets-review-fixes.test.ts
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/cch`:
- Around line 328-340: The computed desired_replicas (max(CURRENT_REPLICAS,
MIN_REPLICAS)) is never used when restoring scale: change the call to
restore_update_scaling to pass desired_replicas instead of CURRENT_REPLICAS so
the deployment is scaled to the intended value; update any related log/messages
if needed and ensure wait_for_deployment_rollout still uses the same context
("恢复副本") after calling restore_update_scaling; reference variables:
desired_replicas, CURRENT_REPLICAS, MIN_REPLICAS and function
restore_update_scaling (and wait_for_deployment_rollout) when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04ceb767-7f60-4c34-adda-278bf01674ff
📒 Files selected for processing (4)
docs/k8s-deployment.mdscripts/cchscripts/deploy-k8s.shtests/unit/k8s-deploy-assets-review-fixes.test.ts
| local desired_replicas="$CURRENT_REPLICAS" | ||
| if [[ "$desired_replicas" -lt "$MIN_REPLICAS" ]]; then | ||
| desired_replicas="$MIN_REPLICAS" | ||
| fi | ||
| info "Step 5/6: Scaling back to $desired_replicas replicas..." | ||
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" |
There was a problem hiding this comment.
The log says "Scaling back to $desired_replicas replicas..." but restore_update_scaling is called with "$CURRENT_REPLICAS" (the pre-update count), not "$desired_replicas" (the clamped value that enforces ≥ MIN_REPLICAS). If CURRENT_REPLICAS < MIN_REPLICAS at update time (e.g., the user manually scaled down, or the HPA minReplicas was recently raised), the deployment is left at $CURRENT_REPLICAS while the HPA is restored to $MIN_REPLICAS, causing a brief conflict window with fewer replicas than the HPA floor.
| local desired_replicas="$CURRENT_REPLICAS" | |
| if [[ "$desired_replicas" -lt "$MIN_REPLICAS" ]]; then | |
| desired_replicas="$MIN_REPLICAS" | |
| fi | |
| info "Step 5/6: Scaling back to $desired_replicas replicas..." | |
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" | |
| local desired_replicas="$CURRENT_REPLICAS" | |
| if [[ "$desired_replicas" -lt "$MIN_REPLICAS" ]]; then | |
| desired_replicas="$MIN_REPLICAS" | |
| fi | |
| info "Step 5/6: Scaling back to $desired_replicas replicas..." | |
| restore_update_scaling "$desired_replicas" "$MIN_REPLICAS" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/cch
Line: 329-334
Comment:
**Step 5 scales to wrong replica count despite correct log message**
The log says `"Scaling back to $desired_replicas replicas..."` but `restore_update_scaling` is called with `"$CURRENT_REPLICAS"` (the pre-update count), not `"$desired_replicas"` (the clamped value that enforces `≥ MIN_REPLICAS`). If `CURRENT_REPLICAS < MIN_REPLICAS` at update time (e.g., the user manually scaled down, or the HPA `minReplicas` was recently raised), the deployment is left at `$CURRENT_REPLICAS` while the HPA is restored to `$MIN_REPLICAS`, causing a brief conflict window with fewer replicas than the HPA floor.
```suggestion
local desired_replicas="$CURRENT_REPLICAS"
if [[ "$desired_replicas" -lt "$MIN_REPLICAS" ]]; then
desired_replicas="$MIN_REPLICAS"
fi
info "Step 5/6: Scaling back to $desired_replicas replicas..."
restore_update_scaling "$desired_replicas" "$MIN_REPLICAS"
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 46651790: Step 5 now passes desired_replicas into restore_update_scaling, so the actual scale target matches the log message and the max(CURRENT_REPLICAS, MIN_REPLICAS) clamp.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8772f40a2a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if $KUBECTL get crd ingressroutes.traefik.io &>/dev/null; then | ||
| INGRESS_VARIANT="traefik" | ||
| APP_SERVICE_TYPE="ClusterIP" | ||
| log_info "Ingress: Traefik IngressRoute (host=$INGRESS_HOST)" | ||
| return |
There was a problem hiding this comment.
detect_ingress_variant selects the Traefik path based only on the ingressroutes.traefik.io CRD and returns immediately, so standard Ingress (and --ingress-class) is skipped whenever that CRD exists. In clusters where Traefik CRDs are present but the Traefik controller is disabled or another controller handles ingress, this deploys only IngressRoute resources that no controller reconciles, leaving the service externally unreachable after an apparently successful rollout.
Useful? React with 👍 / 👎.
| if [[ -x "$SCRIPT_DIR/deploy-k8s.sh" ]]; then | ||
| info "转交给 scripts/deploy-k8s.sh (推荐使用完整的安装流程)" | ||
| exec bash "$SCRIPT_DIR/deploy-k8s.sh" "$@" |
There was a problem hiding this comment.
cmd_install assumes deploy-k8s.sh lives next to cch, but this repo’s installer links cch into /usr/local/bin. When users run that symlinked command, SCRIPT_DIR points to /usr/local/bin, so cch install always errors with “scripts/deploy-k8s.sh 未找到” instead of forwarding to the deploy script. This breaks the documented install subcommand for the installed CLI.
Useful? React with 👍 / 👎.
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: | ||
| - ALL |
There was a problem hiding this comment.
The container securityContext drops capabilities but does not set runAsNonRoot. Because the runtime image does not declare a non-root USER, Kubernetes will run this workload as UID 0 by default, which weakens pod isolation and increases impact of container escape/application RCE. Adding runAsNonRoot (and ideally a fixed non-root UID) is needed to make the hardening effective.
Useful? React with 👍 / 👎.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/cch`:
- Around line 263-268: 当前代码在 HPA 不存在时把 CURRENT_REPLICAS/MIN_REPLICAS 兜底为固定
2，会在后面恢复步骤把部署副本数不正确地改成 2；请改为先检查 HPA 是否存在 (kubectl get hpa claude-code-hub
返回非零时视为不存在)，若存在再读取并赋值到 CURRENT_REPLICAS 和 MIN_REPLICAS，否则不要强行设置为 2，而是从
Deployment（kubectl get deployment claude-code-hub -o
jsonpath='{.spec.replicas}') 获取原始副本数并赋给 CURRENT_REPLICAS（或直接留空/标记为“无
HPA”以跳过后续恢复逻辑），确保后续恢复逻辑针对 SYMBOLS CURRENT_REPLICAS/MIN_REPLICAS 不会把副本数错误地重写为 2。
- Around line 250-296: The script may lock the Deployment to a stale local image
digest even when the earlier k3s pre-pull (sudo k3s ctr images pull "$IMAGE")
failed, because IMAGE_DIGEST is resolved from local cache regardless of pull
success; change the flow so you record the success of the pull (e.g. set a flag
like PULLED=true on successful pull) and only compute
IMAGE_DIGEST/IMAGE_BY_DIGEST and call kubectl set image with the digest when
that pull succeeded; if the pull failed, skip resolving digest and instead use
the tag plus the rollout-restart fallback (the existing set image/app="$IMAGE" +
rollout restart path) to avoid pinning to a possibly-old local digest. Ensure
the referenced variables IMAGE, IMAGE_DIGEST, IMAGE_BY_DIGEST and the pull
command are the points of change.
- Around line 318-324: The health-check failure path should ensure the
deployment undo always attempts to restore replicas and waits for the rollback
to finish even with set -e; modify the block that runs health_check_in_pod so
that after logging the error you run the rollback command (KUBECTL rollout undo
deployment/claude-code-hub) but do not let a failed undo abort the
script—capture its exit and still call
restore_update_scaling("$CURRENT_REPLICAS","$MIN_REPLICAS"); then call KUBECTL
rollout status deployment/claude-code-hub -n "$NAMESPACE" (with a sensible
timeout) and handle its failure explicitly (log and exit non-zero) so the
function health_check_in_pod, the restore_update_scaling call, and waiting on
rollout status always run in that order regardless of undo command exit status.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47ed371d-f69c-43f1-90c7-eaea76778ff7
📒 Files selected for processing (2)
scripts/cchtests/unit/k8s-deploy-assets-review-fixes.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/unit/k8s-deploy-assets-review-fixes.test.ts
| # Step 2: (k3s only) pre-pull image | ||
| info "Step 2/6: Preparing image..." | ||
| if [[ "$RUNTIME" == "k3s" ]]; then | ||
| if sudo k3s ctr images pull "$IMAGE" >/dev/null 2>&1; then | ||
| ok "Image pre-pulled via k3s ctr" | ||
| else | ||
| warn "k3s ctr pull 失败,依赖 Always imagePullPolicy" | ||
| fi | ||
| else | ||
| info "Standard k8s: 依赖 imagePullPolicy=Always 在 rollout 时拉取" | ||
| fi | ||
| echo "" | ||
|
|
||
| # Step 3: Scale down to 1 for migration | ||
| local CURRENT_REPLICAS MIN_REPLICAS | ||
| CURRENT_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.status.currentReplicas}' 2>/dev/null || echo "2") | ||
| MIN_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.spec.minReplicas}' 2>/dev/null || echo "2") | ||
| [[ -z "$CURRENT_REPLICAS" || "$CURRENT_REPLICAS" == "null" ]] && CURRENT_REPLICAS=2 | ||
| [[ -z "$MIN_REPLICAS" || "$MIN_REPLICAS" == "null" ]] && MIN_REPLICAS=2 | ||
|
|
||
| info "Step 3/6: Scaling down to 1 replica for migration (was $CURRENT_REPLICAS)..." | ||
| if $KUBECTL -n "$NAMESPACE" get hpa claude-code-hub &>/dev/null; then | ||
| $KUBECTL -n "$NAMESPACE" patch hpa claude-code-hub --type merge -p '{"spec":{"minReplicas":1}}' >/dev/null | ||
| fi | ||
| $KUBECTL -n "$NAMESPACE" scale deployment/claude-code-hub --replicas=1 >/dev/null | ||
| if ! wait_for_deployment_rollout 180s "缩容到 1 副本"; then | ||
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" | ||
| exit 1 | ||
| fi | ||
| ok "Scaled to 1 replica" | ||
| echo "" | ||
|
|
||
| # Step 4: Update image + migrate | ||
| info "Step 4/6: Updating image on single instance (auto-migration)..." | ||
| if [[ "$RUNTIME" == "k3s" ]]; then | ||
| # k3s: 用 digest 固定,避免 tag 相同导致 no-op rollout | ||
| local IMAGE_DIGEST IMAGE_BY_DIGEST | ||
| IMAGE_DIGEST=$(sudo k3s ctr images ls 2>/dev/null | awk -v img="$IMAGE" '$1==img {print $3; exit}') | ||
| if [[ -n "$IMAGE_DIGEST" ]] && [[ "${IMAGE_DIGEST#sha256:}" != "$IMAGE_DIGEST" ]]; then | ||
| IMAGE_BY_DIGEST="${IMAGE%:*}@${IMAGE_DIGEST}" | ||
| info " digest: $IMAGE_DIGEST" | ||
| $KUBECTL -n "$NAMESPACE" set image deployment/claude-code-hub app="$IMAGE_BY_DIGEST" >/dev/null | ||
| else | ||
| warn "无法解析 digest,回落到 rollout restart" | ||
| $KUBECTL -n "$NAMESPACE" set image deployment/claude-code-hub app="$IMAGE" >/dev/null || true | ||
| $KUBECTL -n "$NAMESPACE" rollout restart deployment/claude-code-hub >/dev/null | ||
| fi |
There was a problem hiding this comment.
避免在 k3s 预拉取失败后固定到本地旧 digest。
Line 253 预拉取失败后，Line 287 仍会从本机镜像缓存解析 $IMAGE 的 digest；如果缓存里是旧的 latest，Line 291 会把 Deployment 固定到旧镜像，导致 update 静默部署旧版本。建议只在本次 ctr images pull 成功后使用 digest，否则走 tag + rollout restart fallback。
建议修复
# Step 2: (k3s only) pre-pull image
+ local K3S_IMAGE_PREPULLED="false"
info "Step 2/6: Preparing image..."
if [[ "$RUNTIME" == "k3s" ]]; then
if sudo k3s ctr images pull "$IMAGE" >/dev/null 2>&1; then
+ K3S_IMAGE_PREPULLED="true"
ok "Image pre-pulled via k3s ctr"
else
warn "k3s ctr pull 失败,依赖 Always imagePullPolicy"
fi
@@
if [[ "$RUNTIME" == "k3s" ]]; then
# k3s: 用 digest 固定,避免 tag 相同导致 no-op rollout
local IMAGE_DIGEST IMAGE_BY_DIGEST
- IMAGE_DIGEST=$(sudo k3s ctr images ls 2>/dev/null | awk -v img="$IMAGE" '$1==img {print $3; exit}')
+ IMAGE_DIGEST=""
+ if [[ "$K3S_IMAGE_PREPULLED" == "true" ]]; then
+ IMAGE_DIGEST=$(sudo k3s ctr images ls 2>/dev/null | awk -v img="$IMAGE" '$1==img {print $3; exit}')
+ fi
if [[ -n "$IMAGE_DIGEST" ]] && [[ "${IMAGE_DIGEST#sha256:}" != "$IMAGE_DIGEST" ]]; then
IMAGE_BY_DIGEST="${IMAGE%:*}@${IMAGE_DIGEST}"
info " digest: $IMAGE_DIGEST"
$KUBECTL -n "$NAMESPACE" set image deployment/claude-code-hub app="$IMAGE_BY_DIGEST" >/dev/null
else
- warn "无法解析 digest,回落到 rollout restart"
+ warn "未使用 digest,回落到 tag + rollout restart"
$KUBECTL -n "$NAMESPACE" set image deployment/claude-code-hub app="$IMAGE" >/dev/null || true
$KUBECTL -n "$NAMESPACE" rollout restart deployment/claude-code-hub >/dev/null
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/cch` around lines 250 - 296, The script may lock the Deployment to a
stale local image digest even when the earlier k3s pre-pull (sudo k3s ctr images
pull "$IMAGE") failed, because IMAGE_DIGEST is resolved from local cache
regardless of pull success; change the flow so you record the success of the
pull (e.g. set a flag like PULLED=true on successful pull) and only compute
IMAGE_DIGEST/IMAGE_BY_DIGEST and call kubectl set image with the digest when
that pull succeeded; if the pull failed, skip resolving digest and instead use
the tag plus the rollout-restart fallback (the existing set image/app="$IMAGE" +
rollout restart path) to avoid pinning to a possibly-old local digest. Ensure
the referenced variables IMAGE, IMAGE_DIGEST, IMAGE_BY_DIGEST and the pull
command are the points of change.
| # Step 3: Scale down to 1 for migration | ||
| local CURRENT_REPLICAS MIN_REPLICAS | ||
| CURRENT_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.status.currentReplicas}' 2>/dev/null || echo "2") | ||
| MIN_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.spec.minReplicas}' 2>/dev/null || echo "2") | ||
| [[ -z "$CURRENT_REPLICAS" || "$CURRENT_REPLICAS" == "null" ]] && CURRENT_REPLICAS=2 | ||
| [[ -z "$MIN_REPLICAS" || "$MIN_REPLICAS" == "null" ]] && MIN_REPLICAS=2 |
There was a problem hiding this comment.
无 HPA 时不要把升级前副本数兜底成固定 2。
Line 265-268 在 HPA 不存在时把 CURRENT_REPLICAS/MIN_REPLICAS 设为 2，但 doctor 已把 HPA 视为非必需；这种部署执行 cch update 后会被 Line 334 恢复成 2 副本，覆盖升级前的 Deployment 副本数。
建议修复
# Step 3: Scale down to 1 for migration
local CURRENT_REPLICAS MIN_REPLICAS
- CURRENT_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.status.currentReplicas}' 2>/dev/null || echo "2")
- MIN_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.spec.minReplicas}' 2>/dev/null || echo "2")
- [[ -z "$CURRENT_REPLICAS" || "$CURRENT_REPLICAS" == "null" ]] && CURRENT_REPLICAS=2
- [[ -z "$MIN_REPLICAS" || "$MIN_REPLICAS" == "null" ]] && MIN_REPLICAS=2
+ if $KUBECTL -n "$NAMESPACE" get hpa claude-code-hub &>/dev/null; then
+ CURRENT_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.status.currentReplicas}' 2>/dev/null || echo "")
+ MIN_REPLICAS=$($KUBECTL -n "$NAMESPACE" get hpa claude-code-hub -o jsonpath='{.spec.minReplicas}' 2>/dev/null || echo "")
+ else
+ CURRENT_REPLICAS=$($KUBECTL -n "$NAMESPACE" get deployment claude-code-hub -o jsonpath='{.spec.replicas}' 2>/dev/null || echo "")
+ MIN_REPLICAS="$CURRENT_REPLICAS"
+ fi
+ [[ -z "$CURRENT_REPLICAS" || "$CURRENT_REPLICAS" == "null" ]] && CURRENT_REPLICAS=1
+ [[ -z "$MIN_REPLICAS" || "$MIN_REPLICAS" == "null" ]] && MIN_REPLICAS="$CURRENT_REPLICAS"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/cch` around lines 263 - 268, 当前代码在 HPA 不存在时把
CURRENT_REPLICAS/MIN_REPLICAS 兜底为固定 2，会在后面恢复步骤把部署副本数不正确地改成 2；请改为先检查 HPA 是否存在
(kubectl get hpa claude-code-hub 返回非零时视为不存在)，若存在再读取并赋值到 CURRENT_REPLICAS 和
MIN_REPLICAS，否则不要强行设置为 2，而是从 Deployment（kubectl get deployment claude-code-hub
-o jsonpath='{.spec.replicas}') 获取原始副本数并赋给 CURRENT_REPLICAS（或直接留空/标记为“无
HPA”以跳过后续恢复逻辑），确保后续恢复逻辑针对 SYMBOLS CURRENT_REPLICAS/MIN_REPLICAS 不会把副本数错误地重写为 2。
| if health_check_in_pod; then | ||
| ok "Migration + startup OK" | ||
| else | ||
| err "DB 未通过健康检查,正在回滚..." | ||
| $KUBECTL -n "$NAMESPACE" rollout undo deployment/claude-code-hub >/dev/null | ||
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" | ||
| exit 1 |
There was a problem hiding this comment.
健康检查失败后的回滚也应恢复副本并等待完成。
Line 322 在 set -e 下若 rollout undo 失败会直接退出，跳过 Line 323 的副本恢复；即使 undo 成功，当前也未等待回滚 rollout 完成，自动化调用会看到已退出但集群仍在回滚中。建议对齐 Line 309-314 的失败路径。
建议修复
else
err "DB 未通过健康检查,正在回滚..."
- $KUBECTL -n "$NAMESPACE" rollout undo deployment/claude-code-hub >/dev/null
+ $KUBECTL -n "$NAMESPACE" rollout undo deployment/claude-code-hub >/dev/null || true
restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS"
+ wait_for_deployment_rollout 300s "回滚后的 deployment" || true
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if health_check_in_pod; then | |
| ok "Migration + startup OK" | |
| else | |
| err "DB 未通过健康检查,正在回滚..." | |
| $KUBECTL -n "$NAMESPACE" rollout undo deployment/claude-code-hub >/dev/null | |
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" | |
| exit 1 | |
| if health_check_in_pod; then | |
| ok "Migration + startup OK" | |
| else | |
| err "DB 未通过健康检查,正在回滚..." | |
| $KUBECTL -n "$NAMESPACE" rollout undo deployment/claude-code-hub >/dev/null || true | |
| restore_update_scaling "$CURRENT_REPLICAS" "$MIN_REPLICAS" | |
| wait_for_deployment_rollout 300s "回滚后的 deployment" || true | |
| exit 1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/cch` around lines 318 - 324, The health-check failure path should
ensure the deployment undo always attempts to restore replicas and waits for the
rollback to finish even with set -e; modify the block that runs
health_check_in_pod so that after logging the error you run the rollback command
(KUBECTL rollout undo deployment/claude-code-hub) but do not let a failed undo
abort the script—capture its exit and still call
restore_update_scaling("$CURRENT_REPLICAS","$MIN_REPLICAS"); then call KUBECTL
rollout status deployment/claude-code-hub -n "$NAMESPACE" (with a sensible
timeout) and handle its failure explicitly (log and exit non-zero) so the
function health_check_in_pod, the restore_update_scaling call, and waiting on
rollout status always run in that order regardless of undo command exit status.
1 participant
Summary
scripts/deploy-k8s.sh一键部署脚本,兼容 k3s + 标准 Kubernetes(EKS/GKE/AKS/自建),对齐现有scripts/deploy.sh的 CLI 风格scripts/cch运维管理 CLI(update/backup/info/doctor/uninstall等 14 个子命令)deploy/k8s/**15 份 manifest 模板(带{{VAR}}占位符,部署时渲染)docs/k8s-deployment.md10 节完整部署运维指南,README 中英双语挂入口What's new
scripts/deploy-k8s.sh--install-k3s,幂等升级,带自动回滚scripts/cchdeploy/k8s/deploy/k8s/README.mddocs/k8s-deployment.mdREADME.md/README.en.mdCompatibility
k3s ctr)+ digest 固定升级,local-pathStorageClass 自动识别imagePullPolicy=Always,相同 tag 时用rollout restart强制拉新cp -RT/date -Iseconds/xargs -r等 GNU 专属用法;b64d()包装 base64(GNU-d/ BSD-D/ openssl 兜底)三轮 Codex 审查结果
关键安全/稳定性改造:
--from-literal改为--from-file+ 600 权限 tmp(去除进程表泄露)AUTO_MIGRATE=true启动时自动迁移,避免并发竞态rollout undo+ 副本数恢复cch uninstall需显式CCH_CONFIRM_UNINSTALL=<ns>授权--disable-networkpolicyflag 适配 Ingress Controller 在非标准 namespace 的集群Test plan
bash -n scripts/deploy-k8s.sh scripts/cch通过kubectl apply --dry-run=client -R15/15 manifest schema 通过--dry-renderNodePort 场景:15 YAMLs,0 占位符残留--dry-renderIngress 场景(自定义 ns / storage-class / ingress-host):15 YAMLs,0 占位符残留cch help/cch version/deploy-k8s.sh --help输出正确cch update(需要用户在目标环境验证)Breaking changes
无。纯增量,不影响现有 Docker Compose 部署链路。
Related
对应本地已有的
~/claude-code-hub-k8s/工作目录,本次将其资产正式迁入仓库并做开源化改造。🤖 Generated with Claude Code
Greptile Summary
This PR adds a complete Kubernetes/k3s deployment layer: a one-click deploy script, a
cchmanagement CLI with 14 subcommands, 15 manifest templates, and accompanying docs. Three rounds of prior review addressed the major P0/P1 findings (secret process-table exposure, stale migration NetworkPolicy allowlist, Redis probe password leak, config-file sourcing RCE, and the rollback/replica-scaling logic). The remaining findings are two small gaps inscripts/cchand floating image tags on the StatefulSets.Confidence Score: 5/5
Safe to merge; all P0/P1 issues from prior rounds are resolved and the two remaining gaps are minor P2 quality improvements.
All critical findings from three prior review rounds are verified fixed in the current HEAD: secret process-table exposure, stale migration NetworkPolicy allowlist, Redis probe password leak, config-file sourcing RCE, and the replica scale-target bug. Remaining findings are the health-check rollback path missing a wait call (cosmetic inconsistency, rollback still completes) and a hardcoded CURRENT_REPLICAS fallback that only matters when HPA is manually deleted. Both are P2.
scripts/cch — two small logic gaps in cmd_update; deploy/k8s/postgres/statefulset.yaml and deploy/k8s/redis/statefulset.yaml for floating image tags.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant cch participant K8s User->>cch: cch update cch->>K8s: Step 1 — cmd_backup (pg_dump via kubectl exec) K8s-->>cch: backup OK / fail → abort or confirm cch->>K8s: Step 2 — k3s ctr pull image (k3s only) cch->>K8s: Step 3 — patch HPA minReplicas=1, scale to 1 K8s-->>cch: rollout status (180s timeout) cch->>K8s: Step 4 — set image / rollout restart K8s-->>cch: rollout status (600s timeout) alt rollout fails cch->>K8s: rollout undo + restore scaling + wait 300s cch-->>User: exit 1 end cch->>K8s: health_check_in_pod (node fetch /api/health/ready) alt health check fails cch->>K8s: rollout undo + restore scaling Note over cch,K8s: no wait for rollback here cch-->>User: exit 1 end cch->>K8s: Step 5 — restore HPA minReplicas + scale to desired K8s-->>cch: rollout status (300s timeout) cch->>K8s: Step 6 — final health_check_in_pod cch-->>User: Upgrade completePrompt To Fix All With AI
Reviews (5): Last reviewed commit: "fix(deploy): resolve review edge cases" | Re-trigger Greptile