-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next Gen Chart #132
Next Gen Chart #132
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
charts/n8n/templates/deployment.webhooks.yaml (3)
43-46
: 🛠️ Refactor suggestionAdd default security context for enhanced security.
Consider adding default security context values to enforce running as non-root user, which is a Kubernetes security best practice.
securityContext: + {{- $defaultSecurityContext := dict "runAsNonRoot" true "runAsUser" 1000 "runAsGroup" 1000 }} + {{- if .Values.webhook.podSecurityContext }} {{- toYaml .Values.webhook.podSecurityContext | nindent 8 }} + {{- else }} + {{- toYaml $defaultSecurityContext | nindent 8 }} + {{- end }}
69-76
: 🛠️ Refactor suggestionAdd default probe configurations.
Consider adding default liveness and readiness probe configurations to ensure proper health checking of the webhook component.
{{- with .Values.webhook.livenessProbe }} livenessProbe: {{- toYaml . | nindent 12 }} + {{- else }} + livenessProbe: + httpGet: + path: /healthz + port: http + initialDelaySeconds: 30 + periodSeconds: 10 {{- end }} {{- with .Values.webhook.readinessProbe }} readinessProbe: {{- toYaml . | nindent 12 }} + {{- else }} + readinessProbe: + httpGet: + path: /healthz + port: http + initialDelaySeconds: 5 + periodSeconds: 10 {{- end }}
87-87
: 🛠️ Refactor suggestionAdd default resource limits.
Consider adding default resource limits to prevent unbounded resource consumption and ensure proper scheduling.
resources: + {{- $defaultResources := dict "requests" (dict "cpu" "100m" "memory" "128Mi") "limits" (dict "cpu" "500m" "memory" "256Mi") }} + {{- if .Values.webhook.resources }} {{- toYaml .Values.webhook.resources | nindent 12 }} + {{- else }} + {{- toYaml $defaultResources | nindent 12 }} + {{- end }}charts/n8n/templates/_helpers.tpl (2)
102-108
:⚠️ Potential issueFix inconsistent value path in serviceAccountName function.
The function still references the old path
.Values.serviceAccount.name
in the else condition while using.Values.n8n.serviceAccount
in the if condition.Apply this diff to fix the inconsistency:
{{- if .Values.n8n.serviceAccount.create }} {{- default (include "n8n.fullname" .) .Values.n8n.serviceAccount.name }} {{- else }} -{{- default "default" .Values.serviceAccount.name }} +{{- default "default" .Values.n8n.serviceAccount.name }} {{- end }}
145-162
: 🛠️ Refactor suggestionEnhance toEnvVars to handle array values.
The
toEnvVars
function handles maps and scalar values but doesn't explicitly handle arrays. This could be problematic for configuration values that are lists.Apply this diff to add array support:
{{- range $key, $value := .values }} {{- if kindIs "map" $value -}} {{- dict "values" $value "prefix" (printf "%s%s" $prefix ($key | upper)) "isSecret" $.isSecret | include "toEnvVars" -}} + {{- else if kindIs "slice" $value -}} + {{- if $.isSecret -}} + {{ $prefix }}{{ $key | upper }}: {{ join "," $value | b64enc }}{{ "\n" }} + {{- else -}} + {{ $prefix }}{{ $key | upper }}={{ join "," $value }}{{ "\n" }} + {{- end -}} {{- else -}} {{- if $.isSecret -}} {{ $prefix }}{{ $key | upper }}: {{ $value | b64enc }}{{ "\n" }} {{- else -}} {{ $prefix }}{{ $key | upper }}={{ $value }}{{ "\n" }} {{- end -}} {{- end -}} {{- end -}}
🧹 Nitpick comments (1)
charts/n8n/templates/deployment.webhooks.yaml (1)
13-18
: Add validation for deployment strategy parameters.Consider adding validation for
maxSurge
andmaxUnavailable
values to ensure they are valid percentages or integers.{{- if eq .Values.webhook.deploymentStrategy.type "RollingUpdate" }} rollingUpdate: - maxSurge: {{ default "25%" .Values.webhook.deploymentStrategy.maxSurge }} - maxUnavailable: {{ default "25%" .Values.webhook.deploymentStrategy.maxUnavailable }} + maxSurge: {{ include "n8n.validateUpdateStrategy" (dict "value" (default "25%" .Values.webhook.deploymentStrategy.maxSurge) "context" $) }} + maxUnavailable: {{ include "n8n.validateUpdateStrategy" (dict "value" (default "25%" .Values.webhook.deploymentStrategy.maxUnavailable) "context" $) }} {{- end }}Add this helper template to
_helpers.tpl
:{{- define "n8n.validateUpdateStrategy" -}} {{- $value := .value -}} {{- if hasSuffix "%" $value -}} {{- $percentage := trimSuffix "%" $value | atoi -}} {{- if and (ge $percentage 0) (le $percentage 100) -}} {{- $value -}} {{- else -}} {{- fail (printf "Invalid percentage value %s: must be between 0%% and 100%%" $value) -}} {{- end -}} {{- else -}} {{- $number := atoi $value -}} {{- if ge $number 0 -}} {{- $value -}} {{- else -}} {{- fail (printf "Invalid value %s: must be a positive integer" $value) -}} {{- end -}} {{- end -}} {{- end -}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/n8n/templates/_helpers.tpl
(1 hunks)charts/n8n/templates/deployment.webhooks.yaml
(1 hunks)charts/n8n/values.yaml
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint-test
charts/n8n/values.yaml
[failure] 45-45:
45:6 [comments] too few spaces before comment
[failure] 49-49:
49:10 [comments] too few spaces before comment
[failure] 68-68:
68:16 [comments] too few spaces before comment
[failure] 113-113:
113:19 [comments] too few spaces before comment
[failure] 238-238:
238:16 [comments] too few spaces before comment
[failure] 283-283:
283:19 [comments] too few spaces before comment
[failure] 404-404:
404:16 [comments] too few spaces before comment
[failure] 452-452:
452:19 [comments] too few spaces before comment
[failure] 501-501:
501:4 [comments] missing starting space in comment
🪛 GitHub Actions: Lint and Test Charts
charts/n8n/values.yaml
[error] 45-45: Too few spaces before comment at line 45, column 6
🪛 yamllint (1.35.1)
charts/n8n/templates/deployment.webhooks.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (7)
charts/n8n/templates/deployment.webhooks.yaml (1)
1-12
: LGTM! Well-structured deployment configuration.The deployment metadata and scaling configuration are properly organized with webhook-specific paths, following Kubernetes best practices.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/_helpers.tpl (1)
54-100
: Verify the impact of removing environment variable generation.The
n8n.deploymentPodEnvironments
function is being removed. This function previously handled critical environment variables likeN8N_PORT
,N8N_ENCRYPTION_KEY
, and Redis configuration. Ensure these environment variables are properly handled in the new implementation.Run the following script to verify environment variable handling:
✅ Verification successful
Environment variables are now properly handled through ConfigMaps and Secrets ✅
The removal of
n8n.deploymentPodEnvironments
is actually an improvement, as environment variables are now managed through Kubernetes-native ConfigMaps and Secrets. All critical environment variables can still be configured:
- Port configuration is handled directly in container spec
- Encryption key can be set via
n8n.config.encryption_key
- Redis configuration has a dedicated section in values.yaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if critical environment variables are handled in the new implementation. # Test: Search for critical environment variables in templates. echo "Checking for N8N_PORT..." rg -A 5 'N8N_PORT' echo "Checking for N8N_ENCRYPTION_KEY..." rg -A 5 'N8N_ENCRYPTION_KEY' echo "Checking for QUEUE_BULL_REDIS..." rg -A 5 'QUEUE_BULL_REDIS'Length of output: 2408
Script:
#!/bin/bash # Search for deployment and environment configurations echo "Checking deployment templates..." fd -e yaml -e tpl deployment charts/n8n/templates --exec cat {} echo -e "\nChecking for environment configurations in values.yaml..." cat charts/n8n/values.yaml echo -e "\nChecking for other template functions..." rg -A 10 '{{- define "n8n.' charts/n8n/templatesLength of output: 30287
charts/n8n/values.yaml (5)
1-9
: LGTM! Good documentation addition.The README section provides a clear overview of the values structure, making it easier for users to understand the configuration options.
44-63
: LGTM! Well-structured n8n configuration section.The n8n configuration section is well-organized with:
- Clear documentation of environment variable transformation.
- Separation of config and secret values.
- Good examples and references to n8n documentation.
🧰 Tools
🪛 GitHub Check: lint-test
[failure] 45-45:
45:6 [comments] too few spaces before comment
[failure] 49-49:
49:10 [comments] too few spaces before comment🪛 GitHub Actions: Lint and Test Charts
[error] 45-45: Too few spaces before comment at line 45, column 6
229-394
: LGTM! Comprehensive worker configuration.The worker configuration section provides:
- Clear separation of worker-specific settings.
- Consistent structure with main n8n configuration.
- Good defaults for security context and probes.
🧰 Tools
🪛 GitHub Check: lint-test
[failure] 238-238:
238:16 [comments] too few spaces before comment
[failure] 283-283:
283:19 [comments] too few spaces before comment
395-561
: LGTM! Well-designed webhook configuration.The webhook configuration section:
- Follows the same pattern as worker configuration.
- Includes clear documentation about webhook processing.
- Provides good defaults for security and monitoring.
🧰 Tools
🪛 GitHub Check: lint-test
[failure] 404-404:
404:16 [comments] too few spaces before comment
[failure] 452-452:
452:19 [comments] too few spaces before comment
[failure] 501-501:
501:4 [comments] missing starting space in comment
Line range hint
599-608
: LGTM! Good Redis integration.The Redis configuration section:
- Uses Bitnami Redis chart.
- Provides sensible defaults for standalone architecture.
- Includes persistence configuration.
🧰 Tools
🪛 GitHub Check: lint-test
[failure] 45-45:
45:6 [comments] too few spaces before comment
[failure] 49-49:
49:10 [comments] too few spaces before comment
[failure] 68-68:
68:16 [comments] too few spaces before comment
[failure] 113-113:
113:19 [comments] too few spaces before comment
[failure] 238-238:
238:16 [comments] too few spaces before comment
[failure] 283-283:
283:19 [comments] too few spaces before comment
[failure] 404-404:
404:16 [comments] too few spaces before comment
[failure] 452-452:
452:19 [comments] too few spaces before comment
[failure] 501-501:
501:4 [comments] missing starting space in comment🪛 GitHub Actions: Lint and Test Charts
[error] 45-45: Too few spaces before comment at line 45, column 6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
charts/n8n/templates/.hpa.yaml (1)
Line range hint
16-17
: Fix inconsistent configuration pathsThe file mixes old and new configuration paths:
- Uses
.Values.n8n.autoscaling.enabled
at the top- But still uses
.Values.autoscaling.minReplicas
and.Values.autoscaling.maxReplicas
Apply this diff to fix the paths:
- minReplicas: {{ .Values.autoscaling.minReplicas }} - maxReplicas: {{ .Values.autoscaling.maxReplicas }} + minReplicas: {{ .Values.n8n.autoscaling.minReplicas }} + maxReplicas: {{ .Values.n8n.autoscaling.maxReplicas }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
♻️ Duplicate comments (3)
charts/n8n/templates/_helpers.tpl (2)
102-108
:⚠️ Potential issueFix inconsistent value path in serviceAccountName function.
The else condition still references the old path
.Values.serviceAccount.name
while the if condition uses.Values.n8n.serviceAccount
. This inconsistency could cause issues.Apply this diff to fix the inconsistency:
{{- if .Values.n8n.serviceAccount.create }} {{- default (include "n8n.fullname" .) .Values.n8n.serviceAccount.name }} {{- else }} -{{- default "default" .Values.serviceAccount.name }} +{{- default "default" .Values.n8n.serviceAccount.name }} {{- end }}
145-162
: 🛠️ Refactor suggestionEnhance toEnvVars to handle array values.
The
toEnvVars
function handles maps and scalar values but doesn't explicitly handle arrays. This could be problematic for configuration values that are lists.Apply this diff to add array support:
{{- range $key, $value := .values }} {{- if kindIs "map" $value -}} {{- dict "values" $value "prefix" (printf "%s%s" $prefix ($key | upper)) "isSecret" $.isSecret | include "toEnvVars" -}} + {{- else if kindIs "slice" $value -}} + {{- if $.isSecret -}} + {{ $prefix }}{{ $key | upper }}: {{ join "," $value | b64enc }}{{ "\n" }} + {{- else -}} + {{ $prefix }}{{ $key | upper }}: {{ join "," $value | quote }}{{ "\n" }} + {{- end -}} {{- else -}}charts/n8n/values.yaml (1)
46-46
:⚠️ Potential issueFix comment formatting to pass linting.
Several comments have formatting issues that need to be addressed to pass the linting checks.
Apply these fixes:
-n8n: # Example: the YAML entry +n8n: # Example: the YAML entry - serviceAccount: # Specifies whether + serviceAccount: # Specifies whether - persistence: # If true + persistence: # If true - serviceAccount: # Specifies whether + serviceAccount: # Specifies whether - persistence: # If true + persistence: # If true - serviceAccount: # Specifies whether + serviceAccount: # Specifies whether - #Command Arguments + # Command ArgumentsAlso applies to: 136-136, 264-264, 306-306, 428-428, 475-475, 524-524
🧰 Tools
🪛 GitHub Check: lint-test
[failure] 46-46:
46:6 [comments] too few spaces before comment🪛 GitHub Actions: Lint and Test Charts
[error] 46-46: Too few spaces before comment at column 6
🧹 Nitpick comments (3)
charts/n8n/README.md (2)
5-5
: Fix grammatical issues in the description.The description has minor grammatical issues that should be fixed for better readability.
Apply this diff to fix the issues:
-A Kubernetes Helm chart for n8n a free and open fair-code licensed node based Workflow Automation Tool. +A Kubernetes Helm chart for n8n, a free and open fair-code licensed node-based Workflow Automation Tool.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: Possible missing comma found.
Context: ...at-square) A Kubernetes Helm chart for n8n a free and open fair-code licensed node...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ... n8n a free and open fair-code licensed node based Workflow Automation Tool. Easily automa...(BASED_HYPHEN)
31-162
: Add descriptions for configuration values.Many configuration values in the values table lack descriptions, which could make it harder for users to understand their purpose and usage.
Consider adding descriptions for values like:
fullnameOverride
image.pullPolicy
image.repository
image.tag
imagePullSecrets
And other values that currently show empty descriptions.charts/n8n/values.yaml (1)
Line range hint
623-633
: Enhance Redis configuration documentation and security settings.The Redis configuration section could benefit from:
- Additional documentation about Redis usage in n8n
- Security-related settings like password protection
- Resource limits configuration
Consider adding these configurations:
redis: enabled: false architecture: standalone auth: enabled: true password: "" # Set a strong password master: persistence: enabled: true existingClaim: "" size: 2Gi resources: limits: memory: 256Mi cpu: 100m requests: memory: 128Mi cpu: 50m # Add security context securityContext: enabled: true fsGroup: 1001 runAsUser: 1001🧰 Tools
🪛 GitHub Check: lint-test
[failure] 46-46:
46:6 [comments] too few spaces before comment
[failure] 136-136:
136:19 [comments] too few spaces before comment
[failure] 264-264:
264:16 [comments] too few spaces before comment
[failure] 306-306:
306:19 [comments] too few spaces before comment
[failure] 428-428:
428:16 [comments] too few spaces before comment
[failure] 475-475:
475:19 [comments] too few spaces before comment
[failure] 524-524:
524:4 [comments] missing starting space in comment🪛 GitHub Actions: Lint and Test Charts
[error] 46-46: Too few spaces before comment at column 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
charts/n8n/Chart.yaml
(2 hunks)charts/n8n/README.md
(1 hunks)charts/n8n/templates/.hpa.yaml
(1 hunks)charts/n8n/templates/_helpers.tpl
(1 hunks)charts/n8n/templates/configmap.yaml
(1 hunks)charts/n8n/templates/deployment.yaml
(2 hunks)charts/n8n/templates/ingress.yaml
(3 hunks)charts/n8n/values.yaml
(1 hunks)examples/values_small_prodlike.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/n8n/templates/ingress.yaml
🧰 Additional context used
🪛 yamllint (1.35.1)
charts/n8n/templates/.hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/configmap.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.yaml
[warning] 9-9: wrong indentation: expected 0 but found 2
(indentation)
🪛 GitHub Check: lint-test
charts/n8n/values.yaml
[failure] 46-46:
46:6 [comments] too few spaces before comment
[failure] 136-136:
136:19 [comments] too few spaces before comment
[failure] 264-264:
264:16 [comments] too few spaces before comment
[failure] 306-306:
306:19 [comments] too few spaces before comment
[failure] 428-428:
428:16 [comments] too few spaces before comment
[failure] 475-475:
475:19 [comments] too few spaces before comment
[failure] 524-524:
524:4 [comments] missing starting space in comment
🪛 GitHub Actions: Lint and Test Charts
charts/n8n/values.yaml
[error] 46-46: Too few spaces before comment at column 6
🪛 LanguageTool
charts/n8n/README.md
[uncategorized] ~5-~5: Possible missing comma found.
Context: ...at-square) A Kubernetes Helm chart for n8n a free and open fair-code licensed node...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ... n8n a free and open fair-code licensed node based Workflow Automation Tool. Easily automa...
(BASED_HYPHEN)
🔇 Additional comments (7)
charts/n8n/templates/configmap.yaml (1)
5-5
: Inconsistent ConfigMap naming across deployment filesThe ConfigMap name change is incomplete and will cause issues.
charts/n8n/Chart.yaml (1)
3-4
: Version bump aligns with breaking changesThe major version bump to 1.0.0 correctly reflects the breaking changes in this PR.
examples/values_small_prodlike.yaml (2)
19-19
: Replace placeholder encryption keyThe encryption key is currently set to a placeholder value
<your-secure-encryption-key>
. This should be replaced with a secure random value in production.
5-16
: Well-structured production configurationThe configuration provides a solid foundation for production deployment:
- Proper PostgreSQL configuration with SSL
- Reasonable memory limits and requests
- Connection pooling configured
Also applies to: 40-44
charts/n8n/templates/deployment.yaml (2)
59-67
: Improved configuration injectionThe switch to
envFrom
withconfigMapRef
andsecretRef
is a better approach for managing environment variables.
91-94
: Enhanced volume mount flexibilityThe addition of
extraVolumeMounts
provides better flexibility for custom volume configurations.charts/n8n/README.md (1)
14-14
: Update placeholder email for n8n maintainer.Using placeholder emails in production documentation is not recommended. Please update with the correct email or remove it if not available.
- name: valkey | ||
version: 2.2.3 | ||
repository: oci://registry-1.docker.io/bitnamicharts | ||
condition: redis.enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
The verification confirms both issues and reveals more concerns:
- Valkey is not advertised as a Redis-compatible replacement but a separate key/value store
- The
redis.enabled
condition is misleading and should bevalkey.enabled
- Migration documentation is crucial as this is not a drop-in replacement
- The entire dependency change from Redis to Valkey needs architectural review as they are different products
🔗 Analysis chain
The Redis to Valkey migration has several issues:
- The condition
redis.enabled
should be renamed tovalkey.enabled
- Migration documentation is needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Valkey's compatibility with Redis
curl -s "https://raw.githubusercontent.com/bitnami/charts/main/bitnami/valkey/values.yaml" | head -n 50
Length of output: 2117
Script:
#!/bin/bash
# Search for Redis-related content in Valkey's full values.yaml and README
echo "=== Checking values.yaml for Redis mentions ==="
curl -s "https://raw.githubusercontent.com/bitnami/charts/main/bitnami/valkey/values.yaml" | grep -i "redis" || echo "No Redis mentions found"
echo -e "\n=== Checking README for Redis compatibility or migration info ==="
curl -s "https://raw.githubusercontent.com/bitnami/charts/main/bitnami/valkey/README.md" | grep -i -A 5 -B 5 "redis" || echo "No Redis mentions found"
echo -e "\n=== Checking if Valkey is meant to be Redis compatible ==="
curl -s "https://raw.githubusercontent.com/bitnami/charts/main/bitnami/valkey/Chart.yaml" | grep -i "description" -A 2
Length of output: 2656
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (7)
examples/values_small_prod.yaml (4)
8-9
: Consider making database host configurableThe database host is hardcoded to
db-rw
. Consider making it more flexible by using a template or allowing it to be overridden, especially for different environments.- host: db-rw + host: {{ .Values.n8n.config.db.postgresdb.host | default "db-rw" }}
28-39
: Enhance volume security configurationWhile the volume configuration is correct, consider adding security context and file permissions.
extraVolumes: - name: db-ca-cert secret: secretName: db-ca items: - key: ca.crt path: ca.crt + mode: 0400 # Read-only for owner
49-54
: Add security headers to ingress configurationConsider adding security headers to enhance the application's security posture.
annotations: cert-manager.io/cluster-issuer: letsencrypt-prod nginx.ingress.kubernetes.io/proxy-body-size: "0" nginx.ingress.kubernetes.io/proxy-buffering: "off" nginx.ingress.kubernetes.io/proxy-request-buffering: "off" nginx.ingress.kubernetes.io/ssl-redirect: "true" + nginx.ingress.kubernetes.io/configuration-snippet: | + more_set_headers "X-Frame-Options: DENY"; + more_set_headers "X-Content-Type-Options: nosniff"; + more_set_headers "X-XSS-Protection: 1; mode=block";
73-77
: Consider high availability setup for productionA single instance PostgreSQL setup (
instances: 1
) might not be suitable for production use.Consider:
- Increasing the number of instances for high availability
- Implementing proper backup strategies
- Setting up read replicas if needed
- instances: 1 + instances: 3charts/n8n/templates/_helpers.tpl (1)
78-94
: Enhance thetoEnvVars
function for better robustness.The function handles maps and scalar values well, but could be improved in the following areas:
- Add support for array values
- Add error handling for invalid values
- Add support for boolean values
Apply this diff to enhance the function:
{{- define "toEnvVars" -}} {{- $prefix := "" }} {{- if .prefix }} {{- $prefix = printf "%s_" .prefix }} {{- end }} {{- range $key, $value := .values }} {{- if kindIs "map" $value -}} {{- dict "values" $value "prefix" (printf "%s%s" $prefix ($key | upper)) "isSecret" $.isSecret | include "toEnvVars" -}} + {{- else if kindIs "slice" $value -}} + {{- if $.isSecret -}} +{{ $prefix }}{{ $key | upper }}: {{ join "," $value | b64enc }}{{ "\n" }} + {{- else -}} +{{ $prefix }}{{ $key | upper }}: {{ join "," $value | quote }}{{ "\n" }} + {{- end -}} + {{- else if kindIs "bool" $value -}} + {{- if $.isSecret -}} +{{ $prefix }}{{ $key | upper }}: {{ $value | toString | b64enc }}{{ "\n" }} + {{- else -}} +{{ $prefix }}{{ $key | upper }}: {{ $value | toString | quote }}{{ "\n" }} + {{- end -}} {{- else -}} + {{- if not $value -}} + {{- fail (printf "Value for key %q cannot be null" $key) -}} + {{- end -}} {{- if $.isSecret -}} {{ $prefix }}{{ $key | upper }}: {{ $value | b64enc }}{{ "\n" }} {{- else -}} {{ $prefix }}{{ $key | upper }}: {{ $value | quote }}{{ "\n" }} {{- end -}} {{- end -}} {{- end -}} {{- end }}charts/n8n/values.yaml (2)
Line range hint
252-419
: Document worker scaling considerations.The worker configuration section introduces horizontal scaling capabilities but lacks guidance on:
- Resource allocation recommendations
- Scaling thresholds
- Impact on database connections
Add documentation comments like:
worker: enabled: false + # -- Worker Configuration + # Considerations when scaling workers: + # 1. Each worker requires its own database connection + # 2. Recommended resource allocation per worker: 1CPU, 2Gi memory + # 3. Monitor queue size to determine optimal scaling thresholds count: 2 concurrency: 2
Line range hint
627-637
: Consider documenting Redis persistence implications.The Redis configuration enables persistence by default but lacks guidance on:
- Data recovery procedures
- Backup strategies
- Storage requirements
Add documentation comments like:
redis: enabled: false architecture: standalone + # -- Redis Persistence Configuration + # Important considerations: + # 1. Enable periodic RDB snapshots for data recovery + # 2. Implement backup strategy for persistent volumes + # 3. Monitor disk usage: size should accommodate 2x dataset size master: persistence: enabled: true existingClaim: "" size: 2Gi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/n8n/templates/_helpers.tpl
(1 hunks)charts/n8n/templates/hpa.yaml
(2 hunks)charts/n8n/values.yaml
(2 hunks)examples/values_small_prod.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
charts/n8n/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (5)
examples/values_small_prod.yaml (1)
18-19
:⚠️ Potential issueReplace placeholder encryption key
The encryption key contains a placeholder value which is not secure for production use.
Please ensure to:
- Generate a secure random encryption key
- Store it in a Kubernetes secret
- Reference it using secretKeyRef similar to the database password
- encryption_key: "<your-secure-encryption-key>" + encryption_key: + valueFrom: + secretKeyRef: + name: n8n-secrets + key: encryption-keycharts/n8n/templates/hpa.yaml (1)
Line range hint
1-31
: LGTM! Consistent use of.Values.n8n.autoscaling
namespace.The HPA configuration has been correctly updated to use the new
.Values.n8n.autoscaling
namespace consistently across all parameters. This aligns with the PR's objective of restructuring configurations.charts/n8n/values.yaml (3)
1-8
: Great addition of a high-level README section!The README section provides a clear overview of the values structure, making it easier for users to understand and navigate the configuration.
421-424
: Excellent documentation of webhook scaling architecture!The comment clearly explains the architectural decision to separate webhook processing into a dedicated instance, with a reference to the relevant GitHub issue for more details.
44-64
: Consider adding validation for sensitive configurations.The n8n configuration section handles sensitive data like database passwords and encryption keys. Consider adding validation or documentation to ensure these are properly set in production environments.
Run this script to check for sensitive configurations in the codebase:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
charts/n8n/templates/_helpers.tpl (1)
77-94
: 🛠️ Refactor suggestionEnhance toEnvVars to handle array values
The function handles maps and scalar values but doesn't explicitly handle arrays. This could be problematic for configuration values that are lists.
Apply this diff to add array support:
{{- range $key, $value := .values }} {{- if kindIs "map" $value -}} {{- dict "values" $value "prefix" (printf "%s%s" $prefix ($key | upper)) "isSecret" $.isSecret | include "toEnvVars" -}} + {{- else if kindIs "slice" $value -}} + {{- if $.isSecret -}} + {{ $prefix }}{{ $key | upper }}: {{ join "," $value | b64enc }}{{ "\n" }} + {{- else -}} + {{ $prefix }}{{ $key | upper }}: {{ join "," $value | quote }}{{ "\n" }} + {{- end -}} {{- else -}}charts/n8n/templates/configmap.yaml (1)
1-10
:⚠️ Potential issueBreaking change: ConfigMap name and structure updated
The changes include:
- ConfigMap name now prefixed with "app-config-"
- Configuration moved from
.Values.config
to.Values.main.config
- Data structure changed from JSON to environment variables
Ensure all deployments are updated to reference the new ConfigMap name:
- Update references in deployment.yaml
- Update references in deployment.worker.yaml
- Update references in deployment.webhooks.yaml
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.yaml (1)
1-12
:⚠️ Potential issueBreaking change: Secret name and structure updated
The changes include:
- Secret name now prefixed with "app-secret-"
- Configuration moved from
.Values.secret
to.Values.main.secret
- Removed N8N_ENCRYPTION_KEY handling
- Added explicit Secret type
Ensure all deployments are updated to reference the new Secret name:
- Update references in deployment.yaml
- Update references in deployment.worker.yaml
- Update references in deployment.webhooks.yaml
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🧹 Nitpick comments (5)
examples/values_small.yaml (1)
1-17
: Ensure New Configuration Structure is Correct and SecureThis new values file introduces the “main” namespace for configuration settings. Please verify that:
- The structure under
main
(config, secret, resources, service) is consistent with the new chart design.- The placeholder
<your-secure-encryption-key>
is replaced with an actual, secure key before production use.- The resource limits and requests, as well as the service type and port, align with your deployment requirements.
examples/values_small_prod.yaml (1)
15-15
: Fix Typographical Error in Comment.
The comment on the password field contains a typo ("reaad"). Please correct it to "read" to improve clarity.README.md (3)
29-40
: Overview Section & Modifier Hyphenation Consistency
The newly added “## Overview” section and bullet list clearly outline the structure of thevalues.yaml
file. However, several items mention “Kubernetes specific settings”. For improved clarity and consistency, consider using “Kubernetes‐specific settings” (with a hyphen) as recommended by the static analysis hints.🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~36-~36: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~39-~39: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Val...(SPECIFIC_HYPHEN)
42-48
: Settings Configuration Documentation
The “## Setting Configuration Values and Environment Variables” section concisely explains how YAML configuration is transformed into environment variables. Note that in line 46 there is an extra closing parenthesis after/examples
—this appears to be a typographical error and should be corrected.
110-720
: Comprehensive Configuration Documentation in the Values File
The extensive YAML documentation covering general settings, ingress, persistence, security contexts, lifecycle hooks, autoscaling, and other Kubernetes configurations is very thorough. A few suggestions for improvement:
- Consider splitting this lengthy section into subsections or including a table of contents to enhance navigability as the file grows.
- Revisit all instances where the phrase “Kubernetes specific” is used and update them to “Kubernetes‐specific” for consistent modifier punctuation (as noted in the static analysis hints).
Overall, the detailed documentation gives users a robust reference for customizing their n8n deployments via the Helm chart.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
README.md
(4 hunks)charts/n8n/templates/NOTES.txt
(1 hunks)charts/n8n/templates/_helpers.tpl
(1 hunks)charts/n8n/templates/configmap.yaml
(1 hunks)charts/n8n/templates/deployment.webhooks.yaml
(1 hunks)charts/n8n/templates/deployment.worker.yaml
(3 hunks)charts/n8n/templates/deployment.yaml
(2 hunks)charts/n8n/templates/hpa.yaml
(2 hunks)charts/n8n/templates/ingress.yaml
(3 hunks)charts/n8n/templates/pvc.yaml
(2 hunks)charts/n8n/templates/secret.yaml
(1 hunks)charts/n8n/templates/service.yaml
(1 hunks)charts/n8n/templates/serviceaccount.yaml
(1 hunks)charts/n8n/templates/tests/test-connection.yaml
(1 hunks)charts/n8n/values.yaml
(2 hunks)examples/values_small.yaml
(1 hunks)examples/values_small_prod.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/n8n/templates/ingress.yaml
- charts/n8n/templates/service.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~31-~31: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~36-~36: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~39-~39: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Val...
(SPECIFIC_HYPHEN)
charts/n8n/templates/NOTES.txt
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ...}" services {{ include "n8n.fullname" . }}) export NODE_IP=$(kubectl get nodes ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...e }} svc -w {{ include "n8n.fullname" . }}' export SERVICE_IP=$(kubectl get svc...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...amespace }} {{ include "n8n.fullname" . }} --template "{{"{{ range (index .status...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 YAMLlint (1.35.1)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/configmap.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.webhooks.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.yaml
[warning] 9-9: wrong indentation: expected 0 but found 2
(indentation)
charts/n8n/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (84)
charts/n8n/templates/_helpers.tpl (2)
54-61
: Breaking change: Service account configuration path updatedThe service account configuration has been moved from
.Values.serviceAccount
to.Values.main.serviceAccount
. This is a breaking change that requires users to update their values files.Consider adding migration documentation to help users transition from the old to the new configuration structure.
63-74
: Breaking change: Persistence configuration path updatedThe persistence configuration has been moved from
.Values.persistence
to.Values.main.persistence
. This is a breaking change that requires users to update their values files.Consider documenting the new configuration structure in the chart's README.md.
charts/n8n/templates/serviceaccount.yaml (1)
1-12
: LGTM: ServiceAccount template updated correctlyThe ServiceAccount template has been correctly updated to use the new
.Values.main.serviceAccount
path while maintaining the same functionality.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/tests/test-connection.yaml (1)
12-18
: Update Test Connection Script & Verify Alpine Image UtilitiesThe container image for the test pod is now changed to Alpine, and the connection test script has been enhanced to include an
nslookup
and awget
call using the new service port reference (.Values.main.service.port
). Please ensure that the Alpine image has the necessary utilities (or a custom image is used) because Alpine’s minimal base may not include bothnslookup
andwget
by default.charts/n8n/templates/pvc.yaml (1)
1-30
: Confirm Updated Persistence Settings and Verify YAML TemplatingThe PVC template has been updated to use the new
.Values.main.persistence
hierarchy. All elements—including annotations, accessModes, storage requests, and storageClass settings—appear to be correctly templated.Note: A YAMLlint warning is reported at line 1 due to templating syntax; verify that this is a false positive and that the templated YAML renders correctly during deployment.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/hpa.yaml (1)
1-33
: Align Autoscaling Configuration with New Values HierarchyThe HPA template now references autoscaling configurations under
.Values.main.autoscaling
, including minimum/maximum replicas and target resource utilization. This change streamlines the configuration.Reminder: If YAMLlint issues (e.g., at line 1) persist, double-check that they result from the templating syntax and not from a genuine error.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/NOTES.txt (1)
8-17
: Document Breaking Change for Service Type ConfigurationThe NOTES file now uses the new service value reference (i.e.
default "ClusterIP" .Values.main.service.type
) to dynamically generate service endpoints based on the service type (NodePort, LoadBalancer, ClusterIP). This change is breaking compared to previous configurations.Please ensure that this breaking change is clearly documented in the chart’s release notes, Chart.yaml comments, or README upgrade documentation so that users know how to update their configurations upon upgrading.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Loose punctuation mark.
Context: ...}" services {{ include "n8n.fullname" . }}) export NODE_IP=$(kubectl get nodes ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...e }} svc -w {{ include "n8n.fullname" . }}' export SERVICE_IP=$(kubectl get svc...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...amespace }} {{ include "n8n.fullname" . }} --template "{{"{{ range (index .status...(UNLIKELY_OPENING_PUNCTUATION)
examples/values_small_prod.yaml (7)
1-10
: Improve Documentation and Overall Readability.
The introductory README-style comments clearly outline the file’s purpose and structure. This improved documentation helps users understand the configuration hierarchy quickly.
20-20
: Fix SSL Configuration Property Name.
The SSL property is set asreject_Unauthorized: true
. For consistency with standard naming conventions and previous recommendations, change it torejectUnauthorized: true
.
26-31
: Verify Extra Environment Variables Setup.
TheextraEnv
block correctly maps the secret reference forDB_POSTGRESDB_PASSWORD
. Just ensure that the referenced secret (db-app
, keypassword
) exists and is up to date.
32-37
: Review Volume Mount Configuration.
The configuration underextraVolumeMounts
(and the correspondingextraVolumes
block) is structured clearly to mount the CA certificate. Confirm that the mount paths and secret names align with your deployment’s security and operational requirements.
46-50
: Validate Resource Requests and Limits.
The resource definitions for memory underresources
set a limit of 2048Mi and a request of 512Mi. Verify these values reflect your production needs and cluster capacity.
51-68
: Review Ingress Configuration.
The ingress section includes necessary annotations (e.g., for cert-manager and buffering settings) and TLS configurations. Confirm that these settings are compatible with your nginx ingress controller and cert-manager setup.
71-93
: Validate CNPG DB Cluster Definition.
The new CNPG Cluster resource is defined usingapiVersion: postgresql.cnpg.io/v1
and includes bootstrap and resource specifications. Please verify that your cluster environment supports this CRD and that the parameters (e.g., shared_buffers, memory requests/limits) meet your operational requirements.charts/n8n/templates/deployment.worker.yaml (13)
1-1
: Worker Deployment Enable Check.
The conditional{{- if .Values.worker.enabled }}
ensures that the worker deployment is only created when enabled. This is clear and correct.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
9-10
: Worker Replica Count Configuration.
Using{{ .Values.worker.count }}
to set the replicas when autoscaling is disabled is straightforward. Please ensure this value is defined appropriately in your values file.
13-17
: Configure Deployment Strategy for Workers.
The strategy block correctly utilizes.Values.worker.deploymentStrategy.type
and, when set to "RollingUpdate", applies defaults formaxSurge
andmaxUnavailable
. Verify that these defaults (“25%”) are suitable for your worker update strategy.
27-29
: Apply Worker-Specific Pod Annotations.
The deployment template conditionally adds pod annotations from.Values.worker.podAnnotations
, which enhances customization per environment.
33-35
: Include Additional Worker Pod Labels.
Appending any provided labels from.Values.worker.podLabels
increases the flexibility for pod selection.
44-44
: Worker Pod Security Context Verification.
The template applies security context settings via.Values.worker.podSecurityContext
. Ensure these settings comply with your cluster’s security best practices.
45-47
: Render Worker Init Containers Template.
Using thetpl
function to render.Values.worker.initContainers
is a good approach. Verify that the templated content renders as intended in the final manifest.
50-51
: Worker Container Identification.
The container’s name is set to{{ .Chart.Name }}-worker
, which helps differentiate it from other components in your deployment.
55-60
: Worker Command and Arguments Configuration.
The template allows overriding the worker’s command and arguments through.Values.worker.command
and.Values.worker.commandArgs
, with sensible defaults (["n8n"]
and["worker", "--concurrency={{ .Values.worker.concurrency }}"]
). Ensure these defaults align with your process startup requirements.
71-71
: Flexible Worker Port Setting.
Usingget .Values.worker.config "port" | default 5678
allows dynamic port configuration. Confirm that the port specified (or default) is not conflicting with other services.
73-80
: Verify Health Probe Configuration for Worker.
The worker deployment references.Values.main.livenessProbe
and.Values.main.readinessProbe
for health checks. Please verify that using the main probe settings for workers is intentional; if worker-specific probes are needed, consider defining them under the worker key.
81-82
: Worker Resource Specifications.
Resource requests and limits are set with.Values.worker.workerResources
. Confirm that these values are tuned for the expected load of worker processes.
86-97
: Worker Scheduling and Volume Declaration.
Node selectors, affinity, tolerations, and volume definitions are properly referenced from worker-specific values. This provides flexibility in scheduling and storage configuration.charts/n8n/templates/deployment.webhooks.yaml (16)
1-1
: Webhook Deployment Enable Check.
The condition{{- if .Values.webhook.enabled }}
properly gates the creation of the webhook deployment.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
1-1
: YAML Syntax Concern.
A YAMLlint error indicates a syntax issue at the very beginning of the file (line 1). Please double-check the file structure to ensure that template delimiters or preceding content do not trigger a YAML parsing error during linting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
9-10
: Webhook Replica Count Configuration.
Using.Values.webhook.replicaCount
to set the number of replicas when autoscaling is disabled is clear; just confirm that this value meets your operational needs.
12-18
: Configure Webhook Deployment Strategy.
The deployment strategy is now sourced from.Values.webhook.deploymentStrategy
. The use of defaults formaxSurge
andmaxUnavailable
helps in maintaining a predictable rollout process.
22-23
: Webhook Selector Label Specification.
Labeling the pods withapp.kubernetes.io/type: webhook
is effective for resource targeting.
27-29
: Apply Webhook Pod Annotations.
The annotations from.Values.webhook.podAnnotations
are rendered correctly, facilitating configuration changes through checksum updates.
33-35
: Include Webhook-specific Pod Labels.
Additional pod labels from.Values.webhook.podLabels
enable further identification and scheduling options.
41-42
: Webhook Pod Security Context.
The webhook deployment applies the pod security context via.Values.webhook.podSecurityContext
. Please verify these values with your security stance.
44-47
: Render Webhook Init Containers.
Utilizing thetpl
function to render init containers based on.Values.webhook.initContainers
provides flexibility for pre-container initialization steps.
49-57
: Webhook Container Image and Command Configuration.
The container image and pull policy are correctly defined. The command block accounts for possible overrides while defaulting to["n8n"]
if no command is provided.
58-64
: Webhook Container Arguments Setup.
The template uses.Values.webhook.commandArgs
if provided; otherwise, it defaults to["webhook"]
. This ensures the container runs in the appropriate mode.
66-68
: Webhook Container Port Configuration.
The container port is determined fromget .Values.main.config "port" | default 5678
. Verify whether the webhook should share the same port as the main application or if a distinct port is required.
69-76
: Webhook Health Probes.
Liveness and readiness probes are pulled from.Values.webhook.livenessProbe
and.Values.webhook.readinessProbe
, respectively. Check that these settings result in effective health checking for your webhook component.
77-81
: Import Environment Variables for Webhook.
TheenvFrom
block correctly imports configurations via references to the ConfigMap and Secret. Ensure that the names generated by{{ include "n8n.fullname" . }}
match your resource definitions.
82-90
: Webhook Container Additional Settings.
Lifecycle hooks, security context, and resource configurations are applied as expected. Review these settings to ensure the webhook container operates within desired resource and lifecycle parameters.
92-102
: Webhook Scheduling and Volume Settings.
The node selector, affinity, tolerations, and volumes are configured with webhook-specific values, which aids in precise pod scheduling and storage management.charts/n8n/templates/deployment.yaml (11)
8-10
: Main Deployment Replica Configuration.
If autoscaling is disabled, the replica count is set using.Values.main.replicaCount
. This conditional setup is clear; please verify that the value suits your load expectations.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 9-9: wrong indentation: expected 0 but found 2
(indentation)
11-17
: Main Deployment Strategy Setup.
The deployment strategy is defined using.Values.main.deploymentStrategy.type
with a conditional rolling update section that applies defaults formaxSurge
andmaxUnavailable
. This setup supports controlled rollouts.
18-22
: Set Master Pod Selector and Labels.
Labels such asapp.kubernetes.io/type: master
ensure that the pods are identifiable as master components. The inclusion of additional labels from.Values.main.podLabels
offers further customization.
23-29
: Configure Main Pod Annotations.
The metadata includes a checksum for config changes along with annotations from.Values.main.podAnnotations
, which supports automated pod restarts when configurations change.
36-42
: Main Deployment Base Settings.
Image pull secrets, the service account, and pod security context are properly specified. These settings are fundamental for secure deployments and should be reviewed periodically for compliance with organizational standards.
43-48
: Main Init Containers Block.
The template conditionally adds init containers using the content from.Values.main.initContainers
, providing flexibility for pre-start operations.
49-58
: Main Container Command and Image Configuration.
The primary container supports command overrides through.Values.main.command
, and its image is constructed using values from the image configuration. This design facilitates updates without requiring image rebuilds.
59-67
: Inject Environment Variables into Main Container.
Environment variables are imported from ConfigMaps and Secrets usingenvFrom
, and any extra environment variables defined in.Values.main.extraEnv
are iterated over. This mechanism standardizes configuration injection.
68-70
: Configure Health Probes and Lifecycle.
Liveness and readiness probes are included, and the container lifecycle configuration is applied from.Values.main.lifecycle
. Validate that the health check configuration (including the port derived from.Values.main.config
) meets your requirements.
71-74
: Main Container Resource Specification.
The container’s resource requests and limits are defined by.Values.main.resources
. This provides a great degree of flexibility to tailor performance based on workload needs.
89-94
: Volume Mounts and Volumes Configuration.
The main deployment includes a mandatory data volume mount via{{ include "n8n.pvc" . }}
, with optional extra volume mounts and volumes rendered from.Values.main.extraVolumeMounts
and.Values.main.extraVolumes
, respectively. Ensure these settings reflect your storage and persistence strategies.charts/n8n/values.yaml (21)
1-10
: Enhanced Documentation in Values File.
The new README comments at the beginning offer a high-level overview of the values hierarchy, greatly improving discoverability and ease of configuration for users.
25-30
: Image Configuration.
The image settings (repository, pull policy, and tag) are clearly defined. Consider specifying an explicit tag to avoid accidental upgrades.
31-47
: Ingress Settings.
The ingress block is documented with annotations, a custom class, hosts, and TLS settings. Verify that these values match your production and security requirements.
48-70
: Main Application Config Overview.
Themain
section explains how configuration and secrets are transformed into environment variables. The inline examples and comments provide good guidance, although actual configuration values should be provided as needed.
81-108
: Persistence Settings for the Main Application.
The persistence block is comprehensive, including access modes and size definitions with extensive commentary. This clarity helps users choose between dynamic provisioning and using existing claims.
119-123
: Extra Volumes and Mounts Customization.
Optional extra volumes and mounts are provided to allow users to further extend the deployment with custom manifests (like mounting additional certificates).
125-127
: Replica Count Definition.
ThereplicaCount
is set for the main application, which is straightforward. Adjust this number as load requirements dictate.
128-139
: Deployment Strategy Section.
The deployment strategy is clearly documented with sample values, and the default is set to "Recreate". This section offers guidance and the flexibility to opt into RollingUpdate when desired.
140-148
: Service Account Configuration.
Service account settings include options for creation, annotations, and naming. These details help ensure that the container has the correct permissions during runtime.
149-165
: Pod Annotations, Labels, and Security Context.
The file clearly separates settings for pod annotations, labels, and security contexts. This organization promotes consistency and easier maintenance of security settings across deployments.
166-169
: Lifecycle Hook Explanation.
The lifecycle section now includes clear commentary and examples, allowing users to add custom startup behavior if necessary.
177-187
: Container Command Customization.
Providing an override for the container command is useful for addressing runtime issues or custom initialization. The inline examples further illustrate potential use cases.
189-214
: Health Probe Configurations.
Detailed sample configurations for both liveness and readiness probes are provided, which help in adjusting timeouts and thresholds as needed.
215-224
: Init Containers Documentation.
The commented example for init containers explains how to add pre-start commands. This aids users who wish to extend the container initialization process.
226-233
: Service Settings Clarification.
Service type and port definitions are clearly laid out, ensuring that users can easily understand and modify the service exposure settings.
234-245
: Resource Settings Placeholder.
Leaving theresources
block empty by default encourages users to consciously set resource limits based on their environment's capabilities.
246-252
: Autoscaling Configuration.
Autoscaling parameters are defined with sensible defaults. Verify that the min/max replica counts and utilization targets match your cluster’s scaling behavior.
253-256
: Cluster Scheduling Options.
The nodes selector, tolerations, and affinity sections are included for advanced scheduling. This structured approach allows for fine-grained control over pod placement.
257-339
: Worker Settings Overview.
The worker-related configuration is organized similarly to the main section and includes customizations for replica count, persistence, and deployment strategy. This consistency enhances maintainability and clarity.
426-565
: Webhook Configuration.
The webhook section mirrors the main and worker configurations, providing settings for persistence, deployment strategy, and command overrides. Ensure that these values are tailored to your webhook processing needs.
632-643
: Redis Configuration.
The Redis settings are clearly defined and default to standalone mode with persistence enabled. Adjust these values if your deployment requires a clustered or more resilient setup.README.md (9)
1-7
: INFO Block Introduction
The new info block at the top effectively highlights the need for additional maintainers and contributors. The call to join the discussion via the provided link is clear and inviting.
16-16
: Helm Chart Source Code Link
The added line clearly states the source code location with a direct link. Ensure that this URL is maintained as the repository evolves.
25-26
: Clarify Database Options Requirement
The addition of “- external Postgres DB or embedded SQLite (bundled with n8n)” helps clarify available database options. It might be beneficial to briefly explain the trade-offs (e.g., for embedded SQLite) so that users can make an informed decision.
49-60
: YAML Example for Environment Variable Transformation
This YAML snippet clearly demonstrates how specific keys (e.g.n8n.encryption_key
,db.type
, etc.) are converted into environment variables. The inline comments make it easy to understand the transformation process.
71-73
: Installation Command Update
The installation section now reflects the major version update (from0.25.2
to1.0.0
) and includes the updated OCI registry URL. This update is clearly communicated and aligns with the overall redesign.
76-98
: Updated Examples Section
The examples provided (including the YAML configuration for a small deployment with NodePort) are well-structured and give users a practical reference. The updated example is comprehensive and integrates clearly with the new configuration hierarchy.
102-108
: N8N Specific Config Section Clarity
The “## N8N Specific Config Section” along with its descriptive text emphasizes that all configuration values are adjustable. This detailed explanation sets the right expectations for users customizing the chart.
746-759
: Scaling and Advanced Configuration Options
The section on scaling clearly explains how to enable queue-mode, configure worker and webhook instances, and optionally use an external Redis server. The examples provided are clear and informative, effectively guiding users on how to scale their deployment.🧰 Tools
🪛 LanguageTool
[formatting] ~752-~752: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... instances and limited high availability, because the controller instance remains as Sing...(COMMA_BEFORE_BECAUSE)
789-794
: Chart Release Workflow Instructions
The release workflow instructions at the end of the README provide a concise, step-by-step guide for updating the chart version and creating a new release. This systematic approach will aid maintainers in ensuring consistency during deployments and releases.🧰 Tools
🪛 LanguageTool
[duplication] ~793-~793: Possible typo: you repeated a word.
Context: ... 3. In GitHub create a new release with the the chart version number as the tag and a t...(ENGLISH_WORD_REPEAT_RULE)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
27-37
: Overview Section Redesign
The section formerly titled “Configuration” is now “Overview” and offers a structured list of the subsections within the values file.
• Consider consistently hyphenating “Kubernetes-specific” (e.g. “Worker-related settings + Kubernetes‑specific settings”) to adhere to common style conventions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
101-107
: N8N Specific Config Section – Content Clarification
The “N8N Specific Config Section” (lines 101–107) now explains that every possible n8n config value can be set. However, the sentence “this Charts just forwards everything down to the n8n pods” contains a grammatical issue. Consider changing it to “this chart just forwards everything down to the n8n pods.”
1-744
: Overall Documentation Quality and Consistency
The README has been substantially reworked to improve clarity and structure. The revised headings, enhanced examples, and updated installation instructions all contribute to a better user experience. Please double-check minor grammatical errors (like the “Charts” typo) and consider the hyphenation suggestion from earlier. Overall, the documentation now better aligns with the chart’s architectural changes and version upgrade.🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
🔇 Additional comments (10)
README.md (10)
1-7
: Info Block Update – Maintainers Callout
The new info block (lines 1–7) inviting maintainers and contributors is clear and engaging. It effectively signals community involvement.
14-14
: Helm Chart Source Link Added
Line 14 now explicitly provides the source code URL of the Helm chart. This improves transparency and ease of access to the repository.
24-24
: Enhanced Requirements Detail
The requirements list now mentions “external Postgres DB or embedded SQLite,” which clarifies the options available to the user.
39-44
: Configuration Directions Clarification
The updated instructions (lines 39–44) clearly explain that n8n–specific settings should be added under eithermain.config:
ormain.secret:
. This guidance is straightforward and helpful.
45-47
: Important Note Emphasis Updated
The callout using the [!IMPORTANT] directive (lines 45–47) now emphasizes that the YAML configuration is transformed 1:1 into environment variables. The change makes potential impacts very clear to users.
48-59
: YAML Example for ENV Variables
The example YAML snippet (lines 48–59) effectively illustrates how configuration entries are converted into environment variables. Its inline comments (e.g. “==> turns into ENV: …”) provide an excellent reference.
71-71
: Installation Command Version Update
The Helm install command (line 71) now specifies version 1.0.0, which aligns with the major version upgrade in the Chart. Please ensure that this version update is reflected in all related documentation and configuration files.
75-97
: Updated Example Configuration
The “Examples” section (lines 75–97) has been restructured to provide a clear, practical configuration sample for a small deployment. The example correctly demonstrates how to set parameters for the n8n app, secrets, resources, and service type.
732-744
: Redis Configuration Update
The Redis section (lines 732–744) now clearly specifies that Redis is disabled by default, with an explicit “architecture: standalone” setting and an enabled persistence for the master. This update makes the configuration intent very explicit.
695-731
: Additional Resources and Template Examples
The sections covering additional resources and templates (lines 695–731) provide commented examples for including arbitrary Kubernetes manifests alongside the chart. These examples are comprehensive and serve as a useful reference for advanced customizations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
README.md (1)
758-761
:⚠️ Potential issueUpdate outdated scaling configuration example.
The documentation refers to
scaling.enable
andscaling.worker.count
, but the values.yaml usesworker.enabled
andworker.count
.Update the documentation to match the actual configuration:
-set `scaling.enable` to true. +set `worker.enabled` to true. This chart is configured to spawn two worker instances. ```yaml -scaling: - enabled: true +worker: + enabled: true + count: 2</blockquote></details> </blockquote></details> <details> <summary>♻️ Duplicate comments (12)</summary><blockquote> <details> <summary>examples/8gears.yaml (4)</summary><blockquote> `20-21`: _:warning: Potential issue_ **Fix SSL configuration property name.** The SSL configuration property has incorrect casing which might cause issues. Apply this diff to fix the property name: ```diff - reject_Unauthorized: true + rejectUnauthorized: true
83-85
: 🛠️ Refactor suggestionOptimize PostgreSQL parameters for production.
The current shared_buffers value (64MB) is too low for production use, and other important parameters are missing.
Apply this diff to optimize the PostgreSQL parameters:
postgresql: parameters: - shared_buffers: "64MB" + shared_buffers: "128MB" + max_connections: "100" + work_mem: "4MB" + maintenance_work_mem: "64MB" + effective_cache_size: "512MB" + random_page_cost: "1.1"
45-49
: 🛠️ Refactor suggestionAdd CPU resource configuration.
Memory resources are specified but CPU resources are missing. This could lead to resource contention issues.
Apply this diff to add CPU resource configuration:
resources: limits: memory: 2048Mi + cpu: "1" requests: memory: 512Mi + cpu: "200m"
11-15
: 🛠️ Refactor suggestionAdd missing PostgreSQL configurations.
The database configuration is missing important parameters like database name and port.
Apply this diff to add missing database configurations:
db: type: postgresdb postgresdb: host: db-rw + database: n8n + port: 5432 user: n8nexamples/values_small_prod.yaml (4)
20-21
:⚠️ Potential issueFix SSL configuration property name.
The SSL configuration property has incorrect casing which might cause issues.
83-85
: 🛠️ Refactor suggestionOptimize PostgreSQL parameters for production.
The current shared_buffers value (64MB) is too low for production use, and other important parameters are missing.
45-49
: 🛠️ Refactor suggestionAdd CPU resource configuration.
Memory resources are specified but CPU resources are missing. This could lead to resource contention issues.
10-14
: 🛠️ Refactor suggestionAdd missing PostgreSQL configurations.
The database configuration is missing important parameters like database name and port.
examples/values_full.yaml (4)
20-21
:⚠️ Potential issueFix SSL configuration property name.
The SSL configuration property has incorrect casing which might cause issues.
101-102
: 🛠️ Refactor suggestionOptimize PostgreSQL parameters for production.
The current shared_buffers value (64MB) is too low for production use, and other important parameters are missing.
51-55
: 🛠️ Refactor suggestionAdd CPU resource configuration.
Memory resources are specified but CPU resources are missing. This could lead to resource contention issues.
11-15
: 🛠️ Refactor suggestionAdd missing PostgreSQL configurations.
The database configuration is missing important parameters like database name and port.
🧹 Nitpick comments (10)
charts/n8n/templates/_helpers.tpl (1)
77-94
: Add support for array values in environment variables.The function handles maps and scalar values well, but could be enhanced to support array values, which are common in Kubernetes configurations.
Apply this diff to add array support:
{{- range $key, $value := .values }} {{- if kindIs "map" $value -}} {{- dict "values" $value "prefix" (printf "%s%s" $prefix ($key | upper)) "isSecret" $.isSecret | include "toEnvVars" -}} + {{- else if kindIs "slice" $value -}} + {{- if $.isSecret -}} +{{ $prefix }}{{ $key | upper }}: {{ join "," $value | b64enc }}{{ "\n" }} + {{- else -}} +{{ $prefix }}{{ $key | upper }}: {{ join "," $value | quote }}{{ "\n" }} + {{- end -}} {{- else -}}examples/values_full.yaml (1)
60-60
: Document or remove placeholder configuration.The
ASDF: "1234"
appears to be a placeholder configuration. This should be documented or removed to prevent confusion.Apply this diff to document the placeholder:
- ASDF: "1234" + # Example configuration - replace with actual webhook configuration + # WEBHOOK_CONFIG: "your-config-value"charts/n8n/templates/deployment.yaml (2)
8-9
: Fix indentation in replicas section.The indentation in the replicas section is incorrect according to YAML standards.
Apply this diff to fix the indentation:
- {{- if not .Values.main.autoscaling.enabled }} - replicas: {{ .Values.main.replicaCount }} +{{- if not .Values.main.autoscaling.enabled }} +replicas: {{ .Values.main.replicaCount }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 9-9: wrong indentation: expected 0 but found 2
(indentation)
68-72
: Consider using a more explicit environment variable configuration.The current environment variable configuration uses a ternary operator that might be confusing. Consider using a more explicit approach.
Apply this diff to make the configuration more explicit:
- env: {{ not (empty .Values.main.extraEnv) | ternary nil "[]" }} - {{- range $key, $value := .Values.main.extraEnv }} - - name: {{ $key }} - {{- toYaml $value | nindent 14 }} - {{- end }} + {{- if .Values.main.extraEnv }} + env: + {{- range $key, $value := .Values.main.extraEnv }} + - name: {{ $key }} + {{- toYaml $value | nindent 14 }} + {{- end }} + {{- end }}charts/n8n/templates/deployment.webhook.yaml (1)
2-3
: Consider adding explicit error message for Redis validation.The Redis validation is good, but adding an explicit error message would help users understand why the validation failed.
Consider adding a comment explaining that Redis is required for webhook functionality.
{{- /* Validate Redis configuration */}} +{{- /* Redis is required for webhook functionality as it's used for message queuing */}} {{- include "n8n.validateRedis" . }}
charts/n8n/values.yaml (4)
432-442
: Fix trailing whitespace.Remove trailing whitespace to maintain consistent formatting.
Apply this diff to fix the formatting:
- enabled: false + enabled: false🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 432-432: trailing spaces
(trailing-spaces)
[error] 442-442: trailing spaces
(trailing-spaces)
50-70
: Consider adding validation for encryption key.The encryption key is a critical security parameter. When running n8n in a stateless configuration, it's essential to ensure this key is properly set.
Consider adding a validation check in the helm templates to ensure the encryption key is set when running in stateless mode. Example validation in
_helpers.tpl
:{{- define "n8n.validateEncryptionKey" -}} {{- if and .Values.main.config.n8n.stateless (not .Values.main.secret.n8n.encryption_key) -}} {{- fail "When running n8n in stateless mode, encryption_key must be set" -}} {{- end -}} {{- end -}}
81-96
: Improve resource configuration example.The example configuration shows memory limits but lacks CPU limits, which are equally important for proper resource management.
Consider updating the example to include CPU limits:
resources: limits: + cpu: 1000m memory: 2048Mi requests: + cpu: 200m memory: 512Mi
739-741
: Inconsistent Redis persistence configuration.Redis master persistence is enabled by default but the size is relatively small for production use.
Consider:
- Making persistence disabled by default to match the Redis service's enabled state (false)
- Adding a comment about sizing considerations for production use
- Adding configuration for backup strategy
persistence: - enabled: true + enabled: false existingClaim: "" size: 2Gi + # For production, consider increasing size based on your workflow storage needs + # Recommended minimum for production: 10GiREADME.md (1)
29-37
: Add hyphens to "Kubernetes-specific".For consistency and proper technical writing, "Kubernetes specific" should be hyphenated when used as a compound modifier.
Apply these changes:
-The `values.yaml` file is divided into a multiple n8n and Kubernetes specific sections. +The `values.yaml` file is divided into multiple n8n and Kubernetes-specific sections. -3. Main n8n app configuration + Kubernetes specific settings +3. Main n8n app configuration + Kubernetes-specific settings -4. Worker related settings + Kubernetes specific settings +4. Worker related settings + Kubernetes-specific settings -5. Webhook related settings + Kubernetes specific settings +5. Webhook related settings + Kubernetes-specific settings -7. Redis related settings + Kubernetes specific settings +7. Redis related settings + Kubernetes-specific settings🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md
(3 hunks)charts/n8n/templates/_helpers.tpl
(1 hunks)charts/n8n/templates/configmap.webhook.yaml
(1 hunks)charts/n8n/templates/deployment.webhook.yaml
(1 hunks)charts/n8n/templates/deployment.yaml
(2 hunks)charts/n8n/templates/secret.webhook.yaml
(1 hunks)charts/n8n/templates/service.webhook.yaml
(1 hunks)charts/n8n/values.yaml
(2 hunks)examples/8gears.yaml
(1 hunks)examples/values_full.yaml
(1 hunks)examples/values_small_prod.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/configmap.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
examples/values_full.yaml
[error] 61-61: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[warning] 87-87: too many blank lines
(3 > 2) (empty-lines)
charts/n8n/templates/service.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/values.yaml
[error] 432-432: trailing spaces
(trailing-spaces)
[error] 442-442: trailing spaces
(trailing-spaces)
charts/n8n/templates/deployment.yaml
[warning] 9-9: wrong indentation: expected 0 but found 2
(indentation)
🪛 LanguageTool
README.md
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
🔇 Additional comments (8)
charts/n8n/templates/_helpers.tpl (3)
54-61
: LGTM! Value path inconsistency fixed.The function now consistently uses
.Values.main.serviceAccount
path throughout, fixing the previously flagged inconsistency.
64-74
: LGTM! Persistence configuration properly restructured.The function correctly handles all persistence scenarios (emptyDir, existingClaim, dynamic) while using the restructured
.Values.main.persistence
path.
97-102
: LGTM! Essential Redis validation for webhooks.The function correctly enforces the Redis dependency for webhook functionality with a clear error message.
charts/n8n/templates/configmap.webhook.yaml (1)
1-10
: LGTM! Well-structured webhook ConfigMap.The ConfigMap is properly configured with:
- Conditional creation based on webhook.config
- Correct use of helper functions for name and labels
- Appropriate use of toEnvVars for data conversion
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.webhook.yaml (1)
1-12
: LGTM! Well-structured webhook Secret.The Secret is properly configured with:
- Conditional creation based on webhook.secret
- Correct use of helper functions
- Appropriate Secret type (Opaque)
- Proper base64 encoding via toEnvVars isSecret flag
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/service.webhook.yaml (1)
1-22
: LGTM! Well-structured webhook Service.The Service is properly configured with:
- Conditional creation based on webhook.enabled
- Correct selector labels for webhook pods
- Sensible defaults for service type (ClusterIP) and port (80)
- Proper restructuring under .Values.webhook namespace
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.webhook.yaml (1)
97-101
: Consider using a more explicit environment variable configuration.The current environment variable configuration uses a ternary operator that might be confusing. Consider using a more explicit approach.
charts/n8n/values.yaml (1)
261-265
:❌ Incorrect review comment
Verify worker concurrency setting.
The worker concurrency is set to 10 in the comment but 2 in the actual configuration. This inconsistency needs to be resolved.
🌐 Web query:
What is the recommended worker concurrency setting for n8n?
💡 Result:
n8n recommends setting worker concurrency to 5 or higher for optimal performance in queue mode[4]. This balances resource utilization while avoiding database connection pool exhaustion when scaling with multiple workers[4]. Key considerations include:
Default behavior: Workers start with a default concurrency of
10
unless configured otherwise[3][5].Workload type:
- For I/O-heavy workflows (API calls, file operations), higher concurrency (e.g., 10-30) improves parallel processing efficiency[9].
- For CPU-intensive tasks (complex data transformations), lower concurrency prevents thread blocking[9].
Scaling guidance:
- Use multiple workers with moderate concurrency rather than few workers with high concurrency to:
- Distribute load across CPU cores[9]
- Reduce crash impact from individual workflows[2]
- Avoid database bottlenecks (connection limits)[4]
Configuration methods:
n8n worker --concurrency=15 # Command-line flag
N8N_CONCURRENCY_PRODUCTION_LIMIT=20 n8n worker # Environment variable (self-hosted)[4]
Monitor performance metrics and adjust based on:
- Memory usage per execution[2]
- Database connection limits[4]
- Execution completion times[9]
For enterprise Cloud deployments, contact n8n support to enable queue mode and discuss tailored configurations[1][4].
Citations:
- 1: https://docs.n8n.io/manage-cloud/concurrency/
- 2: https://community.n8n.io/t/handling-of-the-n8n-worker-processes/5213
- 3: https://docs.n8n.io/hosting/scaling/concurrency-control/
- 4: https://docs.n8n.io/hosting/scaling/queue-mode/
- 5: https://community.n8n.io/t/worker-concurrency/43749/2
- 6: https://community.n8n.io/t/best-practices-for-concurrent-queries/29778
- 7: https://community.n8n.io/t/unable-to-set-concurrency-for-worker/29545
- 8: https://community.n8n.io/t/execute-one-workflow-instance-at-a-time/18618
- 9: https://community.n8n.io/t/differences-and-uses-workers-concurrencies/32138
- 10: https://www.youtube.com/watch?v=PnoE0xV8BX8
Clarification on Worker Parameters in
charts/n8n/values.yaml
The current configuration is correct. Note that the configuration distinguishes between the number of workers (
count: 2
) and the number of jobs each worker can handle concurrently (concurrency: 10
). According to n8n recommendations:
- Worker Count (
count
) controls the number of worker instances.- Worker Concurrency (
concurrency
)—which defaults to 10—is the number of jobs a worker can process in parallel, and should be set to 5 or higher for optimal performance in queue mode (with 10 being perfectly acceptable).There is no conflict between the comment and the configuration values since they serve different purposes.
Likely an incorrect or invalid review comment.
# - -c | ||
# - chmod o+rx /root; chown -R node /root/.n8n || true; chown -R node /root/.n8n; ln -s /root/.n8n /home/node; chown -R node /home/node || true; node /usr/local/bin/n8n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern in sample command.
The sample command includes operations that run n8n as root, which is a security risk.
Consider adding a warning comment about the security implications:
- # - chmod o+rx /root; chown -R node /root/.n8n || true; chown -R node /root/.n8n; ln -s /root/.n8n /home/node; chown -R node /home/node || true; node /usr/local/bin/n8n
+ # WARNING: The following command runs n8n as root, which is a security risk. Use only for debugging.
+ # - chmod o+rx /root; chown -R node /root/.n8n || true; chown -R node /root/.n8n; ln -s /root/.n8n /home/node; chown -R node /home/node || true; node /usr/local/bin/n8n
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# - -c | |
# - chmod o+rx /root; chown -R node /root/.n8n || true; chown -R node /root/.n8n; ln -s /root/.n8n /home/node; chown -R node /home/node || true; node /usr/local/bin/n8n | |
# - -c | |
# WARNING: The following command runs n8n as root, which is a security risk. Use only for debugging. | |
# - chmod o+rx /root; chown -R node /root/.n8n || true; chown -R node /root/.n8n; ln -s /root/.n8n /home/node; chown -R node /home/node || true; node /usr/local/bin/n8n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
README.md (2)
79-96
: Enhance the configuration example with comments and common use cases.The current example could be more educational by including comments and covering common use cases.
-#small deployment with nodeport for local testing or small deployments +# Example: Basic deployment with NodePort +# Use case: Local testing or small deployments + main: config: n8n: + # Disable usage tracking hide_usage_page: true + # Optional: Configure logging + # log_level: debug + secret: n8n: + # Required: Set a secure encryption key for data protection encryption_key: "<your-secure-encryption-key>" + + # Resource limits for the main n8n instance resources: limits: memory: 2048Mi requests: memory: 512Mi + + # Service configuration for accessing n8n service: type: NodePort port: 5678 + +# Optional: Enable worker mode for better scalability +worker: + enabled: false + +# Optional: Enable webhook mode for dedicated webhook processing +webhook: + enabled: false
29-37
: Fix hyphenation in "Kubernetes-specific".For consistency and correctness, add hyphens when using "Kubernetes-specific" as a modifier.
-The `values.yaml` file is divided into a multiple n8n and Kubernetes specific sections. +The `values.yaml` file is divided into a multiple n8n and Kubernetes-specific sections. 1. Global and chart wide values, like the image repository, image tag, etc. 2. Ingress, (default is nginx, but you can change it to your own ingress controller) -3. Main n8n app configuration + Kubernetes specific settings -4. Worker related settings + Kubernetes specific settings -5. Webhook related settings + Kubernetes specific settings +3. Main n8n app configuration + Kubernetes-specific settings +4. Worker related settings + Kubernetes-specific settings +5. Webhook related settings + Kubernetes-specific settings 6. Raw Resources to pass through your own manifests like GatewayAPI, ServiceMonitor etc. -7. Redis related settings + Kubernetes specific settings +7. Redis related settings + Kubernetes-specific settings🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~29-~29: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~35-~35: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~37-~37: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
# Bitnami Valkey configuration | ||
# https://artifacthub.io/packages/helm/bitnami/valkey | ||
redis: | ||
enabled: false | ||
# Other default redis values: https://github.com/bitnami/charts/blob/master/bitnami/redis/values.yaml | ||
``` | ||
architecture: standalone | ||
|
||
# Typical Values Example | ||
primary: | ||
persistence: | ||
enabled: false | ||
existingClaim: "" | ||
size: 2Gi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance Redis configuration documentation.
The Redis configuration section should include more details about requirements, security considerations, and best practices.
# Bitnami Valkey configuration
# https://artifacthub.io/packages/helm/bitnami/valkey
redis:
enabled: false
architecture: standalone
+ # Redis is required when using worker or webhook modes
+ # For production, consider:
+ # - Using a separate Redis instance
+ # - Enabling persistence
+ # - Configuring authentication
+ # - Setting resource limits
+
primary:
persistence:
enabled: false
existingClaim: ""
size: 2Gi
+ # For production, consider:
+ # storageClass: "fast-storage"
+ # accessModes:
+ # - ReadWriteOnce
+
+ # Security settings
+ auth:
+ enabled: true
+ # Generate a strong password for production
+ password: ""
+
+ # Resource limits
+ resources:
+ requests:
+ memory: 256Mi
+ cpu: 100m
+ limits:
+ memory: 1Gi
+ cpu: 500m
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Bitnami Valkey configuration | |
# https://artifacthub.io/packages/helm/bitnami/valkey | |
redis: | |
enabled: false | |
# Other default redis values: https://github.com/bitnami/charts/blob/master/bitnami/redis/values.yaml | |
``` | |
architecture: standalone | |
# Typical Values Example | |
primary: | |
persistence: | |
enabled: false | |
existingClaim: "" | |
size: 2Gi | |
# Bitnami Valkey configuration | |
# https://artifacthub.io/packages/helm/bitnami/valkey | |
redis: | |
enabled: false | |
architecture: standalone | |
# Redis is required when using worker or webhook modes | |
# For production, consider: | |
# - Using a separate Redis instance | |
# - Enabling persistence | |
# - Configuring authentication | |
# - Setting resource limits | |
primary: | |
persistence: | |
enabled: false | |
existingClaim: "" | |
size: 2Gi | |
# For production, consider: | |
# storageClass: "fast-storage" | |
# accessModes: | |
# - ReadWriteOnce | |
# Security settings | |
auth: | |
enabled: true | |
# Generate a strong password for production | |
password: "" | |
# Resource limits | |
resources: | |
requests: | |
memory: 256Mi | |
cpu: 100m | |
limits: | |
memory: 1Gi | |
cpu: 500m |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
754-765
: 🛠️ Refactor suggestionEnhance Redis configuration documentation.
The Redis configuration section would benefit from more detailed guidance on security and production readiness.
# Bitnami Valkey configuration # https://artifacthub.io/packages/helm/bitnami/valkey redis: enabled: false architecture: standalone + # Redis is required for: + # - Queue mode (worker scaling) + # - Webhook mode + # + # Production considerations: + # 1. Enable persistence for data durability + # 2. Configure authentication + # 3. Set appropriate resource limits + # 4. Consider using an external Redis instance for better management primary: persistence: enabled: false existingClaim: "" size: 2Gi
🧹 Nitpick comments (3)
README.md (3)
42-50
: Fix hyphenation in Kubernetes-specific terms.For consistency and correctness, add hyphens when using "Kubernetes-specific" as a modifier.
-The `values.yaml` file is divided into a multiple n8n and Kubernetes specific sections. +The `values.yaml` file is divided into a multiple n8n and Kubernetes-specific sections. -3. Main n8n app configuration + Kubernetes specific settings +3. Main n8n app configuration + Kubernetes-specific settings -4. Worker related settings + Kubernetes specific settings +4. Worker related settings + Kubernetes-specific settings -5. Webhook related settings + Kubernetes specific settings +5. Webhook related settings + Kubernetes-specific settings -7. Redis related settings + Kubernetes specific settings +7. Redis related settings + Kubernetes-specific settings🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~46-~46: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~47-~47: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~48-~48: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~50-~50: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
54-72
: Enhance environment variables documentation.The example shows how configuration is transformed into environment variables, but it would be helpful to:
- Add a note about case sensitivity
- Clarify the nesting behavior
- Document any limitations
These n8n specific settings should be added to `main.config:` or `main.secret:` in the `values.yaml` file. See the [example](#examples) section and other example in the `/examples` directory of this repo. > [!IMPORTANT] -The YAML nodes `config` and `secret` in the values.yaml are transformed 1:1 into ENV variables. +The YAML nodes `config` and `secret` in the values.yaml are transformed 1:1 into ENV variables: +- Keys are converted to uppercase +- Nested keys are joined with underscores +- Values are preserved as-is +- Special characters in keys should be avoided
769-810
: Clarify scaling implications and requirements.The scaling section should better highlight the prerequisites and limitations.
## Scaling and Advanced Configuration Options n8n provides a **queue-mode**, where the workload is shared between multiple instances of the same n8n installation. + +> [!IMPORTANT] +> Prerequisites for scaling: +> 1. Redis server (internal or external) +> 2. Consistent database configuration across instances +> 3. Shared file storage if using filesystem-based features +> +> Limitations: +> - The main instance remains a single point of failure +> - Some features may require additional configuration when scaled🧰 Tools
🪛 LanguageTool
[formatting] ~774-~774: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... instances and limited high availability, because the controller instance remains as Sing...(COMMA_BEFORE_BECAUSE)
[uncategorized] ~806-~806: Unless this sentence is a question, use ‘it is’ instead of ‘is it’.
Context: ...ver is spawned. At last scaling option is it possible to create dedicated webhook in...(IS_IT_WORD_ORDER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
🪛 LanguageTool
README.md
[uncategorized] ~42-~42: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~46-~46: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~47-~47: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~48-~48: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~50-~50: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
🔇 Additional comments (1)
README.md (1)
1-20
: LGTM! The warning and migration guide are well-structured.The warning about breaking changes and migration guide are clear and informative, matching the PR objectives.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
8-10
: Enhance the warning message with Redis requirements.The warning about breaking changes is good, but consider adding a note about Redis requirements since it's a critical dependency for worker and webhook modes.
> [!WARNING] > Version 1.0.0 of this Chart includes breaking changes and is not backwards compatible with previous versions. +> Redis is now required when using worker or webhook modes. > Please review the migration guide below before upgrading.
28-36
: Fix hyphenation in Kubernetes-specific terms.For better readability and consistency, add hyphens when using "Kubernetes-specific" as a modifier.
-The `values.yaml` file is divided into a multiple n8n and Kubernetes specific sections. +The `values.yaml` file is divided into a multiple n8n and Kubernetes-specific sections. 1. Global and chart wide values, like the image repository, image tag, etc. 2. Ingress, (default is nginx, but you can change it to your own ingress controller) -3. Main n8n app configuration + Kubernetes specific settings -4. Worker related settings + Kubernetes specific settings -5. Webhook related settings + Kubernetes specific settings +3. Main n8n app configuration + Kubernetes-specific settings +4. Worker related settings + Kubernetes-specific settings +5. Webhook related settings + Kubernetes-specific settings 6. Raw Resources to pass through your own manifests like GatewayAPI, ServiceMonitor etc. -7. Redis related settings + Kubernetes specific settings +7. Redis related settings + Kubernetes-specific settings🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...(SPECIFIC_HYPHEN)
[uncategorized] ~32-~32: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...(SPECIFIC_HYPHEN)
[uncategorized] ~36-~36: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...(SPECIFIC_HYPHEN)
754-761
: Expand the migration guide with more details.The migration guide would be more helpful with specific examples of value changes and their impact.
## Migration Guide to Version 1.0.0 This version includes a complete redesign of the chart to better accommodate n8n configuration options. Key changes include: - Values restructured under `.Values.main`, `.Values.worker`, and `.Values.webhook` - Updated deployment configurations - New Redis integration requirements + +### Value Changes Examples + +Before: +```yaml +n8n: + encryption_key: "secret" +``` + +After: +```yaml +main: + secret: + n8n: + encryption_key: "secret" +``` + +### Impact on Existing Deployments +- All existing values need to be moved under the new structure +- Worker configurations require Redis +- Webhook configurations require Redis
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~28-~28: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...file is divided into a multiple n8n and Kubernetes specific sections. 1. Global and chart wide val...
(SPECIFIC_HYPHEN)
[uncategorized] ~32-~32: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...roller) 3. Main n8n app configuration + Kubernetes specific settings 4. Worker related settings + K...
(SPECIFIC_HYPHEN)
[uncategorized] ~33-~33: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...c settings 4. Worker related settings + Kubernetes specific settings 5. Webhook related settings + ...
(SPECIFIC_HYPHEN)
[uncategorized] ~34-~34: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... settings 5. Webhook related settings + Kubernetes specific settings 6. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~36-~36: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 7. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
🪛 markdownlint-cli2 (0.17.2)
README.md
7-7: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
🔇 Additional comments (1)
README.md (1)
742-751
: Enhance Redis security configuration examples.The Redis configuration section could benefit from additional security best practices and production recommendations.
redis: enabled: false architecture: standalone + # For production environments: + auth: + enabled: true + # Use a strong password and store it securely + password: "" + + # Enable TLS for secure communication + tls: + enabled: false + # Configure certificates for production + certificatesSecret: "" + primary: persistence: enabled: false existingClaim: "" size: 2Gi + # For production: + # - Use a storage class with data replication + # - Consider backup strategies + # storageClass: "managed-premium"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
charts/n8n/values.yaml (1)
646-655
: 🛠️ Refactor suggestionRedis configuration needs security enhancements.
The Redis configuration is minimal and lacks security settings. Consider adding:
- Authentication configuration
- TLS settings
- Network policies
redis: enabled: false architecture: standalone + auth: + enabled: true + password: "" # Required when auth.enabled is true + tls: + enabled: false + # certificatesSecret: "" + # certFilename: "" + # certKeyFilename: "" + # certCAFilename: "" + networkPolicy: + enabled: true + allowExternal: false
🧹 Nitpick comments (1)
charts/n8n/values.yaml (1)
611-643
: Consider adding validation for custom resources.The additional resources section allows arbitrary Kubernetes manifests. While this provides flexibility, it's important to validate these resources to prevent potential security risks or misconfigurations.
Consider adding validation through admission controllers or OPA policies to ensure custom resources follow security best practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/n8n/values.yaml
(2 hunks)
🔇 Additional comments (4)
charts/n8n/values.yaml (4)
1-10
: Great addition of a high-level README section!The new README section provides a clear overview of the values.yaml structure, making it easier for users to understand and navigate the configuration options.
152-156
: Good security practices with non-root user configuration!The podSecurityContext configuration across main, worker, and webhook components follows security best practices by:
- Running as non-root user (1000)
- Setting appropriate group and fsGroup
- Explicitly enabling runAsNonRoot
Also applies to: 326-330, 508-512
361-362
: Security concern in sample commands.The sample commands include operations that run n8n as root, which is a security risk.
Also applies to: 543-544
427-431
: Excellent documentation for webhook scaling!The webhook configuration includes clear documentation about its purpose and relationship with Redis, helping users understand the implications of enabling webhook scaling.
# The config: {} dictionary is converted to environmental variables in the ConfigMap. | ||
main: | ||
# See https://docs.n8n.io/hosting/configuration/environment-variables/ for all values. | ||
config: {} | ||
# n8n: | ||
# db: | ||
# type: postgresdb | ||
# postgresdb: | ||
# host: 192.168.0.52 | ||
|
||
# Dictionary for secrets, unlike config:, the values here will end up in the secret file. | ||
# The YAML entry db.postgresdb.password: my_secret is transformed DB_POSTGRESDB_password=bXlfc2VjcmV0 | ||
# See https://docs.n8n.io/hosting/configuration/environment-variables/ | ||
secret: {} | ||
# n8n: | ||
# if you run n8n stateless, you should provide an encryption key here. | ||
# encryption_key: | ||
# | ||
# database: | ||
# postgresdb: | ||
# password: 'big secret' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider adding validation for sensitive configuration.
The config
and secret
dictionaries under the main
section handle sensitive data like database credentials and encryption keys. While the documentation links are helpful, consider adding validation to ensure critical configurations are not missed.
🏁 Script executed:
#!/bin/bash
# Check if any sensitive configuration keys are present in the values file
rg -i "password|secret|key|token|credential" charts/n8n/values.yaml
Length of output: 905
Sensitive Configuration Validation Required
The charts/n8n/values.yaml
file currently has empty dictionaries for both config
and secret
under the main
section. Although sensitive keys (like password
, encryption_key
, and related tokens) appear in commented examples, there isn’t any active validation to enforce that critical configuration values are provided. To prevent misconfigurations or potential security oversights, please consider adding validation logic (for example, using Helm’s required
function) that checks these sensitive settings when they are needed.
- Ensure that if sensitive values are required (e.g., for database credentials or encryption in stateless mode), the chart fails fast if they are missing.
- Validate both the
config
andsecret
entries to guarantee that non-commented values, when provided, meet the expected format and criteria.
This is a redesign and new implementation of the n8n helm chart.
It is not backwards compatible, but should provide more flexibility and customizations.
It addresses the issues discussed in #125
Summary by CodeRabbit
New Features
Refactor
Documentation