Skip to content

Persist and restore TSIG keys for zone sources#626

Merged
bal-e merged 2 commits into
mainfrom
persist-zone-source-tsig-keys
May 6, 2026
Merged

Persist and restore TSIG keys for zone sources#626
bal-e merged 2 commits into
mainfrom
persist-zone-source-tsig-keys

Conversation

@bal-e
Copy link
Copy Markdown
Contributor

@bal-e bal-e commented May 5, 2026

A re-implementation of #590. GH seems confused about that PR, so it's hard to see it's original diff, but I think this is the core of it.


  • If you are changing Rust code or integration tests (Cargo.*, crates/, etc/, integration-tests/, src/):
    • Did you run the integration tests with act through the act-wrapper (as described in TESTING.md)?

@bal-e bal-e requested a review from tertsdiepraam May 5, 2026 09:39
@bal-e bal-e self-assigned this May 5, 2026
@bal-e bal-e force-pushed the persist-zone-source-tsig-keys branch from 1775511 to e8cc640 Compare May 5, 2026 09:49
@ximon18
Copy link
Copy Markdown
Member

ximon18 commented May 6, 2026

Manual testing with the .ch zone shows this working for me, the TSIG key is again used after a restart to query the source nameserver which without this PR is not the case.

Copy link
Copy Markdown
Member

@ximon18 ximon18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some minor comments.

Comment thread src/main.rs Outdated
// to the zones and policies being restored.
// Load the TSIG store file.
match state.tsig_store.init_from_file(&config) {
Ok(()) => debug!("Loaded the TSIG store"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent logging:

  • State::init_from_file() loads the file then logs at info level and notes the path that was loaded from.
  • TsigStore::init_from_file() doesn't log, instead main does, and that log message doesn't mention the path that was loaded from.

Also, though not caused by this PR, neither for main state nor TSIG store state do we log the path we are trying to load from before loading, nor on error, so in the unhappy case the path used is not mentioned at all - unless I'm missing something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right; I've added a commit which logs every time.

Comment thread src/main.rs

let mut state = center::State::default();

// Load the TSIG store file.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would there be a TSIG store file if there was no state file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the user deleted the state file (or lost it) and doesn't want to re-enter their TSIG keys?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but we don't generally support restoring from a partial/corrupted state, e.g. once the state file is lost any zone state files become orphans, those zones are no longer reflected in main state IIRC. This seems a bit inconsistent.

Comment thread src/zone/state/mod.rs
@ximon18 ximon18 linked an issue May 6, 2026 that may be closed by this pull request
@ximon18 ximon18 added this to the 0.1.0-beta1 milestone May 6, 2026
@bal-e bal-e force-pushed the persist-zone-source-tsig-keys branch from 4b1fe83 to e7031df Compare May 6, 2026 09:57
@bal-e bal-e merged commit fe1a745 into main May 6, 2026
9 checks passed
@bal-e bal-e deleted the persist-zone-source-tsig-keys branch May 6, 2026 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TSIG support

2 participants