Test zones for the CNAME function in Recursor.pm in Engine by matsduf · Pull Request #1220 · zonemaster/zonemaster

matsduf · 2023-11-15T18:44:05Z

Purpose

Test zone for unit tests for zonemaster/zonemaster-engine#1288

How to test this PR

Create unit tests based on these test zones when the PR in engine has been completed.

marc-vanderwal

Looks fine. I just saw a few typos and I have one comment about a problem case.

Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>

… cases

matsduf

@marc-vanderwal, also see the dig output in test-zone-data/Engine/Recursor-PM/README.md in this PR.

marc-vanderwal · 2023-11-23T06:59:43Z

@@ -20,6 +20,446 @@ See [CNAME.md] for specification of the scenarios.

 ## zonemaster-cli commands and their output for each test scenario


But these are outputs from dig, not zonemaster-cli. And I suppose that they were obtained with CoreDNS.

The outputs are with dig against the CoreDNS configuration. They are there to show what Zonemaster would get if the unit tests query for those names with query type A.

I am waiting for zonemaster/zonemaster-engine#1288 to be done.

This is different since there is no test case to run, is there? I do not see how I could run zonemaster-cli for these scenarios.

zonemaster/zonemaster-engine#1288 has been updated to include the changes discussed in the work group. Note that two unit tests are failing due to an issue with CoreDNS, see my other comments.
You should now be able to test this PR properly.

matsduf · 2023-11-24T17:42:06Z

@marc-vanderwal and @tgreenx: Please re-review.

matsduf · 2023-11-28T09:22:35Z

I want to hear from @tgreenx before merging. It should match zonemaster/zonemaster-engine#1288.

tgreenx · 2023-11-28T10:57:21Z

+;; ANSWER SECTION:
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.
+looped-cname-in-zone-1.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-1.cname.recursor.engine.xa.


Why are there so many of the same CNAME record here? This is also what I get when running locally. Is it a bug in CoreDNS? There should be just one for this test scenario; otherwise Engine will output CNAME_MULTIPLE_FOR_NAME instead of CNAME_LOOP_INNER.

I guess it is a bug in CoreDNS, but on the other hand, the CNAME records are identical and I feel they should be considered to be one. I will check.

