Skip to content
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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Next Gen Chart #132

wants to merge 8 commits into from

Conversation

Vad1mo
Copy link
Member

@Vad1mo Vad1mo commented Jan 10, 2025

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

    • Introduced a modern configuration structure separating core, worker, and webhook components.
    • Added enhanced handling for nested environment variables.
  • Refactor & Updates

    • Major Helm chart upgrade (v1.0.0) with a complete restructuring of service, deployment, persistence, autoscaling, and secret configurations.
    • Updated templates to align with the latest Kubernetes API standards.
  • Documentation

    • Redesigned README and example files for improved clarity and easier deployment guidance.

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Add 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 suggestion

Add 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 suggestion

Add 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 issue

Fix 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 suggestion

Enhance 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 and maxUnavailable 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2cd5187 and 0a2a2a0.

📒 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 like N8N_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/templates

Length 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

charts/n8n/values.yaml Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a 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 paths

The 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 issue

Fix 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 suggestion

Enhance 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 issue

Fix 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 Arguments

Also 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:

  1. Additional documentation about Redis usage in n8n
  2. Security-related settings like password protection
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0a2a2a0 and dc638a4.

📒 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 files

The ConfigMap name change is incomplete and will cause issues.

charts/n8n/Chart.yaml (1)

3-4: Version bump aligns with breaking changes

The 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 key

The 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 configuration

The 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 injection

The switch to envFrom with configMapRef and secretRef is a better approach for managing environment variables.


91-94: Enhanced volume mount flexibility

The 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.

charts/n8n/templates/configmap.yaml Show resolved Hide resolved
Comment on lines +28 to 31
- name: valkey
version: 2.2.3
repository: oci://registry-1.docker.io/bitnamicharts
condition: redis.enabled
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

⚠️ Review comment is correct and reveals additional concerns

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 be valkey.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

⚠️ Valkey integration requires additional work

The Redis to Valkey migration has several issues:

  1. The condition redis.enabled should be renamed to valkey.enabled
  2. 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

Copy link

@coderabbitai coderabbitai bot left a 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 configurable

The 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 configuration

While 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 configuration

Consider 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 production

A single instance PostgreSQL setup (instances: 1) might not be suitable for production use.

Consider:

  1. Increasing the number of instances for high availability
  2. Implementing proper backup strategies
  3. Setting up read replicas if needed
-      instances: 1
+      instances: 3
charts/n8n/templates/_helpers.tpl (1)

78-94: Enhance the toEnvVars function for better robustness.

The function handles maps and scalar values well, but could be improved in the following areas:

  1. Add support for array values
  2. Add error handling for invalid values
  3. 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:

  1. Resource allocation recommendations
  2. Scaling thresholds
  3. 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:

  1. Data recovery procedures
  2. Backup strategies
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between dc638a4 and 0dbe9a3.

📒 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 issue

Replace placeholder encryption key

The encryption key contains a placeholder value which is not secure for production use.

Please ensure to:

  1. Generate a secure random encryption key
  2. Store it in a Kubernetes secret
  3. Reference it using secretKeyRef similar to the database password
-      encryption_key: "<your-secure-encryption-key>"
+      encryption_key:
+        valueFrom:
+          secretKeyRef:
+            name: n8n-secrets
+            key: encryption-key
charts/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:

examples/values_small_prod.yaml Show resolved Hide resolved
examples/values_small_prod.yaml Show resolved Hide resolved
examples/values_small_prod.yaml Show resolved Hide resolved
examples/values_small_prod.yaml Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a 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 suggestion

Enhance 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 issue

Breaking change: ConfigMap name and structure updated

The changes include:

  1. ConfigMap name now prefixed with "app-config-"
  2. Configuration moved from .Values.config to .Values.main.config
  3. 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 issue

Breaking change: Secret name and structure updated

The changes include:

  1. Secret name now prefixed with "app-secret-"
  2. Configuration moved from .Values.secret to .Values.main.secret
  3. Removed N8N_ENCRYPTION_KEY handling
  4. 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 Secure

This 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 the values.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

📥 Commits

Reviewing files that changed from the base of the PR and between 0dbe9a3 and 7892338.

📒 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 updated

The 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 updated

The 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 correctly

The 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 Utilities

The container image for the test pod is now changed to Alpine, and the connection test script has been enhanced to include an nslookup and a wget 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 both nslookup and wget by default.

charts/n8n/templates/pvc.yaml (1)

1-30: Confirm Updated Persistence Settings and Verify YAML Templating

The 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 Hierarchy

The 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 Configuration

The 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 as reject_Unauthorized: true. For consistency with standard naming conventions and previous recommendations, change it to rejectUnauthorized: true.


26-31: Verify Extra Environment Variables Setup.
The extraEnv block correctly maps the secret reference for DB_POSTGRESDB_PASSWORD. Just ensure that the referenced secret (db-app, key password) exists and is up to date.


32-37: Review Volume Mount Configuration.
The configuration under extraVolumeMounts (and the corresponding extraVolumes 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 under resources 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 using apiVersion: 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 for maxSurge and maxUnavailable. 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 the tpl 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.
Using get .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 for maxSurge and maxUnavailable helps in maintaining a predictable rollout process.


22-23: Webhook Selector Label Specification.
Labeling the pods with app.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 the tpl 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 from get .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.
The envFrom 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 for maxSurge and maxUnavailable. This setup supports controlled rollouts.


18-22: Set Master Pod Selector and Labels.
Labels such as app.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 using envFrom, 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.
The main 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.
The replicaCount 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 the resources 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 (from 0.25.2 to 1.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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant