search

StackExchange / dnscontrol

Infrastructure as code for DNS!
https://dnscontrol.org/
MIT License
3.13k stars 397 forks source link

NS1: Revert nameserver "trailing dot" change #2317

Closed tlimoncelli closed 1 year ago

tlimoncelli commented 1 year ago

https://github.com/StackExchange/dnscontrol/pull/2295 seems to be a regression

@flz @costasd

I don't have an NS1 account so I need assistance.

tlimoncelli commented 1 year ago

CC @das7pad

@das7pad added an integration test (https://github.com/StackExchange/dnscontrol/pull/2311) which should determine if models.ToNameservers or models.ToNameserversStripTD is correct.

Here's how to use it: (Use your own domain/tokens)

export NS1_DOMAIN=example.com
export NS1_TOKEN=redacted
go test -run TestDNSProviders -v -verbose -provider GCLOUD

When I ran this on GCLOUD the result looks like...

...
...
        --- PASS: TestDNSProviders/dnscontroltest-gcloud.com/Post_cleanup:Empty#34 (0.48s)
        --- PASS: TestDNSProviders/dnscontroltest-gcloud.com/No_trailing_dot_in_nameserver (0.00s)
PASS
ok      github.com/StackExchange/dnscontrol/v3/integrationTest  89.500s

Let me know what NS1 reports.

flz commented 1 year ago

Using origin/master:

=== RUN   TestDNSProviders
=== RUN   TestDNSProviders/test-ft.com
    integration_test.go:119: Failed getting nameservers provider code already removed nameserver trailing dot (dns1.p01.nsone.net)
--- FAIL: TestDNSProviders (0.38s)
    --- FAIL: TestDNSProviders/test-ft.com (0.37s)
FAIL
exit status 1
FAIL    github.com/StackExchange/dnscontrol/v3/integrationTest  0.888s

If I git revert the offending commit, this particular test passes:

        --- PASS: TestDNSProviders/test-ft.com/No_trailing_dot_in_nameserver (0.00s)

... but ...

=== RUN   TestDNSProviders/test-ft.com/48:NS1_URLFWD_tests:Add_a_urlfwd
    integration_test.go:230: 
        + CREATE URLFWD urlfwd1.test-ft.com / http://example.com 302 2 0 ttl=300
    integration_test.go:251: Expected 0 corrections on second run, but found 2.
    integration_test.go:253: UNEXPECTED #0: + CREATE URLFWD urlfwd1.test-ft.com / http://example.com 302 2 0 ttl=300
    integration_test.go:253: UNEXPECTED #1: - DELETE NS1_URLFWD urlfwd1.test-ft.com / http://example.com 302 2 0 ttl=300

This failure can be addressed separately.

tlimoncelli commented 1 year ago

I've split out the URLFWD to a separate issue: https://github.com/StackExchange/dnscontrol/issues/2319

tlimoncelli commented 1 year ago

flz:

If I git revert the offending commit, this particular test passes:

Ok, good to know!

@costasd can you confirm?