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

Add sentry_alert_rule, the Terraform provider's second resource

Browse Source 
Confirmed with the project owner first: alerting's REST API has no
PUT /rules/{id} at all -- confirmed down to rulestore.Store, which has
Create/List/Get/Delete but no Update method to even wire one to, a real
pre-existing gap in alerting's own API, not something new to this task.
Decided to model sentry_alert_rule as create/destroy only rather than
fake an in-place update via delete-then-recreate inside the resource:
every attribute carries a RequiresReplace plan modifier, so a config
change destroys and recreates the rule, surfacing in the plan output the
real side effect that has (alert_state/delivery-log continuity resets)
instead of hiding it. Adding a real PUT /rules/{id} to alerting would
remove this constraint but is a change to a different module's REST
API, out of scope here.

internal/provider/client.go's new rule type and createRule/getRule/
deleteRule methods talk the exact same JSON contract
sentryctl alerts apply already uses against alerting/internal/httpapi.
GET /rules/{id} actually returns rulestore.RuleWithState (Rule's fields
promoted via anonymous embedding, plus a "state" object) -- the local
rule type has no field for "state" by design, and a new client test
proves that extra key doesn't break parsing.

alerting is a genuinely separate service from api (its own base URL),
so this needed the provider to talk to more than one Sentry service for
the first time: providerData now wraps two *client instances (api,
alerting), with a new alerting_endpoint provider attribute defaulting
the same way sentryctl's --alerting-api/$SENTRYCTL_ALERTING_API_URL
does. dashboardResource's Configure updated to pull .api out of the new
wrapper type instead of a bare *client.

Schema mirrors sentry_dashboard's established pattern: comparator/
threshold_value/renotify_interval_minutes stay nullable (only meaningful
for threshold-condition rules), enabled/for_minutes/query_language are
Optional+Computed with a Terraform-side default matching the API's own
default (true/0/"") rather than leaving the API as sole source of truth
the way dashboard's default_earliest/default_latest deliberately do --
these three have no *pointer* type in the API's Rule struct, so their
"default when omitted" is unconditional, not a real API-side default
that could drift independently.

