From 64745e450697b053e3351b95e8e5894403ae1fb3 Mon Sep 17 00:00:00 2001 From: trick77 Date: Sun, 16 Aug 2026 14:52:25 +0200 Subject: [PATCH] Compress AGENTS.md wording, move the Splunk props.conf to docs/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 11,283 -> 9,988 chars (-11%). This file is nearly all steering, so the cut is wording only: no rule, threshold, table name, env var or test name was removed. 57 rule markers verified present. - Move the Splunk props.conf stanza to docs/splunk-props.conf — it is owned by the platform team and kept for reference, not applied here. - Drop the boilerplate 'this file provides guidance to coding agents' opener. - Tighten the long invariants (service_id, commit-SHA correlation, team keys, layering, field naming) without dropping a clause. - Gitignore .claude/ so worktrees stay out of git status. --- .gitignore | 3 +++ AGENTS.md | 49 +++++++++++++++--------------------------- docs/splunk-props.conf | 14 ++++++++++++ 3 files changed, 34 insertions(+), 32 deletions(-) create mode 100644 docs/splunk-props.conf diff --git a/.gitignore b/.gitignore index ae93f06..97b442d 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ hack/* .vscode/ *.swp .DS_Store + +# Claude Code worktrees +.claude/ diff --git a/AGENTS.md b/AGENTS.md index dc145b0..17fe5db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,7 +1,5 @@ # AGENTS.md -This file provides guidance to coding agents (Claude Code, Cursor, etc.) when working in this repository. - ## Commands ```bash @@ -22,27 +20,27 @@ If `docker ps` fails, ask the user to start OrbStack. ## Architecture invariants -- **Append-only ingestion.** Every webhook handler does `INSERT … ON CONFLICT (delivery_id) DO NOTHING`. Never `UPDATE` or `DELETE` event rows. Webhook retries must be idempotent. `delivery_id` is the dedup key for each source. +- **Append-only ingestion.** Every webhook handler does `INSERT … ON CONFLICT (delivery_id) DO NOTHING`; never `UPDATE` or `DELETE` event rows. `delivery_id` is the per-source dedup key, so retries are idempotent. - **Raw payload always stored.** `payload JSONB` keeps the full request body even if fields are extracted into typed columns. Don't drop fields you don't currently use. -- **`riptide.json` is config, not data.** `openshift/collector/riptide.json` is the in-repo sample; it declares teams (name + `group_email`) and org-wide automation rules. Edits go through PRs. The running pod hot-reloads via mtime in `RiptideConfigStore.maybe_reload()`. Do not propose moving the config into Postgres. -- **Per-team bearer keys live in a separate file**: in production it is mounted from the `riptide-collector-team-keys` Secret (never committed); the in-repo `openshift/collector/team-keys.json` is a dev sample with deterministic test hashes (raw dev bearers documented in `compose.yaml`). Stored as sha256 hashes; `TeamKeysStore` hot-reloads it the same way as the config. The bearer **is** the team identity — every webhook is tagged with `team = caller_team`. -- **No `service` column. No `service_id` on the wire.** Per-source aggregations group by `repo_full_name` / `pipeline_name` / `app_name` / `repo`; org-wide rollups group by `team`. Cross-source joins for BB↔Pipeline use `commit_sha`; Argo CD joins are described in the next bullet. Identifiers are lowercased at ingest (`commit_sha`, `revision`, `repo_full_name`, `branch_name`, `repo`) so SQL joins are case-stable. Do not propose adding a unified `service` column or `service_id` field — it served only single-pane labelling and was dropped. +- **`riptide.json` is config, not data.** `openshift/collector/riptide.json` (in-repo sample) declares teams (name + `group_email`) and org-wide automation rules; edits go through PRs and the pod hot-reloads via mtime in `RiptideConfigStore.maybe_reload()`. Never propose moving it into Postgres. +- **Per-team bearer keys live in a separate file**, mounted in production from the `riptide-collector-team-keys` Secret (never committed); `openshift/collector/team-keys.json` is a dev sample with deterministic test hashes (raw dev bearers in `compose.yaml`). Stored as sha256, hot-reloaded by `TeamKeysStore` like the config. The bearer **is** the team identity — every webhook is tagged `team = caller_team`. +- **No `service` column, no `service_id` on the wire.** Per-source aggregations group by `repo_full_name` / `pipeline_name` / `app_name` / `repo`; org-wide rollups by `team`. Identifiers are lowercased at ingest (`commit_sha`, `revision`, `repo_full_name`, `branch_name`, `repo`) so joins are case-stable. Never propose a unified `service` column or `service_id` — it served only single-pane labelling and was dropped. - **`automation` is org-wide.** Bot definitions live at the config root, not per team. -- **Metrics are computed on read, not at ingest.** Don't add aggregation tables or scheduled rollup jobs in v1. Schema additions should preserve raw events; new metrics are SQL queries against existing rows or future materialized views. -- **Commit SHA joins Bitbucket↔Pipeline; Argo CD needs `payload->'images'`.** `bitbucket_events.commit_sha = pipeline_events.commit_sha` is a deterministic join (App-repo SHA on both sides). `argocd_events.revision` is the **GitOps-repo SHA** (proven empirically: four Apps for one service share one revision), so it does NOT directly match the other two. The App-repo SHA is embedded in image tags rendered via `.app.status.summary.images`; the receiver stores them in `payload->'images'`. A future correlator extracts SHAs from those tags to bridge Argo CD events to pipeline events. Do not propose adding a `service_id` or hand-coded service-name mappings to fix correlation — the image-tag SHA is the contract. +- **Metrics are computed on read, not at ingest.** No aggregation tables or scheduled rollup jobs in v1. Schema additions preserve raw events; new metrics are SQL against existing rows or future materialized views. +- **Commit SHA joins Bitbucket↔Pipeline; Argo CD needs `payload->'images'`.** `bitbucket_events.commit_sha = pipeline_events.commit_sha` is deterministic (App-repo SHA both sides). `argocd_events.revision` is the **GitOps-repo SHA** — proven empirically, four Apps for one service share one revision — so it does NOT match the other two. The App-repo SHA rides in image tags from `.app.status.summary.images`, stored as `payload->'images'`; a future correlator extracts it to bridge Argo CD to pipeline events. The image-tag SHA is the contract — never propose `service_id` or hand-coded name mappings to fix correlation. - **`change_type` lives on Bitbucket events only.** Don't denormalise it onto pipeline / Argo rows; join Pipeline rows via `commit_sha` and Argo rows via the image-tag SHA in `payload->'images'` at read time. - **CI events are source-tagged, not source-routed.** All pipeline events from any CI (Jenkins, Tekton, …) land in the single `pipeline_events` table via `POST /webhooks/pipeline`, distinguished by the `source` column. Do not add per-CI tables or endpoints. The dedup key is `source#pipeline_name#run_id#phase`. - **Noergler events carry finops + reviewer-precision only.** The `noergler_events` table is `event_type`-discriminated (`completed` | `feedback`) and is fed by `POST /webhooks/noergler` from optional noergler instances. Do not re-emit PR lifecycle from noergler — `bitbucket_events` already covers open / merged / declined. Dedup keys: `completed#` and `feedback##`. -- **Senders verify reachability + bearer at startup via `GET /auth/ping`.** Authenticated endpoint returning `{"status":"ok","team":""}`. Use this from any sender (noergler, future ones) to fail-fast on a wrong token. Don't reuse `/health` (unauth liveness) or `/ready` (unauth readiness) for this — those answer different questions. +- **Senders verify reachability + bearer at startup via `GET /auth/ping`** — authenticated, returns `{"status":"ok","team":""}`, so a wrong token fails fast. Never reuse `/health` (unauth liveness) or `/ready` (unauth readiness) for it; those answer different questions. - **`modified_at` has a Postgres trigger** (`riptide_set_modified_at`), not just SQLAlchemy `onupdate`. Raw-SQL updates also bump it. Keep the trigger when changing migrations. - **Database is external.** `riptide-collector` does NOT manage Postgres. Do not add a Postgres Deployment to `openshift/`. - **Pyright strict for `src/`, standard for `tests/` and `migrations/`.** New code under `src/` must satisfy strict mode — no `Any` leaks; narrow `Optional`s with `isinstance` or helpers like `_as_dict()` in `routers/bitbucket.py`. ## Repo conventions -- **Layering.** Routers do HTTP + auth + dispatch only. Source-specific payload extraction lives in `parsers_.py` (e.g. `parsers_bitbucket.py`) as pure functions returning a typed `*EventDraft` dataclass — no HTTP, no DB, no config. The router computes config-derived fields (e.g. `automation_source`) and persists. Don't put extraction logic in routers; don't duplicate JSON-shape coercion (`_as_dict`/`_as_list`-style helpers belong with the extractor that uses them). -- Single Python package, `riptide_collector` (flat top-level, not a namespace package). Future suite components (e.g. `riptide-api`, `riptide-dashboard`) get their own top-level package, e.g. `riptide_dashboard` — leave architectural room for them. -- Webhook routers are factories that return an `APIRouter`. Bitbucket needs the config for automation detection (`make_router(config, session_factory, auth_dep)`); Pipeline, ArgoCD, and Noergler don't, so they take just `(session_factory, auth_dep)`. They're wired up in `src/riptide_collector/main.py::create_app`. Add the config only when a router actually needs `automation` rules or team metadata. +- **Layering.** Routers do HTTP + auth + dispatch only. Payload extraction lives in `parsers_.py` (e.g. `parsers_bitbucket.py`) as pure functions returning a typed `*EventDraft` — no HTTP, no DB, no config. The router computes config-derived fields (`automation_source`) and persists. Never put extraction in routers, and keep JSON-shape coercion helpers with the extractor that uses them. +- Single flat package `riptide_collector`, not a namespace package. Future suite components (`riptide-api`, `riptide-dashboard`) get their own top-level package — leave room for them. +- Webhook routers are factories returning an `APIRouter`, wired in `main.py::create_app`. Bitbucket takes the config for automation detection (`make_router(config, session_factory, auth_dep)`); Pipeline, ArgoCD and Noergler take just `(session_factory, auth_dep)`. Pass the config only when a router needs `automation` rules or team metadata. - Pydantic schemas: **strict** for `/webhooks/pipeline` and `/webhooks/argocd` (we own the contract — invalid payloads must 422); **permissive raw-dict parsing** for Bitbucket (its payload shapes vary; we best-effort extract). - Use `_as_dict()` / `_as_list()` helpers in `routers/bitbucket.py` to coerce arbitrary JSON shapes — basedpyright strict won't accept chained `.get()` on `Optional[dict]`. - Tests use real Postgres via testcontainers, never SQLite. The `client` fixture in `tests/conftest.py` depends on `session_factory` which truncates tables per test. @@ -50,32 +48,19 @@ If `docker ps` fails, ask the user to start OrbStack. ## Logging & Splunk -- **One JSON object per line, on stdout.** OpenShift's Splunk Connect for Kubernetes (SCK) tails the container log; Splunk auto-extracts fields via `KV_MODE=json` for sourcetype `riptide:collector:json` (set as pod annotation in `openshift/collector/deployment.yaml`). +- **One JSON object per line, on stdout.** Splunk Connect for Kubernetes tails the container log and auto-extracts via `KV_MODE=json` for sourcetype `riptide:collector:json` (pod annotation in `openshift/collector/deployment.yaml`). - **Stdlib loggers (uvicorn, sqlalchemy, alembic) are bridged through structlog.** Do NOT add separate logging handlers or re-init `logging.basicConfig` — `configure_logging()` in `logging_config.py` is the single entry point. -- **Splunk-reserved field names are forbidden as kwargs**: `source`, `sourcetype`, `host`, `index`, `time`, `_time`, `_raw`, `event`. The CI vendor field is `ci_system` (not `source`); the structlog event name lives in `msg` (renamed from `event`); severity lives in `log_level` (renamed from `level`). A runtime processor (`_strip_reserved`) namespaces accidental reserved kwargs under `splunk_` as a safety net — do not rely on it; pick the right name from the start. -- **Field-naming convention** for non-reserved kwargs: prefer generic names that mean the same across sources (`event_type`, `status`, `phase`, `delivery_id`, `team`, `repo`, `commit_sha`). Don't pre-namespace with the source name (`noergler_event_type`, `pipeline_status`) — `webhook_source` already disambiguates in `stats by webhook_source, event_type`. Only namespace when two sources legitimately mean different things by the same word and would collide in a single Splunk panel. -- **Webhook handlers emit exactly one `msg=webhook_processed` log per request** with required fields `webhook_source ∈ {bitbucket,pipeline,argocd,noergler}`, `outcome ∈ {accepted,deduped,ignored,skipped}`, `delivery_id`, `team`. Source-specific fields go alongside (e.g. `app`, `revision`, `phase` for argocd). Include `delivery_id` even on `ignored`/`skipped` paths so triage has a key. +- **Splunk-reserved names are forbidden as kwargs**: `source`, `sourcetype`, `host`, `index`, `time`, `_time`, `_raw`, `event`. CI vendor is `ci_system`, the structlog event name is `msg`, severity is `log_level`. `_strip_reserved` namespaces accidents under `splunk_` as a safety net — never rely on it, pick the right name. +- **Field naming:** generic names that mean the same across sources (`event_type`, `status`, `phase`, `delivery_id`, `team`, `repo`, `commit_sha`). Never pre-namespace with the source (`noergler_event_type`) — `webhook_source` already disambiguates in `stats by webhook_source, event_type`. Namespace only when two sources genuinely mean different things by one word and would collide in a panel. +- **Exactly one `msg=webhook_processed` per request**, with `webhook_source ∈ {bitbucket,pipeline,argocd,noergler}`, `outcome ∈ {accepted,deduped,ignored,skipped}`, `delivery_id`, `team`, plus source-specific fields (`app`, `revision`, `phase` for argocd). Include `delivery_id` even on `ignored`/`skipped` so triage has a key. - **`outcome=deduped`** is detected via `RETURNING delivery_id` on the `INSERT ... ON CONFLICT DO NOTHING` — a `None` scalar means the row already existed. Preserve this when adding new sources. - **Persist failures**: wrap the `async with session_factory()` block in `try/except Exception: logger.exception("webhook_persist_failed", ...); raise`. Never swallow. -- **Access log** is emitted by the `access_log` middleware in `main.py` as `msg=http_request` with `request_id`, `method`, `path`, `status_code`, `duration_ms`. `request_id` is bound to contextvars so any log within the request inherits it. `/health` and `/ready` are silenced; uvicorn.access is set to WARNING (do not lower it). -- **Splunk `props.conf` snippet** (owned by platform team, kept here for reference): - ``` - [riptide:collector:json] - SHOULD_LINEMERGE = false - LINE_BREAKER = ([\r\n]+) - KV_MODE = json - TIME_PREFIX = "timestamp":\s*" - TIME_FORMAT = %Y-%m-%dT%H:%M:%S.%6NZ - TRUNCATE = 0 - ``` +- **Access log**: `access_log` middleware in `main.py` emits `msg=http_request` with `request_id`, `method`, `path`, `status_code`, `duration_ms`. `request_id` is bound to contextvars so every log in the request inherits it. `/health` and `/ready` are silenced; uvicorn.access stays at WARNING. +- The Splunk `props.conf` stanza is owned by the platform team; a copy for reference lives in [`docs/splunk-props.conf`](docs/splunk-props.conf). ## OpenShift layout -`openshift/` is **suite-level**, structured per-component. The collector lives in `openshift/collector/`. When adding a new component: -1. Create `openshift//` with its own `kustomization.yaml`. -2. Add it to the `resources:` list in `openshift/kustomization.yaml`. -3. Every container needs explicit `requests` AND `limits` for cpu and memory — no exceptions. -4. Use `runAsNonRoot: true` and `readOnlyRootFilesystem: true`; no fixed `runAsUser` (OpenShift assigns a random UID per project). +`openshift/` is suite-level, one directory per component (`openshift/collector/`). Adding a component: create `openshift//` with its own `kustomization.yaml`, add it to `resources:` in `openshift/kustomization.yaml`, give every container explicit cpu+memory `requests` AND `limits` (no exceptions), and set `runAsNonRoot: true` + `readOnlyRootFilesystem: true` with no fixed `runAsUser` (OpenShift assigns a random UID per project). ## What's intentionally out of v1 diff --git a/docs/splunk-props.conf b/docs/splunk-props.conf new file mode 100644 index 0000000..7907b3f --- /dev/null +++ b/docs/splunk-props.conf @@ -0,0 +1,14 @@ +# Splunk props.conf (reference) + +Owned by the platform team; kept here so a change to the log shape can be checked +against what Splunk expects. Not applied by this repo. + +``` +[riptide:collector:json] +SHOULD_LINEMERGE = false +LINE_BREAKER = ([\r\n]+) +KV_MODE = json +TIME_PREFIX = "timestamp":\s*" +TIME_FORMAT = %Y-%m-%dT%H:%M:%S.%6NZ +TRUNCATE = 0 +```