-
Notifications
You must be signed in to change notification settings - Fork 33
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.
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
Provide a way for third-party test modules to be installed. #1323
base: develop
Are you sure you want to change the base?
Conversation
…lled as test modules
@gbicann, thank you for your proposal. We will review this. Making Zonemaster more flexible is positive. When you do some POC runs, do not forget to create a custom profile.json (e.g. in /etc/zonemaster) where the test cases are enabled for testing, or else they will not be run. |
@matsduf thanks for the feedback.
So Would you prefer that patching the profile has to be done separately? |
I can see how you have done it, but I cannot see how you envision How should Zonemaster know that there is a custom test module?
Your proposal would always run all test cases in a custom module. In standard Zonemaster only test cases listed in profile.json are run, which makes it possible to disable some test cases in a module, but not necessarily all. |
I like the idea of making it possible to add a plugin with addition custom test cases to zonemaster, but I am also curious about you example of additional test cases.
If you with your example mean that SOA MNAME must be a valid IDNA 2008 domain name and the same thing for the domain part of the SOA RNAME, I do not feel that those tests are alien to Zonemaster testing. I can envision including those. |
On reflection, I agree that perhaps it would be better to add custom modules using an option in the profile. Perhaps a {
"custom_modules": [
"My::Module"
]
} Let me know what you think about that, I could probably rework this merge request to work that way. Please also let me know where documentation would need to be updated.
That's good to hear. The full description of this test case is here. I have an implementation, so perhaps this could be dropped into |
I've now implemented this approach in the I am minded to close this PR and submit a new one with this approach instead. Let me know what you think. |
We will discuss this in the work group.
I do not find the specification. Can you point exactly at the content? Syntax09 is fine, but specification must be completed first, and the implementation must match the specification. We will review and possibly update the specification to match the "standards". Your implementation can be sent in as a PR, but we must complete the specification before working on the implementation. |
Creating a new test case, e.g. Syntax09, verifying that SOA MNAME and RNAME have valid IDN labels if any A-label or A-label like is found, is not very complicated. It should be done in two steps, first the specification is composed and approved, and then an implementation following the specification is created or finalized. If you have a start of a specification it would be helpful. You could provide that as an issue or PR to the zonemaster/zonemaster repository where it would belong. You could also provide a list of other test cases that you think are missing today as in issue to zonemaster/zonemaster and we will look at that. Adding flexibility to allow for a custom test module might require multiple changes in Zonemaster. We should discuss this in the Zonemeaster work group (Internetstiftelsen and Afnic) first to see what path we suggest. Or if we find severe blockers e.g. due to some magic in the code. This would also create a new API in Zonemaster, i.e. between a custom module and Zonemaster-Engine. That API should be defined and documented so that custom modules are well-behaved and Zonemaster retain the interface needed for the custom module. Nothing that I write here should be taken as a promise since this has to be discussed and prioritized in the Zonemaster project. What is the ICANN time line? |
Thank you @gbicann for the feature request and for exploring the design space with your two drafts! We discussed this at our workgroup meeting yesterday and everyone agrees we want to see this feature implemented. We also had some ideas on what would be a fitting design. I promised to draw up a design proposal. I'll link to here in a day or two so you can review it. |
Hi @mattias-p and @matsduf, thank you so much for your time and attention on this, it's much appreciated. I look forward to seeing your design proposal. I hope my prototyping has been useful. To answer Mats' question about ICANN's timeline: the new RST system needs to be ready by the end of 2024, however the additions to Zonemaster are a very small part of that project and do not block any other work so there is no particular urgency. I'm interested to hear that you think there might be value in including our additional test cases into the main Zonemaster code base. We have four test cases in our plan which are as follows:
None of these are technical tests, they are checking to see that a registry operator has configured the TLD zone in accordance with ICANN policy. Therefore their applicability to other users of Zonemaster might be quite limited: this is why I proposed a way to load a custom test module, rather than just adding test cases to the existing test modules. By the way, if we do end up implementing a custom test module, we'd almost certainly publish it under an open source license on CPAN, so you could always just add it as a dependency and incorporate it into Zonemaster that way :) |
Checking that SOA MNAME and RNAME fields have are valid IDN names, or just plain LDH names, could be integrated in Zonemaster. When it comes to RNAME is should be from label two, I guess. Checking all DNS records in apex is more complicated. That should already be covered by DNSSEC05 and customize the levels for the message tags, shouldn't it? That should already be covered by DNSSEC01 and customize the levels for the message tags, shouldn't it? That should be covered by upcoming DNSSEC03 (still in develop branch), shouldn't it? It will soon be released and merged to master branch.
It will also open for the possibility to fork the standard test cases if changes of levels of message tags is not enogh. |
Hello @gbicann, I'm sorry I took so long to get back to you. There was more nuance to this than I first anticipated, and of course other things got in between as well. Anyways, I've made a new design for the custom test modules feature. It's expressed in a series of issues where each issue introduces one new idea and each idea builds on the previous one. Here they are in order:
I based the new design on some of your ideas, applying them not only to the custom test modules but to the built-in ones as well to great effect. The scope of the new design is a bit larger than your drafts and the exact APIs and some of the mechanics had to change to work with other important use cases. I would like to thank you for your efforts in drafting this feature. Your drafts greatly informed my understanding of your needs. I showed the new design to the other members of the Zonemaster work group and their initial reaction was very positive. If you want to review it from the perspective of your use case and let us know how it fits, I would appreciate that. But I do believe I have you covered. If you want to implement any of this you are very welcome. Otherwise we'll do it within the work group. I'm not sure about our prioritization of the feature but I believe it's fairly high. |
Hi @mattias-p and @matsduf, apologies for the long delay in replying. Our development team has made the decision that the additional test cases we need for RST v2.0 will be implemented in our Java-based test runner, since this is the preferred language that's used for most ICANN-internal projects, and is therefore easier for the Org to maintain. However, speaking as a Perl hacker and regular user of Zonemaster, what you outlined above looks really good, and I am pretty sure it will be of use in the future, so I would like to offer my thanks for your efforts. It's greatly appreciated! |
Purpose
This PR adds a new method called
install_custom_test_module()
toZonemaster::Engine::Test
. It allows custom modules outside theZonemaster::Engine::Test::
namespace to be added to Zonemaster as test modules. Currently the only way to do this is to patch themodules.txt
file and intrude upon the Zonemaster namespace.Context
I am developing a new Registry System Testing (RST) service that uses Zonemaster for DNS and DNSSEC testing. There are a handful of RST test cases that Zonemaster doesn't currently cover, which I don't think are appropriate for inclusion in the main Zonemaster test suite (for example, checking for the presence of valid IDNA2008 labels in SOA
mname
andrname
fields, which is not something that most Zonemaster users will care about). However, I still want to use Zonemaster to run these test cases, and I think the ability to install custom modules may be more generally useful (for example, in future it might allow plugins for Zonemaster to be distributed separately, via CPAN).Changes
There are two main changes:
install_custom_test_module()
method has been added toZonemaster::Engine::Test
. It accepts a class name in a string, eg:It adds the module name to the internal module list, and updates the effective profile to add the module's test cases to the test sequence, checking to make sure that they don't collide with existing test case names.
run_all_for()
,run_module()
andrun_one()
methods have been changed so that if a module name looks like a Perl class name (that is, it contains::
) it is used as-is rather than being prefixed withZonemaster::Engine::Test::
.How to test this PR
This PR includes a test cast. Use
prove t/custom.t
to run it.