Skip to content

Add custom config files for local servers (#226)#231

Open
sdairs wants to merge 4 commits into
mainfrom
issue-226-custom-config-files
Open

Add custom config files for local servers (#226)#231
sdairs wants to merge 4 commits into
mainfrom
issue-226-custom-config-files

Conversation

@sdairs
Copy link
Copy Markdown
Collaborator

@sdairs sdairs commented Jun 3, 2026

Closes #226.

What

Adds support for custom ClickHouse config files for locally-managed servers:

  • A global config store at ~/.clickhouse/configs/. Drop named config files there.
  • clickhousectl local server start --config-file <NAME> applies one by name.
  • clickhousectl local server configs lists what's available (and prints the dir). Supports --json.
mkdir -p ~/.clickhouse/configs
cat > ~/.clickhouse/configs/analytics.xml <<'XML'
<clickhouse>
    <query_log><database>system</database><table>query_log</table></query_log>
</clickhouse>
XML

clickhousectl local server configs
clickhousectl local server start --config-file analytics

How it works (and why not literal --config-file)

The original code rejected --config-file because a custom config could redirect the data directory and break the managed lifecycle. While implementing this I verified ClickHouse's behaviour empirically:

  • Passing a partial file as --config-file replaces 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.
  • ClickHouse does merge a 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/ (as chctl-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-file clears 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_from tests for --config-file and the configs subcommand.
  • src/local/output.rs: JSON + display tests for the new output type.
  • Manually verified end-to-end against ClickHouse 26.6: overlay merges (displayName() reflects the override), defaults remain intact, data stays under .clickhouse/servers/<name>/data/, and restart-without-flag reverts.

cargo build, cargo test, and cargo clippy --workspace --all-targets all 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 as config.d/chctl-config.<ext> so partial overrides merge with built-in defaults. Data path and ports stay forced on the command line so list/stop/remove/dotenv behavior is unchanged; restarting without the flag clears the overlay.

New clickhousectl local server configs lists available files and the configs directory (supports --json). Trailing -- --config-file=... remains rejected, with messaging pointing at the new flag. Adds paths::configs_dir(), ConfigNotFound / InvalidConfigName errors, 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.

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>
@sdairs sdairs requested a review from iskakaushik as a code owner June 3, 2026 13:31
Comment thread crates/clickhousectl/src/local/config.rs
@sdairs sdairs linked an issue Jun 3, 2026 that may be closed by this pull request
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>
@sdairs
Copy link
Copy Markdown
Collaborator Author

sdairs commented Jun 3, 2026

fyi @dave-connors-3 @rodrigargar

Copy link
Copy Markdown
Collaborator

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

LGTM!

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>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread crates/clickhousectl/src/local/config.rs
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.

Config files for local servers enable system.query_log by default

2 participants