feat(sinks): validate all sinks at config-compile time - #26048
feat(sinks): validate all sinks at config-compile time#26048thomasqueirozb wants to merge 14 commits into
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
vector/src/sinks/azure_blob/config.rs
Line 287 in 19eef9d
Parse Azure Blob connection strings during validation
For azure_blob configurations that provide a malformed connection_string, this branch just copies the secret into validated state, while ParsedConnectionString::parse and container URL parsing still run later in build_client. Because vector validate --no-environment skips that build path, these pure configuration errors continue to pass compile-time validation; parse and retain the validated connection/container URL state here, deferring only credential construction to build.
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eddcfa6ff
ℹ️ 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".
| host_key, | ||
| message_key, | ||
| source_type_key, |
There was a problem hiding this comment.
When influxdb_logs leaves host_key/message_key/source_type_key unset and the config sets custom global log_schema keys, these fallback paths are now retained during validate(), which runs inside builder.build_with_warnings() before normal startup calls config::init_log_schema(...) in app::load_configs. That captures the built-in defaults instead of the configured global schema, so the sink encodes/renames the wrong log fields at runtime; defer resolving these log_schema() fallbacks until build after global schema initialization, or initialize the schema before sink validation.
Useful? React with 👍 / 👎.
|
|
||
| let request_limits = self.request.into_settings(); | ||
|
|
||
| Ok(ValidatedZerobus { request_limits }) |
There was a problem hiding this comment.
For databricks_zerobus configs with invalid generic batch options such as batch.max_events = 0 or batch.timeout_secs <= 0, validate() returns retained state after only the custom structural checks and request settings. The build path still passes self.batch into ZerobusSink::new, where into_batcher_settings() rejects those values, so vector validate --no-environment now misses a pure sink config error that still fails at startup; compute and retain the batch settings here like the other migrated sinks.
Useful? React with 👍 / 👎.
| .limit_max_events(MAX_PAYLOAD_EVENTS)? | ||
| .into_batcher_settings()?; | ||
|
|
||
| Ok(ValidatedKinesisFirehose { batch_settings }) |
There was a problem hiding this comment.
For aws_kinesis_firehose (and the streams sink that uses the same helper), a malformed pure encoder config such as an invalid Avro schema still passes vector validate --no-environment because this validated state only covers batching. Startup later calls build_sink, which runs config.encoding.build()? after AWS client construction, so the new compile-time validation path does not catch these encoding errors; build and retain the transformer/encoder during validation or otherwise run the same pure encoder check here.
Useful? React with 👍 / 👎.
|
|
||
| fn validate(&self) -> crate::Result<ValidatedConsoleSink> { | ||
| let transformer = self.encoding.transformer(); | ||
| let (framer, serializer) = self.encoding.build(SinkType::StreamBased)?; |
There was a problem hiding this comment.
For a console sink using encoding.codec = protobuf, vector validate --no-environment now calls encoding.build() here, and ProtobufSerializerConfig::build() opens the configured descriptor file via get_message_descriptor. That makes no-environment validation fail whenever the descriptor is unavailable even though startup-time file access should stay in build; validate only the pure serializer options here, or defer serializer construction for codecs that read files.
Useful? React with 👍 / 👎.
| let index = | ||
| Template::try_from(self.token.inner()).expect("unable to parse token as Template"); |
There was a problem hiding this comment.
When sematext_logs.token is a template such as {{ index }}, this validation only parses and retains it, while build() later wraps it as ElasticsearchConfig.bulk.index and calls the Elasticsearch build path that rejects unconfined routing templates. Since vector validate --no-environment stops at this validate(), those configs still pass validation but fail at startup; construct and validate the derived Elasticsearch config (or run its common_mode check) before retaining the Sematext state.
Useful? React with 👍 / 👎.
1 participant
Summary
Extends the validated sink lifecycle to all sinks and makes
DynValidatedSinka supertrait ofSinkConfig, removing the per-sinkas_dyn_validatedboilerplate. This also fixesvector validate --no-environmentto catch sink configuration errors (including template path-confinement violations) at config-compile time.Vector configuration
NA
How did you test this PR?
make check-clippyandcargo test -p vector --no-run --features sources-demo_logs.Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References