Profile fixes #1356
Merged
Profile fixes #1356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattias-p
added
the
V-Minor
mattias-p
requested review from
matsduf,
tgreenx,
marc-vanderwal and
MichaelTimbert
matsduf
reviewed
Jun 4, 2024
lib/Zonemaster/Engine/Profile.pm
Outdated
Comment on lines
89
to
90
| if ( $_[0] ne $RESOLVER_SOURCE_OS_DEFAULT and not Net::IP::XS->new( $_[0] ) ) { | ||
| die "Property resolver.source must be an IP address or the exact string $RESOLVER_SOURCE_OS_DEFAULT"; |
There was a problem hiding this comment.
As far as I can see Net::IP::XS->new( "10.0.1/24" ) is true.
True. I'd prefer if we could merge these two PRs in any order though, and whoever comes in second place gets to rebase. Is that okay with you? |
The profile module specifies a contract that each profile property must respect. This change makes all properties respect the contract with regard to definedness, default values and hierarchy. To ease upholding certain aspects of the contract the new utility method all_properties() was added. Validation error messages for resolver.source4 and resolver.source6 are clarified regarding the empty string.
mattias-p
force-pushed
the
1355-profile-contract
branch
from
ff0984d
to
5f5edfb
Compare
marc-vanderwal
approved these changes
Jun 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
The initial aim of this PR was to add some missing properties to the default profile. But in the process of making the update I discovered more problems with the profile and I'm including fixes for all of them here.
Context
Fixes #1355.
Conflicts but is not blocked by #1257.
I marked this minor because of the new Zonemaster::Engine::Profile->all_properties method. Otherwise this would be V-Patch.
Changes
cacheasnrootscacheresolver.source4resolver.source6How to test this PR
Run the unit tests.