Commit Graph
18 Commits
Author SHA1 Message Date
Alexey MilovidovandGitHub 842b133f8d Merge pull request #100399 from ClickHouse/clang-tidy-init-variables
Enable clang-tidy check for uninitialized variables
2026-05-31 21:56:29 -07:00
Alexey MilovidovandClaude Opus 4.6 9013663756 Fix build failures from newly enabled warnings after merge with master
- Replace deprecated `throw()` with `noexcept` on `reallocarray` in `malloc.cpp`
- Add `static` to file-local functions: `percentile`, `emitTCPBufferPercentiles`,
  `updateHTTPConnectionPoolTCPBufferMetrics`, `logIcebergFileStats`,
  `getRequiredHeaderPositions`
- Add forward declarations for cross-TU functions: `registerDiskCache`,
  `getRandomizedStats`, `mainEntryClickHouseDockerInit`, `operator==`
- Suppress `-Wdisabled-macro-expansion` around `sa_handler` macro usage
  in `docker-init.cpp`

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-10 03:53:51 +00:00
Alexey MilovidovandClaude Opus 4.6 607951c83f Fix uninitialized variables after merge with master
Initialize variables flagged by `cppcoreguidelines-init-variables` and
`cppcoreguidelines-pro-type-member-init` clang-tidy checks in files
newly introduced or modified after merging master:

- `itoa.cpp`: initialize `r1`, `r2` output parameters
- `docker-init.cpp`: initialize `n`, `found`, `run_uid`, `run_gid`
- `Generator.h`, `Runner.h`: initialize `seed`, `thread_idx` members
- `HTTPConnectionPool.cpp`: extend NOLINT for `hicpp-member-init`
- `TCPSocketMemInfo.cpp`: zero-initialize C netlink structs before `memcpy`
- `JSONAllValues.cpp`: initialize `type_index`
- `stem.cpp`: initialize `upper_bound`
- `WriteHelpers.cpp`: initialize `bits` and `result` variables
- `Squashing.cpp`: initialize `bytes_to_take`
- `ExpressionListParsers.cpp`: initialize `max_value`
- `optimizeLazyMaterialization.cpp`: initialize `step` pointer to nullptr
- `MergeTreeIndexConditionText.cpp`: initialize `is_trivial`, `required_substring_is_prefix`

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-10 03:46:01 +00:00
Rahul 148a9da80b distroless: drop redundant static from functions in anonymous namespace 2026-03-19 21:41:44 -07:00
Rahul 66541a766b distroless: refactor initClickHouseDB into per-step functions
Per reviewer feedback: extract `createClickHouseDatabase` and
`runInitScripts` as separate static functions that return `false`
immediately on the first error. Replace the `init_ok` flag with
short-circuit `&&`. Server cleanup is always performed.

Matches the `set -e` fail-fast semantics of the original
`entrypoint.sh` more clearly.
2026-03-19 18:10:40 -07:00
Rahul de84b57526 distroless: fix clang-tidy dead-store in initClickHouseDB
`clang-analyzer-deadcode.DeadStores` flagged the `init_ok = false` writes
inside the init-script loop because `initClickHouseDB` returned `void` —
the flag was set but never read after the loop exited.

Change `initClickHouseDB` to return `bool`:
- early-skip paths (`database_exists`, no files, `shutdown_requested`)
  return `true`
- fork-failure and server-not-ready paths return `false`
- end of function returns `init_ok`

Caller checks the result and returns `1` on failure, matching the
fail-fast behaviour of the other init functions.
2026-03-18 16:01:34 -07:00
Rahul 61ead16e6c distroless: address code review — fail fast on init errors
- `manageClickHouseUser`: return `bool` instead of `void`; return `false`
  on invalid `CLICKHOUSE_USER`/`CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT` so
  the caller exits non-zero instead of silently starting with a wrong auth
  config.
- `createDirectoryAndChown`: check return value at all call sites (keeper
  dirs, server data/log/tmp/disk dirs) and `return 1` on failure.
- `initClickHouseDB`: check `runCommand`/`runPipeline` exit codes for
  `CREATE DATABASE` and every init script; stop on first failure (matches
  the `set -e` behavior of the original `entrypoint.sh`). Also fixes a
  use-after-free UB: the `runPipeline` `.sql.gz` call now uses a named
  `decompress_args` variable instead of a brace-initializer temporary.
2026-03-18 15:33:34 -07:00
RahulandClaude Sonnet 4.6 5097d999e9 fix: address code review issues in distroless Docker images
- `docker-init.cpp`: fix passthrough mode for short tool names ("client",
  "local", etc.) — prepend "clickhouse-" since distroless images only have
  "clickhouse-*" symlinks, not bare names
- `Dockerfile.distroless` (server, keeper): replace `printf '%b' ... \n ...`
  with individual `printf` calls per line to avoid portability ambiguity
- `Dockerfile.distroless` (keeper): add `clickhouse-keeper` to PACKAGES so
  the default `/etc/clickhouse-keeper/keeper_config.xml` is present in
  the image; copy it to /output during assembly

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
2026-03-16 07:59:13 -07:00
RahulandClaude Sonnet 4.6 b09815f161 distroless: fix .sql.gz quote escaping and clear supplementary groups
Fix 1: The `.sql.gz` filename escaping used backslash-escape (`\'`)
which is not valid ClickHouse SQL. Changed to doubled-quote (`''`)
which is the correct SQL escape for single quotes inside string
literals.

Fix 2: Added `setgroups(0, nullptr)` before `setgid`/`setuid` in the
fork+setuid directory creation path. This clears supplementary groups
before dropping privileges, preventing potential group-based privilege
leakage.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
2026-03-10 08:01:52 -07:00
RahulandClaude Opus 4.6 e79de9ee83 distroless: fix SQL injection, PID 1 signal handling, and NFS mkdir
- Escape single quotes in `.sql.gz` file paths before interpolating into
  the `file()` SQL function, preventing injection via crafted filenames
  in `/docker-entrypoint-initdb.d/`

- Install `SIGTERM`/`SIGINT` handlers before `initClickHouseDB` so that
  `docker stop` during the init phase triggers graceful shutdown instead
  of being silently dropped (Linux ignores unhandled signals for PID 1).
  The handler forwards the signal to the temporary init server and sets
  a flag that breaks the health-check polling loop.

- When `CLICKHOUSE_DO_NOT_CHOWN=1` and running as root, fork a child
  that drops to the target UID/GID before calling `fs::create_directories`.
  This matches the bash entrypoint behavior (`clickhouse su … mkdir`) and
  fixes directory creation on NFS mounts where root is mapped to nobody.
  Distroless has no `mkdir` binary, so the child uses the C++ filesystem
  library directly after `setuid`/`setgid`.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-09 08:21:11 -07:00
RahulandClaude Opus 4.6 4910118161 distroless: harden docker-init entrypoint
- Validate `CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT` is exactly "0" or "1"
  before embedding in XML, preventing injection via the env variable
- Handle EINTR in all `waitpid` calls so signals don't cause spurious
  child-process failure reports
- Use `ends_with` instead of manual `substr` for init-script file
  extension matching
- Use CTAD for `clickhouse_tools` array to avoid hardcoded size
- Remove deprecated `version` field from distroless docker-compose.yml

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-06 12:50:05 -08:00
Rahul 0f5054fb45 distroless: fix temp server startup — add -- before --listen_host
ClickHouse server uses Poco for CLI parsing. Options before "--" must
be registered; unregistered options trigger "Unknown option specified".
The "--" separator marks positional args, which ClickHouse treats as
config.xml overrides (as documented in the server --help output).

Without "--", starting the initdb temp server fails immediately:
  Unknown option specified: listen_host=127.0.0.1

This matches the pattern used in entrypoint.sh (line 191):
  clickhouse su USER:GROUP clickhouse-server --config-file=... -- --listen_host=127.0.0.1

Fixes: clickhouse-initdb-skip-user-setup [4/6] and
       clickhouse-override-tcp-port [6/6] Docker server image tests.
2026-03-05 21:43:20 -08:00
Rahul bdc211c8ea distroless: fix directory creation when running as non-root
When do_chown is false (running as uid 101, non-root), createDirectoryAndChown
was delegating to 'clickhouse su UID:GID mkdir -p DIR'. The distroless image
has no mkdir binary, so this always failed, silently leaving coordination
subdirectories and server runtime paths uncreated.

