From 92052f00e2b8d8728d723caf107cfd0cb9ea6988 Mon Sep 17 00:00:00 2001 From: njucz Date: Tue, 14 Jul 2020 16:11:37 +0800 Subject: [PATCH 1/4] 1. fix TC bug 2. check whether two dns zone have the same name --- .../services/dns/dns_zone_data_source.go | 43 ++++++++++--------- .../dns/tests/dns_zone_data_source_test.go | 3 +- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/azurerm/internal/services/dns/dns_zone_data_source.go b/azurerm/internal/services/dns/dns_zone_data_source.go index 3c4586a34e8a..d7f2aa5d2ea7 100644 --- a/azurerm/internal/services/dns/dns_zone_data_source.go +++ b/azurerm/internal/services/dns/dns_zone_data_source.go @@ -5,8 +5,9 @@ import ( "fmt" "time" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/dns/parse" + "github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2018-05-01/dns" - "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" @@ -77,14 +78,12 @@ func dataSourceArmDnsZoneRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error reading DNS Zone %q (Resource Group %q): %+v", name, resourceGroup, err) } } else { - rgClient := meta.(*clients.Client).Resource.GroupsClient - - resp, resourceGroup, err = findZone(client, rgClient, ctx, name) + resp, resourceGroup, err = findZone(client, ctx, name) if err != nil { return err } - if resourceGroup == "" { + if resp.ID == nil || *resp.ID == "" { return fmt.Errorf("Error: DNS Zone %q was not found", name) } } @@ -109,26 +108,30 @@ func dataSourceArmDnsZoneRead(d *schema.ResourceData, meta interface{}) error { return tags.FlattenAndSet(d, resp.Tags) } -func findZone(client *dns.ZonesClient, rgClient *resources.GroupsClient, ctx context.Context, name string) (dns.Zone, string, error) { - groups, err := rgClient.List(ctx, "", nil) +func findZone(client *dns.ZonesClient, ctx context.Context, name string) (dns.Zone, string, error) { + zonesIterator, err := client.ListComplete(ctx, nil) if err != nil { - return dns.Zone{}, "", fmt.Errorf("Error listing Resource Groups: %+v", err) + return dns.Zone{}, "", fmt.Errorf("listing DNS Zones: %+v", err) } - for _, g := range groups.Values() { - resourceGroup := *g.Name - - zones, err := client.ListByResourceGroup(ctx, resourceGroup, nil) - if err != nil { - return dns.Zone{}, "", fmt.Errorf("Error listing DNS Zones (Resource Group: %s): %+v", resourceGroup, err) - } - - for _, z := range zones.Values() { - if *z.Name == name { - return z, resourceGroup, nil + var found *dns.Zone + for zonesIterator.NotDone() { + zone := zonesIterator.Value() + if *zone.Name == name { + if found != nil { + return dns.Zone{}, "", fmt.Errorf("found multiple DNS zones with name %q, please specify the resource group", name) } + found = &zone + } + if err := zonesIterator.NextWithContext(ctx); err != nil { + return dns.Zone{}, "", fmt.Errorf("listing DNS Zones: %+v", err) } } - return dns.Zone{}, "", nil + if found == nil { + return dns.Zone{}, "", fmt.Errorf("could not find DNS zone with name: %q", name) + } + + id, _ := parse.DnsZoneID(*found.ID) + return *found, id.ResourceGroup, nil } diff --git a/azurerm/internal/services/dns/tests/dns_zone_data_source_test.go b/azurerm/internal/services/dns/tests/dns_zone_data_source_test.go index 94e5127124b7..736cf8accc0a 100644 --- a/azurerm/internal/services/dns/tests/dns_zone_data_source_test.go +++ b/azurerm/internal/services/dns/tests/dns_zone_data_source_test.go @@ -47,7 +47,8 @@ func TestAccDataSourceAzureRMDNSZone_tags(t *testing.T) { func TestAccDataSourceAzureRMDNSZone_withoutResourceGroupName(t *testing.T) { data := acceptance.BuildTestData(t, "data.azurerm_dns_zone", "test") - resourceGroupName := fmt.Sprintf("acctestRG-%d", data.RandomInteger) + // resource group of DNS zone is always small case + resourceGroupName := fmt.Sprintf("acctestrg-%d", data.RandomInteger) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, From cb6cc6076e5663490576feb89c258c2deb27ea74 Mon Sep 17 00:00:00 2001 From: njucz Date: Tue, 14 Jul 2020 17:37:36 +0800 Subject: [PATCH 2/4] update --- .../internal/services/dns/dns_zone_data_source.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/azurerm/internal/services/dns/dns_zone_data_source.go b/azurerm/internal/services/dns/dns_zone_data_source.go index d7f2aa5d2ea7..70d5917f2c63 100644 --- a/azurerm/internal/services/dns/dns_zone_data_source.go +++ b/azurerm/internal/services/dns/dns_zone_data_source.go @@ -5,11 +5,10 @@ import ( "fmt" "time" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/dns/parse" - "github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2018-05-01/dns" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/dns/parse" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -117,7 +116,7 @@ func findZone(client *dns.ZonesClient, ctx context.Context, name string) (dns.Zo var found *dns.Zone for zonesIterator.NotDone() { zone := zonesIterator.Value() - if *zone.Name == name { + if zone.Name != nil && *zone.Name == name { if found != nil { return dns.Zone{}, "", fmt.Errorf("found multiple DNS zones with name %q, please specify the resource group", name) } @@ -128,10 +127,13 @@ func findZone(client *dns.ZonesClient, ctx context.Context, name string) (dns.Zo } } - if found == nil { + if found == nil || found.ID == nil { return dns.Zone{}, "", fmt.Errorf("could not find DNS zone with name: %q", name) } - id, _ := parse.DnsZoneID(*found.ID) + id, err := parse.DnsZoneID(*found.ID) + if err != nil { + return dns.Zone{}, "", fmt.Errorf("DNS zone id not valid: %+v", err) + } return *found, id.ResourceGroup, nil } From 0270e7a7003a3be8c467bbaae51fe28b1725bb09 Mon Sep 17 00:00:00 2001 From: njucz Date: Wed, 15 Jul 2020 10:39:15 +0800 Subject: [PATCH 3/4] update --- .../services/dns/dns_zone_data_source.go | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/azurerm/internal/services/dns/dns_zone_data_source.go b/azurerm/internal/services/dns/dns_zone_data_source.go index 70d5917f2c63..28e29783d864 100644 --- a/azurerm/internal/services/dns/dns_zone_data_source.go +++ b/azurerm/internal/services/dns/dns_zone_data_source.go @@ -77,14 +77,17 @@ func dataSourceArmDnsZoneRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error reading DNS Zone %q (Resource Group %q): %+v", name, resourceGroup, err) } } else { - resp, resourceGroup, err = findZone(client, ctx, name) + var zone *dns.Zone + zone, resourceGroup, err = findZone(client, ctx, name) if err != nil { return err } - if resp.ID == nil || *resp.ID == "" { + if zone == nil { return fmt.Errorf("Error: DNS Zone %q was not found", name) } + + resp = *zone } d.SetId(*resp.ID) @@ -107,10 +110,10 @@ func dataSourceArmDnsZoneRead(d *schema.ResourceData, meta interface{}) error { return tags.FlattenAndSet(d, resp.Tags) } -func findZone(client *dns.ZonesClient, ctx context.Context, name string) (dns.Zone, string, error) { +func findZone(client *dns.ZonesClient, ctx context.Context, name string) (*dns.Zone, string, error) { zonesIterator, err := client.ListComplete(ctx, nil) if err != nil { - return dns.Zone{}, "", fmt.Errorf("listing DNS Zones: %+v", err) + return nil, "", fmt.Errorf("listing DNS Zones: %+v", err) } var found *dns.Zone @@ -118,22 +121,22 @@ func findZone(client *dns.ZonesClient, ctx context.Context, name string) (dns.Zo zone := zonesIterator.Value() if zone.Name != nil && *zone.Name == name { if found != nil { - return dns.Zone{}, "", fmt.Errorf("found multiple DNS zones with name %q, please specify the resource group", name) + return nil, "", fmt.Errorf("found multiple DNS zones with name %q, please specify the resource group", name) } found = &zone } if err := zonesIterator.NextWithContext(ctx); err != nil { - return dns.Zone{}, "", fmt.Errorf("listing DNS Zones: %+v", err) + return nil, "", fmt.Errorf("listing DNS Zones: %+v", err) } } if found == nil || found.ID == nil { - return dns.Zone{}, "", fmt.Errorf("could not find DNS zone with name: %q", name) + return nil, "", fmt.Errorf("could not find DNS zone with name: %q", name) } id, err := parse.DnsZoneID(*found.ID) if err != nil { - return dns.Zone{}, "", fmt.Errorf("DNS zone id not valid: %+v", err) + return nil, "", fmt.Errorf("DNS zone id not valid: %+v", err) } - return *found, id.ResourceGroup, nil + return found, id.ResourceGroup, nil } From dfac8a5a72fe34208fa37a163b9e0cbe7d004516 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Thu, 16 Jul 2020 12:40:32 +0100 Subject: [PATCH 4/4] added nil and empty check for id --- azurerm/internal/services/dns/dns_zone_data_source.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/azurerm/internal/services/dns/dns_zone_data_source.go b/azurerm/internal/services/dns/dns_zone_data_source.go index 28e29783d864..96b6f3383256 100644 --- a/azurerm/internal/services/dns/dns_zone_data_source.go +++ b/azurerm/internal/services/dns/dns_zone_data_source.go @@ -90,7 +90,11 @@ func dataSourceArmDnsZoneRead(d *schema.ResourceData, meta interface{}) error { resp = *zone } + if resp.ID == nil || *resp.ID == "" { + return fmt.Errorf("failed reading ID for DNS Zone %q (Resource Group %q)", name, resourceGroup) + } d.SetId(*resp.ID) + d.Set("name", name) d.Set("resource_group_name", resourceGroup)