Repository navigation
Conversation
c1d8132 to
7fb675e
Compare
7fb675e to
ffe1904
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Tests are missing for CIDR parsing, IPv6 canonicalization, IPv4-mapped IPv6 unmapping, cross-source SAN deduplication, and rejection of mapped loopback/link-local addresses.
1 open issue
What changed in this PR
Adds Calico-reported interface IPs to hypervisor certificate SANs so TLS connections can use additional host interfaces.
Changes:
- Merges selected interface addresses with node-status addresses, normalizing and deduplicating IPs while filtering invalid or non-routable entries.
- Adds include/exclude regex flags and matching Helm values, defaulting to numbered VLAN interfaces.
- Adds tests for interface filtering, address merging, malformed annotations, and annotation removal.
| File | Description |
|---|---|
internal/controller/node_certificate_controller.go |
Parses Calico annotations, filters interfaces, and normalizes certificate IP SANs. |
internal/controller/node_certificate_controller_test.go |
Adds annotation processing and certificate reconciliation tests. |
cmd/main.go |
Defines interface filter flags and passes them to the controller. |
charts/openstack-hypervisor-operator/values.yaml |
Adds filter defaults and controller arguments. |
charts/openstack-hypervisor-operator/templates/deployment.yaml |
Exposes filter values through environment variables. |
Review effort: Balanced
notandy
left a comment
notandy
left a comment
There was a problem hiding this comment.
Can't you just use the downward api to pass down the calico projectcalico.org/Interfaces annotation? 300+ lines for just adding another IP to the certificate?
33bfdfd to
a6c2e9f
Compare
The certificate only covered the node's status addresses, so
connections over other host interfaces (e.g. vlan networks reported
by Calico) failed TLS validation. Which interfaces to admit is
configurable via --certificate-interface-{include,exclude}-regex.
Signed-off-by: Fabian Wiesel <[email protected]>
a6c2e9f to
f118bff
Compare
Merging this branch will decrease overall coverage
Coverage detailsCompared to commit cbd917d (run #40846960949)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files:
|
How can I use the downward api to transport a value from a node to a crd from the certificate-manager? I thought, it was only to get values from nodes to pods/containers.
Half of which were tests... and 70 lines of logic. Rest is passing values through the stack (Chart->CLI->Controller) If LoC are the problem, I tried to compress it. Now ~50 lines of logic, ~50 lines of setup, ~130 lines test. |
What
Adds IP SANs from Calico's
projectcalico.org/Interfacesnode annotation to the hypervisor certificate, alongside the existingnode.Status.AddressesSANs.Why
The libvirt certificate only covered the node's status addresses, so connections over other host interfaces (e.g. vlan networks reported by Calico) failed TLS validation.
How to configure
Which interfaces are admitted is configurable via two new flags (and the matching Helm values):
--certificate-interface-include-regex(default^vlan_[0-9]+$) — only matching interface names are considered; an empty expression includes all interfaces--certificate-interface-exclude-regex(default"") — matching interfaces are excluded; the exclude expression wins over the include oneAddresses are normalized (IPv6 canonical form, IPv4-mapped IPv6) and non-routable entries (loopback, link-local, unspecified, multicast, zoned) are skipped, so a malformed or unexpected annotation cannot make cert-manager reject the whole certificate. Only plain IP addresses are accepted — Calico validates addresses with
net.ParseIPbefore writing the annotation, so CIDR notation and anything else is simply skipped.Notes
projectcalico.org/Interfacesannotation is populated bycalico-node's interface/IP autodetection at startup (the defaultIP_AUTODETECTION_METHODbehavior; nothing is written when detection is disabled, e.g.IP=none). That detection also skips virtual and CNI interfaces (lo,cali.*,tunl.*,vxlan.calico,docker.*,veth.*, ...), so only real host interfaces appear in the annotation. The annotation was introduced with Calico v3.31 — on older Calico, or where it is not written, this feature is a silent no-op.ssa-node-certificate— merge that PR first.vlan_<digits>interfaces; the primary interface is already represented by the node status addresses.