Change how the nsupdate code works #3871
Conversation
* Now supports individual keys for different NS servers. * Updates will only be sent to known NS servers.
|
I'm unsure how to handle the migration/schema update that this pull-request contain. If it needs to be handled differently please let me know how, or if there is any developer references available. |
|
Thanks for your contribution! This is not a small change, so I'd like to have an issue that describes the issue/feature request that you are trying to fix. In regard to your question: The library we are using for schema validation is schema. There is no dedicated developer guide sadly. What I can tell you is that all migrations need to be done inside the method Furthermore, these are changes that need to be tested, so if you have the resources I'd love if you could add a test case that shows that the code works as written. |
|
I’ll create a Issue, before resubmitting test cases and settings migration code.
I can try however this will involve setting up a (mock) dns server with domains and all.
Not sure if that has been done in other parts of the code and can be reused.
When I did a casual review I could not find any examples on ‘data migration’ for Optional parameters, but admittedly I did not spend a lot of time looking :-/
/JockeF
… On 29 Jan 2025, at 16:47, Enno G. ***@***.***> wrote:
Thanks for your contribution! This is not a small change, so I'd like to have an issue that describes the issue/feature request that you are trying to fix.
In regard to your question: The library we are using for schema validation is schema <https://github.com/keleshev/schema>. There is no dedicated developer guide sadly. What I can tell you is that all migrations need to be done inside the method migrate and that it migrates from version x-1 to version x.
Furthermore, these are changes that need to be tested, so if you have the resources I'd love if you could add a test case that shows that the code works as written.
—
Reply to this email directly, view it on GitHub <#3871 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFCTP6UJP2TBGDFT6PYKFN32NDSZ3AVCNFSM6AAAAABUHHEGKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRRHA3DANZSHA>.
You are receiving this because you authored the thread.
|
|
Thanks for you efforts also on this PR. Setting up a mock DNS server is possible via a supervisord unit, for your unit-tests this shouldn't be needed though. Just try to validate that the configuration file contains a piece of text that you expect. What you are attempting to do is part of the much more complicated integration testsuite. An example of a migration of optional parameters is not found because optional parameters are not present per default in the settings. Since in your case there is no migration to be done due to a missing relationship of zone and currently assigned key (if I understand correctly), you can just drop the old key inside the method helper.key_delete("example", settings)Starting with version 3.4.0 we have no default values present in the settings file anymore, as such there is no need to add new keys. |
Linked Items
Fixes #
Description
Refactor the existing nsupdate code so that it supports different sig keys for different servers.
Updates will only be attempted if the name server is know (in the configuration settings)
Behaviour changes
Old: The code assumes you use the same tsig-key for all name servers, and it will send updates to any server using the same key.
New: The update allows (force) you to configure tsig-keys for each name server that you want to be able to send updates to.
Category
This is related to a:
Tests