feat: Add official Helm chart for Kubernetes deployment #363
Conversation
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
Add comprehensive Helm chart for deploying Open Notebook on Kubernetes clusters. Features: - Complete Bitnami-style documentation with @param annotations - JSON schema for values validation - Support for 17 AI providers (OpenAI, Anthropic, Google, Azure, etc.) - Automatic secret creation for API keys - Health checks (liveness, readiness, startup probes) - Persistent volume support for app and SurrealDB - Service types: ClusterIP, NodePort, LoadBalancer - Ingress configuration with TLS support - Network policy for traffic control - ServiceMonitor for Prometheus monitoring - Pod disruption budget for high availability - Resource management and security contexts - Worker retry configuration Installation: helm install open-notebook ./helm/open-notebook Configuration: See helm/open-notebook/values.yaml for all options See helm/open-notebook/README.md for documentation
This was referenced Dec 29, 2025
1 task
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
13 issues found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="helm/values.yaml">
<violation number="1" location="helm/values.yaml:103">
P1: Security: Hardcoded default database credentials `root/root` are a significant security risk. Users who deploy without explicitly setting credentials will have an insecure installation. Consider generating a random password by default or requiring explicit password configuration (e.g., by failing validation when password equals 'root').</violation>
</file>
<file name="helm/templates/_helpers.tpl">
<violation number="1" location="helm/templates/_helpers.tpl:186">
P2: The condition `{{ if .Values.config.aiProviders.openai.apiKey }}` wraps the entire block, meaning `existingSecret` won't work unless `apiKey` is also set. Users expecting to use only `existingSecret` for security best practices will find it doesn't create the env var. Consider using `{{ if or .Values.config.aiProviders.openai.apiKey .Values.config.aiProviders.openai.existingSecret }}` instead.</violation>
<violation number="2" location="helm/templates/_helpers.tpl:357">
P2: Azure OpenAI secret key is hardcoded as `azure-openai-api-key` instead of using a configurable `secretKey` field like all other AI providers. This inconsistency prevents users from using existing secrets with different key names.</violation>
</file>
<file name="helm/templates/NOTES.txt">
<violation number="1" location="helm/templates/NOTES.txt:44">
P1: Logic error: The outer `if eq .Values.service.type "ClusterIP"` condition makes the nested NodePort and LoadBalancer checks unreachable. Users with those service types will see no URL instructions. Remove the outer ClusterIP condition so all service types are handled.</violation>
<violation number="2" location="helm/templates/NOTES.txt:81">
P1: Security: Password printed in plain text when `existingSecret` is not set. NOTES.txt output is visible in terminal and may be logged. Consider masking it like the existingSecret case.</violation>
<violation number="3" location="helm/templates/NOTES.txt:87">
P1: Security: Password exposed in command example via `--pass {{ .Values.surrealdb.auth.password }}`. Use a placeholder like `<your-password>` or reference the secret instead.</violation>
</file>
<file name="helm/templates/configmap.yaml">
<violation number="1" location="helm/templates/configmap.yaml:16">
P1: This ConfigMap template creates a new ConfigMap when `extraEnvVarsCM` is set, but the values.yaml documentation states it should be the "Name of existing ConfigMap". This design contradiction can cause resource conflicts if users provide a name of their pre-existing ConfigMap. Either the documentation needs to be updated to clarify this auto-creates a ConfigMap, or this template should be removed and users should create their own ConfigMap externally.</violation>
</file>
<file name="helm/templates/deployment.yaml">
<violation number="1" location="helm/templates/deployment.yaml:98">
P1: Password is embedded as plain text in the Deployment spec instead of referencing the Secret. The password is already stored in the Secret (secret.yaml lines 73-75), so this should use `secretKeyRef` to reference it, avoiding exposure of the password in the Deployment manifest.</violation>
</file>
<file name="helm/templates/statefulset.yaml">
<violation number="1" location="helm/templates/statefulset.yaml:59">
P1: Security issue: Database password exposed in command-line arguments. Command args are visible in process listings (`ps aux`), `kubectl describe pod` output, and various logs. Use environment variables from a Secret instead:
```yaml
env:
- name: SURREAL_USER
valueFrom:
secretKeyRef:
name: surrealdb-secret
key: username
- name: SURREAL_PASS
valueFrom:
secretKeyRef:
name: surrealdb-secret
key: password
Then reference $(SURREAL_USER) and $(SURREAL_PASS) in the command, or use SurrealDB's environment variable support directly.
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
Owner
|
@thuanpham582002 .. this will be a great addition to the project. Thanks for pushing through it. |
…endency - Update all template file license headers from Apache 2.0 to MIT - Add Bitnami common library chart as dependency in Chart.yaml - Refactor labels to use common.labels.standard helper for better Kubernetes best practices - Remove Chart.lock as chart now has external dependency - Update values.yaml comment to remove Bitnami branding - Update README.md license section to MIT License This aligns the Helm chart with the project's MIT license and leverages Bitnami's common library for better maintainability and standard Kubernetes labels.
Add GitHub Actions workflows for Helm chart CI/CD: - helm-lint-test.yaml: Automated linting and testing with chart-testing - helm-release-oci.yaml: Automated OCI-based releases to GHCR - ct.yaml: Chart-testing configuration - cr.yaml: Chart-releaser configuration Features: - Lint and test charts on every PR and push - Create kind cluster for integration testing - Automatic releases to OCI registry (GHCR) on main branch pushes - Auto-generated release notes for chart releases This follows industry best practices from helm-charts reference pattern.
Fix 13 security vulnerabilities identified by cubic-dev-ai bot: Critical Security Fixes: - Remove hardcoded default password "root" in values.yaml - Fix SurrealDB password exposure in command args (now uses env vars) - Remove plain text OPEN_NOTEBOOK_PASSWORD from deployment env - Mask passwords in installation notes (NOTES.txt) - Fix hardcoded database path to use configurable mountPath Medium Priority Fixes: - Fix existingSecret condition to support both password and existingSecret - Fix Azure OpenAI hardcoded secret key name to use configurable value - Fix schema type inconsistencies (NodePort as integers, secretKey as object) - Quote StorageClass to prevent YAML parsing issues Additional Improvements: - Update Chart.yaml to apiVersion v2 (required for dependencies) - Add helper functions for secret name resolution - Restructure secret.yaml to create separate secrets for app, SurrealDB, and API keys All passwords now use Kubernetes secretKeyRef instead of plain text values. Tested with helm lint and helm template - all tests pass.
Author
Contributor
|
@thuanpham582002 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
12 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="helm/templates/NOTES.txt">
<violation number="1" location="helm/templates/NOTES.txt:34">
P1: Unreachable conditional branches: The outer `if eq .Values.service.type "ClusterIP"` makes the inner checks for `NodePort` and `LoadBalancer` unreachable. Users deploying with non-ClusterIP service types will see no access instructions. Remove the outer condition to allow all service type branches to be evaluated.</violation>
</file>
<file name="helm/cr.yaml">
<violation number="1" location="helm/cr.yaml:3">
P3: Typo in comment: "GitHubs" should be "GitHub's" (possessive form).</violation>
</file>
<file name=".github/workflows/helm-lint-test.yaml">
<violation number="1" location=".github/workflows/helm-lint-test.yaml:12">
P2: Missing explicit `permissions` specification. Following security best practices, add a permissions block with minimal required access (`contents: read`) at the job or workflow level.</violation>
</file>
<file name="helm/values.schema.json">
<violation number="1" location="helm/values.schema.json:95">
P2: `nodePort` should be typed as integer (or integer|null) for consistency with other port definitions and Kubernetes spec. NodePorts are numeric values in Kubernetes.</violation>
<violation number="2" location="helm/values.schema.json:304">
P2: `httpNodePort` should include port range validation (`minimum: 1, maximum: 65535`) for consistency with other port definitions and to prevent invalid values.</violation>
<violation number="3" location="helm/values.schema.json:305">
P2: `apiNodePort` should include port range validation (`minimum: 1, maximum: 65535`) for consistency with other port definitions and to prevent invalid values.</violation>
</file>
<file name=".github/workflows/helm-release-oci.yaml">
<violation number="1" location=".github/workflows/helm-release-oci.yaml:56">
P2: GHCR requires lowercase repository paths, but `GITHUB_REPOSITORY_OWNER` preserves original casing. Convert to lowercase to prevent failures for users with mixed-case GitHub usernames/organizations.</violation>
</file>
<file name="helm/templates/secret.yaml">
<violation number="1" location="helm/templates/secret.yaml:28">
P1: Missing YAML document separator `---` before the second Secret resource. When multiple Kubernetes resources are defined in the same template file, they must be separated by `---`. Without this, the chart will fail to deploy when both API keys and auth password secrets need to be created.</violation>
<violation number="2" location="helm/templates/secret.yaml:99">
P1: Missing YAML document separator `---` before the SurrealDB Secret resource. This will cause YAML parsing errors when this secret needs to be created alongside the previous secrets.</violation>
</file>
<file name="helm/templates/_helpers.tpl">
<violation number="1" location="helm/templates/_helpers.tpl:218">
P1: The outer condition only checks for `apiKey`, so if a user provides only `existingSecret` (without `apiKey`), this entire block is skipped and the `ANTHROPIC_API_KEY` env var won't be set. This should use `or` like the OpenAI pattern does. The same issue affects all other providers: google, mistral, deepseek, openrouter, groq, xai, elevenlabs, voyage, azureOpenai, langchain, firecrawl, and jina.</violation>
</file>
<file name="helm/templates/statefulset.yaml">
<violation number="1" location="helm/templates/statefulset.yaml:64">
P1: Security: Credentials are exposed as command-line arguments. The `$(SURREAL_USER)` and `$(SURREAL_PASS)` syntax expands these into literal command-line arguments visible in process listings, `/proc/<pid>/cmdline`, and `kubectl describe pod` output. Since SURREAL_USER and SURREAL_PASS environment variables are already set, SurrealDB will read them automatically - remove the `--user` and `--pass` flags to avoid credential exposure.</violation>
</file>
<file name="helm/templates/deployment.yaml">
<violation number="1" location="helm/templates/deployment.yaml:149">
P2: Startup probe ignores `probeType` configuration while liveness and readiness probes respect it. Users setting `startupProbe.probeType: tcpSocket` will have their configuration silently ignored. Add the same probeType conditional logic used for liveness/readiness probes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
helm/templates/NOTES.txt
Outdated
|
|
||
| Your release is named {{ $releaseName }}. | ||
|
|
||
| {{- if eq .Values.service.type "ClusterIP" }} |
Contributor
There was a problem hiding this comment.
P1: Unreachable conditional branches: The outer if eq .Values.service.type "ClusterIP" makes the inner checks for NodePort and LoadBalancer unreachable. Users deploying with non-ClusterIP service types will see no access instructions. Remove the outer condition to allow all service type branches to be evaluated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/NOTES.txt, line 34:
<comment>Unreachable conditional branches: The outer `if eq .Values.service.type "ClusterIP"` makes the inner checks for `NodePort` and `LoadBalancer` unreachable. Users deploying with non-ClusterIP service types will see no access instructions. Remove the outer condition to allow all service type branches to be evaluated.</comment>
<file context>
@@ -0,0 +1,113 @@
+
+Your release is named {{ $releaseName }}.
+
+{{- if eq .Values.service.type "ClusterIP" }}
+
+1. Get the application URL by running these commands:
</file context>
✅ Addressed in 76cc880
| */}} | ||
| {{- $apiSecretName := printf "%s-secret" (include "common.names.fullname" .) }} | ||
| {{- if and .Values.secrets.create (not .Values.secrets.existingSecret) }} | ||
| apiVersion: v1 |
Contributor
There was a problem hiding this comment.
P1: Missing YAML document separator --- before the second Secret resource. When multiple Kubernetes resources are defined in the same template file, they must be separated by ---. Without this, the chart will fail to deploy when both API keys and auth password secrets need to be created.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/secret.yaml, line 28:
<comment>Missing YAML document separator `---` before the second Secret resource. When multiple Kubernetes resources are defined in the same template file, they must be separated by `---`. Without this, the chart will fail to deploy when both API keys and auth password secrets need to be created.</comment>
<file context>
@@ -0,0 +1,116 @@
+*/}}
+{{- $apiSecretName := printf "%s-secret" (include "common.names.fullname" .) }}
+{{- if and .Values.secrets.create (not .Values.secrets.existingSecret) }}
+apiVersion: v1
+kind: Secret
+metadata:
</file context>
✅ Addressed in 76cc880
| @@ -0,0 +1,116 @@ | |||
| {{- /* | |||
Contributor
There was a problem hiding this comment.
P1: Missing YAML document separator --- before the SurrealDB Secret resource. This will cause YAML parsing errors when this secret needs to be created alongside the previous secrets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/secret.yaml, line 99:
<comment>Missing YAML document separator `---` before the SurrealDB Secret resource. This will cause YAML parsing errors when this secret needs to be created alongside the previous secrets.</comment>
<file context>
@@ -0,0 +1,116 @@
+ labels: {{- include "open-notebook.labels" . | nindent 4 }}
+type: Opaque
+stringData:
+ {{ .Values.config.auth.secretKey | default "open-notebook-password" }}: {{ .Values.config.auth.password | quote }}
+{{- end }}
+
</file context>
✅ Addressed in 76cc880
helm/templates/_helpers.tpl
Outdated
| {{- end }} | ||
| {{- end }} | ||
|
|
||
| {{ if .Values.config.aiProviders.anthropic.apiKey -}} |
Contributor
There was a problem hiding this comment.
P1: The outer condition only checks for apiKey, so if a user provides only existingSecret (without apiKey), this entire block is skipped and the ANTHROPIC_API_KEY env var won't be set. This should use or like the OpenAI pattern does. The same issue affects all other providers: google, mistral, deepseek, openrouter, groq, xai, elevenlabs, voyage, azureOpenai, langchain, firecrawl, and jina.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/_helpers.tpl, line 218:
<comment>The outer condition only checks for `apiKey`, so if a user provides only `existingSecret` (without `apiKey`), this entire block is skipped and the `ANTHROPIC_API_KEY` env var won't be set. This should use `or` like the OpenAI pattern does. The same issue affects all other providers: google, mistral, deepseek, openrouter, groq, xai, elevenlabs, voyage, azureOpenai, langchain, firecrawl, and jina.</comment>
<file context>
@@ -0,0 +1,446 @@
+{{- end }}
+{{- end }}
+
+{{ if .Values.config.aiProviders.anthropic.apiKey -}}
+{{ if .Values.config.aiProviders.anthropic.existingSecret -}}
+- name: ANTHROPIC_API_KEY
</file context>
✅ Addressed in 76cc880
helm/templates/statefulset.yaml
Outdated
| - info | ||
| - --user | ||
| - $(SURREAL_USER) | ||
| - --pass |
Contributor
There was a problem hiding this comment.
P1: Security: Credentials are exposed as command-line arguments. The $(SURREAL_USER) and $(SURREAL_PASS) syntax expands these into literal command-line arguments visible in process listings, /proc/<pid>/cmdline, and kubectl describe pod output. Since SURREAL_USER and SURREAL_PASS environment variables are already set, SurrealDB will read them automatically - remove the --user and --pass flags to avoid credential exposure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/statefulset.yaml, line 64:
<comment>Security: Credentials are exposed as command-line arguments. The `$(SURREAL_USER)` and `$(SURREAL_PASS)` syntax expands these into literal command-line arguments visible in process listings, `/proc/<pid>/cmdline`, and `kubectl describe pod` output. Since SURREAL_USER and SURREAL_PASS environment variables are already set, SurrealDB will read them automatically - remove the `--user` and `--pass` flags to avoid credential exposure.</comment>
<file context>
@@ -0,0 +1,142 @@
+ - info
+ - --user
+ - $(SURREAL_USER)
+ - --pass
+ - $(SURREAL_PASS)
+ - --
</file context>
✅ Addressed in 76cc880
helm/values.schema.json
Outdated
| "properties": { | ||
| "http": { "type": "integer", "minimum": 1, "maximum": 65535 }, | ||
| "api": { "type": "integer", "minimum": 1, "maximum": 65535 }, | ||
| "httpNodePort": { "type": ["integer", "null"] }, |
Contributor
There was a problem hiding this comment.
P2: httpNodePort should include port range validation (minimum: 1, maximum: 65535) for consistency with other port definitions and to prevent invalid values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/values.schema.json, line 304:
<comment>`httpNodePort` should include port range validation (`minimum: 1, maximum: 65535`) for consistency with other port definitions and to prevent invalid values.</comment>
<file context>
@@ -0,0 +1,610 @@
+ "properties": {
+ "http": { "type": "integer", "minimum": 1, "maximum": 65535 },
+ "api": { "type": "integer", "minimum": 1, "maximum": 65535 },
+ "httpNodePort": { "type": ["integer", "null"] },
+ "apiNodePort": { "type": ["integer", "null"] }
+ }
</file context>
✅ Addressed in 76cc880
helm/values.schema.json
Outdated
| "properties": { | ||
| "type": { "enum": ["ClusterIP", "LoadBalancer", "NodePort"] }, | ||
| "port": { "type": "integer", "minimum": 1, "maximum": 65535 }, | ||
| "nodePort": { "type": "string" }, |
Contributor
There was a problem hiding this comment.
P2: nodePort should be typed as integer (or integer|null) for consistency with other port definitions and Kubernetes spec. NodePorts are numeric values in Kubernetes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/values.schema.json, line 95:
<comment>`nodePort` should be typed as integer (or integer|null) for consistency with other port definitions and Kubernetes spec. NodePorts are numeric values in Kubernetes.</comment>
<file context>
@@ -0,0 +1,610 @@
+ "properties": {
+ "type": { "enum": ["ClusterIP", "LoadBalancer", "NodePort"] },
+ "port": { "type": "integer", "minimum": 1, "maximum": 65535 },
+ "nodePort": { "type": "string" },
+ "annotations": { "type": "object" }
+ }
</file context>
✅ Addressed in 76cc880
| run: | | ||
| for pkg in .cr-release-packages/*.tgz; do | ||
| if [ -f "${pkg}" ]; then | ||
| helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/helm-charts" |
Contributor
There was a problem hiding this comment.
P2: GHCR requires lowercase repository paths, but GITHUB_REPOSITORY_OWNER preserves original casing. Convert to lowercase to prevent failures for users with mixed-case GitHub usernames/organizations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/helm-release-oci.yaml, line 56:
<comment>GHCR requires lowercase repository paths, but `GITHUB_REPOSITORY_OWNER` preserves original casing. Convert to lowercase to prevent failures for users with mixed-case GitHub usernames/organizations.</comment>
<file context>
@@ -0,0 +1,58 @@
+ run: |
+ for pkg in .cr-release-packages/*.tgz; do
+ if [ -f "${pkg}" ]; then
+ helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/helm-charts"
+ fi
+ done
</file context>
✅ Addressed in 76cc880
| successThreshold: {{ .Values.app.readinessProbe.successThreshold }} | ||
| {{- end }} | ||
| {{- if .Values.app.startupProbe.enabled }} | ||
| startupProbe: |
Contributor
There was a problem hiding this comment.
P2: Startup probe ignores probeType configuration while liveness and readiness probes respect it. Users setting startupProbe.probeType: tcpSocket will have their configuration silently ignored. Add the same probeType conditional logic used for liveness/readiness probes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/deployment.yaml, line 149:
<comment>Startup probe ignores `probeType` configuration while liveness and readiness probes respect it. Users setting `startupProbe.probeType: tcpSocket` will have their configuration silently ignored. Add the same probeType conditional logic used for liveness/readiness probes.</comment>
<file context>
@@ -0,0 +1,188 @@
+ successThreshold: {{ .Values.app.readinessProbe.successThreshold }}
+ {{- end }}
+ {{- if .Values.app.startupProbe.enabled }}
+ startupProbe:
+ httpGet:
+ path: {{ .Values.app.startupProbe.path }}
</file context>
✅ Addressed in 76cc880
helm/cr.yaml
Outdated
| @@ -0,0 +1,5 @@ | |||
| sign: false | |||
|
|
|||
| # Enable automatic generation of release notes using GitHubs release notes generator. | |||
Contributor
There was a problem hiding this comment.
P3: Typo in comment: "GitHubs" should be "GitHub's" (possessive form).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/cr.yaml, line 3:
<comment>Typo in comment: "GitHubs" should be "GitHub's" (possessive form).</comment>
<file context>
@@ -0,0 +1,5 @@
+sign: false
+
+# Enable automatic generation of release notes using GitHubs release notes generator.
+# see: https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes
+generate-release-notes: true
</file context>
✅ Addressed in 76cc880
This commit addresses 12 additional security and configuration issues identified by cubic-dev-ai bot review on Jan 6, 2026. P1 Critical Fixes: - Add YAML document separators to secret.yaml for multiple Secret resources - Fix existingSecret conditions for 13 AI providers (anthropic, google, mistral, deepseek, openrouter, groq, xai, elevenlabs, voyage, azureOpenai, langchain, firecrawl, jina) - Remove --user/--pass command args from SurrealDB StatefulSet (credentials visible in ps aux, /proc/<pid>/cmdline) - Fix unreachable conditional branches in NOTES.txt (allow all service types to show instructions) P2 Medium Fixes: - Add explicit permissions to helm-lint-test.yaml workflow (contents: read) - Fix schema port validations (nodePort type, add min/max for httpNodePort/apiNodePort) - Fix GHCR repository path case (convert GITHUB_REPOSITORY_OWNER to lowercase) - Add probeType support to startup probe in deployment.yaml (respect httpGet/tcpSocket config) P3 Low Fix: - cr.yaml typo already corrected Additional Fix: - Change surrealdb.service.nodePort from empty string to null to match schema All changes verified with helm lint and template rendering tests.
Author
Contributor
|
@thuanpham582002 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
6 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="helm/templates/secret.yaml">
<violation number="1" location="helm/templates/secret.yaml:90">
P2: YAML document separator `---` should be inside the conditional block, not outside. When the `{{- if }}` condition evaluates to false, this produces empty YAML documents which may cause issues when applying templates. Move `---` inside the `{{- if }}` block on the line after.</violation>
</file>
<file name="helm/templates/configmap.yaml">
<violation number="1" location="helm/templates/configmap.yaml:22">
P2: The condition only checks `extraEnvVarsCM` but the ConfigMap data is populated from `extraEnvVars`. If a user sets `extraEnvVarsCM` but leaves `extraEnvVars` empty (the default), an empty ConfigMap will be created. The condition should verify both values are set.</violation>
</file>
<file name="helm/templates/deployment.yaml">
<violation number="1" location="helm/templates/deployment.yaml:70">
P1: Init container will fail to run `chown` without root privileges. Add a securityContext with `runAsUser: 0` to allow the init container to change file ownership on the persistent volume.</violation>
</file>
<file name="helm/values.yaml">
<violation number="1" location="helm/values.yaml:104">
P2: SurrealDB password defaults to empty string but README documentation states default is `root`. This mismatch could cause confusion or deployment failures. Either update the default to match documentation or update the README.</violation>
<violation number="2" location="helm/values.yaml:374">
P1: Startup probe path `/api/health` is inconsistent with liveness/readiness probes which use `/health`. This could cause startup failures if the application only exposes a single health endpoint. The README also documents this should be `/health`.</violation>
</file>
<file name="helm/templates/statefulset.yaml">
<violation number="1" location="helm/templates/statefulset.yaml:63">
P1: SurrealDB command is missing `--bind` flag. The container exposes `{{ .Values.surrealdb.service.port }}` and probes check that port, but SurrealDB defaults to listening on port 8000. If the service port is configured to a different value, the deployment will fail because probes will connect to the wrong port.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - name: init-permissions | ||
| image: {{ include "open-notebook.image" . }} | ||
| imagePullPolicy: {{ .Values.image.pullPolicy }} | ||
| command: |
Contributor
There was a problem hiding this comment.
P1: Init container will fail to run chown without root privileges. Add a securityContext with runAsUser: 0 to allow the init container to change file ownership on the persistent volume.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/deployment.yaml, line 70:
<comment>Init container will fail to run `chown` without root privileges. Add a securityContext with `runAsUser: 0` to allow the init container to change file ownership on the persistent volume.</comment>
<file context>
@@ -0,0 +1,193 @@
+ - name: init-permissions
+ image: {{ include "open-notebook.image" . }}
+ imagePullPolicy: {{ .Values.image.pullPolicy }}
+ command:
+ - /bin/sh
+ - -cx
</file context>
✅ Addressed in dea929a
helm/values.yaml
Outdated
| startupProbe: | ||
| enabled: false | ||
| probeType: httpGet | ||
| path: /api/health |
Contributor
There was a problem hiding this comment.
P1: Startup probe path /api/health is inconsistent with liveness/readiness probes which use /health. This could cause startup failures if the application only exposes a single health endpoint. The README also documents this should be /health.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/values.yaml, line 374:
<comment>Startup probe path `/api/health` is inconsistent with liveness/readiness probes which use `/health`. This could cause startup failures if the application only exposes a single health endpoint. The README also documents this should be `/health`.</comment>
<file context>
@@ -0,0 +1,883 @@
+ startupProbe:
+ enabled: false
+ probeType: httpGet
+ path: /api/health
+ port: api
+ initialDelaySeconds: 0
</file context>
✅ Addressed in dea929a
| - --log | ||
| - info | ||
| - -- | ||
| - rocksdb:{{ .Values.surrealdb.persistence.mountPath }}/mydatabase.db |
Contributor
There was a problem hiding this comment.
P1: SurrealDB command is missing --bind flag. The container exposes {{ .Values.surrealdb.service.port }} and probes check that port, but SurrealDB defaults to listening on port 8000. If the service port is configured to a different value, the deployment will fail because probes will connect to the wrong port.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/statefulset.yaml, line 63:
<comment>SurrealDB command is missing `--bind` flag. The container exposes `{{ .Values.surrealdb.service.port }}` and probes check that port, but SurrealDB defaults to listening on port 8000. If the service port is configured to a different value, the deployment will fail because probes will connect to the wrong port.</comment>
<file context>
@@ -0,0 +1,138 @@
+ - --log
+ - info
+ - --
+ - rocksdb:{{ .Values.surrealdb.persistence.mountPath }}/mydatabase.db
+ env:
+ - name: SURREAL_USER
</file context>
✅ Addressed in dea929a
helm/templates/secret.yaml
Outdated
| {{/* | ||
| Secret for application authentication password | ||
| */}} | ||
| --- |
Contributor
There was a problem hiding this comment.
P2: YAML document separator --- should be inside the conditional block, not outside. When the {{- if }} condition evaluates to false, this produces empty YAML documents which may cause issues when applying templates. Move --- inside the {{- if }} block on the line after.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/secret.yaml, line 90:
<comment>YAML document separator `---` should be inside the conditional block, not outside. When the `{{- if }}` condition evaluates to false, this produces empty YAML documents which may cause issues when applying templates. Move `---` inside the `{{- if }}` block on the line after.</comment>
<file context>
@@ -0,0 +1,118 @@
+{{/*
+Secret for application authentication password
+*/}}
+---
+{{- if and .Values.config.auth.password (not .Values.config.auth.existingSecret) }}
+apiVersion: v1
</file context>
✅ Addressed in dea929a
helm/templates/configmap.yaml
Outdated
| OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| SOFTWARE. | ||
| */ -}} | ||
| {{- if .Values.app.extraEnvVarsCM }} |
Contributor
There was a problem hiding this comment.
P2: The condition only checks extraEnvVarsCM but the ConfigMap data is populated from extraEnvVars. If a user sets extraEnvVarsCM but leaves extraEnvVars empty (the default), an empty ConfigMap will be created. The condition should verify both values are set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/configmap.yaml, line 22:
<comment>The condition only checks `extraEnvVarsCM` but the ConfigMap data is populated from `extraEnvVars`. If a user sets `extraEnvVarsCM` but leaves `extraEnvVars` empty (the default), an empty ConfigMap will be created. The condition should verify both values are set.</comment>
<file context>
@@ -0,0 +1,32 @@
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+*/ -}}
+{{- if .Values.app.extraEnvVarsCM }}
+apiVersion: v1
+kind: ConfigMap
</file context>
✅ Addressed in dea929a
helm/values.yaml
Outdated
| ## @param surrealdb.auth.password SurrealDB password | ||
| ## NOTE: Must be set if existingSecret is not provided | ||
| ## | ||
| password: "" |
Contributor
There was a problem hiding this comment.
P2: SurrealDB password defaults to empty string but README documentation states default is root. This mismatch could cause confusion or deployment failures. Either update the default to match documentation or update the README.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/values.yaml, line 104:
<comment>SurrealDB password defaults to empty string but README documentation states default is `root`. This mismatch could cause confusion or deployment failures. Either update the default to match documentation or update the README.</comment>
<file context>
@@ -0,0 +1,883 @@
+ ## @param surrealdb.auth.password SurrealDB password
+ ## NOTE: Must be set if existingSecret is not provided
+ ##
+ password: ""
+ ## @param surrealdb.auth.existingSecret Existing secret containing SurrealDB credentials
+ ## NOTE: Must contain keys specified in secretKey.username and secretKey.password
</file context>
✅ Addressed in dea929a
This commit addresses 6 additional security and configuration issues identified by cubic-dev-ai bot review on Jan 6, 2026. P1 Critical Fixes: - Add runAsUser: 0 to init container securityContext (allows chown to run) - Fix startup probe path from /api/health to /health (consistency) - Add --bind flag to SurrealDB command (respects configured port) P2 Medium Fixes: - Move YAML document separators inside conditional blocks (secret.yaml) - Fix ConfigMap condition to check both extraEnvVarsCM and extraEnvVars - Update SurrealDB password default from "" to "root" (matches README) All changes verified with helm lint and template rendering tests.
Author
Contributor
|
@thuanpham582002 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
5 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="helm/templates/_helpers.tpl">
<violation number="1" location="helm/templates/_helpers.tpl:207">
P2: Security: API keys provided directly via values are stored as plain environment variable values rather than in a Kubernetes Secret. Consider always creating a Secret for API keys (even when provided via values) and referencing them via `secretKeyRef`. This prevents exposure through `kubectl get deployment -o yaml` and provides better access control.</violation>
</file>
<file name="helm/templates/secret.yaml">
<violation number="1" location="helm/templates/secret.yaml:77">
P2: Inconsistent secret key pattern for openaiCompatible. All other providers use a configurable `secretKey` field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: `{{ .Values.config.aiProviders.openaiCompatible.secretKey | default "openai-compatible-api-key" }}`</violation>
<violation number="2" location="helm/templates/secret.yaml:83">
P2: Inconsistent secret key pattern for vertexai credentials. All other providers use a configurable `secretKey` field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: `{{ .Values.config.aiProviders.vertexai.secretKey | default "google-application-credentials-json" }}`</violation>
</file>
<file name="helm/templates/NOTES.txt">
<violation number="1" location="helm/templates/NOTES.txt:49">
P2: The first `kubectl port-forward` command blocks execution, preventing the subsequent API port-forward from running. Users need to background the process or run separate terminals. Consider adding `&` to run in background or provide instructions for running in separate terminals.</violation>
</file>
<file name="helm/values.yaml">
<violation number="1" location="helm/values.yaml:104">
P1: Hardcoded default database credentials (`root`/`root`) pose a security risk. Consider either removing the default password to force users to set one, or using a randomly generated value. At minimum, add a validation that prevents deployment with default credentials.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ## @param surrealdb.auth.password SurrealDB password | ||
| ## NOTE: Must be set if existingSecret is not provided | ||
| ## | ||
| password: "root" |
Contributor
There was a problem hiding this comment.
P1: Hardcoded default database credentials (root/root) pose a security risk. Consider either removing the default password to force users to set one, or using a randomly generated value. At minimum, add a validation that prevents deployment with default credentials.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/values.yaml, line 104:
<comment>Hardcoded default database credentials (`root`/`root`) pose a security risk. Consider either removing the default password to force users to set one, or using a randomly generated value. At minimum, add a validation that prevents deployment with default credentials.</comment>
<file context>
@@ -0,0 +1,883 @@
+ ## @param surrealdb.auth.password SurrealDB password
+ ## NOTE: Must be set if existingSecret is not provided
+ ##
+ password: "root"
+ ## @param surrealdb.auth.existingSecret Existing secret containing SurrealDB credentials
+ ## NOTE: Must contain keys specified in secretKey.username and secretKey.password
</file context>
| @@ -0,0 +1,446 @@ | |||
| {{- /* | |||
Contributor
There was a problem hiding this comment.
P2: Security: API keys provided directly via values are stored as plain environment variable values rather than in a Kubernetes Secret. Consider always creating a Secret for API keys (even when provided via values) and referencing them via secretKeyRef. This prevents exposure through kubectl get deployment -o yaml and provides better access control.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/_helpers.tpl, line 207:
<comment>Security: API keys provided directly via values are stored as plain environment variable values rather than in a Kubernetes Secret. Consider always creating a Secret for API keys (even when provided via values) and referencing them via `secretKeyRef`. This prevents exposure through `kubectl get deployment -o yaml` and provides better access control.</comment>
<file context>
@@ -0,0 +1,446 @@
+ value: {{ .Values.config.tts.batchSize | quote }}
+{{ if or .Values.config.aiProviders.openai.apiKey .Values.config.aiProviders.openai.existingSecret -}}
+{{ if .Values.config.aiProviders.openai.existingSecret -}}
+- name: OPENAI_API_KEY
+ valueFrom:
+ secretKeyRef:
</file context>
| {{ .Values.config.aiProviders.azureOpenai.secretKey | default "azure-openai-api-key" }}: {{ .Values.config.aiProviders.azureOpenai.apiKey | quote }} | ||
| {{- end }} | ||
| {{- if .Values.config.aiProviders.vertexai.credentials }} | ||
| GOOGLE_APPLICATION_CREDENTIALS_JSON: {{ .Values.config.aiProviders.vertexai.credentials | quote }} |
Contributor
There was a problem hiding this comment.
P2: Inconsistent secret key pattern for vertexai credentials. All other providers use a configurable secretKey field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: {{ .Values.config.aiProviders.vertexai.secretKey | default "google-application-credentials-json" }}
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/secret.yaml, line 83:
<comment>Inconsistent secret key pattern for vertexai credentials. All other providers use a configurable `secretKey` field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: `{{ .Values.config.aiProviders.vertexai.secretKey | default "google-application-credentials-json" }}`</comment>
<file context>
@@ -0,0 +1,118 @@
+ {{ .Values.config.aiProviders.azureOpenai.secretKey | default "azure-openai-api-key" }}: {{ .Values.config.aiProviders.azureOpenai.apiKey | quote }}
+ {{- end }}
+ {{- if .Values.config.aiProviders.vertexai.credentials }}
+ GOOGLE_APPLICATION_CREDENTIALS_JSON: {{ .Values.config.aiProviders.vertexai.credentials | quote }}
+ {{- end }}
+{{- end }}
</file context>
| {{ .Values.config.aiProviders.langchain.secretKey | default "langchain-api-key" }}: {{ .Values.config.aiProviders.langchain.apiKey | quote }} | ||
| {{- end }} | ||
| {{- if .Values.config.aiProviders.openaiCompatible.apiKey }} | ||
| OPENAI_COMPATIBLE_API_KEY: {{ .Values.config.aiProviders.openaiCompatible.apiKey | quote }} |
Contributor
There was a problem hiding this comment.
P2: Inconsistent secret key pattern for openaiCompatible. All other providers use a configurable secretKey field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: {{ .Values.config.aiProviders.openaiCompatible.secretKey | default "openai-compatible-api-key" }}
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/secret.yaml, line 77:
<comment>Inconsistent secret key pattern for openaiCompatible. All other providers use a configurable `secretKey` field with a default, but this one uses a hardcoded key name. Consider using the same pattern for consistency: `{{ .Values.config.aiProviders.openaiCompatible.secretKey | default "openai-compatible-api-key" }}`</comment>
<file context>
@@ -0,0 +1,118 @@
+ {{ .Values.config.aiProviders.langchain.secretKey | default "langchain-api-key" }}: {{ .Values.config.aiProviders.langchain.apiKey | quote }}
+ {{- end }}
+ {{- if .Values.config.aiProviders.openaiCompatible.apiKey }}
+ OPENAI_COMPATIBLE_API_KEY: {{ .Values.config.aiProviders.openaiCompatible.apiKey | quote }}
+ {{- end }}
+ {{- if .Values.config.aiProviders.azureOpenai.apiKey }}
</file context>
| export POD_NAME=$(kubectl get pods --namespace {{ $svcNamespace }} -l "app.kubernetes.io/name={{ include "common.names.name" . }},app.kubernetes.io/instance={{ $releaseName }}" -o jsonpath="{.items[0].metadata.name}") | ||
| export CONTAINER_PORT=$(kubectl get pod --namespace {{ $svcNamespace }} $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}") | ||
| echo "Visit http://127.0.0.1:8080 to use your application" | ||
| kubectl --namespace {{ $svcNamespace }} port-forward $POD_NAME 8080:{{ $svcPort }} |
Contributor
There was a problem hiding this comment.
P2: The first kubectl port-forward command blocks execution, preventing the subsequent API port-forward from running. Users need to background the process or run separate terminals. Consider adding & to run in background or provide instructions for running in separate terminals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helm/templates/NOTES.txt, line 49:
<comment>The first `kubectl port-forward` command blocks execution, preventing the subsequent API port-forward from running. Users need to background the process or run separate terminals. Consider adding `&` to run in background or provide instructions for running in separate terminals.</comment>
<file context>
@@ -0,0 +1,110 @@
+ export POD_NAME=$(kubectl get pods --namespace {{ $svcNamespace }} -l "app.kubernetes.io/name={{ include "common.names.name" . }},app.kubernetes.io/instance={{ $releaseName }}" -o jsonpath="{.items[0].metadata.name}")
+ export CONTAINER_PORT=$(kubectl get pod --namespace {{ $svcNamespace }} $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}")
+ echo "Visit http://127.0.0.1:8080 to use your application"
+ kubectl --namespace {{ $svcNamespace }} port-forward $POD_NAME 8080:{{ $svcPort }}
+ echo "API available at http://127.0.0.1:5055"
+ kubectl --namespace {{ $svcNamespace }} port-forward $POD_NAME 5055:{{ $apiPort }}
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What
Add comprehensive Helm chart for deploying Open Notebook on Kubernetes
Why
Features
Installation
Configuration Examples
Basic with OpenAI
NodePort Service
See helm/open-notebook/values.yaml for all configuration options.
See helm/open-notebook/README.md for detailed documentation.
Testing
The Helm chart has been tested and validated:
helm lintpassed with 0 errorsChecklist
Closes #366