Introduce a new zone loader #15
Draft
Conversation
This file contains hidden or 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
This will be used to monitor filesystem paths, for the zonefile loader as well as for Nameshed state.
This sets up 'LoaderState', currently without any automatic refresh mechanisms.
This provides a mid-level API similar to 'zonetree'. It has some important differences: - It does not support proper queries. - It uses 'domain::new'. - It can hold old versions of a zone. - It provides direct access to the SOA record. - It offers a synchronous API. The idea is to enhance it to store both unsigned and signed versions of a zone simultaneously, by separating the regular and signing-related records. This will provide a compact, uniform way to store zone data before and after signing.
- Also implement 'Debug' for 'ZoneContents'
This reimplements IXFR and AXFR based zone loading, strongly based on the existing implementation in the zone maintainer. The new code uses 'domain::new' wherever possible, and produces zone updates in the form of 'ZoneContents'. The interface should be changed in the future to improve performance by processing the zone changes in parallel and on dedicated threads (using 'tokio::task::spawn_blocking()'). This would be useful for coalescing IXFR diffs, computing diffs when an AXFR is received, and comparing zones if the user forces a reload. It's hard to tell what the performance impact of converting between the old-base and 'domain::new' is. It wouldn't be too hard to inline the XFR interpretation (which is especially easy for AXFRs), and that would eliminate most of the conversions.
This adds a simple zonefile loader based on 'new-zonefile'. It only returns an uncompressed zone, which needs to be compressed manually when integrated into 'ZoneContents'.
If a user is concerned that Nameshed's cache of loaded zone data is incorrect, reloading can be used to fetch a new version of the zone without relying on previous loaded zone data or to verify that the local copy of the latest version of the zone is correct. If ZONEMD is supported in the future, reloading could simply check the ZONEMD record against the local copy to verify correctness. Most of the time, Nameshed will be performing refreshes instead, which do trust the local copy and can perform more efficient incremental zone transfers.
Zones are refreshed periodically (as per the SOA REFRESH timer) and in response to source-specific events (e.g. NOTIFY messages and filesystem notifications). Because of its frequency, effort is taken to minimize the cost of a refresh, by avoiding work if the zone has not changed.
- Also use 'refresh' as the general term for 'refresh / reload'. The next step is to connect the new zone storage to the existing zonetree work so it is compatible with the rest of the codebase.
- Factor out old-new record conversions - Fix adding IXFR records to the right vec
The 'RefreshMonitor' is a transparent time scheduler. It can provide us with (an atomic snapshot of) the list of currently scheduled zone refreshes, which will be very useful for reporting Nameshed's state.
- The refresh/reload state logic (i.e. repeatedly locking the zone state and comparing the new data to what's there) has been unified between 'zonefile' and 'server'. - 'loader::refresh()' and 'reload()' now return mutex guards, so that the caller has better guarantees about state consistency across the function boundaries. - 'zone::loader::RefreshTimerState' has been implemented, which tracks the per-zone refresh timer state, and synchronizes it with the global 'RefreshMonitor' schedule. - When a zone reload/refresh occurs, the refresh timer is now updated accordingly, and a followup reload/refresh (if enqueued) is started.
This is pretty hacky, and there's plenty of work to go, but a local run seems to be working fine.
Contributor
tertsdiepraam
left a comment
tertsdiepraam
left a comment
There was a problem hiding this comment.
I might be dense, but I can't find where you remove the functionality you're replacing? Which of these files contains that?
Contributor
There was a problem hiding this comment.
The rustfmt config change definitely doesn't make this easy to review.
Contributor
Author
There was a problem hiding this comment.
Huh, I thought I had re-formatted the same code directly on main so it wouldn't show up in the diff.
| /// This is equal to `previous + soa.refresh`. If the SOA record | ||
| /// changes (e.g. due to a new version of the zone being loaded), this | ||
| /// is recomputed, and the refresh is rescheduled accordingly. | ||
| scheduled: Instant, |
Contributor
There was a problem hiding this comment.
We talked about this in a huddle, but we should probably also store SystemTime or chrono::DateTime or something else serializable/displayable for the CLI.
src/lib.rs
Outdated
| //! Rotonda | ||
| #![allow(renamed_and_removed_lints)] | ||
| #![allow(clippy::unknown_clippy_lints)] | ||
| #![allow(clippy::uninlined_format_args)] |
Contributor
There was a problem hiding this comment.
Didn't we want fewer of these? ;p
Contributor
Author
There was a problem hiding this comment.
Yes, but these pop up in all the code unrelated to mine and make it harder for me to use Clippy. The right solution might be to fix all that code separately, but I think this particular lint represents a style choice and we may not want to enforce it.
Contributor
There was a problem hiding this comment.
I wonder why I'm not seeing them then? Are these still a problem?
Contributor
Author
Ah yeah, this PR supersedes all the zone maintainer code, and |
Further clear out old zone maintainer code.
Member
|
Would it be in scope to fix #22 as part of this work? |
These were not being detected by older Rust versions.
It turns out Domain's XFR response interpretation code doesn't quite work as I expected, and returns 'None' in a case it should return a 'Some'. This can be worked around in the future, perhaps by manually traversing the XFR response.
We only reload files when the user requests it. Perhaps automatic tracking functionality would be useful for status commands, to inform the user there may be unsynchronized changes; but that's for later.
As part of the state management system, this will be fleshed out to save/load a TSIG secrets file.
- Implement a versioned serialization specification. - Implement top-level loading/saving commands for 'TsigStore'.
Member
|
Note: We decided to prioritize other work first and come back to this later. |
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This lays out a brand new zone loader, based on the existing zonemaintainer code. It builds in proper support for loading zonefiles, and uses new-base very prominently, but more importantly, demonstrates a new state management paradigm that I hope to use throughout Nameshed. It makes it much easier to examine and consistently snapshot the program's state. The difficulty is that I've never seen any other uses of this paradigm before, which means that I have to invent some things (e.g. scheduling events) from scratch. But I'm very confident in its reliability.
Tasks:
ZoneContentswrt signer-created zone versions@ximon18, I'd appreciate if you could test this out in your local sandbox. The new-base zone loader doesn't support quite as many record types yet, but I'm willing to address those now.