From 2f586ce6b3d5fb0cb1778687550b0b8b3e5bc548 Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Wed, 26 Jul 2017 12:44:48 +0100 Subject: [PATCH] Refactoring / fixing PR comments --- azurerm/import_arm_dns_mx_record_test.go | 31 +++++++- azurerm/resource_arm_dns_mx_record.go | 49 ++++++------ azurerm/resource_arm_dns_mx_record_test.go | 86 ++++++++++++---------- 3 files changed, 100 insertions(+), 66 deletions(-) diff --git a/azurerm/import_arm_dns_mx_record_test.go b/azurerm/import_arm_dns_mx_record_test.go index ef87fffd1338..95e448dae335 100644 --- a/azurerm/import_arm_dns_mx_record_test.go +++ b/azurerm/import_arm_dns_mx_record_test.go @@ -1,7 +1,6 @@ package azurerm import ( - "fmt" "testing" "github.com/hashicorp/terraform/helper/acctest" @@ -12,18 +11,42 @@ func TestAccAzureRMDnsMxRecord_importBasic(t *testing.T) { resourceName := "azurerm_dns_mx_record.test" ri := acctest.RandInt() - config := fmt.Sprintf(testAccAzureRMDnsMxRecord_basic, ri, ri, ri) + config := testAccAzureRMDnsMxRecord_basic(ri) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMDnsMxRecordDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: config, }, - resource.TestStep{ + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAzureRMDnsMxRecord_importWithTags(t *testing.T) { + resourceName := "azurerm_dns_mx_record.test" + + ri := acctest.RandInt() + config := testAccAzureRMDnsMxRecord_withTags(ri) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMDnsMxRecordDestroy, + Steps: []resource.TestStep{ + { + Config: config, + }, + + { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, diff --git a/azurerm/resource_arm_dns_mx_record.go b/azurerm/resource_arm_dns_mx_record.go index 1c32cfd67c20..ec7b0b1e67fd 100644 --- a/azurerm/resource_arm_dns_mx_record.go +++ b/azurerm/resource_arm_dns_mx_record.go @@ -22,34 +22,35 @@ func resourceArmDnsMxRecord() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "name": &schema.Schema{ + "name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "resource_group_name": &schema.Schema{ + "resource_group_name": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "zone_name": &schema.Schema{ + "zone_name": { Type: schema.TypeString, Required: true, }, - "record": &schema.Schema{ + "record": { Type: schema.TypeSet, Required: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "preference": &schema.Schema{ + "preference": { + // TODO: this should become an Int Type: schema.TypeString, Required: true, }, - "exchange": &schema.Schema{ + "exchange": { Type: schema.TypeString, Required: true, }, @@ -58,7 +59,7 @@ func resourceArmDnsMxRecord() *schema.Resource { Set: resourceArmDnsMxRecordHash, }, - "ttl": &schema.Schema{ + "ttl": { Type: schema.TypeInt, Required: true, }, @@ -69,31 +70,30 @@ func resourceArmDnsMxRecord() *schema.Resource { } func resourceArmDnsMxRecordCreateOrUpdate(d *schema.ResourceData, meta interface{}) error { - dnsClient := meta.(*ArmClient).dnsClient + client := meta.(*ArmClient).dnsClient name := d.Get("name").(string) resGroup := d.Get("resource_group_name").(string) zoneName := d.Get("zone_name").(string) ttl := int64(d.Get("ttl").(int)) - tags := d.Get("tags").(map[string]interface{}) - metadata := expandTags(tags) - records, err := expandAzureRmDnsMxRecords(d) - props := dns.RecordSetProperties{ - Metadata: metadata, - TTL: &ttl, - MxRecords: &records, + if err != nil { + return err } parameters := dns.RecordSet{ - Name: &name, - RecordSetProperties: &props, + Name: &name, + RecordSetProperties: &dns.RecordSetProperties{ + Metadata: expandTags(tags), + TTL: &ttl, + MxRecords: &records, + }, } - //last parameter is set to empty to allow updates to records after creation - // (per SDK, set it to '*' to prevent updates, all other values are ignored) - resp, err := dnsClient.CreateOrUpdate(resGroup, zoneName, name, dns.MX, parameters, "", "") + eTag := "" + ifNoneMatch := "" // set to empty to allow updates to records after creation + resp, err := client.CreateOrUpdate(resGroup, zoneName, name, dns.MX, parameters, eTag, ifNoneMatch) if err != nil { return err } @@ -108,7 +108,7 @@ func resourceArmDnsMxRecordCreateOrUpdate(d *schema.ResourceData, meta interface } func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error { - dnsClient := meta.(*ArmClient).dnsClient + client := meta.(*ArmClient).dnsClient id, err := parseAzureResourceID(d.Id()) if err != nil { @@ -119,7 +119,7 @@ func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error name := id.Path["MX"] zoneName := id.Path["dnszones"] - resp, err := dnsClient.Get(resGroup, zoneName, name, dns.MX) + resp, err := client.Get(resGroup, zoneName, name, dns.MX) if err != nil { return fmt.Errorf("Error reading DNS MX record %s: %v", name, err) } @@ -142,7 +142,7 @@ func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error } func resourceArmDnsMxRecordDelete(d *schema.ResourceData, meta interface{}) error { - dnsClient := meta.(*ArmClient).dnsClient + client := meta.(*ArmClient).dnsClient id, err := parseAzureResourceID(d.Id()) if err != nil { @@ -153,7 +153,7 @@ func resourceArmDnsMxRecordDelete(d *schema.ResourceData, meta interface{}) erro name := id.Path["MX"] zoneName := id.Path["dnszones"] - resp, error := dnsClient.Delete(resGroup, zoneName, name, dns.MX, "") + resp, error := client.Delete(resGroup, zoneName, name, dns.MX, "") if resp.StatusCode != http.StatusOK { return fmt.Errorf("Error deleting DNS MX Record %s: %+v", name, error) } @@ -207,6 +207,7 @@ func expandAzureRmDnsMxRecords(d *schema.ResourceData) ([]dns.MxRecord, error) { func resourceArmDnsMxRecordHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["preference"].(string))) buf.WriteString(fmt.Sprintf("%s-", m["exchange"].(string))) diff --git a/azurerm/resource_arm_dns_mx_record_test.go b/azurerm/resource_arm_dns_mx_record_test.go index 95aadbbdfee7..8a6c614a5ca7 100644 --- a/azurerm/resource_arm_dns_mx_record_test.go +++ b/azurerm/resource_arm_dns_mx_record_test.go @@ -12,18 +12,19 @@ import ( ) func TestAccAzureRMDnsMxRecord_basic(t *testing.T) { + resourceName := "azurerm_dns_mx_record.test" ri := acctest.RandInt() - config := fmt.Sprintf(testAccAzureRMDnsMxRecord_basic, ri, ri, ri) + config := testAccAzureRMDnsMxRecord_basic(ri) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMDnsMxRecordDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: config, Check: resource.ComposeTestCheckFunc( - testCheckAzureRMDnsMxRecordExists("azurerm_dns_mx_record.test"), + testCheckAzureRMDnsMxRecordExists(resourceName), ), }, }, @@ -31,30 +32,28 @@ func TestAccAzureRMDnsMxRecord_basic(t *testing.T) { } func TestAccAzureRMDnsMxRecord_updateRecords(t *testing.T) { + resourceName := "azurerm_dns_mx_record.test" ri := acctest.RandInt() - preConfig := fmt.Sprintf(testAccAzureRMDnsMxRecord_basic, ri, ri, ri) - postConfig := fmt.Sprintf(testAccAzureRMDnsMxRecord_updateRecords, ri, ri, ri) + preConfig := testAccAzureRMDnsMxRecord_basic(ri) + postConfig := testAccAzureRMDnsMxRecord_updateRecords(ri) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMDnsMxRecordDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: preConfig, Check: resource.ComposeTestCheckFunc( - testCheckAzureRMDnsMxRecordExists("azurerm_dns_mx_record.test"), - resource.TestCheckResourceAttr( - "azurerm_dns_mx_record.test", "record.#", "2"), + testCheckAzureRMDnsMxRecordExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "record.#", "2"), ), }, - - resource.TestStep{ + { Config: postConfig, Check: resource.ComposeTestCheckFunc( - testCheckAzureRMDnsMxRecordExists("azurerm_dns_mx_record.test"), - resource.TestCheckResourceAttr( - "azurerm_dns_mx_record.test", "record.#", "3"), + testCheckAzureRMDnsMxRecordExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "record.#", "3"), ), }, }, @@ -62,30 +61,28 @@ func TestAccAzureRMDnsMxRecord_updateRecords(t *testing.T) { } func TestAccAzureRMDnsMxRecord_withTags(t *testing.T) { + resourceName := "azurerm_dns_mx_record.test" ri := acctest.RandInt() - preConfig := fmt.Sprintf(testAccAzureRMDnsMxRecord_withTags, ri, ri, ri) - postConfig := fmt.Sprintf(testAccAzureRMDnsMxRecord_withTagsUpdate, ri, ri, ri) + preConfig := testAccAzureRMDnsMxRecord_withTags(ri) + postConfig := testAccAzureRMDnsMxRecord_withTagsUpdate(ri) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMDnsMxRecordDestroy, Steps: []resource.TestStep{ - resource.TestStep{ + { Config: preConfig, Check: resource.ComposeTestCheckFunc( - testCheckAzureRMDnsMxRecordExists("azurerm_dns_mx_record.test"), - resource.TestCheckResourceAttr( - "azurerm_dns_mx_record.test", "tags.%", "2"), + testCheckAzureRMDnsMxRecordExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), ), }, - - resource.TestStep{ + { Config: postConfig, Check: resource.ComposeTestCheckFunc( - testCheckAzureRMDnsMxRecordExists("azurerm_dns_mx_record.test"), - resource.TestCheckResourceAttr( - "azurerm_dns_mx_record.test", "tags.%", "1"), + testCheckAzureRMDnsMxRecordExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), ), }, }, @@ -110,7 +107,7 @@ func testCheckAzureRMDnsMxRecordExists(name string) resource.TestCheckFunc { conn := testAccProvider.Meta().(*ArmClient).dnsClient resp, err := conn.Get(resourceGroup, zoneName, mxName, dns.MX) if err != nil { - return fmt.Errorf("Bad: Get MX RecordSet: %v", err) + return fmt.Errorf("Bad: Get MX RecordSet: %+v", err) } if resp.StatusCode == http.StatusNotFound { @@ -136,23 +133,26 @@ func testCheckAzureRMDnsMxRecordDestroy(s *terraform.State) error { resp, err := conn.Get(resourceGroup, zoneName, mxName, dns.MX) if err != nil { - return nil - } + if resp.StatusCode == http.StatusNotFound { + return nil + } - if resp.StatusCode != http.StatusNotFound { - return fmt.Errorf("DNS MX record still exists:\n%#v", resp.RecordSetProperties) + return err } + return fmt.Errorf("DNS MX record still exists:\n%#v", resp.RecordSetProperties) } return nil } -var testAccAzureRMDnsMxRecord_basic = ` +func testAccAzureRMDnsMxRecord_basic(rInt int) string { + return fmt.Sprintf(` resource "azurerm_resource_group" "test" { name = "acctestRG_%d" location = "West US" } + resource "azurerm_dns_zone" "test" { name = "acctestzone%d.com" resource_group_name = "${azurerm_resource_group.test.name}" @@ -174,13 +174,16 @@ resource "azurerm_dns_mx_record" "test" { exchange = "mail2.contoso.com" } } -` +`, rInt, rInt, rInt) +} -var testAccAzureRMDnsMxRecord_updateRecords = ` +func testAccAzureRMDnsMxRecord_updateRecords(rInt int) string { + return fmt.Sprintf(` resource "azurerm_resource_group" "test" { name = "acctestRG_%d" location = "West US" } + resource "azurerm_dns_zone" "test" { name = "acctestzone%d.com" resource_group_name = "${azurerm_resource_group.test.name}" @@ -207,13 +210,16 @@ resource "azurerm_dns_mx_record" "test" { exchange = "mail3.contoso.com" } } -` +`, rInt, rInt, rInt) +} -var testAccAzureRMDnsMxRecord_withTags = ` +func testAccAzureRMDnsMxRecord_withTags(rInt int) string { + return fmt.Sprintf(` resource "azurerm_resource_group" "test" { name = "acctestRG_%d" location = "West US" } + resource "azurerm_dns_zone" "test" { name = "acctestzone%d.com" resource_group_name = "${azurerm_resource_group.test.name}" @@ -240,13 +246,16 @@ resource "azurerm_dns_mx_record" "test" { cost_center = "MSFT" } } -` +`, rInt, rInt, rInt) +} -var testAccAzureRMDnsMxRecord_withTagsUpdate = ` +func testAccAzureRMDnsMxRecord_withTagsUpdate(rInt int) string { + return fmt.Sprintf(` resource "azurerm_resource_group" "test" { name = "acctestRG_%d" location = "West US" } + resource "azurerm_dns_zone" "test" { name = "acctestzone%d.com" resource_group_name = "${azurerm_resource_group.test.name}" @@ -272,4 +281,5 @@ resource "azurerm_dns_mx_record" "test" { environment = "staging" } } -` +`, rInt, rInt, rInt) +}