Since we are already running as the target user, fs::create_directories works
directly — no user-switch or external binary is needed. The NFS root-squash
concern that motivated the clickhouse-su-mkdir approach does not apply when
the process is already non-root.
2026-03-05 07:17:15 -08:00
Rahul 960c8d5197 distroless: fix clang-tidy modernize-use-starts-ends-with warning 2026-03-05 06:37:18 -08:00
Rahul e5d3c14edc distroless: fix override-cmd test hang by exec'ing non-option args directly
The docker-library `override-cmd` test runs:
  docker run "$image" echo "Hello $random"

With the ENTRYPOINT `["/usr/bin/clickhouse", "docker-init"]`, docker-init
receives `["echo", "Hello ..."]` as extra_args. Previously, `echo` was not
in the explicit passthrough list, so docker-init started the full server,
which ran indefinitely — causing a ~4.5-hour timeout in CI.

Fix: mirror `entrypoint.sh` behaviour: if the first extra argument does not
start with `--`, exec it directly without server startup. For recognized
ClickHouse tool names (client, local, etc.) the bin_dir path is used for
correct argv[0]-based dispatch; for generic commands (echo, date, bash, …)
PATH resolution via execvp handles it.
2026-03-04 18:38:55 -08:00
Rahul 27853cc34b distroless: fix config.xml permissions and client passthrough
Two bugs addressed:

1. `Dockerfile.distroless` — `cp -r` without `-p` preserves mode bits but
   resets ownership to root:root. `clickhouse install` sets `config.xml` to
   mode 0400 owned by `clickhouse:clickhouse`; after the copy the file becomes
   `root:root 0400`, which uid 101 cannot read (errno 13 EACCES). Fix: expand
   the `chown -R clickhouse:clickhouse` target from just `users.d` to the
   entire `/output/etc/clickhouse-server` tree.

2. `docker-init.cpp` — the official Docker-library test suite runs client
   queries by spawning a second container from the same image:
     `docker run "$image" clickhouse-client --host ... --query ...`
   Because the distroless ENTRYPOINT is `clickhouse docker-init`, the
   client tool name and its arguments land in `extra_args` and were being
   forwarded to `clickhouse-server`, causing "Unknown option specified" errors.
   Fix: before any server-startup logic, detect a recognised client-side
   subcommand as the first extra arg (`clickhouse-client`, `clickhouse-local`,
   `clickhouse-keeper-client`, and their short forms) and exec it directly
   via its `/usr/bin/` symlink, bypassing server initialisation entirely.
2026-03-04 11:53:40 -08:00
Rahul 17b77fd0bd Fix style and clang-tidy violations in docker-init.cpp
- Replace `#include <signal.h>` with `#include <csignal>` (deprecated C header)
- Remove `#include <sstream>`; replace `std::istringstream` with manual newline split
- Wrap all `close()`/`dup2()` calls with `(void)` (return value must be acknowledged)
- Split `int lhs_status = 0, rhs_status = 0;` into two declarations
  (readability-isolate-declaration)
- Replace loop-runs-once `has_init_files` detection with iterator comparison
  (clang-diagnostic-unreachable-code-loop-increment)
- Add `// NOLINT(concurrency-mt-unsafe)` to `strerror` and `sleep` call sites
- Rename `initClickHouseDb` → `initClickHouseDB` (abbreviations must be all-caps)
2026-03-03 12:03:20 -08:00
Rahul 739b60df50 Add distroless Docker image variant for server and keeper
- Add `clickhouse docker-init` C++ subcommand that replaces `entrypoint.sh`
  for shell-free distroless environments (fork/exec, POSIX-only, no shell deps)
- Add `docker/server/Dockerfile.distroless` and `docker/keeper/Dockerfile.distroless`
  as parallel build targets alongside existing ubuntu/alpine variants
- Add `--no-distroless` CI flag and fix package selector to use `.deb` for distroless
- Add structural smoke-test harness under `docker/tests/distroless/`

The distroless images use `gcr.io/distroless/cc-debian12:nonroot` as base,
expose the same ports as ubuntu, run as uid/gid 101, and include a debug
variant (`distroless-debug`) with a busybox shell for troubleshooting.
2026-03-03 11:45:23 -08:00