MethodsV2: add Get-Parent-NS-Names-and-IPs method by marc-vanderwal · Pull Request #1418 · zonemaster/zonemaster

marc-vanderwal · 2025-08-11T14:03:29Z

Purpose

This PR introduces a new method in MethodsV2, named Get-Parent-NS-Names-and-IPs method, which obtains the set of name server names and IP addresses of a zone’s parent zone.

Some DNSSEC test cases need to look at information in the parent zone. There might be problems there, and we need to report the name servers that the information was obtained from. For good ergonomics, it’d be nice to report the name servers as name server/IP pairs, for example “ns1.parent.example/198.54.113.1”, like we do for name servers that appear in the parent’s delegation or in the zone apex’s NS record set.

However, no such method currently exists. A method called Get-Parent-NS-IP does exist, and I could reuse its outline for the Get-Parent-NS-Names-and-IPs method. Then, Get-Parent-NS-IP can be implemented as a method that uses the result of Get-Parent-NS-Names-and-IPs, exactly like Get-Del-NS-IPs and Get-Zone-NS-IPs.

I also took the opportunity to rename Get-Parent-NS-IP to Get-Parent-NS-IPs (in plural form) in order to make the method’s name consistent with other similarly-named methods in MethodsV2.

Test scenario specifications are also updated to test Get-Parent-NS-Names-and-IPs instead of Get-Parent-NS-IP.

Context

Discussion on pull request #1412.

Changes

Add new MethodsV2 method Get-Parent-NS-Names-and-IPs;
Redefine Get-Parent-NS-IP so that it uses the result of Get-Parent-NS-Names-and-IPs;
Rename Get-Parent-NS-IP to Get-Parent-NS-IPs and update test case specifications that use this method;
In MethodsV2 test scenario specification, test Get-Parent-NS-Names-and-IPs instead of Get-Parent-NS-IPs and update expected data for MethodsV2 test scenarios;
As a bonus, fix some minor typos and errors I’ve encountered while preparing this PR.

How to test this PR

Review. This PR is best reviewed commit by commit.

matsduf · 2025-08-11T18:44:53Z

* In MethodsV2 test scenario specification, test _Get-Parent-NS-Names-and-IPs_ instead of _Get-Parent-NS-IPs_ and update expected data for MethodsV2 test scenarios;

More than that is needed. Since the names of the parent name servers are included we must have additional scenarios that verifies that it returns the correct names in inconsistent contexts. Also context where the same IP address is used by more than one name.

marc-vanderwal · 2025-08-12T08:35:16Z

Okay, so far, I made the following changes:

Undid a local change that was accidentally committed
Made clear that when following CNAME chains, the original NS name should be kept
Reverted the Handled Servers set to a set of (IP, domain name) pairs as it was before, to make sure we never query the same IP twice with the same domain name.

This spec needs some more work because the edge case where the parent has multiple NS records pointing to the same IP isn’t handled properly.

marc-vanderwal · 2025-08-12T14:10:25Z

As for extra test scenarios in MethodsV2, what are we potentially interested in? I’m thinking of:

grandparent and parent zones list two name servers for the parent zone which resolve to the same IPs;
inconsistencies between the delegation in grandparent zone and the in-zone NS record set in parent zone, and more specifically:
- no names and no IPs in common between grandparent and parent,
- no names but at least one IP in common between grandparent and parent,
- grandparent delegation’s NS records are a subset of parent zone’s NS records,
- grandparent delegation’s NS records are a superset of parent zone’s NS records,
- grandparent delegation’s NS records are a different set from parent zone’s NS records.

Is there anything else I might’ve missed?

matsduf · 2025-08-12T18:44:02Z

As for extra test scenarios in MethodsV2, what are we potentially interested in? I’m thinking of:
* [x]  grandparent and parent zones list two name servers for the parent zone which resolve to the same IPs;

Yes.

* [x]  inconsistencies between the delegation in grandparent zone and the in-zone NS record set in parent zone, and more specifically:

Yes.

  * [ ]  no names and no IPs in common between grandparent and parent,
  * [ ]  no names but at least one IP in common between grandparent and parent,
  * [ ]  grandparent delegation’s NS records are a subset of parent zone’s NS records,
  * [ ]  grandparent delegation’s NS records are a superset of parent zone’s NS records,
  * [ ]  grandparent delegation’s NS records are a different set from parent zone’s NS records.

I do not think that is needed. The purpose of the method is to collect as many parent servers as possible to get as many child servers (names and IPs) as possible.

Grandparent servers uses different NS names of the same parent name server (IP).
Parent servers have different NS names for the same name server.

matsduf · 2025-08-12T18:58:20Z

+       1. For each IP address add the name server name, IP address and *Zone
+          Name* tuple to the *Remaining Servers* set unless the IP address is
+          already listed in *Handled Servers* together with *Zone Name*.


