Add custom config files for local servers (#226)#231
Open
sdairs wants to merge 4 commits into
Open
Conversation
Let `clickhousectl local server start --config-file <NAME>` apply a custom ClickHouse config from a global store at `~/.clickhouse/configs/`, and add `local server configs` to list what's available. The named file is staged into the server's `config.d/` directory rather than passed as `--config-file`. ClickHouse merges `config.d/` on top of its built-in defaults, so a partial override file (e.g. just `<query_log>`) takes effect without replacing the whole config — passing it as `--config-file` would replace the embedded defaults and a partial file fails to start. The forced `--path=./` and port flags remain command-line overrides that win over the file, so the managed server lifecycle (list/stop/remove/dotenv) is preserved regardless of the file's contents. Restarting without `--config-file` clears the overlay. Name-only resolution from the configs dir (.xml/.yaml/.yml, extension optional); the trailing `-- --config-file=...` passthrough stays rejected and now points users at the new flag. Adds pure-logic unit tests for resolution/listing/overlay staging, clap parse tests for the new flag and subcommand, and `--json` output for the list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
resolve_config_in built paths with dir.join(name) and only checked is_file(), so --config-file could resolve outside ~/.clickhouse/configs/ via absolute paths or .. segments and copy those files into the server overlay. Add validate_config_name (mirrors validate_server_name) to reject path separators and .. before resolution, keeping the named-config store contract sound. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
Resolve conflicts in error.rs and local/mod.rs. Main's commit 93f7f74 ("Simplify JSON output mode; foreground server start ignores --json") deliberately changed --foreground to silently ignore --json rather than error, superseding this branch's JsonForegroundConflict approach. Drop the JsonForegroundConflict error variant and its test to align with main; keep the custom-config-file additions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 90455ca. Configure here.
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
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.

Closes #226.
What
Adds support for custom ClickHouse config files for locally-managed servers:
~/.clickhouse/configs/. Drop named config files there.clickhousectl local server start --config-file <NAME>applies one by name.clickhousectl local server configslists what's available (and prints the dir). Supports--json.How it works (and why not literal
--config-file)The original code rejected
--config-filebecause a custom config could redirect the data directory and break the managed lifecycle. While implementing this I verified ClickHouse's behaviour empirically:--config-filereplaces the embedded defaults — the server exits (code 180) because required sections are missing. So the intuitive "partial files merge" assumption does not hold for--config-file.config.d/directory found next to its working dir on top of its built-in defaults.So the named file is staged into the server's
config.d/(aschctl-config.<ext>) instead of being passed as--config-file. This gives the intended DX — a file with just<query_log>takes effect without reproducing a whole config — while the forced--path=./and port flags stay as command-line overrides that win over the file. The managed lifecycle (list/stop/remove/dotenv) is preserved regardless of the file's contents. Restarting without--config-fileclears the overlay and reverts to plain defaults.Name-only resolution (
.xml/.yaml/.yml, extension optional, ambiguous/missing names error helpfully). The trailing-- --config-file=...passthrough stays rejected and now points users at the new flag.Tests
src/local/config.rs: pure-logic unit tests for resolution (bare name, explicit ext, not-found, ambiguous) and overlay staging (extension preservation, clear-on-None, switch extension).src/local/cli.rs:Cli::try_parse_fromtests for--config-fileand theconfigssubcommand.src/local/output.rs: JSON + display tests for the new output type.displayName()reflects the override), defaults remain intact, data stays under.clickhouse/servers/<name>/data/, and restart-without-flag reverts.cargo build,cargo test, andcargo clippy --workspace --all-targetsall pass.Note
Low Risk
Changes are confined to local server startup and config file staging under ~/.clickhouse, with path validation and preserved managed data/ports; no cloud or auth impact.
Overview
Adds named custom ClickHouse configs for locally managed servers via a global store at
~/.clickhouse/configs/.clickhousectl local server start --config-file <NAME>resolves a name (optional.xml/.yaml/.yml, with clear errors for missing or ambiguous names) and stages the file into the server data dir asconfig.d/chctl-config.<ext>so partial overrides merge with built-in defaults. Data path and ports stay forced on the command line solist/stop/remove/dotenvbehavior is unchanged; restarting without the flag clears the overlay.New
clickhousectl local server configslists available files and the configs directory (supports--json). Trailing-- --config-file=...remains rejected, with messaging pointing at the new flag. Addspaths::configs_dir(),ConfigNotFound/InvalidConfigNameerrors, and path-traversal checks on config names.Reviewed by Cursor Bugbot for commit 90455ca. Bugbot is set up for automated code reviews on this repo. Configure here.