Conversation
Will need changing based on PR #589.
ximon18
requested review from
bal-e and
tertsdiepraam
and removed request for
bal-e
bal-e
approved these changes
May 6, 2026
Contributor
bal-e
left a comment
bal-e
left a comment
There was a problem hiding this comment.
This looks great! I'm glad it didn't end up being a big PR, it looks like most of the surrounding architecture and control flow suited this addition well. I only have some minor comments; none of them are a blocker for merging. A few them only ask to add some code comments; that's probably the most important stuff in here, because it's easy to forget once the PR is merged. But again, not a blocker.
Also add a guard and improve the actual logged output.
Primarily to cover the wider than perhaps expected use of the soa() fn.
Currently fails due to an unrelated TSIG bug causing dig to fail.
ximon18
added a commit
that referenced
this pull request
May 8, 2026
- Merges in the ixfr-out and full-signer-update-state branches as these were needed for local testing, but should be obsoleted / synced with main when PRs #631 ("Save last serial and key tags in zone state")and #605 ("Add IXFR out support") get merged. - Extends the persist-restore system test to cover more cases. Should perhaps be split out into separate smaller tests. - Actually adds restored diffs to the zone storage to be served by IXFR out. Will need updating to match the changes/fixes that have since been made in PR #605. - Fixes an issue where persisted diffs from multiple SOA serials, e.g. 1..2..3 would be condensed on restore so that only a single IXFR diff from 1..3 would be available instead of two diffs from 1..2 and 2..3 being available. - Clears the set of known persisted data file paths for a zone if any of those files are missing or cannot be parsed during restoration.
As step.if doesn't seem to be honoured by nektos/act.
Not via TCP but via single SOA response, per RFC 1995. Also adds a system test using dig.
Incremental signing doesn't change the loaded zone so don't expect a non-empty loaded diff.
(and correctly, as in theory you could, especially with keep serial policy, have requested the right serial number from the loaded review server and have received a signed diff back...!)
And log an error.
Not on the review servers.
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
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.
Implements IXFR out based using internally created signed diffs which are moved to zone storage after they have been made available to zone persistence. Code was based on XFR code in
domain.System tests have been moved out on request to #650.
Lacks:
Cargo.*,crates/,etc/,integration-tests/,src/):actthrough theact-wrapper(as described inTESTING.md)?