🐛 Fix validation of worker topology names in Cluster resource

[dlipovetsky]

What this PR does / why we need it:
The worker topology name is used to generate the name of a Kubernetes resource (MachineDelpoyment or MachinePool), and must therefore be a valid Kubernetes resource name. The existing validation does not ensure this.

The first commit adds tests; without a fix, they fail, as expected.
The second commit fixes the validation.

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 #12068

/area clusterclass

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[k8s-ci-robot]

[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

[dlipovetsky]

When I authored the fix, I noticed a disagreement between the validation and the API types. I will keep this as a draft PR until we resolve the disagreement.

The API types say that a worker topology Name may be up to 255 characters:-

• https://github.com/mesosphere/cluster-api//blob/155e6604aea01770c979631265c057e5e7eff45e/api/v1beta2/cluster_types.go#L683-L690
• https://github.com/mesosphere/cluster-api//blob/155e6604aea01770c979631265c057e5e7eff45e/api/v1beta2/cluster_types.go#L794-L801

However, both MachineDeployment and MachinePool validation limits the name to 63 characters, because it checks that the name is a valid label value:

• https://github.com/mesosphere/cluster-api//blob/59bf58d3cea398eaedbc95d1b0774257c12a0bff/internal/topology/check/compatibility.go#L327
• https://github.com/mesosphere/cluster-api//blob/59bf58d3cea398eaedbc95d1b0774257c12a0bff/internal/topology/check/compatibility.go#L391

My draft PR replaces this value check with a stricter one, but the length is not changed.

As an aside, I noticed that our validation imposes this 63 character limit on the Cluster and MachineDeployment names:

• https://github.com/mesosphere/cluster-api//blob/1d6b58e0c74788ddf7bf6060d26a3a6bfb8f9a16/internal/webhooks/cluster.go#L167

• cluster-api/internal/webhooks/machinedeployment.go

  Line 197 in fba0337

  if errs := validation.IsValidLabelValue(newMD.Name); len(errs) != 0 {

[sbueringer]

@dlipovetsky

When I authored the fix, I noticed a disagreement between the validation and the API types. I will keep this as a draft PR until we resolve the disagreement.

When I introduced MaxLength on the field I missed that we already had that validation in the webhook. So I assumed that we have to keep supporting longer strings.

The MD webhook was not a factor there because we limit the name actually used for the MD here:

cluster-api/exp/topology/desiredstate/desired_state.go

Line 802 in 155e660

name, err := topologynames.MachineDeploymentNameGenerator(nameTemplate, s.Current.Cluster.Name, machineDeploymentTopology.Name).GenerateName()

As we already had that validation in the webhook. Let's reduce the MaxLengths accordingly