Skip to content

add extraContainers support#180

Open
ArnaudTA wants to merge 1 commit into8gears:mainfrom
ArnaudTA:tech/support-extra-containers
Open

add extraContainers support#180
ArnaudTA wants to merge 1 commit into8gears:mainfrom
ArnaudTA:tech/support-extra-containers

Conversation

@ArnaudTA
Copy link
Copy Markdown

@ArnaudTA ArnaudTA commented Apr 23, 2025

basic support for extraContainers support.

Totally inspired by the Vault chart:
https://artifacthub.io/packages/helm/hashicorp/vault

Have a good day :)

Summary by CodeRabbit

  • New Features

    • Support for adding extra (sidecar) containers to main, worker, and webhook deployments via Helm values.
    • Allows injecting custom containers alongside primary containers to extend functionality (e.g., logging, metrics, proxies).
  • Documentation

    • Helm values updated to document new extraContainers fields and include example sidecar entries for each deployment type.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 23, 2025

Walkthrough

Adds support for injecting user-defined extra containers (sidecars) into main, worker, and webhook Helm deployment templates via new extraContainers fields in values.yaml; templates conditionally render and append the provided YAML into each pod's containers list when defined.

Changes

Cohort / File(s) Change Summary
Main deployment
charts/n8n/templates/deployment.yaml
Conditionally render and append main.extraContainers into the pod containers list using tpl(toYaml(...)) when defined.
Worker deployment
charts/n8n/templates/deployment.worker.yaml
Conditionally render and append worker.extraContainers into the pod containers list using tpl(toYaml(...)) when defined.
Webhook deployment
charts/n8n/templates/deployment.webhook.yaml
Conditionally render and append webhook.extraContainers into the pod containers list using tpl(toYaml(...)) when defined.
Values
charts/n8n/values.yaml
Added extraContainers: null entries (with example snippet) under main, worker, and webhook to expose sidecar configuration.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HelmChart
    participant Kubernetes
    User->>HelmChart: Add extraContainers in values.yaml (main/worker/webhook)
    HelmChart->>HelmChart: tpl(toYaml(.Values.*.extraContainers)) -> render YAML snippets
    HelmChart->>Kubernetes: Apply rendered deployments (pods include extra containers)
    Kubernetes-->>User: Pods created with main + extra sidecar containers
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add extraContainers support' directly and clearly describes the main change: adding support for extraContainers to the Helm chart across three deployment templates and values file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@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 (4)
charts/n8n/values.yaml (2)

451-452: Repeat usage example suggestion. The same guidance for documenting main.extraContainers applies here under worker.extraContainers.


639-640: Repeat usage example suggestion. The same guidance for documenting main.extraContainers applies here under webhook.extraContainers.

charts/n8n/templates/deployment.worker.yaml (1)

117-119: Repeat Helm templating suggestion. Wrap the extraContainers rendering in tpl to permit templating inside sidecar definitions, mirroring the initContainers approach.

charts/n8n/templates/deployment.webhook.yaml (1)

121-123: Repeat Helm templating suggestion. Wrap the extraContainers rendering in tpl to permit templating inside sidecar definitions, mirroring the initContainers approach.

🧹 Nitpick comments (1)
charts/n8n/values.yaml (1)

263-264: Add usage example for extraContainers. It would be helpful to include a commented snippet in values.yaml showing how to configure sidecar containers, for example:

# extraContainers:
#   - name: my-sidecar
#     image: busybox:latest
#     command: ["sh", "-c", "echo hello"]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 565fbf7 and e2d877a.

📒 Files selected for processing (4)
  • 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 (3 hunks)

@ArnaudTA ArnaudTA force-pushed the tech/support-extra-containers branch from 9f0c559 to 91739cd Compare April 23, 2025 23:59
@ArnaudTA
Copy link
Copy Markdown
Author

ArnaudTA commented May 5, 2025

anybody here ?

Copy link
Copy Markdown

@devshark devshark left a comment

Choose a reason for hiding this comment

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

I need this to enable cloudsql proxy sidecar

@raivil
Copy link
Copy Markdown

raivil commented Jul 17, 2025

+1 for this feature

Copy link
Copy Markdown

@LeoDiazL LeoDiazL left a comment

Choose a reason for hiding this comment

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

I would recommend adding a note about this in the README, explaining that this would be a container running in the same pod, and that because of that, Kubernetes expects it to stay running.

@cloudwithdan
Copy link
Copy Markdown

Need this feature also, thanks @LeoDiazL !

@obsidian33
Copy link
Copy Markdown

@ArnaudTA are you able to push this along? if not I can pick it back up in a separate PR and reference this one. I really want to get this merged in.

@obsidian33 obsidian33 mentioned this pull request Oct 7, 2025
8 tasks
@obsidian33
Copy link
Copy Markdown

@LeoDiazL I addressed your comments in #252

@ArnaudTA ArnaudTA force-pushed the tech/support-extra-containers branch from e8952e9 to 449d057 Compare January 20, 2026 16:01
With the help of coderabbit <3
@ArnaudTA ArnaudTA force-pushed the tech/support-extra-containers branch from 449d057 to 1a9d033 Compare January 20, 2026 16:03
@ArnaudTA
Copy link
Copy Markdown
Author

Hello, suggestions applied, are you ok to merge ?

@arthurgbranco
Copy link
Copy Markdown

arthurgbranco commented Mar 6, 2026

Can we approve and merge this? Really need this feature.
cc. @Vad1mo

edit: For anyone seeing, for my use case, I was able to resolve my issue by using this:
#252 (review)

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.

7 participants