fixup! feat(preflight): Add a check for storage containers

* Update storagecontainer check to work with latest changes
* Combine mock clients into one file
* Add storagecontainer unit tests
* Fix: If the storagecontainer check does not fail, return "allowed: true"
* Fix: Use helpers to correctly handle cluster identifier type

Author: dlipovetsky

pkg/webhook/preflight/nutanix/checker.go

@@ -25,10 +25,12 @@ func New(kclient ctrlclient.Client, cluster *clusterv1.Cluster) preflight.Checke
 		v3clientFactory: newV3Client,
 		v4clientFactory: newV4Client,
 
-		vmImageCheckFunc:             vmImageCheck,
-		initNutanixConfigurationFunc: initNutanixConfiguration,
-		initCredentialsCheckFunc:     initCredentialsCheck,
-		initVMImageChecksFunc:        initVMImageChecks,
+		vmImageCheckFunc:               vmImageCheck,
+		storageContainerCheckFunc:      storageContainerCheck,
+		initNutanixConfigurationFunc:   initNutanixConfiguration,
+		initCredentialsCheckFunc:       initCredentialsCheck,
+		initVMImageChecksFunc:          initVMImageChecks,
+		initStorageContainerChecksFunc: initStorageContainerChecks,
 	}
 }
 
@@ -53,6 +55,13 @@ type nutanixChecker struct {
 		field string,
 	) preflight.Check
 
+	storageContainerCheckFunc func(
+		n *nutanixChecker,
+		nodeSpec *carenv1.NutanixNodeSpec,
+		field string,
+		csiSpec *carenv1.CSIProvider,
+	) preflight.Check
+
 	initNutanixConfigurationFunc func(
 		n *nutanixChecker,
 	) preflight.Check
@@ -66,6 +75,10 @@ type nutanixChecker struct {
 		n *nutanixChecker,
 	) []preflight.Check
 
+	initStorageContainerChecksFunc func(
+		n *nutanixChecker,
+	) []preflight.Check
+
 	log logr.Logger
 }
 
@@ -82,7 +95,7 @@ func (n *nutanixChecker) Init(
 	}
 
 	checks = append(checks, n.initVMImageChecksFunc(n)...)
-	checks = append(checks, n.initStorageContainerChecks()...)
+	checks = append(checks, n.initStorageContainerChecksFunc(n)...)
 
 	// Add more checks here as needed.
 

pkg/webhook/preflight/nutanix/checker_test.go

