Repository navigation
Add Helm o6n deploy path - #590
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 51 minutes and 41 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces Helm-based deployment infrastructure for coding-agent by adding configurable agent base_url support, flexible observability endpoint resolution, deploy-tools Docker image with kubectl/helm, and comprehensive Helm values/templates including a complete o6n DeepSeek configuration. The deployment script converts from manual kubectl commands to Helm upgrade/install with dry-run and apply modes. ChangesHelm Deployment Infrastructure and Agent Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/deploy-tools.Dockerfile (1)
1-32:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffAdd a non-root USER directive.
The image runs as root, violating the principle of least privilege. Deploy tooling should run with minimal permissions.
🔒 Proposed fix to add a non-root user
RUN set -eux; \ apt-get update; \ apt-get install -y --no-install-recommends ca-certificates curl tar gzip; \ arch="${TARGETARCH:-$(dpkg --print-architecture)}"; \ case "$arch" in \ amd64|x86_64) arch="amd64" ;; \ arm64|aarch64) arch="arm64" ;; \ *) echo "unsupported architecture: $arch" >&2; exit 1 ;; \ esac; \ kubectl_url="https://dl.k8s.io/release/v${KUBECTL_VERSION}/bin/linux/${arch}/kubectl"; \ curl -fsSL -o /tmp/kubectl "${kubectl_url}"; \ curl -fsSL -o /tmp/kubectl.sha256 "${kubectl_url}.sha256"; \ echo "$(cat /tmp/kubectl.sha256) /tmp/kubectl" | sha256sum -c -; \ install -m 0755 /tmp/kubectl /usr/local/bin/kubectl; \ helm_archive="helm-v${HELM_VERSION}-linux-${arch}.tar.gz"; \ helm_url="https://get.helm.sh/${helm_archive}"; \ curl -fsSL -o /tmp/helm.tgz "${helm_url}"; \ curl -fsSL -o /tmp/helm.tgz.sha256sum "${helm_url}.sha256sum"; \ echo "$(cut -d ' ' -f 1 /tmp/helm.tgz.sha256sum) /tmp/helm.tgz" | sha256sum -c -; \ tar -xzf /tmp/helm.tgz -C /tmp; \ mv "/tmp/linux-${arch}/helm" /usr/local/bin/helm; \ chmod 0755 /usr/local/bin/helm; \ rm -rf /tmp/kubectl /tmp/kubectl.sha256 /tmp/helm.tgz /tmp/helm.tgz.sha256sum "/tmp/linux-${arch}"; \ - rm -rf /var/lib/apt/lists/* + rm -rf /var/lib/apt/lists/*; \ + useradd -r -u 10001 -g 0 deploy-user + +USER 10001:0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/deploy-tools.Dockerfile` around lines 1 - 32, The image currently runs as root (the FROM ${PYTHON_BASE_IMAGE} + RUN block installs kubectl/helm into /usr/local/bin), so add a non-root user and switch to it with a USER directive. Modify the Dockerfile to: 1) after installing and moving kubectl/helm in the existing RUN block, create a non-root group/user (e.g., groupadd/useradd or addgroup/adduser) and set a fixed UID/GID; 2) chown the installed binaries and any runtime directories (e.g., /usr/local/bin/kubectl, /usr/local/bin/helm and any HOME) to that user inside the same RUN so ownership is correct; 3) set ENV HOME to the new user home if needed; and 4) add USER <username> near the end of the Dockerfile so subsequent containers run unprivileged. Ensure all privileged install steps remain before the USER change so installs succeed.Source: Linters/SAST tools
🧹 Nitpick comments (2)
scripts/deploy-coding-agent-k8s.sh (1)
42-58: ⚡ Quick winConsider using a bash array for
helmarguments.The current approach builds
helm_argsas a multi-line string and relies on unquoted expansion for word splitting. While this works with the current controlled inputs, it is fragile and will break if any value contains spaces or special characters.♻️ Proposed refactor using bash array
-helm_args=" -upgrade --install ${HELM_RELEASE} ${HELM_CHART_DIR} ---namespace ${K8S_NAMESPACE} ---values ${HELM_VALUES} ---set image.repository=${IMAGE_REPOSITORY} ---set image.tag=${IMAGE_TAG} ---wait ---timeout ${ROLLOUT_TIMEOUT} -" +helm_args=( + upgrade --install "${HELM_RELEASE}" "${HELM_CHART_DIR}" + --namespace "${K8S_NAMESPACE}" + --values "${HELM_VALUES}" + --set "image.repository=${IMAGE_REPOSITORY}" + --set "image.tag=${IMAGE_TAG}" + --wait + --timeout "${ROLLOUT_TIMEOUT}" +) if [ "$HELM_DEPLOY_MODE" = "dry-run" ]; then # Default mode proves the rendered release without mutating the cluster. - helm $helm_args --dry-run=client + helm "${helm_args[@]}" --dry-run=client exit 0 fi -helm $helm_args +helm "${helm_args[@]}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/deploy-coding-agent-k8s.sh` around lines 42 - 58, The helm argument construction uses a multi-line string variable helm_args and unquoted expansion which is brittle; change helm_args into a bash array (e.g., helm_args=(upgrade --install "${HELM_RELEASE}" "${HELM_CHART_DIR}" --namespace "${K8S_NAMESPACE}" --values "${HELM_VALUES}" --set "image.repository=${IMAGE_REPOSITORY}" --set "image.tag=${IMAGE_TAG}" --wait --timeout "${ROLLOUT_TIMEOUT}")) and update both invocations to call helm "${helm_args[@]}" (and helm "${helm_args[@]}" --dry-run=client in the HELM_DEPLOY_MODE dry-run branch) so each value is properly quoted and word-splitting is safe; ensure you use array append or literal array initialization where helm_args is defined and replace the existing string usage accordingly.coding-agent/helm/values.yaml (1)
100-108: 💤 Low valueConsider using
nullinstead of empty strings for consistency.The observability defaults use empty strings (
backend: "",endpoint: "",endpointEnv: "") while other optional fields in the schema usenull(e.g.,nodePort: null,baseUrl: null). Usingnullconsistently would better express "no value provided" vs. "explicitly set to empty."♻️ Proposed consistency improvement
observability: enabled: false - backend: "" - endpoint: "" - endpointEnv: "" + backend: null + endpoint: null + endpointEnv: null timeoutSeconds: 2.0 headers: {} publicKeyEnv: LANGFUSE_PUBLIC_KEY secretKeyEnv: LANGFUSE_SECRET_KEY🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/helm/values.yaml` around lines 100 - 108, The observability defaults use empty strings for optional fields which is inconsistent with other schema fields that use null; change observability.backend, observability.endpoint, and observability.endpointEnv from "" to null so they consistently represent "no value provided" (leave other keys like enabled, timeoutSeconds, headers, publicKeyEnv, secretKeyEnv unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docker/deploy-tools.Dockerfile`:
- Around line 1-32: The image currently runs as root (the FROM
${PYTHON_BASE_IMAGE} + RUN block installs kubectl/helm into /usr/local/bin), so
add a non-root user and switch to it with a USER directive. Modify the
Dockerfile to: 1) after installing and moving kubectl/helm in the existing RUN
block, create a non-root group/user (e.g., groupadd/useradd or addgroup/adduser)
and set a fixed UID/GID; 2) chown the installed binaries and any runtime
directories (e.g., /usr/local/bin/kubectl, /usr/local/bin/helm and any HOME) to
that user inside the same RUN so ownership is correct; 3) set ENV HOME to the
new user home if needed; and 4) add USER <username> near the end of the
Dockerfile so subsequent containers run unprivileged. Ensure all privileged
install steps remain before the USER change so installs succeed.
---
Nitpick comments:
In `@coding-agent/helm/values.yaml`:
- Around line 100-108: The observability defaults use empty strings for optional
fields which is inconsistent with other schema fields that use null; change
observability.backend, observability.endpoint, and observability.endpointEnv
from "" to null so they consistently represent "no value provided" (leave other
keys like enabled, timeoutSeconds, headers, publicKeyEnv, secretKeyEnv
unchanged).
In `@scripts/deploy-coding-agent-k8s.sh`:
- Around line 42-58: The helm argument construction uses a multi-line string
variable helm_args and unquoted expansion which is brittle; change helm_args
into a bash array (e.g., helm_args=(upgrade --install "${HELM_RELEASE}"
"${HELM_CHART_DIR}" --namespace "${K8S_NAMESPACE}" --values "${HELM_VALUES}"
--set "image.repository=${IMAGE_REPOSITORY}" --set "image.tag=${IMAGE_TAG}"
--wait --timeout "${ROLLOUT_TIMEOUT}")) and update both invocations to call helm
"${helm_args[@]}" (and helm "${helm_args[@]}" --dry-run=client in the
HELM_DEPLOY_MODE dry-run branch) so each value is properly quoted and
word-splitting is safe; ensure you use array append or literal array
initialization where helm_args is defined and replace the existing string usage
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 834bac96-ba5b-4bd7-8177-723ae13883a5
📒 Files selected for processing (18)
.woodpecker/ci.yml.woodpecker/deploy.ymlcoding-agent/docs/deployment/phase2-k8s.mdcoding-agent/docs/deployment/woodpecker.mdcoding-agent/helm/templates/configmap-agent-config.yamlcoding-agent/helm/templates/service.yamlcoding-agent/helm/values-o6n-deepseek.yamlcoding-agent/helm/values.yamlcoding-agent/src/agentkit/config/loader.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/observability.pycoding-agent/tests/agentkit/config/test_loader.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/coding_agent/test_observability.pycoding-agent/tests/deploy/test_deploy_script.pycoding-agent/tests/deploy/test_helm_chart.pydocker/deploy-tools.Dockerfilescripts/deploy-coding-agent-k8s.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
New Features
Deployment Updates
Documentation