This will result in additional names for the same IP will get lost, won't it?

Yes, I saw the same problem as well.

The solution is to always add name, IP and zone name tuples to Remaining Servers without checking Handled Servers. Then, right after taking such a tuple out of the set at the beginning of the loop, check Handled Servers if the IP and zone name was already processed; if so, then if there is already an entry with the same IP in Parent Name Servers, we add it to the Parent Name Servers.

This lets us avoid sending duplicate requests for the same (IP, QNAME) tuple while capturing all the names bound to the IP address.

I’ve just pushed a new version of the spec that does this.

marc-vanderwal · 2025-08-13T09:25:50Z

We aren’t quite there yet, because I realize there are two problems in my latest work:

Adding items to Remaining Servers unconditionally means that we’re adding the same name servers over and over again only to skip over them because the addresses were handled already. There’s got to be a better way.
I don’t think the expected output for the new test scenario PARENT-NS-SAME-IP-2 is correct. Because we merge the delegations from the grandparent and the NS records from the parent zone, we should expect that the parent name servers contains ns1, ns1a, ns1b (all with same IP) and ns2. Currently I only list ns1a, ns1b and ns2.

marc-vanderwal · 2025-08-13T12:36:09Z

We aren’t quite there yet, because I realize there are two problems in my latest work:

Adding items to Remaining Servers unconditionally means that we’re adding the same name servers over and over again only to skip over them because the addresses were handled already. There’s got to be a better way.

I don’t think the expected output for the new test scenario PARENT-NS-SAME-IP-2 is correct. Because we merge the delegations from the grandparent and the NS records from the parent zone, we should expect that the parent name servers contains ns1, ns1a, ns1b (all with same IP) and ns2. Currently I only list ns1a, ns1b and ns2.

Fixed.

Introduce a new method that obtains the set of name server names and IP addresses of the parent zone. Some DNSSEC test cases need to look at information in the parent zone. There might be problems there, and we need to report the name servers that the information was obtained from. We’d like to report the name servers as name server/IP pairs, for example “ns1.parent.example/198.54.113.1”, like other test cases usually do. However, it turns out that there is no method that returns the parent’s name server name/IP pairs. Fortunately, there already exists one that returns only the IP addresses, named Get-Parent-NS-IP. The outline of that latter method is reused to define the new method that this commit introduces, Get-Parent-NS-Names-and-IPs, then Get-Parent-NS-IP is rewritten in order to take advantage of Get-Parent-NS-Names-and-IPs. This approach is identical to the way Get-Del-NS-IPs and Get-Zone-NS-IPs are specified: they use the result of a corresponding Get-{Del,Zone}-NS-Names-and-IPs method and extract a set of IP addresses from it. It does add one more layer of method interdependency, but the extra cost is negligible: the only penalty is that Get-Parent-NS-IP now iterates over the result of Get-Parent-NS-Names-and-IPs instead of computing its result directly.

This test case refers to an algorithm to determine the parent zone. It used to be outlined in the *Get-Parent-NS-IP* method, but a previous commit has moved it to another method, *Get-Parent-NS-Names-and-IPs*. This commit updates the link so that it points to the place where the outline now lives.

Use the plural form, in order to bring the method name in line with all other methods that all follow a Get-…-IPs naming scheme.

After introducing Get-Parent-NS-Names-and-IPs, update the test scenario specification in order to test the method appropriately. Because Get-Parent-NS-IPs is now defined in terms of Get-Parent-NS-Names-and-IPs, only the latter needs to be tested. But the existing scenarios only tested the former. This commit modifies the test zone specification, and updates the expected output for MethodsV2 to list parent NS names and IP addresses instead of only parent NS IP addresses.

There was a little mixup in the section heading levels in the test scenarios: the first scenarios used level 2 heading, then moved on to level 3 headings. This is fixed by using the test scenario specification Address03 as a template and moving all scenarios into level 3 headings. The table of contents at the beginning also needed an update. As a bonus change, clean up whitespace in the .md file, using Emacs’s M-x whitespace-cleanup command.

Fix some minor typos I’ve stumbled upon while proofreading changes in documents I touched in previous commits.

Some steps were incorrectly numbered in the .md files. While likely not to be a problem in practice because of the HTML rendering forcing the correct numbers, Emacs’s M-x markdown-cleanup-list-numbers command cleaned up the list numbering in many more places than I thought.

tgreenx

LGTM. Small indentation nits

…erging of zonemaster#1418

tgreenx · 2025-10-28T09:54:01Z

This has yet to be implemented in Engine.

matsduf · 2025-10-28T12:37:07Z

This has yet to be implemented in Engine.

Correct. Maybe we were too fast merge.

marc-vanderwal · 2025-10-29T11:09:30Z

