Skip to content

✨ Additional validation in Cluster/ClusterClass webhook for chained upgrades #12816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+158 −2
Open

✨ Additional validation in Cluster/ClusterClass webhook for chained upgrades #12816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c05f39
Add UT for cluster webhook
Karthik-K-N Oct 1, 2025
099c49d
Add validation for ClusterClass update scenario
Karthik-K-N Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions internal/webhooks/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2703,7 +2703,6 @@ func TestClusterTopologyValidationForTopologyClassChange(t *testing.T) {
Build(),
wantErr: false,
},

{
name: "Reject cluster.topology.class change with an incompatible infrastructureCluster Kind ref change",
firstClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
Expand Down Expand Up @@ -2762,7 +2761,6 @@ func TestClusterTopologyValidationForTopologyClassChange(t *testing.T) {
Build(),
wantErr: false,
},

{
name: "Reject cluster.topology.class change with an incompatible controlPlane Kind ref change",
firstClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
Expand Down Expand Up @@ -3123,6 +3121,38 @@ func TestClusterTopologyValidationForTopologyClassChange(t *testing.T) {
Build(),
wantErr: true,
},

// Kubernetes Version changes.
{
name: "Accept cluster.topology.class change with a compatible Kubernetes Version",
firstClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(refToUnstructured(ref)).
WithControlPlaneTemplate(refToUnstructured(ref)).
WithControlPlaneInfrastructureMachineTemplate(refToUnstructured(ref)).
Build(),
secondClass: builder.ClusterClass(metav1.NamespaceDefault, "class2").
WithInfrastructureClusterTemplate(refToUnstructured(compatibleNameChangeRef)).
WithControlPlaneTemplate(refToUnstructured(ref)).
WithControlPlaneInfrastructureMachineTemplate(refToUnstructured(ref)).
WithVersions("v1.22.2", "v1.23.2").
Build(),
wantErr: false,
},
{
name: "Reject cluster.topology.class change with an incompatible Kubernetes Version",
firstClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(refToUnstructured(ref)).
WithControlPlaneTemplate(refToUnstructured(ref)).
WithControlPlaneInfrastructureMachineTemplate(refToUnstructured(ref)).
Build(),
secondClass: builder.ClusterClass(metav1.NamespaceDefault, "class2").
WithInfrastructureClusterTemplate(refToUnstructured(compatibleNameChangeRef)).
WithControlPlaneTemplate(refToUnstructured(ref)).
WithControlPlaneInfrastructureMachineTemplate(refToUnstructured(ref)).
WithVersions("v1.33.0", "v1.34.0").
Build(),
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(*testing.T) {
Expand Down
30 changes: 30 additions & 0 deletions internal/webhooks/clusterclass.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ func (webhook *ClusterClass) validate(ctx context.Context, oldClusterClass, newC
return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("ClusterClass").GroupKind(), newClusterClass.Name, allErrs)
}

// Ensure New ClusterClass contains Kubernetes versions of all the Clusters of ClusterClass.
allErrs = append(allErrs,
webhook.validateKubernetesVersionsOfClusters(clusters, oldClusterClass, newClusterClass)...)

// Ensure no MachineDeploymentClass currently in use has been removed from the ClusterClass.
allErrs = append(allErrs,
webhook.validateRemovedMachineDeploymentClassesAreNotUsed(clusters, oldClusterClass, newClusterClass)...)
Expand Down Expand Up @@ -246,6 +250,32 @@ func validateUpdatesToMachineHealthCheckClasses(clusters []clusterv1.Cluster, ol
return allErrs
}

func (webhook *ClusterClass) validateKubernetesVersionsOfClusters(clusters []clusterv1.Cluster, _, newClusterClass *clusterv1.ClusterClass) field.ErrorList {
var allErrs field.ErrorList

// If there is no KubernetesVersions is set in the ClusterClass return early.
if len(newClusterClass.Spec.KubernetesVersions) == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asking my question regarding this point here

ClusterClass webhook: on create/update: ensure versions of all Clusters using that ClusterClass are included in ClusterClass versions (if CC has versions)

I am bit confused about this check (if CC has versions)*

For example we have a CC A with versions 1.32, 1.33, Then we create new CC B for rebase with no version. Since there are no KubernetesVersion set on CC B, this check makes it as valid.
But in this case what will the Cluster use, will it continue to remain in its version?

Copy link
Member

@sbueringer sbueringer Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.

The idea is to only check Cluster.spec.topology.version if a ClusterClass has kubernetesVersions defined.
As defining kubernetesVersions on the CC is entirely optional it is valid to use a CC without versions or rebase to one without versions.

We only want to make sure that if the CC has versions the Cluster uses one of them. Basically either you opt-into defining the versions in the CC and you get the corresponding features, or you don't and you get the same behavior as today

For example we have a CC A with versions 1.32, 1.33, Then we create new CC B for rebase with no version. Since there are no KubernetesVersion set on CC B, this check makes it as valid.

That's correct

But in this case what will the Cluster use, will it continue to remain in its version?

Yes. The Cluster will just continue to use the version it has.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, Thanks for clarifying.

return allErrs
}

kubernetesVersions := sets.Set[string]{}
for _, v := range newClusterClass.Spec.KubernetesVersions {
kubernetesVersions.Insert(v)
}

// Error if any Cluster's Kubernetes version is not set in the ClusterClass.
for _, c := range clusters {
if !kubernetesVersions.Has(c.Spec.Topology.Version) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "kubernetesVersions"),
fmt.Sprintf("Kubernetes Version %s is used by Cluster %q but not set in ClusterClass",
c.Spec.Topology.Version, c.Name),
))
}
}

