Add logviewer: web UI for the ClickHouse log store #1

Merged

benvin merged 3 commits from benvin/initial into main 2026-08-23 17:20:33 +10:00

Conversation 3 Commits 3 Files Changed 22

+1642 -1

unkin-agent commented 2026-08-23 16:39:48 +10:00

Member

Copy Link

Copy Source

Why

The vector -> JetStream -> ClickHouse logging stack has no UI: exploring logs.raw means hand-writing clickhouse-client queries, and at ~281M rows/day with no text index a careless query is expensive. logviewer gives a browsable, tail-able web view with guardrails baked in, sitting behind oauth2-proxy at logviewer.unkin.net.

Changes

• Adds a single-binary Go web app: stdlib-only server, go:embed UI, vendored Bootstrap 3.4.1 + jQuery 3.7.1 (no CDN), mediamover conventions
• Queries ClickHouse over HTTP as the readonly logreader user (CH_URL/CH_USER/CH_PASSWORD); the app does no auth itself — oauth2-proxy is the boundary
• Enforces time bounds server-side: missing since/until default to a 15m window ending now, windows wider than 72h (table TTL) are rejected, so no request can run unbounded
• Binds every filter value as a ClickHouse param_* query parameter; sets max_execution_time=30 on every query
• Supports a raw SQL WHERE fragment (monospace box in the UI) that is parenthesised and ANDed inside the parameterized outer query with the enforced bounds and LIMIT (max 1000) still applied; safety relies on the readonly user
• Adds GET /api/query (filters: namespace/host/pod/container/app/severity/stream/source, fuzzy q, sql, limit/offset), GET /api/tail (exclusive ms cursor, clamped to 15m, ascending), GET /api/facets (top 20 namespaces/apps/hosts in window), GET /healthz (CH ping)
• UI: time-range picker (15m/1h/6h/1d/custom), facet dropdowns, debounced fuzzy search, toggleable SQL box, paged severity-coloured log table with expandable label/field rows, Tail toggle with 1s polling, autoscroll and pause-on-scroll-up
• Adds Dockerfile (multi-stage, distroless nonroot), Makefile with patch/minor/major tag helpers, and Woodpecker build/test + tag-driven image push to artifactapi.k8s.syd1.au.unkin.net/docker-internal/logviewer (CA-baked buildx plugin, k8s resources set)
• Adds mocked-ClickHouse tests covering bounds enforcement and unbounded rejection, parameter binding, sql-fragment wrapping + LIMIT, tail cursor advance/clamp, facets grouping, healthz; go test -race, vet and gofmt clean

## Why The vector -> JetStream -> ClickHouse logging stack has no UI: exploring `logs.raw` means hand-writing clickhouse-client queries, and at ~281M rows/day with no text index a careless query is expensive. logviewer gives a browsable, tail-able web view with guardrails baked in, sitting behind oauth2-proxy at logviewer.unkin.net. ## Changes - Adds a single-binary Go web app: stdlib-only server, `go:embed` UI, vendored Bootstrap 3.4.1 + jQuery 3.7.1 (no CDN), mediamover conventions - Queries ClickHouse over HTTP as the readonly `logreader` user (`CH_URL`/`CH_USER`/`CH_PASSWORD`); the app does no auth itself — oauth2-proxy is the boundary - Enforces time bounds server-side: missing since/until default to a 15m window ending now, windows wider than 72h (table TTL) are rejected, so no request can run unbounded - Binds every filter value as a ClickHouse `param_*` query parameter; sets `max_execution_time=30` on every query - Supports a raw SQL WHERE fragment (monospace box in the UI) that is parenthesised and ANDed inside the parameterized outer query with the enforced bounds and LIMIT (max 1000) still applied; safety relies on the readonly user - Adds `GET /api/query` (filters: namespace/host/pod/container/app/severity/stream/source, fuzzy `q`, `sql`, limit/offset), `GET /api/tail` (exclusive ms cursor, clamped to 15m, ascending), `GET /api/facets` (top 20 namespaces/apps/hosts in window), `GET /healthz` (CH ping) - UI: time-range picker (15m/1h/6h/1d/custom), facet dropdowns, debounced fuzzy search, toggleable SQL box, paged severity-coloured log table with expandable label/field rows, Tail toggle with 1s polling, autoscroll and pause-on-scroll-up - Adds Dockerfile (multi-stage, distroless nonroot), Makefile with patch/minor/major tag helpers, and Woodpecker build/test + tag-driven image push to `artifactapi.k8s.syd1.au.unkin.net/docker-internal/logviewer` (CA-baked buildx plugin, k8s resources set) - Adds mocked-ClickHouse tests covering bounds enforcement and unbounded rejection, parameter binding, sql-fragment wrapping + LIMIT, tail cursor advance/clamp, facets grouping, healthz; `go test -race`, vet and gofmt clean