This has yet to be implemented in Engine.

Correct. Maybe we were too fast merge.

No, actually it’s fine. When that specification was stabilized, I went ahead and did the implementation as well in order to save time later. I have just rebased my work, did some finishing touches and created zonemaster/zonemaster-engine#1475.

marc-vanderwal requested review from MichaelTimbert, matsduf, mattias-p and tgreenx August 11, 2025 14:03

marc-vanderwal mentioned this pull request Aug 11, 2025

Updates the test case specification for DNSSEC01 #1412

Merged

matsduf reviewed Aug 11, 2025

View reviewed changes

Comment thread test-zone-data/start-coredns.sh Outdated

matsduf requested changes Aug 11, 2025

View reviewed changes

Comment thread docs/public/specifications/tests/MethodsV2.md

Comment thread docs/public/specifications/tests/MethodsV2.md Outdated

Comment thread docs/public/specifications/tests/MethodsV2.md

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch 2 times, most recently from 0c69ad5 to 1b6e7d8 Compare August 12, 2025 06:33

marc-vanderwal marked this pull request as draft August 12, 2025 13:54

matsduf reviewed Aug 12, 2025

View reviewed changes

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch from 1b6e7d8 to bcc44eb Compare August 13, 2025 07:58

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch from bcc44eb to 8757726 Compare August 13, 2025 09:51

marc-vanderwal marked this pull request as ready for review August 13, 2025 12:36

matsduf added A-TestCase Area: Test case specification or implementation of test case RC-Features Release category: Features. labels Aug 19, 2025

matsduf requested changes Aug 19, 2025

View reviewed changes

marc-vanderwal added 7 commits September 1, 2025 16:12

MethodsV2: rename Get-Parent-NS-{IP => IPs}

2ed8949

Use the plural form, in order to bring the method name in line with all other methods that all follow a Get-…-IPs naming scheme.

Fix minor typos

1a93fe0

Fix some minor typos I’ve stumbled upon while proofreading changes in documents I touched in previous commits.

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch from 8757726 to e3527a3 Compare September 1, 2025 14:15

marc-vanderwal requested a review from matsduf September 3, 2025 05:54

tgreenx previously approved these changes Oct 7, 2025

View reviewed changes

marc-vanderwal dismissed tgreenx’s stale review via 54df2f9 October 15, 2025 15:26

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch from e3527a3 to 54df2f9 Compare October 15, 2025 15:26

matsduf requested changes Oct 15, 2025

View reviewed changes

Comment thread docs/public/specifications/test-zones/MethodsV2/methodsv2.md Outdated

Comment thread docs/public/specifications/test-zones/MethodsV2/methodsv2.md Outdated

marc-vanderwal added 2 commits October 23, 2025 12:43

MethodsV2: add scenarios PARENT-NS-SAME-IP-{1..2}

d8640bc

Fix consistency in MethodsV2 test scenario spec

86adb23

marc-vanderwal force-pushed the feature/methodsv2-parent-ns-names-and-ips branch from 54df2f9 to 86adb23 Compare October 23, 2025 10:43

matsduf approved these changes Oct 23, 2025

View reviewed changes

matsduf requested review from tgreenx and tolvmannen October 23, 2025 13:55

matsduf added this to the v2025.2 milestone Oct 23, 2025

marc-vanderwal merged commit cce0707 into zonemaster:develop Oct 27, 2025

matsduf added a commit to matsduf/zonemaster that referenced this pull request Oct 27, 2025

Updates to have both name and IP address for name servers after the m…

e063bdd

…erging of zonemaster#1418

marc-vanderwal mentioned this pull request Oct 29, 2025

Implement new MethodV2: Get-Parent-NS-Names-and-IPs zonemaster/zonemaster-engine#1475

Merged

marc-vanderwal deleted the feature/methodsv2-parent-ns-names-and-ips branch October 29, 2025 14:22

Uh oh!

Conversation

marc-vanderwal commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Context

Changes

How to test this PR

Uh oh!

Uh oh!

matsduf commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marc-vanderwal commented Aug 12, 2025

Uh oh!

marc-vanderwal commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matsduf commented Aug 12, 2025

Uh oh!

matsduf Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

marc-vanderwal Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

marc-vanderwal commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc-vanderwal commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgreenx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgreenx commented Oct 28, 2025

Uh oh!

matsduf commented Oct 28, 2025

Uh oh!

marc-vanderwal commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marc-vanderwal commented Aug 11, 2025 •

edited

Loading

matsduf commented Aug 11, 2025 •

edited

Loading

marc-vanderwal commented Aug 12, 2025 •

edited

Loading

marc-vanderwal commented Aug 13, 2025 •

edited

Loading

marc-vanderwal commented Aug 13, 2025 •

edited

Loading