@@ -17,22 +17,24 @@ import (
 
 func TestNutanixChecker_Init(t *testing.T) {
 	tests := []struct {
-		name                    string
-		nutanixConfig           *carenv1.NutanixClusterConfigSpec
-		workerNodeConfigs       map[string]*carenv1.NutanixWorkerNodeConfigSpec
-		expectedCheckCount      int
-		expectedFirstCheckName  string
-		expectedSecondCheckName string
-		vmImageCheckCount       int
+		name                       string
+		nutanixConfig              *carenv1.NutanixClusterConfigSpec
+		workerNodeConfigs          map[string]*carenv1.NutanixWorkerNodeConfigSpec
+		expectedCheckCount         int
+		expectedFirstCheckName     string
+		expectedSecondCheckName    string
+		vmImageCheckCount          int
+		storageContainerCheckCount int
 	}{
 		{
-			name:                    "basic initialization with no configs",
-			nutanixConfig:           nil,
-			workerNodeConfigs:       nil,
-			expectedCheckCount:      2, // config check and credentials check
-			expectedFirstCheckName:  "NutanixConfiguration",
-			expectedSecondCheckName: "NutanixCredentials",
-			vmImageCheckCount:       0,
+			name:                       "basic initialization with no configs",
+			nutanixConfig:              nil,
+			workerNodeConfigs:          nil,
+			expectedCheckCount:         2, // config check and credentials check
+			expectedFirstCheckName:     "NutanixConfiguration",
+			expectedSecondCheckName:    "NutanixCredentials",
+			vmImageCheckCount:          0,
+			storageContainerCheckCount: 0,
 		},
 		{
 			name: "initialization with control plane config",
@@ -41,11 +43,12 @@ func TestNutanixChecker_Init(t *testing.T) {
 					Nutanix: &carenv1.NutanixNodeSpec{},
 				},
 			},
-			workerNodeConfigs:       nil,
-			expectedCheckCount:      3, // config check, credentials check, 1 VM image check
-			expectedFirstCheckName:  "NutanixConfiguration",
-			expectedSecondCheckName: "NutanixCredentials",
-			vmImageCheckCount:       1,
+			workerNodeConfigs:          nil,
+			expectedCheckCount:         4, // config check, credentials check, 1 VM image check, 1 storage container check
+			expectedFirstCheckName:     "NutanixConfiguration",
+			expectedSecondCheckName:    "NutanixCredentials",
+			vmImageCheckCount:          1,
+			storageContainerCheckCount: 1,
 		},
 		{
 			name:          "initialization with worker node configs",
@@ -58,10 +61,11 @@ func TestNutanixChecker_Init(t *testing.T) {
 					Nutanix: &carenv1.NutanixNodeSpec{},
 				},
 			},
-			expectedCheckCount:      4, // config check, credentials check, 2 VM image checks
-			expectedFirstCheckName:  "NutanixConfiguration",
-			expectedSecondCheckName: "NutanixCredentials",
-			vmImageCheckCount:       2,
+			expectedCheckCount:         6, // config check, credentials check, 2 VM image checks, 2 storage container checks
+			expectedFirstCheckName:     "NutanixConfiguration",
+			expectedSecondCheckName:    "NutanixCredentials",
+			vmImageCheckCount:          2,
+			storageContainerCheckCount: 2,
 		},
 		{
 			name: "initialization with both control plane and worker node configs",
@@ -75,10 +79,12 @@ func TestNutanixChecker_Init(t *testing.T) {
 					Nutanix: &carenv1.NutanixNodeSpec{},
 				},
 			},
-			expectedCheckCount:      4, // config check, credentials check, 2 VM image checks (1 CP + 1 worker)
-			expectedFirstCheckName:  "NutanixConfiguration",
-			expectedSecondCheckName: "NutanixCredentials",
-			vmImageCheckCount:       2,
+			// config check, credentials check, 2 VM image checks (1 CP + 1 worker), 2 storage container checks (1 CP + 1 worker)
+			expectedCheckCount:         6,
+			expectedFirstCheckName:     "NutanixConfiguration",
+			expectedSecondCheckName:    "NutanixCredentials",
+			vmImageCheckCount:          2,
+			storageContainerCheckCount: 2,
 		},
 	}
 
