Filter out from properties of tags object. (#1107)

* Filter out from properties of tags object. * Limit the filtering for $type to rule resource only and add validation logic to avoid $type from end user as tag name. * Update acceptance tests to validate the $type in attributes. * Add more non-exist check for metric alert rule. * Update the validation logic for test cases. * Move filtering logic into seprated utility function for future reuse. * Update the comment for filterTags function. * Remove the validation of $type in common function. * add test case for filterTag function. * Refactoring tags to match the others ``` $ acctests azurerm TestValidateMetricAlertRuleTags === RUN TestValidateMetricAlertRuleTags --- PASS: TestValidateMetricAlertRuleTags (0.00s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 0.025s ```
hashicorp · Apr 20, 2018 · ebba5af · ebba5af
1 parent 503fc49
commit ebba5af
Show file tree

Hide file tree

Showing 5 changed files with 141 additions and 10 deletions.
diff --git a/azurerm/import_arm_metric_alertrule_test.go b/azurerm/import_arm_metric_alertrule_test.go
@@ -20,11 +20,19 @@ func TestAccAzureRMMetricAlertRule_importVirtualMachineCpu(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: config,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMMetricAlertRuleExists(resourceName),
+					resource.TestCheckNoResourceAttr(resourceName, "tags.$type"),
+				),
 			},
 			{
 				ResourceName:      resourceName,
 				ImportState:       true,
 				ImportStateVerify: true,
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMMetricAlertRuleExists(resourceName),
+					resource.TestCheckNoResourceAttr(resourceName, "tags.$type"),
+				),
 			},
 		},
 	})

diff --git a/azurerm/resource_arm_metric_alertrule.go b/azurerm/resource_arm_metric_alertrule.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"log"
 
+	"strings"
+
 	"github.com/Azure/azure-sdk-for-go/services/monitor/mgmt/2017-05-01-preview/insights"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
@@ -138,7 +140,12 @@ func resourceArmMetricAlertRule() *schema.Resource {
 				},
 			},
 
-			"tags": tagsSchema(),
+			"tags": {
+				Type:         schema.TypeMap,
+				Optional:     true,
+				Computed:     true,
+				ValidateFunc: validateMetricAlertRuleTags,
+			},
 		},
 	}
 }
@@ -276,7 +283,10 @@ func resourceArmMetricAlertRuleRead(d *schema.ResourceData, meta interface{}) er
 		d.Set("webhook_action", webhook_actions)
 	}
 
-	flattenAndSetTags(d, resp.Tags)
+	// Return a new tag map filtered by the specified tag names.
+	tagMap := filterTags(resp.Tags, "$type")
+
+	flattenAndSetTags(d, tagMap)
 
 	return nil
 }
@@ -411,3 +421,18 @@ func resourceGroupAndAlertRuleNameFromId(alertRuleId string) (string, string, er
 
 	return resourceGroup, name, nil
 }
+
+func validateMetricAlertRuleTags(v interface{}, f string) (ws []string, es []error) {
+	// Normal validation required by any AzureRM resource.
+	ws, es = validateAzureRMTags(v, f)
+
+	tagsMap := v.(map[string]interface{})
+
+	for k := range tagsMap {
+		if strings.EqualFold(k, "$type") {
+			es = append(es, fmt.Errorf("the %q is not allowed as tag name", k))
+		}
+	}
+
+	return ws, es
+}
diff --git a/azurerm/resource_arm_metric_alertrule_test.go b/azurerm/resource_arm_metric_alertrule_test.go
@@ -11,6 +11,60 @@ import (
 	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
 )
 
