Do not use TypeMap for non-primitive types, use TypeList

Currently, TypeMap doesn't support non-primitive type values. ref : hashicorp/terraform-plugin-sdk#62 When using non-primitive values with TypeMap, terraform crashes at runtime with a following error message. ``` panic: Unknown validation type: 6 ``` This error is originated at https://github.com/hashicorp/terraform-plugin-sdk/blob/v2.11.0/helper/schema/schema.go#L2017 Only TypeBool, TypeInt, TypeFloat and TypeString are supported. So, in this change, use TypeList with new custom resource to store non-primitive values.
cloudflare · Mar 21, 2022 · 7e1f0cd · 7e1f0cd
1 parent ed246a3
commit 7e1f0cd
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 5 deletions.
diff --git a/.changelog/1520.txt b/.changelog/1520.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/cloudflare_notification_policy: Fix unexpected crashes when using cloudflare_notification_policy with a filters attribute
+```
diff --git a/cloudflare/resource_cloudflare_notification_policy.go b/cloudflare/resource_cloudflare_notification_policy.go
@@ -155,7 +155,7 @@ func buildNotificationPolicy(d *schema.ResourceData) cloudflare.NotificationPoli
 	}
 
 	if filters, ok := d.GetOk("filters"); ok {
-		notificationPolicy.Filters = filters.(map[string][]string)
+		notificationPolicy.Filters = filters.([]interface{})[0].(map[string][]string)
 	}
 
 	if conditions, ok := d.GetOk("conditions"); ok {

diff --git a/cloudflare/resource_cloudflare_notification_policy_test.go b/cloudflare/resource_cloudflare_notification_policy_test.go
@@ -84,3 +84,103 @@ func testCheckCloudflareNotificationPolicyUpdated(resName, policyName, policyDes
     }
   }`, resName, policyName, policyDesc, accountID)
 }
+
+func TestAccCloudflareNotificationPolicyWithFiltersAttribute(t *testing.T) {
+	// Temporarily unset CLOUDFLARE_API_TOKEN if it is set as the notification
+	// service does not yet support the API tokens and it results in
+	// misleading state error messages.
+	if os.Getenv("CLOUDFLARE_API_TOKEN") != "" {
+		defer func(apiToken string) {
+			os.Setenv("CLOUDFLARE_API_TOKEN", apiToken)
+		}(os.Getenv("CLOUDFLARE_API_TOKEN"))
+		os.Setenv("CLOUDFLARE_API_TOKEN", "")
+	}
+
+	rnd := generateRandomResourceName()
+	resourceName := "cloudflare_notification_policy." + rnd
+	updatedPolicyName := "updated Advanced Security Events Alert from terraform provider"
+	updatedPolicyDesc := "updated description"
+	accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
+	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheckAccount(t)
+		},
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testCheckCloudflareNotificationPolicyWithFiltersAttribute(rnd, accountID, zoneID),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", "test Advanced Security Events Alert from terraform provider"),
+					resource.TestCheckResourceAttr(resourceName, "description", "test description"),
+					resource.TestCheckResourceAttr(resourceName, "enabled", "true"),
+					resource.TestCheckResourceAttr(resourceName, "alert_type", "clickhouse_alert_fw_ent_anomaly"),
+					resource.TestCheckResourceAttr(resourceName, "account_id", accountID),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.services.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.services.0", "waf"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.zones.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.zones.0", zoneID),
+				),
+			},
+			{
+				Config: testCheckCloudflareNotificationPolicyWithFiltersAttributeUpdated(rnd, updatedPolicyName, updatedPolicyDesc, accountID, zoneID),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", updatedPolicyName),
+					resource.TestCheckResourceAttr(resourceName, "description", updatedPolicyDesc),
+					resource.TestCheckResourceAttr(resourceName, "enabled", "true"),
+					resource.TestCheckResourceAttr(resourceName, "alert_type", "clickhouse_alert_fw_ent_anomaly"),
+					resource.TestCheckResourceAttr(resourceName, "account_id", accountID),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.services.#", "2"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.services.0", "waf"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.services.1", "firewallrules"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.zones.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "filters.0.zones.0", zoneID),
+				),
+			},
+		},
+	})
+}
+
+func testCheckCloudflareNotificationPolicyWithFiltersAttribute(name, accountID, zoneID string) string {
+	return fmt.Sprintf(`
+  resource "cloudflare_notification_policy" "%[1]s" {
+    name        = "test Advanced Security Events Alert from terraform provider"
+    account_id  = "%[2]s"
+    description = "test description"
+    enabled     =  true
+    alert_type  = "clickhouse_alert_fw_ent_anomaly"
+    email_integration {
+      name =  ""
+      id   =  "[email protected]"
+    }
+    filters {
+      services = [
+        "waf",
+      ]
+      zones = ["%[3]s"]
+    }
+  }`, name, accountID, zoneID)
+}
+
+func testCheckCloudflareNotificationPolicyWithFiltersAttributeUpdated(resName, policyName, policyDesc, accountID, zoneID string) string {
+	return fmt.Sprintf(`
+  resource "cloudflare_notification_policy" "%[1]s" {
+    name        = "%[2]s"
+    account_id  = "%[4]s"
+    description = "%[3]s"
+    enabled     =  true
+    alert_type  = "clickhouse_alert_fw_ent_anomaly"
+    email_integration {
+      name =  ""
+      id   =  "[email protected]"
+    }
+    filters {
+      services = [
+        "waf",
+        "firewallrules",
+      ]
+      zones = ["%[5]s"]
+    }
+  }`, resName, policyName, policyDesc, accountID, zoneID)
+}
diff --git a/cloudflare/schema_cloudflare_notification_policy.go b/cloudflare/schema_cloudflare_notification_policy.go
@@ -25,11 +25,26 @@ func resourceCloudflareNotificationPolicySchema() map[string]*schema.Schema {
 			Required: true,
 		},
 		"filters": {
-			Type:     schema.TypeMap,
+			Type:     schema.TypeList,
 			Optional: true,
-			Elem: &schema.Schema{
-				Type: schema.TypeList,
-				Elem: schema.TypeString,
+			MaxItems: 1,
+			Elem: &schema.Resource{
+				Schema: map[string]*schema.Schema{
+					"zones": {
+						Type: schema.TypeList,
+						Elem: &schema.Schema{
+							Type: schema.TypeString,
+						},
+						Optional: true,
+					},
+					"services": {
+						Type: schema.TypeList,
+						Elem: &schema.Schema{
+							Type: schema.TypeString,
+						},
+						Optional: true,
+					},
+				},
 			},
 		},
 		"created": {