Verified: client tests are real httptest.Server round trips (same
pattern as sentry_dashboard's). Schema validation needs no Terraform
binary. TestAccAlertRuleResource_basic is a real acceptance test,
skip-gated by TF_ACC same as the dashboard one, including a
plancheck.ExpectResourceAction assertion that a config change actually
plans destroy-then-create -- the concrete, checked version of the
"create/destroy only" design decision, not just a comment. 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 00:15:45 -07:00
parent 49dd050689
commit b98f397221
11 changed files with 742 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
terraform/README.md
+81 -28
View File
@@ -3,25 +3,30 @@
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. This is a first
slice, not a finished provider: one resource (`sentry_dashboard`), built
on [HashiCorp's `terraform-plugin-framework`][framework] (the actively-
developed library, not the legacy SDKv2 -- there's no existing provider
code here to migrate, so there's no reason to start on the framework
HashiCorp itself steers new providers away from).
no phase before this one had actually built any of it. Two resources so
far (`sentry_dashboard`, `sentry_alert_rule`), not a finished provider,
built on [HashiCorp's `terraform-plugin-framework`][framework] (the
actively-developed library, not the legacy SDKv2 -- there's no existing
provider code here to migrate, so there's no reason to start on the
framework HashiCorp itself steers new providers away from).
[framework]: https://developer.hashicorp.com/terraform/plugin/framework
## Why `sentry_dashboard` first
## Why these two resources first
`cli/README.md` already frames the underlying REST contract this way:
`cli/README.md` already frames the dashboards REST contract this way:
`POST /dashboards`, `GET`/`PUT`/`DELETE /dashboards/{id}` are "the seed
of a future Terraform provider: one JSON contract, multiple callers (web
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.
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.
## What's built
@@ -62,19 +67,57 @@ 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.
```hcl
provider "sentry" {
endpoint = "http://localhost:8080" # or $SENTRY_API_ENDPOINT
alerting_endpoint = "http://localhost:8081" # or $SENTRY_ALERTING_API_ENDPOINT -- alerting is a separate service, own base URL
token = var.sentry_api_token # or $SENTRY_API_TOKEN -- shared by both services, same as sentryctl's one $SENTRYCTL_TOKEN
}
resource "sentry_alert_rule" "example" {
name = "Checkout 5xx spike"
query = "service=checkout status>=500 | stats count"
condition_type = "threshold"
comparator = "gt"
threshold_value = 50
eval_interval_seconds = 60
notification_target_id = "target-abc123" # must already exist -- see "Also not built" below
}
```
Supports `terraform import sentry_alert_rule.example <rule-id>`.
**`sentry_alert_rule` is create/destroy only, not update-in-place.**
`alerting`'s REST API has no `PUT /rules/{id}` at all -- confirmed down
to `rulestore.Store`, which has `Create`/`List`/`Get`/`Delete` but no
`Update` method to even wire one to, a real, pre-existing gap in
`alerting`'s own API, not something specific to Terraform. Every
attribute on this resource carries a `RequiresReplace` plan modifier, so
changing anything (the query, the threshold, even just `description`)
destroys and recreates the rule rather than updating it -- which also
resets `alert_state`/delivery-log continuity for that rule, a real
operational side effect worth knowing about before relying on this in
a pipeline that changes rules often. Faking an in-place update via
delete-then-recreate inside the resource itself was considered and
rejected for the same reason: it would hide that side effect instead of
surfacing it in the plan output the way `RequiresReplace` does. Adding a
real `PUT /rules/{id}` to `alerting` would remove this constraint, but
is a change to a different module's REST API, out of scope for this
pass.
**Also not built, all real and disclosed, not attempted here:**
- Alert rules / notification targets (`/alerting`'s REST surface --
`POST /rules`, etc.) -- a second provider "family" of resources, no
code shared with dashboards beyond this same client-pattern
discipline.
- Notification targets (`/alerting`'s `POST`/`GET`/`DELETE /targets`) --
a `sentry_alert_rule`'s `notification_target_id` has to name a target
created some other way (`sentryctl`, `curl`, the web UI) until this
exists.
- Tenant/RBAC resources (`enterprise-auth`'s tenant/membership/grant
surface) -- meaningfully different auth model (offline operator flags
today, not a stable REST API a provider could safely drive
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."
- A `sentry_dashboard` data source (read-only lookup by ID/name) --
straightforward given the resource already exists, just not built
- Data sources (read-only lookup by ID/name) for either resource --
straightforward given the resources already exist, just not built
yet.
- Publishing to the real Terraform Registry -- `main.go`'s `Address`
(`registry.terraform.io/sentry/sentry`) is the address a real
@@ -92,29 +135,39 @@ go test ./...
`internal/provider/client_test.go` runs real HTTP round trips against a
`httptest.Server` (same pattern `cli/cmd/sentryctl`'s own tests use
against the same `api/dashboards` endpoints) -- real request
against the same `api/dashboards`/`alerting` endpoints) -- real request
construction (method, path, `Authorization` header, JSON body), real
response parsing, including the 404-vs-other-error distinction
`Read`/`Delete` need to implement Terraform's "resource deleted
out-of-band" convention correctly.
`internal/provider/provider_test.go` validates the provider and
out-of-band" convention correctly, and (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.
`internal/provider/provider_test.go` validates the provider and both
resource schemas are internally well-formed (attribute names, the
`Required`/`Computed` split) without needing a Terraform binary or a
live `api` service at all.
live `api`/`alerting` service at all.
`internal/provider/dashboard_resource_test.go`'s
`TestAccDashboardResource_basic` is a real acceptance test using
`TestAccDashboardResource_basic` and
`internal/provider/alert_rule_resource_test.go`'s
`TestAccAlertRuleResource_basic` are real acceptance tests 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). Even with `TF_ACC=1` it
also needs a real running `api` service (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."
vars for Postgres/ClickHouse tests elsewhere). The alert rule test also
uses 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. Even with
`TF_ACC=1` both 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