🌱 api: relax validation for Machine .status.addresses to maximum of 128 instead of 32 items

[chrischdi]

What this PR does / why we need it:

This PR relaxes the validation which was added in v1.11 to allow more entries in the Machine's .status.addresses slice.

• Before: maximum of 32 items
• After: maximum of 128 items

This came up when trying to bump CAPZ to Cluster API 1.11 at:

kubernetes-sigs/cluster-api-provider-azure#5979

CAPZ has a test which uses Azure CNI which leads to have 111 entries at AzureMachine.status.addresses.

Patching that up to the Machine fails due to the validation added in v1.11.

The entries are:

• 1x InternalDNS
• 1x InternalIP's
• 109 additional InternalIP's

To still get more information about the reasoning on why CAPZ has this use case of having more addresses then 32, I asked the question at https://kubernetes.slack.com/archives/CEX9HENG7/p1764231240877609

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 #

/area api

[chrischdi]

/cherry-pick release-1.12

[k8s-infra-cherrypick-robot]

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

In response to this:

/cherry-pick release-1.12

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.

[chrischdi]

/cherry-pick release-1.11

[k8s-infra-cherrypick-robot]

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

In response to this:

/cherry-pick release-1.11

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.

[neolit123]

i think we might want to remove this validation or set it to a higher value like 256, which is in the CIDR limits for most CNI plugins...or even 512 to be safer.

what are the security implication of the field not having a limit?
is it just a foot gun for the admin to somehow self-ddos, or something in those lines?

the case with CAPZ here i think was that they decided to add a test for CAPZ+Azure CNI, making sure that the CNI can work at it's limits. yet, reading here the actual maximum is 250:
https://learn.microsoft.com/en-us/azure/aks/azure-cni-overlay

110 is a kubelet's maxPods default:
https://kubernetes.io/docs/setup/best-practices/cluster-large/
but the field has no limit.

[chrischdi]

i think we might want to remove this validation or set it to a higher value like 256, which is in the CIDR limits for most CNI plugins...or even 512 to be safer.

what are the security implication of the field not having a limit? is it just a foot gun for the admin to somehow self-ddos, or something in those lines?

the case with CAPZ here i think was that they decided to add a test for CAPZ+Azure CNI, making sure that the CNI can work at it's limits. yet, reading here the actual maximum is 250: learn.microsoft.com/en-us/azure/aks/azure-cni-overlay

110 is a kubelet's maxPods default: kubernetes.io/docs/setup/best-practices/cluster-large but the field has no limit.

We should not remove this validation as we want it for CEL to be able to calculate the cost of a validation.
We added it everywhere on purpose and tried to be sensible finding the right defaults.

The number in Azure I think does not corelate with number of pods able to run, as these are pod IPs and not machine ips.
But to be sure I asked in the CAPZ channel.

We should be okay with also setting 256 or even 512, but I'd like to understand the use case. It not really makes sense to CAPI to know all pod ips.

It is easy to relax the validation, which then is not a breaking change. Making validation more strict on the other hand is a breaking change we should try to prevent.

[fabriziopandini]

I'm ok in raising the value, but I would prefer to not increase it too much without having a better understanding of use cases.
Let's wait for beginning next week when most of the US folks will be back.
/hold

[damdo]

Looks reasonable to me, thanks @chrischdi

[mboersma]

@chrischdi looking at the CAPZ test, I'm not sure why it requires so many addresses. I don't think this is specifically the intent of the Creating a highly available cluster with Azure CNI v1 spec, but maybe that's how it's been and the new validation caught it.

Having said that, it seems entirely reasonable to relax the validation somewhat here.

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 835be51d2683d1383e2bdb8ebd34af2679de4199

[fabriziopandini]

/lgtm
/approve

[k8s-ci-robot]

[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

[chrischdi]

/hold cancel

[k8s-infra-cherrypick-robot]

@chrischdi: new pull request created: #13090

In response to this:

/cherry-pick release-1.12

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.

[k8s-infra-cherrypick-robot]

@chrischdi: new pull request created: #13091

In response to this:

/cherry-pick release-1.11

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.