Remove deprecated features planned for v2023.2

[tgreenx]

Purpose

This PR removes deprecated features that were planned for v2023.2 See the Changes section below.

Context

Module Zonemaster::Engine::Net::IP was deprecated by #1202
Profile options dnssec and edns_size were deprecated by #1147

Changes

• Remove module Zonemaster::Engine::Net::IP
• Remove profile options dnssec and edns_size
• Update unitary tests

How to test this PR

Tests are updated and should pass.

Manual testing:

$ git log -1 --oneline
910e9c5e (HEAD -> clean-deprecated, origin/clean-deprecated) Remove deprecated features planned for v2023.2

1. Check that Zonemaster::Engine::Net::IP is not available anymore

$ perl -MZonemaster::Engine -E 'use Zonemaster::Engine::Net::IP'
Can't locate Zonemaster/Engine/Net/IP.pm in @INC (you may need to install the Zonemaster::Engine::Net::IP module)

2. Check that setting profile option dnssec or edns_size in a custom profile return an error

$ cat share/custom.json
{
    "resolver" : {
        "defaults" : {
           "debug" : false,
           "dnssec" : false,
           "edns_size" : 0
        }
    },
    "test_cases": [
        "address01"
    ]
}

$ zonemaster-cli zonemaster.net --profile share/custom.json
Loading profile from share/custom.json.
Unknown property 'resolver.defaults.edns_size' at /home/tgreen/zonemaster/zonemaster-engine/lib/Zonemaster/Engine/Profile.pm line 311.

[mattias-p]

I applaud this initiative. I just have one suggestion.

[mattias-p]

I believe there is a different cleanup for this that is cleaner. I.e. replacing lines 466 and 467

        $flags{q{dnssec}}    = $href->{edns_details}{do} // $flags{q{dnssec}};
        $flags{q{edns_size}} = $href->{edns_details}{size} // $flags{q{edns_size}};

with

        $self->dns->dnssec( $href->{edns_details}{do} );
        $self->dns->edns_size( $href->{edns_details}{size} );

That should have the same effect with less and simpler code.

[tgreenx]

Unfortunately no that is not equivalent. Note that both options are still valid customizable flags, i.e. they can be changed in the Test Case implementations (with any call to Zonemaster::Engine::Nameserver::query()) - it is only the possibility of having/customizing them in the profile that is removed here.

But honestly, I'm not even sure this part of the code ("Reset to defaults", lines 537 to 543) is needed at all, since every options are checked whenever a new query is done (lines 454 to 463).

[mattias-p]

But honestly, I'm not even sure this part of the code ("Reset to defaults", lines 537 to 543) is needed at all, since every options are checked whenever a new query is done (lines 454 to 463).

I agree with you. I also don't think "restore" describes very well what actually happens. To me "restore" bringing back the state that was before you came. This code resets the state to whatever happens to be in the profile right now.

Should we just remove it? Maybe in the beginning of next release cycle so we have more time to find weird bugs?

Unfortunately no that is not equivalent.

Oh, you're right. I was clearly going too fast in my review. Or I was seeing what I wanted to see. I really dislike how %flags is used in _query().

[tgreenx]

But honestly, I'm not even sure this part of the code ("Reset to defaults", lines 537 to 543) is needed at all, since every options are checked whenever a new query is done (lines 454 to 463).

I agree with you. I also don't think "restore" describes very well what actually happens. To me "restore" bringing back the state that was before you came. This code resets the state to whatever happens to be in the profile right now.

Should we just remove it? Maybe in the beginning of next release cycle so we have more time to find weird bugs?

That can come in another PR.

[tgreenx]

@mattias-p @matsduf I just fixed a conflict, please re-review.

[matsduf]

Release testing: Looks fine.