From 63173857263b4bba5c1e192d8d90940c817d463d Mon Sep 17 00:00:00 2001 From: Frederic Francois Date: Wed, 28 Jun 2023 17:15:19 +0100 Subject: [PATCH 1/5] Add option to re-use host_name with `dns_zone with soa_record` creation When a DNS zone is created in Azure, there is a default SOA record which is created. In this provider, a dns_zone with soa_record is done by: 1. creating the dns_zone 2. overwriting the soa_record of the zone with the data supplied in the soa_record config block The `soa_record` block makes it mandatory to add the host_name argument when it is specified. This is NOT desired in most cases since the host_name is the MNAME SOA field which is the name of the primary nameserver and is set by Azure. This commit attempts to fix this issue by: 1. allowing the `host_name` to be omitted when `soa_record` is specified 2. the host_name value will be derived from the original SOA record when the zone is created. This issue is partly a bug in Azure API since it is is mentioned in the [documentation](https://learn.microsoft.com/en-us/azure/dns/dns-zones-records#soa-records) that it should not be possible to update the 'host' property of the SOA record. --- internal/services/dns/dns_zone_resource.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/internal/services/dns/dns_zone_resource.go b/internal/services/dns/dns_zone_resource.go index d56130798c0c..3e78814bb3c5 100644 --- a/internal/services/dns/dns_zone_resource.go +++ b/internal/services/dns/dns_zone_resource.go @@ -87,7 +87,7 @@ func resourceDnsZone() *pluginsdk.Resource { "host_name": { Type: pluginsdk.TypeString, - Required: true, + Optional: true, ValidateFunc: validation.StringIsNotEmpty, }, @@ -180,13 +180,27 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("creating/updating %s: %+v", id, err) } + d.SetId(id.ID()) + if v, ok := d.GetOk("soa_record"); ok { soaRecord := v.([]interface{})[0].(map[string]interface{}) + + inputSOARecord := expandArmDNSZoneSOARecord(soaRecord) + + if *inputSOARecord.Host == "" { + resourceDnsZoneRead(d, meta) + if v1, ok := d.GetOk("soa_record"); ok { + readSoaRecord := v1.([]interface{})[0].(map[string]interface{}) + readSOARecord := expandArmDNSZoneSOARecord(readSoaRecord) + inputSOARecord.Host = readSOARecord.Host + } + } + rsParameters := recordsets.RecordSet{ Properties: &recordsets.RecordSetProperties{ TTL: utils.Int64(int64(soaRecord["ttl"].(int))), Metadata: tags.Expand(soaRecord["tags"].(map[string]interface{})), - SOARecord: expandArmDNSZoneSOARecord(soaRecord), + SOARecord: inputSOARecord, }, } @@ -200,8 +214,6 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er } } - d.SetId(id.ID()) - return resourceDnsZoneRead(d, meta) } From 54fc3f02168c48f69a52c5acbe693ffd8536bcd2 Mon Sep 17 00:00:00 2001 From: Frederic Francois Date: Sat, 1 Jul 2023 06:36:58 +0100 Subject: [PATCH 2/5] Add requested changes 1. get soaRecord only rather than whole zone read 2. return Set(id) to original position 3. update documentation 4. update tests --- internal/services/dns/dns_zone_resource.go | 36 +++++++++++++++---- .../services/dns/dns_zone_resource_test.go | 1 - website/docs/r/dns_zone.html.markdown | 2 +- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/internal/services/dns/dns_zone_resource.go b/internal/services/dns/dns_zone_resource.go index 3e78814bb3c5..031c208e6503 100644 --- a/internal/services/dns/dns_zone_resource.go +++ b/internal/services/dns/dns_zone_resource.go @@ -1,6 +1,7 @@ package dns import ( + "context" "fmt" "strings" "time" @@ -88,6 +89,7 @@ func resourceDnsZone() *pluginsdk.Resource { "host_name": { Type: pluginsdk.TypeString, Optional: true, + Computed: true, ValidateFunc: validation.StringIsNotEmpty, }, @@ -145,6 +147,13 @@ func resourceDnsZone() *pluginsdk.Resource { "tags": commonschema.Tags(), }, + // CustomizeDiff: pluginsdk.CustomDiffWithAll( + // pluginsdk.ValueChangeConditionFunc() + // pluginsdk.ForceNewIfChange("host_name", func(ctx context.Context, old, new, meta interface{}) bool { + // return false + // //return (old != nil && old.(string) != "") && (new == nil || new.(string) == "") + // }), + // ), } } @@ -180,20 +189,18 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er return fmt.Errorf("creating/updating %s: %+v", id, err) } - d.SetId(id.ID()) - if v, ok := d.GetOk("soa_record"); ok { soaRecord := v.([]interface{})[0].(map[string]interface{}) inputSOARecord := expandArmDNSZoneSOARecord(soaRecord) if *inputSOARecord.Host == "" { - resourceDnsZoneRead(d, meta) - if v1, ok := d.GetOk("soa_record"); ok { - readSoaRecord := v1.([]interface{})[0].(map[string]interface{}) - readSOARecord := expandArmDNSZoneSOARecord(readSoaRecord) - inputSOARecord.Host = readSOARecord.Host + host, err := soaRecordHostName(ctx, d.Timeout(pluginsdk.TimeoutRead), recordSetsClient, id) + if err != nil { + return fmt.Errorf("creating/updating %s: %+v", id, err) } + + inputSOARecord.Host = host } rsParameters := recordsets.RecordSet{ @@ -214,6 +221,8 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er } } + d.SetId(id.ID()) + return resourceDnsZoneRead(d, meta) } @@ -374,3 +383,16 @@ func flattenArmDNSZoneSOARecord(input *recordsets.RecordSet) []interface{} { return output } + +func soaRecordHostName(ctx context.Context, timeout time.Duration, recordSetsClient *recordsets.RecordSetsClient, id zones.DnsZoneId) (*string, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + soaRecord := recordsets.NewRecordTypeID(id.SubscriptionId, id.ResourceGroupName, id.DnsZoneName, recordsets.RecordTypeSOA, "@") + soaRecordResp, err := recordSetsClient.Get(ctx, soaRecord) + if err != nil { + return nil, fmt.Errorf("retrieving %s: %+v", id, err) + } + + return soaRecordResp.Model.Properties.SOARecord.Host, nil +} diff --git a/internal/services/dns/dns_zone_resource_test.go b/internal/services/dns/dns_zone_resource_test.go index bd1f167892fd..4acd4dc150dc 100644 --- a/internal/services/dns/dns_zone_resource_test.go +++ b/internal/services/dns/dns_zone_resource_test.go @@ -206,7 +206,6 @@ resource "azurerm_dns_zone" "test" { soa_record { email = "testemail.com" - host_name = "testhost.contoso.com" } } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) diff --git a/website/docs/r/dns_zone.html.markdown b/website/docs/r/dns_zone.html.markdown index 1ba6b9742beb..a77998dfdf41 100644 --- a/website/docs/r/dns_zone.html.markdown +++ b/website/docs/r/dns_zone.html.markdown @@ -42,7 +42,7 @@ The `soa_record` block supports: * `email` - (Required) The email contact for the SOA record. -* `host_name` - (Required) The domain name of the authoritative name server for the SOA record. +* `host_name` - (Optional) The domain name of the authoritative name server for the SOA record. If not set, computed value from Azure will be used. * `expire_time` - (Optional) The expire time for the SOA record. Defaults to `2419200`. From 3e00fc16bda080325d52be3f4942102653cbdf09 Mon Sep 17 00:00:00 2001 From: Frederic Francois Date: Mon, 3 Jul 2023 09:14:57 +0100 Subject: [PATCH 3/5] fix review comments --- internal/services/dns/dns_zone_resource.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/internal/services/dns/dns_zone_resource.go b/internal/services/dns/dns_zone_resource.go index 031c208e6503..4a79938524fc 100644 --- a/internal/services/dns/dns_zone_resource.go +++ b/internal/services/dns/dns_zone_resource.go @@ -147,13 +147,6 @@ func resourceDnsZone() *pluginsdk.Resource { "tags": commonschema.Tags(), }, - // CustomizeDiff: pluginsdk.CustomDiffWithAll( - // pluginsdk.ValueChangeConditionFunc() - // pluginsdk.ForceNewIfChange("host_name", func(ctx context.Context, old, new, meta interface{}) bool { - // return false - // //return (old != nil && old.(string) != "") && (new == nil || new.(string) == "") - // }), - // ), } } @@ -385,14 +378,15 @@ func flattenArmDNSZoneSOARecord(input *recordsets.RecordSet) []interface{} { } func soaRecordHostName(ctx context.Context, timeout time.Duration, recordSetsClient *recordsets.RecordSetsClient, id zones.DnsZoneId) (*string, error) { - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - soaRecord := recordsets.NewRecordTypeID(id.SubscriptionId, id.ResourceGroupName, id.DnsZoneName, recordsets.RecordTypeSOA, "@") soaRecordResp, err := recordSetsClient.Get(ctx, soaRecord) if err != nil { return nil, fmt.Errorf("retrieving %s: %+v", id, err) } + if soaRecordResp.Model.Properties.SOARecord.Host == nil { + return nil, fmt.Errorf("retrieved nil host for %s", id) + } + return soaRecordResp.Model.Properties.SOARecord.Host, nil } From b32a1d66155521beafb8c1b31014cb83a1a93903 Mon Sep 17 00:00:00 2001 From: Frederic Francois Date: Tue, 4 Jul 2023 07:51:06 +0100 Subject: [PATCH 4/5] fix reviews --- internal/services/dns/dns_zone_resource.go | 28 ++++++++++++++++--- .../services/dns/dns_zone_resource_test.go | 2 +- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/services/dns/dns_zone_resource.go b/internal/services/dns/dns_zone_resource.go index 4a79938524fc..6ebb779f40de 100644 --- a/internal/services/dns/dns_zone_resource.go +++ b/internal/services/dns/dns_zone_resource.go @@ -188,7 +188,7 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er inputSOARecord := expandArmDNSZoneSOARecord(soaRecord) if *inputSOARecord.Host == "" { - host, err := soaRecordHostName(ctx, d.Timeout(pluginsdk.TimeoutRead), recordSetsClient, id) + host, err := soaRecordHostName(ctx, recordSetsClient, id) if err != nil { return fmt.Errorf("creating/updating %s: %+v", id, err) } @@ -377,16 +377,36 @@ func flattenArmDNSZoneSOARecord(input *recordsets.RecordSet) []interface{} { return output } -func soaRecordHostName(ctx context.Context, timeout time.Duration, recordSetsClient *recordsets.RecordSetsClient, id zones.DnsZoneId) (*string, error) { +func soaRecordHostName(ctx context.Context, recordSetsClient *recordsets.RecordSetsClient, id zones.DnsZoneId) (*string, error) { soaRecord := recordsets.NewRecordTypeID(id.SubscriptionId, id.ResourceGroupName, id.DnsZoneName, recordsets.RecordTypeSOA, "@") soaRecordResp, err := recordSetsClient.Get(ctx, soaRecord) if err != nil { return nil, fmt.Errorf("retrieving %s: %+v", id, err) } - if soaRecordResp.Model.Properties.SOARecord.Host == nil { - return nil, fmt.Errorf("retrieved nil host for %s", id) + if err := checkSoaRecordResponseForNil(soaRecordResp); err != nil { + return nil, fmt.Errorf("soa record response for %s has error: %+v", id, err) } return soaRecordResp.Model.Properties.SOARecord.Host, nil } + +func checkSoaRecordResponseForNil(soaRecordResp recordsets.GetOperationResponse) error { + if soaRecordResp.Model == nil { + return fmt.Errorf("model is nil") + } + + if soaRecordResp.Model.Properties == nil { + return fmt.Errorf("properties is nil") + } + + if soaRecordResp.Model.Properties.SOARecord == nil { + return fmt.Errorf("soa record is nil") + } + + if soaRecordResp.Model.Properties.SOARecord.Host == nil { + return fmt.Errorf("host is nil") + } + + return nil +} diff --git a/internal/services/dns/dns_zone_resource_test.go b/internal/services/dns/dns_zone_resource_test.go index 4acd4dc150dc..9155681d121d 100644 --- a/internal/services/dns/dns_zone_resource_test.go +++ b/internal/services/dns/dns_zone_resource_test.go @@ -205,7 +205,7 @@ resource "azurerm_dns_zone" "test" { resource_group_name = azurerm_resource_group.test.name soa_record { - email = "testemail.com" + email = "testemail.com" } } `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) From dcc654d323f2623cf61ea61f14729b4e7c6124f7 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Fri, 7 Jul 2023 15:57:06 +0200 Subject: [PATCH 5/5] update processing of host, remove ForceNew, add tests --- internal/services/dns/dns_zone_resource.go | 75 +++++++------------ .../services/dns/dns_zone_resource_test.go | 40 +++++++++- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/internal/services/dns/dns_zone_resource.go b/internal/services/dns/dns_zone_resource.go index 6ebb779f40de..ff29ba535669 100644 --- a/internal/services/dns/dns_zone_resource.go +++ b/internal/services/dns/dns_zone_resource.go @@ -1,11 +1,11 @@ package dns import ( - "context" "fmt" "strings" "time" + "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/lang/response" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/dns/2018-05-01/zones" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/services/dns/migration" "github.com/hashicorp/terraform-provider-azurerm/internal/services/dns/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" @@ -77,7 +78,7 @@ func resourceDnsZone() *pluginsdk.Resource { MaxItems: 1, Optional: true, Computed: true, - ForceNew: true, + //ForceNew: true, Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "email": { @@ -87,10 +88,9 @@ func resourceDnsZone() *pluginsdk.Resource { }, "host_name": { - Type: pluginsdk.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringIsNotEmpty, + Type: pluginsdk.TypeString, + Optional: !features.FourPointOhBeta(), // (@jackofallops) - This should not be set or updatable to meet API design, see https://learn.microsoft.com/en-us/azure/dns/dns-zones-records#soa-records + Computed: true, }, "expire_time": { @@ -185,17 +185,21 @@ func resourceDnsZoneCreateUpdate(d *pluginsdk.ResourceData, meta interface{}) er if v, ok := d.GetOk("soa_record"); ok { soaRecord := v.([]interface{})[0].(map[string]interface{}) - inputSOARecord := expandArmDNSZoneSOARecord(soaRecord) - - if *inputSOARecord.Host == "" { - host, err := soaRecordHostName(ctx, recordSetsClient, id) - if err != nil { - return fmt.Errorf("creating/updating %s: %+v", id, err) - } + soaRecordID := recordsets.NewRecordTypeID(id.SubscriptionId, id.ResourceGroupName, id.DnsZoneName, recordsets.RecordTypeSOA, "@") + soaRecordResp, err := recordSetsClient.Get(ctx, soaRecordID) + if err != nil { + return fmt.Errorf("retrieving %s to update SOA: %+v", id, err) + } - inputSOARecord.Host = host + props := soaRecordResp.Model.Properties + if props == nil || props.SOARecord == nil { + return fmt.Errorf("could not read SOA properties for %s", id) } + inputSOARecord := expandArmDNSZoneSOARecord(soaRecord) + + inputSOARecord.Host = props.SOARecord.Host + rsParameters := recordsets.RecordSet{ Properties: &recordsets.RecordSetProperties{ TTL: utils.Int64(int64(soaRecord["ttl"].(int))), @@ -292,15 +296,20 @@ func resourceDnsZoneDelete(d *pluginsdk.ResourceData, meta interface{}) error { } func expandArmDNSZoneSOARecord(input map[string]interface{}) *recordsets.SoaRecord { - return &recordsets.SoaRecord{ + result := &recordsets.SoaRecord{ Email: utils.String(input["email"].(string)), - Host: utils.String(input["host_name"].(string)), ExpireTime: utils.Int64(int64(input["expire_time"].(int))), MinimumTTL: utils.Int64(int64(input["minimum_ttl"].(int))), RefreshTime: utils.Int64(int64(input["refresh_time"].(int))), RetryTime: utils.Int64(int64(input["retry_time"].(int))), SerialNumber: utils.Int64(int64(input["serial_number"].(int))), } + + if !features.FourPointOhBeta() && input["host_name"].(string) != "" { + result.Host = pointer.To(input["host_name"].(string)) + } + + return result } func flattenArmDNSZoneSOARecord(input *recordsets.RecordSet) []interface{} { @@ -376,37 +385,3 @@ func flattenArmDNSZoneSOARecord(input *recordsets.RecordSet) []interface{} { return output } - -func soaRecordHostName(ctx context.Context, recordSetsClient *recordsets.RecordSetsClient, id zones.DnsZoneId) (*string, error) { - soaRecord := recordsets.NewRecordTypeID(id.SubscriptionId, id.ResourceGroupName, id.DnsZoneName, recordsets.RecordTypeSOA, "@") - soaRecordResp, err := recordSetsClient.Get(ctx, soaRecord) - if err != nil { - return nil, fmt.Errorf("retrieving %s: %+v", id, err) - } - - if err := checkSoaRecordResponseForNil(soaRecordResp); err != nil { - return nil, fmt.Errorf("soa record response for %s has error: %+v", id, err) - } - - return soaRecordResp.Model.Properties.SOARecord.Host, nil -} - -func checkSoaRecordResponseForNil(soaRecordResp recordsets.GetOperationResponse) error { - if soaRecordResp.Model == nil { - return fmt.Errorf("model is nil") - } - - if soaRecordResp.Model.Properties == nil { - return fmt.Errorf("properties is nil") - } - - if soaRecordResp.Model.Properties.SOARecord == nil { - return fmt.Errorf("soa record is nil") - } - - if soaRecordResp.Model.Properties.SOARecord.Host == nil { - return fmt.Errorf("host is nil") - } - - return nil -} diff --git a/internal/services/dns/dns_zone_resource_test.go b/internal/services/dns/dns_zone_resource_test.go index 9155681d121d..cbcba96bca86 100644 --- a/internal/services/dns/dns_zone_resource_test.go +++ b/internal/services/dns/dns_zone_resource_test.go @@ -71,7 +71,7 @@ func TestAccDnsZone_withTags(t *testing.T) { }) } -func TestAccDnsZone_withSOARecord(t *testing.T) { +func TestAccDnsZone_basicSOARecord(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_dns_zone", "test") r := DnsZoneResource{} @@ -83,6 +83,43 @@ func TestAccDnsZone_withSOARecord(t *testing.T) { ), }, data.ImportStep(), + }) +} + +func TestAccDnsZone_completeSOARecord(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_dns_zone", "test") + r := DnsZoneResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.withCompletedSOARecord(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func TestAccDnsZone_updateSOARecord(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_dns_zone", "test") + r := DnsZoneResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + { + Config: r.withBasicSOARecord(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), { Config: r.withCompletedSOARecord(data), Check: acceptance.ComposeTestCheckFunc( @@ -228,7 +265,6 @@ resource "azurerm_dns_zone" "test" { soa_record { email = "testemail.com" - host_name = "testhost.contoso.com" expire_time = 2419200 minimum_ttl = 200 refresh_time = 2600