[magnum-auto-healer] Add certificate validation - #3170
Conversation
The controller always sets InsecureSkipVerify. The two modern Magnum drivers in use today both use CAPI and thus kubeadm under the hood. kubeadm injects nodes internal IPs as SANs into the certs it generates, meaning certificate validation should pass when using the same root cert. Thus, we add two new configuration knobs: one to point to a CA file to use (defaulting to the well-known location k8s mounts certs to) and another to explicitly disable verification as an escape hatch. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Assisted-by: Claude Sonnet 4.8 <noreply@anthropic.com>
kubernetes-prow
Bot
added
the
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
cncf-cla: yes
|
@stephenfin: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| caPool, err = cert.NewPool(check.CAFile) | ||
| if err != nil { | ||
| log.Errorf("Node %s, failed to load CA file %s: %v", nodeName, check.CAFile, err) | ||
| return true |
There was a problem hiding this comment.
shouldn't it return false since a fail has happened when loading the CA?
| tr := &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, | ||
| tlsConfig := &tls.Config{InsecureSkipVerify: check.TLSInsecure} | ||
| if check.CAFile != "" { |
There was a problem hiding this comment.
do we need to load CAFile if TLSInsecure is set?
2 participants
What this PR does / why we need it:
The controller always sets InsecureSkipVerify. The two modern Magnum drivers in use today both use CAPI and thus kubeadm under the hood. kubeadm injects nodes internal IPs as SANs into the certs it generates, meaning certificate validation should pass when using the same root cert. Thus, we add two new configuration knobs: one to point to a CA file to use (defaulting to the well-known location k8s mounts certs to) and another to explicitly disable verification as an escape hatch.
Which issue this PR fixes(if applicable):
(none)
Special notes for reviewers:
Release note: