Skip to content

feat(timeline): add PostgreSQL timeline storage - #1314

Open
syscod3 wants to merge 7 commits into
skyhook-io:mainfrom
syscode-labs:feat/postgres-timeline-upstream
Open

feat(timeline): add PostgreSQL timeline storage#1314
syscod3 wants to merge 7 commits into
skyhook-io:mainfrom
syscode-labs:feat/postgres-timeline-upstream

Conversation

@syscod3

@syscod3 syscod3 commented Aug 1, 2026

Copy link
Copy Markdown

Description

Radar can now persist timeline history in an externally managed PostgreSQL database, so events survive pod restarts and Helm upgrades without requiring Radar to install or operate a database.

The PostgreSQL adapter implements the existing timeline store contract, applies versioned transactional migrations under an advisory lock, validates the resulting schema at startup, and preserves the existing SQLite and memory behavior. The chart accepts only a same-namespace Secret reference; the DSN is read from RADAR_TIMELINE_POSTGRES_DSN and is never persisted to config or exposed as a CLI argument.

This implements the PostgreSQL portion of #80. MySQL support and SQLite history import are intentionally out of scope.

Type of change

  • New feature (non-breaking change that adds functionality)

How has this been tested?

  • Added/updated unit tests
  • Tested PostgreSQL integration against PostgreSQL 17
  • Ran go test ./... with RADAR_TEST_POSTGRES_DSN
  • Ran go test ./... in pkg/
  • Ran Helm lint, Helm unit tests, and chart rendering checks
  • Ran frontend typecheck, tests, lint, and build
  • Ran shared k8s-ui tests

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my code
  • I have added comments where necessary
  • My changes generate no new warnings
  • Any dependent changes have been merged

Related issues

Implements the PostgreSQL portion of #80.


Personal Notes

I was in need of a more robust db for persistence and I decided to fork the db interface, this is being used on my personal cluster for some days and the code is heavily AI assisted, so I expect to be properly grilled on this PR, please bear with me.

Thank you.


Note

Medium Risk
New persistence path touches startup, migrations, and shared DB semantics for multi-replica deploys; misconfiguration or migration issues can block Radar boot, though credentials stay out of config files.

Overview
Adds PostgreSQL as a third timeline backend alongside memory and SQLite, so cluster change history can live in an external database instead of a single PVC—useful for restarts, rolling updates, and multi-replica setups.

The new PostgresStore (pgx) implements the existing store contract: embedded migrations under an advisory lock, schema validation at startup, append/query/grouping behavior aligned with SQLite (including K8s event upserts and seq paging), age-based retention cleanup, and no SQLite-style max-size pruning. BuildTimelineStoreConfig now returns errors for unknown backends or postgres without a DSN; explorer/desktop read the DSN only from RADAR_TIMELINE_POSTGRES_DSN (not config.json). Failed postgres init is fatal (no memory fallback), and timeline init failures now fail subsystem startup.

Helm gains timeline.storage=postgres, timeline.postgres.existingSecret / secretKey, template guards when postgres is chosen without a secret, deployment env injection for the DSN, and unittest/CI coverage including a Postgres 17 service job for ./internal/timeline tests. Docs and chart README describe operator-managed secrets and credential rotation (pod restart required).

Reviewed by Cursor Bugbot for commit 873ab97. Bugbot is set up for automated code reviews on this repo. Configure here.

@syscod3
syscod3 marked this pull request as ready for review August 1, 2026 20:47
PerformContextSwitch always called SwitchContext, which hard-rejects
any call while running in-cluster (context switching is disabled
there by design, per docs/configuration.md). /connection/retry calls
PerformContextSwitch(currentContext) to reconnect after a transient
disconnect, so in-cluster reconnect could never succeed — it always
failed on "cannot switch context when running in-cluster" instead of
retrying, surfacing the generic connection-failed screen with a
misleading raw error.

In-cluster mode has one fixed context, so there's nothing to switch;
skip straight to the connectivity test + subsystem reinit.
Both sides added independent, non-overlapping blocks at the same
location in deployment.yaml: the postgres existingSecret validation
+ RADAR_TIMELINE_POSTGRES_DSN env var (this branch) and the basePath
validation + unconditional MY_POD_NAMESPACE/RADAR_IMAGE (upstream).
Kept both. Dropped the rbac.selfUpgrade gate on MY_POD_NAMESPACE per
upstream's own comment explaining that gate was wrong — RBAC on the
Role decides self-upgrade capability, not the presence of the env var.
Verified: helm template renders correctly with timeline.storage=postgres
and separately confirms MY_POD_NAMESPACE now emits unconditionally.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 873ab97. Configure here.

query.WriteString(" ORDER BY seq ASC")
} else {
query.WriteString(" ORDER BY timestamp DESC")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Query ignores sequence options

High Severity

buildQuery never applies UntilSeq or SequenceOrder, so PostgreSQL queries keep using timestamp order and skip the arrival-order bounds that SQLite and memory honor. Capacity activity paging and backwards timeline reads can return the wrong events or miss late arrivals.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 873ab97. Configure here.

stats.LastCleanupError = s.lastCleanupEr
s.cleanupMu.RUnlock()

return stats

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stats omits sequence bounds

High Severity

Stats never populates OldestSeq or NewestSeq. Timeline responses therefore omit X-Radar-Timeline-Min-Seq, so clients cannot detect retention gaps after cleanup and may treat missing history as an empty delta.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 873ab97. Configure here.

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.

1 participant