Skip to content

Commit

Permalink
Make profile properties respect their contract
Browse files Browse the repository at this point in the history 
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.

The documentation of resolver.source4 and resolver.source6 is fixed to
not state that undef is a valid value. This brings the property
documentation in line with both implementation and the contractual
requirements for properties.
  • Loading branch information
@mattias-p
mattias-p committed Jun 3, 2024
1 parent e688b27 commit ff0984d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 78 deletions.
8 changes: 3 additions & 5 deletions lib/Zonemaster/Engine/Nameserver/Cache.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@ sub get_cache_type {
my ( $class, $profile ) = @_;
my $cache_type = 'LocalCache';

if ( $profile->get( 'cache' ) ) {
my %cache_config = %{ $profile->get( 'cache' ) };
my %cache_config = %{ $profile->get( 'cache' ) };

if ( exists $cache_config{'redis'} ) {
$cache_type = 'RedisCache';
}
if ( exists $cache_config{'redis'} ) {
$cache_type = 'RedisCache';
}

return $cache_type;
Expand Down
39 changes: 26 additions & 13 deletions lib/Zonemaster/Engine/Profile.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use Zonemaster::Engine::Constants qw( $RESOLVER_SOURCE_OS_DEFAULT $DURATION_5_MI
my %profile_properties_details = (
q{cache} => {
type => q{HashRef},
default => {},
test => sub {
my @allowed_keys = ( 'redis' );
foreach my $cache_database ( keys %{$_[0]} ) {
Expand Down Expand Up @@ -83,6 +84,7 @@ my %profile_properties_details = (
},
q{resolver.source} => {
type => q{Str},
default => 'os_default',
test => sub {
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";
Expand All @@ -91,17 +93,19 @@ my %profile_properties_details = (
},
q{resolver.source4} => {
type => q{Str},
default => '',
test => sub {
if ( $_[0] and $_[0] ne '' and not Net::IP::XS::ip_is_ipv4( $_[0] ) ) {
die "Property resolver.source4 must be an IPv4 address, the empty string or undefined";
if ( $_[0] ne '' and not Net::IP::XS::ip_is_ipv4( $_[0] ) ) {
die "Property resolver.source4 must be an IPv4 address, the empty string";
}
}
},
q{resolver.source6} => {
type => q{Str},
default => '',
test => sub {
if ( $_[0] and $_[0] ne '' and not Net::IP::XS::ip_is_ipv6( $_[0] ) ) {
die "Property resolver.source6 must be an IPv6 address, the empty string or undefined";
if ( $_[0] ne '' and not Net::IP::XS::ip_is_ipv6( $_[0] ) ) {
die "Property resolver.source6 must be an IPv6 address, the empty string";
}
}
},
Expand All @@ -116,6 +120,7 @@ my %profile_properties_details = (
},
q{asnroots} => {
type => q{ArrayRef},
default => ["asnlookup.zonemaster.net"],
test => sub {
foreach my $ndd ( @{$_[0]} ) {
die "Property asnroots has a NULL item" if not defined $ndd;
Expand Down Expand Up @@ -306,6 +311,11 @@ sub default {
return $new;
}

sub all_properties {
my ( $class ) = @_;
return sort keys %profile_properties_details;
}

sub check_validity {
my ( $self ) = @_;
my $resolver = $self->{profile}{resolver};
Expand Down Expand Up @@ -634,6 +644,12 @@ Serialize the profile to the L</JSON REPRESENTATION> format.
Returns a string.
=head2 all_properties
Get the names of all properties.
Returns a sorted list of strings.
=head1 SUBROUTINES
=head2 _get_profile_paths
Expand Down Expand Up @@ -718,16 +734,16 @@ Default C<"os_default">.
=head2 resolver.source4
A string that is an IPv4 address or the empty string or undefined.
A string that is an IPv4 address or the empty string.
The source address all resolver objects should use when sending queries over IPv4.
If the empty string or undefined, use the OS default IPv4 address if available.
If the empty string, use the OS default IPv4 address if available.
Default "" (empty string).
=head2 resolver.source6
A string that is an IPv6 address or the empty string or undefined.
A string that is an IPv6 address or the empty string.
The source address all resolver objects should use when sending queries over IPv6.
If the empty string or undefined, use the OS default IPv6 address if available.
If the empty string, use the OS default IPv6 address if available.
Default "" (empty string).
=head2 net.ipv4
Expand Down Expand Up @@ -770,12 +786,9 @@ Default C<"asnlookup.zonemaster.net">.
=head2 cache (EXPERIMENTAL)
A hash of hashes. The currently supported keys are C<"redis">.
Default C<{}>.
See more information in L<cache.redis>.
Undefined by default.
=head2 cache.redis (EXPERIMENTAL)
=head3 redis
A hashref. The currently supported keys are C<"server"> and C<"expire">.
Expand Down
72 changes: 12 additions & 60 deletions t/profiles.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,9 @@ subtest 'new() returns a new profile every time' => sub {
subtest 'new() returns a profile with all properties unset' => sub {
my $profile = Zonemaster::Engine::Profile->new;

is $profile->get( 'resolver.defaults.usevc' ), undef, 'resolver.defaults.usevc is unset';
is $profile->get( 'resolver.defaults.retrans' ), undef, 'resolver.defaults.retrans is unset';
is $profile->get( 'resolver.defaults.recurse' ), undef, 'resolver.defaults.recurse is unset';
is $profile->get( 'resolver.defaults.retry' ), undef, 'resolver.defaults.retry is unset';
is $profile->get( 'resolver.defaults.igntc' ), undef, 'resolver.defaults.igntc is unset';
is $profile->get( 'resolver.defaults.fallback' ), undef, 'resolver.defaults.fallback is unset';
is $profile->get( 'net.ipv4' ), undef, 'net.ipv4 is unset';
is $profile->get( 'net.ipv6' ), undef, 'net.ipv6 is unset';
is $profile->get( 'no_network' ), undef, 'no_network is unset';
is $profile->get( 'asnroots' ), undef, 'asnroots is unset';
is $profile->get( 'logfilter' ), undef, 'logfilter is unset';
is $profile->get( 'test_levels' ), undef, 'test_levels is unset';
is $profile->get( 'test_cases' ), undef, 'test_cases is unset';
is $profile->get( 'cache' ), undef, 'cache is unset';
for my $property ( Zonemaster::Engine::Profile->all_properties ) {
is $profile->get( $property ), undef, "$property is unset";
}
};

subtest 'default() returns a new profile every time' => sub {
Expand All @@ -206,18 +195,9 @@ subtest 'default() returns a new profile every time' => sub {
subtest 'default() returns a profile with all properties set' => sub {
my $profile = Zonemaster::Engine::Profile->default;

ok defined( $profile->get( 'resolver.defaults.usevc' ) ), 'resolver.defaults.usevc is set';
ok defined( $profile->get( 'resolver.defaults.recurse' ) ), 'resolver.defaults.recurse is set';
ok defined( $profile->get( 'resolver.defaults.igntc' ) ), 'resolver.defaults.igntc is set';
ok defined( $profile->get( 'resolver.defaults.fallback' ) ), 'resolver.defaults.fallback is set';
ok defined( $profile->get( 'net.ipv4' ) ), 'net.ipv4 is set';
ok defined( $profile->get( 'net.ipv6' ) ), 'net.ipv6 is set';
ok defined( $profile->get( 'no_network' ) ), 'no_network is set';
ok defined( $profile->get( 'resolver.defaults.retry' ) ), 'resolver.defaults.retry is set';
ok defined( $profile->get( 'resolver.defaults.retrans' ) ), 'resolver.defaults.retrans is set';
ok defined( $profile->get( 'logfilter' ) ), 'logfilter is set';
ok defined( $profile->get( 'test_levels' ) ), 'test_levels is set';
ok defined( $profile->get( 'test_cases' ) ), 'test_cases is set';
for my $property ( Zonemaster::Engine::Profile->all_properties ) {
ok defined( $profile->get( $property ) ), "$property is set";
}
};

subtest 'from_json() returns a new profile every time' => sub {
Expand All @@ -232,23 +212,9 @@ subtest 'from_json() returns a new profile every time' => sub {
subtest 'from_json("{}") returns a profile with all properties unset' => sub {
my $profile = Zonemaster::Engine::Profile->from_json( "{}" );

is $profile->get( 'resolver.defaults.usevc' ), undef, 'resolver.defaults.usevc is unset';
is $profile->get( 'resolver.defaults.recurse' ), undef, 'resolver.defaults.recurse is unset';
is $profile->get( 'resolver.defaults.igntc' ), undef, 'resolver.defaults.igntc is unset';
is $profile->get( 'resolver.defaults.fallback' ), undef, 'resolver.defaults.fallback is unset';
is $profile->get( 'net.ipv4' ), undef, 'net.ipv4 is unset';
is $profile->get( 'net.ipv6' ), undef, 'net.ipv6 is unset';
is $profile->get( 'no_network' ), undef, 'no_network is unset';
is $profile->get( 'resolver.defaults.retry' ), undef, 'resolver.defaults.retry is unset';
is $profile->get( 'resolver.defaults.retrans' ), undef, 'resolver.defaults.retrans is unset';
is $profile->get( 'resolver.source' ), undef, 'resolver.source is unset';
is $profile->get( 'resolver.source4' ), undef, 'resolver.source4 is unset';
is $profile->get( 'resolver.source6' ), undef, 'resolver.source6 is unset';
is $profile->get( 'asnroots' ), undef, 'asnroots is unset';
is $profile->get( 'logfilter' ), undef, 'logfilter is unset';
is $profile->get( 'test_levels' ), undef, 'test_levels is unset';
is $profile->get( 'test_cases' ), undef, 'test_cases is unset';
is $profile->get( 'cache' ), undef, 'cache is unset';
for my $property ( Zonemaster::Engine::Profile->all_properties ) {
is $profile->get( $property ), undef, "$property is unset";
}
};

subtest 'from_json() parses values from a string' => sub {
Expand Down Expand Up @@ -509,23 +475,9 @@ subtest 'set() updates values for set properties' => sub {
subtest 'set() dies on attempts to unset properties' => sub {
my $profile = Zonemaster::Engine::Profile->from_json( $EXAMPLE_PROFILE_1 );

throws_ok { $profile->set( 'resolver.defaults.usevc', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.usevc';
throws_ok { $profile->set( 'resolver.defaults.recurse', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.recurse';
throws_ok { $profile->set( 'resolver.defaults.igntc', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.igntc';
throws_ok { $profile->set( 'resolver.defaults.fallback', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.fallback';
throws_ok { $profile->set( 'net.ipv4', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset net.ipv4';
throws_ok { $profile->set( 'net.ipv6', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset net.ipv6';
throws_ok { $profile->set( 'no_network', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset no_network';
throws_ok { $profile->set( 'resolver.defaults.retry', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.retry';
throws_ok { $profile->set( 'resolver.defaults.retrans', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.defaults.retans';
throws_ok { $profile->set( 'resolver.source', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.source';
throws_ok { $profile->set( 'resolver.source4', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.source4';
throws_ok { $profile->set( 'resolver.source6', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset resolver.source6';
throws_ok { $profile->set( 'asnroots', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset asnroots';
throws_ok { $profile->set( 'logfilter', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset logfilter';
throws_ok { $profile->set( 'test_levels', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset test_levels';
throws_ok { $profile->set( 'test_cases', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset test_cases';
throws_ok { $profile->set( 'cache', undef ); } qr/^.* can not be undef/, 'dies on attempt to unset cache';
for my $property ( Zonemaster::Engine::Profile->all_properties ) {
throws_ok { $profile->set( $property, undef ); } qr/^.* can not be undef/, "dies on attempt to unset $property";
}
};

subtest 'set() dies if the given property name is invalid' => sub {
Expand Down

0 comments on commit ff0984d

Please sign in to comment.