Merge pull request kubernetes-sigs#2303 from killianmuldoon/pr-fix-vs…

…pherevm-nil

🐛 Fix nil pointer error in retrieveVcenterSession

controllers/vspherevm_controller.go

@@ -28,6 +28,7 @@ import (
  apierrors "k8s.io/apimachinery/pkg/api/errors"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  apitypes "k8s.io/apimachinery/pkg/types"
+ "k8s.io/klog/v2"
  "k8s.io/utils/pointer"
  clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
  "sigs.k8s.io/cluster-api/controllers/remote"
@@ -569,6 +570,9 @@ func (r vmReconciler) retrieveVcenterSession(ctx goctx.Context, vsphereVM *infra
  params)
  }
 
+ if cluster.Spec.InfrastructureRef == nil {
+ return nil, errors.Errorf("cannot retrieve vCenter session for cluster %s: .spec.infrastructureRef is nil", klog.KObj(cluster))
+ }
  key := ctrlclient.ObjectKey{
  Namespace: cluster.Namespace,
  Name: cluster.Spec.InfrastructureRef.Name,

controllers/vspherevm_controller_test.go

@@ -420,18 +420,6 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) {
  },
  }
 
- cluster := &clusterv1.Cluster{
- ObjectMeta: metav1.ObjectMeta{
- Name: "valid-cluster",
- Namespace: "test",
- },
- Spec: clusterv1.ClusterSpec{
- InfrastructureRef: &corev1.ObjectReference{
- Name: vsphereCluster.Name,
- },
- },
- }
-
  machine := &clusterv1.Machine{
  ObjectMeta: metav1.ObjectMeta{
  Name: "foo",
@@ -442,8 +430,6 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) {
  },
  }
 
- initObjs := createMachineOwnerHierarchy(machine)
-
  vsphereMachine := &infrav1.VSphereMachine{
  ObjectMeta: metav1.ObjectMeta{
  Name: "foo-vm",
@@ -480,26 +466,71 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) {
  Status: infrav1.VSphereVMStatus{},
  }
 
- initObjs = append(initObjs, secret, vsphereVM, vsphereMachine, machine, cluster, vsphereCluster)
- controllerMgrContext := fake.NewControllerManagerContext(initObjs...)
+ t.Run("Retrieve credentials from cluster", func(t *testing.T) {
+ cluster := &clusterv1.Cluster{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "valid-cluster",
+ Namespace: "test",
+ },
+ Spec: clusterv1.ClusterSpec{
+ InfrastructureRef: &corev1.ObjectReference{
+ Name: vsphereCluster.Name,
+ },
+ },
+ }
 
- controllerContext := &context.ControllerContext{
- ControllerManagerContext: controllerMgrContext,
- Recorder: record.New(apirecord.NewFakeRecorder(100)),
- Logger: log.Log,
- }
- r := vmReconciler{ControllerContext: controllerContext}
-
- _, err = r.Reconcile(goctx.Background(), ctrl.Request{NamespacedName: util.ObjectKey(vsphereVM)})
- g := NewWithT(t)
- g.Expect(err).NotTo(HaveOccurred())
-
- vm := &infrav1.VSphereVM{}
- vmKey := util.ObjectKey(vsphereVM)
- g.Expect(r.Client.Get(goctx.Background(), vmKey, vm)).NotTo(HaveOccurred())
- g.Expect(conditions.Has(vm, infrav1.VCenterAvailableCondition)).To(BeTrue())
- vCenterCondition := conditions.Get(vm, infrav1.VCenterAvailableCondition)
- g.Expect(vCenterCondition.Status).To(Equal(corev1.ConditionTrue))
+ initObjs := createMachineOwnerHierarchy(machine)
+ initObjs = append(initObjs, secret, vsphereVM, vsphereMachine, machine, cluster, vsphereCluster)
+ controllerMgrContext := fake.NewControllerManagerContext(initObjs...)
+
+ controllerContext := &context.ControllerContext{
+ ControllerManagerContext: controllerMgrContext,
+ Recorder: record.New(apirecord.NewFakeRecorder(100)),
+ Logger: log.Log,
+ }
+ r := vmReconciler{ControllerContext: controllerContext}
+
+ _, err = r.Reconcile(goctx.Background(), ctrl.Request{NamespacedName: util.ObjectKey(vsphereVM)})
+ g := NewWithT(t)
+ g.Expect(err).NotTo(HaveOccurred())
+
+ vm := &infrav1.VSphereVM{}
+ vmKey := util.ObjectKey(vsphereVM)
+ g.Expect(r.Client.Get(goctx.Background(), vmKey, vm)).NotTo(HaveOccurred())
+ g.Expect(conditions.Has(vm, infrav1.VCenterAvailableCondition)).To(BeTrue())
+ vCenterCondition := conditions.Get(vm, infrav1.VCenterAvailableCondition)
+ g.Expect(vCenterCondition.Status).To(Equal(corev1.ConditionTrue))
+ },
+ )
+
+ t.Run("Error if cluster infrastructureRef is nil", func(t *testing.T) {
+ cluster := &clusterv1.Cluster{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "valid-cluster",
+ Namespace: "test",
+ },
+
+ // InfrastructureRef is nil so we should get an error.
+ Spec: clusterv1.ClusterSpec{
+ InfrastructureRef: nil,
+ },
+ }
+ initObjs := createMachineOwnerHierarchy(machine)
+ initObjs = append(initObjs, secret, vsphereVM, vsphereMachine, machine, cluster, vsphereCluster)
+ controllerMgrContext := fake.NewControllerManagerContext(initObjs...)
+
+ controllerContext := &context.ControllerContext{
+ ControllerManagerContext: controllerMgrContext,
+ Recorder: record.New(apirecord.NewFakeRecorder(100)),
+ Logger: log.Log,
+ }
+ r := vmReconciler{ControllerContext: controllerContext}
+
+ _, err = r.Reconcile(goctx.Background(), ctrl.Request{NamespacedName: util.ObjectKey(vsphereVM)})
+ g := NewWithT(t)
+ g.Expect(err).To(HaveOccurred())
+ },
+ )
 }
 
 func Test_reconcile(t *testing.T) {