ec_traffic_filter: Fix rule ordering bug

ec/acc/deployment_traffic_filter_test.go

@@ -31,8 +31,10 @@ func TestAccDeploymentTrafficFilter_basic(t *testing.T) {
 	randomName := prefix + acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	startCfg := "testdata/deployment_traffic_filter_basic.tf"
 	updateCfg := "testdata/deployment_traffic_filter_basic_update.tf"
+	updateLargeCfg := "testdata/deployment_traffic_filter_basic_update_large.tf"
 	cfg := fixtureAccDeploymentTrafficFilterResourceBasic(t, startCfg, randomName, getRegion())
 	updateConfigCfg := fixtureAccDeploymentTrafficFilterResourceBasic(t, updateCfg, randomName, getRegion())
+	updateLargeConfigCfg := fixtureAccDeploymentTrafficFilterResourceBasic(t, updateLargeCfg, randomName, getRegion())
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },
@@ -48,20 +50,34 @@ func TestAccDeploymentTrafficFilter_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resName, "rule.0.source", "0.0.0.0/0"),
 				),
 			},
-			// Ensure that no diff is generated.
-			{Config: cfg, PlanOnly: true},
 			{
 				Config: updateConfigCfg,
 				Check: checkBasicDeploymentTrafficFilterResource(resName, randomName,
 					resource.TestCheckResourceAttr(resName, "include_by_default", "false"),
 					resource.TestCheckResourceAttr(resName, "type", "ip"),
 					resource.TestCheckResourceAttr(resName, "rule.#", "2"),
-					resource.TestCheckResourceAttr(resName, "rule.0.source", "0.0.0.0/0"),
-					resource.TestCheckResourceAttr(resName, "rule.1.source", "1.1.1.0/24"),
+					resource.TestCheckTypeSetElemNestedAttrs(resName, "rule.*", map[string]string{
+						"source": "0.0.0.0/0",
+					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resName, "rule.*", map[string]string{
+						"source": "1.1.1.0/24",
+					}),
+				),
+			},
+			{
+				Config: updateLargeConfigCfg,
+				Check: checkBasicDeploymentTrafficFilterResource(resName, randomName,
+					resource.TestCheckResourceAttr(resName, "include_by_default", "false"),
+					resource.TestCheckResourceAttr(resName, "type", "ip"),
+					resource.TestCheckResourceAttr(resName, "rule.#", "16"),
+					resource.TestCheckTypeSetElemNestedAttrs(resName, "rule.*", map[string]string{
+						"source": "8.8.8.8/24",
+					}),
+					resource.TestCheckTypeSetElemNestedAttrs(resName, "rule.*", map[string]string{
+						"source": "8.8.4.4/24",
+					}),
 				),
 			},
-			// Ensure that no diff is generated.
-			{Config: updateConfigCfg, PlanOnly: true},
 			{
 				ResourceName:            resName,
 				ImportState:             true,

ec/acc/testdata/deployment_traffic_filter_basic_update_large.tf

@@ -0,0 +1,20 @@
+variable "google_dns_rules" {
+  type = list(string)
+  default = [
+    "8.8.8.8/24", "8.8.4.4/24", "8.8.8.9/24", "8.8.4.10/24", "8.8.8.11/24", "8.8.4.12/24", "8.8.8.13/24", "8.8.4.14/24",
+    "9.8.8.8/24", "10.8.4.4/24", "11.8.8.9/24", "12.8.4.10/24", "13.8.8.11/24", "14.8.4.12/24", "15.8.8.13/24", "16.8.4.14/24",
+  ]
+}
+
+resource "ec_deployment_traffic_filter" "basic" {
+  name   = "%s"
+  region = "%s"
+  type   = "ip"
+
+  dynamic "rule" {
+    for_each = var.google_dns_rules
+    content {
+      source = rule.value
+    }
+  }
+}

ec/ecresource/trafficfilterresource/expanders.go

@@ -24,17 +24,17 @@ import (
 )
 
 func expandModel(d *schema.ResourceData) *models.TrafficFilterRulesetRequest {
-	var rulesIface = d.Get("rule").([]interface{})
+	var ruleSet = d.Get("rule").(*schema.Set)
 	var request = models.TrafficFilterRulesetRequest{
 		Name:             ec.String(d.Get("name").(string)),
 		Type:             ec.String(d.Get("type").(string)),
 		Region:           ec.String(d.Get("region").(string)),
 		Description:      d.Get("description").(string),
 		IncludeByDefault: ec.Bool(d.Get("include_by_default").(bool)),
-		Rules:            make([]*models.TrafficFilterRule, 0, len(rulesIface)),
+		Rules:            make([]*models.TrafficFilterRule, 0, ruleSet.Len()),
 	}
 
-	for _, r := range rulesIface {
+	for _, r := range ruleSet.List() {
 		var m = r.(map[string]interface{})
 		var rule = models.TrafficFilterRule{
 			Source: ec.String(m["source"].(string)),

ec/ecresource/trafficfilterresource/expanders_test.go

@@ -34,6 +34,30 @@ func Test_expandModel(t *testing.T) {
 		State:  newSampleTrafficFilter(),
 		Schema: newSchema(),
 	})
+	trafficFilterMultipleRD := util.NewResourceData(t, util.ResDataParams{
+		ID: "some-random-id",
+		State: map[string]interface{}{
+			"name":               "my traffic filter",
+			"type":               "ip",
+			"include_by_default": false,
+			"region":             "us-east-1",
+			"rule": []interface{}{
+				map[string]interface{}{
+					"source": "1.1.1.1/24",
+				},
+				map[string]interface{}{
+					"source": "1.1.1.0/16",
+				},
+				map[string]interface{}{
+					"source": "0.0.0.0/0",
+				},
+				map[string]interface{}{
+					"source": "1.1.1.1",
+				},
+			},
+		},
+		Schema: newSchema(),
+	})
 	type args struct {
 		d *schema.ResourceData
 	}
@@ -47,13 +71,28 @@ func Test_expandModel(t *testing.T) {
 			args: args{d: trafficFilterRD},
 			want: &models.TrafficFilterRulesetRequest{
 				Name:             ec.String("my traffic filter"),
-				Description:      "",
 				Type:             ec.String("ip"),
 				IncludeByDefault: ec.Bool(false),
 				Region:           ec.String("us-east-1"),
 				Rules: []*models.TrafficFilterRule{
+					{Source: ec.String("0.0.0.0/0")},
 					{Source: ec.String("1.1.1.1")},
+				},
+			},
+		},
+		{
+			name: "parses the resource with a lot of traffic rules",
+			args: args{d: trafficFilterMultipleRD},
+			want: &models.TrafficFilterRulesetRequest{
+				Name:             ec.String("my traffic filter"),
+				Type:             ec.String("ip"),
+				IncludeByDefault: ec.Bool(false),
+				Region:           ec.String("us-east-1"),
+				Rules: []*models.TrafficFilterRule{
 					{Source: ec.String("0.0.0.0/0")},
+					{Source: ec.String("1.1.1.1")},
+					{Source: ec.String("1.1.1.0/16")},
+					{Source: ec.String("1.1.1.1/24")},
 				},
 			},
 		},

ec/ecresource/trafficfilterresource/flatteners.go

@@ -52,8 +52,8 @@ func modelToState(d *schema.ResourceData, res *models.TrafficFilterRulesetInfo)
 	return nil
 }
 
-func flattenRules(rules []*models.TrafficFilterRule) []interface{} {
-	var result = make([]interface{}, 0, len(rules))
+func flattenRules(rules []*models.TrafficFilterRule) *schema.Set {
+	result := schema.NewSet(trafficFilterRuleHash, []interface{}{})
 	for _, rule := range rules {
 		var m = make(map[string]interface{})
 		m["source"] = *rule.Source
@@ -66,11 +66,7 @@ func flattenRules(rules []*models.TrafficFilterRule) []interface{} {
 			m["id"] = rule.ID
 		}
 
-		result = append(result, m)
-	}
-
-	if len(result) > 0 {
-		return result
+		result.Add(m)
 	}
 
 	return result

ec/ecresource/trafficfilterresource/flatteners_test.go

@@ -44,11 +44,92 @@ func Test_modelToState(t *testing.T) {
 		},
 	}
 
+	trafficFilterSchemaArgMultipleR := schema.TestResourceDataRaw(t, newSchema(), nil)
+	trafficFilterSchemaArgMultipleR.SetId("some-random-id")
+
+	remoteStateMultipleRules := models.TrafficFilterRulesetInfo{
+		ID:               ec.String("some-random-id"),
+		Name:             ec.String("my traffic filter"),
+		Type:             ec.String("ip"),
+		IncludeByDefault: ec.Bool(false),
+		Region:           ec.String("us-east-1"),
+		Rules: []*models.TrafficFilterRule{
+			{Source: ec.String("1.1.1.0/16")},
+			{Source: ec.String("1.1.1.1/24")},
+			{Source: ec.String("0.0.0.0/0")},
+			{Source: ec.String("1.1.1.1")},
+		},
+	}
+
+	trafficFilterSchemaArgMultipleRWithDesc := schema.TestResourceDataRaw(t, newSchema(), nil)
+	trafficFilterSchemaArgMultipleRWithDesc.SetId("some-random-id")
+
+	remoteStateMultipleRulesWithDesc := models.TrafficFilterRulesetInfo{
+		ID:               ec.String("some-random-id"),
+		Name:             ec.String("my traffic filter"),
+		Type:             ec.String("ip"),
+		IncludeByDefault: ec.Bool(false),
+		Region:           ec.String("us-east-1"),
+		Rules: []*models.TrafficFilterRule{
+			{Source: ec.String("1.1.1.0/16"), Description: "some network"},
+			{Source: ec.String("1.1.1.1/24"), Description: "a specific IP"},
+			{Source: ec.String("0.0.0.0/0"), Description: "all internet traffic"},
+		},
+	}
+
 	wantTrafficFilter := util.NewResourceData(t, util.ResDataParams{
 		ID:     "some-random-id",
 		State:  newSampleTrafficFilter(),
 		Schema: newSchema(),
 	})
+	wantTrafficFilterMultipleR := util.NewResourceData(t, util.ResDataParams{
+		ID: "some-random-id",
+		State: map[string]interface{}{
+			"name":               "my traffic filter",
+			"type":               "ip",
+			"include_by_default": false,
+			"region":             "us-east-1",
+			"rule": []interface{}{
+				map[string]interface{}{
+					"source": "1.1.1.1/24",
+				},
+				map[string]interface{}{
+					"source": "1.1.1.0/16",
+				},
+				map[string]interface{}{
+					"source": "0.0.0.0/0",
+				},
+				map[string]interface{}{
+					"source": "1.1.1.1",
+				},
+			},
+		},
+		Schema: newSchema(),
+	})
+	wantTrafficFilterMultipleRWithDesc := util.NewResourceData(t, util.ResDataParams{
+		ID: "some-random-id",
+		State: map[string]interface{}{
+			"name":               "my traffic filter",
+			"type":               "ip",
+			"include_by_default": false,
+			"region":             "us-east-1",
+			"rule": []interface{}{
+				map[string]interface{}{
+					"source":      "1.1.1.1/24",
+					"description": "a specific IP",
+				},
+				map[string]interface{}{
+					"source":      "1.1.1.0/16",
+					"description": "some network",
+				},
+				map[string]interface{}{
+					"source":      "0.0.0.0/0",
+					"description": "all internet traffic",
+				},
+			},
+		},
+		Schema: newSchema(),
+	})
 	type args struct {
 		d   *schema.ResourceData
 		res *models.TrafficFilterRulesetInfo
@@ -64,6 +145,16 @@ func Test_modelToState(t *testing.T) {
 			args: args{d: trafficFilterSchemaArg, res: &remoteState},
 			want: wantTrafficFilter,
 		},
+		{
+			name: "flattens the resource with multiple rules",
+			args: args{d: trafficFilterSchemaArgMultipleR, res: &remoteStateMultipleRules},
+			want: wantTrafficFilterMultipleR,
+		},
+		{
+			name: "flattens the resource with multiple rules with descriptions",
+			args: args{d: trafficFilterSchemaArgMultipleRWithDesc, res: &remoteStateMultipleRulesWithDesc},
+			want: wantTrafficFilterMultipleRWithDesc,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

ec/ecresource/trafficfilterresource/schema.go

@@ -18,6 +18,8 @@
 package trafficfilterresource
 
 import (
+	"bytes"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -40,7 +42,8 @@ func newSchema() map[string]*schema.Schema {
 			Required:    true,
 		},
 		"rule": {
-			Type:        schema.TypeList,
+			Type:        schema.TypeSet,
+			Set:         trafficFilterRuleHash,
 			Description: "Required list of rules, which the ruleset is made of.",
 			Required:    true,
 			MinItems:    1,
@@ -80,3 +83,13 @@ func newSchema() map[string]*schema.Schema {
 		},
 	}
 }
+
+func trafficFilterRuleHash(v interface{}) int {
+	var buf bytes.Buffer
+	m := v.(map[string]interface{})
+	buf.WriteString(m["source"].(string))
+	if m["description"] != nil {
+		buf.WriteString(m["description"].(string))
+	}
+	return schema.HashString(buf.String())
+}

ec/ecresource/trafficfilterresource/schema_test.go

@@ -0,0 +1,69 @@
+// Licensed to Elasticsearch B.V. under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Elasticsearch B.V. licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package trafficfilterresource
+
+import "testing"
+
+func Test_trafficFilterRuleHash(t *testing.T) {
+	type args struct {
+		v interface{}
+	}
+	tests := []struct {
+		name string
+		args args
+		want int
+	}{
+		{
+			name: "hash a rule without description",
+			args: args{v: map[string]interface{}{
+				"source": "8.8.8.8/24",
+			}},
+			want: 1202035824,
+		},
+		{
+			name: "hash a rule with description",
+			args: args{v: map[string]interface{}{
+				"source":      "8.8.8.8/24",
+				"description": "google dns",
+			}},
+			want: 1579348650,
+		},
+		{
+			name: "hash a rule different without description",
+			args: args{v: map[string]interface{}{
+				"source": "8.8.4.4/24",
+			}},
+			want: 2058478515,
+		},
+		{
+			name: "hash a rule different with description",
+			args: args{v: map[string]interface{}{
+				"source":      "8.8.4.4/24",
+				"description": "alternate google dns",
+			}},
+			want: 766352945,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := trafficFilterRuleHash(tt.args.v); got != tt.want {
+				t.Errorf("trafficFilterRuleHash() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}