diff --git a/models/dns.go b/models/dns.go index 20f853270..09fc07288 100644 --- a/models/dns.go +++ b/models/dns.go @@ -2,6 +2,7 @@ package models import ( "encoding/json" + "fmt" "strings" ) @@ -49,6 +50,7 @@ type DNSProviderConfig struct { // Nameserver describes a nameserver. type Nameserver struct { Name string `json:"name"` // Normalized to a FQDN with NO trailing "." + // NB(tlim): DomainConfig.Nameservers are stored WITH a trailing "." (Sorry!) } func (n *Nameserver) String() string { @@ -56,6 +58,8 @@ func (n *Nameserver) String() string { } // StringsToNameservers constructs a list of *Nameserver structs using a list of FQDNs. +// Deprecated. Please use ToNameservers, or maybe ToNameserversStripTD instead. +// See https://github.com/StackExchange/dnscontrol/issues/491 func StringsToNameservers(nss []string) []*Nameserver { nservers := []*Nameserver{} for _, ns := range nss { @@ -64,7 +68,37 @@ func StringsToNameservers(nss []string) []*Nameserver { return nservers } -// NameserversToStrings constructs a list of lists from *Nameserver structs +// ToNameservers turns a list of strings into a list of Nameservers. +// It is an error if any string has a trailing dot. Either remove the +// trailing dot before you call this or (much preferred) use ToNameserversStripTD. +func ToNameservers(nss []string) ([]*Nameserver, error) { + nservers := []*Nameserver{} + for _, ns := range nss { + if strings.HasSuffix(ns, ".") { + return nil, fmt.Errorf("provider code leaves trailing dot on nameserver") + // If you see this error, maybe the provider should call + // ToNameserversStripTD instead. + } + nservers = append(nservers, &Nameserver{Name: ns}) + } + return nservers, nil +} + +// ToNameserversStripTD is like ToNameservers but strips the trailing +// dot from each item. It is an error if there is no trailing dot. +func ToNameserversStripTD(nss []string) ([]*Nameserver, error) { + nservers := []*Nameserver{} + for _, ns := range nss { + if !strings.HasSuffix(ns, ".") { + return nil, fmt.Errorf("provider code already removed nameserver trailing dot (%v)", ns) + // If you see this error, maybe the provider should call ToNameservers instead. + } + nservers = append(nservers, &Nameserver{Name: ns[0 : len(ns)-1]}) + } + return nservers, nil +} + +// NameserversToStrings constructs a list of strings from *Nameserver structs func NameserversToStrings(nss []*Nameserver) (s []string) { for _, ns := range nss { s = append(s, ns.Name) diff --git a/models/tdwarn.go b/models/tdwarn.go new file mode 100644 index 000000000..ca2860b94 --- /dev/null +++ b/models/tdwarn.go @@ -0,0 +1,13 @@ +package models + +import "fmt" + +var dotwarned = map[string]bool{} + +func WarnNameserverDot(p, w string) { + if dotwarned[p] { + return + } + fmt.Printf("Warning: provider %s could be improved. See https://github.com/StackExchange/dnscontrol/issues/491 (%s)\n", p, w) + dotwarned[p] = true +} diff --git a/models/tonameservers_test.go b/models/tonameservers_test.go new file mode 100644 index 000000000..91ae4c436 --- /dev/null +++ b/models/tonameservers_test.go @@ -0,0 +1,51 @@ +package models + +import ( + "testing" +) + +func TestToNameservers(t *testing.T) { + nss, e := ToNameservers([]string{"example.com", "example2.tld"}) + if e != nil { + t.Errorf("error e %v (%v)", e, nss) + } + if len(nss) != 2 { + t.Errorf("error len: %v", nss) + } + if nss[0].Name != "example.com" { + t.Errorf("error 0: %v", nss[0].Name) + } + if nss[1].Name != "example2.tld" { + t.Errorf("error 1: %v", nss[1].Name) + } +} + +func TestToNameservers_neg(t *testing.T) { + nss, e := ToNameservers([]string{"example.com.", "example2.tld."}) + if e == nil { + t.Errorf("error 3 (%v)", nss) + } +} + +func TestToNameserversStripTD(t *testing.T) { + nss, e := ToNameserversStripTD([]string{"example.com.", "example2.tld."}) + if e != nil { + t.Errorf("error e %v (%v)", e, nss) + } + if len(nss) != 2 { + t.Errorf("error len: %v", nss) + } + if nss[0].Name != "example.com" { + t.Errorf("error 0: %v", nss[0].Name) + } + if nss[1].Name != "example2.tld" { + t.Errorf("error 1: %v", nss[1].Name) + } +} + +func TestToNameserversStripTD_neg(t *testing.T) { + nss, e := ToNameserversStripTD([]string{"example.com", "example2.tld"}) + if e == nil { + t.Errorf("error e (%v)", nss) + } +} diff --git a/pkg/nameservers/nameservers.go b/pkg/nameservers/nameservers.go index 2ac143546..e74d082ba 100644 --- a/pkg/nameservers/nameservers.go +++ b/pkg/nameservers/nameservers.go @@ -26,16 +26,21 @@ func DetermineNameservers(dc *models.DomainConfig) ([]*models.Nameserver, error) if err != nil { return nil, err } + // Clean up the nameservers due to + // https://github.com/StackExchange/dnscontrol/issues/491 + // In the far future, this warning will become a fatal error. + for i, _ := range nss { + if strings.HasSuffix(nss[i].Name, ".") { + models.WarnNameserverDot(dnsProvider.Name, fmt.Sprintf("DetermineNameservers (%s) (%s)", dc.Name, nss[i].Name)) + nss[i].Name = strings.TrimSuffix(nss[i].Name, ".") + } + } + take := len(nss) if n > 0 && n < take { take = n } for i := 0; i < take; i++ { - nss[i].Name = strings.TrimRight(nss[i].Name, ".") - // FIXME(tlim): Rather than correct broken providers, we should print - // a warning that the provider should be updated to store the FQDN - // with no trailing dot. See also providers/namedotcom/nameservers.go - // Bug https://github.com/StackExchange/dnscontrol/issues/491 ns = append(ns, nss[i]) } } diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go index a0330fa48..8f0122c88 100644 --- a/pkg/normalize/validate.go +++ b/pkg/normalize/validate.go @@ -288,9 +288,14 @@ func NormalizeAndValidateConfig(config *models.DNSConfig) (errs []error) { // Normalize Nameservers. for _, ns := range domain.Nameservers { - ns.Name = dnsutil.AddOrigin(ns.Name, domain.Name) - ns.Name = strings.TrimRight(ns.Name, ".") + // NB(tlim): Like any target, NAMESERVER() is input by the user + // as a shortname or a FQDN+dot. It is stored as FQDN+dot. + // Normalize it like we do any target to assure it is FQDN+dot + ns.Name = dnsutil.AddOrigin(ns.Name, domain.Name+".") + ns.Name = strings.TrimSuffix(ns.Name, ".") + checkTarget(ns.Name) } + // Normalize Records. models.PostProcessRecords(domain.Records) for _, rec := range domain.Records { diff --git a/providers/azuredns/azureDnsProvider.go b/providers/azuredns/azureDnsProvider.go index 718db1c23..0e68c271e 100644 --- a/providers/azuredns/azureDnsProvider.go +++ b/providers/azuredns/azureDnsProvider.go @@ -112,13 +112,13 @@ func (a *azureDnsProvider) GetNameservers(domain string) ([]*models.Nameserver, return nil, errNoExist{domain} } - var ns []*models.Nameserver + var nss []string if zone.ZoneProperties != nil { - for _, azureNs := range *zone.ZoneProperties.NameServers { - ns = append(ns, &models.Nameserver{Name: azureNs}) + for _, ns := range *zone.ZoneProperties.NameServers { + nss = append(nss, ns) } } - return ns, nil + return models.ToNameserversStripTD(nss) } func (a *azureDnsProvider) ListZones() ([]string, error) { diff --git a/providers/bind/bindProvider.go b/providers/bind/bindProvider.go index d7f8554c2..f1072ce04 100644 --- a/providers/bind/bindProvider.go +++ b/providers/bind/bindProvider.go @@ -62,8 +62,13 @@ func initBind(config map[string]string, providermeta json.RawMessage) (providers return nil, err } } - api.nameservers = models.StringsToNameservers(api.DefaultNS) - return api, nil + var nss []string + for _, ns := range api.DefaultNS { + nss = append(nss, ns[0:len(ns)-1]) + } + var err error + api.nameservers, err = models.ToNameservers(nss) + return api, err } func init() { diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index 39e5974c7..8d15676cc 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -91,7 +91,7 @@ func (c *CloudflareApi) GetNameservers(domain string) ([]*models.Nameserver, err if !ok { return nil, fmt.Errorf("Nameservers for %s not found in cloudflare account", domain) } - return models.StringsToNameservers(ns), nil + return models.ToNameservers(ns) } func (c *CloudflareApi) ListZones() ([]string, error) { diff --git a/providers/digitalocean/digitaloceanProvider.go b/providers/digitalocean/digitaloceanProvider.go index 1fbd196c6..f0c7deeed 100644 --- a/providers/digitalocean/digitaloceanProvider.go +++ b/providers/digitalocean/digitaloceanProvider.go @@ -93,7 +93,7 @@ func (api *DoApi) EnsureDomainExists(domain string) error { // GetNameservers returns the nameservers for domain. func (api *DoApi) GetNameservers(domain string) ([]*models.Nameserver, error) { - return models.StringsToNameservers(defaultNameServerNames), nil + return models.ToNameservers(defaultNameServerNames) } // GetZoneRecords gets the records of a zone and returns them in RecordConfig format. diff --git a/providers/dnsimple/dnsimpleProvider.go b/providers/dnsimple/dnsimpleProvider.go index d404ee210..5d423319f 100644 --- a/providers/dnsimple/dnsimpleProvider.go +++ b/providers/dnsimple/dnsimpleProvider.go @@ -55,7 +55,7 @@ type DnsimpleApi struct { // GetNameservers returns the name servers for a domain. func (c *DnsimpleApi) GetNameservers(domainName string) ([]*models.Nameserver, error) { - return models.StringsToNameservers(defaultNameServerNames), nil + return models.ToNameservers(defaultNameServerNames) } // GetZoneRecords gets the records of a zone and returns them in RecordConfig format. diff --git a/providers/gandi_v5/gandi_v5Provider.go b/providers/gandi_v5/gandi_v5Provider.go index c156ceae2..4d136c47a 100644 --- a/providers/gandi_v5/gandi_v5Provider.go +++ b/providers/gandi_v5/gandi_v5Provider.go @@ -310,7 +310,7 @@ func (client *api) GetNameservers(domain string) ([]*models.Nameserver, error) { if err != nil { return nil, err } - return models.StringsToNameservers(nameservers), nil + return models.ToNameservers(nameservers) } // GetRegistrarCorrections returns a list of corrections for this registrar. diff --git a/providers/gcloud/gcloudProvider.go b/providers/gcloud/gcloudProvider.go index 769703b75..7191e1f2e 100644 --- a/providers/gcloud/gcloudProvider.go +++ b/providers/gcloud/gcloudProvider.go @@ -123,7 +123,7 @@ func (g *gcloud) GetNameservers(domain string) ([]*models.Nameserver, error) { if err != nil { return nil, err } - return models.StringsToNameservers(zone.NameServers), nil + return models.ToNameserversStripTD(zone.NameServers) } type key struct { diff --git a/providers/namedotcom/nameservers.go b/providers/namedotcom/nameservers.go index ab485a158..4db323306 100644 --- a/providers/namedotcom/nameservers.go +++ b/providers/namedotcom/nameservers.go @@ -28,7 +28,7 @@ func (n *NameCom) GetNameservers(domain string) ([]*models.Nameserver, error) { toUse[idx] = matches[0] } } - return models.StringsToNameservers(toUse), nil + return models.ToNameservers(toUse) } func (n *NameCom) getNameserversRaw(domain string) ([]string, error) { @@ -55,9 +55,6 @@ func (n *NameCom) GetRegistrarCorrections(dc *models.DomainConfig) ([]*models.Co expected := []string{} for _, ns := range dc.Nameservers { expected = append(expected, ns.Name) - // FIXME(tlim): This should store a FQDN with no trailing ".". - // See pkg/nameservers/nameservers.go for details. - // Bug https://github.com/StackExchange/dnscontrol/issues/491 } sort.Strings(expected) expectedNameservers := strings.Join(expected, ",") diff --git a/providers/route53/route53Provider.go b/providers/route53/route53Provider.go index 47917de53..21f3b406a 100644 --- a/providers/route53/route53Provider.go +++ b/providers/route53/route53Provider.go @@ -172,13 +172,14 @@ func (r *route53Provider) GetNameservers(domain string) ([]*models.Nameserver, e if err != nil { return nil, err } - ns := []*models.Nameserver{} + + var nss []string if z.DelegationSet != nil { for _, nsPtr := range z.DelegationSet.NameServers { - ns = append(ns, &models.Nameserver{Name: *nsPtr}) + nss = append(nss, *nsPtr) } } - return ns, nil + return models.ToNameservers(nss) } // GetZoneRecords gets the records of a zone and returns them in RecordConfig format.