ec_deployment: Change remote_cluster to TypeSet (#368)

Updates the `elasticsearch.remote_cluster` blocks from `schema.TypeList` to `schema.TypeSet`, allowing users to set the blocks in an arbirary order. Additionally, it updates the `elasticsearch.remote_cluster.alias` to be Required instead of Optional since it was never intended to be optional. Signed-off-by: Marc Lopez Rubio <[email protected]>
elastic · Sep 10, 2021 · 0a8f288 · 0a8f288
1 parent 2a3883f
commit 0a8f288
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 121 deletions.
diff --git a/.changelog/368.txt b/.changelog/368.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/ec_deployment: Changes the `ec_deployment.elasticsearch.remote_cluster` block to `schema.TypeSet` to allow specifying the blocks in any order.
+```
diff --git a/docs/resources/ec_deployment.md b/docs/resources/ec_deployment.md
@@ -287,7 +287,7 @@ The optional `elasticsearch.config` block supports the following arguments:
 The optional `elasticsearch.remote_cluster` block can be set multiple times. It represents one or multiple remote clusters to which the local Elasticsearch cluster connects for Cross Cluster Search and supports the following settings:
 
 * `deployment_id` (Required) Remote deployment ID.
-* `alias` (Optional) Alias for the Cross Cluster Search binding.
+* `alias` (Required) Alias for the Cross Cluster Search binding.
 * `ref_id` (Optional) Remote Elasticsearch `ref_id`. The default value `main-elasticsearch` is recommended.
 * `skip_unavailable` (Optional) If true, skip the cluster during search when disconnected. Defaults to `false`.
 

diff --git a/ec/acc/deployment_ccs_test.go b/ec/acc/deployment_ccs_test.go
@@ -29,7 +29,7 @@ import (
 // This test case takes that on a ccs "ec_deployment".
 func TestAccDeployment_ccs(t *testing.T) {
 	ccsResName := "ec_deployment.ccs"
-	sourceResName := "ec_deployment.source_ccs"
+	sourceResName := "ec_deployment.source_ccs.0"
 
 	ccsRandomName := prefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	sourceRandomName := prefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
@@ -51,17 +51,23 @@ func TestAccDeployment_ccs(t *testing.T) {
 				// Create a CCS deployment with the default settings.
 				Config: cfg,
 				Check: resource.ComposeAggregateTestCheckFunc(
+
 					// CCS Checks
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.#", "1"),
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.topology.#", "1"),
 					resource.TestCheckResourceAttrSet(ccsResName, "elasticsearch.0.topology.0.instance_configuration_id"),
 					// CCS defaults to 1g.
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.topology.0.size", "1g"),
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.topology.0.size_resource", "memory"),
+
 					// Remote cluster settings
-					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.#", "1"),
+					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.#", "3"),
 					resource.TestCheckResourceAttrSet(ccsResName, "elasticsearch.0.remote_cluster.0.deployment_id"),
-					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.0.alias", "my_source_ccs"),
+					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.0.alias", fmt.Sprint(sourceRandomName, "-0")),
+					resource.TestCheckResourceAttrSet(ccsResName, "elasticsearch.0.remote_cluster.1.deployment_id"),
+					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.1.alias", fmt.Sprint(sourceRandomName, "-1")),
+					resource.TestCheckResourceAttrSet(ccsResName, "elasticsearch.0.remote_cluster.2.deployment_id"),
+					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.remote_cluster.2.alias", fmt.Sprint(sourceRandomName, "-2")),
 
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.topology.0.node_type_data", ""),
 					resource.TestCheckResourceAttr(ccsResName, "elasticsearch.0.topology.0.node_type_ingest", ""),

diff --git a/ec/acc/testdata/deployment_ccs_1.tf b/ec/acc/testdata/deployment_ccs_1.tf
@@ -10,15 +10,19 @@ resource "ec_deployment" "ccs" {
   deployment_template_id = "%s"
 
   elasticsearch {
-    remote_cluster {
-      deployment_id = ec_deployment.source_ccs.id
-      alias         = "my_source_ccs"
+    dynamic "remote_cluster" {
+      for_each = ec_deployment.source_ccs
+      content {
+        deployment_id = remote_cluster.value.id
+        alias         = remote_cluster.value.name
+      }
     }
   }
 }
 
 resource "ec_deployment" "source_ccs" {
-  name                   = "%s"
+  count                  = 3
+  name                   = "%s-${count.index}"
   region                 = "%s"
   version                = data.ec_stack.latest.version
   deployment_template_id = "%s"
@@ -30,4 +34,4 @@ resource "ec_deployment" "source_ccs" {
       size       = "1g"
     }
   }
-}
+}
diff --git a/ec/ecresource/deploymentresource/elasticsearch_flatteners.go b/ec/ecresource/deploymentresource/elasticsearch_flatteners.go
@@ -75,8 +75,8 @@ func flattenEsResources(in []*models.ElasticsearchResourceInfo, name string, rem
 
 		m["config"] = flattenEsConfig(plan.Elasticsearch)
 
-		if r := flattenEsRemotes(remotes); len(r) > 0 {
-			m["remote_cluster"] = r
+		if remotes := flattenEsRemotes(remotes); remotes.Len() > 0 {
+			m["remote_cluster"] = remotes
 		}
 
 		extensions := schema.NewSet(esExtensionHash, nil)
@@ -241,8 +241,8 @@ func flattenEsConfig(cfg *models.ElasticsearchConfiguration) []interface{} {
 	return []interface{}{m}
 }
 
-func flattenEsRemotes(in models.RemoteResources) []interface{} {
-	var res []interface{}
+func flattenEsRemotes(in models.RemoteResources) *schema.Set {
+	res := newElasticsearchRemoteSet()
 	for _, r := range in.Resources {
 		var m = make(map[string]interface{})
 		if r.DeploymentID != nil && *r.DeploymentID != "" {
@@ -260,12 +260,19 @@ func flattenEsRemotes(in models.RemoteResources) []interface{} {
 		if r.SkipUnavailable != nil {
 			m["skip_unavailable"] = *r.SkipUnavailable
 		}
-		res = append(res, m)
+		res.Add(m)
 	}
 
 	return res
 }
 
+func newElasticsearchRemoteSet(remotes ...interface{}) *schema.Set {
+	return schema.NewSet(
+		schema.HashResource(elasticsearchRemoteCluster().Elem.(*schema.Resource)),
+		remotes,
+	)
+}
+
 func flattenEsBundles(in []*models.ElasticsearchUserBundle) []interface{} {
 	result := make([]interface{}, 0, len(in))
 	for _, bundle := range in {

diff --git a/ec/ecresource/deploymentresource/elasticsearch_flatteners_test.go b/ec/ecresource/deploymentresource/elasticsearch_flatteners_test.go
@@ -252,96 +252,6 @@ func Test_flattenEsResource(t *testing.T) {
 				}},
 			}},
 		},
-		{
-			name: "resource with remote clusters set",
-			args: args{
-				in: []*models.ElasticsearchResourceInfo{{
-					Region: ec.String("some-region"),
-					RefID:  ec.String("main-elasticsearch"),
-					Info: &models.ElasticsearchClusterInfo{
-						ClusterID:   &mock.ValidClusterID,
-						ClusterName: ec.String("some-name"),
-						Region:      "some-region",
-						Status:      ec.String("started"),
-						Metadata: &models.ClusterMetadataInfo{
-							Endpoint: "othercluster.cloud.elastic.co",
-							Ports: &models.ClusterMetadataPortInfo{
-								HTTP:  ec.Int32(9200),
-								HTTPS: ec.Int32(9243),
-							},
-						},
-						PlanInfo: &models.ElasticsearchClusterPlansInfo{
-							Current: &models.ElasticsearchClusterPlanInfo{
-								Plan: &models.ElasticsearchClusterPlan{
-									Elasticsearch: &models.ElasticsearchConfiguration{
-										Version: "7.7.0",
-									},
-									ClusterTopology: []*models.ElasticsearchClusterTopologyElement{{
-										ID:                      "hot_content",
-										ZoneCount:               1,
-										InstanceConfigurationID: "aws.data.highio.i3",
-										Size: &models.TopologySize{
-											Resource: ec.String("memory"),
-											Value:    ec.Int32(2048),
-										},
-										NodeType: &models.ElasticsearchNodeType{
-											Data:   ec.Bool(true),
-											Ingest: ec.Bool(true),
-											Master: ec.Bool(true),
-											Ml:     ec.Bool(false),
-										},
-									}},
-								},
-							},
-						},
-					},
-				}},
-				remotes: models.RemoteResources{Resources: []*models.RemoteResourceRef{
-					{
-						DeploymentID:       ec.String("someid"),
-						Alias:              ec.String("alias"),
-						SkipUnavailable:    ec.Bool(true),
-						ElasticsearchRefID: ec.String("main-elasticsearch"),
-					},
-					{
-						DeploymentID:       ec.String("some other id"),
-						ElasticsearchRefID: ec.String("main-elasticsearch"),
-					},
-				}},
-			},
-			want: []interface{}{map[string]interface{}{
-				"ref_id":         "main-elasticsearch",
-				"resource_id":    mock.ValidClusterID,
-				"region":         "some-region",
-				"http_endpoint":  "http://othercluster.cloud.elastic.co:9200",
-				"https_endpoint": "https://othercluster.cloud.elastic.co:9243",
-				"config":         func() []interface{} { return nil }(),
-				"remote_cluster": []interface{}{
-					map[string]interface{}{
-						"alias":            "alias",
-						"deployment_id":    "someid",
-						"ref_id":           "main-elasticsearch",
-						"skip_unavailable": true,
-					},
-					map[string]interface{}{
-						"deployment_id": "some other id",
-						"ref_id":        "main-elasticsearch",
-					},
-				},
-				"topology": []interface{}{map[string]interface{}{
-					"config":                    func() []interface{} { return nil }(),
-					"id":                        "hot_content",
-					"instance_configuration_id": "aws.data.highio.i3",
-					"size":                      "2g",
-					"size_resource":             "memory",
-					"node_type_data":            "true",
-					"node_type_ingest":          "true",
-					"node_type_master":          "true",
-					"node_type_ml":              "false",
-					"zone_count":                int32(1),
-				}},
-			}},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/ec/ecresource/deploymentresource/elasticsearch_remote_cluster_expanders.go b/ec/ecresource/deploymentresource/elasticsearch_remote_cluster_expanders.go
@@ -31,7 +31,7 @@ func handleRemoteClusters(d *schema.ResourceData, client *api.API) error {
 	}
 
 	remoteResources := expandRemoteClusters(
-		d.Get("elasticsearch.0.remote_cluster").([]interface{}),
+		d.Get("elasticsearch.0.remote_cluster").(*schema.Set),
 	)
 
 	return esremoteclustersapi.Update(esremoteclustersapi.UpdateParams{
@@ -42,10 +42,10 @@ func handleRemoteClusters(d *schema.ResourceData, client *api.API) error {
 	})
 }
 
-func expandRemoteClusters(raw []interface{}) *models.RemoteResources {
+func expandRemoteClusters(set *schema.Set) *models.RemoteResources {
 	res := models.RemoteResources{Resources: []*models.RemoteResourceRef{}}
 
-	for _, r := range raw {
+	for _, r := range set.List() {
 		var resourceRef models.RemoteResourceRef
 		m := r.(map[string]interface{})
 
@@ -73,7 +73,7 @@ func expandRemoteClusters(raw []interface{}) *models.RemoteResources {
 
 func keyIsEmptyUnchanged(d *schema.ResourceData, k string) bool {
 	old, new := d.GetChange(k)
-	oldSlice := old.([]interface{})
-	newSlice := new.([]interface{})
-	return len(oldSlice) == 0 && len(newSlice) == 0
+	oldSlice := old.(*schema.Set)
+	newSlice := new.(*schema.Set)
+	return oldSlice.Len() == 0 && newSlice.Len() == 0
 }
diff --git a/ec/ecresource/deploymentresource/elasticsearch_remote_cluster_expanders_test.go b/ec/ecresource/deploymentresource/elasticsearch_remote_cluster_expanders_test.go
@@ -105,7 +105,7 @@ func Test_handleRemoteClusters(t *testing.T) {
 
 func Test_expandRemoteClusters(t *testing.T) {
 	type args struct {
-		raw []interface{}
+		set *schema.Set
 	}
 	tests := []struct {
 		name string
@@ -114,11 +114,12 @@ func Test_expandRemoteClusters(t *testing.T) {
 	}{
 		{
 			name: "wants no error or empty res",
+			args: args{set: newElasticsearchRemoteSet()},
 			want: &models.RemoteResources{Resources: []*models.RemoteResourceRef{}},
 		},
 		{
 			name: "expands remotes",
-			args: args{raw: []interface{}{
+			args: args{set: newElasticsearchRemoteSet([]interface{}{
 				map[string]interface{}{
 					"alias":            "alias",
 					"deployment_id":    "someid",
@@ -129,24 +130,24 @@ func Test_expandRemoteClusters(t *testing.T) {
 					"deployment_id": "some other id",
 					"ref_id":        "main-elasticsearch",
 				},
-			}},
+			}...)},
 			want: &models.RemoteResources{Resources: []*models.RemoteResourceRef{
 				{
-					Alias:              ec.String("alias"),
-					DeploymentID:       ec.String("someid"),
+					DeploymentID:       ec.String("some other id"),
 					ElasticsearchRefID: ec.String("main-elasticsearch"),
-					SkipUnavailable:    ec.Bool(true),
 				},
 				{
-					DeploymentID:       ec.String("some other id"),
+					Alias:              ec.String("alias"),
+					DeploymentID:       ec.String("someid"),
 					ElasticsearchRefID: ec.String("main-elasticsearch"),
+					SkipUnavailable:    ec.Bool(true),
 				},
 			}},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := expandRemoteClusters(tt.args.raw)
+			got := expandRemoteClusters(tt.args.set)
 			assert.Equal(t, tt.want, got)
 		})
 	}

diff --git a/ec/ecresource/deploymentresource/schema_elasticsearch.go b/ec/ecresource/deploymentresource/schema_elasticsearch.go
@@ -319,9 +319,8 @@ func elasticsearchConfig() *schema.Schema {
 
 func elasticsearchRemoteCluster() *schema.Schema {
 	return &schema.Schema{
-		Type:        schema.TypeList,
+		Type:        schema.TypeSet,
 		Optional:    true,
-		MinItems:    1,
 		Description: "Optional Elasticsearch remote clusters to configure for the Elasticsearch resource, can be set multiple times",
 		Elem: &schema.Resource{
 			Schema: map[string]*schema.Schema{
@@ -335,7 +334,7 @@ func elasticsearchRemoteCluster() *schema.Schema {
 					Description:  "Alias for this Cross Cluster Search binding",
 					Type:         schema.TypeString,
 					ValidateFunc: validation.StringIsNotEmpty,
-					Optional:     true,
+					Required:     true,
 				},
 				"ref_id": {
 					Description: `Remote elasticsearch "ref_id", it is best left to the default value`,