fix: improve error handling for Prism Client

pkg/client/client.go

@@ -20,6 +20,8 @@
 	"context"
 	"encoding/json"
 	"fmt"
+	"net"
+	"net/url"
 	"os"
 	"path/filepath"
 
@@ -43,6 +45,9 @@
 	capxNamespaceKey    = "POD_NAMESPACE"
 )
 
+var ErrPrismAddressNotSet = fmt.Errorf("cannot get credentials if Prism Address is not set")
+var ErrPrismPortNotSet = fmt.Errorf("cannot get credentials if Prism Port is not set")
+
 type NutanixClientHelper struct {
 	secretInformer    coreinformers.SecretInformer
 	configMapInformer coreinformers.ConfigMapInformer
@@ -63,15 +68,18 @@
 	// If PrismCentral is set, add the required env provider
 	prismCentralInfo := nutanixCluster.Spec.PrismCentral
 	if prismCentralInfo != nil {
-		if prismCentralInfo.Address == "" {
-			return nil, fmt.Errorf("cannot get credentials if Prism Address is not set")
+		address, err := validateAndSanitizePrismCentralInfoAddress(prismCentralInfo.Address)
+		if err != nil {
+			return nil, err
 		}
+		prismCentralInfo.Address = address
 		if prismCentralInfo.Port == 0 {
-			return nil, fmt.Errorf("cannot get credentials if Prism Port is not set")
+			return nil, ErrPrismPortNotSet
 		}
-		credentialRef := prismCentralInfo.CredentialRef
-		if credentialRef == nil {
-			return nil, fmt.Errorf("credentialRef must be set on prismCentral attribute for cluster %s in namespace %s", nutanixCluster.Name, nutanixCluster.Namespace)
+		credentialRef, err := GetCredentialRefForCluster(nutanixCluster)
+		if err != nil {
+			//nolint:wrapcheck // error is alredy wrapped
+			return nil, err
 		}
 		// If namespace is empty, use the cluster namespace
 		if credentialRef.Namespace == "" {
@@ -94,7 +102,7 @@
 	// Add env provider for CAPX manager
 	npe, err := n.getManagerNutanixPrismEndpoint()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create prism endpoint %w", err)
 	}
 	// If namespaces is not set, set it to the namespace of the CAPX manager
 	if npe.CredentialRef.Namespace == "" {
@@ -115,15 +123,14 @@
 		*npe,
 		n.secretInformer,
 		n.configMapInformer))
-
 	// init env with providers
 	env := environment.NewEnvironment(
 		providers...,
 	)
 	// fetch endpoint details
 	me, err := env.GetManagementEndpoint(envTypes.Topology{})
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get management endpoint object %w", err)
 	}
 	creds := prismgoclient.Credentials{
 		URL:      me.Address.Host,
@@ -155,26 +162,25 @@
 	}
 	cli, err := nutanixClientV3.NewV3Client(cred, clientOpts...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create new nutanix client %w", err)
 	}
 
 	// Check if the client is working
 	_, err = cli.V3.GetCurrentLoggedInUser(context.Background())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get current logged in user with client %w", err)
 	}
-
 	return cli, nil
 }
 
 func (n *NutanixClientHelper) getManagerNutanixPrismEndpoint() (*credentialTypes.NutanixPrismEndpoint, error) {
 	npe := &credentialTypes.NutanixPrismEndpoint{}
 	config, err := n.readEndpointConfig()
 	if err != nil {
-		return npe, err
+		return nil, fmt.Errorf("failed to read config %w", err)
 	}
 	if err = json.Unmarshal(config, npe); err != nil {
-		return npe, err
+		return nil, fmt.Errorf("failed to unmarshal config %w", err)
 	}
 	if npe.CredentialRef == nil {
 		return nil, fmt.Errorf("credentialRef must be set on CAPX manager")
@@ -196,16 +202,46 @@
 	if nutanixCluster == nil {
 		return nil, fmt.Errorf("cannot get credential reference if nutanix cluster object is nil")
 	}
-	prismCentralinfo := nutanixCluster.Spec.PrismCentral
-	if prismCentralinfo == nil {
+	prismCentralInfo := nutanixCluster.Spec.PrismCentral
+	if prismCentralInfo == nil {
 		return nil, nil
 	}
-	if prismCentralinfo.CredentialRef == nil {
+	if prismCentralInfo.CredentialRef == nil {
 		return nil, fmt.Errorf("credentialRef must be set on prismCentral attribute for cluster %s in namespace %s", nutanixCluster.Name, nutanixCluster.Namespace)
 	}
-	if prismCentralinfo.CredentialRef.Kind != credentialTypes.SecretKind {
+	if prismCentralInfo.CredentialRef.Kind != credentialTypes.SecretKind {
 		return nil, nil
 	}
 
-	return prismCentralinfo.CredentialRef, nil
+	return prismCentralInfo.CredentialRef, nil
+}
+
+func validateAndSanitizePrismCentralInfoAddress(address string) (string, error) {
+	if address == "" {
+		return "", ErrPrismAddressNotSet
+	}
+	u, urlParseErr := url.Parse(address)
+	if urlParseErr != nil {
+		ipHost, ipErr := parseIP(address)
+		if ipErr != nil {
+			return "", fmt.Errorf("failed to resolve %s as url or ip addr %w", address, ipErr)
+		}
+		return ipHost, nil
+	}
+	if u.Scheme != "" || u.Port() != "" {
+		return u.Hostname(), nil
+	}
+	return address, nil
+}
+
+func parseIP(s string) (string, error) {
+	ipStr, _, err := net.SplitHostPort(s)
+	if err == nil {
+		return ipStr, nil
+	}
+	ip := net.ParseIP(s)
+	if ip == nil {
+		return "", fmt.Errorf("invalid IP %s", ip)
+	}
+	return ip.String(), nil
 }

pkg/client/client_test.go

@@ -0,0 +1,76 @@
+package client
+
+import (
+	"errors"
+	"testing"
+)
+
+func Test_validateAndSanitizePrismCentralInfoAddress(t *testing.T) {
+	tests := []struct {
+		name            string
+		providedAddress string
+		expectedAddress string
+		expectedErr     error
+	}{
+		{
+			"with scheme",
+			"https://prism-bowser.ntnxsherlock.com",
+			"prism-bowser.ntnxsherlock.com",
+			nil,
+		},
+		{
+			"with scheme and port",
+			"https://prism-bowser.ntnxsherlock.com:9440",
+			"prism-bowser.ntnxsherlock.com",
+			nil,
+		},
+		{
+			"as expected from the user",
+			"prism-bowser.ntnxsherlock.com",
+			"prism-bowser.ntnxsherlock.com",
+			nil,
+		},
+		{
+			"not set",
+			"",
+			"",
+			ErrPrismAddressNotSet,
+		},
+		{
+			"using a IP with scheme and port",
+			"https://1.2.3.4:9440",
+			"1.2.3.4",
+			nil,
+		},
+		{
+			"using a IP with scheme",
+			"https://1.2.3.4",
+			"1.2.3.4",
+			nil,
+		},
+		{
+			"using a IP with port",
+			"1.2.3.4:9440",
+			"1.2.3.4",
+			nil,
+		},
+		{
+			"just an IP",
+			"1.2.3.4",
+			"1.2.3.4",
+			nil,
+		},
+	}
+
+	for _, test := range tests {
+		s, err := validateAndSanitizePrismCentralInfoAddress(test.providedAddress)
+		if err != nil {
+			if test.expectedErr == nil || !errors.Is(err, test.expectedErr) {
+				t.Errorf("validateAndSanitizePrismCentralInfoAddress() error = %v, wantErr = %v", err, test.expectedErr)
+			}
+		}
+		if s != test.expectedAddress {
+			t.Errorf("validateAndSanitizePrismCentralInfoAddress() got = %v, want = %v", s, test.expectedAddress)
+		}
+	}
+}