🐛 Relax minLength for bootstrap.dataSecretName to 0 #12164

mboersma · 2025-05-07T20:15:53Z

What this PR does / why we need it:

Changes the validation for Machine spec's bootstrap.dataSecretName to // +kubebuilder:validation:MinLength=0. This allows an empty string, which is an existing use case for managed MachinePools.

Which issue(s) this PR fixes:

Fixes #12161

/area bootstrap

mboersma · 2025-05-07T20:31:12Z

/cc @sbueringer

nawazkh · 2025-05-07T21:14:52Z

Thank you for this!
/lgtm

k8s-ci-robot · 2025-05-07T21:14:59Z

LGTM label has been added.

Git tree hash: 6c8de6c1cd29aca39dd336a0b4546570f37a6c26

bootstrap/kubeadm/api/v1beta1/kubeadmconfig_types.go

sbueringer · 2025-05-08T08:01:39Z

/cherry-pick release-1.10

k8s-infra-cherrypick-robot · 2025-05-08T08:01:41Z

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.10

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.

sbueringer · 2025-05-08T08:01:43Z

@mboersma One question

enxebre · 2025-05-08T09:26:26Z

api/v1beta2/machine_types.go

@@ -696,7 +696,7 @@ type Bootstrap struct {
 	// dataSecretName is the name of the secret that stores the bootstrap data script.
 	// If nil, the Machine should remain in the Pending state.
 	// +optional
-	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MinLength=0


consider make this not a pointer in v1beta2 based on the outcome of #12153 (comment)

I think that should not be part of this PR as this PR is a bug fix for a specific issue and we'll also backport it

fabriziopandini · 2025-05-08T13:22:34Z

What is the use case for setting dataSecretName to ""?
TBH It doesn't seems a clean API to me

mboersma · 2025-05-08T13:29:59Z

What is the use case for setting dataSecretName to ""?

In an Azure managed MachinePool, for example, spec.template.spec.bootstrap.dataSecretName is not actually used, so our templates and tests and docs set it to an empty string in that case.

Now with CAPI v1.10.x, we see this error:

spec.template.spec.bootstrap.dataSecretName in body should be at least 1 chars long

We've had a couple customers report it already, even though we haven't finished integrating CAPI v1.10 yet because of this.

We can work around it in CAPZ by setting the dataSecretName to "unused" everywhere, but this seems more backward-compatible (intentionally or otherwise).

sbueringer · 2025-05-08T13:32:00Z

I think we can debate what to do for v1beta2, but for v1beta1 we have to allow minLength=0 again and should make that change ASAP. Otherwise we keep a change in v1.10 that breaks MachinePool implementations across a few providers (which is definitely a breaking change, and we should not do that within v1beta1)

sbueringer · 2025-05-08T13:32:46Z

/lgtm

k8s-ci-robot · 2025-05-08T13:32:53Z

LGTM label has been added.

Git tree hash: 4c6ff23cbe7cde2f98278a40e38c44f611b46557

richardcase · 2025-05-08T13:33:07Z

In an Azure managed MachinePool, for example, spec.template.spec.bootstrap.dataSecretName is not actually used, so our templates and tests and docs set it to an empty string in that case.

This is a similar situation for AWS MachinePools

sbueringer · 2025-05-08T13:34:14Z

In my opinion this is another reason why we should stop reusing the MachineSpec struct between MD and MP.

We would be able to clean this up for MD/MS/Machine without breaking MP if we wouldn't reuse the struct

(edit, ah nope, MachinePool Machines doesn't allow that...)

damdo

/lgtm

Is there a specific approver for this, or is it you @sbueringer ?

sbueringer · 2025-05-08T15:57:37Z

I would like to wait for a response from Fabrizio

sbueringer · 2025-05-09T07:32:54Z

cc @fabriziopandini ^^

fabriziopandini · 2025-05-09T08:44:26Z

I'm ok in unblocking this fix for v1beta1/v1.10, but I would like we seek for a better solution for v1beta2
/approve

k8s-ci-robot · 2025-05-09T08:44:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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:

~~OWNERS~~ [fabriziopandini]

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

k8s-infra-cherrypick-robot · 2025-05-09T09:07:52Z

@sbueringer: #12164 failed to apply on top of branch "release-1.10":

Applying: Relax minLength for bootstrap.dataSecretName to 0
Using index info to reconstruct a base tree...
M	api/v1beta1/machine_types.go
A	api/v1beta2/machine_types.go
M	config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
M	config/crd/bases/cluster.x-k8s.io_machinepools.yaml
M	config/crd/bases/cluster.x-k8s.io_machines.yaml
M	config/crd/bases/cluster.x-k8s.io_machinesets.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/crd/bases/cluster.x-k8s.io_machinesets.yaml
CONFLICT (content): Merge conflict in config/crd/bases/cluster.x-k8s.io_machinesets.yaml
Auto-merging config/crd/bases/cluster.x-k8s.io_machines.yaml
CONFLICT (content): Merge conflict in config/crd/bases/cluster.x-k8s.io_machines.yaml
Auto-merging config/crd/bases/cluster.x-k8s.io_machinepools.yaml
CONFLICT (content): Merge conflict in config/crd/bases/cluster.x-k8s.io_machinepools.yaml
Auto-merging config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
CONFLICT (content): Merge conflict in config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
CONFLICT (modify/delete): api/v1beta2/machine_types.go deleted in HEAD and modified in Relax minLength for bootstrap.dataSecretName to 0. Version Relax minLength for bootstrap.dataSecretName to 0 of api/v1beta2/machine_types.go left in tree.
Auto-merging api/v1beta1/machine_types.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Relax minLength for bootstrap.dataSecretName to 0

In response to this:

/cherry-pick release-1.10

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.

tamalsaha · 2025-05-09T09:14:28Z

Thanks! Can we get v1.10.2?

chrischdi · 2025-05-09T09:31:20Z

Thanks! Can we get v1.10.2?

It first needs a PR to cherry-pick it to release-1.10 branch.

If I'm right and plans do not change, v1.10.2 is planned to be released on 20th may.

https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/release/releases/release-1.11.md

tamalsaha · 2025-05-09T09:41:56Z

I see. An earlier patch release will be much appreciated as v0.10.x release of capz/capg/capa all are blocked on this pr.

sbueringer · 2025-05-09T11:12:01Z

I think you have to discuss this with the release team.

Btw this could have been detected earlier by bumping to beta.0 or any of the subsequent pre releases.

sbueringer · 2025-05-09T11:13:43Z

Can someone open a manual cherry-pick for release-1.10 please?

sbueringer · 2025-05-09T11:15:43Z

I'm ok in unblocking this fix for v1beta1/v1.10, but I would like we seek for a better solution for v1beta2 /approve

@richardcase @mboersma Can one of you please open an issue for that?

I'll then link it accordingly in the v1beta2 & "Graduate MachinePool" umbrella issues.

richardcase · 2025-05-09T12:29:49Z

I'm ok in unblocking this fix for v1beta1/v1.10, but I would like we seek for a better solution for v1beta2

I'll add it to the list tracking issue for v1beta2 machinepools

damdo · 2025-05-09T12:53:43Z

Here is the backport PR @sbueringer #12180

k8s-ci-robot added area/bootstrap Issues or PRs related to bootstrap providers cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 7, 2025

k8s-ci-robot requested review from richardcase and vincepri May 7, 2025 20:16

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 7, 2025

k8s-ci-robot requested a review from sbueringer May 7, 2025 20:31

k8s-ci-robot assigned nawazkh May 7, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 7, 2025

nawazkh removed their assignment May 7, 2025

tamalsaha mentioned this pull request May 8, 2025

Bump CAPI to v1.10.2 kubernetes-sigs/cluster-api-provider-gcp#1459

Merged

3 tasks

sbueringer reviewed May 8, 2025

View reviewed changes

bootstrap/kubeadm/api/v1beta1/kubeadmconfig_types.go Outdated Show resolved Hide resolved

enxebre reviewed May 8, 2025

View reviewed changes

Relax minLength for bootstrap.dataSecretName to 0

6597677

mboersma force-pushed the datasecretname-min-length-zero branch from 94e077b to 6597677 Compare May 8, 2025 13:10

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2025

k8s-ci-robot assigned sbueringer May 8, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 8, 2025

damdo mentioned this pull request May 8, 2025

🌱 Bump CAPI to v1.10.2 kubernetes-sigs/cluster-api-provider-aws#5447

Merged

5 tasks

damdo reviewed May 8, 2025

View reviewed changes

k8s-ci-robot assigned damdo May 8, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 9, 2025

k8s-ci-robot merged commit 7e6eaac into kubernetes-sigs:main May 9, 2025
23 checks passed

k8s-ci-robot added this to the v1.11 milestone May 9, 2025

richardcase mentioned this pull request May 9, 2025

Tracking issue for v1Beta2 - MachinePools #12178

Open

16 tasks

mboersma deleted the datasecretname-min-length-zero branch May 9, 2025 12:53

damdo mentioned this pull request May 9, 2025

🐛 [release-1.10] Relax minLength for bootstrap.dataSecretName to 0 #12180

Merged

🐛 Relax minLength for bootstrap.dataSecretName to 0 #12164

🐛 Relax minLength for bootstrap.dataSecretName to 0 #12164

Uh oh!

Conversation

mboersma commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mboersma commented May 7, 2025

Uh oh!

nawazkh commented May 7, 2025

Uh oh!

k8s-ci-robot commented May 7, 2025

Uh oh!

Uh oh!

sbueringer commented May 8, 2025

Uh oh!

k8s-infra-cherrypick-robot commented May 8, 2025

Uh oh!

sbueringer commented May 8, 2025

Uh oh!

enxebre May 8, 2025

Choose a reason for hiding this comment

Uh oh!

sbueringer May 8, 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!

fabriziopandini commented May 8, 2025

Uh oh!

mboersma commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented May 8, 2025

Uh oh!

sbueringer commented May 8, 2025

Uh oh!

k8s-ci-robot commented May 8, 2025

Uh oh!

richardcase commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

sbueringer commented May 8, 2025

Uh oh!

sbueringer commented May 9, 2025

Uh oh!

fabriziopandini commented May 9, 2025

Uh oh!

k8s-ci-robot commented May 9, 2025

Uh oh!

Uh oh!

k8s-infra-cherrypick-robot commented May 9, 2025

Uh oh!

tamalsaha commented May 9, 2025

Uh oh!

chrischdi commented May 9, 2025

Uh oh!

tamalsaha commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

sbueringer commented May 9, 2025

Uh oh!

sbueringer commented May 9, 2025

Uh oh!

richardcase commented May 9, 2025

Uh oh!

damdo commented May 9, 2025

Uh oh!

Uh oh!

mboersma commented May 7, 2025 •

edited

Loading

sbueringer May 8, 2025 •

edited

Loading

mboersma commented May 8, 2025 •

edited

Loading

richardcase commented May 8, 2025 •

edited

Loading

sbueringer commented May 8, 2025 •

edited

Loading

tamalsaha commented May 9, 2025 •

edited

Loading

sbueringer commented May 9, 2025 •

edited

Loading