Repository navigation
chore(metallb): bump to v0.16.1 and rotate host-network port denylist - #2699
Merged
Merged
Conversation
github-actions
Bot
added
size/L
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request upgrades the MetalLB component to v0.16.0, bringing in upstream improvements such as the transition to FRR-K8s as the default BGP backend and the removal of kube-rbac-proxy in favor of native TLS. The changes ensure that cozystack-built images remain compatible with the new health probe requirements and update security policies to reflect the new network listener set. Extensive testing has been introduced to prevent future regressions in chart rendering and network security configurations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpgrade MetalLB to v0.16.0, add frr-k8s subchart and CRDs, refactor Helm templates/values/schemas for TLS and HTTPS metrics, expand RBAC/NetworkPolicy and Rego guards, add Helm unit tests and Makefile ChangesMetalLB v0.15.2 → v0.16.0 upgrade with FRR-K8s as default BGP backend
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates MetalLB to version 0.16.0, migrating the BGP backend to frr-k8s and updating the Cilium network policy to manage new host-network ports. It also introduces unit tests for both components. Feedback identifies that port 7946 (memberlist gossip) uses both TCP and UDP, so the network policy should explicitly use "protocol: ANY" to ensure both are denied, as Cilium defaults to TCP. Test assertions should be updated accordingly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@packages/system/metallb/charts/metallb/charts/crds/templates/crds.yaml`:
- Around line 633-643: The schema for the localASN field is inconsistent: change
the format for localASN from "int32" to "int64" so it can represent the declared
maximum (4294967295) and match sibling fields myASN/peerASN; locate the localASN
property in the CRD template (the block with description "LocalASN allows
advertising...") and update the format value to int64 only.
In `@packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.md`:
- Line 3: Update the stale badges in the CRDs README by replacing the Version
and AppVersion values currently showing "0.0.0" with the actual chart version
"0.0.25" to match the crds Chart.yaml; specifically edit the badge line in
crds/README.md so the Version and AppVersion badges reflect 0.0.25 (ensure both
badges that currently show 0.0.0 are updated).
In `@packages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.json`:
- Around line 307-309: The schema has a typo in the frr section where the
property name is "resources:" (with a trailing colon) instead of "resources", so
values.yaml's resources key won't validate; locate the frr section inside
values.schema.json and rename the key "resources:" to "resources" (matching the
other sections like reloader, frrMetrics, frrStatus) or regenerate the schema
from an upstream Metallb release that contains the fix so the correct
"resources" property is produced.
- Around line 240-249: The schema for serviceMonitor.interval currently
restricts "interval" to integer|null which rejects Prometheus duration strings;
update the "interval" schema (the anyOf for "interval" under serviceMonitor) to
accept string durations (e.g., change the anyOf to allow {"type":"string"} or
{"type":"string","pattern":"^\\d+[smhd]$"} plus null) so values like "30s" or
"1m" validate, and apply this as an upstream chart change or a local schema
patch during the Helm pull rather than committing a manual vendor edit.
In `@packages/system/metallb/charts/metallb/templates/controller.yaml`:
- Around line 74-85: When TLS metrics are enabled
(.Values.tls.controllerMetricsTLSSecret) and annotations-based scraping is
enabled (.Values.prometheus.scrapeAnnotations), add the prometheus.io/scheme:
"https" annotation alongside existing prometheus.io/scrape and
prometheus.io/port annotations in the controller pod template; update the
controller.yaml template logic that emits the scrape annotations to
conditionally emit prometheus.io/scheme when
.Values.tls.controllerMetricsTLSSecret is truthy so Prometheus uses HTTPS for
port 9120.
In `@packages/system/metallb/charts/metallb/values.yaml`:
- Around line 371-373: The override sets frr-k8s.prometheus to null which makes
unguarded accesses to .Values.prometheus.secureMetricsPort in the controller
template fail; either add secureMetricsPort under the frr-k8s.prometheus values
block in the values override, or change the controller template to guard both
usages (the --metrics-bind-address argument and the containerPort field) with
conditionals that check .Values.prometheus and
.Values.prometheus.secureMetricsPort and only render those lines when the value
exists (provide a sensible default or skip the flags/port when absent).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b40c292e-3662-47e8-b00d-65c342b4c5d4
⛔ Files ignored due to path filters (2)
packages/system/metallb/charts/metallb/Chart.lockis excluded by!**/*.lockpackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
packages/system/cilium-networkpolicy/Makefilepackages/system/cilium-networkpolicy/templates/networkpolicy.yamlpackages/system/cilium-networkpolicy/tests/networkpolicy_test.yamlpackages/system/metallb/Makefilepackages/system/metallb/charts/metallb/Chart.yamlpackages/system/metallb/charts/metallb/README.mdpackages/system/metallb/charts/metallb/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/crds/templates/crds.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_bgpsessionstates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrk8sconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrnodestates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txtpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/_helpers.tplpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/controller.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/rbac.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-accounts.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-monitor.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/status-cleaner.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/webhooks.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.jsonpackages/system/metallb/charts/metallb/charts/frr-k8s/values.yamlpackages/system/metallb/charts/metallb/policy/controller.regopackages/system/metallb/charts/metallb/policy/speaker.regopackages/system/metallb/charts/metallb/templates/NOTES.txtpackages/system/metallb/charts/metallb/templates/_helpers.tplpackages/system/metallb/charts/metallb/templates/controller.yamlpackages/system/metallb/charts/metallb/templates/exclude-l2-config.yamlpackages/system/metallb/charts/metallb/templates/networkpolicy.yamlpackages/system/metallb/charts/metallb/templates/rbac.yamlpackages/system/metallb/charts/metallb/templates/servicemonitor.yamlpackages/system/metallb/charts/metallb/templates/speaker.yamlpackages/system/metallb/charts/metallb/values.schema.jsonpackages/system/metallb/charts/metallb/values.yamlpackages/system/metallb/tests/metallb_test.yamlpackages/system/metallb/values.yaml
💤 Files with no reviewable changes (1)
- packages/system/metallb/charts/metallb/templates/_helpers.tpl
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
from
May 20, 2026 22:52
9c46e44 to
686638e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/system/metallb/charts/metallb/templates/speaker.yaml (1)
143-154:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
prometheus.io/scheme: "https"annotation for speaker when TLS is enabled.Similar to the controller, the speaker's scrape annotations (lines 145-150) emit
prometheus.io/scrapeandprometheus.io/portbut notprometheus.io/scheme. With metrics now served over HTTPS, annotation-based Prometheus scrapers will fail. Conditionally emitprometheus.io/scheme: "https".🤖 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 `@packages/system/metallb/charts/metallb/templates/speaker.yaml` around lines 143 - 154, Update the speaker pod annotations block to emit prometheus.io/scheme: "https" when scrape annotations are enabled and speaker TLS is enabled: inside the existing conditional that checks .Values.prometheus.scrapeAnnotations (and alongside where prometheus.io/port is conditionally emitted depending on .Values.speaker.frr.enabled), add a nested conditional that checks the TLS flag (e.g. .Values.speaker.tls.enabled) and, if true, emits prometheus.io/scheme: "https" so annotation-based Prometheus scrapers use HTTPS for speaker metrics.
♻️ Duplicate comments (2)
packages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.json (1)
307-309:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSchema property key typo:
resources:should beresources.This key won’t match
values.yaml(frr.resources), so the intended field is not validated.🤖 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 `@packages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.json` around lines 307 - 309, Fix the typo in the JSON schema by renaming the property key "resources:" to "resources" in values.schema.json so it matches the actual values.yaml field (frr.resources) and will be validated; locate the object currently defined as "resources:" and remove the trailing colon from the key, leaving the rest of the definition (type: "object") unchanged.packages/system/metallb/charts/metallb/templates/controller.yaml (1)
23-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
prometheus.io/scheme: "https"annotation when TLS metrics are enabled.The metrics endpoint is now HTTPS-only (port 9120 with TLS), but the scrape annotations at lines 25-27 only emit
prometheus.io/scrapeandprometheus.io/port. Prometheus defaults to HTTP, so annotation-based scrapers will fail whentls.controllerMetricsTLSSecretis set. Emitprometheus.io/scheme: "https"conditionally.🤖 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 `@packages/system/metallb/charts/metallb/templates/controller.yaml` around lines 23 - 28, The scrape annotations block in templates/controller.yaml only emits prometheus.io/scrape and prometheus.io/port, so add a conditional emission of prometheus.io/scheme: "https" when TLS for controller metrics is enabled; specifically, within the same if that checks .Values.prometheus.scrapeAnnotations (or the outer or .Values.controller.podAnnotations block) detect .Values.tls.controllerMetricsTLSSecret and emit prometheus.io/scheme: "https" alongside prometheus.io/port (referencing .Values.prometheus.metricsPort) so annotation-based scrapers use HTTPS when controller metrics TLS is configured.
🧹 Nitpick comments (1)
packages/system/metallb/charts/metallb/policy/speaker.rego (1)
24-26: 💤 Low valuePre-existing: Error messages reference "controller" instead of "speaker".
Lines 25 and 33 emit messages like
"controller nodeSelector does not include..."and"controller tolerations does not include..."but these rules validate the speaker DaemonSet. This is a pre-existing issue, but since this file is being touched, consider correcting the messages.🔧 Suggested fix
deny[msg] { input.kind == "DaemonSet" endswith(input.metadata.name, "-speaker") not input.spec.template.spec.nodeSelector["kubernetes.io/os"] == "linux" - msg = "controller nodeSelector does not include '\"kubernetes.io/os\": linux'" + msg = "speaker nodeSelector does not include '\"kubernetes.io/os\": linux'" } # validate tolerations include the builtins when custom ones are provided deny[msg] { input.kind == "DaemonSet" endswith(input.metadata.name, "-speaker") not input.spec.template.spec.tolerations[0] == { "key": "node-role.kubernetes.io/master", "effect": "NoSchedule", "operator": "Exists" } - msg = "controller tolerations does not include node-role.kubernetes.io/master:NoSchedule" + msg = "speaker tolerations does not include node-role.kubernetes.io/master:NoSchedule" }Also applies to: 30-34
🤖 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 `@packages/system/metallb/charts/metallb/policy/speaker.rego` around lines 24 - 26, The error messages for the speaker DaemonSet are incorrectly referring to "controller"; update the msg strings used in the rules that validate nodeSelector and tolerations so they say "speaker" instead of "controller" (i.e., change the msg assignments tied to the nodeSelector check `not input.spec.template.spec.nodeSelector["kubernetes.io/os"] == "linux"` and the tolerations check that produce the message about tolerations) and ensure the same correction is applied to the other similar msg occurrence in the file.
🤖 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 `@packages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.json`:
- Around line 265-359: The schema defines "crds" in the wrong location (nested
under the prometheus block) which prevents validating the top-level crds key;
move the "crds" object definition out of the prometheus/frrk8s block into the
root-level "properties" alongside "frrk8s" so the top-level "crds" path is
validated, ensuring its "description", "type" and "properties" ("enabled",
"validationFailurePolicy") remain unchanged and update any "required" arrays if
you expect crds to be required.
---
Outside diff comments:
In `@packages/system/metallb/charts/metallb/templates/speaker.yaml`:
- Around line 143-154: Update the speaker pod annotations block to emit
prometheus.io/scheme: "https" when scrape annotations are enabled and speaker
TLS is enabled: inside the existing conditional that checks
.Values.prometheus.scrapeAnnotations (and alongside where prometheus.io/port is
conditionally emitted depending on .Values.speaker.frr.enabled), add a nested
conditional that checks the TLS flag (e.g. .Values.speaker.tls.enabled) and, if
true, emits prometheus.io/scheme: "https" so annotation-based Prometheus
scrapers use HTTPS for speaker metrics.
---
Duplicate comments:
In `@packages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.json`:
- Around line 307-309: Fix the typo in the JSON schema by renaming the property
key "resources:" to "resources" in values.schema.json so it matches the actual
values.yaml field (frr.resources) and will be validated; locate the object
currently defined as "resources:" and remove the trailing colon from the key,
leaving the rest of the definition (type: "object") unchanged.
In `@packages/system/metallb/charts/metallb/templates/controller.yaml`:
- Around line 23-28: The scrape annotations block in templates/controller.yaml
only emits prometheus.io/scrape and prometheus.io/port, so add a conditional
emission of prometheus.io/scheme: "https" when TLS for controller metrics is
enabled; specifically, within the same if that checks
.Values.prometheus.scrapeAnnotations (or the outer or
.Values.controller.podAnnotations block) detect
.Values.tls.controllerMetricsTLSSecret and emit prometheus.io/scheme: "https"
alongside prometheus.io/port (referencing .Values.prometheus.metricsPort) so
annotation-based scrapers use HTTPS when controller metrics TLS is configured.
---
Nitpick comments:
In `@packages/system/metallb/charts/metallb/policy/speaker.rego`:
- Around line 24-26: The error messages for the speaker DaemonSet are
incorrectly referring to "controller"; update the msg strings used in the rules
that validate nodeSelector and tolerations so they say "speaker" instead of
"controller" (i.e., change the msg assignments tied to the nodeSelector check
`not input.spec.template.spec.nodeSelector["kubernetes.io/os"] == "linux"` and
the tolerations check that produce the message about tolerations) and ensure the
same correction is applied to the other similar msg occurrence in the file.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06392d7b-92da-4f7b-bbea-949817af42bd
⛔ Files ignored due to path filters (2)
packages/system/metallb/charts/metallb/Chart.lockis excluded by!**/*.lockpackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
packages/system/cilium-networkpolicy/Makefilepackages/system/cilium-networkpolicy/templates/networkpolicy.yamlpackages/system/cilium-networkpolicy/tests/networkpolicy_test.yamlpackages/system/metallb/Makefilepackages/system/metallb/charts/metallb/Chart.yamlpackages/system/metallb/charts/metallb/README.mdpackages/system/metallb/charts/metallb/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/crds/templates/crds.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_bgpsessionstates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrk8sconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrnodestates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txtpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/_helpers.tplpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/controller.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/rbac.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-accounts.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-monitor.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/status-cleaner.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/webhooks.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.jsonpackages/system/metallb/charts/metallb/charts/frr-k8s/values.yamlpackages/system/metallb/charts/metallb/policy/controller.regopackages/system/metallb/charts/metallb/policy/speaker.regopackages/system/metallb/charts/metallb/templates/NOTES.txtpackages/system/metallb/charts/metallb/templates/_helpers.tplpackages/system/metallb/charts/metallb/templates/controller.yamlpackages/system/metallb/charts/metallb/templates/exclude-l2-config.yamlpackages/system/metallb/charts/metallb/templates/networkpolicy.yamlpackages/system/metallb/charts/metallb/templates/rbac.yamlpackages/system/metallb/charts/metallb/templates/servicemonitor.yamlpackages/system/metallb/charts/metallb/templates/speaker.yamlpackages/system/metallb/charts/metallb/values.schema.jsonpackages/system/metallb/charts/metallb/values.yamlpackages/system/metallb/tests/metallb_test.yamlpackages/system/metallb/values.yaml
💤 Files with no reviewable changes (1)
- packages/system/metallb/charts/metallb/templates/_helpers.tpl
✅ Files skipped from review due to trivial changes (6)
- packages/system/metallb/charts/metallb/charts/frr-k8s/.helmignore
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.md
- packages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yaml
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignore
- packages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txt
- packages/system/metallb/charts/metallb/templates/NOTES.txt
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
from
May 20, 2026 23:33
686638e to
33232bd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/system/metallb/charts/metallb/templates/networkpolicy.yaml`:
- Around line 2-13: The vendored template creates a namespaced default-deny
NetworkPolicy when .Values.networkpolicies.defaultDeny is true but there are no
allow rules for the new frr-k8s / status-cleaner pods, which would strand those
workloads; for this bump do not enable defaultDeny in Cozystack: revert or
change the consumer/defaults so .Values.networkpolicies.defaultDeny is false (or
add a validation check preventing it from being set true) and add a note to
track opening an upstream fix rather than patching the vendored template
(references: .Values.networkpolicies.defaultDeny, NetworkPolicy name
default-deny, podSelector).
In `@packages/system/metallb/charts/metallb/templates/speaker.yaml`:
- Around line 281-283: The Helm template currently uses "if
.Values.speaker.bgpDebounceTimeout" which treats 0 as false and drops a valid
user-provided 0; change the guard to detect presence of the key instead (e.g.,
use hasKey/.Values map existence checks or an explicit nil comparison on
.Values.speaker.bgpDebounceTimeout) so that a numeric 0 is honored and the flag
"--bgp-debounce-timeout={{ .Values.speaker.bgpDebounceTimeout }}" is rendered
when the value is set; update the template around the conditional that
references .Values.speaker.bgpDebounceTimeout (speaker.yaml template) and apply
this change upstream in the vendored chart.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 923b1190-a5da-4348-9710-8ea7a7d3ecdc
⛔ Files ignored due to path filters (2)
packages/system/metallb/charts/metallb/Chart.lockis excluded by!**/*.lockpackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
packages/system/cilium-networkpolicy/Makefilepackages/system/cilium-networkpolicy/templates/networkpolicy.yamlpackages/system/cilium-networkpolicy/tests/networkpolicy_test.yamlpackages/system/metallb/Makefilepackages/system/metallb/charts/metallb/Chart.yamlpackages/system/metallb/charts/metallb/README.mdpackages/system/metallb/charts/metallb/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/crds/templates/crds.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_bgpsessionstates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrk8sconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrnodestates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txtpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/_helpers.tplpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/controller.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/rbac.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-accounts.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-monitor.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/status-cleaner.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/webhooks.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.jsonpackages/system/metallb/charts/metallb/charts/frr-k8s/values.yamlpackages/system/metallb/charts/metallb/policy/controller.regopackages/system/metallb/charts/metallb/policy/speaker.regopackages/system/metallb/charts/metallb/templates/NOTES.txtpackages/system/metallb/charts/metallb/templates/_helpers.tplpackages/system/metallb/charts/metallb/templates/controller.yamlpackages/system/metallb/charts/metallb/templates/exclude-l2-config.yamlpackages/system/metallb/charts/metallb/templates/networkpolicy.yamlpackages/system/metallb/charts/metallb/templates/rbac.yamlpackages/system/metallb/charts/metallb/templates/servicemonitor.yamlpackages/system/metallb/charts/metallb/templates/speaker.yamlpackages/system/metallb/charts/metallb/values.schema.jsonpackages/system/metallb/charts/metallb/values.yamlpackages/system/metallb/tests/metallb_test.yamlpackages/system/metallb/values.yaml
💤 Files with no reviewable changes (1)
- packages/system/metallb/charts/metallb/templates/_helpers.tpl
✅ Files skipped from review due to trivial changes (9)
- packages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txt
- packages/system/metallb/charts/metallb/charts/frr-k8s/.helmignore
- packages/system/metallb/charts/metallb/charts/crds/Chart.yaml
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.md
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignore
- packages/system/metallb/charts/metallb/charts/frr-k8s/values.yaml
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/Chart.yaml
- packages/system/metallb/charts/metallb/README.md
- packages/system/metallb/charts/metallb/Chart.yaml
This was referenced May 20, 2026
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
2 times, most recently
from
May 21, 2026 01:07
b79c0ce to
317b580
Compare
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
from
May 21, 2026 08:31
317b580 to
881969a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/system/metallb/charts/metallb/policy/speaker.rego`:
- Around line 20-34: The deny rules that validate the speaker DaemonSet (the
rule blocks using input.kind == "DaemonSet" and endswith(input.metadata.name,
"-speaker")) have copy-paste error messages that say "controller"; update the
msg strings in both rules to reference "speaker" instead (e.g., change
"controller nodeSelector does not include ..." and "controller tolerations does
not include ..." to say "speaker nodeSelector ..." and "speaker tolerations ..."
respectively) so the messages match the validated resource.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9764c81b-5011-4c86-aa5e-8410510031a9
⛔ Files ignored due to path filters (2)
packages/system/metallb/charts/metallb/Chart.lockis excluded by!**/*.lockpackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
packages/system/cilium-networkpolicy/Makefilepackages/system/cilium-networkpolicy/templates/networkpolicy.yamlpackages/system/cilium-networkpolicy/tests/networkpolicy_test.yamlpackages/system/metallb/Makefilepackages/system/metallb/charts/metallb/Chart.yamlpackages/system/metallb/charts/metallb/README.mdpackages/system/metallb/charts/metallb/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/crds/templates/crds.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignorepackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/Chart.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.mdpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_bgpsessionstates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrk8sconfigurations.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/frrk8s.metallb.io_frrnodestates.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txtpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/_helpers.tplpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/controller.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/rbac.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-accounts.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/service-monitor.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/status-cleaner.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/templates/webhooks.yamlpackages/system/metallb/charts/metallb/charts/frr-k8s/values.schema.jsonpackages/system/metallb/charts/metallb/charts/frr-k8s/values.yamlpackages/system/metallb/charts/metallb/policy/controller.regopackages/system/metallb/charts/metallb/policy/speaker.regopackages/system/metallb/charts/metallb/templates/NOTES.txtpackages/system/metallb/charts/metallb/templates/_helpers.tplpackages/system/metallb/charts/metallb/templates/controller.yamlpackages/system/metallb/charts/metallb/templates/exclude-l2-config.yamlpackages/system/metallb/charts/metallb/templates/networkpolicy.yamlpackages/system/metallb/charts/metallb/templates/rbac.yamlpackages/system/metallb/charts/metallb/templates/servicemonitor.yamlpackages/system/metallb/charts/metallb/templates/speaker.yamlpackages/system/metallb/charts/metallb/values.schema.jsonpackages/system/metallb/charts/metallb/values.yamlpackages/system/metallb/tests/metallb_test.yamlpackages/system/metallb/values.yaml
💤 Files with no reviewable changes (1)
- packages/system/metallb/charts/metallb/templates/_helpers.tpl
✅ Files skipped from review due to trivial changes (9)
- packages/system/metallb/charts/metallb/charts/frr-k8s/templates/NOTES.txt
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/README.md
- packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/.helmignore
- packages/system/metallb/charts/metallb/charts/frr-k8s/Chart.yaml
- packages/system/metallb/charts/metallb/charts/frr-k8s/.helmignore
- packages/system/metallb/charts/metallb/templates/exclude-l2-config.yaml
- packages/system/metallb/charts/metallb/Chart.yaml
- packages/system/metallb/charts/metallb/templates/NOTES.txt
- packages/system/metallb/charts/metallb/README.md
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
from
May 21, 2026 09:46
881969a to
9a1277d
Compare
Contributor
Author
|
Diagnosis from SSH breakpoint: Root cause: vendored Fix in 9a1277d: pin |
Aleksei Sviridkin (lexfrei)
added a commit
to lexfrei/metallb
that referenced
this pull request
May 21, 2026
Since v0.16.0 kube-rbac-proxy is gone and both controller and speaker serve `/metrics` directly over HTTPS on port 9120 (`tls.controllerMetricsTLSSecret` / `tls.speakerMetricsTLSSecret` provide certs; a self-signed cert is auto-generated if neither is set, per the chart README). Prometheus annotation-based scrapers default to HTTP, so omitting `prometheus.io/scheme: "https"` from the controller and speaker pod templates breaks annotation-based scraping — `prometheus.io/scrape: "true"` + `prometheus.io/port: "9120"` alone causes Prometheus to issue HTTP requests against an HTTPS-only endpoint and the scrape fails with a TLS handshake error. Emit `prometheus.io/scheme: "https"` alongside the existing scrape annotations whenever `prometheus.scrapeAnnotations` is enabled. The speaker keeps the existing guard that suppresses both `port` and `scheme` under legacy FRR mode (multi-port pod — a single annotation pair cannot cover both `speaker` and `frr` metrics endpoints; users on FRR mode are expected to use ServiceMonitor). ServiceMonitor users are unaffected (they declare `scheme: https` explicitly in the upstream chart's ServiceMonitor template). Only the annotation-based scraping path needed this fix. Found during downstream consumer review while bumping cozystack to MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context. Signed-off-by: Aleksei Sviridkin <[email protected]>
Aleksei Sviridkin (lexfrei)
added a commit
to lexfrei/metallb
that referenced
this pull request
May 21, 2026
`charts/metallb/values.schema.json` declared
`speaker.bgpDebounceTimeout` as `anyOf: integer minimum 0 | null`, but
the speaker binary rejects 0:
speaker/main.go:134
parsed, err := strconv.Atoi(*bgpDebounceTimeoutMs)
if err != nil || parsed <= 0 {
level.Error(logger).Log("msg", "invalid BGP debounce timeout
value, must be a positive integer value", ...)
os.Exit(1)
}
An operator who set `speaker.bgpDebounceTimeout: 0` in helm values
passed schema validation, but the rendered speaker DaemonSet would
crash on startup with `os.Exit(1)`. (In current upstream, the
template's truthy guard `{{- if .Values.speaker.bgpDebounceTimeout }}`
happens to drop the flag for the falsy value 0, masking the schema
miss with a silent fallback to the 3000 ms default — confusing UX
either way.)
Align the schema with the runtime contract: `minimum: 1`. After this
change, `speaker.bgpDebounceTimeout: 0` is rejected at helm install
with a clear error pointing at the field, instead of crashing the
speaker later or silently ignoring the input.
Verified locally:
- `helm template ... --set speaker.bgpDebounceTimeout=0` is now
rejected with "at '/speaker/bgpDebounceTimeout': minimum: got 0,
want 1".
- `helm template ... --set speaker.bgpDebounceTimeout=1` renders
`--bgp-debounce-timeout=1` (boundary minimum, matches binary's
`parsed > 0` check).
- `helm template ... --set speaker.bgpDebounceTimeout=3000` renders
the flag unchanged.
- Unset (default null) keeps the flag out of the args list.
- Negative values stay rejected.
- `helm lint` clean.
Found during downstream consumer review while bumping cozystack to
MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context.
Signed-off-by: Aleksei Sviridkin <[email protected]>
Aleksei Sviridkin (lexfrei)
added a commit
to lexfrei/metallb
that referenced
this pull request
May 21, 2026
The BGPPeer v1beta2 spec declared LocalASN with the kubebuilder annotation `+kubebuilder:validation:Maximum=4294967295` but no `Format` annotation, so controller-gen emitted `format: int32` in the generated CRD. int32 cannot represent 4,294,967,295 (its maximum is 2,147,483,647), so the schema is internally inconsistent. The sibling fields `ASN` (peerASN) and `MyASN` already carry `+kubebuilder:validation:Format=int64` for the same uint32 type with the same maximum. Align LocalASN with them: a one-line annotation addition + the matching controller-gen regeneration of the CRDs and the all-in-one manifests. Found during downstream consumer review while bumping cozystack to MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context. Signed-off-by: Aleksei Sviridkin <[email protected]>
2 of 5 tasks
Contributor
Author
|
Tracking issue for the upstream MetalLB fixes filed alongside this bump: #2714. |
Federico Paolinelli (fedepaol)
pushed a commit
to metallb/metallb
that referenced
this pull request
May 27, 2026
The BGPPeer v1beta2 spec declared LocalASN with the kubebuilder annotation `+kubebuilder:validation:Maximum=4294967295` but no `Format` annotation, so controller-gen emitted `format: int32` in the generated CRD. int32 cannot represent 4,294,967,295 (its maximum is 2,147,483,647), so the schema is internally inconsistent. The sibling fields `ASN` (peerASN) and `MyASN` already carry `+kubebuilder:validation:Format=int64` for the same uint32 type with the same maximum. Align LocalASN with them: a one-line annotation addition + the matching controller-gen regeneration of the CRDs and the all-in-one manifests. Found during downstream consumer review while bumping cozystack to MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context. Signed-off-by: Aleksei Sviridkin <[email protected]>
Upgrade vendored MetalLB chart from 0.15.2 to 0.16.1 and rebuild the cozystack-built controller / speaker images at the new version (pinned to linux/amd64 in the Makefile to match the prior baseline and the e2e Talos sandbox arch — without the pin, a contributor building on an arm64 host like Apple Silicon ships single-arch arm64 images that crashloop on amd64 nodes with "exec format error"). Adopt the upstream-default BGP backend: FRR-K8s replaces the deprecated FRR mode. Rotate the cilium-networkpolicy clusterwide host-network port denylist to match the new metallb endpoints. The Makefile now vendors the frr-k8s subchart instead of dropping it, and pins the chart version so future make update invocations are deterministic. Upstream highlights between 0.15.2 and 0.16.1: - 0.16.0: FRR-K8s is the new default BGP backend; legacy FRR is deprecated. - 0.16.0: new probe contract — metallb controller and speaker serve /healthz and /readyz on a dedicated host-local health port 17472 (distinct from the metrics port). The frr-k8s controller uses its own host-local probe port 7572. The cozystack-built image must move in lockstep with the chart so probes resolve against the new endpoints. - 0.16.0: kube-rbac-proxy is replaced by native TLS; metrics endpoints are HTTPS-only, served on the formerly-rbac-proxy port 9120. Default TLS minimum is now 1.3 (was 1.2); operators with TLS 1.2 scrapers override via metallb.tls.minVersion: VersionTLS12. - 0.16.0: new tls.* and networkpolicies.* value blocks (opt-in defaults). - 0.16.1: chart-only patch on top of 0.16.0 — the liveness/readiness probe port is now a configurable value (default unchanged at 17472) and the controller probes drop the hardcoded host: 127.0.0.1; pods, ServiceMonitor and PodMonitor gain prometheus.io/scheme: https for the HTTPS metrics endpoint; the frr-k8s subchart gains an opt-in ServiceMonitor toggle (default off); one CRD numeric field widens from int32 to int64. - 0.15.3: ConfigurationState CRD exposes reconcile errors per component; Helm chart gained NetworkPolicy support and tightened pod SecurityContext. Adaptations: - packages/system/metallb/Makefile: pin helm pull --version 0.16.1; remove the rm -rf charts/metallb/charts/frr-k8s line so the new default BGP backend ships with the chart; add a test: target running helm unittest; hardcode --platform=linux/amd64 on the buildx invocations so the image arch does not depend on the contributor's host. - packages/system/metallb/charts/metallb/: regenerated via make update. - packages/system/metallb/values.yaml: cozystack-built controller and speaker image tags moved to v0.16.1 with fresh linux/amd64 digests pushed to ghcr.io/cozystack/cozystack. The chart's new probe contract requires the v0.16.x binary; leaving the digests on v0.15.2 would CrashLoopBackOff every pod on main between merge and the next release-prep commit. A header comment documents the backend default switch, the metrics-port move (7472 HTTP → 9120 HTTPS), the new TLS 1.3 default, the new host-network ports (9140, 9141, 19443), and the partial nature of the FRR-mode escape hatch — flipping metallb.frrk8s.enabled=false plus metallb.speaker.frr.enabled=true switches the speaker but does not suppress the idle frr-k8s subchart, because Helm evaluates a dep's `condition:` against the declaring chart's own values context and a grandparent override from the umbrella cannot reach that resolution. - packages/system/metallb/tests/metallb_test.yaml: new helm-unittest suite pinning post-bump invariants — speaker is the single-container daemonset, BGP_TYPE is frr-k8s, frr-k8s daemonset and CRDs are installed, NetworkPolicies stay off by default, controller/speaker image tags exactly match the chart appVersion (the test that would have caught the chart / binary skew up-front), and the documented FRR-mode escape hatch is pinned in both halves (speaker switches to four-container FRR layout, frr-k8s subchart still renders idle — the latter test is the contract that a future cleanup must update together with the values.yaml note). - packages/system/cilium-networkpolicy/templates/networkpolicy.yaml: drop the stale FRR-mode metrics port 7473; add every metallb v0.16 host-network listener that this policy file is responsible for — 7946 (speaker memberlist gossip, TCP/UDP; cluster peers still reach it via the same policy's ingress fromEntities[cluster,host] clause), 9120 (speaker metrics), 9121 (legacy FRR metrics, kept defensively so the documented escape hatch stays safe), 9140 (frr-k8s controller metrics), 9141 (frr-k8s FRR metrics), 19443 (frr-k8s statuscleaner webhook). Port 9443 stays in the deny list with a tightened comment: the host-network bind belongs to the etcd kube-rbac-proxy DaemonSet in cozy-monitoring; the metallb controller webhook collides on the same port number but on pod-IP, out of scope for a world-deny. - packages/system/cilium-networkpolicy/tests/networkpolicy_test.yaml: new helm-unittest suite that enumerates every metallb v0.16 host-network port in the deny list (including 7946 memberlist and the escape-hatch 9121), pins port 9443 to its etcd kube-rbac-proxy owner with the pod-IP collision note, and asserts the legacy 7473 entry is gone. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/bump-metallb
branch
from
May 29, 2026 08:08
9a1277d to
a9c8f20
Compare
Aleksei Sviridkin (lexfrei)
changed the title
Andrei Kvapil (kvaps)
approved these changes
Jun 1, 2026
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
Member
There was a problem hiding this comment.
LGTM. Verified the security-critical parts independently rather than trusting the inline rationale.
cilium-networkpolicy denylist rotation — every metallb v0.16 host-network listener checks out against the vendored chart's actual values: 9120 (prometheus.metricsPort), 9121 (speaker.frr.metricsPort), 9140 (frrk8s.prometheus.secureMetricsPort), 9141 (frrk8s.frr.secureMetricsPort), 19443 (frrk8s.webhookPort), plus 7946 memberlist. The 9443 re-attribution is correct: the host-network bind belongs to the cozy-monitoring etcd kube-rbac-proxy (monitoring-agents/templates/etcd-proxy-scrape.yaml, --secure-listen-address=$(NODE_IP):9443), while the metallb controller webhook on the same number is pod-IP only (out of scope). Dropping 7473 is right — legacy FRR-mode metrics are no longer bound under the frr-k8s default.
Health port 17472 correctly excluded — the speaker runs hostNetwork and --host=localhost is gone in v0.16, so I checked the binary: metallb v0.16.1 hardcodes HealthProbeBindAddress to 127.0.0.1:17472, so /healthz + /readyz never reach world. No gap.
Image digests bumped in lockstep with the chart appVersion (the whole point — chart/binary skew would CrashLoop every pod). The --platform=linux/amd64 pin on the buildx target is a good catch: the Dockerfile is a pure re-tag of upstream quay.io/metallb/*, so building on an arm64 host would otherwise pin an arm64-only digest into values.yaml and break amd64 nodes.
Tests pin the invariants that matter — image tag == appVersion, BGP_TYPE=frr-k8s, frr-k8s DaemonSet present, escape-hatch behavior, NetworkPolicies off by default, and the denylist port set.
The frr-k8s prometheus: null-merge workaround and the legacy-FRR escape-hatch caveat (idle-but-installed subchart due to Helm condition: evaluation context) are both documented honestly in the values header.
Note: the E2E run is red, but unrelated to this change — metallb and cilium-networkpolicy both reconcile fine in the log ("Helm install succeeded for release cozy-metallb/metallb.v1"); the failures are flux-plunger/dashboard reconcile timeouts and an image pre-pull timeout. Worth a re-run.
Aleksei Sviridkin (lexfrei)
added a commit
to lexfrei/metallb
that referenced
this pull request
Jun 21, 2026
`charts/metallb/values.schema.json` declared
`speaker.bgpDebounceTimeout` as `anyOf: integer minimum 0 | null`, but
the speaker binary rejects 0:
speaker/main.go:134
parsed, err := strconv.Atoi(*bgpDebounceTimeoutMs)
if err != nil || parsed <= 0 {
level.Error(logger).Log("msg", "invalid BGP debounce timeout
value, must be a positive integer value", ...)
os.Exit(1)
}
An operator who set `speaker.bgpDebounceTimeout: 0` in helm values
passed schema validation, but the rendered speaker DaemonSet would
crash on startup with `os.Exit(1)`. (In current upstream, the
template's truthy guard `{{- if .Values.speaker.bgpDebounceTimeout }}`
happens to drop the flag for the falsy value 0, masking the schema
miss with a silent fallback to the 3000 ms default — confusing UX
either way.)
Align the schema with the runtime contract: `minimum: 1`. After this
change, `speaker.bgpDebounceTimeout: 0` is rejected at helm install
with a clear error pointing at the field, instead of crashing the
speaker later or silently ignoring the input.
Verified locally:
- `helm template ... --set speaker.bgpDebounceTimeout=0` is now
rejected with "at '/speaker/bgpDebounceTimeout': minimum: got 0,
want 1".
- `helm template ... --set speaker.bgpDebounceTimeout=1` renders
`--bgp-debounce-timeout=1` (boundary minimum, matches binary's
`parsed > 0` check).
- `helm template ... --set speaker.bgpDebounceTimeout=3000` renders
the flag unchanged.
- Unset (default null) keeps the flag out of the args list.
- Negative values stay rejected.
- `helm lint` clean.
Found during downstream consumer review while bumping cozystack to
MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context.
Signed-off-by: Aleksei Sviridkin <[email protected]>
Ori Braunshtein (oribon)
pushed a commit
to metallb/metallb
that referenced
this pull request
Jun 22, 2026
`charts/metallb/values.schema.json` declared
`speaker.bgpDebounceTimeout` as `anyOf: integer minimum 0 | null`, but
the speaker binary rejects 0:
speaker/main.go:134
parsed, err := strconv.Atoi(*bgpDebounceTimeoutMs)
if err != nil || parsed <= 0 {
level.Error(logger).Log("msg", "invalid BGP debounce timeout
value, must be a positive integer value", ...)
os.Exit(1)
}
An operator who set `speaker.bgpDebounceTimeout: 0` in helm values
passed schema validation, but the rendered speaker DaemonSet would
crash on startup with `os.Exit(1)`. (In current upstream, the
template's truthy guard `{{- if .Values.speaker.bgpDebounceTimeout }}`
happens to drop the flag for the falsy value 0, masking the schema
miss with a silent fallback to the 3000 ms default — confusing UX
either way.)
Align the schema with the runtime contract: `minimum: 1`. After this
change, `speaker.bgpDebounceTimeout: 0` is rejected at helm install
with a clear error pointing at the field, instead of crashing the
speaker later or silently ignoring the input.
Verified locally:
- `helm template ... --set speaker.bgpDebounceTimeout=0` is now
rejected with "at '/speaker/bgpDebounceTimeout': minimum: got 0,
want 1".
- `helm template ... --set speaker.bgpDebounceTimeout=1` renders
`--bgp-debounce-timeout=1` (boundary minimum, matches binary's
`parsed > 0` check).
- `helm template ... --set speaker.bgpDebounceTimeout=3000` renders
the flag unchanged.
- Unset (default null) keeps the flag out of the args list.
- Negative values stay rejected.
- `helm lint` clean.
Found during downstream consumer review while bumping cozystack to
MetalLB v0.16.0; see cozystack/cozystack#2699 for the trigger context.
Signed-off-by: Aleksei Sviridkin <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Bump vendored MetalLB chart from v0.15.2 to v0.16.1 and rebuild the cozystack-built
metallb-controller/metallb-speakerimages at the new version. Adopt the upstream-default BGP backend: FRR-K8s replaces the deprecated FRR mode. Rotate the cilium-networkpolicy clusterwide host-network port denylist to match the v0.16 listener set.The chart introduces a new probe contract (
/healthzand/readyzon a dedicated host-local port 17472, distinct from the metrics port), so the cozystack-built controller and speaker images must move in lockstep with the chart. Leaving the digests on v0.15.2 would CrashLoopBackOff every pod on main between merge and the next release-prep commit.Upstream highlights between 0.15.2 and 0.16.1:
metallb.tls.minVersion: VersionTLS12.tls.*andnetworkpolicies.*value blocks (opt-in).host: 127.0.0.1, pods/ServiceMonitor/PodMonitor gainprometheus.io/scheme: https, the frr-k8s subchart gains an opt-in ServiceMonitor toggle (default off), and one CRD numeric field widens from int32 to int64.ConfigurationStateCRD exposes reconcile errors per component; chart gained NetworkPolicy support and tightened pod SecurityContext.The
packages/system/metallb/values.yamlheader documents the changes operators see on upgrade, including the partial nature of the legacy-FRR escape hatch (Helm evaluates depcondition:against the declaring chart's values context, so a grandparent override from the umbrella cannot suppress the frr-k8s subchart through that condition; the subchart stays installed but idle when the speaker is flipped to FRR mode).New helm-unittest coverage pins the post-bump invariants and would have caught the original chart/binary skew before merge: controller/speaker image tags exactly match the chart
appVersion, speaker is the single-container DaemonSet withBGP_TYPE=frr-k8s, frr-k8s DaemonSet and CRDs are installed, NetworkPolicies stay off by default. The cilium-networkpolicy chart gains its first unittest suite, enumerating every metallb v0.16 host-network port plus the unrelated etcd kube-rbac-proxy port that shares port number 9443 (pod-IP vs host-network).Release note