Refactor a map of Validator to a map of ProviderCmkClients

aws · Jun 17, 2022 · f9c9fba · f9c9fba
1 parent ba8207c
commit f9c9fba
Show file tree

Hide file tree

Showing 8 changed files with 236 additions and 156 deletions.
diff --git a/pkg/executables/cmk.go b/pkg/executables/cmk.go
@@ -380,6 +380,10 @@ func NewCmk(executable Executable, writer filewriter.FileWriter, config decoder.
 	}
 }
 
+func (c *Cmk) GetManagementApiEndpoint(ctx context.Context) string {
+	return c.config.ManagementUrl
+}
+
 // ValidateCloudStackConnection Calls `cmk sync` to ensure that the endpoint and credentials + domain are valid
 func (c *Cmk) ValidateCloudStackConnection(ctx context.Context) error {
 	command := newCmkCommand("sync")

diff --git a/pkg/providers/cloudstack/cloudstack.go b/pkg/providers/cloudstack/cloudstack.go
@@ -63,7 +63,7 @@ type cloudstackProvider struct {
 	selfSigned            bool
 	templateBuilder       *CloudStackTemplateBuilder
 	skipIpCheck           bool
-	validators            map[string]*Validator
+	validator             *Validator
 }
 
 func (p *cloudstackProvider) PreBootstrapSetup(ctx context.Context, cluster *types.Cluster) error {
@@ -201,7 +201,7 @@ type ProviderKubectlClient interface {
 	SetEksaControllerEnvVar(ctx context.Context, envVar, envVarVal, kubeconfig string) error
 }
 
-func NewProvider(datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineConfigs map[string]*v1alpha1.CloudStackMachineConfig, clusterConfig *v1alpha1.Cluster, providerKubectlClient ProviderKubectlClient, providerCmkClients map[string]ProviderCmkClient, writer filewriter.FileWriter, now types.NowFunc, skipIpCheck bool) *cloudstackProvider {
+func NewProvider(datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineConfigs map[string]*v1alpha1.CloudStackMachineConfig, clusterConfig *v1alpha1.Cluster, providerKubectlClient ProviderKubectlClient, providerCmkClients CmkClientMap, writer filewriter.FileWriter, now types.NowFunc, skipIpCheck bool) *cloudstackProvider {
 	return NewProviderCustomNet(
 		datacenterConfig,
 		machineConfigs,
@@ -214,7 +214,7 @@ func NewProvider(datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineC
 	)
 }
 
-func NewProviderCustomNet(datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineConfigs map[string]*v1alpha1.CloudStackMachineConfig, clusterConfig *v1alpha1.Cluster, providerKubectlClient ProviderKubectlClient, providerCmkClients map[string]ProviderCmkClient, writer filewriter.FileWriter, now types.NowFunc, skipIpCheck bool) *cloudstackProvider {
+func NewProviderCustomNet(datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineConfigs map[string]*v1alpha1.CloudStackMachineConfig, clusterConfig *v1alpha1.Cluster, providerKubectlClient ProviderKubectlClient, providerCmkClients CmkClientMap, writer filewriter.FileWriter, now types.NowFunc, skipIpCheck bool) *cloudstackProvider {
 	var controlPlaneMachineSpec, etcdMachineSpec *v1alpha1.CloudStackMachineConfigSpec
 	workerNodeGroupMachineSpecs := make(map[string]v1alpha1.CloudStackMachineConfigSpec, len(machineConfigs))
 	if clusterConfig.Spec.ControlPlaneConfiguration.MachineGroupRef != nil && machineConfigs[clusterConfig.Spec.ControlPlaneConfiguration.MachineGroupRef.Name] != nil {
@@ -225,10 +225,6 @@ func NewProviderCustomNet(datacenterConfig *v1alpha1.CloudStackDatacenterConfig,
 			etcdMachineSpec = &machineConfigs[clusterConfig.Spec.ExternalEtcdConfiguration.MachineGroupRef.Name].Spec
 		}
 	}
-	validators := map[string]*Validator{}
-	for profileName, providerCmkClient := range providerCmkClients {
-		validators[profileName] = NewValidator(providerCmkClient)
-	}
 	return &cloudstackProvider{
 		datacenterConfig:      datacenterConfig,
 		machineConfigs:        machineConfigs,
@@ -244,7 +240,7 @@ func NewProviderCustomNet(datacenterConfig *v1alpha1.CloudStackDatacenterConfig,
 			now:                         now,
 		},
 		skipIpCheck: skipIpCheck,
-		validators:  validators,
+		validator:   NewValidator(providerCmkClients),
 	}
 }
 
@@ -385,16 +381,14 @@ func (p *cloudstackProvider) validateEnv(ctx context.Context) error {
 }
 
 func (p *cloudstackProvider) validateClusterSpec(ctx context.Context, clusterSpec *cluster.Spec) (err error) {
-	for _, validator := range p.validators {
-		if err := validator.validateCloudStackAccess(ctx); err != nil {
-			return err
-		}
-		if err := validator.ValidateCloudStackDatacenterConfig(ctx, p.datacenterConfig); err != nil {
-			return err
-		}
-		if err := validator.ValidateClusterMachineConfigs(ctx, NewSpec(clusterSpec, p.machineConfigs, p.datacenterConfig)); err != nil {
-			return err
-		}
+	if err := p.validator.validateCloudStackAccess(ctx, p.datacenterConfig); err != nil {
+		return err
+	}
+	if err := p.validator.ValidateCloudStackDatacenterConfig(ctx, p.datacenterConfig); err != nil {
+		return err
+	}
+	if err := p.validator.ValidateClusterMachineConfigs(ctx, NewSpec(clusterSpec, p.machineConfigs, p.datacenterConfig)); err != nil {
+		return err
 	}
 	return nil
 }

diff --git a/pkg/providers/cloudstack/cloudstack_test.go b/pkg/providers/cloudstack/cloudstack_test.go
@@ -68,6 +68,7 @@ func givenWildcardCmk(mockCtrl *gomock.Controller) ProviderCmkClient {
 	cmk.EXPECT().ValidateDomainPresent(gomock.Any(), gomock.Any()).AnyTimes()
 	cmk.EXPECT().ValidateAccountPresent(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 	cmk.EXPECT().ValidateNetworkPresent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
+	cmk.EXPECT().GetManagementApiEndpoint(gomock.Any()).AnyTimes().Return("http://127.16.0.1:8080/client/api")
 	return cmk
 }
 
@@ -193,8 +194,8 @@ func newProviderWithKubectl(t *testing.T, datacenterConfig *v1alpha1.CloudStackD
 
 func newProvider(t *testing.T, datacenterConfig *v1alpha1.CloudStackDatacenterConfig, machineConfigs map[string]*v1alpha1.CloudStackMachineConfig, clusterConfig *v1alpha1.Cluster, kubectl ProviderKubectlClient, cmk ProviderCmkClient) *cloudstackProvider {
 	_, writer := test.NewWriter(t)
-	cmks := map[string]ProviderCmkClient{}
-	cmks["mockCmk"] = cmk
+	cmks := CmkClientMap{}
+	cmks["Global"] = cmk
 
 	return NewProviderCustomNet(
 		datacenterConfig,

diff --git a/pkg/providers/cloudstack/decoder/decoder.go b/pkg/providers/cloudstack/decoder/decoder.go
@@ -13,6 +13,7 @@ const (
 	EksacloudStackCloudConfigB64SecretKey = "EKSA_CLOUDSTACK_B64ENCODED_SECRET"
 	CloudStackCloudConfigB64SecretKey     = "CLOUDSTACK_B64ENCODED_SECRET"
 	EksaCloudStackHostPathToMount         = "EKSA_CLOUDSTACK_HOST_PATHS_TO_MOUNT"
+	CloudStackGlobalAZ                    = "Global"
 )
 
 // ParseCloudStackSecret parses the input b64 string into the ini object to extract out the api key, secret key, and url
@@ -37,7 +38,7 @@ func ParseCloudStackSecret() (*CloudStackExecConfig, error) {
 		if section.Name() == "DEFAULT" {
 			continue
 		}
-		if section.Name() == "Global" {
+		if section.Name() == CloudStackGlobalAZ {
 			foundGlobalSection = true
 		}
 

diff --git a/pkg/providers/cloudstack/decoder/decoder_test.go b/pkg/providers/cloudstack/decoder/decoder_test.go
@@ -42,7 +42,7 @@ func TestCloudStackConfigDecoder(t *testing.T) {
 			wantConfig: &decoder.CloudStackExecConfig{
 				Profiles: []decoder.CloudStackProfileConfig{
 					{
-						Name:          "Global",
+						Name:          decoder.CloudStackGlobalAZ,
 						ApiKey:        "test-key1",
 						SecretKey:     "test-secret1",
 						ManagementUrl: "http://127.16.0.1:8080/client/api",
@@ -59,7 +59,7 @@ func TestCloudStackConfigDecoder(t *testing.T) {
 			wantConfig: &decoder.CloudStackExecConfig{
 				Profiles: []decoder.CloudStackProfileConfig{
 					{
-						Name:          "Global",
+						Name:          decoder.CloudStackGlobalAZ,
 						ApiKey:        "test-key1",
 						SecretKey:     "test-secret1",
 						ManagementUrl: "http://127.16.0.1:8080/client/api",
@@ -98,7 +98,7 @@ func TestCloudStackConfigDecoder(t *testing.T) {
 			wantConfig: &decoder.CloudStackExecConfig{
 				Profiles: []decoder.CloudStackProfileConfig{
 					{
-						Name:          "Global",
+						Name:          decoder.CloudStackGlobalAZ,
 						ApiKey:        "test-key1",
 						SecretKey:     "test-secret1",
 						ManagementUrl: "http://127.16.0.1:8080/client/api",

diff --git a/pkg/providers/cloudstack/mocks/client.go b/pkg/providers/cloudstack/mocks/client.go