Don't rely on GVK being set on regular objects

Signed-off-by: Stefan Büringer buringerst@vmware.com
kubernetes-sigs · Jan 10, 2024 · 06ebe94 · 06ebe94
1 parent 46dcda2
commit 06ebe94
Show file tree

Hide file tree

Showing 22 changed files with 88 additions and 77 deletions.
diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go
@@ -416,7 +416,7 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 	if scope.Config.Spec.InitConfiguration == nil {
 		scope.Config.Spec.InitConfiguration = &bootstrapv1.InitConfiguration{
 			TypeMeta: metav1.TypeMeta{
-				APIVersion: "kubeadm.k8s.io/v1beta1",
+				APIVersion: "kubeadm.k8s.io/v1beta3",
 				Kind:       "InitConfiguration",
 			},
 		}
@@ -431,7 +431,7 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 	if scope.Config.Spec.ClusterConfiguration == nil {
 		scope.Config.Spec.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{
 			TypeMeta: metav1.TypeMeta{
-				APIVersion: "kubeadm.k8s.io/v1beta1",
+				APIVersion: "kubeadm.k8s.io/v1beta3",
 				Kind:       "ClusterConfiguration",
 			},
 		}

diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go
@@ -142,8 +142,8 @@ func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testi
 
 	config := newKubeadmConfig(metav1.NamespaceDefault, "cfg")
 	config.SetOwnerReferences(util.EnsureOwnerRef(config.GetOwnerReferences(), metav1.OwnerReference{
-		APIVersion: machine.APIVersion,
-		Kind:       machine.Kind,
+		APIVersion: clusterv1.GroupVersion.String(),
+		Kind:       "Machine",
 		Name:       machine.Name,
 		UID:        machine.UID,
 	}))
@@ -207,8 +207,8 @@ func TestKubeadmConfigReconciler_TestSecretOwnerReferenceReconciliation(t *testi
 
 		actual.SetOwnerReferences([]metav1.OwnerReference{
 			{
-				APIVersion: machine.APIVersion,
-				Kind:       machine.Kind,
+				APIVersion: clusterv1.GroupVersion.String(),
+				Kind:       "Machine",
 				Name:       machine.Name,
 				UID:        machine.UID,
 				Controller: ptr.To(true),

diff --git a/bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go b/bootstrap/kubeadm/internal/locking/control_plane_init_mutex.go
@@ -181,8 +181,8 @@ func (s *semaphore) setMetadata(cluster *clusterv1.Cluster) {
 		},
 		OwnerReferences: []metav1.OwnerReference{
 			{
-				APIVersion: cluster.APIVersion,
-				Kind:       cluster.Kind,
+				APIVersion: clusterv1.GroupVersion.String(),
+				Kind:       "Cluster",
 				Name:       cluster.Name,
 				UID:        cluster.UID,
 			},

diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go
@@ -119,8 +119,8 @@ func (c *ClusterResourceSetBinding) DeleteBinding(clusterResourceSet *ClusterRes
 		}
 	}
 	c.OwnerReferences = removeOwnerRef(c.GetOwnerReferences(), metav1.OwnerReference{
-		APIVersion: clusterResourceSet.APIVersion,
-		Kind:       clusterResourceSet.Kind,
+		APIVersion: GroupVersion.String(),
+		Kind:       "ClusterResourceSet",
 		Name:       clusterResourceSet.Name,
 	})
 }

diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go
@@ -200,8 +200,8 @@ func (r *ClusterResourceSetReconciler) reconcileDelete(ctx context.Context, clus
 
 		clusterResourceSetBinding.RemoveBinding(crs)
 		clusterResourceSetBinding.OwnerReferences = util.RemoveOwnerRef(clusterResourceSetBinding.GetOwnerReferences(), metav1.OwnerReference{
-			APIVersion: crs.APIVersion,
-			Kind:       crs.Kind,
+			APIVersion: addonsv1.GroupVersion.String(),
+			Kind:       "ClusterResourceSet",
 			Name:       crs.Name,
 		})
 
@@ -296,7 +296,7 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 	// Ensure that the owner references are set on the ClusterResourceSetBinding.
 	clusterResourceSetBinding.SetOwnerReferences(util.EnsureOwnerRef(clusterResourceSetBinding.GetOwnerReferences(), metav1.OwnerReference{
 		APIVersion: addonsv1.GroupVersion.String(),
-		Kind:       clusterResourceSet.Kind,
+		Kind:       "ClusterResourceSet",
 		Name:       clusterResourceSet.Name,
 		UID:        clusterResourceSet.UID,
 	}))

diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go
@@ -225,7 +225,7 @@ func (r *MachinePoolReconciler) reconcile(ctx context.Context, cluster *clusterv
 	// Ensure the MachinePool is owned by the Cluster it belongs to.
 	mp.SetOwnerReferences(util.EnsureOwnerRef(mp.GetOwnerReferences(), metav1.OwnerReference{
 		APIVersion: clusterv1.GroupVersion.String(),
-		Kind:       cluster.Kind,
+		Kind:       "Cluster",
 		Name:       cluster.Name,
 		UID:        cluster.UID,
 	}))

diff --git a/exp/internal/controllers/machinepool_controller_noderef.go b/exp/internal/controllers/machinepool_controller_noderef.go
@@ -189,8 +189,8 @@ func (r *MachinePoolReconciler) getNodeReferences(ctx context.Context, c client.
 				ready++
 			}
 			nodeRefs = append(nodeRefs, corev1.ObjectReference{
-				Kind:       node.Kind,
-				APIVersion: node.APIVersion,
+				APIVersion: corev1.SchemeGroupVersion.String(),
+				Kind:       "Node",
 				Name:       node.Name,
 				UID:        node.UID,
 			})

diff --git a/internal/contract/references.go b/internal/contract/references.go
@@ -23,7 +23,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/klog/v2"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // Ref provide a helper struct for working with references in Unstructured objects.
@@ -88,7 +87,9 @@ func SetNestedRef(obj, refObj *unstructured.Unstructured, fields ...string) erro
 }
 
 // ObjToRef returns a reference to the given object.
-func ObjToRef(obj client.Object) *corev1.ObjectReference {
+// Note: This function only operates on Unstructured instead of client.Object
+// because it is only safe to assume for Unstructured that the GVK is set.
+func ObjToRef(obj *unstructured.Unstructured) *corev1.ObjectReference {
 	gvk := obj.GetObjectKind().GroupVersionKind()
 	return &corev1.ObjectReference{
 		Kind:       gvk.Kind,

diff --git a/internal/controllers/clusterclass/clusterclass_controller_test.go b/internal/controllers/clusterclass/clusterclass_controller_test.go
@@ -185,7 +185,7 @@ func assertInfrastructureClusterTemplate(ctx context.Context, actualClusterClass
 	if err := env.Get(ctx, actualInfraClusterTemplateKey, actualInfraClusterTemplate); err != nil {
 		return err
 	}
-	if err := assertHasOwnerReference(actualInfraClusterTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
+	if err := assertHasOwnerReference(actualInfraClusterTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
 		return err
 	}
 
@@ -205,7 +205,7 @@ func assertControlPlaneTemplate(ctx context.Context, actualClusterClass *cluster
 	if err := env.Get(ctx, actualControlPlaneTemplateKey, actualControlPlaneTemplate); err != nil {
 		return err
 	}
-	if err := assertHasOwnerReference(actualControlPlaneTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
+	if err := assertHasOwnerReference(actualControlPlaneTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
 		return err
 	}
 
@@ -226,7 +226,7 @@ func assertControlPlaneTemplate(ctx context.Context, actualClusterClass *cluster
 		if err := env.Get(ctx, actualInfrastructureMachineTemplateKey, actualInfrastructureMachineTemplate); err != nil {
 			return err
 		}
-		if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
+		if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
 			return err
 		}
 
@@ -260,7 +260,7 @@ func assertMachineDeploymentClass(ctx context.Context, actualClusterClass *clust
 	if err := env.Get(ctx, actualInfrastructureMachineTemplateKey, actualInfrastructureMachineTemplate); err != nil {
 		return err
 	}
-	if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
+	if err := assertHasOwnerReference(actualInfrastructureMachineTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
 		return err
 	}
 
@@ -280,7 +280,7 @@ func assertMachineDeploymentClass(ctx context.Context, actualClusterClass *clust
 	if err := env.Get(ctx, actualBootstrapTemplateKey, actualBootstrapTemplate); err != nil {
 		return err
 	}
-	if err := assertHasOwnerReference(actualBootstrapTemplate, *ownerReferenceTo(actualClusterClass)); err != nil {
+	if err := assertHasOwnerReference(actualBootstrapTemplate, *ownerReferenceTo(actualClusterClass, clusterv1.GroupVersion.WithKind("ClusterClass"))); err != nil {
 		return err
 	}
 

diff --git a/internal/controllers/clusterclass/suite_test.go b/internal/controllers/clusterclass/suite_test.go
@@ -90,12 +90,15 @@ func TestMain(m *testing.M) {
 	}))
 }
 
-func ownerReferenceTo(obj client.Object) *metav1.OwnerReference {
+// ownerReferenceTo converts an object to an OwnerReference.
+// Note: We pass in gvk explicitly as we can't rely on GVK being set on all objects
+// (only on Unstructured).
+func ownerReferenceTo(obj client.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference {
 	return &metav1.OwnerReference{
-		Kind:       obj.GetObjectKind().GroupVersionKind().Kind,
+		APIVersion: gvk.GroupVersion().String(),
+		Kind:       gvk.Kind,
 		Name:       obj.GetName(),
 		UID:        obj.GetUID(),
-		APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
 	}
 }
 

diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go
@@ -87,8 +87,8 @@ func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result,
 	// Set the Machine NodeRef.
 	if machine.Status.NodeRef == nil {
 		machine.Status.NodeRef = &corev1.ObjectReference{
-			Kind:       node.Kind,
-			APIVersion: node.APIVersion,
+			APIVersion: corev1.SchemeGroupVersion.String(),
+			Kind:       "Node",
 			Name:       node.Name,
 			UID:        node.UID,
 		}

diff --git a/internal/controllers/machinedeployment/suite_test.go b/internal/controllers/machinedeployment/suite_test.go
@@ -196,8 +196,8 @@ func fakeMachineNodeRef(m *clusterv1.Machine, pid string, g *WithT) {
 
 	patchMachine = client.MergeFrom(m.DeepCopy())
 	m.Status.NodeRef = &corev1.ObjectReference{
-		APIVersion: node.APIVersion,
-		Kind:       node.Kind,
+		APIVersion: corev1.SchemeGroupVersion.String(),
+		Kind:       "Node",
 		Name:       node.Name,
 	}
 	g.Expect(env.Status().Patch(ctx, m, patchMachine)).To(Succeed())

diff --git a/internal/controllers/machineset/suite_test.go b/internal/controllers/machineset/suite_test.go
@@ -205,8 +205,8 @@ func fakeMachineNodeRef(m *clusterv1.Machine, pid string, g *WithT) {
 
 	patchMachine = client.MergeFrom(m.DeepCopy())
 	m.Status.NodeRef = &corev1.ObjectReference{
-		APIVersion: node.APIVersion,
-		Kind:       node.Kind,
+		APIVersion: corev1.SchemeGroupVersion.String(),
+		Kind:       "Node",
 		Name:       node.Name,
 	}
 	g.Expect(env.Status().Patch(ctx, m, patchMachine)).To(Succeed())

diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go
@@ -208,7 +208,7 @@ func computeControlPlaneInfrastructureMachineTemplate(_ context.Context, s *scop
 		// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
 		// in case of errors in between creating this template and updating the Cluster object
 		// with the reference to the ControlPlane object using this template.
-		ownerRef: ownerReferenceTo(s.Current.Cluster),
+		ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
 	})
 }
 
@@ -623,7 +623,7 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
 		// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
 		// in case of errors in between creating this template and creating/updating the MachineDeployment object
 		// with the reference to this template.
-		ownerRef: ownerReferenceTo(s.Current.Cluster),
+		ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
 	})
 	if err != nil {
 		return nil, err
@@ -651,7 +651,7 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
 		// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
 		// in case of errors in between creating this template and creating/updating the MachineDeployment object
 		// with the reference to this template.
-		ownerRef: ownerReferenceTo(s.Current.Cluster),
+		ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
 	})
 	if err != nil {
 		return nil, err
@@ -719,8 +719,8 @@ func computeMachineDeployment(ctx context.Context, s *scope.Scope, machineDeploy
 
 	desiredMachineDeploymentObj := &clusterv1.MachineDeployment{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       clusterv1.GroupVersion.WithKind("MachineDeployment").Kind,
 			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "MachineDeployment",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -977,7 +977,7 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c
 		// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
 		// in case of errors in between creating this template and creating/updating the MachinePool object
 		// with the reference to this template.
-		ownerRef: ownerReferenceTo(s.Current.Cluster),
+		ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
 	})
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to compute bootstrap object for topology %q", machinePoolTopology.Name)
@@ -1005,7 +1005,7 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c
 		// Note: we are adding an ownerRef to Cluster so the template will be automatically garbage collected
 		// in case of errors in between creating this template and creating/updating the MachinePool object
 		// with the reference to this template.
-		ownerRef: ownerReferenceTo(s.Current.Cluster),
+		ownerRef: ownerReferenceTo(s.Current.Cluster, clusterv1.GroupVersion.WithKind("Cluster")),
 	})
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to compute infrastructure object for topology %q", machinePoolTopology.Name)
@@ -1068,8 +1068,8 @@ func computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology c
 
 	desiredMachinePoolObj := &expv1.MachinePool{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       expv1.GroupVersion.WithKind("MachinePool").Kind,
 			APIVersion: expv1.GroupVersion.String(),
+			Kind:       "MachinePool",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -1345,21 +1345,24 @@ func templateToTemplate(in templateToInput) (*unstructured.Unstructured, error)
 	return template, nil
 }
 
-func ownerReferenceTo(obj client.Object) *metav1.OwnerReference {
+// ownerReferenceTo converts an object to an OwnerReference.
+// Note: We pass in gvk explicitly as we can't rely on GVK being set on all objects
+// (only on Unstructured).
+func ownerReferenceTo(obj client.Object, gvk schema.GroupVersionKind) *metav1.OwnerReference {
 	return &metav1.OwnerReference{
-		Kind:       obj.GetObjectKind().GroupVersionKind().Kind,
+		APIVersion: gvk.GroupVersion().String(),
+		Kind:       gvk.Kind,
 		Name:       obj.GetName(),
 		UID:        obj.GetUID(),
-		APIVersion: obj.GetObjectKind().GroupVersionKind().GroupVersion().String(),
 	}
 }
 
 func computeMachineHealthCheck(ctx context.Context, healthCheckTarget client.Object, selector *metav1.LabelSelector, clusterName string, check *clusterv1.MachineHealthCheckClass) *clusterv1.MachineHealthCheck {
 	// Create a MachineHealthCheck with the spec given in the ClusterClass.
 	mhc := &clusterv1.MachineHealthCheck{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       clusterv1.GroupVersion.WithKind("MachineHealthCheck").Kind,
 			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "MachineHealthCheck",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      healthCheckTarget.GetName(),

diff --git a/internal/controllers/topology/cluster/desired_state_test.go b/internal/controllers/topology/cluster/desired_state_test.go
@@ -144,7 +144,7 @@ func TestComputeInfrastructureCluster(t *testing.T) {
 		// aggregating current cluster objects into ClusterState (simulating getCurrentState)
 		scope := scope.New(clusterWithInfrastructureRef)
 		scope.Current.InfrastructureCluster = infrastructureClusterTemplate.DeepCopy()
-		scope.Current.InfrastructureCluster.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim)})
+		scope.Current.InfrastructureCluster.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim, corev1.SchemeGroupVersion.WithKind("Secret"))})
 		scope.Blueprint = blueprint
 
 		obj, err := computeInfrastructureCluster(ctx, scope)
@@ -623,7 +623,7 @@ func TestComputeControlPlane(t *testing.T) {
 				}).
 				Build(),
 		}
-		s.Current.ControlPlane.Object.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim)})
+		s.Current.ControlPlane.Object.SetOwnerReferences([]metav1.OwnerReference{*ownerReferenceTo(shim, corev1.SchemeGroupVersion.WithKind("Secret"))})
 		s.Blueprint = blueprint
 
 		r := &Reconciler{}
@@ -2877,8 +2877,8 @@ func Test_computeMachineHealthCheck(t *testing.T) {
 	clusterName := "cluster1"
 	want := &clusterv1.MachineHealthCheck{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       clusterv1.GroupVersion.WithKind("MachineHealthCheck").Kind,
 			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "MachineHealthCheck",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "md1",