Merge pull request #13129 from tombuildsstuff/validation

provider/azurerm: Moving Reused JSON Validation -> Core

builtin/providers/azurerm/resource_arm_virtual_machine_extension.go

@@ -1,13 +1,13 @@
 package azurerm
 
 import (
-	"encoding/json"
 	"fmt"
 	"net/http"
-	"reflect"
 
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/structure"
+	"github.com/hashicorp/terraform/helper/validation"
 )
 
 func resourceArmVirtualMachineExtensions() *schema.Resource {
@@ -21,7 +21,7 @@ func resourceArmVirtualMachineExtensions() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
@@ -29,29 +29,29 @@ func resourceArmVirtualMachineExtensions() *schema.Resource {
 
 			"location": locationSchema(),
 
-			"resource_group_name": &schema.Schema{
+			"resource_group_name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
 
-			"virtual_machine_name": &schema.Schema{
+			"virtual_machine_name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
 
-			"publisher": &schema.Schema{
+			"publisher": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"type": &schema.Schema{
+			"type": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"type_handler_version": &schema.Schema{
+			"type_handler_version": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
@@ -61,20 +61,20 @@ func resourceArmVirtualMachineExtensions() *schema.Resource {
 				Optional: true,
 			},
 
-			"settings": &schema.Schema{
+			"settings": {
 				Type:             schema.TypeString,
 				Optional:         true,
-				ValidateFunc:     validateJsonString,
-				DiffSuppressFunc: suppressDiffVirtualMachineExtensionSettings,
+				ValidateFunc:     validation.ValidateJsonString,
+				DiffSuppressFunc: structure.SuppressJsonDiff,
 			},
 
 			// due to the sensitive nature, these are not returned by the API
-			"protected_settings": &schema.Schema{
+			"protected_settings": {
 				Type:             schema.TypeString,
 				Optional:         true,
 				Sensitive:        true,
-				ValidateFunc:     validateJsonString,
-				DiffSuppressFunc: suppressDiffVirtualMachineExtensionSettings,
+				ValidateFunc:     validation.ValidateJsonString,
+				DiffSuppressFunc: structure.SuppressJsonDiff,
 			},
 
 			"tags": tagsSchema(),
@@ -107,7 +107,7 @@ func resourceArmVirtualMachineExtensionsCreate(d *schema.ResourceData, meta inte
 	}
 
 	if settingsString := d.Get("settings").(string); settingsString != "" {
-		settings, err := expandArmVirtualMachineExtensionSettings(settingsString)
+		settings, err := structure.ExpandJsonFromString(settingsString)
 		if err != nil {
 			return fmt.Errorf("unable to parse settings: %s", err)
 		}
@@ -115,7 +115,7 @@ func resourceArmVirtualMachineExtensionsCreate(d *schema.ResourceData, meta inte
 	}
 
 	if protectedSettingsString := d.Get("protected_settings").(string); protectedSettingsString != "" {
-		protectedSettings, err := expandArmVirtualMachineExtensionSettings(protectedSettingsString)
+		protectedSettings, err := structure.ExpandJsonFromString(protectedSettingsString)
 		if err != nil {
 			return fmt.Errorf("unable to parse protected_settings: %s", err)
 		}
@@ -172,7 +172,7 @@ func resourceArmVirtualMachineExtensionsRead(d *schema.ResourceData, meta interf
 	d.Set("auto_upgrade_minor_version", resp.VirtualMachineExtensionProperties.AutoUpgradeMinorVersion)
 
 	if resp.VirtualMachineExtensionProperties.Settings != nil {
-		settings, err := flattenArmVirtualMachineExtensionSettings(*resp.VirtualMachineExtensionProperties.Settings)
+		settings, err := structure.FlattenJsonToString(*resp.VirtualMachineExtensionProperties.Settings)
 		if err != nil {
 			return fmt.Errorf("unable to parse settings from response: %s", err)
 		}
@@ -199,34 +199,3 @@ func resourceArmVirtualMachineExtensionsDelete(d *schema.ResourceData, meta inte
 
 	return nil
 }
-
-func expandArmVirtualMachineExtensionSettings(jsonString string) (map[string]interface{}, error) {
-	var result map[string]interface{}
-
-	err := json.Unmarshal([]byte(jsonString), &result)
-
-	return result, err
-}
-
-func flattenArmVirtualMachineExtensionSettings(settingsMap map[string]interface{}) (string, error) {
-	result, err := json.Marshal(settingsMap)
-	if err != nil {
-		return "", err
-	}
-
-	return string(result), nil
-}
-
-func suppressDiffVirtualMachineExtensionSettings(k, old, new string, d *schema.ResourceData) bool {
-	oldMap, err := expandArmVirtualMachineExtensionSettings(old)
-	if err != nil {
-		return false
-	}
-
-	newMap, err := expandArmVirtualMachineExtensionSettings(new)
-	if err != nil {
-		return false
-	}
-
-	return reflect.DeepEqual(oldMap, newMap)
-}

builtin/providers/azurerm/resource_arm_virtual_machine_scale_set.go

@@ -9,6 +9,8 @@ import (
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
 	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/structure"
+	"github.com/hashicorp/terraform/helper/validation"
 )
 
 func resourceArmVirtualMachineScaleSet() *schema.Resource {
@@ -374,16 +376,16 @@ func resourceArmVirtualMachineScaleSet() *schema.Resource {
 						"settings": {
 							Type:             schema.TypeString,
 							Optional:         true,
-							ValidateFunc:     validateJsonString,
-							DiffSuppressFunc: suppressDiffVirtualMachineExtensionSettings,
+							ValidateFunc:     validation.ValidateJsonString,
+							DiffSuppressFunc: structure.SuppressJsonDiff,
 						},
 
 						"protected_settings": {
 							Type:             schema.TypeString,
 							Optional:         true,
 							Sensitive:        true,
-							ValidateFunc:     validateJsonString,
-							DiffSuppressFunc: suppressDiffVirtualMachineExtensionSettings,
+							ValidateFunc:     validation.ValidateJsonString,
+							DiffSuppressFunc: structure.SuppressJsonDiff,
 						},
 					},
 				},
@@ -784,7 +786,7 @@ func flattenAzureRmVirtualMachineScaleSetExtensionProfile(profile *compute.Virtu
 			}
 
 			if properties.Settings != nil {
-				settings, err := flattenArmVirtualMachineExtensionSettings(*properties.Settings)
+				settings, err := structure.FlattenJsonToString(*properties.Settings)
 				if err != nil {
 					return nil, err
 				}
@@ -1227,7 +1229,7 @@ func expandAzureRMVirtualMachineScaleSetExtensions(d *schema.ResourceData) (*com
 		}
 
 		if s := config["settings"].(string); s != "" {
-			settings, err := expandArmVirtualMachineExtensionSettings(s)
+			settings, err := structure.ExpandJsonFromString(s)
 			if err != nil {
 				return nil, fmt.Errorf("unable to parse settings: %s", err)
 			}
@@ -1235,7 +1237,7 @@ func expandAzureRMVirtualMachineScaleSetExtensions(d *schema.ResourceData) (*com
 		}
 
 		if s := config["protected_settings"].(string); s != "" {
-			protectedSettings, err := expandArmVirtualMachineExtensionSettings(s)
+			protectedSettings, err := structure.ExpandJsonFromString(s)
 			if err != nil {
 				return nil, fmt.Errorf("unable to parse protected_settings: %s", err)
 			}

builtin/providers/azurerm/validators.go

@@ -6,13 +6,6 @@ import (
 	"github.com/satori/uuid"
 )
 
-func validateJsonString(v interface{}, k string) (ws []string, errors []error) {
-	if _, err := normalizeJsonString(v); err != nil {
-		errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
-	}
-	return
-}
-
 func validateUUID(v interface{}, k string) (ws []string, errors []error) {
 	if _, err := uuid.FromString(v.(string)); err != nil {
 		errors = append(errors, fmt.Errorf("%q is an invalid UUUID: %s", k, err))

builtin/providers/azurerm/validators_test.go

@@ -1,58 +0,0 @@
-package azurerm
-
-import "testing"
-
-func TestValidateJsonString(t *testing.T) {
-	type testCases struct {
-		Value    string
-		ErrCount int
-	}
-
-	invalidCases := []testCases{
-		{
-			Value:    `{0:"1"}`,
-			ErrCount: 1,
-		},
-		{
-			Value:    `{'abc':1}`,
-			ErrCount: 1,
-		},
-		{
-			Value:    `{"def":}`,
-			ErrCount: 1,
-		},
-		{
-			Value:    `{"xyz":[}}`,
-			ErrCount: 1,
-		},
-	}
-
-	for _, tc := range invalidCases {
-		_, errors := validateJsonString(tc.Value, "json")
-		if len(errors) != tc.ErrCount {
-			t.Fatalf("Expected %q to trigger a validation error.", tc.Value)
-		}
-	}
-
-	validCases := []testCases{
-		{
-			Value:    ``,
-			ErrCount: 0,
-		},
-		{
-			Value:    `{}`,
-			ErrCount: 0,
-		},
-		{
-			Value:    `{"abc":["1","2"]}`,
-			ErrCount: 0,
-		},
-	}
-
-	for _, tc := range validCases {
-		_, errors := validateJsonString(tc.Value, "json")
-		if len(errors) != tc.ErrCount {
-			t.Fatalf("Expected %q not to trigger a validation error.", tc.Value)
-		}
-	}
-}

helper/structure/expand_json.go

@@ -0,0 +1,11 @@
+package structure
+
+import "encoding/json"
+
+func ExpandJsonFromString(jsonString string) (map[string]interface{}, error) {
+	var result map[string]interface{}
+
+	err := json.Unmarshal([]byte(jsonString), &result)
+
+	return result, err
+}

helper/structure/expand_json_test.go

@@ -0,0 +1,48 @@
+package structure
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestExpandJson_emptyString(t *testing.T) {
+	_, err := ExpandJsonFromString("")
+	if err == nil {
+		t.Fatal("Expected to throw an error while Expanding JSON")
+	}
+}
+
+func TestExpandJson_singleItem(t *testing.T) {
+	input := `{
+	  "foo": "bar"
+	}`
+	expected := make(map[string]interface{}, 1)
+	expected["foo"] = "bar"
+	actual, err := ExpandJsonFromString(input)
+	if err != nil {
+		t.Fatalf("Expected not to throw an error while Expanding JSON, but got: %s", err)
+	}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("Got:\n\n%+v\n\nExpected:\n\n%+v\n", actual, expected)
+	}
+}
+
+func TestExpandJson_multipleItems(t *testing.T) {
+	input := `{
+	  "foo": "bar",
+	  "hello": "world"
+	}`
+	expected := make(map[string]interface{}, 1)
+	expected["foo"] = "bar"
+	expected["hello"] = "world"
+
+	actual, err := ExpandJsonFromString(input)
+	if err != nil {
+		t.Fatalf("Expected not to throw an error while Expanding JSON, but got: %s", err)
+	}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("Got:\n\n%+v\n\nExpected:\n\n%+v\n", actual, expected)
+	}
+}

helper/structure/flatten_json.go

@@ -0,0 +1,16 @@
+package structure
+
+import "encoding/json"
+
+func FlattenJsonToString(input map[string]interface{}) (string, error) {
+	if len(input) == 0 {
+		return "", nil
+	}
+
+	result, err := json.Marshal(input)
+	if err != nil {
+		return "", err
+	}
+
+	return string(result), nil
+}

helper/structure/flatten_json_test.go

@@ -0,0 +1,47 @@
+package structure
+
+import (
+	"testing"
+)
+
+func TestFlattenJson_empty(t *testing.T) {
+	input := make(map[string]interface{}, 0)
+	expected := ""
+	actual, err := FlattenJsonToString(input)
+	if err != nil {
+		t.Fatalf("Expected not to throw an error while Flattening JSON, but got: %s", err)
+	}
+
+	if expected != actual {
+		t.Fatalf("Got: `%+v`. Expected: `%+v`", actual, expected)
+	}
+}
+
+func TestFlattenJson_singleItem(t *testing.T) {
+	input := make(map[string]interface{}, 1)
+	input["foo"] = "bar"
+	expected := `{"foo":"bar"}`
+	actual, err := FlattenJsonToString(input)
+	if err != nil {
+		t.Fatalf("Expected not to throw an error while Flattening JSON, but got: %s", err)
+	}
+
+	if expected != actual {
+		t.Fatalf("Got: `%+v`. Expected: `%+v`", actual, expected)
+	}
+}
+
+func TestFlattenJson_multipleItems(t *testing.T) {
+	input := make(map[string]interface{}, 1)
+	input["foo"] = "bar"
+	input["bar"] = "foo"
+	expected := `{"bar":"foo","foo":"bar"}`
+	actual, err := FlattenJsonToString(input)
+	if err != nil {
+		t.Fatalf("Expected not to throw an error while Flattening JSON, but got: %s", err)
+	}
+
+	if expected != actual {
+		t.Fatalf("Got: `%+v`. Expected: `%+v`", actual, expected)
+	}
+}

helper/structure/normalize_json.go

@@ -1,11 +1,11 @@
-package azurerm
+package structure
 
 import "encoding/json"
 
 // Takes a value containing JSON string and passes it through
 // the JSON parser to normalize it, returns either a parsing
 // error or normalized JSON string.
-func normalizeJsonString(jsonString interface{}) (string, error) {
+func NormalizeJsonString(jsonString interface{}) (string, error) {
 	var j interface{}
 
 	if jsonString == nil || jsonString.(string) == "" {

helper/structure/normalize_json_test.go

@@ -1,11 +1,10 @@
-package azurerm
+package structure
 
-import "testing"
-
-func TestNormalizeJsonString(t *testing.T) {
-	var err error
-	var actual string
+import (
+	"testing"
+)
 
+func TestNormalizeJsonString_valid(t *testing.T) {
 	// Well formatted and valid.
 	validJson := `{
    "abc": {
@@ -22,7 +21,7 @@ func TestNormalizeJsonString(t *testing.T) {
 }`
 	expected := `{"abc":{"def":123,"xyz":[{"a":"ホリネズミ"},{"b":"1\\n2"}]}}`
 
-	actual, err = normalizeJsonString(validJson)
+	actual, err := NormalizeJsonString(validJson)
 	if err != nil {
 		t.Fatalf("Expected not to throw an error while parsing JSON, but got: %s", err)
 	}
@@ -30,7 +29,9 @@ func TestNormalizeJsonString(t *testing.T) {
 	if actual != expected {
 		t.Fatalf("Got:\n\n%s\n\nExpected:\n\n%s\n", actual, expected)
 	}
+}
 
+func TestNormalizeJsonString_invalid(t *testing.T) {
 	// Well formatted but not valid,
 	// missing closing squre bracket.
 	invalidJson := `{
@@ -43,7 +44,8 @@ func TestNormalizeJsonString(t *testing.T) {
       }
    }
 }`
-	actual, err = normalizeJsonString(invalidJson)
+	expected := `{"abc":{"def":123,"xyz":[{"a":"ホリネズミ"},{"b":"1\\n2"}]}}`
+	actual, err := NormalizeJsonString(invalidJson)
 	if err == nil {
 		t.Fatalf("Expected to throw an error while parsing JSON, but got: %s", err)
 	}

helper/structure/suppress_json_diff.go

@@ -0,0 +1,21 @@
+package structure
+
+import (
+	"reflect"
+
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func SuppressJsonDiff(k, old, new string, d *schema.ResourceData) bool {
+	oldMap, err := ExpandJsonFromString(old)
+	if err != nil {
+		return false
+	}
+
+	newMap, err := ExpandJsonFromString(new)
+	if err != nil {
+		return false
+	}
+
+	return reflect.DeepEqual(oldMap, newMap)
+}

helper/structure/suppress_json_diff_test.go

@@ -0,0 +1,51 @@
+package structure
+
+import (
+	"testing"
+)
+
+func TestSuppressJsonDiff_same(t *testing.T) {
+	original := `{ "enabled": true }`
+	new := `{ "enabled": true }`
+	expected := true
+
+	actual := SuppressJsonDiff("test", original, new, nil)
+	if actual != expected {
+		t.Fatal("[ERROR] Identical JSON values shouldn't cause a diff")
+	}
+}
+
+func TestSuppressJsonDiff_sameWithWhitespace(t *testing.T) {
+	original := `{
+	  "enabled": true
+	}`
+	new := `{ "enabled": true }`
+	expected := true
+
+	actual := SuppressJsonDiff("test", original, new, nil)
+	if actual != expected {
+		t.Fatal("[ERROR] Identical JSON values shouldn't cause a diff")
+	}
+}
+
+func TestSuppressJsonDiff_differentValue(t *testing.T) {
+	original := `{ "enabled": true }`
+	new := `{ "enabled": false }`
+	expected := false
+
+	actual := SuppressJsonDiff("test", original, new, nil)
+	if actual != expected {
+		t.Fatal("[ERROR] Different JSON values should cause a diff")
+	}
+}
+
+func TestSuppressJsonDiff_newValue(t *testing.T) {
+	original := `{ "enabled": true }`
+	new := `{ "enabled": false, "world": "round" }`
+	expected := false
+
+	actual := SuppressJsonDiff("test", original, new, nil)
+	if actual != expected {
+		t.Fatal("[ERROR] Different JSON values should cause a diff")
+	}
+}

helper/validation/validation.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/structure"
 )
 
 // IntBetween returns a SchemaValidateFunc which tests if the provided value
@@ -98,3 +99,10 @@ func CIDRNetwork(min, max int) schema.SchemaValidateFunc {
 		return
 	}
 }
+
+func ValidateJsonString(v interface{}, k string) (ws []string, errors []error) {
+	if _, err := structure.NormalizeJsonString(v); err != nil {
+		errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
+	}
+	return
+}

helper/validation/validation_test.go

@@ -65,6 +65,61 @@ func TestValidationStringInSlice(t *testing.T) {
 	})
 }
 
+func TestValidateJsonString(t *testing.T) {
+	type testCases struct {
+		Value    string
+		ErrCount int
+	}
+
+	invalidCases := []testCases{
+		{
+			Value:    `{0:"1"}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{'abc':1}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{"def":}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{"xyz":[}}`,
+			ErrCount: 1,
+		},
+	}
+
+	for _, tc := range invalidCases {
+		_, errors := ValidateJsonString(tc.Value, "json")
+		if len(errors) != tc.ErrCount {
+			t.Fatalf("Expected %q to trigger a validation error.", tc.Value)
+		}
+	}
+
+	validCases := []testCases{
+		{
+			Value:    ``,
+			ErrCount: 0,
+		},
+		{
+			Value:    `{}`,
+			ErrCount: 0,
+		},
+		{
+			Value:    `{"abc":["1","2"]}`,
+			ErrCount: 0,
+		},
+	}
+
+	for _, tc := range validCases {
+		_, errors := ValidateJsonString(tc.Value, "json")
+		if len(errors) != tc.ErrCount {
+			t.Fatalf("Expected %q not to trigger a validation error.", tc.Value)
+		}
+	}
+}
+
 func runTestCases(t *testing.T, cases []testCase) {
 	matchErr := func(errs []error, r *regexp.Regexp) bool {
 		// err must match one provided