issue-702, improvement of cloud provider settings flow

.secrets.baseline

@@ -75,6 +75,10 @@
     {
       "path": "detect_secrets.filters.allowlist.is_line_allowlisted"
     },
+    {
+      "path": "detect_secrets.filters.common.is_baseline_file",
+      "filename": ".secrets.baseline"
+    },
     {
       "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies",
       "min_level": 2
@@ -315,21 +319,21 @@
         "filename": "apis/clusters/v1beta1/postgresql_types.go",
         "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db",
         "is_verified": false,
-        "line_number": 351
+        "line_number": 355
       },
       {
         "type": "Secret Keyword",
         "filename": "apis/clusters/v1beta1/postgresql_types.go",
         "hashed_secret": "a3d7d4a96d18c8fc5a1cf9c9c01c45b4690b4008",
         "is_verified": false,
-        "line_number": 357
+        "line_number": 361
       },
       {
         "type": "Secret Keyword",
         "filename": "apis/clusters/v1beta1/postgresql_types.go",
         "hashed_secret": "a57ce131bd944bdf8ba2f2f93e179dc416ed0315",
         "is_verified": false,
-        "line_number": 477
+        "line_number": 481
       }
     ],
     "apis/clusters/v1beta1/redis_types.go": [
@@ -386,7 +390,7 @@
         "filename": "apis/clusters/v1beta1/zz_generated.deepcopy.go",
         "hashed_secret": "44e17306b837162269a410204daaa5ecee4ec22c",
         "is_verified": false,
-        "line_number": 2198
+        "line_number": 2290
       }
     ],
     "apis/kafkamanagement/v1beta1/kafkauser_types.go": [
@@ -696,7 +700,7 @@
         "filename": "doc/clusters/kafka.md",
         "hashed_secret": "92429d82a41e930486c6de5ebda9602d55c39986",
         "is_verified": false,
-        "line_number": 166
+        "line_number": 184
       }
     ],
     "doc/kafkamanagment/kafka-user.md": [
@@ -1126,5 +1130,5 @@
       }
     ]
   },
-  "generated_at": "2024-02-13T09:35:50Z"
+  "generated_at": "2024-02-12T11:41:39Z"
 }

Makefile

@@ -83,7 +83,7 @@ test-webhooks:
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./apis/clusters/v1beta1 -coverprofile cover.out
 
 .PHONY: test
- test: manifests generate fmt vet docker-build-server-stub run-server-stub envtest test-clusters test-clusterresources test-webhooks test-kafkamanagement test-users stop-server-stub
+ test: manifests generate fmt vet docker-build-server-stub run-server-stub envtest test-webhooks test-clusters test-clusterresources test-webhooks test-kafkamanagement test-users stop-server-stub
 
 .PHONY: goimports
 goimports:

apis/clusters/v1beta1/cassandra_webhook.go

@@ -310,7 +310,7 @@ func (cs *CassandraSpec) validateDataCentresUpdate(oldSpec CassandraSpec) error
 			return fmt.Errorf("deleting nodes is not supported. Number of nodes must be greater than: %v", oldDC.NodesNumber)
 		}
 
-		err := newDC.validateImmutableCloudProviderSettingsUpdate(oldDC.CloudProviderSettings)
+		err := newDC.validateImmutableCloudProviderSettingsUpdate(&oldDC.GenericDataCentreSpec)
 		if err != nil {
 			return err
 		}

apis/clusters/v1beta1/generic_spec.go

