fix: nil check replicas ptr before de-referencing

This change does a nil check on the int32 ptr specifying the number of replica's inside the TalosControlPlane spec. Signed-off-by: Gerard de Leeuw <[email protected]> Co-authored-by: Artem Chernyshev <[email protected]>
siderolabs · Apr 17, 2023 · e9b6948 · e9b6948
1 parent b10e2e7
commit e9b6948
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 12 deletions.
diff --git a/api/v1alpha3/taloscontrolplane_types.go b/api/v1alpha3/taloscontrolplane_types.go
@@ -62,6 +62,16 @@ type TalosControlPlaneSpec struct {
 	RolloutStrategy *RolloutStrategy `json:"rolloutStrategy,omitempty"`
 }
 
+// GetReplicas reads spec replicas in a safe way.
+// If replicas is nil it will return 0.
+func (s *TalosControlPlaneSpec) GetReplicas() int32 {
+	if s.Replicas == nil {
+		return 0
+	}
+
+	return *s.Replicas
+}
+
 // RolloutStrategy describes how to replace existing machines
 // with new ones.
 type RolloutStrategy struct {

diff --git a/controllers/scale.go b/controllers/scale.go
@@ -24,11 +24,7 @@ import (
 
 func (r *TalosControlPlaneReconciler) scaleUpControlPlane(ctx context.Context, cluster *clusterv1.Cluster, tcp *controlplanev1.TalosControlPlane, controlPlane *ControlPlane) (ctrl.Result, error) {
 	numMachines := len(controlPlane.Machines)
-	desiredReplicas := 0
-
-	if tcp.Spec.Replicas != nil {
-		desiredReplicas = int(*tcp.Spec.Replicas)
-	}
+	desiredReplicas := tcp.Spec.GetReplicas()
 
 	conditions.MarkFalse(tcp, controlplanev1.ResizedCondition, controlplanev1.ScalingUpReason, clusterv1.ConditionSeverityWarning,
 		"Scaling up control plane to %d replicas (actual %d)",
@@ -48,11 +44,7 @@ func (r *TalosControlPlaneReconciler) scaleDownControlPlane(
 	machinesRequireUpgrade collections.Machines) (ctrl.Result, error) {
 
 	numMachines := len(controlPlane.Machines)
-	desiredReplicas := 0
-
-	if tcp.Spec.Replicas != nil {
-		desiredReplicas = int(*tcp.Spec.Replicas)
-	}
+	desiredReplicas := tcp.Spec.GetReplicas()
 
 	conditions.MarkFalse(tcp, controlplanev1.ResizedCondition, controlplanev1.ScalingDownReason, clusterv1.ConditionSeverityWarning,
 		"Scaling down control plane to %d replicas (actual %d)",

diff --git a/controllers/taloscontrolplane_controller.go b/controllers/taloscontrolplane_controller.go
@@ -726,7 +726,7 @@ func (r *TalosControlPlaneReconciler) reconcileMachines(ctx context.Context, clu
 
 	// If we've made it this far, we can assume that all ownedMachines are up to date
 	numMachines := len(machines.Items)
-	desiredReplicas := int(*tcp.Spec.Replicas)
+	desiredReplicas := int(tcp.Spec.GetReplicas())
 
 	controlPlane, err := newControlPlane(ctx, r.Client, cluster, tcp, collections.FromMachineList(machines))
 	if err != nil {

diff --git a/controllers/upgrade.go b/controllers/upgrade.go
@@ -32,7 +32,7 @@ func (r *TalosControlPlaneReconciler) upgradeControlPlane(
 	switch tcp.Spec.RolloutStrategy.Type {
 	case controlplanev1.RollingUpdateStrategyType:
 		// RolloutStrategy is currently defaulted and validated to be RollingUpdate
-		maxNodes := *tcp.Spec.Replicas + int32(tcp.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntValue())
+		maxNodes := tcp.Spec.GetReplicas() + int32(tcp.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntValue())
 		if int32(controlPlane.Machines.Len()) < maxNodes {
 			// scaleUp ensures that we don't continue scaling up while waiting for Machines to have NodeRefs
 			return r.scaleUpControlPlane(ctx, cluster, tcp, controlPlane)