feat: add the possibility to set additionalLabels on all the resources#242
feat: add the possibility to set additionalLabels on all the resources#242jhaumont wants to merge 4 commits into8gears:mainfrom
Conversation
WalkthroughAdds a new top-level Helm value Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Helm as Helm CLI
participant Templates as n8n Chart Templates
participant K8s as Kubernetes API
User->>Helm: helm install/upgrade (with values.yaml)
Helm->>Templates: Render templates with .Values
alt .Values.additionalLabels present
note right of Templates#lightblue: toYaml(.Values.additionalLabels)\ninjected at top of labels blocks
Templates-->>Templates: Emit additionalLabels into label helper\nand pod/deployment metadata
else
Templates-->>Templates: Emit existing labels only
end
Templates-->>Helm: Return rendered manifests
Helm->>K8s: Apply manifests
K8s-->>User: Resources created/updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
README.md (1)
122-127: Document reserved label keys to avoid selector breakageUsers could accidentally override required labels. Add an explicit note under additionalLabels about not setting these: app.kubernetes.io/name, app.kubernetes.io/instance, app.kubernetes.io/type, helm.sh/chart, app.kubernetes.io/version, app.kubernetes.io/managed-by.
additionalLabels: {} # tags.datadoghq.com/env: "<ENV>" # tags.datadoghq.com/service: "<SERVICE>" # tags.datadoghq.com/version: "<VERSION>" + # NOTE: Do not override reserved labels used by this chart/selectors: + # - app.kubernetes.io/name + # - app.kubernetes.io/instance + # - app.kubernetes.io/type + # - helm.sh/chart + # - app.kubernetes.io/version + # - app.kubernetes.io/managed-bycharts/n8n/values.yaml (1)
27-32: Add safety note about reserved labels in defaultsMirror the README warning in values.yaml so it’s visible next to the setting.
# Additional labels to be added to all deployed resources by the chart additionalLabels: {} # tags.datadoghq.com/env: "<ENV>" # tags.datadoghq.com/service: "<SERVICE>" # tags.datadoghq.com/version: "<VERSION>" + # NOTE: Do not override reserved labels used by this chart/selectors: + # - app.kubernetes.io/name + # - app.kubernetes.io/instance + # - app.kubernetes.io/type + # - helm.sh/chart + # - app.kubernetes.io/version + # - app.kubernetes.io/managed-bycharts/n8n/Chart.yaml (1)
38-39: Artifact Hub changelog: use “added” and tighten wordingThis is a new capability, not a change.
- - kind: changed - description: "add the possibility to set additionalLabels on all the resources" + - kind: added + description: "Add support for setting additionalLabels on all resources"charts/n8n/templates/deployment.yaml (1)
40-42: Optional hardening for label valuesIf you expect numeric/boolean-like values in additionalLabels, consider quoting values at render time (map iteration with |quote) to avoid YAML implicit typing surprises, or note this in README.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
README.md(1 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/_helpers.tpl(1 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T08:05:39.337Z
Learnt from: tsch157
PR: 8gears/n8n-helm-chart#239
File: charts/n8n/Chart.yaml:3-4
Timestamp: 2025-09-02T08:05:39.337Z
Learning: n8n 1.110.0 is a pre-release version and should not be used in production deployments. Always verify if a version is stable before recommending upgrades in the n8n Helm chart.
Applied to files:
charts/n8n/Chart.yaml
🔇 Additional comments (4)
charts/n8n/Chart.yaml (1)
3-3: Version bump acknowledged; sync install docsChart version is 1.1.0. Please update the install command in README to use --version 1.1.0 so users don’t pin to 1.0.0.
charts/n8n/templates/deployment.webhook.yaml (2)
43-45: Global pod labels: correct placement and override precedenceIncluding additionalLabels in the pod template labels (before webhook-specific podLabels) ensures pod-specific labels can override global ones if keys collide. Good call keeping them out of matchLabels.
43-45: (waiting for script results)charts/n8n/templates/deployment.yaml (1)
40-42: Global pod labels: correct placement and override precedenceBlock is correctly inserted so that main.podLabels can override global additionalLabels on collision; selectors remain unchanged. Looks good.
|
@Vad1mo I take account some feedbacks from coderabbitai |
PhillypHenning
left a comment
PhillypHenning
left a comment
There was a problem hiding this comment.
Left some comments, mainly make the label examples generic and don't perform a minor bump for the chart.
Non-Blocking, I'm not a maintainer, just helping these guys out.
| apiVersion: v2 | ||
| name: n8n | ||
| version: 1.0.14 | ||
| version: 1.1.0 |
There was a problem hiding this comment.
Wouldn't recommend doing a minor version bump for this change. That should be left to the maintainers.
There was a problem hiding this comment.
Not align with this if I follow the contributing guide: https://github.com/8gears/n8n-helm-chart/blob/main/CONTRIBUTING.md#chart-versioning-schema
|
Tested PR locally, I don't see any issues with the resulting kubernetes manifests. 👍🏻 {{- if .Values.additionalLabels }}
{{- toYaml .Values.additionalLabels }}
{{ end -}}This will remove the newline and make the resulting manifests cleaner. Test with: cd charts/n8n
helm template jhaumont-n8n ./ --values ./values.yaml > config.yml |
@PhillypHenning thanks, good catch ! I always lost with the dash in go templating 😄 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
charts/n8n/config.yml (4)
22-28: Render labels with trim markers and nindent to avoid stray newlines/indent drift.Adopt:
{{- with .Values.additionalLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
to keep manifests tidy (aligns with feedback mentioned in the PR).Apply in templates where metadata.labels are emitted (Service, ServiceAccount, Deployment, test Pod).
81-86: Harden container securityContext (allowPrivilegeEscalation=false, drop caps).Container securityContext is currently {}. Recommend secure defaults to satisfy CKV_K8S_20 and common hardening.
Apply in templates and values:
- securityContext: - {} + securityContext: + {{- toYaml .Values.containerSecurityContext | nindent 12 }}In values.yaml:
+containerSecurityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALLAlso applies to: 101-103
125-134: Harden test Pod too (non-root and no privilege escalation).The helm test pod currently has no security context.
Suggested template changes:
spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 + runAsGroup: 1000 containers: - name: wget image: alpine + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"]
1-14: Guard against user overriding selector labels via additionalLabels.Document (and ideally validate) that keys used in selectors (app.kubernetes.io/name, instance, type) must not be set in additionalLabels to avoid mismatches between metadata.labels and selectors.
If you prefer a defensive template approach, render additionalLabels after built‑ins for metadata, but never into selectors; and optionally reject conflicting keys:
- labels: - {{- include "n8n.labels" . | nindent 4 }} - {{- with .Values.additionalLabels }}{{- toYaml . | nindent 4 }}{{- end }} + labels: + {{- include "n8n.labels" . | nindent 4 }} + {{- with .Values.additionalLabels }} + {{- $_ := unset (dict) "app.kubernetes.io/name" -}} + {{- $_ := unset $ "app.kubernetes.io/instance" -}} + {{- $_ := unset $ "app.kubernetes.io/type" -}} + {{- toYaml . | nindent 4 }} + {{- end }}Alternatively, note the constraint in README.
Also applies to: 20-28, 47-54, 65-72, 115-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)charts/n8n/config.yml(1 hunks)charts/n8n/templates/_helpers.tpl(1 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- README.md
- charts/n8n/templates/deployment.worker.yaml
- charts/n8n/values.yaml
- charts/n8n/templates/deployment.yaml
- charts/n8n/templates/_helpers.tpl
- charts/n8n/templates/deployment.webhook.yaml
🧰 Additional context used
🪛 Checkov (3.2.334)
charts/n8n/config.yml
[medium] 42-109: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 111-134: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 111-134: Minimize the admission of root containers
(CKV_K8S_23)
🔇 Additional comments (4)
charts/n8n/config.yml (4)
8-14: Global additionalLabels are present on ServiceAccount metadata. LGTM.The rendered labels include the custom label (test: test). This confirms the chart-level propagation works for this resource.
29-39: Selector stability check.Service and Deployment selectors look unchanged and consistent with pod labels. Good for upgrades/rollouts.
Also applies to: 59-63
47-54: Chart.yaml updated to 1.1.0; README contains no matching referencescharts/n8n/Chart.yaml has "version: 1.1.0" (line 3). No occurrences of "n8n-1.1.0" or "version.1.1.0" found in README — update README to reference v1.1.0 if intended.
63-72: Action: rerun templating check locally (helm missing in earlier run)Good: additionalLabels are on the Pod template while the Deployment selector remains unchanged — avoids selector drift. The previous run failed because helm was not found; re-run one of:
helm template test charts/n8n --set additionalLabels.foo=bar | rg -nPU '(?s)selector:\s*\n.\bfoo:\sbar\b' || echo "OK: no additionalLabels in selectors"
If helm is unavailable, run a source check:
rg -n 'additionalLabels' charts/n8n || echo "No additionalLabels occurrences"
rg -nPU '(?s)selector:\s*\n.*\badditionalLabels\b' charts/n8n || echo "OK: no additionalLabels in selectors"
|
🖖 @Vad1mo who can I ping to have a second approval? 🙏 |
|
@Vad1mo Is there any way we could approve workflow runs? I don't have access |
4 participants
We deploy n8n with this chart in a company.
Because of internal policy, we need to add some labels on all the resources. It's only possible on pod & deployment by default.
This PR add the possibility to set custom labels globally.
Summary by CodeRabbit
New Features
Documentation
Chores