From a09f1049fdf69334d83dd69a9f69b46022896b05 Mon Sep 17 00:00:00 2001 From: adil ghaffar Date: Mon, 13 Jun 2022 17:58:31 +0300 Subject: [PATCH] Add checks for not topology owned templates to never reconcile. --- .../topology/cluster/current_state.go | 12 +++++++++ .../topology/cluster/current_state_test.go | 27 ++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/controllers/topology/cluster/current_state.go b/internal/controllers/topology/cluster/current_state.go index de8af7c4ee91..d1f48582337a 100644 --- a/internal/controllers/topology/cluster/current_state.go +++ b/internal/controllers/topology/cluster/current_state.go @@ -119,6 +119,12 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c if err != nil { return nil, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate for %s", tlog.KObj{Obj: res.Object}) } + // check that the referenced object has the ClusterTopologyOwnedLabel label. + // Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not + // owned by the topology. + if !labels.IsTopologyOwned(res.InfrastructureMachineTemplate) { + return nil, fmt.Errorf("referenced infra cluster object %s is not topology owned", tlog.KObj{Obj: res.InfrastructureMachineTemplate}) + } mhc := &clusterv1.MachineHealthCheck{} // MachineHealthCheck always has the same name and namespace as the ControlPlane object it belongs to. @@ -158,6 +164,12 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust for i := range md.Items { m := &md.Items[i] + // check that the MachineDeployment has the ClusterTopologyOwnedLabel label. + // Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not + // owned by the topology. + if !labels.IsTopologyOwned(m) { + return nil, fmt.Errorf(" %s is not topology owned", tlog.KObj{Obj: m}) + } // Retrieve the name which is assigned in Cluster's topology // from a well-defined label. mdTopologyName, ok := m.ObjectMeta.Labels[clusterv1.ClusterTopologyMachineDeploymentLabelName] diff --git a/internal/controllers/topology/cluster/current_state_test.go b/internal/controllers/topology/cluster/current_state_test.go index 3a102446feee..a0430ab1fbad 100644 --- a/internal/controllers/topology/cluster/current_state_test.go +++ b/internal/controllers/topology/cluster/current_state_test.go @@ -57,9 +57,15 @@ func TestGetCurrentState(t *testing.T) { // ControlPlane and ControlPlaneInfrastructureMachineTemplate objects. controlPlaneInfrastructureMachineTemplate := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "cpInfraTemplate"). Build() + controlPlaneInfrastructureMachineTemplate.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}) + controlPlaneInfrastructureMachineTemplateNotTopologyOwned := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "cpInfraTemplate"). + Build() controlPlaneTemplateWithInfrastructureMachine := builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cpTemplateWithInfra1"). WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate). Build() + controlPlaneTemplateWithInfrastructureMachineNotTopologyOwned := builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cpTemplateWithInfra1"). + WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned). + Build() controlPlane := builder.ControlPlane(metav1.NamespaceDefault, "cp1"). Build() controlPlane.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}) @@ -67,7 +73,9 @@ func TestGetCurrentState(t *testing.T) { WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate). Build() controlPlaneWithInfra.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}) - + controlPlaneWithInfraNotTopologyOwned := builder.ControlPlane(metav1.NamespaceDefault, "cp1"). + WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned). + Build() controlPlaneNotTopologyOwned := builder.ControlPlane(metav1.NamespaceDefault, "cp1"). Build() @@ -76,6 +84,10 @@ func TestGetCurrentState(t *testing.T) { WithControlPlaneTemplate(controlPlaneTemplateWithInfrastructureMachine). WithControlPlaneInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate). Build() + clusterClassWithControlPlaneInfraNotTopologyOwned := builder.ClusterClass(metav1.NamespaceDefault, "class1"). + WithControlPlaneTemplate(controlPlaneTemplateWithInfrastructureMachineNotTopologyOwned). + WithControlPlaneInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned). + Build() clusterClassWithNoControlPlaneInfra := builder.ClusterClass(metav1.NamespaceDefault, "class2"). Build() @@ -312,6 +324,19 @@ func TestGetCurrentState(t *testing.T) { "md1": {Object: machineDeployment, BootstrapTemplate: machineDeploymentBootstrap, InfrastructureMachineTemplate: machineDeploymentInfrastructure}}, }, }, + { + name: "Fails if the ControlPlane references an InfrastructureMachineTemplate that is not topology owned", + cluster: builder.Cluster(metav1.NamespaceDefault, "cluster1"). + // No InfrastructureCluster! + WithControlPlane(controlPlaneWithInfraNotTopologyOwned). + Build(), + class: clusterClassWithControlPlaneInfraNotTopologyOwned, + objects: []client.Object{ + controlPlaneWithInfraNotTopologyOwned, + controlPlaneInfrastructureMachineTemplateNotTopologyOwned, + }, + wantErr: true, + }, { name: "Fails if the ControlPlane references an InfrastructureMachineTemplate that does not exist", cluster: builder.Cluster(metav1.NamespaceDefault, "cluster1").