fixup! feat: Nutanix VM image preflight check

Merge separate v3 and v4 clients into one

Author: dlipovetsky

pkg/webhook/preflight/nutanix/checker.go

@@ -22,8 +22,7 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke
 		kclient: kclient,
 		cluster: cluster,
 
-		v3clientFactory: newV3Client,
-		v4clientFactory: newV4Client,
+		nclientFactory: newClient,
 
 		vmImageCheckFunc:             vmImageCheck,
 		initNutanixConfigurationFunc: initNutanixConfiguration,
@@ -39,11 +38,8 @@ type nutanixChecker struct {
 	nutanixClusterConfigSpec                           *carenv1.NutanixClusterConfigSpec
 	nutanixWorkerNodeConfigSpecByMachineDeploymentName map[string]*carenv1.NutanixWorkerNodeConfigSpec
 
-	v3client        v3client
-	v3clientFactory func(prismgoclient.Credentials) (v3client, error)
-
-	v4client        v4client
-	v4clientFactory func(prismgoclient.Credentials) (v4client, error)
+	nclient        client
+	nclientFactory func(prismgoclient.Credentials) (client, error)
 
 	vmImageCheckFunc func(
 		n *nutanixChecker,

pkg/webhook/preflight/nutanix/clients.go

@@ -5,6 +5,7 @@ package nutanix
 
 import (
 	"context"
+	"fmt"
 
 	vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
 
@@ -13,29 +14,10 @@ import (
 	prismv4 "github.com/nutanix-cloud-native/prism-go-client/v4"
 )
 
-type v3client interface {
+// client contains methods to interact with Nutanix Prism v3 and v4 APIs.
+type client interface {
 	GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error)
-}
-
-type v3clientWrapper struct {
-	prismv3.Service
-}
-
-var _ = v3client(&v3clientWrapper{})
 
-func newV3Client(
-	credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine
-) (v3client, error) {
-	client, err := prismv3.NewV3Client(credentials)
-	if err != nil {
-		return nil, err
-	}
-	return &v3clientWrapper{
-		client.V3,
-	}, nil
-}
-
-type v4client interface {
 	GetImageById(id *string) (*vmmv4.GetImageApiResponse, error)
 	ListImages(page_ *int,
 		limit_ *int,
@@ -49,28 +31,53 @@ type v4client interface {
 	)
 }
 
-type v4clientWrapper struct {
-	client *prismv4.Client
+// clientWrapper implements the client interface and wraps both v3 and v4 clients.
+type clientWrapper struct {
+	v3client *prismv3.Client
+	v4client *prismv4.Client
+}
+
+var _ = client(&clientWrapper{})
+
+func newClient(
+	credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine
+) (client, error) {
+	v3c, err := prismv3.NewV3Client(credentials)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create v3 client: %w", err)
+	}
+
+	v4c, err := prismv4.NewV4Client(credentials)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create v4 client: %w", err)
+	}
+
+	return &clientWrapper{
+		v3client: v3c,
+		v4client: v4c,
+	}, nil
 }
 
-var _ = v4client(&v4clientWrapper{})
+func (c *clientWrapper) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) {
+	return c.v3client.V3.GetCurrentLoggedInUser(ctx)
+}
 
-func (c *v4clientWrapper) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) {
-	resp, err := c.client.ImagesApiInstance.GetImageById(id)
+func (c *clientWrapper) GetImageById(id *string) (*vmmv4.GetImageApiResponse, error) {
+	resp, err := c.v4client.ImagesApiInstance.GetImageById(id)
 	if err != nil {
 		return nil, err
 	}
 	return resp, nil
 }
 
-func (c *v4clientWrapper) ListImages(page_ *int,
+func (c *clientWrapper) ListImages(page_ *int,
 	limit_ *int,
 	filter_ *string,
 	orderby_ *string,
 	select_ *string,
 	args ...map[string]interface{},
 ) (*vmmv4.ListImagesApiResponse, error) {
-	resp, err := c.client.ImagesApiInstance.ListImages(
+	resp, err := c.v4client.ImagesApiInstance.ListImages(
 		page_,
 		limit_,
 		filter_,
@@ -83,15 +90,3 @@ func (c *v4clientWrapper) ListImages(page_ *int,
 	}
 	return resp, nil
 }
-
-func newV4Client(
-	credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine
-) (v4client, error) {
-	client, err := prismv4.NewV4Client(credentials)
-	if err != nil {
-		return nil, err
-	}
-	return &v4clientWrapper{
-		client: client,
-	}, nil
-}

pkg/webhook/preflight/nutanix/clients_test.go

@@ -0,0 +1,54 @@
+// Copyright 2024 Nutanix. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package nutanix
+
+import (
+	"context"
+
+	vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
+
+	prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3"
+)
+
+var _ = client(&mocknclient{})
+
+// mocknclient is a mock implementation of the client interface for testing purposes.
+type mocknclient struct {
+	user *prismv3.UserIntentResponse
+	err  error
+
+	getImageByIdFunc func(
+		uuid *string,
+	) (
+		*vmmv4.GetImageApiResponse, error,
+	)
+
+	listImagesFunc func(
+		page,
+		limit *int,
+		filter,
+		orderby,
+		select_ *string,
+		args ...map[string]interface{},
+	) (
+		*vmmv4.ListImagesApiResponse,
+		error,
+	)
+}
+
+func (m *mocknclient) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) {
+	return m.user, m.err
+}
+
+func (m *mocknclient) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) {
+	return m.getImageByIdFunc(uuid)
+}
+
+func (m *mocknclient) ListImages(
+	page, limit *int,
+	filter, orderby, select_ *string,
+	args ...map[string]interface{},
+) (*vmmv4.ListImagesApiResponse, error) {
+	return m.listImagesFunc(page, limit, filter, orderby, select_)
+}

pkg/webhook/preflight/nutanix/credentials.go

@@ -154,39 +154,24 @@ func initCredentialsCheck(
 		Insecure: prismCentralEndpointSpec.Insecure,
 	}
 
-	// Initialize the clients.
-	v4client, err := n.v4clientFactory(credentials)
+	// Initialize the Nutanix client.
+	nclient, err := n.nclientFactory(credentials)
 	if err != nil {
 		result.Allowed = false
 		result.Error = true
 		result.Causes = append(result.Causes,
 			preflight.Cause{
-				Message: fmt.Sprintf("failed to initialize Nutanix v4 client: %s", err),
+				Message: fmt.Sprintf("Failed to initialize Nutanix client: %s", err),
 				Field:   "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
 			},
 		)
-	}
-
-	v3client, err := n.v3clientFactory(credentials)
-	if err != nil {
-		result.Allowed = false
-		result.Error = true
-		result.Causes = append(result.Causes,
-			preflight.Cause{
-				Message: fmt.Sprintf("failed to initialize Nutanix v3 client: %s", err),
-				Field:   "cluster.spec.topology.variables[.name=clusterConfig].nutanix.prismCentralEndpoint.credentials",
-			},
-		)
-	}
-
-	if v3client == nil || v4client == nil {
 		return func(ctx context.Context) preflight.CheckResult {
 			return result
 		}
 	}
 
 	// Validate the credentials using an API call.
-	_, err = v3client.GetCurrentLoggedInUser(ctx)
+	_, err = nclient.GetCurrentLoggedInUser(ctx)
 	if err != nil {
 		result.Allowed = false
 		result.Error = true
@@ -203,8 +188,7 @@ func initCredentialsCheck(
 	}
 
 	// We initialized both clients, and verified the credentials using the v3 client.
-	n.v3client = v3client
-	n.v4client = v4client
+	n.nclient = nclient
 
 	return func(ctx context.Context) preflight.CheckResult {
 		return result

pkg/webhook/preflight/nutanix/credentials_test.go

@@ -7,15 +7,13 @@ import (
 	"context"
 	"testing"
 
-	vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	prismgoclient "github.com/nutanix-cloud-native/prism-go-client"
-	prismv3 "github.com/nutanix-cloud-native/prism-go-client/v3"
 
 	carenv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
 )
@@ -129,47 +127,27 @@ func TestInitCredentialsCheck_InvalidCredentialsFormat(t *testing.T) {
 	assert.Contains(t, result.Causes[0].Message, "failed to parse Prism Central credentials")
 }
 
-func TestInitCredentialsCheck_FailedToCreateV3Client(t *testing.T) {
-	// Simulate a failure in creating the v3 client
-	nc := validNutanixChecker()
-	nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) {
-		return nil, assert.AnError
-	}
-	nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) {
-		return &mockv4client{}, nil
-	}
-	check := nc.initCredentialsCheckFunc(context.Background(), nc)
-	result := check(context.Background())
-	assert.False(t, result.Allowed)
-	assert.True(t, result.Error)
-	assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v3 client")
-}
-
-func TestInitCredentialsCheck_FailedToCreateV4Client(t *testing.T) {
+func TestInitCredentialsCheck_FailedToCreateClient(t *testing.T) {
 	// Simulate a failure in creating the v4 client
 	nc := validNutanixChecker()
-	nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) {
-		return &mockv3client{}, nil
-	}
-	nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) {
+	nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) {
 		return nil, assert.AnError
 	}
+
 	check := nc.initCredentialsCheckFunc(context.Background(), nc)
 	result := check(context.Background())
 	assert.False(t, result.Allowed)
 	assert.True(t, result.Error)
-	assert.Contains(t, result.Causes[0].Message, "failed to initialize Nutanix v4 client")
+	assert.Contains(t, result.Causes[0].Message, "Failed to initialize Nutanix client")
 }
 
 func TestInitCredentialsCheck_FailedToGetCurrentLoggedInUser(t *testing.T) {
 	// Simulate a failure in getting the current logged-in user
 	nc := validNutanixChecker()
-	nc.v3clientFactory = func(_ prismgoclient.Credentials) (v3client, error) {
-		return &mockv3client{err: assert.AnError}, nil
-	}
-	nc.v4clientFactory = func(_ prismgoclient.Credentials) (v4client, error) {
-		return &mockv4client{}, nil
+	nc.nclientFactory = func(_ prismgoclient.Credentials) (client, error) {
+		return &mocknclient{err: assert.AnError}, nil
 	}
+
 	check := nc.initCredentialsCheckFunc(context.Background(), nc)
 	result := check(context.Background())
 	assert.False(t, result.Allowed)
@@ -207,11 +185,8 @@ func validNutanixChecker() *nutanixChecker {
 			},
 		},
 
-		v3clientFactory: func(_ prismgoclient.Credentials) (v3client, error) {
-			return &mockv3client{}, nil
-		},
-		v4clientFactory: func(_ prismgoclient.Credentials) (v4client, error) {
-			return &mockv4client{}, nil
+		nclientFactory: func(_ prismgoclient.Credentials) (client, error) {
+			return &mocknclient{}, nil
 		},
 
 		vmImageCheckFunc:             vmImageCheck,
@@ -233,45 +208,3 @@ func validNutanixChecker() *nutanixChecker {
 		nutanixWorkerNodeConfigSpecByMachineDeploymentName: map[string]*carenv1.NutanixWorkerNodeConfigSpec{},
 	}
 }
-
-type mockv3client struct {
-	user *prismv3.UserIntentResponse
-	err  error
-}
-
-func (m *mockv3client) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) {
-	return m.user, m.err
-}
-
-// mockv4client is a mock implementation of the v4client interface for testing.
-type mockv4client struct {
-	getImageByIdFunc func(
-		uuid *string,
-	) (
-		*vmmv4.GetImageApiResponse, error,
-	)
-
-	listImagesFunc func(
-		page,
-		limit *int,
-		filter,
-		orderby,
-		select_ *string,
-		args ...map[string]interface{},
-	) (
-		*vmmv4.ListImagesApiResponse,
-		error,
-	)
-}
-
-func (m *mockv4client) GetImageById(uuid *string) (*vmmv4.GetImageApiResponse, error) {
-	return m.getImageByIdFunc(uuid)
-}
-
-func (m *mockv4client) ListImages(
-	page, limit *int,
-	filter, orderby, select_ *string,
-	args ...map[string]interface{},
-) (*vmmv4.ListImagesApiResponse, error) {
-	return m.listImagesFunc(page, limit, filter, orderby, select_)
-}