+func TestValidateMetricAlertRuleTags(t *testing.T) {
+	cases := []struct {
+		Name     string
+		Value    map[string]interface{}
+		ErrCount int
+	}{
+		{
+			Name: "Single Valid",
+			Value: map[string]interface{}{
+				"hello": "world",
+			},
+			ErrCount: 0,
+		},
+		{
+			Name: "Single Invalid",
+			Value: map[string]interface{}{
+				"$Type": "hello/world",
+			},
+			ErrCount: 1,
+		},
+		{
+			Name: "Single Invalid lowercase",
+			Value: map[string]interface{}{
+				"$type": "hello/world",
+			},
+			ErrCount: 1,
+		},
+		{
+			Name: "Multiple Valid",
+			Value: map[string]interface{}{
+				"hello": "world",
+				"foo":   "bar",
+			},
+			ErrCount: 0,
+		},
+		{
+			Name: "Multiple Invalid",
+			Value: map[string]interface{}{
+				"hello": "world",
+				"$type": "Microsoft.Foo/Bar",
+			},
+			ErrCount: 1,
+		},
+	}
+
+	for _, tc := range cases {
+		_, errors := validateMetricAlertRuleTags(tc.Value, "azurerm_metric_alert_rule")
+
+		if len(errors) != tc.ErrCount {
+			t.Fatalf("Expected %q to return %d errors but returned %d", tc.Name, tc.ErrCount, len(errors))
+		}
+	}
+}
+
 func TestAccAzureRMMetricAlertRule_virtualMachineCpu(t *testing.T) {
 	resourceName := "azurerm_metric_alertrule.test"
 	ri := acctest.RandInt()
@@ -27,13 +81,15 @@ func TestAccAzureRMMetricAlertRule_virtualMachineCpu(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMMetricAlertRuleExists(resourceName),
 					resource.TestCheckResourceAttr(resourceName, "enabled", "true"),
+					resource.TestCheckNoResourceAttr(resourceName, "tags.$type"),
 				),
 			},
 			{
 				Config: postConfig,
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMMetricAlertRuleExists(resourceName),
 					resource.TestCheckResourceAttr(resourceName, "enabled", "false"),
+					resource.TestCheckNoResourceAttr(resourceName, "tags.$type"),
 				),
 			},
 		},
@@ -54,6 +110,7 @@ func TestAccAzureRMMetricAlertRule_sqlDatabaseStorage(t *testing.T) {
 				Config: config,
 				Check: resource.ComposeTestCheckFunc(
 					testCheckAzureRMMetricAlertRuleExists(resourceName),
+					resource.TestCheckNoResourceAttr(resourceName, "tags.$type"),
 				),
 			},
 		},

diff --git a/azurerm/tags.go b/azurerm/tags.go
@@ -3,6 +3,7 @@ package azurerm
 import (
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
 )
@@ -44,7 +45,7 @@ func tagValueToString(v interface{}) (string, error) {
 	}
 }
 
-func validateAzureRMTags(v interface{}, k string) (ws []string, es []error) {
+func validateAzureRMTags(v interface{}, f string) (ws []string, es []error) {
 	tagsMap := v.(map[string]interface{})
 
 	if len(tagsMap) > 15 {
@@ -64,7 +65,7 @@ func validateAzureRMTags(v interface{}, k string) (ws []string, es []error) {
 		}
 	}
 
-	return
+	return ws, es
 }
 
 func expandTags(tagsMap map[string]interface{}) map[string]*string {
@@ -79,15 +80,36 @@ func expandTags(tagsMap map[string]interface{}) map[string]*string {
 	return output
 }
 
-func flattenAndSetTags(d *schema.ResourceData, tagsMap map[string]*string) {
-	if tagsMap == nil {
-		d.Set("tags", make(map[string]interface{}))
-		return
+func filterTags(tagsMap map[string]*string, tagNames ...string) map[string]*string {
+	if len(tagNames) == 0 {
+		return tagsMap
 	}
 
-	output := make(map[string]interface{}, len(tagsMap))
+	// Build the filter dictionary from passed tag names.
+	filterDict := make(map[string]bool)
+	for _, name := range tagNames {
+		if len(name) > 0 {
+			filterDict[strings.ToLower(name)] = true
+		}
+	}
 
-	for i, v := range tagsMap {
+	// Filter out tag if it exists(case insensitive) in the dictionary.
+	tagsRet := make(map[string]*string)
+	for k, v := range tagsMap {
+		if !filterDict[strings.ToLower(k)] {
+			tagsRet[k] = v
+		}
+	}
+
+	return tagsRet
+}
+
+func flattenAndSetTags(d *schema.ResourceData, tagMap map[string]*string) {
+
+	// If tagsMap is nil, len(tagsMap) will be 0.
+	output := make(map[string]interface{}, len(tagMap))
+
+	for i, v := range tagMap {
 		output[i] = *v
 	}
 

diff --git a/azurerm/tags_test.go b/azurerm/tags_test.go
@@ -94,3 +94,22 @@ func TestExpandARMTags(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterARMTags(t *testing.T) {
+	testData := make(map[string]*string)
+	valueData := [3]string{"value1", "value2", "value3"}
+
+	testData["key1"] = &valueData[0]
+	testData["key2"] = &valueData[1]
+	testData["key3"] = &valueData[2]
+
+	filtered := filterTags(testData, "key1", "key3", "")
+
+	if len(filtered) != 1 {
+		t.Fatalf("Expected 1 result in filtered tag map, got %d", len(filtered))
+	}
+
+	if filtered["key2"] != &valueData[1] {
+		t.Fatalf("Expected %v in filtered tag map, got %v", valueData[1], *filtered["key2"])
+	}
+}