@@ -99,14 +99,47 @@ func (s *GenericClusterSpec) ClusterSettingsUpdateToInstAPI() *models.ClusterSet
 }
 
 type GenericDataCentreSpec struct {
-	Name          string `json:"name,omitempty"`
-	Region        string `json:"region"`
+	// A logical name for the data centre within a cluster.
+	// These names must be unique in the cluster.
+	Name string `json:"name"`
+
+	// Region of the Data Centre.
+	Region string `json:"region"`
+
+	// Name of a cloud provider service.
 	CloudProvider string `json:"cloudProvider"`
+
+	// For customers running in their own account.
+	// Your provider account can be found on the Create Cluster page on the Instaclustr Console,
+	// or the "Provider Account" property on any existing cluster.
+	// For customers provisioning on Instaclustr's cloud provider accounts, this property may be omitted.
+	//
 	//+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"`
+
+	// The private network address block for the Data Centre specified using CIDR address notation.
+	// The network must have a prefix length between /12 and /22 and must be part of a private address space.
+	Network string `json:"network"`
+
+	// List of tags to apply to the Data Centre.
+	// Tags are metadata labels which allow you to identify, categorize and filter clusters.
+	// This can be useful for grouping together clusters into applications, environments, or any category that you require.
+	Tags map[string]string `json:"tags,omitempty"`
+
+	// AWS specific settings for the Data Centre. Cannot be provided with GCP or Azure settings.
+	//
+	//+kubebuilder:validation:MaxItems:=1
+	AWSSettings []*AWSSettings `json:"awsSettings,omitempty"`
+
+	// GCP specific settings for the Data Centre. Cannot be provided with AWS or Azure settings.
+	//
+	//+kubebuilder:validation:MaxItems:=1
+	GCPSettings []*GCPSettings `json:"gcpSettings,omitempty"`
+
+	// Azure specific settings for the Data Centre. Cannot be provided with AWS or GCP settings.
+	//
+	//+kubebuilder:validation:MaxItems:=1
+	AzureSettings []*AzureSettings `json:"azureSettings,omitempty"`
 }
 
 func (s *GenericDataCentreSpec) Equals(o *GenericDataCentreSpec) bool {
@@ -116,7 +149,9 @@ 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)
+		slices.EqualsPtr(s.AWSSettings, o.AWSSettings) &&
+		slices.EqualsPtr(s.GCPSettings, o.GCPSettings) &&
+		slices.EqualsPtr(s.AzureSettings, o.AzureSettings)
 }
 
 func (s *GenericDataCentreSpec) FromInstAPI(model *models.GenericDataCentreFields) {
@@ -126,28 +161,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.cloudProviderSettingsFromInstAPI(model.CloudProviderSettings)
 }
 
 func (s *GenericDataCentreSpec) ToInstAPI() models.GenericDataCentreFields {
@@ -158,6 +172,64 @@ func (s *GenericDataCentreSpec) ToInstAPI() models.GenericDataCentreFields {
 		Region:                s.Region,
 		ProviderAccountName:   s.ProviderAccountName,
 		Tags:                  tagsToInstAPI(s.Tags),
-		CloudProviderSettings: s.CloudProviderSettingsToInstAPI(),
+		CloudProviderSettings: s.cloudProviderSettingsToInstAPI(),
+	}
+}
+
+func (s *GenericDataCentreSpec) cloudProviderSettingsToInstAPI() *models.CloudProviderSettings {
+	var instaModel *models.CloudProviderSettings
+
+	switch {
+	case len(s.AWSSettings) > 0:
+		setting := s.AWSSettings[0]
+		instaModel = &models.CloudProviderSettings{AWSSettings: []*models.AWSSetting{{
+			EBSEncryptionKey:       setting.DiskEncryptionKey,
+			CustomVirtualNetworkID: setting.CustomVirtualNetworkID,
+			BackupBucket:           setting.BackupBucket,
+		}}}
+	case len(s.GCPSettings) > 0:
+		setting := s.GCPSettings[0]
+		instaModel = &models.CloudProviderSettings{GCPSettings: []*models.GCPSetting{{
+			CustomVirtualNetworkID:    setting.CustomVirtualNetworkID,
+			DisableSnapshotAutoExpiry: setting.DisableSnapshotAutoExpiry,
+		}}}
+	case len(s.AzureSettings) > 0:
+		setting := s.AzureSettings[0]
+		instaModel = &models.CloudProviderSettings{AzureSettings: []*models.AzureSetting{{
+			ResourceGroup:          setting.ResourceGroup,
+			CustomVirtualNetworkID: setting.CustomVirtualNetworkID,
+			StorageNetwork:         setting.StorageNetwork,
+		}}}
+	}
+
+	return instaModel
+}
+
+func (s *GenericDataCentreSpec) cloudProviderSettingsFromInstAPI(instaModel *models.CloudProviderSettings) {
+	if instaModel == nil {
+		return
+	}
+
+	switch {
+	case len(instaModel.AWSSettings) > 0:
+		setting := instaModel.AWSSettings[0]
+		s.AWSSettings = []*AWSSettings{{
+			DiskEncryptionKey:      setting.EBSEncryptionKey,
+			CustomVirtualNetworkID: setting.CustomVirtualNetworkID,
+			BackupBucket:           setting.BackupBucket,
+		}}
+	case len(instaModel.GCPSettings) > 0:
+		setting := instaModel.GCPSettings[0]
+		s.GCPSettings = []*GCPSettings{{
+			CustomVirtualNetworkID:    setting.CustomVirtualNetworkID,
+			DisableSnapshotAutoExpiry: setting.DisableSnapshotAutoExpiry,
+		}}
+	case len(instaModel.AzureSettings) > 0:
+		setting := instaModel.AzureSettings[0]
+		s.AzureSettings = []*AzureSettings{{
+			ResourceGroup:          setting.ResourceGroup,
+			CustomVirtualNetworkID: setting.CustomVirtualNetworkID,
+			StorageNetwork:         setting.StorageNetwork,
+		}}
 	}
 }

