[cli] tsig add read TSIG keys from file only - #938
Merged
Conversation
withjannisNLnetLabs
force-pushed
the
tsig-add-yaml
branch
8 times, most recently
from
August 6, 2026 15:44
ccfa1c3 to
5929f7a
Compare
withjannisNLnetLabs
force-pushed
the
tsig-add-yaml
branch
3 times, most recently
from
August 13, 2026 13:17
2c9d3aa to
1b86d28
Compare
withjannisNLnetLabs
force-pushed
the
tsig-add-yaml
branch
from
August 13, 2026 13:34
1b86d28 to
38cac26
Compare
Contributor
|
Might solve #709. |
Contributor
|
Might solve #609. |
bal-e
requested changes
Aug 20, 2026
| } | ||
| let tsig_alg = TsigAlgorithm::from_str(&tsig_alg_raw, true).map_err(|_| { | ||
| format!( | ||
| "Unable to parse {} possible values are {:?}", |
Contributor
There was a problem hiding this comment.
Explicitly mention that the argument that could not be parsed is the TSIG algorithm field.
| _ => unreachable!("Excluded via Clap 'requires' rules"), | ||
| }; | ||
| let tsig_name = TsigKeyName::from_str(&tsig_name_raw) | ||
| .map_err(|err| format!("Invalid TSIG key name: {err}"))?; |
Contributor
There was a problem hiding this comment.
Maybe show tsig_name_raw here?
| line if line.starts_with(keywords.0) => name = Some(value_cleanup(line, &format)?), | ||
| line if line.starts_with(keywords.1) => algorithm = Some(value_cleanup(line, &format)?), | ||
| line if line.starts_with(keywords.2) => secret = Some(value_cleanup(line, &format)?), | ||
| _ => (), |
Contributor
There was a problem hiding this comment.
Detect unexpected lines (i.e. not comment, not blank, not tsig-key:) and report an error.
Comment on lines
+292
to
+294
| line if line.starts_with(keywords.0) => name = Some(value_cleanup(line, &format)?), | ||
| line if line.starts_with(keywords.1) => algorithm = Some(value_cleanup(line, &format)?), | ||
| line if line.starts_with(keywords.2) => secret = Some(value_cleanup(line, &format)?), |
Contributor
There was a problem hiding this comment.
Report an error if any of name, algorithm, secret are Some already.
withjannisNLnetLabs
force-pushed
the
tsig-add-yaml
branch
2 times, most recently
from
August 24, 2026 15:35
7871448 to
276a848
Compare
withjannisNLnetLabs
marked this pull request as ready for review
August 24, 2026 15:59
withjannisNLnetLabs
force-pushed
the
tsig-add-yaml
branch
from
August 25, 2026 13:07
276a848 to
42f4e1a
Compare
ximon18
added
the
cli
bal-e
approved these changes
Aug 27, 2026
bal-e
left a comment
bal-e
left a comment
Contributor
There was a problem hiding this comment.
This looks awesome, well done :)
withjannisNLnetLabs
changed the title
tsig add read TSIG keys from file only
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.
The cli should promote best practices, therefore the ability to add TSIG keys visible over the cli should not be possible. Additionally the cli UX should be unified with the other commands throughout the cli.
nsd/unboundand can be read/written by a lot of tools through yaml.dnstmight support the generation of tsig keys in the yaml'ish format aswellReferences
resolves #898 "Drop support for supplying TSIG secret on the CLI, support NSD/Unbound's TSIG config snippet format"
resolves #709 "TSIG secret can also be a path to a file, but what's the formula?"
resolves #706 "Possible enhancement: cascade tsig add from BIND-compatible TSIG key files"
resolves #609 "Is Base64 encoded TSIG secret key material passed to tsig add ambiguous?"
If you are changing Rust code or integration tests (
Cargo.*,crates/,etc/,integration-tests/,src/):actthrough theact-wrapper(as described inTESTING.md)?If you are adding/deleting man pages:
man_pagesconfig indoc/manual/source/conf.py?Cargo.toml?If you are modifying man pages: