[v4.y] Load cluster ca certificates using OpenSSL::X509::Store#add_file #552
Conversation
cben
force-pushed
the
v4.y-openssl-x509-store-add-file
branch
2 times, most recently
from
03a206e to
2c4f23b
Compare
|
@jperville @DocX I'm backporting the Added tests and CI keeps failing on Mac — any idea if EDIT: note my test was not for root+intermediate scenario; it's a simple sandwitch of 3 unrelated CA certs, of which only the middle one should match. |
| elsif cluster.key?('certificate-authority-data') | ||
| Base64.decode64(cluster['certificate-authority-data']) | ||
| ca_cert_data = Base64.decode64(cluster['certificate-authority-data']) | ||
| cert_store.add_cert(OpenSSL::X509::Certificate.new(ca_cert_data)) |
There was a problem hiding this comment.
Umm, looks like the fix only covered certificate-authority, but certificate-authority-data would still have the bug.
- need to add test with concatenated inline data.
- can we fix it?
- if not, correct CHANGELOG to explain partial fix.
Test sandwitches the real CA cert between two unrelated CA certs (another-ca1.pem, another-ca2.pem, simply copied from two runs of update_certs_k0s.rb). No test for root+intermediate scenario. Fails before the backport of ManageIQ#461: KubeclientConfigTest#test_concatenated_ca [/home/beni/kubeclient/test/test_config.rb:196]: Expected false to be truthy. (some experimenting with order suggests only first cert is honored.) Passes with the fix.
…-add-file Load cluster ca certificates using OpenSSL::X509::Store#add_file (cherry picked from commit 53408c1)
cben
force-pushed
the
v4.y-openssl-x509-store-add-file
branch
2 times, most recently
from
3f43a3e to
1521fcd
Compare
|
Oops, maybe the Mac angle was my mistake. The cert I put in middle of concatenated-ca.pem didn't match the existing certs, so it failed I'm changing CI to complete all tests when one errors to get clearer picture => 🎉 mac passed. Still not handling |
- Helps confirm suspected OS-specific failures. - Normally when one build fails, most other builds already started and some almost complete `bundle install` / started tests. So it's not a big "waste" expensive to let them finish, arguably it's actually a waste to abort them! (sunken cost fallacy? :-) - In case rubocop complains, while I do consider it a merge blocker, it's better contributor (and maintainer) experience to also see test results.
cben
force-pushed
the
v4.y-openssl-x509-store-add-file
branch
from
1521fcd to
b1824ed
Compare
Backporting #461 (which fixed #460) + tests.