From 277d1b6b2d3bcf4fb6e41821a35aa8ba8804f408 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Mon, 27 Mar 2017 15:35:40 +0100 Subject: [PATCH] Refactoring the schema diff/validation -> core --- .../resource_arm_virtual_machine_extension.go | 65 +++++-------------- .../resource_arm_virtual_machine_scale_set.go | 16 +++-- builtin/providers/azurerm/structure.go | 24 ------- builtin/providers/azurerm/validators.go | 7 -- builtin/providers/azurerm/validators_test.go | 58 ----------------- helper/structure/structure.go | 60 +++++++++++++++++ .../structure}/structure_test.go | 10 +-- helper/validation/validation.go | 8 +++ helper/validation/validation_test.go | 55 ++++++++++++++++ 9 files changed, 155 insertions(+), 148 deletions(-) delete mode 100644 builtin/providers/azurerm/structure.go delete mode 100644 builtin/providers/azurerm/validators_test.go create mode 100644 helper/structure/structure.go rename {builtin/providers/azurerm => helper/structure}/structure_test.go (88%) diff --git a/builtin/providers/azurerm/resource_arm_virtual_machine_extension.go b/builtin/providers/azurerm/resource_arm_virtual_machine_extension.go index 129e205c5..be772ce55 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_machine_extension.go +++ b/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) -} diff --git a/builtin/providers/azurerm/resource_arm_virtual_machine_scale_set.go b/builtin/providers/azurerm/resource_arm_virtual_machine_scale_set.go index afadf6552..ae240938a 100644 --- a/builtin/providers/azurerm/resource_arm_virtual_machine_scale_set.go +++ b/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) } diff --git a/builtin/providers/azurerm/structure.go b/builtin/providers/azurerm/structure.go deleted file mode 100644 index 47acd631e..000000000 --- a/builtin/providers/azurerm/structure.go +++ /dev/null @@ -1,24 +0,0 @@ -package azurerm - -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) { - var j interface{} - - if jsonString == nil || jsonString.(string) == "" { - return "", nil - } - - s := jsonString.(string) - - err := json.Unmarshal([]byte(s), &j) - if err != nil { - return s, err - } - - bytes, _ := json.Marshal(j) - return string(bytes[:]), nil -} diff --git a/builtin/providers/azurerm/validators.go b/builtin/providers/azurerm/validators.go index 2b8ef82f2..2151f09e0 100644 --- a/builtin/providers/azurerm/validators.go +++ b/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)) diff --git a/builtin/providers/azurerm/validators_test.go b/builtin/providers/azurerm/validators_test.go deleted file mode 100644 index 66984be06..000000000 --- a/builtin/providers/azurerm/validators_test.go +++ /dev/null @@ -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) - } - } -} diff --git a/helper/structure/structure.go b/helper/structure/structure.go new file mode 100644 index 000000000..3ebfb9ace --- /dev/null +++ b/helper/structure/structure.go @@ -0,0 +1,60 @@ +package structure + +import ( + "encoding/json" + "reflect" + + "github.com/hashicorp/terraform/helper/schema" +) + +// 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) { + var j interface{} + + if jsonString == nil || jsonString.(string) == "" { + return "", nil + } + + s := jsonString.(string) + + err := json.Unmarshal([]byte(s), &j) + if err != nil { + return s, err + } + + bytes, _ := json.Marshal(j) + return string(bytes[:]), nil +} + +func ExpandJsonFromString(jsonString string) (map[string]interface{}, error) { + var result map[string]interface{} + + err := json.Unmarshal([]byte(jsonString), &result) + + return result, err +} + +func FlattenJsonToString(input map[string]interface{}) (string, error) { + result, err := json.Marshal(input) + if err != nil { + return "", err + } + + return string(result), nil +} + +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) +} diff --git a/builtin/providers/azurerm/structure_test.go b/helper/structure/structure_test.go similarity index 88% rename from builtin/providers/azurerm/structure_test.go rename to helper/structure/structure_test.go index 7ad258333..7eb8dc27b 100644 --- a/builtin/providers/azurerm/structure_test.go +++ b/helper/structure/structure_test.go @@ -1,6 +1,8 @@ -package azurerm +package structure -import "testing" +import ( + "testing" +) func TestNormalizeJsonString(t *testing.T) { var err error @@ -22,7 +24,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) } @@ -43,7 +45,7 @@ func TestNormalizeJsonString(t *testing.T) { } } }` - actual, err = normalizeJsonString(invalidJson) + actual, err = NormalizeJsonString(invalidJson) if err == nil { t.Fatalf("Expected to throw an error while parsing JSON, but got: %s", err) } diff --git a/helper/validation/validation.go b/helper/validation/validation.go index 82a9dec72..7b894f540 100644 --- a/helper/validation/validation.go +++ b/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 +} diff --git a/helper/validation/validation_test.go b/helper/validation/validation_test.go index ed488ae32..991955d23 100644 --- a/helper/validation/validation_test.go +++ b/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