Add strict validation for CIDR ranges in the Cluster webhook

[killianmuldoon]

Add a check in the Cluster webhook to ensure each CIDR block only contains valid CIDR blocks with the following rules:

1. No more than two CIDR blocks are specified under Pods or Services
2. If two are specified the blocks need to be from different IP families i.e. one IPv4 and one IPv6
3. The IPFamily for pods and services must be compatible
4. The CIDR ranges are valid CIDR ranges

This change ensures Clusters can not be created or updated with invalid CIDR blocks. This is the value that the Kubernetes control plane components take - e.g. the kube-apiserver flag --service-cluster-ip-range is documented:

A CIDR notation IP range from which to assign service cluster IPs. This must not overlap with any IP ranges assigned to nodes or pods. Max of two dual-stack CIDRs is allowed.

Related to: #7420

/kind feature
/area api
/kind api-change

[fabriziopandini]

/triage accepted
just as a reference the upstream code where Kubernetes components validate CIDR

Services: https://github.com/kubernetes/kubernetes/blob/6dc81c3280996a9a4267bf8ef30d4bda36f8a293/cmd/kube-apiserver/app/options/validation.go#L47-L69

Pods: https://github.com/kubernetes/kubernetes/blob/08749750a96fe7f236befa40076a3117e7b36733/staging/src/k8s.io/cloud-provider/app/core.go#L127-L133

(see #7420 (comment))

[fabriziopandini]

note: this is initially considered an API change because currently we are not prescriptive on what Cluster.Network CIDRs are for, we saw some borderline usage of that info, and we want to move to a place where it is clear that those values are meant for defining values to be passed to Kubernetes components only, so it makes sense to apply the same validations rules
/help

[k8s-ci-robot]

@fabriziopandini:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

note: this is initially considered an API change because currently we are not prescriptive on what Cluster.Network CIDRs are for, we saw some borderline usage of that info, and we want to move to a place where it is clear that those values are meant for defining values to be passed to Kubernetes components only, so it makes sense to apply the same validations rules
/help

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/test-infra repository.

[viveksyngh]

/assign

[viveksyngh]

I would like to help with this.

[killianmuldoon]

@viveksyngh Thanks for the interest! But this issue isn't ready to be worked on yet - we've added the label api-change which means we can't add this until we have a new API version, which I don't think there's been any planning for as of yet. That means this work is at least months away.

/help remove

(to avoid confusion)

[viveksyngh]

Thanks for the info @killianmuldoon

[sbueringer]

Just rememberd something from a different life :).

The "normal" use case is that the Pod CIDRs are passed through to the kube-controller-manager. The kube-controller-manager takes this CIDR and splits it up to assign Node CIDRs to Nodes. Then the "host-local" IPAM plugin on the nodes assigns IPs from the Node CIDR to individual Pods.

I don't remember how this works e.g. when Calico takes over the IPAM part:

• does the kube-controller-manager still implement the Node CIDR IPAM part (and calico only per-Pod IPAM)
• is the CIDR configured in Calico, how does this relate to the one in kube-controller-manager

Just wondering if there are scenarios where the baked in kube-controller-manager validation doesn't matter because the whole Node/Pod IPAM stuff is outsourced to e.g. something like Calico.

[k8s-triage-robot]

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

[fabriziopandini]

/priority important-longterm

[fabriziopandini]

In pipeline for the next API version

/triage accepted
/remove-priority important-longterm
/priority backlog