Only handle root NS records special. #807
Conversation
We introduced special handling for NS records in 1.2.0 under the assumption that ALL NS records can't be deleted. This isn't actually true. Only NS records for the naked domain of the managed zone can't be removed; all other NS records can be. Because of this, 1.2.0 contains a bug where all NS records are removed. This update fixes the situation to only use special handling on NS records that are for the naked root domain of the managed zone, and treat all subdomain NS records as normal records. It also adds a test to ensure this functionality. Fixes #729.
google/resource_dns_record_set.go
Outdated
| // if it's not actually a subdomain | ||
| var domain string | ||
| if d.Get("type").(string) == "NS" { | ||
| if domain == "" { |
There was a problem hiding this comment.
this condition will always be true... var domain is never updated after being set.
There was a problem hiding this comment.
Domain is updated 5 lines below, on line 190.
There was a problem hiding this comment.
domain == "" will always be true. Nothing updates it between 183 and 185.
There was a problem hiding this comment.
Ah, yes, for some reason I was trying to reuse that request between records.
Which makes no sense. o.o
google/resource_dns_record_set.go
Outdated
| domain = mz.DnsName | ||
| } | ||
|
|
||
| if domain == d.Get("name").(string) { |
There was a problem hiding this comment.
why not comparing with record.Name like above?
There was a problem hiding this comment.
Above, we're comparing if a record exists with the same name we've configured. Here, we're testing if the root of the domain is the same as what we're trying to delete.
Basically, testing for two different things:
- On create, we want to make sure any NS records that currently exist are removed in the same request we're adding NS records in.
- On delete, we want to delete the record iff it's not both an NS record and the same name as the zone.
The first is achieved by listing the NS records on a zone, and making note of any that have the same name as the one we're adding, so they can be removed. The second is achieved by retrieving the name of the managed zone, and comparing it to the record we're trying to delete.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
We introduced special handling for NS records in 1.2.0 under the
assumption that ALL NS records can't be deleted. This isn't actually
true. Only NS records for the naked domain of the managed zone can't be
removed; all other NS records can be. Because of this, 1.2.0 contains a
bug where all NS records are removed.
This update fixes the situation to only use special handling on NS
records that are for the naked root domain of the managed zone, and
treat all subdomain NS records as normal records. It also adds a test to
ensure this functionality.
Fixes #729.