unkin-agent added 1 commit 2026-08-23 16:39:48 +10:00

Add logviewer: web UI for the ClickHouse log store ... abc81e60c8

Single Go binary with embedded Bootstrap 3 + jQuery UI, querying logs.raw
over the ClickHouse HTTP interface as the readonly logreader user. Runs
behind oauth2-proxy; the app does no auth itself. Server-side enforced
time bounds (15m default, 72h max), parameterized queries, raw-SQL WHERE
fragment wrapped with enforced bounds and LIMIT, tail polling with a
clamped cursor, facets, healthz. Woodpecker build/test plus tag-driven
image push to artifactapi docker-internal.

unkinben added 1 commit 2026-08-23 16:44:21 +10:00

Split CI into pre-commit/test/build workflows for branch protection ... 09f8fcd83d

Branch protection on this repo will require the contexts
ci/woodpecker/pr/pre-commit, ci/woodpecker/pr/test and
ci/woodpecker/pr/build, mirroring the node-lookup workflow layout.

- Adds .woodpecker/pre-commit.yaml running pre-commit via uvx
- Adds .pre-commit-config.yaml mirroring node-lookup (vendored
  web/static assets excluded)
- Adds .woodpecker/test.yaml with vet + gofmt check + race tests
- Reworks build.yaml to compile via make build plus the existing
  buildx dry-run image check, PR events only
- Renames docker.yaml to release.yaml per node-lookup convention
  (tag-driven docker release unchanged)

unkin-agent commented 2026-08-23 16:44:34 +10:00

Author

Member

Copy Link

Copy Source

Reviewed on a shallow clone of benvin/initial (commit a81d3bb). All checklist items check out; no issues found.

Raw-SQL filter path — whereClause in internal/server/query.go always ANDs the enforced time-bound params first; the optional sql fragment is only ever parenthesised and ANDed into that same outer query (( frag )), never a standalone statement. validateSQLFragment blocks ;, and both the code comment and README are explicit that real safety comes from the readonly logreader CH user, not the fragment gate. LIMIT (capped at 1000) and max_execution_time=30 are applied on every query unconditionally in chClient.query. Every non-SQL value (filters, fuzzy q tokens, since/until) goes through {name:Type} / param_* binding — grepped all 4 fmt.Sprintf call sites in query.go and none interpolate user-controlled strings into SQL text (only whitelisted column/param names and validated ints).

