Factor helpers out of checker

Author: dlipovetsky

pkg/webhook/preflight/nutanix/checker.go

@@ -5,25 +5,20 @@ package nutanix
 
 import (
 	"context"
-	"sync"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 
-	prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4"
-
-	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight"
+	preflightutil "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/util"
 )
 
 type Checker struct {
 	client  ctrlclient.Client
 	cluster *clusterv1.Cluster
 
-	nutanixClient *prismv4.Client
-
-	workerConfigGetterByMachineDeploymentName      map[string]func() (*variables.WorkerNodeConfigSpec, error)
-	workerConfigGetterByMachineDeploymentNameMutex sync.Mutex
+	clientGetter    *ClientGetter
+	variablesGetter *preflightutil.VariablesGetter
 }
 
 func (n *Checker) Init(
@@ -33,10 +28,10 @@ func (n *Checker) Init(
 ) []preflight.Check {
 	n.client = client
 	n.cluster = cluster
+	n.clientGetter = &ClientGetter{client: client, cluster: cluster}
+	n.variablesGetter = preflightutil.NewVariablesGetter(cluster)
 
-	checks := []preflight.Check{
+	return []preflight.Check{
 		n.VMImageCheck,
 	}
-
-	return checks
 }

pkg/webhook/preflight/nutanix/client.go

@@ -7,6 +7,8 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	prism "github.com/nutanix-cloud-native/prism-go-client"
 	prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials"
@@ -15,12 +17,18 @@ import (
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
 )
 
-// clientV4 creates a new Prism V4 client for the Nutanix cluster.
-// It retrieves the Prism Central credentials from the provided client and
-// uses them to create the client. The client is cached for future use.
-// The function returns an error if the credentials cannot be retrieved or
-// if the Prism Central endpoint cannot be parsed.
-func (n *Checker) clientV4(ctx context.Context, clusterConfig *variables.ClusterConfigSpec) (*prismv4.Client, error) {
+// ClientGetter provides methods to create Prism Central clients.
+// These methods are thread-safe and cache the results for efficiency.
+type ClientGetter struct {
+	client        ctrlclient.Client
+	cluster       *clusterv1.Cluster
+	nutanixClient *prismv4.Client
+}
+
+// V4 creates a new Prism V4 client for the Nutanix cluster using Prism Central credentials
+// referenced in the clusterConfig. The client is cached for future use. The function returns an
+// error if the credentials cannot be retrieved or if the Prism Central endpoint cannot be parsed.
+func (g *ClientGetter) V4(ctx context.Context, clusterConfig *variables.ClusterConfigSpec) (*prismv4.Client, error) {
 	return sync.OnceValues(func() (*prismv4.Client, error) {
 		if clusterConfig == nil || clusterConfig.Nutanix == nil {
 			return nil, fmt.Errorf("clusterConfig variable is nil or does not contain Nutanix config")
@@ -31,10 +39,10 @@ func (n *Checker) clientV4(ctx context.Context, clusterConfig *variables.Cluster
 		}
 
 		credentialsSecret := &corev1.Secret{}
-		if err := n.client.Get(
+		if err := g.client.Get(
 			ctx,
 			types.NamespacedName{
-				Namespace: n.cluster.Namespace,
+				Namespace: g.cluster.Namespace,
 				Name:      clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name,
 			},
 			credentialsSecret,
@@ -62,7 +70,7 @@ func (n *Checker) clientV4(ctx context.Context, clusterConfig *variables.Cluster
 		if err != nil {
 			return nil, fmt.Errorf("failed to create Prism V4 client: %w", err)
 		}
-		n.nutanixClient = nutanixClient
-		return n.nutanixClient, nil
+		g.nutanixClient = nutanixClient
+		return g.nutanixClient, nil
 	})()
 }

pkg/webhook/preflight/nutanix/image.go

@@ -21,7 +21,7 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult {
 	}
 
 	// Check control plane VM image.
-	clusterConfig, err := n.getClusterConfig()
+	clusterConfig, err := n.variablesGetter.ClusterConfig()
 	if err != nil {
 		result.Error = true
 		result.Causes = append(result.Causes, metav1.StatusCause{
@@ -47,7 +47,7 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult {
 
 	// Check worker VM images.
 	for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments {
-		workerConfig, err := n.getWorkerConfigForMachineDeployment(md)
+		workerConfig, err := n.variablesGetter.WorkerConfigForMachineDeployment(md)
 		if err != nil {
 			result.Error = true
 			result.Causes = append(result.Causes, metav1.StatusCause{
@@ -95,7 +95,7 @@ func (n *Checker) vmImageCheckForMachineDetails(
 	}
 
 	if details.Image != nil {
-		client, err := n.clientV4(ctx, clusterConfig)
+		client, err := n.clientGetter.V4(ctx, clusterConfig)
 		if err != nil {
 			result.Allowed = false
 			result.Error = true

pkg/webhook/preflight/nutanix/variables.go

@@ -0,0 +1,71 @@
+package nutanix
+
+import (
+	"fmt"
+	"sync"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
+)
+
+// VariablesGetter provides methods to retrieve variables from a Cluster object.
+// These methods are thread-safe and cache the results for efficiency.
+type VariablesGetter struct {
+	cluster                                        *clusterv1.Cluster
+	workerConfigGetterByMachineDeploymentName      map[string]func() (*variables.WorkerNodeConfigSpec, error)
+	workerConfigGetterByMachineDeploymentNameMutex sync.Mutex
+}
+
+func NewVariablesGetter(cluster *clusterv1.Cluster) *VariablesGetter {
+	return &VariablesGetter{
+		cluster: cluster,
+		workerConfigGetterByMachineDeploymentName: make(
+			map[string]func() (*variables.WorkerNodeConfigSpec, error),
+		),
+		workerConfigGetterByMachineDeploymentNameMutex: sync.Mutex{},
+	}
+}
+
+// ClusterConfig retrieves the cluster configuration variables from the Cluster object.
+// This method is thread-safe, and caches the result.
+func (g *VariablesGetter) ClusterConfig() (*variables.ClusterConfigSpec, error) {
+	return sync.OnceValues(func() (*variables.ClusterConfigSpec, error) {
+		if g.cluster.Spec.Topology.Variables == nil {
+			return nil, nil
+		}
+
+		clusterConfig, err := variables.UnmarshalClusterConfigVariable(g.cluster.Spec.Topology.Variables)
+		if err != nil {
+			return nil, fmt.Errorf("failed to unmarshal .variables[.name=clusterConfig]: %w", err)
+		}
+
+		return clusterConfig, nil
+	})()
+}
+
+// WorkerConfigForMachineDeployment retrieves the worker configuration variables for the given MachineDeployment.
+// This method is thread-safe, and caches the result.
+func (g *VariablesGetter) WorkerConfigForMachineDeployment(
+	md clusterv1.MachineDeploymentTopology,
+) (*variables.WorkerNodeConfigSpec, error) {
+	g.workerConfigGetterByMachineDeploymentNameMutex.Lock()
+	defer g.workerConfigGetterByMachineDeploymentNameMutex.Unlock()
+
+	fn, ok := g.workerConfigGetterByMachineDeploymentName[md.Name]
+	if !ok {
+		fn = sync.OnceValues(func() (*variables.WorkerNodeConfigSpec, error) {
+			if md.Variables == nil {
+				return nil, nil
+			}
+
+			workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides)
+			if err != nil {
+				return nil, fmt.Errorf("failed to unmarshal .variables.overrides[.name=workerConfig]: %w", err)
+			}
+
+			return workerConfig, nil
+		})
+	}
+	return fn()
+}