Add sidecar container support to Crossplane Helm chart#7007
Add sidecar container support to Crossplane Helm chart#7007phisco merged 1 commit intocrossplane:mainfrom
Conversation
Add sidecarsCrossplane values option to allow users to specify additional sidecar containers in the Crossplane deployment pod. Signed-off-by: Philippe Scorsolini <p.scorsolini@gmail.com>
📝 WalkthroughWalkthroughAdds Helm chart support for injecting sidecar containers into the Crossplane pod (new Changes
Sequence Diagram(s)sequenceDiagram
actor Controller
participant FunctionRunner
participant PipelineInspectorSidecar
participant Logs
Controller->>FunctionRunner: invoke function (RunFunctionRequest)
FunctionRunner->>PipelineInspectorSidecar: EmitStepRequest (gRPC) [sanitized + meta]
PipelineInspectorSidecar->>Logs: persist/log JSON
PipelineInspectorSidecar-->>FunctionRunner: EmitStepResponse (ack)
FunctionRunner-->>Controller: return function result (continue reconciliation)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Thanks for the changes — do you want a focused review on the security (credential-stripping) and message-size safeguards next? 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cluster/charts/crossplane/templates/deployment.yaml (1)
242-244: Template placement looks correct—nice work!The sidecar containers are correctly injected at the
containers:list level with proper indentation (nindent 6). Thewithblock ensures nothing renders whensidecarsCrossplaneis empty.One thought: since users can inject arbitrary container specifications (including potentially privileged containers), would it be helpful to add a note in the README or values.yaml comment warning users to apply appropriate security contexts to their sidecars? This is optional, but could help users avoid accidentally weakening pod security. What do you think?
cluster/charts/crossplane/README.md (1)
141-141: Documentation entry looks great!The table entry is well-formatted, alphabetically placed, and stays within the 100-column guideline. The description is clear and consistent with the comment in
values.yaml.Could you consider adding a brief example in the README (perhaps in the "Settings File" section) showing how to configure a sidecar? For instance, a simple logging sidecar example might help users get started. This is entirely optional—the current documentation is sufficient for experienced Helm users.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cluster/charts/crossplane/README.mdcluster/charts/crossplane/templates/deployment.yamlcluster/charts/crossplane/values.yaml
🧰 Additional context used
📓 Path-based instructions (2)
**/cluster/**
⚙️ CodeRabbit configuration file
**/cluster/**: Review Kubernetes manifests for security, resource limits, and proper
RBAC. Ensure Helm chart follows best practices. Consider upgrade and
rollback scenarios.
Files:
cluster/charts/crossplane/values.yamlcluster/charts/crossplane/README.mdcluster/charts/crossplane/templates/deployment.yaml
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Ensure Markdown files are wrapped at 100 columns for consistency and
readability. Lines can be longer if it makes links more readable, but
otherwise should wrap at 100 characters. Check for proper heading
structure, clear language, and that documentation is helpful for users.
Files:
cluster/charts/crossplane/README.md
🧠 Learnings (1)
📚 Learning: 2025-10-10T19:15:32.438Z
Learnt from: negz
Repo: crossplane/crossplane PR: 6844
File: cluster/crds/pkg.crossplane.io_deploymentruntimeconfigs.yaml:843-845
Timestamp: 2025-10-10T19:15:32.438Z
Learning: CRDs under cluster/crds/*.yaml are generated from upstream Kubernetes API types via controller-gen (see controller-gen.kubebuilder.io/version annotation). Descriptive text in these files mirrors upstream and should not be edited locally; fixes must be made upstream and pulled in via k8s.io module updates.
Applied to files:
cluster/charts/crossplane/README.md
🔍 Remote MCP
Let me search for relevant information about Crossplane and Helm sidecar patterns using web search:## Summary of Relevant Context for PR Review
Crossplane Project Context
Crossplane is an open source Kubernetes add-on that enables platform teams to assemble infrastructure from multiple vendors, and expose higher level self-service APIs for application teams to consume. Crossplane supports customizations at install time by configuring the Helm chart, and documentation is available in the Helm chart README.
Sidecar Use Case in Crossplane
Within the Crossplane ecosystem, sidecars have a specific architectural purpose: Functions run directly on the gRPC server, which, for security reasons, must run as a sidecar to the Crossplane pod. This indicates that sidecar container support is a recognized requirement for Crossplane's composition functions feature.
Helm Chart Sidecar Implementation Patterns
Other Helm charts implement sidecar container support through an extraContainers parameter, allowing users to define different containers for various pod roles. This is consistent with common industry practice for Helm chart extensibility.
Common Sidecar Use Cases
Sidecar containers can add functionality such as monitoring, logging, or proxying by abstracting features away from the main application, allowing the main container to focus on what it does best. Common use cases include logging and monitoring, where a sidecar container can collect and forward logs from the main application container, abstracting the logging infrastructure away from the main application.
Key Review Considerations
Implementation Completeness: The PR appears to be in draft form—the contributor's checklist indicates that unit tests, e2e tests, and documentation updates are unchecked.
Template Structure: The deployment template changes add a templated block for sidecar volume mounts in the pod spec, which allows users to inject sidecar-related configuration dynamically.
Configuration Flexibility: The implementation uses a values array (sidecarsCrossplane: []), providing users with flexibility to define custom sidecars while maintaining a clean, empty default.
[::web_search::] for Kubernetes sidecar container patterns and Crossplane-specific sidecar requirements.
🔇 Additional comments (1)
cluster/charts/crossplane/values.yaml (1)
218-220: LGTM! Clean addition following existing conventions.The new
sidecarsCrossplanevalue follows the established naming pattern (e.g.,resourcesCrossplane,extraVolumeMountsCrossplane) and uses an empty array default, which is safe and consistent with similar array-typed values in this file.Thanks for keeping the helm-docs comment format consistent!
5095207 to
91db4e3
Compare
ezgidemirel
left a comment
There was a problem hiding this comment.
looks good @phisco , thanks!
|
Yes @jbw976, full container spec can be injected. |
Description of your changes
Add sidecarsCrossplane values option to allow users to specify additional sidecar containers in the Crossplane deployment pod.
I have:
earthly +reviewableto ensure this PR is ready for review.Added or updated unit tests.Added or updated e2e tests.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Followed the API promotion workflow if this PR introduces, removes, or promotes an API.Need help with this checklist? See the cheat sheet.