return allErrs
}

func (webhook *ClusterClass) validateRemovedMachineDeploymentClassesAreNotUsed(clusters []clusterv1.Cluster, oldClusterClass, newClusterClass *clusterv1.ClusterClass) field.ErrorList {
var allErrs field.ErrorList

Expand Down
96 changes: 96 additions & 0 deletions internal/webhooks/clusterclass_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2489,6 +2489,102 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) {
Build(),
expectErr: false,
},
{
name: "pass if ClusterClass does not contain any kubernetes version",
clusters: []client.Object{
builder.Cluster(metav1.NamespaceDefault, "cluster1").
WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}).
WithTopology(
builder.ClusterTopology().
WithClass("class1").
WithVersion("v1.33.0").
Build()).
Build(),
},
oldClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
Build(),
newClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
Build(),
expectErr: false,
},
{
name: "pass if ClusterClass contains cluster's kubernetes version",
clusters: []client.Object{
builder.Cluster(metav1.NamespaceDefault, "cluster1").
WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}).
WithTopology(
builder.ClusterTopology().
WithClass("class1").
WithVersion("v1.33.0").
Build()).
Build(),
builder.Cluster(metav1.NamespaceDefault, "cluster2").
WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}).
WithTopology(
builder.ClusterTopology().
WithClass("class1").
WithVersion("v1.34.0").
Build()).
Build(),
},
oldClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
Build(),
newClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
WithVersions("v1.32.0", "v1.33.0", "v1.34.0").
Build(),
expectErr: false,
},
{
name: "fail if ClusterClass does not contains all of cluster's kubernetes version",
clusters: []client.Object{
builder.Cluster(metav1.NamespaceDefault, "cluster1").
WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}).
WithTopology(
builder.ClusterTopology().
WithClass("class1").
WithVersion("v1.33.0").
Build()).
Build(),
builder.Cluster(metav1.NamespaceDefault, "cluster2").
WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}).
WithTopology(
builder.ClusterTopology().
WithClass("class1").
WithVersion("v1.34.0").
Build()).
Build(),
},
oldClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
Build(),
newClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithInfrastructureClusterTemplate(
builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()).
WithControlPlaneTemplate(
builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1").Build()).
WithVersions("v1.32.0", "v1.33.0").
Build(),
expectErr: true,
},
}

for _, tt := range tests {
Expand Down