From 317ab8c552780e4b75183989a7491a3371aa5d12 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 19 May 2025 17:15:33 -0700 Subject: [PATCH 01/11] feat: Nutanix VM image preflight check --- cmd/main.go | 2 + pkg/webhook/preflight/nutanix/checker.go | 89 ++++++++++++++++++++++++ pkg/webhook/preflight/nutanix/client.go | 60 ++++++++++++++++ pkg/webhook/preflight/nutanix/image.go | 82 ++++++++++++++++++++++ 4 files changed, 233 insertions(+) create mode 100644 pkg/webhook/preflight/nutanix/checker.go create mode 100644 pkg/webhook/preflight/nutanix/client.go create mode 100644 pkg/webhook/preflight/nutanix/image.go diff --git a/cmd/main.go b/cmd/main.go index 48eaac2bf..140dc2667 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -42,6 +42,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/options" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/cluster" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" + preflightnutanix "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/nutanix" ) func main() { @@ -224,6 +225,7 @@ func main() { Handler: preflight.New(mgr.GetClient(), admission.NewDecoder(mgr.GetScheme()), []preflight.Checker{ // Add your preflight checkers here. + &preflightnutanix.Checker{}, }..., ), }) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go new file mode 100644 index 000000000..8e50c0c93 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -0,0 +1,89 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package nutanix + +import ( + "context" + "fmt" + + 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" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "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" +) + +type Checker struct { + client ctrlclient.Client + nutanixClient *prismv4.Client +} + +func (n *Checker) Init( + ctx context.Context, + client ctrlclient.Client, + cluster *clusterv1.Cluster, +) ([]preflight.Check, error) { + n.client = client + + clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal topology variable %q: %w", carenv1.ClusterConfigVariableName, err) + } + + if clusterConfig.Nutanix == nil { + return nil, fmt.Errorf("missing Nutanix configuration in cluster topology") + } + + // Initialize Nutanix client from the credentials referenced by the cluster configuration. + n.nutanixClient, err = newV4Client(ctx, client, cluster.Namespace, clusterConfig) + if err != nil { + return nil, fmt.Errorf("failed to create Nutanix client: %w", err) + } + + checks := []preflight.Check{} + if clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { + checks = append( + checks, + n.VMImageCheck( + &clusterConfig.ControlPlane.Nutanix.MachineDetails, + "controlPlane.nutanix.machineDetails", + ), + ) + } + + if cluster.Spec.Topology.Workers != nil { + for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { + if md.Variables == nil { + continue + } + + workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) + if err != nil { + return nil, fmt.Errorf( + "failed to unmarshal topology variable %q %d: %w", + carenv1.WorkerConfigVariableName, + i, + err, + ) + } + + if workerConfig.Nutanix == nil { + continue + } + + n.VMImageCheck( + &workerConfig.Nutanix.MachineDetails, + fmt.Sprintf( + "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", + md.Name, + ), + ) + } + } + + return checks, nil +} diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go new file mode 100644 index 000000000..ba88ae5f8 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/client.go @@ -0,0 +1,60 @@ +package nutanix + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + 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" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + + carenvariables "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" +) + +func newV4Client( + ctx context.Context, + client ctrlclient.Client, + clusterNamespace string, + clusterConfig *carenvariables.ClusterConfigSpec, +) ( + *prismv4.Client, + error, +) { + if clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { + return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") + } + + credentialsSecret := &corev1.Secret{} + if err := client.Get( + ctx, + types.NamespacedName{ + Namespace: clusterNamespace, + Name: clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name, + }, + credentialsSecret, + ); err != nil { + return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) + } + + // Get username and password + credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) + } + + host, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL() + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) + } + + return prismv4.NewV4Client(prism.Credentials{ + Endpoint: fmt.Sprintf("%s:%d", host, port), + Username: credentials.Username, + Password: credentials.Password, + Insecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure, + }) +} diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go new file mode 100644 index 000000000..a4531c411 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/image.go @@ -0,0 +1,82 @@ +package nutanix + +import ( + "context" + "fmt" + + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + + capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" +) + +func (n *Checker) VMImageCheck(details *carenv1.NutanixMachineDetails, field string) preflight.Check { + return func(ctx context.Context) preflight.CheckResult { + result := preflight.CheckResult{ + Allowed: true, + Field: field, + } + + if details.ImageLookup != nil { + result.Allowed = false + result.Message = "ImageLookup is not yet supported" + return result + } + + if details.Image != nil { + images, err := getVMImages(n.nutanixClient, details.Image) + if err != nil { + result.Allowed = false + result.Error = true + result.Message = fmt.Sprintf("failed to count matching VM Images: %s", err) + return result + } + + if len(images) != 1 { + result.Allowed = false + result.Message = fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)) + return result + } + } + + return result + } +} + +func getVMImages( + client *prismv4.Client, + id *capxv1.NutanixResourceIdentifier, +) ([]vmmv4.Image, error) { + switch { + case id.IsUUID(): + resp, err := client.ImagesApiInstance.GetImageById(id.UUID) + if err != nil { + return nil, err + } + image, ok := resp.GetData().(vmmv4.Image) + if !ok { + return nil, fmt.Errorf("failed to get data returned by GetImageById") + } + return []vmmv4.Image{image}, nil + case id.IsName(): + filter_ := fmt.Sprintf("name eq '%s'", *id.Name) + resp, err := client.ImagesApiInstance.ListImages(nil, nil, &filter_, nil, nil) + if err != nil { + return nil, err + } + if resp == nil || resp.GetData() == nil { + // No images were returned. + return []vmmv4.Image{}, nil + } + images, ok := resp.GetData().([]vmmv4.Image) + if !ok { + return nil, fmt.Errorf("failed to get data returned by ListImages") + } + return images, nil + default: + return nil, fmt.Errorf("image identifier is missing both name and uuid") + } +} From fc06a6c0a527b6fbed77c2a10d93bdf5ff18baee Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 21 May 2025 16:51:57 -0700 Subject: [PATCH 02/11] Implement single VM image check for control plane and workers --- go.mod | 2 +- pkg/webhook/preflight/nutanix/checker.go | 71 ++--------- pkg/webhook/preflight/nutanix/client.go | 28 +++-- pkg/webhook/preflight/nutanix/image.go | 154 +++++++++++++++++++---- 4 files changed, 162 insertions(+), 93 deletions(-) diff --git a/go.mod b/go.mod index d95063fc7..6ba5ed8db 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4 v4.0.1-beta.2 github.com/nutanix/ntnx-api-golang-clients/networking-go-client/v4 v4.0.2-beta.1 github.com/nutanix/ntnx-api-golang-clients/prism-go-client/v4 v4.0.1-beta.1 + github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 github.com/pkg/errors v0.9.1 @@ -111,7 +112,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/nutanix/ntnx-api-golang-clients/storage-go-client/v4 v4.0.2-alpha.3 // indirect - github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4 v4.0.1-beta.1 // indirect github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4 v4.0.1-beta.1 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0-rc5 // indirect diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 8e50c0c93..09a086e79 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -5,85 +5,34 @@ package nutanix import ( "context" - "fmt" + "sync" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" 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" - - carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" - "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" ) type Checker struct { client ctrlclient.Client nutanixClient *prismv4.Client + cluster *clusterv1.Cluster + + clientMutex sync.Mutex } func (n *Checker) Init( ctx context.Context, client ctrlclient.Client, cluster *clusterv1.Cluster, -) ([]preflight.Check, error) { +) []preflight.Check { n.client = client + n.cluster = cluster - clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal topology variable %q: %w", carenv1.ClusterConfigVariableName, err) - } - - if clusterConfig.Nutanix == nil { - return nil, fmt.Errorf("missing Nutanix configuration in cluster topology") - } - - // Initialize Nutanix client from the credentials referenced by the cluster configuration. - n.nutanixClient, err = newV4Client(ctx, client, cluster.Namespace, clusterConfig) - if err != nil { - return nil, fmt.Errorf("failed to create Nutanix client: %w", err) - } - - checks := []preflight.Check{} - if clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { - checks = append( - checks, - n.VMImageCheck( - &clusterConfig.ControlPlane.Nutanix.MachineDetails, - "controlPlane.nutanix.machineDetails", - ), - ) - } - - if cluster.Spec.Topology.Workers != nil { - for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { - if md.Variables == nil { - continue - } - - workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) - if err != nil { - return nil, fmt.Errorf( - "failed to unmarshal topology variable %q %d: %w", - carenv1.WorkerConfigVariableName, - i, - err, - ) - } - - if workerConfig.Nutanix == nil { - continue - } - - n.VMImageCheck( - &workerConfig.Nutanix.MachineDetails, - fmt.Sprintf( - "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", - md.Name, - ), - ) - } + checks := []preflight.Check{ + n.VMImageCheck, } - return checks, nil + return checks } diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go index ba88ae5f8..509dcd97a 100644 --- a/pkg/webhook/preflight/nutanix/client.go +++ b/pkg/webhook/preflight/nutanix/client.go @@ -4,26 +4,40 @@ import ( "context" "fmt" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" - 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" prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" - carenvariables "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" ) -func newV4Client( +func (n *Checker) v4client( ctx context.Context, client ctrlclient.Client, clusterNamespace string, - clusterConfig *carenvariables.ClusterConfigSpec, ) ( *prismv4.Client, error, ) { + n.clientMutex.Lock() + defer n.clientMutex.Unlock() + if n.nutanixClient != nil { + return n.nutanixClient, nil + } + + clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal cluster variable %q: %w", carenv1.ClusterConfigVariableName, err) + } + + if clusterConfig.Nutanix == nil { + return nil, fmt.Errorf("missing Nutanix configuration in cluster topology") + } + if clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index a4531c411..91ae116bd 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -4,45 +4,151 @@ import ( "context" "fmt" - vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "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" ) -func (n *Checker) VMImageCheck(details *carenv1.NutanixMachineDetails, field string) preflight.Check { - return func(ctx context.Context) preflight.CheckResult { - result := preflight.CheckResult{ - Allowed: true, - Field: field, +func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { + result := preflight.CheckResult{ + Allowed: true, + } + + // Check control plane VM image. + clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) + if err != nil { + result.Error = true + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: fmt.Sprintf( + "failed to unmarshal topology variable %q: %s", + carenv1.ClusterConfigVariableName, + err, + ), + Field: "cluster.spec.topology.variables", + }) + } else if clusterConfig != nil { + if clusterConfig.ControlPlane == nil || clusterConfig.ControlPlane.Nutanix == nil { + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: "missing Nutanix configuration in cluster topology", + Field: "cluster.spec.topology.controlPlane.nutanix", + }) } - if details.ImageLookup != nil { - result.Allowed = false - result.Message = "ImageLookup is not yet supported" - return result + n.vmImageCheckForMachineDetails( + ctx, + &clusterConfig.ControlPlane.Nutanix.MachineDetails, + "controlPlane.nutanix.machineDetails", + &result, + ) + } + + // Check worker VM images. + if n.cluster.Spec.Topology.Workers == nil { + return result + } + + for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { + if md.Variables == nil { + continue } - if details.Image != nil { - images, err := getVMImages(n.nutanixClient, details.Image) - if err != nil { - result.Allowed = false - result.Error = true - result.Message = fmt.Sprintf("failed to count matching VM Images: %s", err) - return result + workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) + if err != nil { + result.Error = true + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: fmt.Sprintf( + "failed to unmarshal topology variable %q: %s", + carenv1.WorkerConfigVariableName, + err, + ), + Field: fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s].variables.overrides", + md.Name, + ), + }) + } else if workerConfig != nil { + if workerConfig.Nutanix == nil { + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: "missing Nutanix configuration in worker machine deployment", + Field: fmt.Sprintf("cluster.spec.topology.workers.machineDeployments[.name=%s]"+ + ".variables.overrides[.name=workerConfig].value.nutanix", md.Name), + }) + } else { + n.vmImageCheckForMachineDetails( + ctx, + &workerConfig.Nutanix.MachineDetails, + fmt.Sprintf( + "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", + md.Name, + ), + &result, + ) } + } + } - if len(images) != 1 { - result.Allowed = false - result.Message = fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)) - return result - } + return result +} + +func (n *Checker) vmImageCheckForMachineDetails( + ctx context.Context, + details *carenv1.NutanixMachineDetails, + field string, + result *preflight.CheckResult, +) { + if details.ImageLookup != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: "ImageLookup is not yet supported", + Field: field, + }) + return + } + + if details.Image != nil { + client, err := n.v4client(ctx, n.client, n.cluster.Namespace) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: fmt.Sprintf("failed to get Nutanix client: %s", err), + Field: field, + }) + return } - return result + images, err := getVMImages(client, details.Image) + if err != nil { + result.Allowed = false + result.Error = true + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: fmt.Sprintf("failed to count matching VM Images: %s", err), + Field: field, + }) + return + } + + if len(images) != 1 { + result.Allowed = false + result.Causes = append(result.Causes, metav1.StatusCause{ + Type: "VMImageCheck", + Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), + Field: field, + }) + } } } From bd13c663cfcd52a3fb910451840773c151ed51d3 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 21 May 2025 20:59:29 -0700 Subject: [PATCH 03/11] Efficient, threadsafe data initialization --- pkg/webhook/preflight/nutanix/checker.go | 12 ++- pkg/webhook/preflight/nutanix/client.go | 104 ++++++++++----------- pkg/webhook/preflight/nutanix/image.go | 80 ++++++---------- pkg/webhook/preflight/nutanix/variables.go | 49 ++++++++++ 4 files changed, 135 insertions(+), 110 deletions(-) create mode 100644 pkg/webhook/preflight/nutanix/variables.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 09a086e79..419fbe09a 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -7,19 +7,23 @@ import ( "context" "sync" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" 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" ) type Checker struct { - client ctrlclient.Client + client ctrlclient.Client + cluster *clusterv1.Cluster + nutanixClient *prismv4.Client - cluster *clusterv1.Cluster - clientMutex sync.Mutex + workerConfigGetterByMachineDeploymentName map[string]func() (*variables.WorkerNodeConfigSpec, error) + workerConfigGetterByMachineDeploymentNameMutex sync.Mutex } func (n *Checker) Init( diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go index 509dcd97a..b60efeaff 100644 --- a/pkg/webhook/preflight/nutanix/client.go +++ b/pkg/webhook/preflight/nutanix/client.go @@ -3,72 +3,66 @@ package nutanix import ( "context" "fmt" + "sync" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" prism "github.com/nutanix-cloud-native/prism-go-client" prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" - ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" - carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" ) -func (n *Checker) v4client( - ctx context.Context, - client ctrlclient.Client, - clusterNamespace string, -) ( - *prismv4.Client, - error, -) { - n.clientMutex.Lock() - defer n.clientMutex.Unlock() - if n.nutanixClient != nil { - return n.nutanixClient, nil - } - - clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal cluster variable %q: %w", carenv1.ClusterConfigVariableName, err) - } +// 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) { + 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") + } - if clusterConfig.Nutanix == nil { - return nil, fmt.Errorf("missing Nutanix configuration in cluster topology") - } + if clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { + return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") + } - if clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { - return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") - } + credentialsSecret := &corev1.Secret{} + if err := n.client.Get( + ctx, + types.NamespacedName{ + Namespace: n.cluster.Namespace, + Name: clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name, + }, + credentialsSecret, + ); err != nil { + return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) + } - credentialsSecret := &corev1.Secret{} - if err := client.Get( - ctx, - types.NamespacedName{ - Namespace: clusterNamespace, - Name: clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name, - }, - credentialsSecret, - ); err != nil { - return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) - } + // Get username and password + credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) + } - // Get username and password - credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) - } + host, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL() + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) + } - host, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL() - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) - } - - return prismv4.NewV4Client(prism.Credentials{ - Endpoint: fmt.Sprintf("%s:%d", host, port), - Username: credentials.Username, - Password: credentials.Password, - Insecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure, - }) + nutanixClient, err := prismv4.NewV4Client(prism.Credentials{ + Endpoint: fmt.Sprintf("%s:%d", host, port), + Username: credentials.Username, + Password: credentials.Password, + Insecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure, + }) + if err != nil { + return nil, fmt.Errorf("failed to create Prism V4 client: %w", err) + } + n.nutanixClient = nutanixClient + return n.nutanixClient, nil + })() } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 91ae116bd..fe54a673b 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -4,10 +4,11 @@ import ( "context" "fmt" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" @@ -20,79 +21,55 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { } // Check control plane VM image. - clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) + clusterConfig, err := n.getClusterConfig() if err != nil { result.Error = true result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", - Message: fmt.Sprintf( - "failed to unmarshal topology variable %q: %s", - carenv1.ClusterConfigVariableName, - err, - ), - Field: "cluster.spec.topology.variables", + Type: "VMImageCheck", + Message: fmt.Sprintf("failed to read clusterConfig variable: %s", err), + Field: "cluster.spec.topology.variables", }) - } else if clusterConfig != nil { - if clusterConfig.ControlPlane == nil || clusterConfig.ControlPlane.Nutanix == nil { - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", - Message: "missing Nutanix configuration in cluster topology", - Field: "cluster.spec.topology.controlPlane.nutanix", - }) - } - + } + if clusterConfig != nil && clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { n.vmImageCheckForMachineDetails( ctx, + clusterConfig, &clusterConfig.ControlPlane.Nutanix.MachineDetails, - "controlPlane.nutanix.machineDetails", + "cluster.spec.topology.variables[.name=clusterConfig].controlPlane.nutanix.machineDetails", &result, ) } - // Check worker VM images. + // If there is no worker topology, return early. if n.cluster.Spec.Topology.Workers == nil { return result } + // Check worker VM images. for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { - if md.Variables == nil { - continue - } - - workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) + workerConfig, err := n.getWorkerConfigForMachineDeployment(md) if err != nil { result.Error = true result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", - Message: fmt.Sprintf( - "failed to unmarshal topology variable %q: %s", - carenv1.WorkerConfigVariableName, - err, - ), + Type: "VMImageCheck", + Message: fmt.Sprintf("failed to read workerConfig variable: %s", err), Field: fmt.Sprintf( "cluster.spec.topology.workers.machineDeployments[.name=%s].variables.overrides", md.Name, ), }) - } else if workerConfig != nil { - if workerConfig.Nutanix == nil { - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", - Message: "missing Nutanix configuration in worker machine deployment", - Field: fmt.Sprintf("cluster.spec.topology.workers.machineDeployments[.name=%s]"+ - ".variables.overrides[.name=workerConfig].value.nutanix", md.Name), - }) - } else { - n.vmImageCheckForMachineDetails( - ctx, - &workerConfig.Nutanix.MachineDetails, - fmt.Sprintf( - "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", - md.Name, - ), - &result, - ) - } + } + if workerConfig != nil && workerConfig.Nutanix != nil { + n.vmImageCheckForMachineDetails( + ctx, + clusterConfig, + &workerConfig.Nutanix.MachineDetails, + fmt.Sprintf( + "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", + md.Name, + ), + &result, + ) } } @@ -101,6 +78,7 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { func (n *Checker) vmImageCheckForMachineDetails( ctx context.Context, + clusterConfig *variables.ClusterConfigSpec, details *carenv1.NutanixMachineDetails, field string, result *preflight.CheckResult, @@ -117,7 +95,7 @@ func (n *Checker) vmImageCheckForMachineDetails( } if details.Image != nil { - client, err := n.v4client(ctx, n.client, n.cluster.Namespace) + client, err := n.clientV4(ctx, clusterConfig) if err != nil { result.Allowed = false result.Error = true diff --git a/pkg/webhook/preflight/nutanix/variables.go b/pkg/webhook/preflight/nutanix/variables.go new file mode 100644 index 000000000..834223347 --- /dev/null +++ b/pkg/webhook/preflight/nutanix/variables.go @@ -0,0 +1,49 @@ +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" +) + +func (n *Checker) getClusterConfig() (*variables.ClusterConfigSpec, error) { + return sync.OnceValues(func() (*variables.ClusterConfigSpec, error) { + if n.cluster.Spec.Topology.Variables == nil { + return nil, nil + } + + clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal .variables[.name=clusterConfig]: %w", err) + } + + return clusterConfig, nil + })() +} + +func (n *Checker) getWorkerConfigForMachineDeployment( + md clusterv1.MachineDeploymentTopology, +) (*variables.WorkerNodeConfigSpec, error) { + n.workerConfigGetterByMachineDeploymentNameMutex.Lock() + defer n.workerConfigGetterByMachineDeploymentNameMutex.Unlock() + + fn, ok := n.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() +} From aece8911dad68d9c44efa36c77d1e5f2e9cd2b17 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 22 May 2025 09:26:07 -0700 Subject: [PATCH 04/11] Factor helpers out of checker --- pkg/webhook/preflight/nutanix/checker.go | 17 ++---- pkg/webhook/preflight/nutanix/client.go | 28 ++++++--- pkg/webhook/preflight/nutanix/image.go | 6 +- pkg/webhook/preflight/nutanix/variables.go | 49 --------------- pkg/webhook/preflight/util/variables.go | 71 ++++++++++++++++++++++ 5 files changed, 98 insertions(+), 73 deletions(-) delete mode 100644 pkg/webhook/preflight/nutanix/variables.go create mode 100644 pkg/webhook/preflight/util/variables.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 419fbe09a..7634de511 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/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 } diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go index b60efeaff..564a21cd3 100644 --- a/pkg/webhook/preflight/nutanix/client.go +++ b/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 })() } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index fe54a673b..284240707 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/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 diff --git a/pkg/webhook/preflight/nutanix/variables.go b/pkg/webhook/preflight/nutanix/variables.go deleted file mode 100644 index 834223347..000000000 --- a/pkg/webhook/preflight/nutanix/variables.go +++ /dev/null @@ -1,49 +0,0 @@ -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" -) - -func (n *Checker) getClusterConfig() (*variables.ClusterConfigSpec, error) { - return sync.OnceValues(func() (*variables.ClusterConfigSpec, error) { - if n.cluster.Spec.Topology.Variables == nil { - return nil, nil - } - - clusterConfig, err := variables.UnmarshalClusterConfigVariable(n.cluster.Spec.Topology.Variables) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal .variables[.name=clusterConfig]: %w", err) - } - - return clusterConfig, nil - })() -} - -func (n *Checker) getWorkerConfigForMachineDeployment( - md clusterv1.MachineDeploymentTopology, -) (*variables.WorkerNodeConfigSpec, error) { - n.workerConfigGetterByMachineDeploymentNameMutex.Lock() - defer n.workerConfigGetterByMachineDeploymentNameMutex.Unlock() - - fn, ok := n.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() -} diff --git a/pkg/webhook/preflight/util/variables.go b/pkg/webhook/preflight/util/variables.go new file mode 100644 index 000000000..f16995040 --- /dev/null +++ b/pkg/webhook/preflight/util/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() +} From 171bbf0b12d892f4b384c1dca700cd2c8e5edf96 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 22 May 2025 16:18:15 -0700 Subject: [PATCH 05/11] Return preflight.Cause instead of metav1.StatusCause --- pkg/webhook/preflight/nutanix/checker.go | 3 +- pkg/webhook/preflight/nutanix/image.go | 77 +++++++++++------------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 7634de511..0633b3bc5 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -30,8 +30,7 @@ func (n *Checker) Init( n.cluster = cluster n.clientGetter = &ClientGetter{client: client, cluster: cluster} n.variablesGetter = preflightutil.NewVariablesGetter(cluster) - return []preflight.Check{ - n.VMImageCheck, + n.VMImages, } } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 284240707..c6d4a4b17 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -4,10 +4,8 @@ import ( "context" "fmt" - vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" @@ -15,8 +13,9 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { +func (n *Checker) VMImages(ctx context.Context) preflight.CheckResult { result := preflight.CheckResult{ + Name: "VMImages", Allowed: true, } @@ -24,8 +23,8 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { clusterConfig, err := n.variablesGetter.ClusterConfig() if err != nil { result.Error = true - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", + result.Allowed = false + result.Causes = append(result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to read clusterConfig variable: %s", err), Field: "cluster.spec.topology.variables", }) @@ -40,36 +39,32 @@ func (n *Checker) VMImageCheck(ctx context.Context) preflight.CheckResult { ) } - // If there is no worker topology, return early. - if n.cluster.Spec.Topology.Workers == nil { - return result - } - // Check worker VM images. - for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { - workerConfig, err := n.variablesGetter.WorkerConfigForMachineDeployment(md) - if err != nil { - result.Error = true - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", - Message: fmt.Sprintf("failed to read workerConfig variable: %s", err), - Field: fmt.Sprintf( - "cluster.spec.topology.workers.machineDeployments[.name=%s].variables.overrides", - md.Name, - ), - }) - } - if workerConfig != nil && workerConfig.Nutanix != nil { - n.vmImageCheckForMachineDetails( - ctx, - clusterConfig, - &workerConfig.Nutanix.MachineDetails, - fmt.Sprintf( - "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", - md.Name, - ), - &result, - ) + if n.cluster.Spec.Topology.Workers != nil { + for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { + workerConfig, err := n.variablesGetter.WorkerConfigForMachineDeployment(md) + if err != nil { + result.Error = true + result.Causes = append(result.Causes, preflight.Cause{ + Message: fmt.Sprintf("failed to read workerConfig variable: %s", err), + Field: fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s].variables.overrides", + md.Name, + ), + }) + } + if workerConfig != nil && workerConfig.Nutanix != nil { + n.vmImageCheckForMachineDetails( + ctx, + clusterConfig, + &workerConfig.Nutanix.MachineDetails, + fmt.Sprintf( + "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", + md.Name, + ), + &result, + ) + } } } @@ -86,8 +81,7 @@ func (n *Checker) vmImageCheckForMachineDetails( if details.ImageLookup != nil { result.Allowed = false result.Error = true - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", + result.Causes = append(result.Causes, preflight.Cause{ Message: "ImageLookup is not yet supported", Field: field, }) @@ -99,8 +93,7 @@ func (n *Checker) vmImageCheckForMachineDetails( if err != nil { result.Allowed = false result.Error = true - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", + result.Causes = append(result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to get Nutanix client: %s", err), Field: field, }) @@ -111,8 +104,7 @@ func (n *Checker) vmImageCheckForMachineDetails( if err != nil { result.Allowed = false result.Error = true - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", + result.Causes = append(result.Causes, preflight.Cause{ Message: fmt.Sprintf("failed to count matching VM Images: %s", err), Field: field, }) @@ -121,8 +113,7 @@ func (n *Checker) vmImageCheckForMachineDetails( if len(images) != 1 { result.Allowed = false - result.Causes = append(result.Causes, metav1.StatusCause{ - Type: "VMImageCheck", + result.Causes = append(result.Causes, preflight.Cause{ Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), Field: field, }) From 46a03bdec9d0e24474a3d73d7c5869bbcd91b05f Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Tue, 27 May 2025 16:45:37 -0700 Subject: [PATCH 06/11] Initialize Nutanix client in Init --- pkg/webhook/preflight/nutanix/checker.go | 47 +++++++++++- pkg/webhook/preflight/nutanix/client.go | 97 +++++++++++------------- pkg/webhook/preflight/nutanix/image.go | 20 +---- 3 files changed, 93 insertions(+), 71 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 0633b3bc5..be2d167d6 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -5,10 +5,13 @@ package nutanix import ( "context" + "fmt" 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/pkg/webhook/preflight" preflightutil "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight/util" ) @@ -17,7 +20,7 @@ type Checker struct { client ctrlclient.Client cluster *clusterv1.Cluster - clientGetter *ClientGetter + nutanixClient *prismv4.Client variablesGetter *preflightutil.VariablesGetter } @@ -28,8 +31,48 @@ func (n *Checker) Init( ) []preflight.Check { n.client = client n.cluster = cluster - n.clientGetter = &ClientGetter{client: client, cluster: cluster} n.variablesGetter = preflightutil.NewVariablesGetter(cluster) + + // Initialize the Nutanix client. If it fails, return a check that indicates the error. + clusterConfig, err := n.variablesGetter.ClusterConfig() + if err != nil { + return []preflight.Check{ + func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: "NutanixClientInitialization", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: fmt.Sprintf("failed to read clusterConfig variable: %s", err), + Field: "cluster.spec.topology.variables", + }, + }, + } + }, + } + } + + n.nutanixClient, err = v4client(ctx, client, cluster, clusterConfig.Nutanix) + // TODO Verify the credentials by making a users API call. + if err != nil { + return []preflight.Check{ + func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: "NutanixClientInitialization", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: fmt.Sprintf("failed to initialize Nutanix client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + }, + }, + } + }, + } + } + return []preflight.Check{ n.VMImages, } diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go index 564a21cd3..b9a423581 100644 --- a/pkg/webhook/preflight/nutanix/client.go +++ b/pkg/webhook/preflight/nutanix/client.go @@ -3,7 +3,6 @@ package nutanix import ( "context" "fmt" - "sync" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -14,63 +13,57 @@ import ( prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" - "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" ) -// 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 -} +func v4client(ctx context.Context, + client ctrlclient.Client, + cluster *clusterv1.Cluster, + nutanixSpec *carenv1.NutanixSpec, +) ( + *prismv4.Client, + error, +) { + if nutanixSpec == nil { + return nil, fmt.Errorf("nutanixSpec is nil") + } -// 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") - } + if nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { + return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") + } - if clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { - return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") - } + credentialsSecret := &corev1.Secret{} + if err := client.Get( + ctx, + types.NamespacedName{ + Namespace: cluster.Namespace, + Name: nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, + }, + credentialsSecret, + ); err != nil { + return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) + } - credentialsSecret := &corev1.Secret{} - if err := g.client.Get( - ctx, - types.NamespacedName{ - Namespace: g.cluster.Namespace, - Name: clusterConfig.Nutanix.PrismCentralEndpoint.Credentials.SecretRef.Name, - }, - credentialsSecret, - ); err != nil { - return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) - } + // Get username and password + credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) + } - // Get username and password - credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) - } + host, port, err := nutanixSpec.PrismCentralEndpoint.ParseURL() + if err != nil { + return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) + } - host, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL() - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) - } + nutanixClient, err := prismv4.NewV4Client(prism.Credentials{ + Endpoint: fmt.Sprintf("%s:%d", host, port), + Username: credentials.Username, + Password: credentials.Password, + Insecure: nutanixSpec.PrismCentralEndpoint.Insecure, + }) + if err != nil { + return nil, fmt.Errorf("failed to create Prism V4 client: %w", err) + } - nutanixClient, err := prismv4.NewV4Client(prism.Credentials{ - Endpoint: fmt.Sprintf("%s:%d", host, port), - Username: credentials.Username, - Password: credentials.Password, - Insecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure, - }) - if err != nil { - return nil, fmt.Errorf("failed to create Prism V4 client: %w", err) - } - g.nutanixClient = nutanixClient - return g.nutanixClient, nil - })() + return nutanixClient, nil } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index c6d4a4b17..381d1818a 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -4,12 +4,12 @@ import ( "context" "fmt" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + capxv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" - "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" ) @@ -32,7 +32,6 @@ func (n *Checker) VMImages(ctx context.Context) preflight.CheckResult { if clusterConfig != nil && clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { n.vmImageCheckForMachineDetails( ctx, - clusterConfig, &clusterConfig.ControlPlane.Nutanix.MachineDetails, "cluster.spec.topology.variables[.name=clusterConfig].controlPlane.nutanix.machineDetails", &result, @@ -56,7 +55,6 @@ func (n *Checker) VMImages(ctx context.Context) preflight.CheckResult { if workerConfig != nil && workerConfig.Nutanix != nil { n.vmImageCheckForMachineDetails( ctx, - clusterConfig, &workerConfig.Nutanix.MachineDetails, fmt.Sprintf( "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", @@ -73,7 +71,6 @@ func (n *Checker) VMImages(ctx context.Context) preflight.CheckResult { func (n *Checker) vmImageCheckForMachineDetails( ctx context.Context, - clusterConfig *variables.ClusterConfigSpec, details *carenv1.NutanixMachineDetails, field string, result *preflight.CheckResult, @@ -89,18 +86,7 @@ func (n *Checker) vmImageCheckForMachineDetails( } if details.Image != nil { - client, err := n.clientGetter.V4(ctx, clusterConfig) - if err != nil { - result.Allowed = false - result.Error = true - result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to get Nutanix client: %s", err), - Field: field, - }) - return - } - - images, err := getVMImages(client, details.Image) + images, err := getVMImages(n.nutanixClient, details.Image) if err != nil { result.Allowed = false result.Error = true From 0185a913ecf51bdd8f10899de3e55e34089aeb30 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 28 May 2025 16:19:25 -0700 Subject: [PATCH 07/11] Factor credentials out from client initialization --- pkg/webhook/preflight/nutanix/checker.go | 141 ++++++++++++++----- pkg/webhook/preflight/nutanix/client.go | 46 +----- pkg/webhook/preflight/nutanix/credentials.go | 61 ++++++++ pkg/webhook/preflight/nutanix/image.go | 86 +++-------- 4 files changed, 191 insertions(+), 143 deletions(-) create mode 100644 pkg/webhook/preflight/nutanix/credentials.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index be2d167d6..333f774f7 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -12,16 +12,18 @@ import ( prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "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 - variablesGetter *preflightutil.VariablesGetter + client ctrlclient.Client + cluster *clusterv1.Cluster + nutanixSpec *carenv1.NutanixSpec + nutanixControlPlaneNodeSpec *carenv1.NutanixNodeSpec + nutanixNodeSpecByMachineDeploymentName map[string]*carenv1.NutanixNodeSpec + v4client *prismv4.Client } func (n *Checker) Init( @@ -31,49 +33,114 @@ func (n *Checker) Init( ) []preflight.Check { n.client = client n.cluster = cluster - n.variablesGetter = preflightutil.NewVariablesGetter(cluster) - // Initialize the Nutanix client. If it fails, return a check that indicates the error. - clusterConfig, err := n.variablesGetter.ClusterConfig() + var err error + + n.nutanixSpec, n.nutanixControlPlaneNodeSpec, n.nutanixNodeSpecByMachineDeploymentName, err = getData(cluster) if err != nil { return []preflight.Check{ - func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: "NutanixClientInitialization", - Allowed: false, - Error: true, - Causes: []preflight.Cause{ - { - Message: fmt.Sprintf("failed to read clusterConfig variable: %s", err), - Field: "cluster.spec.topology.variables", - }, - }, - } - }, + errorCheck("TBD", "TBD"), } } - n.nutanixClient, err = v4client(ctx, client, cluster, clusterConfig.Nutanix) - // TODO Verify the credentials by making a users API call. + credentials, err := getCredentials(ctx, client, cluster, n.nutanixSpec) if err != nil { return []preflight.Check{ + errorCheck( + fmt.Sprintf("failed to get Nutanix credentials: %s", err), + "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", + ), + } + } + + n.v4client, err = v4client(credentials, n.nutanixSpec) + if err != nil { + return []preflight.Check{ + errorCheck( + fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), + "cluster.spec.topology.variables[.name=clusterConfig].nutanix", + ), + } + } + + // Initialize checks. + checks := []preflight.Check{} + if n.nutanixControlPlaneNodeSpec != nil { + checks = append(checks, + func(ctx context.Context) preflight.CheckResult { + return n.vmImageCheckForMachineDetails( + &n.nutanixControlPlaneNodeSpec.MachineDetails, + "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", + ) + }, + ) + } + for mdName, nodeSpec := range n.nutanixNodeSpecByMachineDeploymentName { + checks = append(checks, func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: "NutanixClientInitialization", - Allowed: false, - Error: true, - Causes: []preflight.Cause{ - { - Message: fmt.Sprintf("failed to initialize Nutanix client: %s", err), - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix", - }, - }, - } + return n.vmImageCheckForMachineDetails( + &nodeSpec.MachineDetails, + fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ + ".overrides[.name=workerConfig].value.nutanix.machineDetails", + mdName, + ), + ) + }, + ) + } + return checks +} + +func errorCheck(msg, field string) preflight.Check { + return func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: "Nutanix", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: msg, + Field: field, + }, }, } } +} + +func getData( + cluster *clusterv1.Cluster, +) (*carenv1.NutanixSpec, *carenv1.NutanixNodeSpec, map[string]*carenv1.NutanixNodeSpec, error) { + nutanixSpec := &carenv1.NutanixSpec{} + controlPlaneNutanixNodeSpec := &carenv1.NutanixNodeSpec{} + nutanixNodeSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixNodeSpec) - return []preflight.Check{ - n.VMImages, + clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) + if err != nil { + return nil, nil, nil, fmt.Errorf("failed to unmarshal .variables[.name=clusterConfig]: %w", err) } + if clusterConfig != nil && clusterConfig.Nutanix != nil { + nutanixSpec = clusterConfig.Nutanix + } + + if clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { + controlPlaneNutanixNodeSpec = clusterConfig.ControlPlane.Nutanix + } + + if cluster.Spec.Topology.Workers != nil { + for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { + workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) + if err != nil { + return nil, nil, nil, fmt.Errorf( + "failed to unmarshal .variables[.name=workerConfig] for machine deployment %s: %w", + md.Name, err, + ) + } + if workerConfig != nil && workerConfig.Nutanix != nil { + nutanixNodeSpecByMachineDeploymentName[md.Name] = workerConfig.Nutanix + } + } + } + + return nutanixSpec, controlPlaneNutanixNodeSpec, nutanixNodeSpecByMachineDeploymentName, nil } diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go index b9a423581..9ccdb30c3 100644 --- a/pkg/webhook/preflight/nutanix/client.go +++ b/pkg/webhook/preflight/nutanix/client.go @@ -1,69 +1,31 @@ package nutanix import ( - "context" "fmt" - 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" + prismtypes "github.com/nutanix-cloud-native/prism-go-client/environment/types" prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" ) -func v4client(ctx context.Context, - client ctrlclient.Client, - cluster *clusterv1.Cluster, +func v4client( + credentials *prismtypes.ApiCredentials, nutanixSpec *carenv1.NutanixSpec, ) ( *prismv4.Client, error, ) { - if nutanixSpec == nil { - return nil, fmt.Errorf("nutanixSpec is nil") - } - - if nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { - return nil, fmt.Errorf("prism Central credentials reference SecretRef.Name has no value") - } - - credentialsSecret := &corev1.Secret{} - if err := client.Get( - ctx, - types.NamespacedName{ - Namespace: cluster.Namespace, - Name: nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, - }, - credentialsSecret, - ); err != nil { - return nil, fmt.Errorf("failed to get Prism Central credentials Secret: %w", err) - } - - // Get username and password - credentials, err := prismcredentials.ParseCredentials(credentialsSecret.Data["credentials"]) - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central credentials from Secret: %w", err) - } - host, port, err := nutanixSpec.PrismCentralEndpoint.ParseURL() if err != nil { return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) } - nutanixClient, err := prismv4.NewV4Client(prism.Credentials{ + return prismv4.NewV4Client(prism.Credentials{ Endpoint: fmt.Sprintf("%s:%d", host, port), Username: credentials.Username, Password: credentials.Password, Insecure: nutanixSpec.PrismCentralEndpoint.Insecure, }) - if err != nil { - return nil, fmt.Errorf("failed to create Prism V4 client: %w", err) - } - - return nutanixClient, nil } diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go new file mode 100644 index 000000000..0f6267a8f --- /dev/null +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -0,0 +1,61 @@ +package nutanix + +import ( + "context" + "fmt" + + 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" + + prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" + prismtypes "github.com/nutanix-cloud-native/prism-go-client/environment/types" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" +) + +const credentialsSecretDataKey = "credentials" + +func getCredentials( + ctx context.Context, + client ctrlclient.Client, + cluster *clusterv1.Cluster, + nutanixSpec *carenv1.NutanixSpec, +) (*prismtypes.ApiCredentials, error) { + if nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { + return nil, fmt.Errorf("secretRef.name has no value") + } + + credentialsSecret := &corev1.Secret{} + if err := client.Get( + ctx, + types.NamespacedName{ + Namespace: cluster.Namespace, + Name: nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, + }, + credentialsSecret, + ); err != nil { + return nil, fmt.Errorf("failed to get Secret: %w", err) + } + + if len(credentialsSecret.Data) == 0 { + return nil, fmt.Errorf( + "the Secret %s/%s has no data", + cluster.Namespace, + nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, + ) + } + + data, ok := credentialsSecret.Data[credentialsSecretDataKey] + if !ok { + return nil, fmt.Errorf( + "the Secret %s/%s has no data for key %s", + cluster.Namespace, + nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, + credentialsSecretDataKey, + ) + } + // Get username and password + return prismcredentials.ParseCredentials(data) +} diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 381d1818a..451649ede 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -1,7 +1,6 @@ package nutanix import ( - "context" "fmt" vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" @@ -13,68 +12,14 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func (n *Checker) VMImages(ctx context.Context) preflight.CheckResult { - result := preflight.CheckResult{ - Name: "VMImages", - Allowed: true, - } - - // Check control plane VM image. - clusterConfig, err := n.variablesGetter.ClusterConfig() - if err != nil { - result.Error = true - result.Allowed = false - result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to read clusterConfig variable: %s", err), - Field: "cluster.spec.topology.variables", - }) - } - if clusterConfig != nil && clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { - n.vmImageCheckForMachineDetails( - ctx, - &clusterConfig.ControlPlane.Nutanix.MachineDetails, - "cluster.spec.topology.variables[.name=clusterConfig].controlPlane.nutanix.machineDetails", - &result, - ) - } - - // Check worker VM images. - if n.cluster.Spec.Topology.Workers != nil { - for _, md := range n.cluster.Spec.Topology.Workers.MachineDeployments { - workerConfig, err := n.variablesGetter.WorkerConfigForMachineDeployment(md) - if err != nil { - result.Error = true - result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to read workerConfig variable: %s", err), - Field: fmt.Sprintf( - "cluster.spec.topology.workers.machineDeployments[.name=%s].variables.overrides", - md.Name, - ), - }) - } - if workerConfig != nil && workerConfig.Nutanix != nil { - n.vmImageCheckForMachineDetails( - ctx, - &workerConfig.Nutanix.MachineDetails, - fmt.Sprintf( - "workers.machineDeployments[.name=%s].variables.overrides[.name=workerConfig].value.nutanix.machineDetails", - md.Name, - ), - &result, - ) - } - } - } - - return result -} - func (n *Checker) vmImageCheckForMachineDetails( - ctx context.Context, details *carenv1.NutanixMachineDetails, field string, - result *preflight.CheckResult, -) { +) preflight.CheckResult { + result := preflight.CheckResult{ + Name: "VMImage", + Allowed: false, + } if details.ImageLookup != nil { result.Allowed = false result.Error = true @@ -82,19 +27,24 @@ func (n *Checker) vmImageCheckForMachineDetails( Message: "ImageLookup is not yet supported", Field: field, }) - return + return result } if details.Image != nil { - images, err := getVMImages(n.nutanixClient, details.Image) + imagesCh := make(chan []vmmv4.Image) + defer close(imagesCh) + errCh := make(chan error) + defer close(errCh) + + images, err := getVMImages(n.v4client, details.Image) if err != nil { result.Allowed = false result.Error = true result.Causes = append(result.Causes, preflight.Cause{ - Message: fmt.Sprintf("failed to count matching VM Images: %s", err), + Message: fmt.Sprintf("failed to get VM Image: %s", err), Field: field, }) - return + return result } if len(images) != 1 { @@ -103,8 +53,16 @@ func (n *Checker) vmImageCheckForMachineDetails( Message: fmt.Sprintf("expected to find 1 VM Image, found %d", len(images)), Field: field, }) + return result } + + // Found exactly one image. + result.Allowed = true + return result } + + // Neither ImageLookup nor Image is specified. + return result } func getVMImages( From 47322d006749649a5684488ede36b90fa710c5cf Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 29 May 2025 11:53:53 -0700 Subject: [PATCH 08/11] Refactor error handling to report relevant fields --- pkg/webhook/preflight/nutanix/checker.go | 152 ++++++------------- pkg/webhook/preflight/nutanix/client.go | 31 ---- pkg/webhook/preflight/nutanix/credentials.go | 86 ++++++++--- pkg/webhook/preflight/nutanix/image.go | 36 ++++- pkg/webhook/preflight/nutanix/specs.go | 58 +++++++ 5 files changed, 208 insertions(+), 155 deletions(-) delete mode 100644 pkg/webhook/preflight/nutanix/client.go create mode 100644 pkg/webhook/preflight/nutanix/specs.go diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index 333f774f7..b6cce9d10 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -12,135 +12,83 @@ import ( prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" - carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" - "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" ) -type Checker struct { - client ctrlclient.Client - cluster *clusterv1.Cluster - nutanixSpec *carenv1.NutanixSpec - nutanixControlPlaneNodeSpec *carenv1.NutanixNodeSpec - nutanixNodeSpecByMachineDeploymentName map[string]*carenv1.NutanixNodeSpec - v4client *prismv4.Client -} +type Checker struct{} func (n *Checker) Init( ctx context.Context, - client ctrlclient.Client, + kclient ctrlclient.Client, cluster *clusterv1.Cluster, ) []preflight.Check { - n.client = client - n.cluster = cluster - - var err error + prismCentralEndpointSpec, + controlPlaneNutanixNodeSpec, + nutanixNodeSpecByMachineDeploymentName, + errCauses := readNutanixSpecs(cluster) + if len(errCauses) > 0 { + return initErrorCheck(errCauses...) + } - n.nutanixSpec, n.nutanixControlPlaneNodeSpec, n.nutanixNodeSpecByMachineDeploymentName, err = getData(cluster) - if err != nil { - return []preflight.Check{ - errorCheck("TBD", "TBD"), - } + if controlPlaneNutanixNodeSpec == nil && len(nutanixNodeSpecByMachineDeploymentName) == 0 { + // No Nutanix specs found, no checks to run. + return nil } - credentials, err := getCredentials(ctx, client, cluster, n.nutanixSpec) - if err != nil { - return []preflight.Check{ - errorCheck( - fmt.Sprintf("failed to get Nutanix credentials: %s", err), - "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials", - ), - } + // At least one Nutanix spec is defined. Get credentials and create a client, + // because all checks require them. + credentials, errCauses := getCredentials(ctx, kclient, cluster, prismCentralEndpointSpec) + if len(errCauses) > 0 { + return initErrorCheck(errCauses...) } - n.v4client, err = v4client(credentials, n.nutanixSpec) + nv4client, err := prismv4.NewV4Client(*credentials) if err != nil { - return []preflight.Check{ - errorCheck( - fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), - "cluster.spec.topology.variables[.name=clusterConfig].nutanix", - ), - } + return initErrorCheck( + preflight.Cause{ + Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), + Field: "", + }) } // Initialize checks. checks := []preflight.Check{} - if n.nutanixControlPlaneNodeSpec != nil { + if controlPlaneNutanixNodeSpec != nil { checks = append(checks, - func(ctx context.Context) preflight.CheckResult { - return n.vmImageCheckForMachineDetails( - &n.nutanixControlPlaneNodeSpec.MachineDetails, - "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix.machineDetails", - ) - }, + vmImageCheck( + nv4client, + controlPlaneNutanixNodeSpec, + "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix", + ), ) } - for mdName, nodeSpec := range n.nutanixNodeSpecByMachineDeploymentName { + for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { + if nutanixNodeSpecByMachineDeploymentName[md.Name] == nil { + continue + } checks = append(checks, - func(ctx context.Context) preflight.CheckResult { - return n.vmImageCheckForMachineDetails( - &nodeSpec.MachineDetails, - fmt.Sprintf( - "cluster.spec.topology.workers.machineDeployments[.name=%s]"+ - ".overrides[.name=workerConfig].value.nutanix.machineDetails", - mdName, - ), - ) - }, + vmImageCheck( + nv4client, + nutanixNodeSpecByMachineDeploymentName[md.Name], + fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s].variables[.name=workerConfig].value.nutanix", + md.Name, + ), + ), ) } return checks } -func errorCheck(msg, field string) preflight.Check { - return func(ctx context.Context) preflight.CheckResult { - return preflight.CheckResult{ - Name: "Nutanix", - Allowed: false, - Error: true, - Causes: []preflight.Cause{ - { - Message: msg, - Field: field, - }, - }, - } - } -} - -func getData( - cluster *clusterv1.Cluster, -) (*carenv1.NutanixSpec, *carenv1.NutanixNodeSpec, map[string]*carenv1.NutanixNodeSpec, error) { - nutanixSpec := &carenv1.NutanixSpec{} - controlPlaneNutanixNodeSpec := &carenv1.NutanixNodeSpec{} - nutanixNodeSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixNodeSpec) - - clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to unmarshal .variables[.name=clusterConfig]: %w", err) - } - if clusterConfig != nil && clusterConfig.Nutanix != nil { - nutanixSpec = clusterConfig.Nutanix - } - - if clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { - controlPlaneNutanixNodeSpec = clusterConfig.ControlPlane.Nutanix - } - - if cluster.Spec.Topology.Workers != nil { - for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { - workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) - if err != nil { - return nil, nil, nil, fmt.Errorf( - "failed to unmarshal .variables[.name=workerConfig] for machine deployment %s: %w", - md.Name, err, - ) +func initErrorCheck(causes ...preflight.Cause) []preflight.Check { + return []preflight.Check{ + func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: "Nutanix", + Allowed: false, + Error: true, + Causes: causes, } - if workerConfig != nil && workerConfig.Nutanix != nil { - nutanixNodeSpecByMachineDeploymentName[md.Name] = workerConfig.Nutanix - } - } + }, } - - return nutanixSpec, controlPlaneNutanixNodeSpec, nutanixNodeSpecByMachineDeploymentName, nil } diff --git a/pkg/webhook/preflight/nutanix/client.go b/pkg/webhook/preflight/nutanix/client.go deleted file mode 100644 index 9ccdb30c3..000000000 --- a/pkg/webhook/preflight/nutanix/client.go +++ /dev/null @@ -1,31 +0,0 @@ -package nutanix - -import ( - "fmt" - - prism "github.com/nutanix-cloud-native/prism-go-client" - prismtypes "github.com/nutanix-cloud-native/prism-go-client/environment/types" - prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" - - carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" -) - -func v4client( - credentials *prismtypes.ApiCredentials, - nutanixSpec *carenv1.NutanixSpec, -) ( - *prismv4.Client, - error, -) { - host, port, err := nutanixSpec.PrismCentralEndpoint.ParseURL() - if err != nil { - return nil, fmt.Errorf("failed to parse Prism Central endpoint: %w", err) - } - - return prismv4.NewV4Client(prism.Credentials{ - Endpoint: fmt.Sprintf("%s:%d", host, port), - Username: credentials.Username, - Password: credentials.Password, - Insecure: nutanixSpec.PrismCentralEndpoint.Insecure, - }) -} diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index 0f6267a8f..b1b87a269 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -9,10 +9,11 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + prismgoclient "github.com/nutanix-cloud-native/prism-go-client" prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" - prismtypes "github.com/nutanix-cloud-native/prism-go-client/environment/types" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) const credentialsSecretDataKey = "credentials" @@ -21,10 +22,24 @@ func getCredentials( ctx context.Context, client ctrlclient.Client, cluster *clusterv1.Cluster, - nutanixSpec *carenv1.NutanixSpec, -) (*prismtypes.ApiCredentials, error) { - if nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name == "" { - return nil, fmt.Errorf("secretRef.name has no value") + prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec, +) (*prismgoclient.Credentials, []preflight.Cause) { + if prismCentralEndpointSpec == nil { + return nil, []preflight.Cause{ + { + Message: "Prism Central endpoint specification is missing", + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint", + }, + } + } + + if prismCentralEndpointSpec.Credentials.SecretRef.Name == "" { + return nil, []preflight.Cause{ + { + Message: "Prism Central credentials secret reference is missing the name", + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef.name", + }, + } } credentialsSecret := &corev1.Secret{} @@ -32,30 +47,61 @@ func getCredentials( ctx, types.NamespacedName{ Namespace: cluster.Namespace, - Name: nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, + Name: prismCentralEndpointSpec.Credentials.SecretRef.Name, }, credentialsSecret, ); err != nil { - return nil, fmt.Errorf("failed to get Secret: %w", err) + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("failed to get the credentials Secret: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + } } if len(credentialsSecret.Data) == 0 { - return nil, fmt.Errorf( - "the Secret %s/%s has no data", - cluster.Namespace, - nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, - ) + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("credentials Secret has no data"), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + } } data, ok := credentialsSecret.Data[credentialsSecretDataKey] if !ok { - return nil, fmt.Errorf( - "the Secret %s/%s has no data for key %s", - cluster.Namespace, - nutanixSpec.PrismCentralEndpoint.Credentials.SecretRef.Name, - credentialsSecretDataKey, - ) + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("credentials Secret data is missing the key '%s'", credentialsSecretDataKey), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + } + } + + usernamePassword, err := prismcredentials.ParseCredentials(data) + if err != nil { + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("failed to parse credentials from Secret: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", + }, + } } - // Get username and password - return prismcredentials.ParseCredentials(data) + + host, port, err := prismCentralEndpointSpec.ParseURL() + if err != nil { + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("failed to parse Prism Central URL: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.url", + }, + } + } + + return &prismgoclient.Credentials{ + Endpoint: fmt.Sprintf("%s:%d", host, port), + Username: usernamePassword.Username, + Password: usernamePassword.Password, + Insecure: prismCentralEndpointSpec.Insecure, + }, nil } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 451649ede..7ea9ce2fd 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -1,6 +1,7 @@ package nutanix import ( + "context" "fmt" vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content" @@ -12,7 +13,38 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func (n *Checker) vmImageCheckForMachineDetails( +func vmImageCheck( + client *prismv4.Client, + nutanixNodeSpec *carenv1.NutanixNodeSpec, + nutanixNodeSpecField string, +) preflight.Check { + if nutanixNodeSpec == nil { + return func(ctx context.Context) preflight.CheckResult { + return preflight.CheckResult{ + Name: "VMImage", + Allowed: false, + Error: true, + Causes: []preflight.Cause{ + { + Message: fmt.Sprintf("NutanixNodeSpec is missing"), + Field: nutanixNodeSpecField, + }, + }, + } + } + } + + return func(ctx context.Context) preflight.CheckResult { + return vmImageCheckForMachineDetails( + client, + &nutanixNodeSpec.MachineDetails, + fmt.Sprintf("%s.machineDetails", nutanixNodeSpecField), + ) + } +} + +func vmImageCheckForMachineDetails( + client *prismv4.Client, details *carenv1.NutanixMachineDetails, field string, ) preflight.CheckResult { @@ -36,7 +68,7 @@ func (n *Checker) vmImageCheckForMachineDetails( errCh := make(chan error) defer close(errCh) - images, err := getVMImages(n.v4client, details.Image) + images, err := getVMImages(client, details.Image) if err != nil { result.Allowed = false result.Error = true diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go new file mode 100644 index 000000000..0dff4199c --- /dev/null +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -0,0 +1,58 @@ +package nutanix + +import ( + "fmt" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + + carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" + "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" +) + +func readNutanixSpecs( + cluster *clusterv1.Cluster, +) (*carenv1.NutanixPrismCentralEndpointSpec, *carenv1.NutanixNodeSpec, map[string]*carenv1.NutanixNodeSpec, []preflight.Cause) { + var prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec + var controlPlaneNutanixNodeSpec *carenv1.NutanixNodeSpec + nutanixNodeSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixNodeSpec) + + clusterConfig, err := variables.UnmarshalClusterConfigVariable(cluster.Spec.Topology.Variables) + if err != nil { + return nil, nil, nil, []preflight.Cause{ + { + Message: err.Error(), + Field: "cluster.spec.topology.variables[.name=clusterConfig]", + }, + } + } + + if clusterConfig != nil && clusterConfig.Nutanix != nil { + prismCentralEndpointSpec = &clusterConfig.Nutanix.PrismCentralEndpoint + } + + if clusterConfig.ControlPlane != nil && clusterConfig.ControlPlane.Nutanix != nil { + controlPlaneNutanixNodeSpec = clusterConfig.ControlPlane.Nutanix + } + + causes := []preflight.Cause{} + if cluster.Spec.Topology.Workers != nil { + for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { + workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides) + if err != nil { + causes = append(causes, preflight.Cause{ + Message: err.Error(), + Field: fmt.Sprintf( + "cluster.spec.topology.workers.machineDeployments[.name=%s].variables[.name=workerConfig]", + md.Name, + ), + }) + } + + if workerConfig != nil && workerConfig.Nutanix != nil { + nutanixNodeSpecByMachineDeploymentName[md.Name] = workerConfig.Nutanix + } + } + } + return prismCentralEndpointSpec, controlPlaneNutanixNodeSpec, nutanixNodeSpecByMachineDeploymentName, causes +} From dbd0714fb3e6c2d9522e2fd8024291a92e0f506e Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 29 May 2025 11:54:44 -0700 Subject: [PATCH 09/11] Remove unused preflight util package --- pkg/webhook/preflight/util/variables.go | 71 ------------------------- 1 file changed, 71 deletions(-) delete mode 100644 pkg/webhook/preflight/util/variables.go diff --git a/pkg/webhook/preflight/util/variables.go b/pkg/webhook/preflight/util/variables.go deleted file mode 100644 index f16995040..000000000 --- a/pkg/webhook/preflight/util/variables.go +++ /dev/null @@ -1,71 +0,0 @@ -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() -} From 2bb6ab07a1074683cd2d4608677be7f75e624724 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 29 May 2025 12:09:49 -0700 Subject: [PATCH 10/11] Get credentials as part of create client function --- pkg/webhook/preflight/nutanix/checker.go | 25 +++++---------- pkg/webhook/preflight/nutanix/credentials.go | 33 +++++++++++++++++--- pkg/webhook/preflight/nutanix/image.go | 2 +- pkg/webhook/preflight/nutanix/specs.go | 2 +- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/checker.go b/pkg/webhook/preflight/nutanix/checker.go index b6cce9d10..ea70eeea5 100644 --- a/pkg/webhook/preflight/nutanix/checker.go +++ b/pkg/webhook/preflight/nutanix/checker.go @@ -10,8 +10,6 @@ import ( 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/pkg/webhook/preflight" ) @@ -22,40 +20,31 @@ func (n *Checker) Init( kclient ctrlclient.Client, cluster *clusterv1.Cluster, ) []preflight.Check { + // Read Nutanix specs from the cluster. prismCentralEndpointSpec, controlPlaneNutanixNodeSpec, nutanixNodeSpecByMachineDeploymentName, - errCauses := readNutanixSpecs(cluster) + errCauses := specsFromCluster(cluster) if len(errCauses) > 0 { return initErrorCheck(errCauses...) } + // If no Nutanix specs are found, no checks need to run. if controlPlaneNutanixNodeSpec == nil && len(nutanixNodeSpecByMachineDeploymentName) == 0 { - // No Nutanix specs found, no checks to run. return nil } - // At least one Nutanix spec is defined. Get credentials and create a client, - // because all checks require them. - credentials, errCauses := getCredentials(ctx, kclient, cluster, prismCentralEndpointSpec) + // Create a Nutanix client, because all checks require it. + nv4client, errCauses := newV4Client(ctx, kclient, cluster.Namespace, prismCentralEndpointSpec) if len(errCauses) > 0 { return initErrorCheck(errCauses...) } - nv4client, err := prismv4.NewV4Client(*credentials) - if err != nil { - return initErrorCheck( - preflight.Cause{ - Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err), - Field: "", - }) - } - // Initialize checks. checks := []preflight.Check{} if controlPlaneNutanixNodeSpec != nil { checks = append(checks, - vmImageCheck( + newVMImageCheck( nv4client, controlPlaneNutanixNodeSpec, "cluster.spec.topology[.name=clusterConfig].value.controlPlane.nutanix", @@ -67,7 +56,7 @@ func (n *Checker) Init( continue } checks = append(checks, - vmImageCheck( + newVMImageCheck( nv4client, nutanixNodeSpecByMachineDeploymentName[md.Name], fmt.Sprintf( diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index b1b87a269..d22929cee 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -6,11 +6,11 @@ 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" prismgoclient "github.com/nutanix-cloud-native/prism-go-client" prismcredentials "github.com/nutanix-cloud-native/prism-go-client/environment/credentials" + prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4" carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" @@ -18,10 +18,33 @@ import ( const credentialsSecretDataKey = "credentials" +func newV4Client(ctx context.Context, + client ctrlclient.Client, + clusterNamespace string, + prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec, +) (*prismv4.Client, []preflight.Cause) { + credentials, causes := getCredentials(ctx, client, clusterNamespace, prismCentralEndpointSpec) + if len(causes) > 0 { + return nil, causes + } + + nv4client, err := prismv4.NewV4Client(*credentials) + if err != nil { + return nil, []preflight.Cause{ + { + Message: fmt.Sprintf("failed to create Prism Central client: %s", err), + Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint", + }, + } + } + + return nv4client, nil +} + func getCredentials( ctx context.Context, client ctrlclient.Client, - cluster *clusterv1.Cluster, + clusterNamespace string, prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec, ) (*prismgoclient.Credentials, []preflight.Cause) { if prismCentralEndpointSpec == nil { @@ -36,7 +59,7 @@ func getCredentials( if prismCentralEndpointSpec.Credentials.SecretRef.Name == "" { return nil, []preflight.Cause{ { - Message: "Prism Central credentials secret reference is missing the name", + Message: "Prism Central credentials reference is missing the name", Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef.name", }, } @@ -46,14 +69,14 @@ func getCredentials( if err := client.Get( ctx, types.NamespacedName{ - Namespace: cluster.Namespace, + Namespace: clusterNamespace, Name: prismCentralEndpointSpec.Credentials.SecretRef.Name, }, credentialsSecret, ); err != nil { return nil, []preflight.Cause{ { - Message: fmt.Sprintf("failed to get the credentials Secret: %s", err), + Message: fmt.Sprintf("failed to get credentials Secret: %s", err), Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", }, } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 7ea9ce2fd..3dd348f6d 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -13,7 +13,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func vmImageCheck( +func newVMImageCheck( client *prismv4.Client, nutanixNodeSpec *carenv1.NutanixNodeSpec, nutanixNodeSpecField string, diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 0dff4199c..43350fe61 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -10,7 +10,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/webhook/preflight" ) -func readNutanixSpecs( +func specsFromCluster( cluster *clusterv1.Cluster, ) (*carenv1.NutanixPrismCentralEndpointSpec, *carenv1.NutanixNodeSpec, map[string]*carenv1.NutanixNodeSpec, []preflight.Cause) { var prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec From 82699cd312b57009e52fe3e1566cbe85620509e3 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Thu, 29 May 2025 13:52:40 -0700 Subject: [PATCH 11/11] Fix formatting, precommit and linter errors --- pkg/webhook/preflight/nutanix/credentials.go | 20 +++++++++++++++----- pkg/webhook/preflight/nutanix/image.go | 5 ++++- pkg/webhook/preflight/nutanix/specs.go | 10 +++++++++- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pkg/webhook/preflight/nutanix/credentials.go b/pkg/webhook/preflight/nutanix/credentials.go index d22929cee..a9aaec84d 100644 --- a/pkg/webhook/preflight/nutanix/credentials.go +++ b/pkg/webhook/preflight/nutanix/credentials.go @@ -1,3 +1,6 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + package nutanix import ( @@ -22,7 +25,10 @@ func newV4Client(ctx context.Context, client ctrlclient.Client, clusterNamespace string, prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec, -) (*prismv4.Client, []preflight.Cause) { +) ( + *prismv4.Client, + []preflight.Cause, +) { credentials, causes := getCredentials(ctx, client, clusterNamespace, prismCentralEndpointSpec) if len(causes) > 0 { return nil, causes @@ -46,7 +52,10 @@ func getCredentials( client ctrlclient.Client, clusterNamespace string, prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec, -) (*prismgoclient.Credentials, []preflight.Cause) { +) ( + *prismgoclient.Credentials, + []preflight.Cause, +) { if prismCentralEndpointSpec == nil { return nil, []preflight.Cause{ { @@ -59,8 +68,9 @@ func getCredentials( if prismCentralEndpointSpec.Credentials.SecretRef.Name == "" { return nil, []preflight.Cause{ { - Message: "Prism Central credentials reference is missing the name", - Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef.name", + Message: "Prism Central credentials zreference is missing the name", + Field: "cluster.spec.topology.variables[.name=clusterConfig]" + + ".nutanix.prismCentralEndpoint.credentials.secretRef.name", }, } } @@ -85,7 +95,7 @@ func getCredentials( if len(credentialsSecret.Data) == 0 { return nil, []preflight.Cause{ { - Message: fmt.Sprintf("credentials Secret has no data"), + Message: "credentials Secret has no data", Field: "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials.secretRef", }, } diff --git a/pkg/webhook/preflight/nutanix/image.go b/pkg/webhook/preflight/nutanix/image.go index 3dd348f6d..64bd4309b 100644 --- a/pkg/webhook/preflight/nutanix/image.go +++ b/pkg/webhook/preflight/nutanix/image.go @@ -1,3 +1,6 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + package nutanix import ( @@ -26,7 +29,7 @@ func newVMImageCheck( Error: true, Causes: []preflight.Cause{ { - Message: fmt.Sprintf("NutanixNodeSpec is missing"), + Message: "NutanixNodeSpec is missing", Field: nutanixNodeSpecField, }, }, diff --git a/pkg/webhook/preflight/nutanix/specs.go b/pkg/webhook/preflight/nutanix/specs.go index 43350fe61..ac8b50ee6 100644 --- a/pkg/webhook/preflight/nutanix/specs.go +++ b/pkg/webhook/preflight/nutanix/specs.go @@ -1,3 +1,6 @@ +// Copyright 2025 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + package nutanix import ( @@ -12,7 +15,12 @@ import ( func specsFromCluster( cluster *clusterv1.Cluster, -) (*carenv1.NutanixPrismCentralEndpointSpec, *carenv1.NutanixNodeSpec, map[string]*carenv1.NutanixNodeSpec, []preflight.Cause) { +) ( + *carenv1.NutanixPrismCentralEndpointSpec, + *carenv1.NutanixNodeSpec, + map[string]*carenv1.NutanixNodeSpec, + []preflight.Cause, +) { var prismCentralEndpointSpec *carenv1.NutanixPrismCentralEndpointSpec var controlPlaneNutanixNodeSpec *carenv1.NutanixNodeSpec nutanixNodeSpecByMachineDeploymentName := make(map[string]*carenv1.NutanixNodeSpec)