Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Resolve IP addresses directly from Engine for out-of-bailiwick names in fake delegations #1389

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Resolve IP addresses directly from Engine for out-of-bailiwick names in fake delegations #1389

merged 1 commit into from
Oct 15, 2024

Conversation

tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Oct 8, 2024

Purpose

This PR proposes to resolve IP addresses directly from Engine for out-of-bailiwick names in fake delegations. In particular this will allow the use of private (custom) root for this use case.

Context

Fixes #1344

Changes

  • Use Zonemaster::Engine::Recursor->get_addresses_for() instead of Zonemaster::LDNS->name2addr()

How to test this PR

From #1344:

zonemaster-cli --raw  --show-testcase --test basic01 --hints zonemaster/test-zone-data/COMMON/hintfile  --ns ns3-undelegated-child.basic01.xa --ns nsa.dnsnode.net child.parent.good-undel-1.basic01.xa
   0.16 ERROR    Unspecified    FAKE_DELEGATION_NO_IP  domain=child.parent.good-undel-1.basic01.xa; nsname=nsa.dnsnode.net

Names in private root are now properly resolved.

@tgreenx
Resolve IP addresses directly from Engine for out-of-bailiwick names …
6f6503d 
…in fake delegations

In particular this will allow the use of private (custom) root for this use case.
@tgreenx tgreenx added this to the v2024.2 milestone Oct 8, 2024
@tgreenx tgreenx linked an issue Oct 8, 2024 that may be closed by this pull request
add_fake_delegation() does not respect changed root #1344
Closed
matsduf
matsduf approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine when testing.

@matsduf
Copy link
Contributor

matsduf commented Oct 11, 2024 •
edited
Loading

@mattias-p , @marc-vanderwal, @MichaelTimbert - please review.

marc-vanderwal
marc-vanderwal approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@marc-vanderwal marc-vanderwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@tgreenx tgreenx merged commit 45ae3df into zonemaster:develop Oct 15, 2024
3 checks passed
@tgreenx tgreenx deleted the fix-fake-lookups branch October 15, 2024 12:50
@tgreenx tgreenx mentioned this pull request Oct 15, 2024
Closed
@matsduf matsduf added the V-Patch Versioning: The change gives an update of patch in version. label Nov 13, 2024
@matsduf
Copy link
Contributor

matsduf commented Dec 3, 2024

Reviwed for release testing for v2024.2: no issues.

@matsduf matsduf added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc-vanderwal marc-vanderwal marc-vanderwal approved these changes

@matsduf matsduf matsduf approved these changes

@mattias-p mattias-p Awaiting requested review from mattias-p

@tolvmannen tolvmannen Awaiting requested review from tolvmannen

@MichaelTimbert MichaelTimbert Awaiting requested review from MichaelTimbert

Assignees
No one assigned
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Milestone
v2024.2
Development

Successfully merging this pull request may close these issues.

add_fake_delegation() does not respect changed root
3 participants
@tgreenx @matsduf @marc-vanderwal