apis/clusters/v1beta1/kafka_webhook.go

@@ -334,7 +334,7 @@ func (ks *KafkaSpec) validateImmutableDataCentresFieldsUpdate(oldSpec *KafkaSpec
 			return fmt.Errorf("deleting nodes is not supported. Number of nodes must be greater than: %v", oldDC.NodesNumber)
 		}
 
-		err := newDC.validateImmutableCloudProviderSettingsUpdate(oldDC.CloudProviderSettings)
+		err := newDC.validateImmutableCloudProviderSettingsUpdate(&oldDC.GenericDataCentreSpec)
 		if err != nil {
 			return err
 		}

apis/clusters/v1beta1/kafka_webhook_test.go

@@ -194,10 +194,22 @@ var _ = Describe("Kafka Controller", Ordered, func() {
 			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())
 			testKafkaManifest.Spec.DataCentres[0].Network = prevStringField
 
-			prevCloudProviderSettings := kafkaManifest.Spec.DataCentres[0].CloudProviderSettings
-			testKafkaManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{prevCloudProviderSettings[0], prevCloudProviderSettings[0]}
+			prevAWSSettings := kafkaManifest.Spec.DataCentres[0].AWSSettings
+			testKafkaManifest.Spec.DataCentres[0].AWSSettings = []*AWSSettings{prevAWSSettings[0], prevAWSSettings[0]}
 			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())
-			testKafkaManifest.Spec.DataCentres[0].CloudProviderSettings = prevCloudProviderSettings
+			testKafkaManifest.Spec.DataCentres[0].AWSSettings = prevAWSSettings
+
+			prevGCPSettings := kafkaManifest.Spec.DataCentres[0].GCPSettings
+			gcpSettings := &GCPSettings{CustomVirtualNetworkID: "test-network-id", DisableSnapshotAutoExpiry: true}
+			testKafkaManifest.Spec.DataCentres[0].GCPSettings = []*GCPSettings{gcpSettings, gcpSettings}
+			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())
+			testKafkaManifest.Spec.DataCentres[0].GCPSettings = prevGCPSettings
+
+			prevAzureSettings := kafkaManifest.Spec.DataCentres[0].AzureSettings
+			azureSettings := &AzureSettings{ResourceGroup: "test-resource-group", CustomVirtualNetworkID: "test-network-id", StorageNetwork: "test-storage-network"}
+			testKafkaManifest.Spec.DataCentres[0].AzureSettings = []*AzureSettings{azureSettings, azureSettings}
+			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())
+			testKafkaManifest.Spec.DataCentres[0].AzureSettings = prevAzureSettings
 
 			testKafkaManifest.Spec.DataCentres[0].Tags["test"] = "test"
 			Expect(k8sClient.Patch(ctx, &testKafkaManifest, patch)).ShouldNot(Succeed())

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 {

apis/clusters/v1beta1/opensearch_webhook.go

@@ -351,7 +351,7 @@ func (oss *OpenSearchSpec) validateImmutableDataCentresUpdate(oldDCs []*OpenSear
 			return fmt.Errorf("cannot update immutable data centre fields: new spec: %v: old spec: %v", newDCImmutableFields, oldDCImmutableFields)
 		}
 
-		err := oldDC.validateImmutableCloudProviderSettingsUpdate(newDC.CloudProviderSettings)
+		err := oldDC.validateImmutableCloudProviderSettingsUpdate(&newDC.GenericDataCentreSpec)
 		if err != nil {
 			return err
 		}

apis/clusters/v1beta1/opensearch_webhook_test.go

@@ -66,21 +66,10 @@ var _ = Describe("Kafka Controller", Ordered, func() {
 			Expect(k8sClient.Create(ctx, &testOpenSearchManifest)).ShouldNot(Succeed())
 			testOpenSearchManifest.Spec.DataCentres[0].ProviderAccountName = prevStringValue
 
-			providerSettings := openSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0]
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{providerSettings, providerSettings}
-			Expect(k8sClient.Create(ctx, &testOpenSearchManifest)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{providerSettings}
-
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].ResourceGroup = "test"
-			Expect(k8sClient.Create(ctx, &testOpenSearchManifest)).ShouldNot(Succeed())
-
-			prevStringValue = openSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey = ""
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].CustomVirtualNetworkID = "test"
-			Expect(k8sClient.Create(ctx, &testOpenSearchManifest)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].ResourceGroup = ""
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].CustomVirtualNetworkID = ""
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey = prevStringValue
+			awsSettings := openSearchManifest.Spec.DataCentres[0].AWSSettings[0]
+			openSearchManifest.Spec.DataCentres[0].AWSSettings = []*AWSSettings{awsSettings, awsSettings}
+			Expect(k8sClient.Create(ctx, &openSearchManifest)).ShouldNot(Succeed())
+			openSearchManifest.Spec.DataCentres[0].AWSSettings = []*AWSSettings{awsSettings}
 
 			prevStringValue = openSearchManifest.Spec.DataCentres[0].Network
 			testOpenSearchManifest.Spec.DataCentres[0].Network = "test/test"
