Feature: Add Dockerfile and Helm chart for Kubernetes deployment#31
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds containerization and a full Helm chart for deploying the gomodel service: a multi-stage Dockerfile, .dockerignore, updated .gitignore, and a comprehensive Helm chart (manifests, helpers, values, schema, templates, README, and NOTES). Changes
Sequence Diagram(s)sequenceDiagram
actor Operator
participant Helm as "Helm CLI"
participant K8s as "Kubernetes API"
participant Pod as "gomodel Pod"
participant Redis as "Redis (optional)"
Operator->>Helm: helm install ./helm --values helm/values.yaml
Helm->>K8s: create resources (Deployment, Service, ConfigMap, Secrets, Ingress/Gateway, HPA, PDB, ServiceMonitor)
K8s->>K8s: schedule Deployment -> create Pod
K8s->>Pod: start container (/gomodel)
Pod->>Redis: connect using REDIS_URL / REDIS_KEY (if enabled)
Operator->>K8s: kubectl port-forward / logs / probe health endpoint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
There was a problem hiding this comment.
Actionable comments posted: 18
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
helm/gomodel/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
Dockerfilehelm/gomodel/Chart.yamlhelm/gomodel/README.mdhelm/gomodel/charts/redis-24.1.0.tgzhelm/gomodel/templates/NOTES.txthelm/gomodel/templates/_helpers.tplhelm/gomodel/templates/configmap.yamlhelm/gomodel/templates/deployment.yamlhelm/gomodel/templates/gateway.yamlhelm/gomodel/templates/hpa.yamlhelm/gomodel/templates/ingress.yamlhelm/gomodel/templates/pdb.yamlhelm/gomodel/templates/secret.yamlhelm/gomodel/templates/service.yamlhelm/gomodel/templates/servicemonitor.yamlhelm/gomodel/values.yaml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to cmd/gomodel/main.go : Provider packages must be imported in `cmd/gomodel/main.go` with blank imports (e.g., `_ "gomodel/internal/providers/openai"`) to trigger their init() registration
Applied to files:
helm/gomodel/Chart.yaml
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
helm/gomodel/Chart.yamlDockerfile
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to config/config.go : At least one provider API key is required in the environment configuration to start the server
Applied to files:
helm/gomodel/templates/secret.yaml
🪛 Checkov (3.2.334)
helm/gomodel/values.yaml
[low] 36-37: Base64 High Entropy String
(CKV_SECRET_6)
Dockerfile
[low] 18-18: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
[low] 1-33: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🪛 Hadolint (2.14.0)
Dockerfile
[warning] 7-7: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>
(DL3018)
[warning] 18-18: Always tag the version of an image explicitly
(DL3006)
🪛 YAMLlint (1.37.1)
helm/gomodel/templates/configmap.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
helm/gomodel/templates/ingress.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
helm/gomodel/templates/deployment.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
helm/gomodel/templates/servicemonitor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 14-14: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 18-18: too many spaces inside braces
(braces)
[error] 18-18: too many spaces inside braces
(braces)
helm/gomodel/templates/secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 11-11: too many spaces inside braces
(braces)
helm/gomodel/templates/gateway.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
helm/gomodel/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 12-12: too many spaces inside braces
(braces)
[error] 12-12: too many spaces inside braces
(braces)
[error] 13-13: too many spaces inside braces
(braces)
[error] 13-13: too many spaces inside braces
(braces)
[error] 14-14: too many spaces inside braces
(braces)
[error] 14-14: too many spaces inside braces
(braces)
[error] 16-16: too many spaces inside braces
(braces)
[error] 22-22: too many spaces inside braces
(braces)
[error] 22-22: too many spaces inside braces
(braces)
helm/gomodel/templates/service.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
helm/gomodel/templates/pdb.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
🔇 Additional comments (22)
Dockerfile (4)
9-11: LGTM! Good layer caching strategy.Copying dependency files separately before the source code optimizes Docker layer caching.
13-15: LGTM! Build configuration is correct.The build uses appropriate flags for a static binary suitable for distroless containers:
CGO_ENABLED=0ensures a fully static binary-ldflags="-s -w"strips debug information for smaller image size
28-29: LGTM! Good security practice.Running as a non-root user (UID 1000) follows container security best practices and reduces the attack surface.
23-23: No action needed. Theconfigdirectory exists in the repository and is copied into the builder stage by theCOPY . .command on line 14, making theCOPY --from=builder /app/config /app/configcommand valid.helm/gomodel/templates/service.yaml (1)
13-17: The Service and deployment are properly aligned on port naming.The deployment template correctly defines a containerPort with
name: http(helm/gomodel/templates/deployment.yaml), which matches the Service's targetPort reference. The configuration is valid.helm/gomodel/templates/ingress.yaml (1)
1-41: LGTM!The Ingress template is well-structured with proper conditional rendering, TLS support, and correct use of root context (
$) in nested loops. The template follows Helm best practices.helm/gomodel/templates/NOTES.txt (1)
36-50: Good implementation of provider validation.The template correctly validates that at least one provider is enabled and warns users appropriately. This aligns with the requirement that at least one provider API key is needed to start the server.
Based on learnings, at least one provider API key is required in the environment configuration.
helm/gomodel/templates/configmap.yaml (1)
1-15: LGTM!The ConfigMap template is well-structured with proper quoting and conditional inclusion of Redis configuration. The separation of REDIS_KEY (in ConfigMap) and REDIS_URL (in Secret) follows security best practices.
helm/gomodel/templates/servicemonitor.yaml (1)
16-16: No action required — The ServiceMonitor correctly references the "http" port, which matches the port name defined in the Service resource (name: http).helm/gomodel/values.yaml (6)
29-36: LGTM: Good authentication configuration design.The auth configuration provides flexibility with both inline and external secret options, with appropriate warnings about production usage.
38-82: LGTM: Well-structured provider configuration.The provider configuration is consistent across all LLM providers and offers both individual API key management and centralized secret management via
providers.existingSecret.
112-125: LGTM: Sensible metrics configuration.Prometheus metrics are enabled by default with appropriate scrape interval, while ServiceMonitor creation is opt-in (requires Prometheus Operator).
192-199: LGTM: PodDisruptionBudget configuration ensures availability.Enabling PDB by default with
minAvailable: 1ensures at least one pod remains available during voluntary disruptions, which aligns well with the defaultreplicaCount: 2.
201-219: LGTM: Well-configured health probes.Both liveness and readiness probes are properly configured with sensible timeouts and thresholds for a Go application.
221-236: LGTM: Excellent security hardening.The security contexts follow container security best practices:
- Non-root execution
- Read-only root filesystem
- No privilege escalation
- All Linux capabilities dropped
helm/gomodel/templates/deployment.yaml (3)
1-14: LGTM: Proper metadata and replica handling.The conditional replica count correctly defers to the HPA when autoscaling is enabled.
15-33: LGTM: Excellent use of checksum annotations.The ConfigMap and Secret checksums in pod annotations ensure automatic pod restarts when configuration changes, which is a Helm best practice for ensuring config changes are applied.
88-151: LGTM: Well-structured provider and authentication configuration.The environment variable injection follows a clean, consistent pattern across all providers with appropriate conditionals for enabling/disabling features and handling both inline values and existing secrets.
helm/gomodel/templates/_helpers.tpl (4)
4-50: LGTM: Standard Helm helpers correctly implemented.The name, fullname, chart, labels, and selector label helpers follow Helm best practices with proper truncation and DNS compliance.
55-72: LGTM: Clean secret name resolution helpers.The
providerSecretNameandauthSecretNamehelpers provide flexibility by supporting both user-provided secrets and chart-managed secrets with consistent naming.
90-96: LGTM: Smart cache type auto-configuration.The helper automatically sets cache type to "redis" when the Redis subchart is enabled, preventing misconfiguration.
101-104: LGTM: Standard image reference helper.The helper correctly constructs the image reference with tag defaulting to the chart's AppVersion.
kowtom
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Dockerfile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
Dockerfile
🪛 Checkov (3.2.334)
Dockerfile
[low] 1-33: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🔇 Additional comments (4)
Dockerfile (4)
6-7: Good practice: ca-certificates version is now pinned.The version pinning addresses the previous review concern and improves build reproducibility.
9-11: LGTM: Dependency caching is properly optimized.Copying
go.modandgo.sumbefore source files and runninggo mod downloadseparately ensures efficient Docker layer caching.
33-33: LGTM: ENTRYPOINT uses exec form correctly.The exec form
["/gomodel"]ensures the binary receives signals properly and runs as PID 1.
28-29: The Helm chart explicitly specifiesrunAsUser: 1000in both podSecurityContext (line 224) and container securityContext (line 232). TheUSER 1000directive in the Dockerfile is necessary to maintain consistency between the image-level UID and the Kubernetes pod security context, not redundant.Likely an incorrect or invalid review comment.
SantiagoDePolonia
changed the title
|
@kowtom if possible, please remove the redundant /helm/gomodel directory and place the Kubernetes-related files directly in the /helm directory. |
SantiagoDePolonia
left a comment
SantiagoDePolonia
left a comment
There was a problem hiding this comment.
Generally, great job! I've left a few comments for you to work on.
There was a problem hiding this comment.
Pull request overview
This PR introduces containerization and Kubernetes deployment capabilities for the GOModel AI gateway application by adding a multi-stage Dockerfile and comprehensive Helm chart with support for multiple deployment patterns.
Key Changes:
- Multi-stage Docker build using Go 1.24 and distroless runtime image for minimal attack surface
- Complete Helm chart with support for multiple LLM providers (OpenAI, Anthropic, Gemini, Groq, xAI)
- Built-in Redis caching support via Bitnami subchart with configurable TTL and persistence
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Dockerfile | Multi-stage build with Alpine builder and distroless runtime for security |
| helm/gomodel/Chart.yaml | Helm chart metadata with Redis dependency declaration |
| helm/gomodel/Chart.lock | Locked Redis dependency at version 24.1.0 |
| helm/gomodel/values.yaml | Comprehensive configuration with defaults for providers, caching, metrics, and scaling |
| helm/gomodel/README.md | Installation and configuration documentation with examples |
| helm/gomodel/templates/_helpers.tpl | Template helper functions for naming, labels, and URL generation |
| helm/gomodel/templates/deployment.yaml | Kubernetes Deployment with environment configuration and health probes |
| helm/gomodel/templates/service.yaml | ClusterIP Service exposing the application |
| helm/gomodel/templates/configmap.yaml | ConfigMap for non-sensitive configuration |
| helm/gomodel/templates/secret.yaml | Secret management for API keys and authentication |
| helm/gomodel/templates/ingress.yaml | Optional Ingress resource for external access |
| helm/gomodel/templates/gateway.yaml | Gateway API HTTPRoute support |
| helm/gomodel/templates/hpa.yaml | Horizontal Pod Autoscaler for automatic scaling |
| helm/gomodel/templates/pdb.yaml | Pod Disruption Budget for high availability |
| helm/gomodel/templates/servicemonitor.yaml | Prometheus ServiceMonitor integration |
| helm/gomodel/charts/redis-24.1.0.tgz | Packaged Bitnami Redis chart dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @Dockerfile:
- Around line 27-28: The Dockerfile currently overrides the distroless :nonroot
image's default user by setting "USER 1000", which conflicts with the image's
built-in nonroot UID (65532); either remove the "USER 1000" line to use the
distroless nonroot user as intended, or if you really need UID 1000, switch the
base image to a non-:nonroot variant and keep the "USER 1000" directive—pick one
approach and remove the conflicting configuration.
In @helm/gomodel/Chart.yaml:
- Around line 23-24: Update the Chart.yaml maintainers entry to include contact
information: locate the maintainers array and the entry with "name: GOModel
Team" and add either an "email: team@example.com" field or a "url:
https://your-team-url" field (or both) to provide users a way to reach the
maintainers for support or contributions.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Dockerfilehelm/gomodel/Chart.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
Dockerfilehelm/gomodel/Chart.yaml
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to cmd/gomodel/main.go : Provider packages must be imported in `cmd/gomodel/main.go` with blank imports (e.g., `_ "gomodel/internal/providers/openai"`) to trigger their init() registration
Applied to files:
helm/gomodel/Chart.yaml
🪛 Checkov (3.2.334)
Dockerfile
[low] 1-32: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🔇 Additional comments (5)
Dockerfile (3)
6-7: LGTM! ca-certificates version is now pinned.The ca-certificates package is now pinned to a specific version, which addresses the previous review comment and improves build reproducibility.
9-15: LGTM! Build stage follows best practices.The build process correctly:
- Leverages layer caching by copying and downloading dependencies first
- Builds a static binary with CGO disabled
- Strips debug information to reduce binary size
30-32: LGTM! Port and entrypoint configuration is correct.The exposed port and entrypoint are properly configured for the application.
helm/gomodel/Chart.yaml (2)
28-28: Pin Redis dependency to a specific version.The wildcard version
"*"for the Redis dependency prevents reproducible builds and introduces supply chain risk. Different environments may pull different Redis chart versions, leading to unpredictable behavior or breaking changes.🔎 Proposed fix
- version: "*" + version: "^19.0.0"Alternatively, pin to a specific version like
"19.6.0"after verifying the latest stable Redis chart version.To find the latest stable version, run:
#!/bin/bash # Description: Check the latest stable Redis chart version from Bitnami helm search repo bitnami/redis --versions | head -n 5Likely an incorrect or invalid review comment.
6-6: Ensure the Dockerfile sets the application version via -ldflags or update appVersion to match the code's default version.The Chart appVersion is set to "1.0.0" and will be used as the default Docker image tag (per values.yaml). However, the application's
internal/version/version.gohasVersion = "dev"by default, with a comment stating these variables should be set via-ldflagsduring the build process. The Dockerfile does not currently pass-ldflagsto set the version. This creates a mismatch: the deployed image will be tagged "1.0.0" but internally report version "dev" unless the build process explicitly sets it.Either update the Dockerfile to pass
-ldflags="-X gomodel/internal/version.Version=1.0.0"during the build, or adjust the appVersion to reflect the intended versioning strategy.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @Dockerfile:
- Line 14: The Dockerfile uses a broad COPY . . which pulls the entire repo into
the build context; add a .dockerignore file at the repo root that lists the
provided patterns (e.g., .git, .github/, helm/, *.md, LICENSE, *_test.go,
testdata/, .env, .env.*, .vscode/, .idea/, .DS_Store, node_modules/, dist/,
build/) to exclude secrets and unnecessary files from the image build and speed
up context transfer; ensure the .dockerignore is committed so COPY . . no longer
copies those entries during docker build.
- Line 18: Replace the mutable base image tag in the Dockerfile (the line
starting with "FROM gcr.io/distroless/static-debian12:nonroot") with an
immutable digest; run `docker pull gcr.io/distroless/static-debian12:nonroot`
and then `docker image inspect gcr.io/distroless/static-debian12:nonroot
--format='{{index .RepoDigests 0}}'` to get the RepoDigest, and update the FROM
line to use that digest (e.g. FROM
gcr.io/distroless/static-debian12@sha256:...). Ensure you commit the updated
Dockerfile so builds use the pinned digest.
In @helm/gomodel/templates/_helpers.tpl:
- Around line 77-85: The Redis URL helper "gomodel.redisUrl" must include
authentication when a password is configured; update the template to check for a
password (e.g. .Values.cache.redis.password with a fallback to
.Values.redis.password) and, when present, build the URL as
redis://:PASSWORD@HOST:PORT (i.e. include the leading colon before the
password), otherwise fall back to the existing host-only URLs or empty string;
ensure you handle both the cache-specific and top-level redis value keys so
deployed authenticated Redis instances receive credentials via the generated
URL.
In @helm/gomodel/templates/deployment.yaml:
- Around line 107-109: The deployment currently mounts an ephemeral volume named
"cache" using emptyDir which loses data on pod restart; make cache persistence
configurable so when values like cache.type == "disk" (or similar) are used
without Redis the chart can provision a PersistentVolumeClaim instead of
emptyDir. Update the templates to branch on the cache type/value (e.g.,
values.cache.type or values.cache.persistence.enabled) and render a PVC-backed
volume and volumeClaimTemplates when persistence is enabled, while keeping
emptyDir as the default for ephemeral cache; ensure the volume name "cache" and
any claimName references match the PVC so pods mount the correct volume.
- Around line 64-76: The env var REDIS_URL is marked optional in the template's
secretKeyRef which lets pods start without a required Redis URL when Redis is
enabled; change the secretKeyRef for REDIS_URL to remove or set optional: false
(i.e., make it required) and/or add a conditional validation in
values.schema.json so that when .Values.redis.enabled or .Values.cache.type ==
"redis" the chart requires the secret (provided via include
"gomodel.providerSecretName") to contain REDIS_URL; update the template block
guarded by the if or add a schema "required" rule tying redis.enabled/cache.type
to the REDIS_URL presence to ensure deployment fails fast when missing.
In @helm/gomodel/templates/secret.yaml:
- Around line 1-14: The secret template can render with only REDIS_URL and no
provider API keys, which breaks runtime since at least one provider key is
required; add a template helper (e.g., "gomodel.hasProviderKeys") that checks
Values.providers and specific provider key entries (or enabled flags) and use it
to guard the Secret rendering (replace the current top-level condition that uses
.Values.providers.existingSecret) so the Secret is only created when
hasProviderKeys is true; update references to the provider data helper
"gomodel.providerSecretData" to assume at least one key exists and fail-fast in
rendering when hasProviderKeys is false.
In @helm/gomodel/values.schema.json:
- Around line 296-305: Add an anyOf constraint to the autoscaling object schema
so at least one HPA metric is required: add an "anyOf" with required
["targetCPUUtilizationPercentage"] and required
["targetMemoryUtilizationPercentage"] to the "autoscaling" object (which
currently defines properties enabled, minReplicas, maxReplicas,
targetCPUUtilizationPercentage, targetMemoryUtilizationPercentage) so validation
fails when neither target is provided; ensure this logical validation only
applies when autoscaling.enabled is true (either by conditioning the schema
under the enabled flag or by placing the anyOf alongside a condition that checks
"enabled": true).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
Dockerfilehelm/gomodel/templates/_helpers.tplhelm/gomodel/templates/deployment.yamlhelm/gomodel/templates/pdb.yamlhelm/gomodel/templates/secret.yamlhelm/gomodel/values.schema.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to config/config.go : At least one provider API key is required in the environment configuration to start the server
Applied to files:
helm/gomodel/templates/secret.yaml
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to cmd/gomodel/main.go : Provider packages must be imported in `cmd/gomodel/main.go` with blank imports (e.g., `_ "gomodel/internal/providers/openai"`) to trigger their init() registration
Applied to files:
helm/gomodel/templates/secret.yaml
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
Dockerfile
🪛 Checkov (3.2.334)
Dockerfile
[low] 1-29: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-29: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 YAMLlint (1.37.1)
helm/gomodel/templates/secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
helm/gomodel/templates/deployment.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
helm/gomodel/templates/pdb.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 11-11: too many spaces inside braces
(braces)
[error] 12-12: too many spaces inside braces
(braces)
[error] 13-13: too many spaces inside braces
(braces)
🔇 Additional comments (13)
Dockerfile (1)
20-29: LGTM: Runtime stage is well-configured.The runtime stage correctly:
- Copies only necessary artifacts (ca-certificates, binary, YAML configs)
- Uses the distroless nonroot user (no conflicting USER directive)
- Sets appropriate working directory and exposes the service port
The pattern of copying only
*.yamlfiles ensures source code doesn't leak into the production image.helm/gomodel/values.schema.json (1)
1-398: LGTM: Comprehensive schema with good validation coverage.The schema provides thorough validation for the Helm chart values:
- Provider configuration enforced via
anyOfrequiring at least one enabled provider or existing secret- Appropriate types and enums for Kubernetes resources
- Flexible resource specification (cpu as string/number for K8s quantities)
- Redis subchart passthrough via
additionalProperties: trueThe structure aligns well with Kubernetes API conventions.
helm/gomodel/templates/_helpers.tpl (3)
4-50: LGTM: Standard Helm naming and labeling helpers.These helpers follow Helm best practices:
- DNS-compliant name truncation (63 chars)
- Proper override hierarchy (fullnameOverride > nameOverride > defaults)
- Kubernetes recommended labels structure
- Immutable selector labels as subset of full labels
55-72: LGTM: Secret name helpers with proper fallback logic.Both helpers correctly prioritize user-provided
existingSecretvalues and fall back to generated names with appropriate suffixes. This pattern supports both managed and external secret workflows.
101-137: LGTM: Provider helpers with proper conditional logic.The provider helpers correctly:
- Construct image references with fallback to AppVersion
- Filter provider secrets to only enabled providers with API keys
- Generate environment variables with secret references and optional base URL overrides
- Use uppercase naming convention for environment variables
The conditional checks prevent empty secret entries and ensure only configured providers are included.
helm/gomodel/templates/pdb.yaml (2)
9-11: LGTM: PDB validation correctly implemented.The validation logic ensures that at least one of
minAvailableormaxUnavailableis specified, which is required by Kubernetes. Thefailfunction will prevent template rendering if neither field is provided, giving users a clear error message.This addresses the issue raised in previous reviews and aligns with the commit message.
1-21: Well-structured PodDisruptionBudget template.The template correctly:
- Conditionally renders based on enabled flag
- Validates required fields with helpful error message
- Uses standard label helpers for consistency
- Applies selector labels for pod matching
Note: Static analysis YAML errors are false positives—YAML linters don't recognize Helm template syntax.
helm/gomodel/templates/secret.yaml (1)
16-26: Document secret management best practices.The auth secret includes sensitive credentials directly from values. Ensure chart documentation warns against committing secrets to version control and recommends:
- Using separate, gitignored values files (e.g.,
values-secrets.yaml)- External secret management (sealed-secrets, external-secrets, Vault)
- CI/CD-based secret injection
This was previously raised and remains a documentation concern for production deployments.
Likely an incorrect or invalid review comment.
helm/gomodel/templates/deployment.yaml (5)
1-13: LGTM!Standard Helm deployment structure with correct conditional replica handling for HPA. The static analysis warnings about "too many spaces inside braces" are false positives—
{{ }}is valid Helm template syntax, not YAML braces.
14-26: Good use of checksum annotations for config/secret rotation.The
sha256sumchecksums on the ConfigMap and Secret templates ensure pods are rolled when configuration changes. This is a well-established Helm pattern.
27-43: LGTM!Standard pod and container specification with properly externalized security contexts and image configuration.
98-106: LGTM!Standard pattern for volume mounts, probes, and resources sourced from values. Ensure
values.yamlprovides sensible defaults forlivenessProbe,readinessProbe, andresources.
110-121: LGTM!Standard Helm pattern for optional scheduling constraints using
withblocks.
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @.dockerignore:
- Around line 18-19: Remove the redundant *_test.go pattern from the
.dockerignore because the subsequent recursive pattern **/*_test.go already
excludes all test files; keep only **/*_test.go and delete the standalone
*_test.go entry to avoid duplication.
In @helm/README.md:
- Around line 24-27: The helm install command uses the removed chart directory
"./helm/gomodel"; update the README's helm install invocation (the line starting
with "helm install gomodel ./helm/gomodel") to point to the new chart location
"./helm" (e.g., "helm install gomodel ./helm") so the example matches the
reorganized chart layout.
- Around line 39-43: The helm install command in README uses an outdated chart
path ("./helm/gomodel"); update that path to point to the current chart
directory where the Chart.yaml lives (for example "./charts/gomodel") so the
command correctly references the relocated chart, and verify the README's helm
install example uses the actual directory name (replace the "./helm/gomodel"
token in the install line).
- Around line 92-95: Update the helm install command string "helm install
gomodel ./helm/gomodel ..." to point at the chart’s new location: replace
"./helm/gomodel" with the repository’s current chart directory (for example
"./charts/gomodel" or the renamed "./helm/gomodel-chart"), then verify by
running helm lint or a dry-run install; ensure the README’s example matches the
actual chart folder name used in the repo.
- Around line 131-132: The helm upgrade command in the README uses the old chart
path "./helm/gomodel"; update that path to the new chart directory under helm
(replace "./helm/gomodel" with the actual current chart folder name, e.g.,
"./helm/<new-chart-dir>") in the line containing the helm upgrade command so it
points to the restructured chart location.
In @helm/templates/_helpers.tpl:
- Around line 122-137: In the gomodel.providerEnvVars template the baseUrl env
var is incorrectly indented into the secretKeyRef block, producing invalid
Kubernetes YAML; fix by moving the $config.baseUrl conditional so the "name: {{
upper $name }}_BASE_URL" and its "value: {{ $config.baseUrl | quote }}" are
emitted as a separate env var entry at the same indentation level as the API key
env var (i.e., sibling to the valueFrom secretKeyRef), keep the surrounding
range over .Values.providers and the existing if-checks (including
$config.enabled and $config.baseUrl) intact, and ensure indentation aligns with
other top-level env var fields in the gomodel.providerEnvVars template.
In @helm/templates/deployment.yaml:
- Around line 62-63: Remove the dead commented env var lines referencing
GOMODEL_CACHE_DIR or replace them with a short explanatory comment;
specifically, locate the commented lines containing "- name: GOMODEL_CACHE_DIR"
and "value: \"/cache\"" in the deployment template and either delete those two
commented lines or keep them but add a one-line comment explaining why it's
disabled (e.g., "disabled: not currently used, enable when model caching is
required") and when to re-enable; ensure no other commented duplicates remain.
In @helm/templates/hpa.yaml:
- Around line 16-17: Add a Helm template validation that checks
.Values.autoscaling.minReplicas ≤ .Values.autoscaling.maxReplicas and fails
early if not; implement it in the hpa template by comparing the integer-cast
values (use int on .Values.autoscaling.minReplicas and
.Values.autoscaling.maxReplicas) and call fail with a clear message when
minReplicas > maxReplicas so chart rendering stops and surfaces the
configuration error.
In @helm/templates/NOTES.txt:
- Around line 56-57: Add a brief user hint after the kubectl port-forward and
curl lines (the command using kubectl --namespace ... port-forward svc/{{
include "gomodel.fullname" . }} 8080:{{ .Values.service.port }} & and the curl
http://127.0.0.1:8080/health line) to tell users they must stop the backgrounded
port-forward when finished (e.g., “Remember to stop port-forwarding when done”)
or replace the backgrounding with a subshell pattern; update NOTES.txt to
include that reminder so users aren’t left with a stray kubectl port-forward.
In @helm/templates/pdb.yaml:
- Around line 12-17: Add a guard in the pdb template to enforce mutual
exclusivity: check if both .Values.podDisruptionBudget.minAvailable and
.Values.podDisruptionBudget.maxUnavailable are set and call Helm's fail() with a
clear message if so; otherwise render the existing minAvailable or
maxUnavailable blocks as-is. This change should be made inside the
helm/templates/pdb.yaml template around the existing minAvailable/maxUnavailable
logic so the template refuses to render when both
.Values.podDisruptionBudget.minAvailable and
.Values.podDisruptionBudget.maxUnavailable are provided.
In @helm/templates/secret.yaml:
- Around line 14-16: The YAML document separator '---' is rendered
unconditionally and can create empty documents when the first secret is skipped;
move the '---' inside the auth secret conditional so it is only emitted when the
auth secret block (the conditional using .Values.auth.masterKey and
.Values.auth.existingSecret) will render. Concretely, wrap or relocate the
literal '---' so it is inside the same if-block that checks and
.Values.auth.masterKey and not .Values.auth.existingSecret, ensuring no
extraneous separators are produced when the auth secret is omitted.
In @helm/values.schema.json:
- Around line 322-331: The autoscaling schema validates minReplicas and
maxReplicas independently but doesn't enforce maxReplicas >= minReplicas; add a
cross-field rule under the autoscaling object (e.g., an allOf with an if/then)
that, when both minReplicas and maxReplicas are present, requires maxReplicas to
be greater than or equal to minReplicas (use the JSON Schema $data keyword / AJV
extension if available in the chart tooling to reference minReplicas in the
maxReplicas minimum). Ensure you reference the properties by name (minReplicas
and maxReplicas) inside the new validation block so invalid ranges are rejected
by the schema before reaching Kubernetes.
- Around line 31-107: The anyOf branches currently allow provider.enabled: true
without requiring credentials; update each provider object (openai, anthropic,
gemini, groq, xai) inside the providers schema to add an if-then: if
properties.enabled const true then require properties.apiKey with minLength:1
(and treat providers.existingSecret as an alternative if that branch is used);
ensure the anyOf branches include this if/then so enabling a provider enforces a
non-empty providers.<provider>.apiKey (or existingSecret) to satisfy validation.
In @helm/values.yaml:
- Around line 104-110: The default Helm values disable Redis auth and
persistence (auth.enabled: false and master.persistence.enabled: false), which
is unsafe for production; either change auth.enabled to true by default or add a
prominent warning comment above both auth.enabled and master.persistence.enabled
explaining these defaults are insecure for production and instructing users to
enable auth and persistence (and provide a short pointer to recommended
configuration or docs); update the same section so the comment explicitly
references auth.enabled and master.persistence.enabled to ensure users
overriding values see the warning.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
helm/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.dockerignorehelm/Chart.yamlhelm/README.mdhelm/templates/NOTES.txthelm/templates/_helpers.tplhelm/templates/configmap.yamlhelm/templates/deployment.yamlhelm/templates/gateway.yamlhelm/templates/hpa.yamlhelm/templates/ingress.yamlhelm/templates/pdb.yamlhelm/templates/secret.yamlhelm/templates/service.yamlhelm/templates/servicemonitor.yamlhelm/values.schema.jsonhelm/values.yaml
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to config/config.go : At least one provider API key is required in the environment configuration to start the server
Applied to files:
helm/templates/secret.yamlhelm/values.schema.json
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
helm/Chart.yaml.dockerignore
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to cmd/gomodel/main.go : Provider packages must be imported in `cmd/gomodel/main.go` with blank imports (e.g., `_ "gomodel/internal/providers/openai"`) to trigger their init() registration
Applied to files:
helm/Chart.yaml
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*_test.go : Use unit tests alongside implementation files with `*_test.go` naming convention for internal and config packages
Applied to files:
.dockerignore
🪛 Checkov (3.2.334)
helm/values.yaml
[low] 36-37: Base64 High Entropy String
(CKV_SECRET_6)
🪛 YAMLlint (1.37.1)
helm/templates/ingress.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
helm/templates/service.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
helm/templates/secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
helm/templates/servicemonitor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 14-14: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 18-18: too many spaces inside braces
(braces)
[error] 18-18: too many spaces inside braces
(braces)
helm/templates/gateway.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
helm/templates/deployment.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
helm/templates/pdb.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
[error] 5-5: too many spaces inside braces
(braces)
[error] 7-7: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 11-11: too many spaces inside braces
(braces)
[error] 12-12: too many spaces inside braces
(braces)
[error] 13-13: too many spaces inside braces
(braces)
helm/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 2-2: too many spaces inside braces
(braces)
[error] 3-3: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
[error] 15-15: too many spaces inside braces
(braces)
[error] 15-15: too many spaces inside braces
(braces)
[error] 16-16: too many spaces inside braces
(braces)
[error] 16-16: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 17-17: too many spaces inside braces
(braces)
[error] 19-19: too many spaces inside braces
(braces)
[error] 25-25: too many spaces inside braces
(braces)
[error] 25-25: too many spaces inside braces
(braces)
helm/templates/configmap.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
[error] 10-10: too many spaces inside braces
(braces)
🔇 Additional comments (15)
helm/templates/ingress.yaml (1)
1-41: LGTM! Helm template syntax is correct.The Ingress template follows Helm best practices and correctly generates Kubernetes Ingress resources with proper conditionals, labels, and backend configuration.
Note: The static analysis hints from YAMLlint are false positives. YAMLlint doesn't recognize Helm template syntax (
{{- }},{{ }}, template functions), which is expected and not an issue.helm/templates/servicemonitor.yaml (1)
1-19: LGTM! ServiceMonitor template is properly structured.The ServiceMonitor template correctly implements Prometheus Operator integration with appropriate conditionals, selector labels, and endpoint configuration.
Note: The static analysis hints from YAMLlint are false positives. YAMLlint doesn't recognize Helm template syntax, which is expected for
.yamlfiles in Helm chart templates directories.helm/README.md (1)
30-36: Critical: Incorrect chart path after directory restructure.Same issue as above - the chart path needs to be updated.
🛠️ Fix the chart path
-helm install gomodel ./helm/gomodel \ +helm install gomodel ./helm \ -n gomodel --create-namespace \ --set providers.openai.enabled=true \ --set providers.openai.apiKey="sk-..." \ --set providers.anthropic.enabled=true \ --set providers.anthropic.apiKey="sk-ant-..." \ --set redis.enabled=true⛔ Skipped due to learnings
Learnt from: CR Repo: ENTERPILOT/GOModel PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-28T22:10:31.364Z Learning: Applies to cmd/gomodel/main.go : Provider packages must be imported in `cmd/gomodel/main.go` with blank imports (e.g., `_ "gomodel/internal/providers/openai"`) to trigger their init() registrationhelm/templates/gateway.yaml (2)
1-28: LGTM! Gateway API HTTPRoute is well-structured.The HTTPRoute configuration correctly implements the Gateway API pattern with proper conditional rendering, metadata, and routing rules. The structure follows Kubernetes Gateway API best practices.
10-10: Parentref.name validation is properly implemented in values.schema.json.The schema correctly validates
gateway.parentRef.nameusing conditional logic: whengateway.enabledistrue, it enforces thatparentRefis required,parentRef.nameis required, and hasminLength: 1to ensure the field is non-empty.helm/templates/service.yaml (2)
1-19: LGTM! Service configuration is correct.The Service definition properly uses helper templates for consistent naming and labeling, with appropriate port configuration and selector labels.
14-17: No action needed. The deployment template correctly defines a containerPort named "http" at lines 41-44, which properly matches the Service's targetPort reference.Likely an incorrect or invalid review comment.
helm/templates/hpa.yaml (1)
2-4: Excellent validation for autoscaling metrics!The validation correctly ensures at least one scaling metric is configured when autoscaling is enabled, preventing misconfiguration.
helm/templates/pdb.yaml (1)
9-11: Good validation for PodDisruptionBudget configuration!The validation correctly ensures at least one disruption constraint is configured.
helm/templates/configmap.yaml (2)
1-15: LGTM! ConfigMap structure is correct.The ConfigMap properly exposes application configuration with appropriate quoting and conditional logic.
11-12: No action required — cache.redis.key already has a default value.The value
cache.redis.keyis already defined invalues.yamlwith the default"gomodel:models", which prevents nil pointer errors when Redis is enabled.Likely an incorrect or invalid review comment.
helm/templates/deployment.yaml (1)
1-121: Deployment template structure looks good.The deployment includes proper checksums for config/secret changes triggering rolling updates, security contexts, resource limits, and health probes. The conditional logic for Redis and authentication is well-structured.
helm/templates/NOTES.txt (1)
1-57: Comprehensive post-install notes with security warnings.The notes provide clear access instructions for all service types and appropriately warn users about missing authentication and provider configurations. This is helpful for production readiness.
helm/values.yaml (1)
1-250: Well-structured values file with good security defaults for the application.The values file has comprehensive configuration options with helpful comments. Security contexts are properly configured (non-root user, dropped capabilities, read-only filesystem), and PDB is enabled by default for availability.
helm/templates/_helpers.tpl (1)
1-105: Helper templates are well-designed.The naming, labeling, and configuration helpers follow Helm best practices. The use of
kindIsandhasKeyfor safe iteration over the providers map is a good defensive pattern.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.dockerignore:
- Around line 1-70: The .dockerignore is well organized; optionally reduce build
context by adding common additional patterns: add vendor/ if you do not vendor
dependencies (leave it out if you intentionally use vendored code), and consider
adding node_modules/, *.log, *.pem, *.key, .terraform/ and .cache/ to further
exclude large or sensitive files; update the .dockerignore file to include the
chosen patterns and commit the change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.dockerignore
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*_test.go : Use unit tests alongside implementation files with `*_test.go` naming convention for internal and config packages
Applied to files:
.dockerignore
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to tests/e2e/**/*.go : E2E tests must use the `-tags=e2e` build tag and use the in-process mock LLM server without Docker or external dependencies
Applied to files:
.dockerignore
📚 Learning: 2025-12-28T22:10:31.364Z
Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module name `gomodel` for all imports and package references
Applied to files:
.dockerignore
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@helm/README.md`:
- Around line 140-143: The Todo section in helm/README.md has formatting and
typo issues: insert a blank line before the "# Todo" heading, fix "Ad" to "Add"
and change "production ready" to "production-ready" in the list entry that
mentions redis, and ensure the file ends with a trailing newline; update the
bullet to read "Add an example for production-ready redis configuration with
persistence and authentication enabled."
In `@helm/values.schema.json`:
- Around line 30-53: The schema currently forces cache.redis.url to have
minLength:1 whenever cache.type is "redis", which rejects the default empty
string used when redis.enabled is true and gomodel.redisUrl will auto-generate
the URL; modify the conditional logic in values.schema.json so the non-empty
requirement applies only when cache.type == "redis" AND redis.enabled == false
(or conversely allow empty cache.redis.url when redis.enabled == true), i.e.,
update the "if"/"then" to check both cache.type and redis.enabled or add an "if"
branch that exempts cache.redis.url minLength when redis.enabled is true so
templates using gomodel.redisUrl succeed.
♻️ Duplicate comments (2)
helm/README.md (1)
24-27: Chart path references still incorrect.All
helm installandhelm upgradecommands reference./helm/gomodel, but according to the PR objectives the chart files are now directly in./helm. These should be updated to./helm.🛠️ Proposed fix
-helm install gomodel ./helm/gomodel \ +helm install gomodel ./helm \ -n gomodel --create-namespace \Apply this change to all occurrences on lines 24, 30, 39, 92, and 131.
Also applies to: 30-36, 39-44, 92-95, 131-132
helm/values.schema.json (1)
481-490: Consider adding cross-field validation for autoscaling replicas.The schema validates
minReplicasandmaxReplicasindividually but doesn't enforcemaxReplicas >= minReplicas. While Kubernetes will catch this, earlier schema validation provides better UX.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
helm/README.mdhelm/values.schema.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-15T04:47:09.662Z
Learnt from: kowtom
Repo: ENTERPILOT/GOModel PR: 31
File: helm/templates/gateway.yaml:9-13
Timestamp: 2026-01-15T04:47:09.662Z
Learning: For Helm charts, prefer declarative validation in values.schema.json using JSON Schema conditionals (if/then) over imperative template-level validation with `fail` statements. Schema validation runs earlier and provides better user experience.
Applied to files:
helm/values.schema.json
🪛 LanguageTool
helm/README.md
[grammar] ~143-~143: Use a hyphen to join words.
Context: ... subchart - Ad an example for production ready redis configuration with persisten...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
helm/README.md
140-140: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
143-143: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (2)
helm/values.schema.json (2)
5-29: Gateway conditional validation looks good.The schema correctly enforces that
gateway.parentRef.nameis required whengateway.enabled: true. This follows the recommended practice of using declarative JSON Schema validation over template-levelfailstatements. Based on learnings.
55-266: Provider credential validation logic is well-structured.The
anyOfcorrectly enforces that:
- Either
providers.existingSecretis provided (for GitOps workflows)- OR each enabled provider must have its
apiKeyspecifiedThe nested
if/thenwithin each provider branch ensures credentials are only required when that specific provider is enabled. This addresses the previously flagged concern about enabling providers without credentials.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@helm/templates/deployment.yaml`:
- Around line 27-31: Add support for configuring a Kubernetes service account in
the Deployment by wiring a serviceAccountName field into the spec using a chart
helper (e.g., reference a helper like "serviceAccountName" or
".Values.serviceAccount.name"); update the deployment template to set
spec.serviceAccountName when provided (use a conditional on
.Values.serviceAccount.create or .Values.serviceAccount.name), add a
corresponding helper in _helpers.tpl to resolve the name, and optionally add a
ServiceAccount manifest that is created when .Values.serviceAccount.create is
true so consumers can enable managed service accounts for RBAC or workload
identity.
♻️ Duplicate comments (2)
helm/templates/deployment.yaml (2)
62-74:optional: trueon REDIS_URL remains a concern.This was flagged in a previous review. When Redis is explicitly enabled via the condition on line 62, allowing the pod to start without
REDIS_URLcan lead to confusing runtime errors rather than a clear deployment failure.
105-107: Cache persistence acknowledged for future work.The
emptyDirvolume for cache was previously discussed, and the author confirmed persistent volume support will be added when implementing production caching.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
helm/templates/deployment.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-15T04:47:09.662Z
Learnt from: kowtom
Repo: ENTERPILOT/GOModel PR: 31
File: helm/templates/gateway.yaml:9-13
Timestamp: 2026-01-15T04:47:09.662Z
Learning: Prefer declarative validation in values.schema.json using JSON Schema conditionals (if/then) for Helm charts, rather than imperative template-level validation with fail statements. Declarative schema validation runs earlier, provides immediate feedback in tooling, and improves user experience by catching issues before rendering templates. Ensure this approach is applied to all Helm template YAML files under helm/templates and reflect constraints in values.schema.json.
Applied to files:
helm/templates/deployment.yaml
🪛 YAMLlint (1.37.1)
helm/templates/deployment.yaml
[error] 4-4: too many spaces inside braces
(braces)
[error] 4-4: too many spaces inside braces
(braces)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[error] 8-8: too many spaces inside braces
(braces)
[error] 9-9: too many spaces inside braces
(braces)
🔇 Additional comments (3)
helm/templates/deployment.yaml (3)
1-13: Well-structured deployment foundation.Good use of conditional replicas to avoid conflicts with HPA, and proper selector labels for pod matching.
14-26: Excellent use of checksum annotations.The
checksum/configandchecksum/secretannotations ensure pods automatically restart when ConfigMap or Secret content changes—a best practice for configuration-driven deployments.
99-119: Probes and scheduling configuration look good.The liveness/readiness probes and scheduling options (nodeSelector, affinity, tolerations) follow standard Helm patterns with proper optional handling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Please replace strings: /helm/gomodel -> /helm |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@helm/values.schema.json`:
- Around line 58-268: The schema's top-level anyOf allows only one branch to
succeed, so enabling multiple providers (providers.openai, providers.anthropic,
providers.gemini, providers.groq, providers.xai) can pass validation if only one
provider has credentials; change this by removing the root anyOf and adding
independent allOf conditionals for each provider: for each provider (openai,
anthropic, gemini, groq, xai) add an if checking providers.<provider>.enabled
=== true and a then requiring anyOf of providers.existingSecret (minLength 1) OR
providers.<provider>.apiKey (minLength 1) so each enabled provider independently
enforces credentials.
♻️ Duplicate comments (2)
Dockerfile (1)
18-18: Pin the distroless runtime image to an immutable digest.Line 18 still uses a mutable tag (
:nonroot), which weakens reproducibility and supply‑chain traceability. Pin to a digest (optionally keep the tag for readability).🔒 Suggested fix
-FROM gcr.io/distroless/static-debian12:nonroot +FROM gcr.io/distroless/static-debian12:nonroot@sha256:<digest>helm/README.md (1)
13-17: Replace the placeholder Helm repo URL or mark it clearly as a placeholder.Line 16 still uses the
your-orgplaceholder, which breaks copy‑paste. Either replace with the actual org URL or explicitly note this is a placeholder until the chart is published.📝 Suggested fix (placeholder made explicit)
-### Add the Helm repository (if published) +### Add the Helm repository (when published) + +*Note: replace `<your-org>` with your GitHub organization once the chart is published.* ```bash -helm repo add gomodel https://your-org.github.io/gomodel +helm repo add gomodel https://<your-org>.github.io/GOModel helm repo update</details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Organization UI **Review profile**: ASSERTIVE **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fbe2fb4c4b15e4f2ee613664ac48749683d48edd and d9a55bec70ce60931230bf03c0f0e6abf75969e2. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `Dockerfile` * `helm/README.md` * `helm/values.schema.json` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📚 Learning: 2025-12-28T22:10:31.364Z</summary>Learnt from: CR
Repo: ENTERPILOT/GOModel PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:10:31.364Z
Learning: Applies to **/*.go : Use the module namegomodelfor all imports and package references**Applied to files:** - `Dockerfile` </details> <details> <summary>📚 Learning: 2026-01-15T04:47:09.662Z</summary>Learnt from: kowtom
Repo: ENTERPILOT/GOModel PR: 31
File: helm/templates/gateway.yaml:9-13
Timestamp: 2026-01-15T04:47:09.662Z
Learning: For Helm charts, prefer declarative validation in values.schema.json using JSON Schema conditionals (if/then) over imperative template-level validation withfailstatements. Schema validation runs earlier and provides better user experience.**Applied to files:** - `helm/values.schema.json` </details> </details><details> <summary>🪛 Checkov (3.2.334)</summary> <details> <summary>Dockerfile</summary> [low] 1-29: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) --- [low] 1-29: Ensure that a user for the container has been created (CKV_DOCKER_3) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>helm/README.md</summary> 144-144: Files should end with a single newline character (MD047, single-trailing-newline) </details> </details> </details> <sub>✏️ Tip: You can disable this entire section by setting `review_details` to `false` in your review settings.</sub> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Add a dockerfile and helm chart for kubernetes deployment
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.