fix(dnstap source): don't delete the socket file during config validation#25538
fix(dnstap source): don't delete the socket file during config validation#25538xfocus3 wants to merge 2 commits into
Conversation
…tion The framestream-based unix source removed any existing socket file and bound the listener eagerly while the source was being built. Component instantiation happens during `vector validate`, so validating a config deleted the socket file of an already-running dnstap source (mode: unix), killing it. Move the socket setup (stale-file removal, bind, buffer sizes, permissions) into the source future so it only runs when the source actually starts, matching the other unix sources. Validation no longer has this destructive side effect. Closes vectordotdev#25513
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39e4b5ea3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let Some(socket_permission) = frame_handler.socket_file_mode() { | ||
| if !(448..=511).contains(&socket_permission) { |
There was a problem hiding this comment.
Preserve validation of invalid socket_file_mode
With this check now inside the returned source future, build_framestream_unix_source returns Ok during component construction for configs such as a dnstap unix source with socket_file_mode = 0o600; vector validate will therefore pass a config that immediately fails when the source starts. The destructive socket operations need to be deferred, but this non-destructive mode validation should still happen during build/config validation or in a dedicated config validator.
Useful? React with 👍 / 👎.
The changelog fragment uses 'framestream' (the frame streams protocol the dnstap source is built on), which the spell-checker doesn't recognize.
Summary
Closes #25513.
vector validate -d(documented as a dry-run) deleted the Unix socket file of any runningdnstapsource inmode: unix, killing the source's accept loop.Root cause
build_framestream_unix_source()(src/sources/util/framestream.rs) removed any existing file at the socket path and bound the listener eagerly, before constructing the source future:This is fine for a real startup, but
vector validateinstantiates all components (viaTopologyPiecesBuilder::build()) to validate them — which runs this destructive code against the live socket of a separate, already-running Vector instance.Fix
Move the socket setup (stale-file removal,
bind, buffer-size sockopts, permission changes) into the returned source future, so it only executes when the source actually starts — not when it's built/validated. This matches how the other unix sources already work (util/unix_stream.rs,util/unix_datagram.rsbind insideBox::pin(async move { ... })).Build/validation errors are now logged and surfaced as a source failure (
Err(())) inside the future, consistent with the other unix sources.How did you test this PR?
build_unix_source_does_not_remove_existing_socket) that creates a file at the socket path, builds the source, and asserts the file still exists afterbuild(it is only managed when the future runs).cargo test --lib --no-default-features --features sources-dnstap sources::util::framestream::test→ 12 passed.cargo check,cargo clippy -- -D warnings, andcargo fmt --checkpass for thesources-dnstapfeature.