From fdac9487f5b7715f15d06234e76b97d700d1870d Mon Sep 17 00:00:00 2001 From: Niklas Dahmen Date: Tue, 30 Apr 2024 10:10:11 +0200 Subject: [PATCH 1/2] feat(dns): Adding SOA deletion short-circuit and test --- .../services/dns/resource_dns_record_set.go | 13 +++--- .../dns/resource_dns_record_set_test.go.erb | 43 +++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/mmv1/third_party/terraform/services/dns/resource_dns_record_set.go b/mmv1/third_party/terraform/services/dns/resource_dns_record_set.go index 58802e8ac510..177ece5f1c0f 100644 --- a/mmv1/third_party/terraform/services/dns/resource_dns_record_set.go +++ b/mmv1/third_party/terraform/services/dns/resource_dns_record_set.go @@ -457,14 +457,17 @@ func resourceDnsRecordSetDelete(d *schema.ResourceData, meta interface{}) error zone := d.Get("managed_zone").(string) - // NS records must always have a value, so we short-circuit delete - // this allows terraform delete to work, but may have unexpected - // side-effects when deleting just that record set. + // NS and SOA records on the root zone must always have a value, + // so we short-circuit delete this allows terraform delete to work, + // but may have unexpected side-effects when deleting just that + // record set. // Unfortunately, you can set NS records on subdomains, and those // CAN and MUST be deleted, so we need to retrieve the managed zone, // check if what we're looking at is a subdomain, and only not delete // if it's not actually a subdomain - if d.Get("type").(string) == "NS" { + // This does not apply to SOA, as they can only be set on the root + // zone. + if d.Get("type").(string) == "NS" || d.Get("type").(string) == "SOA" { mz, err := config.NewDnsClient(userAgent).ManagedZones.Get(project, zone).Do() if err != nil { return fmt.Errorf("Error retrieving managed zone %q from %q: %s", zone, project, err) @@ -472,7 +475,7 @@ func resourceDnsRecordSetDelete(d *schema.ResourceData, meta interface{}) error domain := mz.DnsName if domain == d.Get("name").(string) { - log.Println("[DEBUG] NS records can't be deleted due to API restrictions, so they're being left in place. See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/dns_record_set for more information.") + log.Printf("[DEBUG] root-level %s records can't be deleted due to API restrictions, so they're being left in place. See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/dns_record_set for more information.\n", d.Get("type").(string)) return nil } } diff --git a/mmv1/third_party/terraform/services/dns/resource_dns_record_set_test.go.erb b/mmv1/third_party/terraform/services/dns/resource_dns_record_set_test.go.erb index 4255d51eeb4e..51fd7c52cb64 100644 --- a/mmv1/third_party/terraform/services/dns/resource_dns_record_set_test.go.erb +++ b/mmv1/third_party/terraform/services/dns/resource_dns_record_set_test.go.erb @@ -208,6 +208,30 @@ func TestAccDNSRecordSet_secondaryNS(t *testing.T) { }) } +// tracks fix for https://github.com/hashicorp/terraform-provider-google/issues/12827 +func TestAccDNSRecordSet_deletionSOA(t *testing.T) { + t.Parallel() + + zoneName := fmt.Sprintf("dnszone-test-soa-%s", acctest.RandString(t, 10)) + recordSetName := "google_dns_managed_zone.parent-zone.dns_name" + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckDnsRecordSetDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccDnsRecordSet_SOA(zoneName, recordSetName, 300), + }, + { + ResourceName: "google_dns_record_set.foobar", + ImportStateId: fmt.Sprintf("projects/%s/managedZones/%s/rrsets/%s.hashicorptest.com./SOA", envvar.GetTestProjectFromEnv(), zoneName, zoneName), + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccDNSRecordSet_quotedTXT(t *testing.T) { t.Parallel() @@ -679,6 +703,25 @@ resource "google_dns_record_set" "foobar" { `, zoneName, zoneName, zoneName, ttl) } + +func testAccDnsRecordSet_SOA(name string, recordSetName string, ttl int) string { + return fmt.Sprintf(` +resource "google_dns_managed_zone" "parent-zone" { + name = "%s" + dns_name = "%s.hashicorptest.com." + description = "Test Description" +} + +resource "google_dns_record_set" "foobar" { + managed_zone = google_dns_managed_zone.parent-zone.name + name = %s + type = "SOA" + rrdatas = ["ns-cloud-a1.googledomains.com. cloud-dns-hostmaster.google.com. 629010464 900 900 1800 60"] + ttl = %d +} +`, name, name, recordSetName, ttl) +} + func testAccDnsRecordSet_quotedTXT(name string, ttl int) string { return fmt.Sprintf(` resource "google_dns_managed_zone" "parent-zone" { From a849a387f9ffb4974d0b091f319dfbac956d7ddf Mon Sep 17 00:00:00 2001 From: Niklas Dahmen Date: Tue, 30 Apr 2024 10:10:32 +0200 Subject: [PATCH 2/2] feat(docs): Clarifiying SOA-short-circuit in docs --- .../terraform/website/docs/r/dns_record_set.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/website/docs/r/dns_record_set.html.markdown b/mmv1/third_party/terraform/website/docs/r/dns_record_set.html.markdown index d2612690ab08..a71c25e1d598 100644 --- a/mmv1/third_party/terraform/website/docs/r/dns_record_set.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/dns_record_set.html.markdown @@ -9,7 +9,7 @@ description: |- Manages a set of DNS records within Google Cloud DNS. For more information see [the official documentation](https://cloud.google.com/dns/records/) and [API](https://cloud.google.com/dns/api/v1/resourceRecordSets). -~> **Note:** The provider treats this resource as an authoritative record set. This means existing records (including the default records) for the given type will be overwritten when you create this resource in Terraform. In addition, the Google Cloud DNS API requires NS records to be present at all times, so Terraform will not actually remove NS records during destroy but will report that it did. +~> **Note:** The provider treats this resource as an authoritative record set. This means existing records (including the default records) for the given type will be overwritten when you create this resource in Terraform. In addition, the Google Cloud DNS API requires NS and SOA records to be present at all times, so Terraform will not actually remove NS or SOA records on the root of the zone during destroy but will report that it did. ## Example Usage