From 2d47df5c00c973ec5fd4a1260a48115cbc85e270 Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Fri, 9 Feb 2024 14:49:39 +0200 Subject: [PATCH] issue-702, improvement of cloud provider settings flow --- .secrets.baseline | 4 +- apis/clusters/v1beta1/generic_spec.go | 35 ++---- apis/clusters/v1beta1/kafka_webhook_test.go | 2 +- apis/clusters/v1beta1/opensearch_types.go | 28 ----- apis/clusters/v1beta1/structs.go | 116 ++++++++++++++++++ apis/clusters/v1beta1/validation.go | 27 +++- .../clusters/v1beta1/zz_generated.deepcopy.go | 106 +++++++++++++++- .../clusters.instaclustr.com_cassandras.yaml | 35 ++++-- .../clusters.instaclustr.com_kafkas.yaml | 35 ++++-- ...clusters.instaclustr.com_opensearches.yaml | 35 ++++-- .../bases/clusters.instaclustr.com_redis.yaml | 35 ++++-- pkg/models/apiv2.go | 4 +- 12 files changed, 355 insertions(+), 107 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 91ddac180..ed6f96c70 100644 --- 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 index a75507ed4..84d507365 100644 --- 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 index ecf1a74d8..0c35bf399 100644 --- 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 index 300e783aa..f43470f79 100644 --- 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 index eac0defd9..a81c4ef63 100644 --- 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 index a9b200a74..896a5ddf5 100644 --- 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 +} diff --git a/apis/clusters/v1beta1/zz_generated.deepcopy.go b/apis/clusters/v1beta1/zz_generated.deepcopy.go index 0baf832be..9bdb75716 100644 --- a/apis/clusters/v1beta1/zz_generated.deepcopy.go +++ b/apis/clusters/v1beta1/zz_generated.deepcopy.go @@ -58,6 +58,21 @@ func (in *AWSConnectorSettings) DeepCopy() *AWSConnectorSettings { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AWSSettings) DeepCopyInto(out *AWSSettings) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSSettings. +func (in *AWSSettings) DeepCopy() *AWSSettings { + if in == nil { + return nil + } + out := new(AWSSettings) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AdvancedVisibility) DeepCopyInto(out *AdvancedVisibility) { *out = *in @@ -98,6 +113,21 @@ func (in *AzureConnectorSettings) DeepCopy() *AzureConnectorSettings { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureSettings) DeepCopyInto(out *AzureSettings) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureSettings. +func (in *AzureSettings) DeepCopy() *AzureSettings { + if in == nil { + return nil + } + out := new(AzureSettings) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BundledCassandraSpec) DeepCopyInto(out *BundledCassandraSpec) { *out = *in @@ -601,6 +631,61 @@ func (in *CassandraStatus) DeepCopy() *CassandraStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in CloudProviderOptions) DeepCopyInto(out *CloudProviderOptions) { + { + in := &in + *out = make(CloudProviderOptions, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(CloudProviderOpts) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudProviderOptions. +func (in CloudProviderOptions) DeepCopy() CloudProviderOptions { + if in == nil { + return nil + } + out := new(CloudProviderOptions) + in.DeepCopyInto(out) + return *out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CloudProviderOpts) DeepCopyInto(out *CloudProviderOpts) { + *out = *in + if in.AWSSettings != nil { + in, out := &in.AWSSettings, &out.AWSSettings + *out = new(AWSSettings) + **out = **in + } + if in.GCPSettings != nil { + in, out := &in.GCPSettings, &out.GCPSettings + *out = new(GCPSettings) + **out = **in + } + if in.AzureSettings != nil { + in, out := &in.AzureSettings, &out.AzureSettings + *out = new(AzureSettings) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloudProviderOpts. +func (in *CloudProviderOpts) DeepCopy() *CloudProviderOpts { + if in == nil { + return nil + } + out := new(CloudProviderOpts) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CloudProviderSettings) DeepCopyInto(out *CloudProviderSettings) { *out = *in @@ -941,6 +1026,21 @@ func (in *GCPConnectorSettings) DeepCopy() *GCPConnectorSettings { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GCPSettings) DeepCopyInto(out *GCPSettings) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GCPSettings. +func (in *GCPSettings) DeepCopy() *GCPSettings { + if in == nil { + return nil + } + out := new(GCPSettings) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GenericClusterSpec) DeepCopyInto(out *GenericClusterSpec) { *out = *in @@ -979,12 +1079,12 @@ func (in *GenericDataCentreSpec) DeepCopyInto(out *GenericDataCentreSpec) { } if in.CloudProviderSettings != nil { in, out := &in.CloudProviderSettings, &out.CloudProviderSettings - *out = make([]*CloudProviderSettings, len(*in)) + *out = make(CloudProviderOptions, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] - *out = new(CloudProviderSettings) - **out = **in + *out = new(CloudProviderOpts) + (*in).DeepCopyInto(*out) } } } diff --git a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml index 181d29b0a..0403be774 100644 --- a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml +++ b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml @@ -63,16 +63,31 @@ spec: cloudProviderSettings: items: properties: - backupBucket: - type: string - customVirtualNetworkId: - type: string - disableSnapshotAutoExpiry: - type: string - diskEncryptionKey: - type: string - resourceGroup: - type: string + awsSettings: + properties: + backupBucket: + type: string + customVirtualNetworkID: + type: string + encryptionKey: + type: string + type: object + azureSettings: + properties: + customVirtualNetworkID: + type: string + resourceGroup: + type: string + storageNetwork: + type: string + type: object + gcpSettings: + properties: + customVirtualNetworkID: + type: string + disableSnapshotAutoExpiry: + type: string + type: object type: object type: array continuousBackup: diff --git a/config/crd/bases/clusters.instaclustr.com_kafkas.yaml b/config/crd/bases/clusters.instaclustr.com_kafkas.yaml index 1659e69dd..88c4702eb 100644 --- a/config/crd/bases/clusters.instaclustr.com_kafkas.yaml +++ b/config/crd/bases/clusters.instaclustr.com_kafkas.yaml @@ -69,16 +69,31 @@ spec: cloudProviderSettings: items: properties: - backupBucket: - type: string - customVirtualNetworkId: - type: string - disableSnapshotAutoExpiry: - type: string - diskEncryptionKey: - type: string - resourceGroup: - type: string + awsSettings: + properties: + backupBucket: + type: string + customVirtualNetworkID: + type: string + encryptionKey: + type: string + type: object + azureSettings: + properties: + customVirtualNetworkID: + type: string + resourceGroup: + type: string + storageNetwork: + type: string + type: object + gcpSettings: + properties: + customVirtualNetworkID: + type: string + disableSnapshotAutoExpiry: + type: string + type: object type: object type: array name: diff --git a/config/crd/bases/clusters.instaclustr.com_opensearches.yaml b/config/crd/bases/clusters.instaclustr.com_opensearches.yaml index f70eb8beb..66ba2c6f2 100644 --- a/config/crd/bases/clusters.instaclustr.com_opensearches.yaml +++ b/config/crd/bases/clusters.instaclustr.com_opensearches.yaml @@ -79,16 +79,31 @@ spec: cloudProviderSettings: items: properties: - backupBucket: - type: string - customVirtualNetworkId: - type: string - disableSnapshotAutoExpiry: - type: string - diskEncryptionKey: - type: string - resourceGroup: - type: string + awsSettings: + properties: + backupBucket: + type: string + customVirtualNetworkID: + type: string + encryptionKey: + type: string + type: object + azureSettings: + properties: + customVirtualNetworkID: + type: string + resourceGroup: + type: string + storageNetwork: + type: string + type: object + gcpSettings: + properties: + customVirtualNetworkID: + type: string + disableSnapshotAutoExpiry: + type: string + type: object type: object type: array name: diff --git a/config/crd/bases/clusters.instaclustr.com_redis.yaml b/config/crd/bases/clusters.instaclustr.com_redis.yaml index fcdfc7ae6..6032d2074 100644 --- a/config/crd/bases/clusters.instaclustr.com_redis.yaml +++ b/config/crd/bases/clusters.instaclustr.com_redis.yaml @@ -62,16 +62,31 @@ spec: cloudProviderSettings: items: properties: - backupBucket: - type: string - customVirtualNetworkId: - type: string - disableSnapshotAutoExpiry: - type: string - diskEncryptionKey: - type: string - resourceGroup: - type: string + awsSettings: + properties: + backupBucket: + type: string + customVirtualNetworkID: + type: string + encryptionKey: + type: string + type: object + azureSettings: + properties: + customVirtualNetworkID: + type: string + resourceGroup: + type: string + storageNetwork: + type: string + type: object + gcpSettings: + properties: + customVirtualNetworkID: + type: string + disableSnapshotAutoExpiry: + type: string + type: object type: object type: array masterNodes: diff --git a/pkg/models/apiv2.go b/pkg/models/apiv2.go index 9473551cf..74bcc4cc8 100644 --- a/pkg/models/apiv2.go +++ b/pkg/models/apiv2.go @@ -75,7 +75,9 @@ type GCPSetting struct { } type AzureSetting struct { - ResourceGroup string `json:"resourceGroup,omitempty"` + ResourceGroup string `json:"resourceGroup,omitempty"` + CustomVirtualNetworkID string `json:"customVirtualNetworkID,omitempty"` + StorageNetwork string `json:"storageNetwork,omitempty"` } type Tag struct {