coffey-labs/cairnobs
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Add sentry_dashboard_panel, resolving panels as their own resource

Browse Source 
The open design question named in the last three commits' README --
"a sentry_dashboard_panel resource (or a panels list block on this one)"
-- is resolved: separate resource, matching api/dashboards.Handler's own
shape (a panel is created/updated/deleted independently of its parent
dashboard via its own endpoints, never by rewriting the dashboard's
whole panel list). A nested list block would have forced every panel to
be rewritten on any single panel's change, hiding fine-grained diffs a
separate resource shows naturally -- the more idiomatic Terraform
pattern for independently-lifecycled child resources, and the one that
matches what the API actually does.

Unlike sentry_alert_rule/sentry_notification_target, this resource
supports a real in-place Update -- api/dashboards.Handler actually has a
PUT /dashboards/{id}/panels/{panelId}. Only dashboard_id forces
RequiresReplace: UpdatePanel's SQL matches WHERE id = $panelID AND
dashboard_id = $dashboardID, so changing dashboard_id through the
existing panel's URL wouldn't move it, it would just fail to match --
there's no API operation for "move a panel to a different dashboard."

Panels have no standalone GET endpoint -- only GET /dashboards/{id},
which includes the full panels array. client.go's new getPanel fetches
the parent dashboard and finds the panel by ID within it, returning the
same *apiError{StatusCode: 404} shape a direct GET would whether the
dashboard itself or just the panel within it is gone, so isNotFound
works identically either way. This also means a bare panel ID isn't
enough to import from -- ImportState takes "dashboard_id/panel_id" and
splits on the last "/", the one resource here with a composite import
identifier.

query_language never accepts "sql" for panels specifically -- confirmed
in api/dashboards's own validatePanel ("dashboards only support
pipe-syntax queries, since the dashboard time-range picker is injected
as leading query terms"), a real constraint from the API this client
doesn't re-validate client-side (same "let the API be the one source of
truth for validation" posture the other resources already take), but
documented in the schema so it's not a surprise 400 from Create.

sentry_dashboard_panel gets a matching data source too
(dashboard_id + id both Required, unlike the other three data sources'
single Required id, since getPanel itself needs both).

Verified: client tests are real httptest.Server round trips, including
getPanel finding the right panel within a real dashboard response and
returning a recognizable not-found both when the panel is missing and
when the parent dashboard itself is gone. Schema validation needs no
Terraform binary. TestAccDashboardPanelResource_basic and
TestAccDashboardPanelDataSource_basic are real acceptance tests,
skip-gated by TF_ACC same as the other six -- the resource test proves a
genuine in-place update (a title change, no plancheck needed since
in-place update is the default expectation here, unlike the
create/destroy-only resources). Not run against a live stack in this
environment, same disclosed gap as everything else Docker-gated in this
repo.
This commit is contained in:
jcoffey-dev
2026-08-15 10:40:27 -07:00
parent eb38611aa8
commit 278b24cf67
12 changed files with 969 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
terraform/README.md
+91 -58
View File
@@ -3,8 +3,8 @@
Sentry's Terraform provider -- `CLAUDE.md`'s "Repo conventions" section
names this a first-class deliverable alongside `sentryctl`
("CLI and Terraform provider are first-class, not afterthoughts"), but
no phase before this one had actually built any of it. Three resources
so far (`sentry_dashboard`, `sentry_alert_rule`,
no phase before this one had actually built any of it. Four resources
so far (`sentry_dashboard`, `sentry_dashboard_panel`, `sentry_alert_rule`,
`sentry_notification_target`), each paired with a read-only data source
of the same name, not a finished provider, built on
[HashiCorp's `terraform-plugin-framework`][framework] (the
@@ -14,7 +14,7 @@ framework HashiCorp itself steers new providers away from).
[framework]: https://developer.hashicorp.com/terraform/plugin/framework
## Why these three resources first
## Why these four resources first
`cli/README.md` already frames the dashboards REST contract this way:
`POST /dashboards`, `GET`/`PUT`/`DELETE /dashboards/{id}` are "the seed
@@ -23,15 +23,18 @@ export, CLI apply, eventually a provider)." This provider is that third
caller -- `internal/provider/client.go` talks the exact same JSON shape
`sentryctl dashboards apply` and the web UI's Export JSON button already
use against `api/dashboards.Handler`, not a new contract invented for
Terraform's sake. `sentry_alert_rule` follows against `alerting`'s own
`POST`/`GET`/`DELETE /rules[/{id}]` -- the natural second resource, and
a real second service (`alerting` is a genuinely separate deployment
from `api`, its own base URL), so building it second exercised that this
provider can talk to more than one Sentry service, not just repeat the
dashboards pattern against the same endpoint. `sentry_notification_target`
rounds these two out -- `sentry_alert_rule.notification_target_id` needs
something to actually point at, and without this resource that id could
only ever come from outside Terraform (`sentryctl`, `curl`, the web UI),
Terraform's sake. `sentry_dashboard_panel` follows the same contract's
panel endpoints, as its own resource rather than a block nested inside
`sentry_dashboard` -- see "Panels are their own resource" below.
`sentry_alert_rule` follows against `alerting`'s own `POST`/`GET`/
`DELETE /rules[/{id}]` -- the natural next resource, and a real second
service (`alerting` is a genuinely separate deployment from `api`, its
own base URL), so building it exercised that this provider can talk to
more than one Sentry service, not just repeat the dashboards pattern
against the same endpoint. `sentry_notification_target` rounds these
out -- `sentry_alert_rule.notification_target_id` needs something to
actually point at, and without this resource that id could only ever
come from outside Terraform (`sentryctl`, `curl`, the web UI),
undermining the point of managing rules as code at all.
## What's built
@@ -63,15 +66,37 @@ resource "sentry_dashboard" "example" {
Supports `terraform import sentry_dashboard.example <dashboard-id>`.
**Panels are not managed by this resource.** `api/dashboards.Handler`
exposes panel CRUD as its own endpoints
(`POST`/`PUT`/`DELETE /dashboards/{id}/panels[/{panelId}]`), a
genuinely separate resource shape (a panel belongs to exactly one
dashboard, has its own lifecycle, and the query-language/viz-config
fields deserve their own attribute validation) -- scoped out of this
first pass deliberately, not an oversight. A `sentry_dashboard_panel`
resource (or a panels list block on this one -- an open design question,
not yet decided) is real, disclosed future work.
**Panels are their own resource, not a nested block.** `api/dashboards.
Handler` exposes panel CRUD as its own endpoints (`POST`/`PUT`/
`DELETE /dashboards/{id}/panels[/{panelId}]`) -- a panel belongs to
exactly one dashboard, has its own lifecycle, and is created/updated/
deleted independently, never by rewriting a dashboard's whole panel
list, so `sentry_dashboard_panel` follows that shape rather than a
nested list block (which would force every panel to be rewritten on any
single panel's change, hiding fine-grained diffs a separate resource
shows naturally):
```hcl
resource "sentry_dashboard_panel" "example" {
dashboard_id = sentry_dashboard.example.id
title = "5xx rate over time"
query = "status>=500 | timechart count"
viz_type = "line" # table, line, bar, single_stat, or top_n
# query_language never accepts "sql" for panels -- the API rejects it
# outright (dashboards only support pipe-syntax queries, since the
# time-range picker is injected as leading query terms). Unlike
# sentry_alert_rule/sentry_notification_target, this resource
# supports a real in-place update (api/dashboards.Handler has a real
# PUT for panels) -- only dashboard_id forces a destroy-and-recreate,
# since there's no API operation to move a panel between dashboards.
}
```
Supports `terraform import sentry_dashboard_panel.example
<dashboard-id>/<panel-id>` -- a bare panel ID isn't enough on its own,
since `Read` needs the parent `dashboard_id` to know where to look (see
`client.go`'s `getPanel` doc comment for why: there's no standalone
`GET` for a single panel).
```hcl
provider "sentry" {
@@ -134,15 +159,17 @@ than left implicit.
## Data sources
Each resource above has a matching read-only data source (`data
"sentry_dashboard"`, `data "sentry_alert_rule"`, `data
"sentry_notification_target"`), a single `id` attribute in, every other
attribute out -- a lookup against the same `GET`/`{id}` endpoint the
matching resource's own `Read` already uses, nothing new added to
`client.go` beyond that. Mechanical and low-risk by design: no new
architectural question, no new external service, no new write path --
just reusing the resource's own model/conversion functions
(`dashboardModelFromAPI` etc.) against a `Required` `id` input instead
of a full config.
"sentry_dashboard"`, `data "sentry_dashboard_panel"`, `data
"sentry_alert_rule"`, `data "sentry_notification_target"`) -- a lookup
against the same endpoint the matching resource's own `Read` already
uses, nothing new added to `client.go` beyond that. Mechanical and
low-risk by design: no new architectural question, no new external
service, no new write path -- just reusing the resource's own model/
conversion functions (`dashboardModelFromAPI` etc.) against `Required`
input instead of a full config. Three of the four take a single
`Required` `id`; `sentry_dashboard_panel`'s takes both `dashboard_id`
and `id` (both `Required`), matching `getPanel`'s own two-argument shape
-- there's no standalone lookup for a panel by ID alone.
```hcl
data "sentry_notification_target" "ops" {
@@ -167,7 +194,6 @@ attribute" above.
idempotently -- see `/enterprise/README.md`'s "Bootstrapping a tenant"
section) and Phase 4 commercial licensing, so this would need its own
design pass, not just "add another resource file."
- Dashboard panels (see "Panels are not managed by this resource" above).
- Publishing to the real Terraform Registry -- `main.go`'s `Address`
(`registry.terraform.io/sentry/sentry`) is the address a real
publication would use, but nothing has actually been published; local
@@ -191,38 +217,45 @@ response parsing, including the 404-vs-other-error distinction
out-of-band" convention correctly, (for rules) proving the
`GET /rules/{id}` response's promoted `RuleWithState` fields plus an
extra `"state"` key this client deliberately has no field for still
parse cleanly, and (for targets) proving `secret` really does come back
parse cleanly, (for targets) proving `secret` really does come back
unredacted -- documenting real `alerting` behavior with a test, not just
a comment, so a future change to that behavior would be caught here too.
`internal/provider/provider_test.go` validates the provider's, all
three resources', and all three data sources' schemas are internally
a comment, so a future change to that behavior would be caught here too
-- and (for panels) proving `getPanel` finds the right panel within a
real parent dashboard's `panels` array, and returns a recognizable
not-found both when the panel is missing from an otherwise-real
dashboard response and when the dashboard itself is gone.
`internal/provider/provider_test.go` validates the provider's, all four
resources', and all four data sources' schemas are internally
well-formed (attribute names, the `Required`/`Computed`/`Sensitive`
split -- for data sources, specifically that `id` is `Required` and
everything else is `Computed`) without needing a Terraform binary or a
live `api`/`alerting` service at all.
split -- for data sources, that every attribute except the lookup key(s)
is `Computed`) without needing a Terraform binary or a live
`api`/`alerting` service at all.
Each resource and data source pair has a matching `TestAcc*_basic` in
its own `_test.go` file (`dashboard_resource_test.go`/
`dashboard_data_source_test.go`, and likewise for the other two) -- six
real acceptance tests total, using [`terraform-plugin-testing`][testing]
-- skipped unless `TF_ACC=1` is set, that framework's own standard
convention, the same shape every other live-infrastructure test in this
repo uses (`docker`-gated env vars for Postgres/ClickHouse tests
elsewhere). The rule and target *resource* tests both use a
`plancheck.ExpectResourceAction` assertion proving a config change
actually plans a destroy-then-create, not an in-place update -- the
concrete, checked version of the "create/destroy only" design decision
documented above, not just a claim in a comment; the *data source*
tests each create a resource then look it up via `resource.
TestCheckResourceAttrPair`, proving the data source's `Read` actually
agrees with what the resource wrote, not just that both compile. Even
with `TF_ACC=1` all six tests also need real running `api`/`alerting`
services (Postgres + ClickHouse) to apply against, which this
environment has no Docker access to bring up -- **not run here**, same
disclosed gap as every other live-infra test across this repo (see
`/docs/phase-4-runbook.md`'s "Verification status" section for the
project-wide version of this same caveat). "The test exists and is
correct Go" is not the same claim as "this resource has been applied
`dashboard_data_source_test.go`, and likewise for the other three) --
eight real acceptance tests total, using
[`terraform-plugin-testing`][testing] -- skipped unless `TF_ACC=1` is
set, that framework's own standard convention, the same shape every
other live-infrastructure test in this repo uses (`docker`-gated env
vars for Postgres/ClickHouse tests elsewhere). The rule and target
*resource* tests both use a `plancheck.ExpectResourceAction` assertion
proving a config change actually plans a destroy-then-create, not an
in-place update -- the concrete, checked version of the "create/destroy
only" design decision documented above, not just a claim in a comment;
the panel resource test instead proves a genuine in-place update
(a `title` change with no `plancheck` needed, since the default
expectation -- update, not replace -- is exactly what should happen);
the *data source* tests each create a resource then look it up via
`resource.TestCheckResourceAttrPair`, proving the data source's `Read`
actually agrees with what the resource wrote, not just that both
compile. Even with `TF_ACC=1` all eight tests also need real running
`api`/`alerting` services (Postgres + ClickHouse) to apply against,
which this environment has no Docker access to bring up -- **not run
here**, same disclosed gap as every other live-infra test across this
repo (see `/docs/phase-4-runbook.md`'s "Verification status" section
for the project-wide version of this same caveat). "The test exists and
is correct Go" is not the same claim as "this resource has been applied
for real."
[testing]: https://developer.hashicorp.com/terraform/plugin/testing