Add reverse proxy helm chart - #32
Conversation
9546fee to
17830b8
Compare
17830b8 to
2458171
Compare
|
Hello when do you think the chart will be ready to deploy ? |
jackylamhk
left a comment
There was a problem hiding this comment.
We should probably add a secret mount for static TLS certs. As the certs mount is always present, users cannot currently mount it via extraVolumes and extraVolumeMounts.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces a complete Helm chart for deploying netbird-proxy to Kubernetes. It defines chart metadata, configuration values, reusable helpers, workload and service templates, optional security and storage resources, autoscaling, monitoring, network policies, and post-deployment instructions. ChangesNetBird Proxy Helm Chart
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant Kubernetes
participant ProxyPod
participant Service
participant HealthMonitor
HelmValues->>Kubernetes: Render Deployment and optional resources
Kubernetes->>ProxyPod: Start configured netbird-proxy container
Service->>ProxyPod: Route HTTPS, health, and optional listener traffic
HealthMonitor->>Service: Scrape configured health endpoint
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/netbird-proxy/templates/hpa.yaml`:
- Around line 15-31: The HPA manifest renders a bare "metrics:" header when
neither .Values.autoscaling.targetCPUUtilizationPercentage nor
.Values.autoscaling.targetMemoryUtilizationPercentage is set, producing invalid
YAML; update the template to conditionally emit the entire metrics block only
when at least one target is configured by wrapping the existing metrics: section
and its two conditional resource entries inside a single outer conditional that
checks .Values.autoscaling.targetCPUUtilizationPercentage or
.Values.autoscaling.targetMemoryUtilizationPercentage so that spec.metrics is
omitted entirely when both are unset.
In `@charts/netbird-proxy/templates/pdb.yaml`:
- Around line 12-17: Change the PDB template to treat explicit 0 as a valid
value and prevent both fields from being rendered together: replace the current
truthy checks with nil-checks against .Values.podDisruptionBudget.minAvailable
and .Values.podDisruptionBudget.maxUnavailable (use the Helm "ne" nil test) so 0
is allowed, and add a mutual-exclusivity validation (e.g., a short template that
fails with a clear message if both .Values.podDisruptionBudget.minAvailable and
.Values.podDisruptionBudget.maxUnavailable are non-nil) to ensure only one of
minAvailable or maxUnavailable is rendered.
In `@charts/netbird-proxy/templates/role.yaml`:
- Line 1: Update the template conditional so Role creation is allowed when RBAC
is enabled and the ACME lock method is k8s-lease even if an existing
ServiceAccount is reused; replace the current check that requires
.Values.serviceAccount.create with a compound condition that requires
.Values.rbac.create and (the ACME lock method equals "k8s-lease") and (either
.Values.serviceAccount.create is true OR a user-supplied ServiceAccount name is
present), and apply the same conditional change in the corresponding RoleBinding
template.
In `@charts/netbird-proxy/templates/secret.yaml`:
- Around line 1-11: When creating the Secret resource guarded by
.Values.existingSecret, ensure we fail-fast if the proxy token is missing:
inside the template that renders the Secret (the block that uses {{- if not
.Values.existingSecret }} and creates the Secret with token: {{
.Values.proxyToken | b64enc }}), add a Helm required check such as {{ required
"proxyToken must be set when existingSecret is not provided" .Values.proxyToken
}} before using .Values.proxyToken (or change the outer condition to require
both not .Values.existingSecret and .Values.proxyToken) so the chart render
fails clearly instead of emitting an empty token Secret.
In `@charts/netbird-proxy/templates/service.yaml`:
- Around line 37-42: The Service currently exposes the acme-http port whenever
.Values.acme.enabled is true; change the Helm conditional to only render the
acme-http port when the ACME challenge type is http-01 (e.g. replace or extend
the guard to check .Values.acme.challengeType == "http-01") so the named port
"acme-http" and port value .Values.service.acmePort are only emitted for
http-01; apply the identical conditional change in the other templates (the
Deployment containerPort entry named "acme-http" and any related port
definitions in networkpolicy.yaml) so all three templates (service, deployment,
networkpolicy) only expose/allow the acme-http port when
.Values.acme.challengeType == "http-01" and omit it otherwise.
In `@charts/netbird-proxy/values.yaml`:
- Around line 3-6: The image repository value is wrong: update the
image.repository entry (currently "netbird-proxy") to the correct official
Docker Hub image "netbirdio/reverse-proxy" so Helm will pull the correct image
for deployment; edit the image block (the repository key) in values.yaml to
replace netbird-proxy with netbirdio/reverse-proxy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 615f5362-7919-47b5-8636-a9e69e1a46e3
📒 Files selected for processing (15)
charts/netbird-proxy/Chart.yamlcharts/netbird-proxy/templates/NOTES.txtcharts/netbird-proxy/templates/_helpers.tplcharts/netbird-proxy/templates/deployment.yamlcharts/netbird-proxy/templates/hpa.yamlcharts/netbird-proxy/templates/networkpolicy.yamlcharts/netbird-proxy/templates/pdb.yamlcharts/netbird-proxy/templates/pvc.yamlcharts/netbird-proxy/templates/role.yamlcharts/netbird-proxy/templates/rolebinding.yamlcharts/netbird-proxy/templates/secret.yamlcharts/netbird-proxy/templates/service.yamlcharts/netbird-proxy/templates/serviceaccount.yamlcharts/netbird-proxy/templates/servicemonitor.yamlcharts/netbird-proxy/values.yaml
- Add certVolume.existingSecret to mount a TLS Secret (cert-manager) - Fix image repository to netbirdio/reverse-proxy - Gate ACME HTTP port on http-01 challenge in service, deployment, networkpolicy - Require proxyToken when existingSecret is not provided - Allow Role/RoleBinding when serviceAccount.name is set without create - Treat PDB minAvailable/maxUnavailable 0 as valid and fail on mutual exclusivity - Omit HPA metrics block when no targets are set - Document crowdsec.existingSecret requirement when top-level existingSecret is used
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/netbird-proxy/templates/pvc.yaml (1)
11-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQuote the rendered
storageClassNamevalues.An unset value is handled correctly, but a conventional
"-"value renders as invalid YAML (storageClassName: -). Apply| quoteto both PVC templates.Also applies to: 29-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/netbird-proxy/templates/pvc.yaml` around lines 11 - 12, Update the storageClassName fields in both PVC templates, including the block guarded by certVolume.storageClass, to render the configured value through the Helm quote function. Preserve the existing conditional behavior for unset values while ensuring conventional "-" values produce valid quoted YAML.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@charts/netbird-proxy/templates/pvc.yaml`:
- Around line 11-12: Update the storageClassName fields in both PVC templates,
including the block guarded by certVolume.storageClass, to render the configured
value through the Helm quote function. Preserve the existing conditional
behavior for unset values while ensuring conventional "-" values produce valid
quoted YAML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f13fce84-9dd7-439a-b12b-b8c49da15b68
📒 Files selected for processing (10)
charts/netbird-proxy/templates/deployment.yamlcharts/netbird-proxy/templates/hpa.yamlcharts/netbird-proxy/templates/networkpolicy.yamlcharts/netbird-proxy/templates/pdb.yamlcharts/netbird-proxy/templates/pvc.yamlcharts/netbird-proxy/templates/role.yamlcharts/netbird-proxy/templates/rolebinding.yamlcharts/netbird-proxy/templates/secret.yamlcharts/netbird-proxy/templates/service.yamlcharts/netbird-proxy/values.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/netbird-proxy/templates/pdb.yaml
- charts/netbird-proxy/templates/networkpolicy.yaml
- charts/netbird-proxy/templates/service.yaml
- charts/netbird-proxy/templates/hpa.yaml
- charts/netbird-proxy/templates/secret.yaml
- charts/netbird-proxy/templates/deployment.yaml
Summary
Adds a Helm chart for deploying the NetBird reverse proxy.
Kubernetes Resources
serviceAccount.createproxyTokenset (noexistingSecret)certVolume.enabledorgeolocation.volume.enabledrbac.createautoscaling.enabledpodDisruptionBudget.enabledminAvailableormaxUnavailablenetworkPolicy.enabledserviceMonitor.enabledSummary by CodeRabbit
Release Notes