diff --git a/CHANGELOG.md b/CHANGELOG.md index 30d67f1..029eace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -140,6 +140,15 @@ but container releases use the upstream-derived format documented in - Fixed a race in the reload test that read `/proc/PID/status` for a worker that had already exited, which aborted the listing instead of skipping the vanished process. +- Added a qualified preview ClickHouse HTTP proxying profile that bounds + methods, body size, timeouts, and temporary storage, reports failures by + exception code rather than by statement, and records `NONE` upstream fields + for a request the proxy refused; tests prove a password and column name sent + as request parameters reach neither the access stream nor the error stream. +- Stated that the ClickHouse proxy is not an authorization boundary, because + NGINX parses HTTP rather than SQL and cannot enforce read-only access, + restrict statements, or bound returned rows; those remain ClickHouse + `readonly` settings, quotas, row policies, and grants. - Added a qualified preview health-endpoint profile separating upstream-independent liveness from upstream-reflecting readiness, logging only failing probes, and exposing `stub_status` on a separate listener that diff --git a/docs/CONFIGURATION-PROFILES.md b/docs/CONFIGURATION-PROFILES.md index 6993499..ecaa0b3 100644 --- a/docs/CONFIGURATION-PROFILES.md +++ b/docs/CONFIGURATION-PROFILES.md @@ -1,10 +1,10 @@ # Qualified HTTP and TLS configuration profiles The repository provides minimal static-serving, HTTP reverse-proxy, HTTP -load-balancing, WebSocket-proxying, request-limiting, and health-endpoint -configurations under `examples/profiles`. Static serving, HTTP reverse proxy, -HTTP load balancing, WebSocket proxying, request and connection limiting, -health and readiness endpoints, +load-balancing, WebSocket-proxying, request-limiting, health-endpoint, and +ClickHouse-proxying configurations under `examples/profiles`. Static serving, +HTTP reverse proxy, HTTP load balancing, WebSocket proxying, request and +connection limiting, health and readiness endpoints, ClickHouse HTTP proxying, TLS termination, mutual TLS, and verified HTTPS upstream profiles are exercised on native AMD64 and ARM64 runners with rootless Podman and then with Docker compatibility execution. They remain **preview/unqualified** until an immutable @@ -62,6 +62,81 @@ Its access-event schema is: | `body_bytes_sent` | integer | Response-body bytes sent. | | `request_time` | number | Total request duration in seconds. | +## ClickHouse HTTP proxying + +[`examples/profiles/clickhouse/nginx.conf`](../examples/profiles/clickhouse/nginx.conf) +proxies the ClickHouse HTTP interface. + +### This proxy is not an authorization boundary + +NGINX parses HTTP, not SQL. It cannot make a session read-only, restrict which +statements run, bound how many rows a query returns, or tell a `SELECT` from a +`DROP`. + +Anything that depends on the content of a query belongs to ClickHouse: +`readonly` user settings, quotas, row policies, and per-user grants. Treating +this file as the control that stops clients modifying data is a misreading of +what it does. The profile restricts methods and body size, which bounds the +shape of a request, not its meaning. + +### Credentials and query text in the request target + +This is the profile where the repository-wide logging rule earns its keep. + +The ClickHouse HTTP interface accepts `?query=...`, and many deployments accept +`?user=...&password=...`. The request target therefore routinely carries both +credentials and customer data. Every profile here logs `$uri` and never +`$request`, `$request_uri`, or `$args`, so passwords and query text are +structurally absent from the access stream rather than filtered out of it. + +The test asserts this directly: it issues a request carrying a password and a +column name as parameters, then requires that neither string appears anywhere +in the container's output. + +An operator should confirm the same property for anything downstream. A log +collector, an error tracker, or a reverse proxy in front that records full +request lines reintroduces exactly what this profile removes. + +### Failures are reported by code, not by statement + +`X-ClickHouse-Exception-Code` is recorded as a structured field, which gives +the failure class without putting any part of the query into the log. A request +the proxy refuses never reaches the database, so its upstream fields record +`NONE`, keeping "the database rejected this" distinguishable from "the database +never saw it". + +### Temporary storage + +`proxy_max_temp_file_size 0` with buffering off is deliberate. A result set +larger than the proxy buffers would otherwise spill to `proxy_temp_path`, which +is on the container's `/tmp` tmpfs. One large result could consume the tmpfs +and take the container down, so the response is streamed instead. + +### Timeouts and capacity + +Analytical queries run far longer than web requests, so the read timeout is +large and scoped to the query location. Each waiting query holds a worker +connection, so size `worker_connections` for the expected concurrent query +count rather than for request rate. Retries are disabled: re-running a +statement is unsafe for anything that writes and wasteful for anything else. + +### Qualified behaviour + +The tests run against a stand-in for the ClickHouse HTTP interface built from +this image. It reports what the proxy forwarded and can answer with an +exception code; it executes nothing and implements no part of the ClickHouse +protocol. + +They prove that a POST body reaches the upstream, that credentials and query +text in the request target never reach the log, that an exception code is +recorded, that methods outside the interface are refused, and that an oversized +body is refused before reaching the database. + +They do **not** qualify interoperation with a real ClickHouse server, protocol +or version behaviour, query semantics, compression negotiation, or performance +against real result sets. A deployment must qualify those against its own +server. + ## Extended health and readiness endpoints [`examples/profiles/health/nginx.conf`](../examples/profiles/health/nginx.conf) diff --git a/docs/LOGGING.md b/docs/LOGGING.md index e3338f6..559f811 100644 --- a/docs/LOGGING.md +++ b/docs/LOGGING.md @@ -84,9 +84,9 @@ and [`container/conf.d/default.conf`](../container/conf.d/default.conf). ## Qualified preview HTTP and TLS formats The static, HTTP reverse-proxy, HTTP load-balancing, WebSocket-proxying, -request-limiting, health-endpoint, TLS termination, mutual-TLS, and -verified-upstream examples implement the safer structured contract described -above. +request-limiting, health-endpoint, ClickHouse-proxying, TLS termination, +mutual-TLS, and verified-upstream examples implement the safer structured +contract described above. They use JSON escaping, validate a bounded `X-Request-ID` or generate `$request_id`, and log `$uri` rather than the query- bearing request target. Tests parse every emitted access event, exercise JSON diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 255e1cb..8f9b6d5 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -70,12 +70,10 @@ parallel only where it does not assume an unfrozen NGINX package or module set. ## Package 3: supported configurations and TLS -- [ ] Provide a tested, minimal example for ClickHouse HTTP proxying. - [ ] Extend the qualified defaults to DNS resolution, upstream verification, and the remaining profiles without silently breaking general-purpose use. - [ ] Document configuration mounting, validation, reload, rollback, logging, troubleshooting, and secret redaction. -- [ ] Implement and test structured logging for ClickHouse. - [ ] Qualify runtime collection, rotation ownership, pipeline failure, and retention evidence for the selected logging platform. - [ ] Qualify lifecycle-alert delivery and exact platform cryptographic-policy diff --git a/docs/SUPPORT.md b/docs/SUPPORT.md index 17820ba..4d75148 100644 --- a/docs/SUPPORT.md +++ b/docs/SUPPORT.md @@ -31,6 +31,7 @@ Absence from a matrix means unqualified, not implicitly compatible. | WebSocket proxying profile | Preview/unqualified | Upgrade forwarding, derived connection disposition, `101` relay, and unaffected plain HTTP are tested; frame exchange, session duration, and concurrent session capacity are not. | | Request and connection limiting profile | Preview/unqualified | Rate rejection, connection rejection, bandwidth pacing, and unlimited health checks are tested; workload tuning, zone sizing, and exhausted-zone behaviour are not. | | Health and readiness profile | Preview/unqualified | Upstream-independent liveness, upstream-reflecting readiness, failure-only probe logging, and a default-deny status surface are tested; probe tuning, startup probes, and an allow-listed status surface are not. | +| ClickHouse HTTP proxying profile | Preview/unqualified | Method and body bounds, credential and query-text exclusion from logs, exception-code reporting, and bounded temporary storage are tested against a stand-in; interoperation with a real ClickHouse server is not qualified. | | TLS termination and mTLS profiles | Preview/unqualified | TLS 1.2/1.3, client authentication, leaf renewal, CRL enforcement, and negative cases are tested; production PKI operations and exact-host cryptographic policy remain unqualified. | | Verified HTTPS upstream profile | Preview/unqualified | Chain, hostname, SNI, revocation, overlapping-CA rotation, and restricted-runtime behavior are tested; deployment DNS, egress, and PKI remain operator-owned. | | Linux AMD64 and ARM64 | Preview/unqualified | Native CI exists; release-candidate evidence is not complete. | diff --git a/docs/USE-CASES.md b/docs/USE-CASES.md index d4308e9..1fc0f9d 100644 --- a/docs/USE-CASES.md +++ b/docs/USE-CASES.md @@ -8,9 +8,9 @@ against the released image. The development image has tested preview profiles for static content, HTTP and verified-HTTPS upstream proxying, HTTP load balancing, WebSocket proxying, -request and connection limiting, health and readiness endpoints, TLS -termination, and mutual TLS, including structured logging on unprivileged -ports. The other +request and connection limiting, health and readiness endpoints, ClickHouse +HTTP proxying, TLS termination, and mutual TLS, including structured logging on +unprivileged ports. The other profiles below are design targets for the first release unless stated otherwise. See [Qualified HTTP and TLS configuration profiles](CONFIGURATION-PROFILES.md) for the exact implemented boundary. diff --git a/examples/profiles/clickhouse/nginx.conf b/examples/profiles/clickhouse/nginx.conf new file mode 100644 index 0000000..4f1ec35 --- /dev/null +++ b/examples/profiles/clickhouse/nginx.conf @@ -0,0 +1,176 @@ +worker_processes auto; +pid /tmp/nginx.pid; +error_log /dev/stderr notice; + +events { + worker_connections 1024; +} + +http { + include /etc/nginx/mime.types; + default_type application/octet-stream; + + map $http_x_request_id $correlation_id { + "~^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$" $http_x_request_id; + default $request_id; + } + + # A connection that never produced a request has no method: a failed TLS + # handshake, a malformed request line, or a client that disconnects before + # the request is read. Those are not application requests, so they stay in + # the error stream instead of emitting a structured access event with empty + # fields. + map $request_method $is_application_request { + default 1; + "" 0; + } + + # ClickHouse reports failures with a numeric exception code in a response + # header. Recording it gives operators the failure class without putting + # any part of the query text into the log. + map $upstream_http_x_clickhouse_exception_code $clickhouse_exception_code { + default $upstream_http_x_clickhouse_exception_code; + "" NONE; + } + + # A request the proxy refuses never reaches an upstream, so every upstream + # variable is empty. Naming that case keeps each field in the access event + # non-empty and keeps "the database rejected this" distinguishable from + # "the database never saw it", which is the difference between a query + # problem and a gateway problem. + map $upstream_addr $upstream_addr_value { + default $upstream_addr; + "" NONE; + } + + map $upstream_status $upstream_status_value { + default $upstream_status; + "" NONE; + } + + map $upstream_connect_time $upstream_connect_time_value { + default $upstream_connect_time; + "" NONE; + } + + map $upstream_header_time $upstream_header_time_value { + default $upstream_header_time; + "" NONE; + } + + map $upstream_response_time $upstream_response_time_value { + default $upstream_response_time; + "" NONE; + } + + # The access event records $uri and never $request, $request_uri, or $args. + # + # That rule matters more here than anywhere else in this repository. The + # ClickHouse HTTP interface accepts `?query=...` and, on many deployments, + # `?user=...&password=...`, so the request target routinely carries both + # credentials and customer data. Logging the request line would write + # passwords and query text into the access stream, from where they spread + # into collectors, retention, and backups. + log_format profile_json escape=json + '{"timestamp":"$time_iso8601",' + '"request_id":"$correlation_id",' + '"method":"$request_method",' + '"uri":"$uri",' + '"protocol":"$server_protocol",' + '"status":$status,' + '"body_bytes_sent":$body_bytes_sent,' + '"request_time":$request_time,' + '"clickhouse_exception_code":"$clickhouse_exception_code",' + '"upstream_addr":"$upstream_addr_value",' + '"upstream_status":"$upstream_status_value",' + '"upstream_connect_time":"$upstream_connect_time_value",' + '"upstream_header_time":"$upstream_header_time_value",' + '"upstream_response_time":"$upstream_response_time_value"}'; + access_log /dev/stdout profile_json if=$is_application_request; + + client_body_temp_path /tmp/nginx-client-body; + proxy_temp_path /tmp/nginx-proxy; + fastcgi_temp_path /tmp/nginx-fastcgi; + uwsgi_temp_path /tmp/nginx-uwsgi; + scgi_temp_path /tmp/nginx-scgi; + + server_tokens off; + client_body_timeout 10s; + client_header_timeout 10s; + keepalive_timeout 30s; + send_timeout 30s; + + # Replace this endpoint with the deployment's approved service name. + upstream clickhouse_backend { + server backend:8123; + keepalive 8; + } + + server { + listen 8080 default_server; + listen [::]:8080 default_server; + server_name _; + + add_header X-Content-Type-Options nosniff always; + add_header X-Request-ID $correlation_id always; + + # This proxy is not an authorization boundary. + # + # NGINX parses HTTP, not SQL. It cannot make a session read-only, + # restrict which statements run, bound how many rows a query returns, + # or tell a SELECT from a DROP. Anything that depends on the content of + # a query belongs to ClickHouse: `readonly` user settings, quotas, row + # policies, and per-user grants. Treating this file as the control that + # keeps clients from modifying data is a misreading of what it does. + location = /query { + proxy_pass http://clickhouse_backend/; + proxy_http_version 1.1; + proxy_set_header Connection ""; + proxy_set_header Host $proxy_host; + proxy_set_header X-Forwarded-For $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Request-ID $correlation_id; + + # Queries are sent as a request body. Inserts need a deliberately + # larger value; raise it with a stated reason rather than removing + # the bound, because an unbounded body lets one client occupy + # temporary storage on the way to the database. + client_max_body_size 10m; + + # Analytical queries run far longer than web requests. The timeout + # is scoped to this location so an ordinary endpoint cannot inherit + # it, and each waiting query still holds a worker connection, so + # size `worker_connections` for the expected concurrent query count + # rather than for request rate. + proxy_connect_timeout 5s; + proxy_send_timeout 300s; + proxy_read_timeout 300s; + + # A result set larger than the proxy buffers would otherwise spill + # to `proxy_temp_path`, which is on the container's `/tmp` tmpfs. + # A single large result could then consume the tmpfs and take the + # container down with it. Streaming the response to the client + # instead keeps memory and temporary storage bounded. + proxy_max_temp_file_size 0; + proxy_buffering off; + + # A retry would re-execute the statement. That is unsafe for + # anything that writes, and wasteful for anything that does not. + proxy_next_upstream off; + + limit_except GET HEAD POST { + deny all; + } + } + + location = /healthz { + access_log off; + default_type text/plain; + return 200 "ok\n"; + } + + location / { + return 404; + } + } +} diff --git a/tests/fixtures/profile-clickhouse/nginx.conf b/tests/fixtures/profile-clickhouse/nginx.conf new file mode 100644 index 0000000..e8f5159 --- /dev/null +++ b/tests/fixtures/profile-clickhouse/nginx.conf @@ -0,0 +1,42 @@ +worker_processes 1; +pid /tmp/nginx.pid; +error_log /dev/stderr notice; + +events { + worker_connections 128; +} + +http { + access_log off; + client_body_temp_path /tmp/nginx-client-body; + proxy_temp_path /tmp/nginx-proxy; + fastcgi_temp_path /tmp/nginx-fastcgi; + uwsgi_temp_path /tmp/nginx-uwsgi; + scgi_temp_path /tmp/nginx-scgi; + + server { + listen 8123; + server_name _; + + client_max_body_size 32m; + + # This fixture stands in for the ClickHouse HTTP interface. It reports + # what the proxy forwarded and can return an exception code on demand, + # which covers the part the proxy profile owns. It executes nothing and + # implements no part of the ClickHouse protocol. + # + # A request carrying `fail=1` answers like a ClickHouse error so the + # proxy's handling of the exception-code header can be exercised. + location / { + default_type application/json; + + if ($arg_fail = 1) { + add_header X-ClickHouse-Exception-Code 241 always; + return 500 '{"error":"stand-in exception"}'; + } + + add_header X-ClickHouse-Summary '{"read_rows":"0"}' always; + return 200 '{"request_id":"$http_x_request_id","method":"$request_method","forwarded_proto":"$http_x_forwarded_proto","content_length":"$http_content_length"}\n'; + } + } +} diff --git a/tests/profiles.sh b/tests/profiles.sh index 1405bd9..144e70e 100644 --- a/tests/profiles.sh +++ b/tests/profiles.sh @@ -28,12 +28,18 @@ limited="${prefix}-limited" health_network="${prefix}-health-network" health_backend="${prefix}-health-backend" health_proxy="${prefix}-health-proxy" +# The ClickHouse stand-in also answers to a generic `backend` service name. +clickhouse_network="${prefix}-clickhouse-network" +clickhouse_backend="${prefix}-clickhouse-backend" +clickhouse_proxy="${prefix}-clickhouse-proxy" tmp_root="${PROFILE_TMPDIR:-/tmp}" static_headers=$(mktemp "${tmp_root}/nginx-static-headers.XXXXXX") proxy_headers=$(mktemp "${tmp_root}/nginx-proxy-headers.XXXXXX") proxy_body=$(mktemp "${tmp_root}/nginx-proxy-body.XXXXXX") balancer_body=$(mktemp "${tmp_root}/nginx-balancer-body.XXXXXX") ws_body=$(mktemp "${tmp_root}/nginx-ws-body.XXXXXX") +clickhouse_body=$(mktemp "${tmp_root}/nginx-clickhouse-body.XXXXXX") +clickhouse_payload=$(mktemp "${tmp_root}/nginx-clickhouse-payload.XXXXXX") limited_root=$(mktemp -d "${tmp_root}/nginx-limited-root.XXXXXX") no_new_privileges="no-new-privileges:true" @@ -46,12 +52,15 @@ cleanup() { "${pool_a}" "${pool_b}" "${balancer}" \ "${ws_backend}" "${ws_proxy}" "${limited}" \ "${health_backend}" "${health_proxy}" \ + "${clickhouse_backend}" "${clickhouse_proxy}" \ >/dev/null 2>&1 || true "${runtime}" network rm "${network}" >/dev/null 2>&1 || true "${runtime}" network rm "${ws_network}" >/dev/null 2>&1 || true "${runtime}" network rm "${health_network}" >/dev/null 2>&1 || true + "${runtime}" network rm "${clickhouse_network}" >/dev/null 2>&1 || true rm -f -- "${static_headers}" "${proxy_headers}" "${proxy_body}" \ - "${balancer_body}" "${ws_body}" + "${balancer_body}" "${ws_body}" "${clickhouse_body}" \ + "${clickhouse_payload}" rm -rf -- "${limited_root}" } trap cleanup EXIT @@ -648,13 +657,99 @@ if grep -Fq '"uri":"/healthz"' <<< "${health_logs}"; then exit 1 fi +# --------------------------------------------------------------------------- +# ClickHouse HTTP proxying +# +# The fixture stands in for the ClickHouse HTTP interface. It reports what the +# proxy forwarded and can answer with an exception code; it executes nothing. +# This qualifies the proxy's request handling and credential hygiene, not +# ClickHouse protocol behaviour. +# --------------------------------------------------------------------------- + +"${runtime}" network create "${clickhouse_network}" >/dev/null + +run_restricted "${clickhouse_backend}" 10012 \ + --network "${clickhouse_network}" \ + --network-alias backend \ + --volume "${script_dir}/fixtures/profile-clickhouse/nginx.conf:/etc/nginx/nginx.conf:ro" +"${runtime}" exec "${clickhouse_backend}" nginx -t -q -c /etc/nginx/nginx.conf +assert_process_security "${clickhouse_backend}" + +run_restricted "${clickhouse_proxy}" 10013 \ + --network "${clickhouse_network}" \ + --publish 127.0.0.1::8080 \ + --volume "${examples_dir}/clickhouse/nginx.conf:/etc/nginx/nginx.conf:ro" +clickhouse_binding=$("${runtime}" port "${clickhouse_proxy}" 8080/tcp) +clickhouse_port=${clickhouse_binding##*:} +clickhouse_url="http://127.0.0.1:${clickhouse_port}" +wait_for_http "${clickhouse_url}/healthz" "${clickhouse_proxy}" +assert_process_security "${clickhouse_proxy}" +"${runtime}" exec "${clickhouse_proxy}" nginx -t -q -c /etc/nginx/nginx.conf + +# A query sent as a POST body must reach the upstream unchanged. +printf 'SELECT 1' > "${clickhouse_payload}" +curl --fail --silent --show-error \ + --header 'X-Request-ID: clickhouse.query-1' \ + --data-binary "@${clickhouse_payload}" \ + --output "${clickhouse_body}" \ + "${clickhouse_url}/query" +"${python}" -c \ + 'import json, sys +payload = json.load(open(sys.argv[1], encoding="utf-8")) +assert payload["request_id"] == "clickhouse.query-1", payload +assert payload["method"] == "POST", payload' \ + "${clickhouse_body}" + +# The ClickHouse HTTP interface accepts credentials and query text as request +# parameters, so this is the case that matters most: neither may reach the log. +curl --fail --silent --show-error --output "${null_device}" \ + --header 'X-Request-ID: clickhouse.secret-1' \ + "${clickhouse_url}/query?user=analytics&password=do-not-log-this&query=SELECT%20secret_column" + +# An upstream failure must be reported by its exception code, never by echoing +# the statement that produced it. +test "$(curl --silent --output "${null_device}" --write-out '%{http_code}' \ + --header 'X-Request-ID: clickhouse.failure-1' \ + "${clickhouse_url}/query?fail=1")" = 500 + +# Methods outside the interface are refused by the proxy. +test "$(curl --silent --output "${null_device}" --write-out '%{http_code}' \ + --request DELETE \ + "${clickhouse_url}/query")" = 403 + +# A body beyond the configured bound is refused before it reaches the database. +head -c 11534336 /dev/zero | tr '\0' 'x' > "${clickhouse_payload}" +test "$(curl --silent --output "${null_device}" --write-out '%{http_code}' \ + --header 'X-Request-ID: clickhouse.oversize-1' \ + --data-binary "@${clickhouse_payload}" \ + "${clickhouse_url}/query")" = 413 + +validate_event "${clickhouse_proxy}" \ + --profile clickhouse \ + --uri /query \ + --request-id clickhouse.query-1 \ + --status 200 \ + --method POST \ + --require-field clickhouse_exception_code=NONE \ + --forbidden do-not-log-this \ + --forbidden secret_column \ + --forbidden password= + +validate_event "${clickhouse_proxy}" \ + --profile clickhouse \ + --uri /query \ + --request-id clickhouse.failure-1 \ + --status 500 \ + --require-field clickhouse_exception_code=241 + "${runtime}" stop --time 10 "${static}" "${proxy}" "${balancer}" "${ws_proxy}" \ - "${limited}" "${health_proxy}" >/dev/null + "${limited}" "${health_proxy}" "${clickhouse_proxy}" >/dev/null test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${static}")" = 0 test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${proxy}")" = 0 test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${balancer}")" = 0 test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${ws_proxy}")" = 0 test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${limited}")" = 0 test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${health_proxy}")" = 0 +test "$("${runtime}" inspect --format '{{.State.ExitCode}}' "${clickhouse_proxy}")" = 0 -echo "Static, reverse-proxy, load-balancer, websocket, rate-limited, and health profile qualification passed for ${image}" +echo "Static, reverse-proxy, load-balancer, websocket, rate-limited, health, and ClickHouse profile qualification passed for ${image}" diff --git a/tests/test_profile_logs.py b/tests/test_profile_logs.py index 3a012b2..2b86a38 100644 --- a/tests/test_profile_logs.py +++ b/tests/test_profile_logs.py @@ -261,6 +261,38 @@ def test_repeated_identity_is_rejected_unless_allowed(self) -> None: repeated, ) + def test_clickhouse_requires_a_numeric_or_absent_exception_code(self) -> None: + upstream = { + "upstream_addr": "10.0.0.1:8123", + "upstream_status": "200", + "upstream_connect_time": "0.001", + "upstream_header_time": "0.002", + "upstream_response_time": "0.003", + } + for code in ("NONE", "241", "0"): + with self.subTest(code=code): + valid = event(clickhouse_exception_code=code, **upstream) + self.assertEqual( + logs.parse_events(encoded(valid), "clickhouse"), [valid] + ) + + def test_clickhouse_rejects_an_unvalidated_exception_header(self) -> None: + # The value comes from an upstream response header, so anything that is + # neither digits nor the explicit absent token means it reached the log + # without being checked. + upstream = { + "upstream_addr": "10.0.0.1:8123", + "upstream_status": "200", + "upstream_connect_time": "0.001", + "upstream_header_time": "0.002", + "upstream_response_time": "0.003", + } + for code in ("", "none", "241; DROP", "Exception 241"): + with self.subTest(code=code): + invalid = event(clickhouse_exception_code=code, **upstream) + with self.assertRaises(logs.ProfileLogError): + logs.parse_events(encoded(invalid), "clickhouse") + def test_exactly_one_scenario_event_is_required(self) -> None: observed = [event(), event(request_id="other")] self.assertEqual( diff --git a/tests/validate_profile_logs.py b/tests/validate_profile_logs.py index c4f2665..02b413b 100644 --- a/tests/validate_profile_logs.py +++ b/tests/validate_profile_logs.py @@ -29,6 +29,7 @@ "upstream_response_time", } WEBSOCKET_FIELDS = {"connection_upgrade"} +CLICKHOUSE_FIELDS = {"clickhouse_exception_code"} LIMIT_FIELDS = {"limit_req_result", "limit_conn_result"} # NGINX reports these outcomes; the profile maps the empty value, which means # the limit was not evaluated for that location, onto an explicit token. @@ -55,6 +56,7 @@ "websocket", "rate-limited", "health", + "clickhouse", "tls-termination", "mutual-tls", "tls-upstream", @@ -119,15 +121,18 @@ def parse_events( "load-balancer", "websocket", "health", + "clickhouse", "tls-upstream", } websocket = profile == "websocket" + clickhouse = profile == "clickhouse" limited = profile == "rate-limited" tls = profile in {"tls-termination", "mutual-tls"} fields = ( COMMON_FIELDS | (UPSTREAM_FIELDS if upstream else set()) | (WEBSOCKET_FIELDS if websocket else set()) + | (CLICKHOUSE_FIELDS if clickhouse else set()) | (LIMIT_FIELDS if limited else set()) | (TLS_FIELDS if tls else set()) ) @@ -154,6 +159,13 @@ def parse_events( # the log or the map was changed without updating this contract. if event["connection_upgrade"] not in {"upgrade", "close"}: fail("connection_upgrade must be 'upgrade' or 'close'") + if clickhouse: + code = event["clickhouse_exception_code"] + # Either a numeric ClickHouse exception code or the explicit + # "no exception" token. Anything else means the upstream header + # reached the log unvalidated. + if code != "NONE" and not (isinstance(code, str) and code.isdigit()): + fail("clickhouse_exception_code must be digits or NONE") if limited: if event["limit_req_result"] not in LIMIT_REQ_RESULTS: fail("limit_req_result is not a recognised limit outcome") @@ -231,6 +243,14 @@ def main() -> int: parser.add_argument("--request-id", required=True) parser.add_argument("--status", required=True, type=int) parser.add_argument("--forbidden", action="append", default=[]) + parser.add_argument( + "--method", + default="GET", + help=( + "request method the matching event must carry; a scenario that " + "sends a body states it explicitly" + ), + ) parser.add_argument( "--connection-upgrade", choices=("upgrade", "close"), @@ -278,8 +298,8 @@ def main() -> int: def unmet(event: dict[str, object]) -> str | None: """Return why this event fails the assertions, or None if it passes.""" - if event["method"] != "GET": - return "matching event has an unexpected request method" + if event["method"] != args.method: + return f"expected method {args.method}; observed {event['method']}" if args.connection_upgrade is not None: if "connection_upgrade" not in event: return "profile does not record a connection disposition"