Each domain check now looks up the addresses each nameserver's name resolves to, with the resolver's ResolveIPAddresses, and saves them sorted in DomainState.NameserverAddresses (nameserverAddresses in the state file). A nameserver that stays in the delegation and resolves to different addresses gives one NS Address Change notification naming the domain, the nameserver and the old and new addresses. An added or removed nameserver gets only the existing NS Change. The first check, and the first check after loading a state file without the field, save the addresses without notifying.
What the diff does not show:
ResolveIPAddresses asks every nameserver of the nameserver name's zone for eight record types, about a second per nameserver. Two checks of google.com (four nameservers) no longer fit in one test attempt, so the watcher tests that run domain checks use example.com (two nameservers). google.com stays for the tests that query its nameservers directly.
The resolver returns no address and no error when every server it asks times out.
Disclosures:
Judgement call: a failed or empty lookup keeps the previous addresses and sends nothing, so a nameserver whose name stops resolving shows up as an NS failure on the domain's records, not as an address change (read from the code, not tested).
Judgement call: three existing watcher tests moved from google.com to example.com.
The dashboard and the status API do not show the addresses; not in the issue.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/dnswatcher/issues/105 as planned in https://git.eeqj.de/sneak/dnswatcher/issues/105#issuecomment-107592.
Each domain check now looks up the addresses each nameserver's name resolves to, with the resolver's `ResolveIPAddresses`, and saves them sorted in `DomainState.NameserverAddresses` (`nameserverAddresses` in the state file). A nameserver that stays in the delegation and resolves to different addresses gives one `NS Address Change` notification naming the domain, the nameserver and the old and new addresses. An added or removed nameserver gets only the existing `NS Change`. The first check, and the first check after loading a state file without the field, save the addresses without notifying.
What the diff does not show:
- `ResolveIPAddresses` asks every nameserver of the nameserver name's zone for eight record types, about a second per nameserver. Two checks of `google.com` (four nameservers) no longer fit in one test attempt, so the watcher tests that run domain checks use `example.com` (two nameservers). `google.com` stays for the tests that query its nameservers directly.
- The resolver returns no address and no error when every server it asks times out.
Disclosures:
- Judgement call: a failed or empty lookup keeps the previous addresses and sends nothing, so a nameserver whose name stops resolving shows up as an NS failure on the domain's records, not as an address change (read from the code, not tested).
- Judgement call: three existing watcher tests moved from `google.com` to `example.com`.
- The dashboard and the status API do not show the addresses; not in the issue.
Model: opus-5-5
internal/watcher/nsaddress_test.go, TestNameserverWithNoAddressKeepsPrevious: it covers only a nameserver lookup that fails with an error (names under .invalid). The other half of the rule is untested: a lookup that finds no address and returns no error. That is the case the PR gives as the reason for the rule. resolveNameserverAddresses can save that empty result and every test still passes. Acceptable: the test also looks up a nameserver name that does not exist under a real zone, with addresses saved for it, and asserts that the saved addresses are kept. Live DNS answers such a name with no address and no error; TestResolveIPAddresses_NXDomainReturnsEmpty relies on the same thing.
internal/watcher/watcher_test.go, TestNSAddressChangeDetection and TestNSAddedAndRemovedIsNoAddressChange: each attempt runs two full domain checks of example.com, and each check now looks up the addresses of both nameservers. One attempt therefore uses most of the 8 seconds that internal/livednstest allows it. On a connection slower than the review host's, or with a few slow replies, every attempt runs out of time and the test fails. Acceptable: these tests finish well inside one attempt. For example, run one domain check against a saved state that already holds the live nameserver names with documentation addresses, as the plan in #105 (comment) describes.
Model: opus-5-5
Findings:
1. `internal/watcher/nsaddress_test.go`, `TestNameserverWithNoAddressKeepsPrevious`: it covers only a nameserver lookup that fails with an error (names under `.invalid`). The other half of the rule is untested: a lookup that finds no address and returns no error. That is the case the PR gives as the reason for the rule. `resolveNameserverAddresses` can save that empty result and every test still passes. Acceptable: the test also looks up a nameserver name that does not exist under a real zone, with addresses saved for it, and asserts that the saved addresses are kept. Live DNS answers such a name with no address and no error; `TestResolveIPAddresses_NXDomainReturnsEmpty` relies on the same thing.
2. `internal/watcher/watcher_test.go`, `TestNSAddressChangeDetection` and `TestNSAddedAndRemovedIsNoAddressChange`: each attempt runs two full domain checks of `example.com`, and each check now looks up the addresses of both nameservers. One attempt therefore uses most of the 8 seconds that `internal/livednstest` allows it. On a connection slower than the review host's, or with a few slow replies, every attempt runs out of time and the test fails. Acceptable: these tests finish well inside one attempt. For example, run one domain check against a saved state that already holds the live nameserver names with documentation addresses, as the plan in https://git.eeqj.de/sneak/dnswatcher/issues/105#issuecomment-107592 describes.
Model: opus-5-5
TestNameserverWithNoAddressKeepsPrevious also looks up this-surely-does-not-exist-xyz.example.com. with an address saved for it and asserts the saved address is kept.
TestNSAddressChangeDetection and TestNSAddedAndRemovedIsNoAddressChange now look up the live nameservers of example.com, save them with a documentation address, and run one domain check.
Judgement call: in TestNSAddedAndRemovedIsNoAddressChange only the removed nameserver has an address saved, as the real addresses of the ones that stay are not known before the check; a kept nameserver with unchanged addresses is covered by TestNSAddressChangeAlerts.
Model: opus-5-5
Rework, rebased onto `next`:
1. `TestNameserverWithNoAddressKeepsPrevious` also looks up `this-surely-does-not-exist-xyz.example.com.` with an address saved for it and asserts the saved address is kept.
2. `TestNSAddressChangeDetection` and `TestNSAddedAndRemovedIsNoAddressChange` now look up the live nameservers of `example.com`, save them with a documentation address, and run one domain check.
Judgement call: in `TestNSAddedAndRemovedIsNoAddressChange` only the removed nameserver has an address saved, as the real addresses of the ones that stay are not known before the check; a kept nameserver with unchanged addresses is covered by `TestNSAddressChangeAlerts`.
Model: opus-5-5
Each domain check now looks up the addresses every nameserver's name
resolves to, with the resolver's ResolveIPAddresses, and saves them
sorted in the domain's state. A nameserver that stays in the
delegation and resolves to different addresses sends one NS Address
Change notification naming the domain, the nameserver and the old and
new addresses. Added or removed nameservers get only the NS change
notification. A failed or empty lookup keeps the previous addresses,
because the resolver returns no address without an error when every
server it asks times out. State files without the field load, and the
next check fills it in silently. Watcher tests that run domain checks
use example.com, which has two nameservers, to stay within the
per-attempt limit.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #105 as planned in #105 (comment).
Each domain check now looks up the addresses each nameserver's name resolves to, with the resolver's
ResolveIPAddresses, and saves them sorted inDomainState.NameserverAddresses(nameserverAddressesin the state file). A nameserver that stays in the delegation and resolves to different addresses gives oneNS Address Changenotification naming the domain, the nameserver and the old and new addresses. An added or removed nameserver gets only the existingNS Change. The first check, and the first check after loading a state file without the field, save the addresses without notifying.What the diff does not show:
ResolveIPAddressesasks every nameserver of the nameserver name's zone for eight record types, about a second per nameserver. Two checks ofgoogle.com(four nameservers) no longer fit in one test attempt, so the watcher tests that run domain checks useexample.com(two nameservers).google.comstays for the tests that query its nameservers directly.Disclosures:
google.comtoexample.com.Model: opus-5-5
Findings:
internal/watcher/nsaddress_test.go,TestNameserverWithNoAddressKeepsPrevious: it covers only a nameserver lookup that fails with an error (names under.invalid). The other half of the rule is untested: a lookup that finds no address and returns no error. That is the case the PR gives as the reason for the rule.resolveNameserverAddressescan save that empty result and every test still passes. Acceptable: the test also looks up a nameserver name that does not exist under a real zone, with addresses saved for it, and asserts that the saved addresses are kept. Live DNS answers such a name with no address and no error;TestResolveIPAddresses_NXDomainReturnsEmptyrelies on the same thing.internal/watcher/watcher_test.go,TestNSAddressChangeDetectionandTestNSAddedAndRemovedIsNoAddressChange: each attempt runs two full domain checks ofexample.com, and each check now looks up the addresses of both nameservers. One attempt therefore uses most of the 8 seconds thatinternal/livednstestallows it. On a connection slower than the review host's, or with a few slow replies, every attempt runs out of time and the test fails. Acceptable: these tests finish well inside one attempt. For example, run one domain check against a saved state that already holds the live nameserver names with documentation addresses, as the plan in #105 (comment) describes.Model: opus-5-5
50ff42ed26toac4a17352aRework, rebased onto
next:TestNameserverWithNoAddressKeepsPreviousalso looks upthis-surely-does-not-exist-xyz.example.com.with an address saved for it and asserts the saved address is kept.TestNSAddressChangeDetectionandTestNSAddedAndRemovedIsNoAddressChangenow look up the live nameservers ofexample.com, save them with a documentation address, and run one domain check.Judgement call: in
TestNSAddedAndRemovedIsNoAddressChangeonly the removed nameserver has an address saved, as the real addresses of the ones that stay are not known before the check; a kept nameserver with unchanged addresses is covered byTestNSAddressChangeAlerts.Model: opus-5-5
Review passed on
ac4a173.Model: opus-5-5
ac4a17352ato7f8abb0028