@@ -95,6 +101,7 @@ func TestNutanixChecker_Init(t *testing.T) {
 			configCheckCalled := false
 			credsCheckCalled := false
 			vmImageCheckCount := 0
+			storageContainerCheckCount := 0
 
 			checker.initNutanixConfigurationFunc = func(n *nutanixChecker) preflight.Check {
 				configCheckCalled = true
@@ -127,6 +134,19 @@ func TestNutanixChecker_Init(t *testing.T) {
 				return checks
 			}
 
+			checker.initStorageContainerChecksFunc = func(n *nutanixChecker) []preflight.Check {
+				checks := []preflight.Check{}
+				for i := 0; i < tt.storageContainerCheckCount; i++ {
+					storageContainerCheckCount++
+					checks = append(checks, func(ctx context.Context) preflight.CheckResult {
+						return preflight.CheckResult{
+							Name: fmt.Sprintf("StorageContainerCheck-%d", i),
+						}
+					})
+				}
+				return checks
+			}
+
 			// Call Init
 			ctx := context.Background()
 			checks := checker.Init(ctx)
@@ -141,6 +161,12 @@ func TestNutanixChecker_Init(t *testing.T) {
 			assert.True(t, configCheckCalled, "initNutanixConfiguration should have been called")
 			assert.True(t, credsCheckCalled, "initCredentialsCheck should have been called")
 			assert.Equal(t, tt.vmImageCheckCount, vmImageCheckCount, "Wrong number of VM image checks")
+			assert.Equal(
+				t,
+				tt.storageContainerCheckCount,
+				storageContainerCheckCount,
+				"Wrong number of storage container checks",
+			)
 
 			// Verify the first two checks when we have results
 			if len(checks) >= 2 {

pkg/webhook/preflight/nutanix/clients.go

@@ -6,6 +6,7 @@ package nutanix
 import (
 	"context"
 
+	clustermgmtv4 "github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4/models/clustermgmt/v4/config"
 	vmmv4 "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
 
 	prismgoclient "github.com/nutanix-cloud-native/prism-go-client"
@@ -47,6 +48,24 @@ type v4client interface {
 		*vmmv4.ListImagesApiResponse,
 		error,
 	)
+	GetClusterById(id *string) (*clustermgmtv4.GetClusterApiResponse, error)
+	ListClusters(
+		page_ *int,
+		limit_ *int,
+		filter_ *string,
+		orderby_ *string,
+		apply_ *string,
+		select_ *string,
+		args ...map[string]interface{},
+	) (*clustermgmtv4.ListClustersApiResponse, error)
+	ListStorageContainers(
+		page_ *int,
+		limit_ *int,
+		filter_ *string,
+		orderby_ *string,
+		select_ *string,
+		args ...map[string]interface{},
+	) (*clustermgmtv4.ListStorageContainersApiResponse, error)
 }
 
 type v4clientWrapper struct {
@@ -84,6 +103,60 @@ func (c *v4clientWrapper) ListImages(page_ *int,
 	return resp, nil
 }
 
+func (c *v4clientWrapper) GetClusterById(id *string) (*clustermgmtv4.GetClusterApiResponse, error) {
+	resp, err := c.client.ClustersApiInstance.GetClusterById(id)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+func (c *v4clientWrapper) ListClusters(
+	page_ *int,
+	limit_ *int,
+	filter_ *string,
+	orderby_ *string,
+	apply_ *string,
+	select_ *string,
+	args ...map[string]interface{},
+) (*clustermgmtv4.ListClustersApiResponse, error) {
+	resp, err := c.client.ClustersApiInstance.ListClusters(
+		page_,
+		limit_,
+		filter_,
+		orderby_,
+		apply_,
+		select_,
+		args...,
+	)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+func (c *v4clientWrapper) ListStorageContainers(
+	page_ *int,
+	limit_ *int,
+	filter_ *string,
+	orderby_ *string,
+	select_ *string,
+	args ...map[string]interface{},
+) (*clustermgmtv4.ListStorageContainersApiResponse, error) {
+	resp, err := c.client.StorageContainerAPI.ListStorageContainers(
+		page_,
+		limit_,
+		filter_,
+		orderby_,
+		select_,
+		args...,
+	)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
 func newV4Client(
 	credentials prismgoclient.Credentials, //nolint:gocritic // hugeParam is fine
 ) (v4client, error) {

pkg/webhook/preflight/nutanix/clients_test.go

@@ -0,0 +1,95 @@
+// Copyright 2024 Nutanix. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package nutanix
+
+import (
+	"context"
+
+	clustermgmtv4 "github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4/models/clustermgmt/v4/config"
+	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"
+)
+
+type mockv3client struct {
+	user *prismv3.UserIntentResponse
+	err  error
+}
+
+func (m *mockv3client) GetCurrentLoggedInUser(ctx context.Context) (*prismv3.UserIntentResponse, error) {
+	return m.user, m.err
+}
+
+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,
+	)
+
+	getClusterByIdFunc func(id *string) (*clustermgmtv4.GetClusterApiResponse, error)
+
+	listClustersFunc func(
+		page,
+		limit *int,
+		filter,
+		orderby,
+		apply,
+		select_ *string,
+		args ...map[string]interface{},
+	) (*clustermgmtv4.ListClustersApiResponse, error)
+
+	listStorageContainersFunc func(
+		page,
+		limit *int,
+		filter,
+		orderby,
+		select_ *string,
+		args ...map[string]interface{},
+	) (*clustermgmtv4.ListStorageContainersApiResponse, 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_)
+}
+
+func (m *mockv4client) GetClusterById(id *string) (*clustermgmtv4.GetClusterApiResponse, error) {
+	return m.getClusterByIdFunc(id)
+}
+
+func (m *mockv4client) ListClusters(
+	page, limit *int,
+	filter, orderby, apply, select_ *string,
+	args ...map[string]interface{},
+) (*clustermgmtv4.ListClustersApiResponse, error) {
+	return m.listClustersFunc(page, limit, filter, orderby, apply, select_, args...)
+}
+
+func (m *mockv4client) ListStorageContainers(
+	page, limit *int,
+	filter, orderby, select_ *string,
+	args ...map[string]interface{},
+) (*clustermgmtv4.ListStorageContainersApiResponse, error) {
+	return m.listStorageContainersFunc(page, limit, filter, orderby, select_, args...)
+}