@@ -277,25 +266,25 @@ var _ = Describe("Kafka Controller", Ordered, func() {
 			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())
 			testOpenSearchManifest.Spec.DataCentres[0].NumberOfRacks -= 1
 
-			prevCloudProviderSettings := openSearchManifest.Spec.DataCentres[0].CloudProviderSettings
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{prevCloudProviderSettings[0], prevCloudProviderSettings[0]}
-			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings = []*CloudProviderSettings{prevCloudProviderSettings[0]}
+			prevAWSSettings := openSearchManifest.Spec.DataCentres[0].AWSSettings
+			openSearchManifest.Spec.DataCentres[0].AWSSettings = []*AWSSettings{prevAWSSettings[0], prevAWSSettings[0]}
+			Expect(k8sClient.Patch(ctx, &openSearchManifest, patch)).ShouldNot(Succeed())
+			openSearchManifest.Spec.DataCentres[0].AWSSettings = prevAWSSettings
 
-			prevStringValue = openSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey = "test"
+			prevStringValue = openSearchManifest.Spec.DataCentres[0].AWSSettings[0].DiskEncryptionKey
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].DiskEncryptionKey = "test"
 			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].DiskEncryptionKey = prevStringValue
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].DiskEncryptionKey = prevStringValue
 
-			prevStringValue = openSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].ResourceGroup
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].ResourceGroup = "test"
+			prevStringValue = openSearchManifest.Spec.DataCentres[0].AWSSettings[0].CustomVirtualNetworkID
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].CustomVirtualNetworkID = "test"
 			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].ResourceGroup = prevStringValue
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].CustomVirtualNetworkID = prevStringValue
 
-			prevStringValue = openSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].CustomVirtualNetworkID
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].CustomVirtualNetworkID = "test"
+			prevStringValue = openSearchManifest.Spec.DataCentres[0].AWSSettings[0].BackupBucket
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].BackupBucket = "test"
 			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())
-			testOpenSearchManifest.Spec.DataCentres[0].CloudProviderSettings[0].CustomVirtualNetworkID = prevStringValue
+			testOpenSearchManifest.Spec.DataCentres[0].AWSSettings[0].BackupBucket = prevStringValue
 
 			testOpenSearchManifest.Spec.DataCentres[0].Tags["test"] = "test"
 			Expect(k8sClient.Patch(ctx, &testOpenSearchManifest, patch)).ShouldNot(Succeed())

apis/clusters/v1beta1/postgresql_types.go

@@ -125,6 +125,9 @@ func (pg *PostgreSQL) GetJobID(jobName string) string {
 
 func (pg *PostgreSQL) NewPatch() client.Patch {
 	old := pg.DeepCopy()
+	if old.Annotations == nil {
+		old.Annotations = make(map[string]string)
+	}
 	old.Annotations[models.ResourceStateAnnotation] = ""
 	return client.MergeFrom(old)
 }