Bounds — defaultWindow = 15m, maxWindow = 72h enforced server-side in parseTimeRange for both /api/query and /api/facets; /api/tail clamps its cursor to defaultWindow instead (inherently bounded, can't exceed maxWindow). Covered by TestQueryRejectsUnboundedWindow, TestFacetsRejectsWideWindow, TestTailStaleCursorClamped.

No app-side auth — README states this explicitly (oauth2-proxy is the boundary). CH_PASSWORD only ever flows from env into the X-ClickHouse-Key header; never logged (main.go's startup log omits it) or echoed in any API response.

Assets/tail/healthz — no CDN references anywhere in web/ (grepped), Bootstrap 3.4.1 + jQuery 3.7.1 are genuinely vendored. Tail cursor uses an exclusive lower bound (timestamp > {since_ms}) so no repeats, advances to the max ts_ms seen, tested in TestTailCursorAdvances/TestTailCursorUnchangedWhenNoRows. /healthz does a real SELECT 1 CH ping.

Build/test — ran on the clone myself: gofmt -l . clean, go vet ./... clean, go build ./... clean, go test -race ./... passes (11 tests in internal/server, no third-party deps in go.mod). Tests are meaningful, not rubber-stamped — they assert on the actual SQL text and bound params, not just status codes.

Dockerfile/CI — multi-stage (golang:1.25-alpine → distroless/static-debian12:nonroot), CGO disabled. Diffed .woodpecker/{build,docker}.yaml directly against unkin/mediamover: same CA-baked buildx plugin, backend_options.kubernetes.resources set on every step, serviceAccountName: default matches mediamover's own convention (no new ServiceAccount needed). logviewer's docker.yaml actually quotes "${CI_COMMIT_TAG}" where mediamover's doesn't — a strict improvement. Makefile has patch/minor/major tag-and-push targets per convention.

CI state: both ci/woodpecker/push/build and ci/woodpecker/pr/build green.

VERDICT: CLEAN

Reviewed on a shallow clone of `benvin/initial` (commit a81d3bb). All checklist items check out; no issues found. **Raw-SQL filter path** — `whereClause` in `internal/server/query.go` always ANDs the enforced time-bound params first; the optional `sql` fragment is only ever parenthesised and ANDed into that same outer query (`( frag )`), never a standalone statement. `validateSQLFragment` blocks `;`, and both the code comment and README are explicit that real safety comes from the readonly `logreader` CH user, not the fragment gate. `LIMIT` (capped at 1000) and `max_execution_time=30` are applied on every query unconditionally in `chClient.query`. Every non-SQL value (filters, fuzzy `q` tokens, since/until) goes through `{name:Type}` / `param_*` binding — grepped all 4 `fmt.Sprintf` call sites in query.go and none interpolate user-controlled strings into SQL text (only whitelisted column/param names and validated ints). **Bounds** — `defaultWindow` = 15m, `maxWindow` = 72h enforced server-side in `parseTimeRange` for both `/api/query` and `/api/facets`; `/api/tail` clamps its cursor to `defaultWindow` instead (inherently bounded, can't exceed maxWindow). Covered by `TestQueryRejectsUnboundedWindow`, `TestFacetsRejectsWideWindow`, `TestTailStaleCursorClamped`. **No app-side auth** — README states this explicitly (oauth2-proxy is the boundary). `CH_PASSWORD` only ever flows from env into the `X-ClickHouse-Key` header; never logged (main.go's startup log omits it) or echoed in any API response. **Assets/tail/healthz** — no CDN references anywhere in `web/` (grepped), Bootstrap 3.4.1 + jQuery 3.7.1 are genuinely vendored. Tail cursor uses an exclusive lower bound (`timestamp > {since_ms}`) so no repeats, advances to the max `ts_ms` seen, tested in `TestTailCursorAdvances`/`TestTailCursorUnchangedWhenNoRows`. `/healthz` does a real `SELECT 1` CH ping. **Build/test** — ran on the clone myself: `gofmt -l .` clean, `go vet ./...` clean, `go build ./...` clean, `go test -race ./...` passes (11 tests in internal/server, no third-party deps in go.mod). Tests are meaningful, not rubber-stamped — they assert on the actual SQL text and bound params, not just status codes. **Dockerfile/CI** — multi-stage (`golang:1.25-alpine` → `distroless/static-debian12:nonroot`), CGO disabled. Diffed `.woodpecker/{build,docker}.yaml` directly against `unkin/mediamover`: same CA-baked buildx plugin, `backend_options.kubernetes.resources` set on every step, `serviceAccountName: default` matches mediamover's own convention (no new ServiceAccount needed). logviewer's `docker.yaml` actually quotes `"${CI_COMMIT_TAG}"` where mediamover's doesn't — a strict improvement. `Makefile` has `patch`/`minor`/`major` tag-and-push targets per convention. **CI state**: both `ci/woodpecker/push/build` and `ci/woodpecker/pr/build` green. VERDICT: CLEAN

unkin-agent commented 2026-08-23 16:50:58 +10:00

Author

Member

Copy Link

Copy Source

Delta re-review at 09f8fcd (previously reviewed clean at abc81e6; delta = git diff abc81e6 09f8fcd).

Scope — delta touches only CI/pre-commit files: new .pre-commit-config.yaml, .woodpecker/pre-commit.yaml, .woodpecker/test.yaml, build.yaml reworked, docker.yaml renamed to release.yaml (100% similarity, no content change). No app code touched. Good.

Resources — every step in every workflow sets backend_options.kubernetes.resources (requests/limits): pre-commit (512Mi/1 → 2Gi/2), test (512Mi/1 → 2Gi/2), build's two steps (512Mi/1→2Gi/2 and 1Gi/1→4Gi/2), release's docker step (1Gi/1→4Gi/2). Good.

Push/PR duplication — build.yaml's when narrowed from event: [pull_request, push] to event: pull_request only; pre-commit.yaml and test.yaml are PR-only too. No workflow now double-fires on the same commit. Good.

release.yaml — still event: tag, ref: refs/tags/v*, still the CA-baked buildx plugin (buildkit_config trusts artifactapi's CA), still quotes "${CI_COMMIT_TAG}" in both build_args.VERSION and tags. Unchanged from the already-reviewed docker.yaml (literal rename). Good.

Pre-commit hooks — the hook set (pre-commit-hooks v4.6.0: trailing-whitespace/end-of-file-fixer/check-yaml/check-merge-conflict/mixed-line-ending; pre-commit-golang v0.5.1: go-fmt/go-vet/go-unit-tests) is an exact match for node-lookup's .pre-commit-config.yaml. Good.

Issue — exclude is broader than "vendored assets": exclude: ^web/static/ blankets the whole directory, but web/static/ holds both vendored, minified libs (jquery.min.js, bootstrap.min.js, bootstrap.min.css) and first-party hand-written source (app.js, 253 lines; logviewer.css, 14 lines). As written, the exclude also silently exempts app.js/logviewer.css from trailing-whitespace, end-of-file-fixer, and mixed-line-ending — files that aren't vendored and have no reason to skip basic hygiene checks. Suggest scoping the pattern to the vendored files specifically, e.g. ^web/static/(jquery|bootstrap)\.min\.(js|css)$, so first-party JS/CSS stays covered.

VERDICT: ISSUES

Delta re-review at 09f8fcd (previously reviewed clean at abc81e6; delta = `git diff abc81e6 09f8fcd`). **Scope** — delta touches only CI/pre-commit files: new `.pre-commit-config.yaml`, `.woodpecker/pre-commit.yaml`, `.woodpecker/test.yaml`, `build.yaml` reworked, `docker.yaml` renamed to `release.yaml` (100% similarity, no content change). No app code touched. Good. **Resources** — every step in every workflow sets `backend_options.kubernetes.resources` (requests/limits): pre-commit (512Mi/1 → 2Gi/2), test (512Mi/1 → 2Gi/2), build's two steps (512Mi/1→2Gi/2 and 1Gi/1→4Gi/2), release's docker step (1Gi/1→4Gi/2). Good. **Push/PR duplication** — `build.yaml`'s `when` narrowed from `event: [pull_request, push]` to `event: pull_request` only; pre-commit.yaml and test.yaml are PR-only too. No workflow now double-fires on the same commit. Good. **release.yaml** — still `event: tag, ref: refs/tags/v*`, still the CA-baked buildx plugin (buildkit_config trusts artifactapi's CA), still quotes `"${CI_COMMIT_TAG}"` in both `build_args.VERSION` and `tags`. Unchanged from the already-reviewed `docker.yaml` (literal rename). Good. **Pre-commit hooks** — the hook set (`pre-commit-hooks` v4.6.0: trailing-whitespace/end-of-file-fixer/check-yaml/check-merge-conflict/mixed-line-ending; `pre-commit-golang` v0.5.1: go-fmt/go-vet/go-unit-tests) is an exact match for node-lookup's `.pre-commit-config.yaml`. Good. **Issue — exclude is broader than "vendored assets"**: `exclude: ^web/static/` blankets the whole directory, but `web/static/` holds both vendored, minified libs (`jquery.min.js`, `bootstrap.min.js`, `bootstrap.min.css`) *and* first-party hand-written source (`app.js`, 253 lines; `logviewer.css`, 14 lines). As written, the exclude also silently exempts `app.js`/`logviewer.css` from trailing-whitespace, end-of-file-fixer, and mixed-line-ending — files that aren't vendored and have no reason to skip basic hygiene checks. Suggest scoping the pattern to the vendored files specifically, e.g. `^web/static/(jquery|bootstrap)\.min\.(js|css)$`, so first-party JS/CSS stays covered. VERDICT: ISSUES

unkinben added 1 commit 2026-08-23 16:52:14 +10:00

Narrow pre-commit exclude to vendored minified assets only b07872accd

unkin-agent commented 2026-08-23 16:57:31 +10:00

Author

Member

Copy Link

Copy Source

Addressed review comment 10485 in b07872a: narrowed the pre-commit exclude from ^web/static/ to ^web/static/(jquery|bootstrap)[^/]*\.min\.(js|css)$, so only the vendored jquery.min.js / bootstrap.min.{js,css} are exempt. Hand-written web/static/app.js and web/static/logviewer.css are now covered by the hygiene hooks; pre-commit run --all-files passes with no fixes needed. All three ci/woodpecker/pr/* contexts are green on the new head.

Addressed review comment 10485 in b07872a: narrowed the pre-commit exclude from `^web/static/` to `^web/static/(jquery|bootstrap)[^/]*\.min\.(js|css)$`, so only the vendored jquery.min.js / bootstrap.min.{js,css} are exempt. Hand-written web/static/app.js and web/static/logviewer.css are now covered by the hygiene hooks; `pre-commit run --all-files` passes with no fixes needed. All three ci/woodpecker/pr/* contexts are green on the new head.

benvin merged commit 2e6f58cd13 into main 2026-08-23 17:20:33 +10:00

benvin deleted branch benvin/initial 2026-08-23 17:20:33 +10:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/logviewer#1