I have reported the bug to CoreDNS (coredns/coredns#6421). At least I think it is a bug. I suspect that CoreDNS "thinks" that it should follow CNAME from the response. Some sort of internal resolver.

I failed to find a work-around.

tgreenx · 2023-11-28T10:59:42Z

+;; ANSWER SECTION:
+looped-cname-in-zone-2.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa.
+looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa.
+looped-cname-in-zone-2-b.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa.
+looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa.
+looped-cname-in-zone-2-b.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa.
+looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa.
+looped-cname-in-zone-2-b.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa.
+looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa.
+looped-cname-in-zone-2-b.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa.
+looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600	IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa.


Same comment here. CNAME records are unexpectedly repeated.

Same answer as above.

I have also reported this, but here I found a work-around wht with dig gives the following result:

; <<>> DiG 9.18.18-0ubuntu0.22.04.1-Ubuntu <<>> @127.30.1.31 looped-cname-in-zone-2.cname.recursor.engine.xa ; (1 server found) ;; global options: +cmd ;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 13110 ;; flags: qr aa rd; QUERY: 1, ANSWER: 3, AUTHORITY: 1, ADDITIONAL: 1 ;; WARNING: recursion requested but not available ;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags:; udp: 1232 ; COOKIE: 7a7a75c08fe99560 (echoed) ;; QUESTION SECTION: ;looped-cname-in-zone-2.cname.recursor.engine.xa. IN A ;; ANSWER SECTION: looped-cname-in-zone-2.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa. looped-cname-in-zone-2-a.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-2-b.cname.recursor.engine.xa. looped-cname-in-zone-2-b.cname.recursor.engine.xa. 3600 IN CNAME looped-cname-in-zone-2-a.cname.recursor.engine.xa. ;; AUTHORITY SECTION: cname.recursor.engine.xa. 3600 IN NS ns1.cname.recursor.engine.xa. ;; Query time: 0 msec ;; SERVER: 127.30.1.31#53(127.30.1.31) (UDP) ;; WHEN: Wed Nov 29 13:07:05 UTC 2023 ;; MSG SIZE rcvd: 488

I will update.

tgreenx · 2023-11-28T11:23:58Z

@@ -20,6 +20,446 @@ See [CNAME.md] for specification of the scenarios.

 ## zonemaster-cli commands and their output for each test scenario


zonemaster/zonemaster-engine#1288 has been updated to include the changes discussed in the work group. Note that two unit tests are failing due to an issue with CoreDNS, see my other comments.
You should now be able to test this PR properly.

matsduf · 2023-11-29T13:44:16Z

@marc-vanderwal and @tgreenx: Please re-review.

…AME and EXTRA-CNAME-IN-ANSWER

matsduf · 2023-12-01T12:18:38Z

@marc-vanderwal and @tgreenx: Please re-review.

tgreenx

Commit zonemaster/zonemaster-engine@32244e9 adds a message tag CNAME_START whenever the CNAME lookup functionality is used. This could be added to the list of expected message tags for each scenario.

matsduf · 2023-12-07T17:09:31Z

Commit zonemaster/zonemaster-engine@32244e9 adds a message tag CNAME_START whenever the CNAME lookup functionality is used. This could be added to the list of expected message tags for each scenario.

What is the expected output where I have put "??"? Could you check the table?

Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>

From review comment. Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>

Adds test scenarios for the CNAME function in Recursor.pm in Engine

a6f9a44

matsduf added this to the v2023.2 milestone Nov 15, 2023

matsduf requested a review from tgreenx November 15, 2023 18:44

matsduf mentioned this pull request Nov 15, 2023

Add CNAME followage in Recursor zonemaster/zonemaster-engine#1288

Merged

Merge branch 'develop' into cname-resolver-test-zones

cb33b87

marc-vanderwal reviewed Nov 22, 2023

View reviewed changes

matsduf and others added 5 commits November 22, 2023 22:02

Apply suggestions from document review

4d45188

Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>

Corrects miswriting

0c08dae

Makes specification more precise in two scenarios

ab6af8b

Updates zone specification to make it correct for NXDOMAIN and NODATA…

551f926

… cases

Adds logging from dig output for the scenarios

5355793

matsduf commented Nov 22, 2023

View reviewed changes

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md Outdated

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md

marc-vanderwal reviewed Nov 23, 2023

View reviewed changes

matsduf added 2 commits November 24, 2023 18:39

Updates from review comments

b90cde2

Merge branch 'develop' into cname-resolver-test-zones

5c3d6fa

matsduf requested a review from marc-vanderwal November 24, 2023 17:41

marc-vanderwal previously approved these changes Nov 28, 2023

View reviewed changes

tgreenx reviewed Nov 28, 2023

View reviewed changes

Adds work-around to give more sane DNS response

01eb8c3

matsduf dismissed marc-vanderwal’s stale review via 01eb8c3 November 29, 2023 13:43

matsduf added 2 commits November 30, 2023 16:46

Adds three additional scenarios

42e2d36

Adds three new scenarios, LOOPED-CNAME-IN-ZONE-3, WRONG-CNAME-OWNER-N…

82b3aec

…AME and EXTRA-CNAME-IN-ANSWER

matsduf requested review from marc-vanderwal and tgreenx December 1, 2023 12:18

Merge branch 'develop' into cname-resolver-test-zones

609c23d

tgreenx reviewed Dec 7, 2023

View reviewed changes

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md Outdated

matsduf modified the milestones: v2023.2, v2024.1 Dec 7, 2023

Fixes typo

cba3a9a

tgreenx reviewed May 14, 2024

View reviewed changes

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md Outdated

Apply suggestions from document review

28b83b6

Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>

marc-vanderwal reviewed May 22, 2024

View reviewed changes

Comment thread docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md Outdated

Update docs/public/specifications/test-zones/Engine/Recursor-PM/CNAME.md

29fe434

From review comment. Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>

marc-vanderwal approved these changes May 22, 2024

View reviewed changes

matsduf requested a review from tgreenx May 22, 2024 11:53

tgreenx approved these changes Jun 10, 2024

View reviewed changes

tgreenx modified the milestones: v2024.1, v2024.2 Jun 12, 2024

matsduf merged commit 946f837 into zonemaster:develop Jul 4, 2024

matsduf deleted the cname-resolver-test-zones branch July 4, 2024 14:55

		@@ -20,6 +20,446 @@ See [CNAME.md] for specification of the scenarios.

		## zonemaster-cli commands and their output for each test scenario

Uh oh!

Conversation

matsduf commented Nov 15, 2023

Purpose

How to test this PR

Uh oh!

marc-vanderwal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matsduf left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matsduf Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matsduf commented Nov 24, 2023

Uh oh!

matsduf commented Nov 28, 2023

Uh oh!

tgreenx Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matsduf commented Nov 29, 2023

Uh oh!

matsduf commented Dec 1, 2023

Uh oh!

tgreenx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

matsduf commented Dec 7, 2023

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matsduf left a comment •

edited

Loading

matsduf Nov 24, 2023 •

edited

Loading

tgreenx Nov 28, 2023 •

edited

Loading