✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Karthik-K-N · 2025-10-15T07:34:26Z

What this PR does / why we need it:

This PR adds EncryptionAlgorithm to KubeadmConfig and necessary changes

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes ##10077

/area provider/bootstrap-kubeadm

k8s-ci-robot · 2025-10-15T07:34:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vincepri for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Karthik-K-N · 2025-10-15T07:36:22Z

Need to add Unit test cases and fix existing tests

Not super confident about the changes, Have broken some interfaces as well, Based on the initial review and if the path chosen is correct, I will update accordingly.

@sbueringer Please take a look when time permits.

sbueringer

Thx! I think I covered the open points, but just ping me if you want to discuss anything else.

Definitely feel free to ping me again after the next iteration on the prod code for another quick review

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

bootstrap/kubeadm/types/upstreamv1beta4/conversion.go

sbueringer · 2025-10-16T14:28:03Z

controlplane/kubeadm/internal/controllers/controller.go


 	// Generate Cluster Kubeconfig if needed
-	if result, err := r.reconcileKubeconfig(ctx, controlPlane); !result.IsZero() || err != nil {
+	if result, err := r.reconcileKubeconfig(ctx, controlPlane); err != nil || !result.IsZero() {


Please revert this one

Reverted it.

May not be either needed or relevant wrt to this PR, but I feel its better to maintain a standard for this, I see some places
!result.IsZero() || err != nil and in some other err != nil || !result.IsZero()
IDE suggesting to check error first

Got it. Makes sense and I agree with checking error first. Let's just do this everywhere and in a separate PR.

Feel free to open a PR, but please on top of #12857 / or after 12857 is merged (I get rid of some of these cases in that PR)

Sure, I will do it later. Thank you.

controllers/clustercache/cluster_accessor.go

util/certs/certs.go

util/certs/types.go

util/kubeconfig/kubeconfig.go

Karthik-K-N · 2025-10-17T07:53:39Z

@sbueringer thanks for the review, I have updated accordingly and ready for next round of review. Please take a look when time permints.

I tried in the approach of changing the existing thing rather than avoiding the breakage for users, I will remember it for the next time. Thanks

sbueringer · 2025-10-17T08:40:29Z

I tried in the approach of changing the existing thing rather than avoiding the breakage for users, I will remember it for the next time. Thanks

All good. I think it's a bit of a trade-off and probably also a case-by-case decision. Thx for asking for early feedback! I'll take another look

controllers/clustercache/cluster_accessor.go

controlplane/kubeadm/internal/workload_cluster.go

util/kubeconfig/kubeconfig.go

sbueringer · 2025-10-20T11:36:54Z

util/kubeconfig/kubeconfig.go


 // New creates a new Kubeconfig using the cluster name and specified endpoint.
-func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Signer) (*api.Config, error) {
+func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Signer, options ...KubeConfigurationOption) (*api.Config, error) {


Suggested change

func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Signer, options ...KubeConfigurationOption) (*api.Config, error) {

func New(clusterName, endpoint string, caCert *x509.Certificate, caKey crypto.Signer, options ...KubeconfigOption) (*api.Config, error) {

Just for consistency with a few other places

Thought of doing that bases on your earlier feed back but it gives me warning that the interface Name startes with package name "kubeconfig", if that good to go, then I can rename

Fine to ignore. We often try to consider that but sometimes the alternative is worse 😀

(The current name doesn't trigger the warning but the "problem" (that it stutters) isn't really gone. But I think KubeconfigOption is just the best name here)

Ok, In that case, I can rename it to KubeconfigOption

util/kubeconfig/kubeconfig.go

sbueringer · 2025-10-20T11:42:48Z

Did another quick round of review. I think the e2e test failure might just be a flake but let's see

/retest

If tests are green and the findings above are addressed I would do a full review (just need a bigger block of time for that)

util/kubeconfig/kubeconfig.go

fabriziopandini · 2025-10-21T09:28:03Z

controlplane/kubeadm/internal/control_plane.go

+
+// GetKeyEncryptionAlgorithm returns the control plane EncryptionAlgorithm.
+func (c *ControlPlane) GetKeyEncryptionAlgorithm() bootstrapv1.EncryptionAlgorithmType {
+	return c.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.EncryptionAlgorithm


q: should we return the "default" encryption algo used by CAPI(or kubeadm) in case the field is not set?
Considering we are carrying this info around and using it, being explicit could help in throubleshooting...

Yeah, Thats good option. Updated it.

fabriziopandini · 2025-10-21T09:30:11Z

util/certs/certs.go

+// the default size of 2048.
+func rsaKeySizeFromAlgorithmType(keyEncryptionAlgorithm bootstrapv1.EncryptionAlgorithmType) int {
+	switch keyEncryptionAlgorithm {
+	case bootstrapv1.EncryptionAlgorithmRSA2048, "":


, "" is sort of confusing (added a comment also in control_plane.go)

Have to take another look, but probably we have to handle this here in case folks are not setting the EncyprtionAlgorithm option (or do the defaulting one level higher or so)

But godoc at least would be helpful

fabriziopandini · 2025-10-21T09:32:21Z

controlplane/kubeadm/internal/cluster_test.go

 			g.Expect(err).ToNot(HaveOccurred())

-			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey)
+			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey, "")


Let's use the default encryption algo instead of ""

rebase with main

Karthik-K-N · 2025-10-23T05:16:52Z

Some misconfiguration, will check
https://storage.googleapis.com/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-api/12859/pull-cluster-api-e2e-blocking-main/1981193968041332736/artifacts/clusters/bootstrap/logs/capi-system/capi-controller-manager/capi-controller-manager-7678cb554b-phkfl/manager.log

2025-10-23T03:20:49.644479347Z stderr F 	 stderr: error: error execution phase certs/apiserver: couldn't load CA certificate ca: failure loading ca certificate authority: failed to load key: couldn't load the private key file /etc/kubernetes/pki/ca.key: error reading private key file /etc/kubernetes/pki/ca.key: data does not contain a valid RSA or ECDSA private key

sbueringer · 2025-10-23T05:29:31Z

Maybe similar to #12859 (comment)

k8s-ci-robot · 2025-10-23T07:01:49Z

@Karthik-K-N: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-apidiff-main	`a41ee45`	link	false	`/test pull-cluster-api-apidiff-main`

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.

Instructions 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.

Karthik-K-N · 2025-10-23T08:02:20Z

Just checking again to make sure it passes.
/test pull-cluster-api-e2e-blocking-main

sbueringer · 2025-10-23T08:34:04Z

/test pull-cluster-api-e2e-main

Triggering all for more data

k8s-ci-robot added area/provider/bootstrap-kubeadm Issues or PRs related to CAPBK cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 15, 2025

k8s-ci-robot requested review from elmiko and g-gaston October 15, 2025 07:34

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 15, 2025

sbueringer reviewed Oct 16, 2025

View reviewed changes

Karthik-K-N force-pushed the rsa-key branch from e29086e to 9b93039 Compare October 17, 2025 12:02

sbueringer reviewed Oct 20, 2025

View reviewed changes

util/kubeconfig/kubeconfig.go Outdated Show resolved Hide resolved

fabriziopandini reviewed Oct 21, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2025

Karthik-K-N added 6 commits October 23, 2025 08:13

Add EncryptionAlgorithm to Kubeadm's ClusterConfiguration

cd1a25d

Controller changes

41c4d39

Util changes

eefb585

Test changes

f8dac7e

Address review comments

418ff3d

Address review comments and rebase with main

f0820c0

rebase with main

Karthik-K-N force-pushed the rsa-key branch from 9ce5fd3 to f0820c0 Compare October 23, 2025 03:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2025

Address review comments

a41ee45

	func New(clusterName, endpoint string, caCert x509.Certificate, caKey crypto.Signer, options ...KubeConfigurationOption) (api.Config, error) {
	func New(clusterName, endpoint string, caCert x509.Certificate, caKey crypto.Signer, options ...KubeconfigOption) (api.Config, error) {

Uh oh!

✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Are you sure you want to change the base?

✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Conversation

Karthik-K-N commented Oct 15, 2025

Uh oh!

k8s-ci-robot commented Oct 15, 2025

Uh oh!

Karthik-K-N commented Oct 15, 2025

Uh oh!

sbueringer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karthik-K-N commented Oct 17, 2025

Uh oh!

sbueringer commented Oct 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbueringer commented Oct 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N commented Oct 23, 2025

Uh oh!

sbueringer commented Oct 23, 2025

Uh oh!

k8s-ci-robot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Karthik-K-N commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Oct 23, 2025

sbueringer left a comment •

edited

Loading

sbueringer Oct 17, 2025 •

edited

Loading

sbueringer Oct 23, 2025 •

edited

Loading

Karthik-K-N Oct 23, 2025 •

edited

Loading

sbueringer Oct 21, 2025 •

edited

Loading

k8s-ci-robot commented Oct 23, 2025 •

edited

Loading

Karthik-K-N commented Oct 23, 2025 •

edited

Loading