issue-702, improvement of cloud provider settings flow

instaclustr · Feb 12, 2024 · 2d47df5 · 2d47df5
1 parent 27d6c7c
commit 2d47df5
Show file tree

Hide file tree

Showing 12 changed files with 355 additions and 107 deletions.
diff --git a/.secrets.baseline b/.secrets.baseline
@@ -386,7 +386,7 @@
         "filename": "apis/clusters/v1beta1/zz_generated.deepcopy.go",
         "hashed_secret": "44e17306b837162269a410204daaa5ecee4ec22c",
         "is_verified": false,
-        "line_number": 2223
+        "line_number": 2323
       }
     ],
     "apis/kafkamanagement/v1beta1/kafkauser_types.go": [
@@ -1126,5 +1126,5 @@
       }
     ]
   },
-  "generated_at": "2024-02-08T13:39:05Z"
+  "generated_at": "2024-02-09T12:49:33Z"
 }
diff --git a/apis/clusters/v1beta1/generic_spec.go b/apis/clusters/v1beta1/generic_spec.go
@@ -103,10 +103,10 @@ type GenericDataCentreSpec struct {
 	Region        string `json:"region"`
 	CloudProvider string `json:"cloudProvider"`
 	//+kubebuilder:default:=INSTACLUSTR
-	ProviderAccountName   string                   `json:"accountName,omitempty"`
-	Network               string                   `json:"network"`
-	Tags                  map[string]string        `json:"tags,omitempty"`
-	CloudProviderSettings []*CloudProviderSettings `json:"cloudProviderSettings,omitempty"`
+	ProviderAccountName   string               `json:"accountName,omitempty"`
+	Network               string               `json:"network"`
+	Tags                  map[string]string    `json:"tags,omitempty"`
+	CloudProviderSettings CloudProviderOptions `json:"cloudProviderSettings,omitempty"`
 }
 
 func (s *GenericDataCentreSpec) Equals(o *GenericDataCentreSpec) bool {
@@ -116,7 +116,7 @@ func (s *GenericDataCentreSpec) Equals(o *GenericDataCentreSpec) bool {
 		s.ProviderAccountName == o.ProviderAccountName &&
 		s.Network == o.Network &&
 		areTagsEqual(s.Tags, o.Tags) &&
-		slices.EqualsPtr(s.CloudProviderSettings, o.CloudProviderSettings)
+		s.CloudProviderSettings.Equal(o.CloudProviderSettings)
 }
 
 func (s *GenericDataCentreSpec) FromInstAPI(model *models.GenericDataCentreFields) {
@@ -126,28 +126,7 @@ func (s *GenericDataCentreSpec) FromInstAPI(model *models.GenericDataCentreField
 	s.ProviderAccountName = model.ProviderAccountName
 	s.Network = model.Network
 	s.Tags = tagsFromInstAPI(model.Tags)
-	s.CloudProviderSettings = cloudProviderSettingsFromInstAPI(model)
-}
-
-func (dc *GenericDataCentreSpec) CloudProviderSettingsToInstAPI() models.CloudProviderSettings {
-	instaModel := models.CloudProviderSettings{}
-
-	switch dc.CloudProvider {
-	case models.AWSVPC:
-		for _, providerSettings := range dc.CloudProviderSettings {
-			instaModel.AWSSettings = append(instaModel.AWSSettings, providerSettings.AWSToInstAPI())
-		}
-	case models.AZUREAZ:
-		for _, providerSettings := range dc.CloudProviderSettings {
-			instaModel.AzureSettings = append(instaModel.AzureSettings, providerSettings.AzureToInstAPI())
-		}
-	case models.GCP:
-		for _, providerSettings := range dc.CloudProviderSettings {
-			instaModel.GCPSettings = append(instaModel.GCPSettings, providerSettings.GCPToInstAPI())
-		}
-	}
-
-	return instaModel
+	s.CloudProviderSettings.FromInstAPI(&model.CloudProviderSettings)
 }
 
 func (s *GenericDataCentreSpec) ToInstAPI() models.GenericDataCentreFields {
@@ -158,6 +137,6 @@ func (s *GenericDataCentreSpec) ToInstAPI() models.GenericDataCentreFields {
 		Region:                s.Region,
 		ProviderAccountName:   s.ProviderAccountName,
 		Tags:                  tagsToInstAPI(s.Tags),
-		CloudProviderSettings: s.CloudProviderSettingsToInstAPI(),
+		CloudProviderSettings: s.CloudProviderSettings.ToInstAPI(),
 	}
 }
diff --git a/apis/clusters/v1beta1/kafka_webhook_test.go b/apis/clusters/v1beta1/kafka_webhook_test.go
@@ -195,7 +195,7 @@ var _ = Describe("Kafka Controller", Ordered, func() {
 			testKafkaManifest.Spec.DataCentres[0].Network = prevStringField
 
 			prevCloudProviderSettings := kafkaManifest.Spec.DataCentres[0].CloudProviderSettings
-			testKafkaManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{prevCloudProviderSettings[0], prevCloudProviderSettings[0]}
+			testKafkaManifest.Spec.DataCentres[0].CloudProviderSettings = CloudProviderOptions{prevCloudProviderSettings[0], prevCloudProviderSettings[0]}
 			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())
 			testKafkaManifest.Spec.DataCentres[0].CloudProviderSettings = prevCloudProviderSettings
 

diff --git a/apis/clusters/v1beta1/opensearch_types.go b/apis/clusters/v1beta1/opensearch_types.go
@@ -293,34 +293,6 @@ func tagsFromInstAPI(iTags []*models.Tag) map[string]string {
 	return newTags
 }
 
-func cloudProviderSettingsFromInstAPI(iDC *models.GenericDataCentreFields) (settings []*CloudProviderSettings) {
-	switch iDC.CloudProvider {
-	case models.AWSVPC:
-		for _, awsSetting := range iDC.AWSSettings {
-			settings = append(settings, &CloudProviderSettings{
-				CustomVirtualNetworkID: awsSetting.CustomVirtualNetworkID,
-				DiskEncryptionKey:      awsSetting.EBSEncryptionKey,
-				BackupBucket:           awsSetting.BackupBucket,
-			})
-		}
-	case models.GCP:
-		for _, gcpSetting := range iDC.GCPSettings {
-			settings = append(settings, &CloudProviderSettings{
-				CustomVirtualNetworkID:    gcpSetting.CustomVirtualNetworkID,
-				DisableSnapshotAutoExpiry: gcpSetting.DisableSnapshotAutoExpiry,
-			})
-		}
-	case models.AZUREAZ:
-		for _, azureSetting := range iDC.AzureSettings {
-			settings = append(settings, &CloudProviderSettings{
-				ResourceGroup: azureSetting.ResourceGroup,
-			})
-		}
-	}
-
-	return settings
-}
-
 func (c *OpenSearch) GetSpec() OpenSearchSpec { return c.Spec }
 
 func (c *OpenSearch) IsSpecEqual(spec OpenSearchSpec) bool {

diff --git a/apis/clusters/v1beta1/structs.go b/apis/clusters/v1beta1/structs.go
@@ -832,3 +832,119 @@ func (g GenericResizeSettings) Equal(o GenericResizeSettings) bool {
 
 	return true
 }
+
+type AWSSettings struct {
+	EncryptionKey          string `json:"encryptionKey,omitempty"`
+	CustomVirtualNetworkID string `json:"customVirtualNetworkID,omitempty"`
+	BackupBucket           string `json:"backupBucket,omitempty"`
+}
+
+type GCPSettings struct {
+	CustomVirtualNetworkID    string `json:"customVirtualNetworkID,omitempty"`
+	DisableSnapshotAutoExpiry string `json:"disableSnapshotAutoExpiry,omitempty"`
+}
+
+type AzureSettings struct {
+	ResourceGroup          string `json:"resourceGroup,omitempty"`
+	CustomVirtualNetworkID string `json:"customVirtualNetworkID,omitempty"`
+	StorageNetwork         string `json:"storageNetwork,omitempty"`
+}
+
+type CloudProviderOpts struct {
+	AWSSettings   *AWSSettings   `json:"awsSettings,omitempty"`
+	GCPSettings   *GCPSettings   `json:"gcpSettings,omitempty"`
+	AzureSettings *AzureSettings `json:"azureSettings,omitempty"`
+}
+
+type CloudProviderOptions []*CloudProviderOpts
+
+func (c *CloudProviderOptions) FromInstAPI(instaModel *models.CloudProviderSettings) {
+	var s *CloudProviderOpts
+
+	switch {
+	case len(instaModel.AWSSettings) > 0:
+		s = &CloudProviderOpts{
+			AWSSettings: &AWSSettings{
+				EncryptionKey:          instaModel.AWSSettings[0].EBSEncryptionKey,
+				CustomVirtualNetworkID: instaModel.AWSSettings[0].CustomVirtualNetworkID,
+				BackupBucket:           instaModel.AWSSettings[0].BackupBucket,
+			},
+		}
+	case len(instaModel.GCPSettings) > 0:
+		s = &CloudProviderOpts{
+			GCPSettings: &GCPSettings{
+				CustomVirtualNetworkID:    instaModel.GCPSettings[0].CustomVirtualNetworkID,
+				DisableSnapshotAutoExpiry: instaModel.GCPSettings[0].DisableSnapshotAutoExpiry,
+			},
+		}
+	case len(instaModel.AzureSettings) > 0:
+		s = &CloudProviderOpts{
+			AzureSettings: &AzureSettings{
+				ResourceGroup:          instaModel.AzureSettings[0].ResourceGroup,
+				CustomVirtualNetworkID: instaModel.AzureSettings[0].CustomVirtualNetworkID,
+				StorageNetwork:         instaModel.AzureSettings[0].StorageNetwork,
+			},
+		}
+	}
+
+	if s != nil {
+		*c = append(*c, s)
+	}
+}
+
+func (c *CloudProviderOptions) ToInstAPI() models.CloudProviderSettings {
+	if len(*c) == 0 {
+		return models.CloudProviderSettings{}
+	}
+
+	var instaModel models.CloudProviderSettings
+
+	s := (*c)[0]
+
+	switch {
+	case s.AWSSettings != nil:
+		instaModel.AWSSettings = append(instaModel.AWSSettings, &models.AWSSetting{
+			EBSEncryptionKey:       s.AWSSettings.EncryptionKey,
+			CustomVirtualNetworkID: s.AWSSettings.CustomVirtualNetworkID,
+			BackupBucket:           s.AWSSettings.BackupBucket,
+		})
+	case s.GCPSettings != nil:
+		instaModel.GCPSettings = append(instaModel.GCPSettings, &models.GCPSetting{
+			CustomVirtualNetworkID:    s.GCPSettings.CustomVirtualNetworkID,
+			DisableSnapshotAutoExpiry: s.GCPSettings.DisableSnapshotAutoExpiry,
+		})
+	case s.AzureSettings != nil:
+		instaModel.AzureSettings = append(instaModel.AzureSettings, &models.AzureSetting{
+			ResourceGroup:          s.AzureSettings.ResourceGroup,
+			CustomVirtualNetworkID: s.AzureSettings.CustomVirtualNetworkID,
+			StorageNetwork:         s.AzureSettings.StorageNetwork,
+		})
+	}
+
+	return instaModel
+}
+
+func (c CloudProviderOptions) Equal(o CloudProviderOptions) bool {
+	if len(c) != len(o) {
+		return false
+	}
+
+	for i := range c {
+		switch {
+		case c[i].AWSSettings != nil:
+			if *c[i].AWSSettings != *o[i].AWSSettings {
+				return false
+			}
+		case c[i].GCPSettings != nil:
+			if *c[i].GCPSettings != *o[i].GCPSettings {
+				return false
+			}
+		case c[i].AzureSettings != nil:
+			if *c[i].AzureSettings != *o[i].AzureSettings {
+				return false
+			}
+		}
+	}
+
+	return true
+}
diff --git a/apis/clusters/v1beta1/validation.go b/apis/clusters/v1beta1/validation.go
@@ -18,6 +18,7 @@ package v1beta1
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -27,7 +28,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/instaclustr/operator/pkg/models"
-	"github.com/instaclustr/operator/pkg/utils/slices"
 	"github.com/instaclustr/operator/pkg/validation"
 )
 
@@ -381,7 +381,7 @@ func (s *GenericDataCentreSpec) validateCreation() error {
 	}
 
 	for _, cp := range s.CloudProviderSettings {
-		err := cp.ValidateCreation()
+		err := cp.validateCreation()
 		if err != nil {
 			return err
 		}
@@ -408,10 +408,29 @@ func (s *GenericDataCentreSpec) ValidateOnPremisesCreation() error {
 	return nil
 }
 
-func (s *GenericDataCentreSpec) validateImmutableCloudProviderSettingsUpdate(oldSettings []*CloudProviderSettings) error {
-	if !slices.EqualsPtr(s.CloudProviderSettings, oldSettings) {
+func (s *GenericDataCentreSpec) validateImmutableCloudProviderSettingsUpdate(oldSettings CloudProviderOptions) error {
+	if !s.CloudProviderSettings.Equal(oldSettings) {
 		return models.ErrImmutableCloudProviderSettings
 	}
 
 	return nil
 }
+
+func (c *CloudProviderOpts) validateCreation() error {
+	fieldSet := 0
+	if c.AWSSettings != nil {
+		fieldSet++
+	}
+	if c.AzureSettings != nil {
+		fieldSet++
+	}
+	if c.GCPSettings != nil {
+		fieldSet++
+	}
+
+	if fieldSet > 1 {
+		return errors.New("only one of [awsSettings, gcpSetting, azureSettings] should be set")
+	}
+
+	return nil
+}