Optimize CA cert hash calculation with community.crypto #11758
Conversation
k8s-ci-robot
added
release-note
| - name: Extract public key from CA certificate | ||
| community.crypto.openssl_publickey_info: | ||
| path: "{{ kube_cert_dir }}/ca.crt" | ||
| type: x509 | ||
| register: publickey_info |
There was a problem hiding this comment.
The problem with that is that community.crypto module requires to install python cryptography on the managed host, which is not that easy to do -> see #11158
An alternative to that, I think, is to fetch either the public key or the whole cert (but not the private key, ofc) and use the community.crypto filters ( for instance the x509_info filter ).
Since filter runs on the ansible control node, this only requires to install cryptography on the control node, which is much easier.
There was a problem hiding this comment.
this issue seems more complex than I expected. I'll update the PR later.
thanks for your suggestions.
ErikJiang
force-pushed
the
optimize-ca-cert-hash-calculation
branch
from
b5d703b to
2a53439
Compare
k8s-ci-robot
added
the
do-not-merge/work-in-progress
ErikJiang
force-pushed
the
optimize-ca-cert-hash-calculation
branch
from
2a53439 to
f691d63
Compare
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
ErikJiang
force-pushed
the
optimize-ca-cert-hash-calculation
branch
5 times, most recently
from
bc0ccc1 to
33503a7
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
ErikJiang
force-pushed
the
optimize-ca-cert-hash-calculation
branch
from
33503a7 to
4e9a111
Compare
k8s-ci-robot
added
size/M
|
Great work 🎉 |
|
|
||
| - name: Calculate kubeadm CA cert hash | ||
| set_fact: | ||
| kubeadm_ca_hash: "{{ (ca_cert_content.content | b64decode | community.crypto.x509_certificate_info).public_key_fingerprints.sha256 }}" | ||
| when: ca_cert_content.content | length > 0 | ||
| delegate_to: localhost | ||
| run_once: true |
There was a problem hiding this comment.
This is pretty good, great work !
Just a thought though: do we need to use set fact ? I think it should be possible to simply have that expression directly in the jinja template in kubeadm-client.conf.j2, wdyt ? (adjusting the conditionals).
The only downside I could see is that it would compute it for each host, but I don't think the difference is noticeable.
There was a problem hiding this comment.
directly placing the expression in the kubeadm-client.conf.j2 template is indeed more concise.
| # Supported asymmetric encryption algorithm types for the cluster's keys and certificates. | ||
| # can be one of RSA-2048(default), RSA-3072, RSA-4096, ECDSA-P256 | ||
| # ref: https://kubernetes.io/docs/reference/config-api/kubeadm-config.v1beta4/#kubeadm-k8s-io-v1beta4-ClusterConfiguration | ||
| kube_asymmetric_encryption_algorithm: "RSA-2048" | ||
|
|
There was a problem hiding this comment.
You should probably mention in the commit message why we can do that (== since the switch makes it used only in one role), otherwise it can appear a bit random to the uninitiated ^.
Signed-off-by: bo.jiang <bo.jiang@daocloud.io>
ErikJiang
force-pushed
the
optimize-ca-cert-hash-calculation
branch
from
4e9a111 to
00e4964
Compare
|
Thanks for the great work ! First step to replace our usage of dubious openssl pipelines with community.crypto o/ /approve I don't think we need a release-note though, this should not change anything for the users. /release-note-edit |
k8s-ci-robot
added
kind/cleanup
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ErikJiang, VannTen The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
…igs#11758) Signed-off-by: bo.jiang <bo.jiang@daocloud.io>
What type of PR is this?
/kind feature
What this PR does / why we need it:
this PR optimizes the CA certificate hash calculation by using the
community.cryptomodule.additionally, I've moved the default value of
kube_asymmetric_encryption_algorithmback to theKubernetes/control-planerole, as this variable is currently only used within that role.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: