-
Notifications
You must be signed in to change notification settings - Fork 112
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
base: main
Are you sure you want to change the base?
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)
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 & Updates
Documentation