Repository navigation
Conversation
Add a --data-dir command-line flag (and corresponding VECTOR_DATA_DIR environment variable) to override the data_dir global configuration option. When set, it takes precedence over any data_dir value in the configuration file. This keeps deployment-specific paths out of the configuration file and makes it possible to validate a configuration in a CI environment where the configured data_dir may not exist. Closes vectordotdev#9766
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 946a6e4464
ℹ️ 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".
| #[arg(long, env = "VECTOR_DATA_DIR")] | ||
| pub data_dir: Option<PathBuf>, |
There was a problem hiding this comment.
This option is added only to RootOpts, but the validate subcommand is executed with its own validate::Opts and SubCommand::execute passes only those subcommand opts into validate::validate. As a result, VECTOR_DATA_DIR=/tmp vector validate ... (and vector --data-dir /tmp validate ...) parses the value but never applies it before validate_config/create_tmp_directory, so the documented CI validation case still uses the config file's data_dir and can fail on the nonexistent path.
Useful? React with 👍 / 👎.
| if let Some(data_dir) = data_dir { | ||
| debug!( | ||
| message = "Overriding data_dir from command line.", | ||
| ?data_dir | ||
| ); | ||
| config.global.data_dir = Some(data_dir); |
There was a problem hiding this comment.
This override is applied only to the initial config returned by load_configs; reloads in handle_signal call config::load_from_paths_with_provider_and_secrets directly and TopologyController::reload rejects changes to global options. With --watch-config/SIGHUP and a CLI/env data_dir that differs from the file (or default), the reloaded config loses this assignment, differs from the running global options, and reload fails with GlobalOptionsChanged instead of continuing to use the requested deployment-level data directory.
Useful? React with 👍 / 👎.
| #[arg(long, env = "VECTOR_DATA_DIR")] | ||
| pub data_dir: Option<PathBuf>, |
There was a problem hiding this comment.
Because this is a top-level RootOpts argument without global = true, clap only accepts it before a subcommand, so the advertised vector validate --data-dir /tmp vector.yaml form is rejected as an unexpected argument before any override can happen. If this flag is meant to work with subcommands in the position shown in the PR description, it needs to be global or defined on those subcommands.
Useful? React with 👍 / 👎.
| if let Some(data_dir) = data_dir { | ||
| debug!( | ||
| message = "Overriding data_dir from command line.", | ||
| ?data_dir | ||
| ); | ||
| config.global.data_dir = Some(data_dir); |
There was a problem hiding this comment.
The override happens only after load_from_paths_with_provider_and_secrets has fully built the config, but GlobalOptions::merge rejects two config files that both set different non-default data_dir values during that build. In a split configuration where --data-dir/VECTOR_DATA_DIR is supplied specifically to take precedence, startup still fails with conflicting values for 'data_dir' found before this assignment runs, so the deployment-level override cannot actually supersede all config-file values.
Useful? React with 👍 / 👎.
pront
added
the
type: feature
pront
added
the
meta: awaiting author
github-actions
Bot
added
docs review on hold
Preserve data-directory overrides and upstream config-loading changes. Keep both load_configs lint allowances, mark the consuming data_dir builder as must_use, and extract the reload test's subprocess launcher for Clippy.
github-actions
Bot
removed
the
meta: awaiting author
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17fe374dfe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Keep option and path as separate arguments. The service library owns | ||
| // Windows command-line quoting, including spaces and trailing backslashes. | ||
| args.push(OsString::from("--data-dir")); | ||
| args.push(data_dir.as_os_str().to_owned()); |
There was a problem hiding this comment.
On Windows, when vector service install is given a relative override, this stores it verbatim in the SCM launch arguments. The inspected service path in src/vector_windows.rs passes those arguments to SCM and run_service never restores the installer's working directory, so whenever the service starts in a different working directory, a value such as ./state resolves elsewhere and Vector can write checkpoints/buffers to the wrong location or fail to start. Resolve the override against the installer working directory before registering the service.
Useful? React with 👍 / 👎.
Resolve relative data directory overrides against the installer's working directory before loading configuration and persisting service arguments. Preserve native and absolute paths without requiring the directory to exist. Add isolated working-directory regression tests and document the service installation behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c83ad03a13
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| _core_config_options: { | ||
| "data-dir": { |
There was a problem hiding this comment.
The CLI page template renders each subcommand from that command's own options map (website/layouts/shortcodes/cli/commands.html:66-69), and the validate map at website/cue/reference/cli.cue:483-505 does not reference this new shared option. As a result, the generated vector validate section—the primary usage example in this change—omits --data-dir, while graph and test expose it; add "data-dir": _core_config_options["data-dir"] to validate's options as well.
Useful? React with 👍 / 👎.
Retain upstream Clippy refactors while forwarding the deployment data directory. Verified 25 distinct focused tests, scoped Clippy with warnings denied, and Rust formatting. Prepared with OpenAI assistance; exact model identifier unavailable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e2e67c5b4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut builder: ConfigBuilder = deserialize_config_map(map)?; | ||
| if let Some(data_dir) = &self.data_dir { | ||
| builder.set_data_dir(data_dir); |
There was a problem hiding this comment.
When environment interpolation is enabled and a config has data_dir: "${VECTOR_DATA_DIR}" (the repository's integration configs use this pattern), supplying only --data-dir /tmp still fails: prepare() calls prepare_input and reports the missing environment variable before control reaches this post-deserialization assignment. Thus the higher-priority CLI value cannot replace this configuration-layer value; mask or replace data_dir before interpolation.
Useful? React with 👍 / 👎.
| let current_exe = ::std::env::current_exe().unwrap(); | ||
| let config_paths = self.config_paths_with_formats(); | ||
| let arguments = create_service_arguments(&config_paths).unwrap(); | ||
| let arguments = create_service_arguments(&config_paths, data_dir).unwrap(); |
There was a problem hiding this comment.
On the Windows service install path, an empty CLI value (--data-dir "") enters the new resolver, where std::path::absolute("") returns an error and create_service_arguments returns None (the added test at src/service_arguments.rs:261-268 asserts this); this call then unwraps that None, so ordinary invalid input terminates with a panic instead of an exit code. Reject the empty value during parsing or propagate this failure through cmd.
Useful? React with 👍 / 👎.
pront
added
the
meta: awaiting author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a888d9f9bc
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .map_err(|e| vec![e])?; | ||
|
|
||
| ConfigBuilderLoader::default() | ||
| .data_dir(data_dir) |
There was a problem hiding this comment.
When the configured value is a secret placeholder (for example, data_dir: SECRET[prod.path]) and --data-dir or VECTOR_DATA_DIR supplies the replacement, load_builder_from_paths_with_secrets still scans the unmodified files and awaits SecretBackendLoader::retrieve_secrets before this override is applied. Thus startup or normal validation still fails if the production secret backend is unavailable or omitted, even though the secret value is discarded; make the first secret-collection pass aware of the override or mask this field before collecting placeholders.
Useful? React with 👍 / 👎.
Propagate invalid configuration argument failures before contacting the Windows service manager. Characterize existing empty-value rejection and non-Unicode path support. Verified 31 Linux tests, scoped Clippy and Rust formatting. Windows compilation and service execution remain unverified. Prepared with OpenAI assistance; exact model identifier unavailable.
github-actions
Bot
removed
the
meta: awaiting author
|
Before changing preprocessing for the environment interpolation and secret lookup findings, I'd like to agree on the compatibility boundary. An unset variable or secret used only in the overridden root Should the fix preserve all existing preprocessing behavior outside the overridden |
This branch has not been deployed
2 participants
Summary
Closes #9766. Adds the global
--data-diroption andVECTOR_DATA_DIRenvironment variable to override the deployment's data directory without editing pipeline configuration.Precedence is CLI, then environment, then configuration/default. The option is accepted before or after a subcommand. Each configuration file receives the override before global options are merged, so split files with differing
data_dirvalues can use one deployment override.The override reaches run, validate, graph and config unit-test loading, persists through disk/component reload and provider-builder replacement, and is included in Windows service-install launch arguments. Windows arguments remain separate
OsStringvalues; service-control behavior is unchanged. With neither option nor environment variable set, existing configuration/default behavior is unchanged. Website CLI reference entries document precedence, usage and unset behavior.Usage
This change covers
data_dironly; other global settings are outside its scope.How did you test this PR?
On Rust 1.96.1, at head
82f65de8606dc3f49bd9bb882cdbef075b31d4f6:--no-default-features --features cli-tests --lib --tests -- -D warnings) andcargo fmt --all -- --checkpassedprofile.test.package.vector.strip="debuginfo"override; assertions, optimization, features and repository configuration were unchangedWindows compilation and Windows-specific service tests/runtime remain unverified. Linux parser/helper tests do not establish Windows execution. Full/default-feature tests, full-workspace Clippy, aggregate formatting/Prettier, documentation/CUE and generated-documentation checks were not run. OS signal delivery and remote provider transport remain outside the reload-handler tests.
Two configuration-preprocessing limitations remain open: environment interpolation and secret lookup still occur before the root
data_diroverride. Baseline regressions reproduced errors when the discarded configured value references an unset variable or missing secret backend. Those regressions are retained separately; this revision does not change preprocessing semantics. Resolving this needs a compatibility decision for existing raw interpolation and YAML aliases/merges.Empty CLI/environment path values were already rejected by clap; they were not a reproduced CLI panic. The scoped service change handles fallible argument construction, including the pre-existing malformed-config-glob error path.
Does this PR include user facing changes?
Yes. A feature changelog fragment and CLI reference documentation are included.
These updates were prepared with OpenAI assistance. The exact model identifier was not available in this session. The PR author reviewed and approved the changes; remaining validation is listed above.