Fix zone walking to include non-leaf CNAMEs #352
Merged
Conversation
This file contains 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
Add code to emit the non-leaf CNAMEs to the zone walker callback operation.
ximon18
added a commit
that referenced
this pull request
Jul 12, 2024
- Fixed a bug in TSIG client sequence verification introduced by an earlier fix for server sequence signing (also pushed to PR 345). - Fixed issues with AXFR response batching. - Fixed a Stelline test bug where a request for a UDP client was treated as a request for a TCP client. - Fixed an errant blank line per zone entry bug in processing of the `local-data` Stelline server config setting. - Fixed a zone walking bug that omitted CNAMEs at non-leaf nodes (also pushed as PR #352). - Made the zone argument to net::client::xfr optional. - Made net::client::xfr pass on the messages it receives after it has checked/processed them. - Added a multi-response large zone variant of the AXFR test. - Added a Stelline TSIG variant of the AXFR test. - Renamed the XFR Stelline tests. - Renamed the TSIG client from net::client::auth to net::client::tsig. - Added the NOTIFY middleware back in to the server Stelline tests. - Added the TSIG middleware to the server Stelline tests. - Extended the server Stelline tests to: - Create a 'TESTKEY' in the key store. - Wrap TCP connections created by the client factory in the tsig and xfr clients. - Wrap UDP connections created by the client factory in the tsig client. - Accept a named TSIG key in the `provide-xfr` server config setting. - Accept a named TSIG key in the `STEP N QUERY` Stelline command. - Take the entire ENTRY as input to the client factory, not just the client IP address. (so that any specified TSIG key for the client can be configured) - Accept `$ORIGIN` in `ANSWER SECTION` response RRs. (so that many RRs with a shared origin can be expressed more succinctly) - Change in-memory zone tree node ordering to be deterministic (BTreeMap based instead of HashMap based) in #[cfg(test)] mode. (so that the new large AXFR response test can know which response RRs will be present in which response message) - Removed unused stream connection ID numbering and related Tokio tracing span that was causing unwanted test output indentation. - Removed some unnecessary calls to the zone walking callback op fn.
|
No test is included in this PR, partly because the state of the Stelline server testing in this PR isn't able to test AXFR which is what needs to be queried to exercise zone walking which is what this PR fixes. A manual test however with NSD, domain without the fix and domain with the fix shows that this PR resolves an issue. Given the zone file: A sorted AXFR dig query output when run against NSD and domain with the fix in this PR (applied to the (with dig command: Without the fix in this PR domain misses some records in the AXFR output: |
Merged
partim
approved these changes
Oct 2, 2024
ximon18
added a commit
that referenced
this pull request
Oct 3, 2024
Add code to emit the non-leaf CNAMEs to the zone walker callback operation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Querying a zone for a CNAME works, but when walking the zone (e.g. for diagnostic dumping or to do AXFR) a CNAME that is not at a leaf node (i.e. a CNAME at a.example.com while there also exists a deeper b.a.example.com leaf node) is incorrectly skipped by the current zone walking code.
This PR adds the missing code to emit the non-leaf CNAME to the zone walker callback operation.
This PR does NOT include unit tests to cover this fix because (a) there are no existing unit tests for this code and writing an entire set would time that I don't have right now, (b) strict unit tests are not really possible because they would also have to exercise the code in
nodes.rsto construct the tree before wakling it and so some sort of "integration" test is needed, and (c) the new XFR work being done for PR #335 will include Stelline tests that walk zones (for AXFR) and should visit all parts of the tree.While the Stelline tests will be at the outermost testing layer, exercising domain as a black box, so one could perhaps argue that testing somewhere nearer to the tree walking code would also be good to have, they will at least address the immediate need to exercise the entire zone walking code.