dnsimple / terraform-provider-dnsimple

Terraform DNSimple provider.
https://www.terraform.io/docs/providers/dnsimple/
Mozilla Public License 2.0
22 stars 20 forks source link

dnsimple_domain_delegation name_servers should be a set, not a list #159

Closed paddycarver closed 8 months ago

paddycarver commented 11 months ago

Terraform Version

v1.5.7

Affected Resource(s)

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "dnsimple_domain" "my_domain" {
  name = "dnsimple.com"
}

resource "google_dns_managed_zone" "my_domain" {
  name        = "my-domain"
  dns_name    = "dnsimple.com."
}

resource "dnsimple_domain_delegation" "my_domain" {
  domain       = dnsimple_domain.my_domain.name
  name_servers = google_dns_managed_zone.my_domain.name_servers
}

Debug Output

Please provider a link to a GitHub Gist containing the complete debug output: https://www.terraform.io/docs/internals/debugging.html. Please do NOT paste the debug output in the issue; just paste a link to the Gist.

Expected Behavior

After applying, if the name_servers don't change, there shouldn't be a diff in the plan.

Actual Behavior

I get a permanent diff:

  # dnsimple_domain_delegation.my_domain must be replaced
-/+ resource "dnsimple_domain_delegation" "my_domain" {
      ~ id           = "dnsimple.com" -> (known after apply)
      ~ name_servers = [ # forces replacement
          - "ns-cloud-a4.googledomains.com",
            "ns-cloud-a1.googledomains.com",
            # (1 unchanged element hidden)
            "ns-cloud-a3.googledomains.com",
          + "ns-cloud-a4.googledomains.com",
        ]
        # (1 unchanged attribute hidden)
    }

Steps to Reproduce

  1. terraform apply
  2. terraform plan

Important Factoids

This doesn't seem to happen on every domain, even with the same name servers.

It appears that DNSimple's API treats the list of name servers as unordered, and even after apply is returning them in the original state. I've verified this by manually editing them through the DNSimple console.

If the DNSimple API treats these items as unordered, then a set would be a more appropriate type than a list here, as the list treats changes in order as significant, while the set does not.

DXTimer commented 11 months ago

@paddycarver thank you for taking the time to report. After looking into this for a bit I found that it's caused due to the order. Our API returns a sorted set and our comparison doesn't take this into account. We'll work on a fix this week, in the mean time if you want to avoid the perma-diff I would suggest sorting the NS records.

paddycarver commented 11 months ago

Even with this config:

resource "dnsimple_domain" "my_domain" {
  name = "dnsimple.com"
}

resource "google_dns_managed_zone" "my_domain" {
  name        = "my-domain"
  dns_name    = "dnsimple.com."
}

resource "dnsimple_domain_delegation" "my_domain" {
  domain       = dnsimple_domain.my_domain.name
  name_servers = sort([for s in google_dns_managed_zone.my_domain.name_servers : trimsuffix(s, ".")])
}

I still get the perma-diff. If you look closely, you'll see that in my plan above, the state has a4 first, and the plan is to move it last, because that's what's in the config. So the config is sorted, but the state is not. No matter how many times I apply it, the state does not become sorted. Likewise, https://dnsimple.com/a/{account_number}/domains/{domain}/registration shows the delegation nameservers in sorted order, but if https://dnsimple.com/a/{account_number}/domains/{domain}/name_servers/edit reflects the order seen in the state. No matter how many times I apply or edit by hand, the order in state and on the edit page doesn't change.

Calling the API manually, I can confirm the API isn't sorting the results for me and the raw results match what's in the state. Can also confirm that calling the API to set them to what's in the config manually still returns what's in the state. Even removing ns-cloud-a4 manually through the API, retrieving it from the API to verify ns-cloud-a4 is gone, adding ns-cloud-a4 at the end, and then re-retrieving the name servers from the API still lists ns-cloud-a4 at the end. If the API is sorting, its sort algorithm may be broken? Of all my domains, this impacts all of them with ns-cloud-a[1-4] as the name servers, but none of the other domains with different name servers. It's not at all clear to me where this order is coming from, but it's consistent, and coming from the API.

If the API is sorting the results anyways and the order isn't even internally consistent, that would imply that order is not meaningful. In which case, a set is probably a better choice, as it will never consider a difference in order to be a diff. But if you want to stick with a list, an important wrinkle to remember is that Terraform requires the value in the plan to either be the exact value that was in the config, or the exact value that was in the state.

Let me know if I can be of any assistance!

paddycarver commented 9 months ago

If anyone else is running into this, there's a workaround:

DNSimple apparently only tracks the order for a unique set of nameservers once, no matter how many domains use it, and as long as any domain still uses that set of nameservers, you can't change their order at all, it'll just use the order the existing domain has for them. So if you have example.com and example.org both pointing to ns4.com, ns1.com, ns2.com, and ns3.com, and edit only example.com or example.org to reorder them, DNSimple will happily report that it did what you asked and silently discard your ordering changes.

Instead what you need to do is remove ns4.com from both example.com and example.org, then add it back in the correct order. After removing it from both, the first domain to add it back gets to determine the order, and once that set of nameservers is on more than one domain, it can't be reordered until you go through the removal and re-adding step again.

I think the Terraform provider can just make this a set, honestly, and it will solve the problem.