From 30ae84cd04dd81a9bf208d2b06182eadb9d1eb75 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sat, 15 Aug 2026 10:20:52 -0700 Subject: [PATCH] Add sentry_notification_target, closing the alert-rule-as-code loop sentry_alert_rule.notification_target_id could previously only point at a target created outside Terraform (sentryctl/curl/the web UI) -- without this resource, "manage alert rules as code" was only half true. Same create/destroy-only shape as sentry_alert_rule and for the same reason: alerting has no PUT /targets/{id} either, confirmed down to notifystore.Store (Create/List/Get/Delete, no Update). client.go's notificationTarget type mirrors notifystore.Target's JSON shape. headers stays raw JSON bytes end to end -- the client has no opinion about its shape (neither does alerting's own Target type, json.RawMessage), and the resource layer round-trips it as a plain JSON-text string a caller provides via Terraform's jsonencode(). secret is marked Sensitive in the schema, but alerting's own GET /targets/{id} returns it unredacted (confirmed in notifystore/store.go -- no redaction at the store or handler layer, an existing property of alerting's API, not something this provider introduces). A new client test (TestGetNotificationTargetReturnsSecretUnredacted) documents that real behavior so a future change to it would be caught here, not discovered by surprise. Sensitive keeps the value out of plan/apply console output; it does not keep it out of Terraform state, the standard caveat for any sensitive attribute, named explicitly in the schema description and README rather than left implicit. Examples updated end to end: sentry_alert_rule's example now creates a real sentry_notification_target and references its .id, instead of a placeholder string. Verified: client tests are real httptest.Server round trips. Schema validation needs no Terraform binary. TestAccNotificationTargetResource_basic is a real acceptance test, skip-gated by TF_ACC same as the other two, including a plancheck.ExpectResourceAction assertion that a config change actually plans destroy-then-create, and (since secret really does round-trip unredacted) a real ImportStateVerify on the secret attribute rather than one papered over with ImportStateVerifyIgnore. Not run against a live stack in this environment, same disclosed gap as everything else Docker-gated in this repo. --- CLAUDE.md | 14 +- terraform/README.md | 120 +++++---- .../resources/sentry_alert_rule/resource.tf | 26 +- .../sentry_notification_target/import.sh | 1 + .../sentry_notification_target/resource.tf | 24 ++ terraform/internal/provider/client.go | 55 ++++ terraform/internal/provider/client_test.go | 75 ++++++ .../provider/notification_target_resource.go | 248 ++++++++++++++++++ .../notification_target_resource_test.go | 74 ++++++ terraform/internal/provider/provider.go | 3 +- terraform/internal/provider/provider_test.go | 34 +++ 11 files changed, 607 insertions(+), 67 deletions(-) create mode 100644 terraform/examples/resources/sentry_notification_target/import.sh create mode 100644 terraform/examples/resources/sentry_notification_target/resource.tf create mode 100644 terraform/internal/provider/notification_target_resource.go create mode 100644 terraform/internal/provider/notification_target_resource_test.go diff --git a/CLAUDE.md b/CLAUDE.md index f01ac17..5b22ee7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,15 +20,15 @@ described there without flagging it to me first. UI-only logic. CLI (`sentryctl`) and Terraform provider are first-class, not afterthoughts. **Status**: `sentryctl` has been built out phase by phase since Phase 3. The Terraform provider (`/terraform`) only exists - as of this note -- two resources (`sentry_dashboard`, full CRUD; - `sentry_alert_rule`, create/destroy only -- `alerting` has no - `PUT /rules/{id}` to update against), built on HashiCorp's + as of this note -- three resources (`sentry_dashboard`, full CRUD; + `sentry_alert_rule` and `sentry_notification_target`, both create/ + destroy only -- `alerting` has no `PUT /rules/{id}` or + `PUT /targets/{id}` to update against), built on HashiCorp's `terraform-plugin-framework`, reusing the exact same REST contracts `sentryctl dashboards apply`/web's dashboard export and - `sentryctl alerts apply` already use. Notification targets and - tenant/RBAC resources are real, disclosed future work -- see - `/terraform/README.md` for the full accounting of what is and isn't - built, and the same + `sentryctl alerts apply` already use. Tenant/RBAC resources are real, + disclosed future work -- see `/terraform/README.md` for the full + accounting of what is and isn't built, and the same "written but not run against a live stack" verification caveat as everything else Docker-gated in this repo. diff --git a/terraform/README.md b/terraform/README.md index 990a0d7..ecbc9c8 100644 --- a/terraform/README.md +++ b/terraform/README.md @@ -3,16 +3,17 @@ 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. Two resources so -far (`sentry_dashboard`, `sentry_alert_rule`), not a finished provider, -built on [HashiCorp's `terraform-plugin-framework`][framework] (the +no phase before this one had actually built any of it. Three resources +so far (`sentry_dashboard`, `sentry_alert_rule`, +`sentry_notification_target`), 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 these two resources first +## Why these three resources first `cli/README.md` already frames the dashboards REST contract this way: `POST /dashboards`, `GET`/`PUT`/`DELETE /dashboards/{id}` are "the seed @@ -26,7 +27,11 @@ Terraform's sake. `sentry_alert_rule` follows against `alerting`'s own 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. +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), +undermining the point of managing rules as code at all. ## What's built @@ -74,6 +79,12 @@ provider "sentry" { token = var.sentry_api_token # or $SENTRY_API_TOKEN -- shared by both services, same as sentryctl's one $SENTRYCTL_TOKEN } +resource "sentry_notification_target" "ops" { + name = "Ops Webhook" + kind = "webhook" + webhook_url = "https://ops.example.com/hooks/sentry-alerts" +} + resource "sentry_alert_rule" "example" { name = "Checkout 5xx spike" query = "service=checkout status>=500 | stats count" @@ -81,44 +92,55 @@ resource "sentry_alert_rule" "example" { comparator = "gt" threshold_value = 50 eval_interval_seconds = 60 - notification_target_id = "target-abc123" # must already exist -- see "Also not built" below + notification_target_id = sentry_notification_target.ops.id } ``` -Supports `terraform import sentry_alert_rule.example `. +Supports `terraform import sentry_alert_rule.example ` and +`terraform import sentry_notification_target.example `. -**`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. +**`sentry_alert_rule` and `sentry_notification_target` are both +create/destroy only, not update-in-place.** `alerting`'s REST API has no +`PUT /rules/{id}` or `PUT /targets/{id}` at all -- confirmed down to +`rulestore.Store`/`notifystore.Store`, both of which have +`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 both resources carries a +`RequiresReplace` plan modifier, so changing anything (a rule's query or +threshold, a target's webhook URL, even just a `description`) destroys +and recreates it rather than updating it in place -- for rules, that +also resets `alert_state`/delivery-log continuity, 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 either resource 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 +real `PUT` endpoints to `alerting` would remove this constraint, but is +a change to a different module's REST API, out of scope for this pass. + +**`sentry_notification_target`'s `secret` attribute is `Sensitive` but +still lands in Terraform state in plaintext.** `alerting`'s own +`GET /targets/{id}` returns `secret` unredacted (confirmed in +`notifystore/store.go` -- no redaction at the store or handler layer, an +existing property of `alerting`'s API, not introduced by this +provider), and a resource has to store whatever `Read` returns to avoid +Terraform showing a permanent diff. `Sensitive: true` keeps it out of +plan/apply console output; it does not keep it out of the state file -- +the standard, well-known Terraform caveat for any sensitive attribute +(encrypt the state backend, restrict who can read it), named here rather +than left implicit. **Also not built, all real and disclosed, not attempted here:** -- 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." -- Data sources (read-only lookup by ID/name) for either resource -- - straightforward given the resources already exist, just not built - yet. +- Data sources (read-only lookup by ID/name) for any of the three + resources -- straightforward given the resources already exist, just + not built yet. +- 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 @@ -139,31 +161,35 @@ 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, and (for rules) proving the +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. -`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`/`alerting` service at all. +parse cleanly, and (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 and all +three resource schemas are internally well-formed (attribute names, the +`Required`/`Computed`/`Sensitive` split) without needing a Terraform +binary or a live `api`/`alerting` service at all. `internal/provider/dashboard_resource_test.go`'s -`TestAccDashboardResource_basic` and +`TestAccDashboardResource_basic`, `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 +`TestAccAlertRuleResource_basic`, and +`internal/provider/notification_target_resource_test.go`'s +`TestAccNotificationTargetResource_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). The alert rule test also -uses a `plancheck.ExpectResourceAction` assertion proving a config +vars for Postgres/ClickHouse tests elsewhere). The rule and target 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. 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 +`TF_ACC=1` all three 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 diff --git a/terraform/examples/resources/sentry_alert_rule/resource.tf b/terraform/examples/resources/sentry_alert_rule/resource.tf index 8f4e85c..0fd6a38 100644 --- a/terraform/examples/resources/sentry_alert_rule/resource.tf +++ b/terraform/examples/resources/sentry_alert_rule/resource.tf @@ -1,16 +1,18 @@ -# notification_target_id has to name an already-existing target -- -# no sentry_notification_target resource exists yet (see the provider -# README), so create one via sentryctl/curl/the web UI first and pass -# its id in here. +resource "sentry_notification_target" "ops_webhook" { + name = "Ops Webhook" + kind = "webhook" + webhook_url = "https://ops.example.com/hooks/sentry-alerts" +} + resource "sentry_alert_rule" "checkout_5xx" { - name = "Checkout 5xx spike" - query = "service=checkout status>=500 | stats count" - condition_type = "threshold" - comparator = "gt" - threshold_value = 50 - eval_interval_seconds = 60 - for_minutes = 5 - notification_target_id = "target-abc123" + name = "Checkout 5xx spike" + query = "service=checkout status>=500 | stats count" + condition_type = "threshold" + comparator = "gt" + threshold_value = 50 + eval_interval_seconds = 60 + for_minutes = 5 + notification_target_id = sentry_notification_target.ops_webhook.id } # Create/destroy only -- alerting has no PUT /rules/{id} today, so diff --git a/terraform/examples/resources/sentry_notification_target/import.sh b/terraform/examples/resources/sentry_notification_target/import.sh new file mode 100644 index 0000000..a17fd2b --- /dev/null +++ b/terraform/examples/resources/sentry_notification_target/import.sh @@ -0,0 +1 @@ +terraform import sentry_notification_target.ops_webhook diff --git a/terraform/examples/resources/sentry_notification_target/resource.tf b/terraform/examples/resources/sentry_notification_target/resource.tf new file mode 100644 index 0000000..d03eab4 --- /dev/null +++ b/terraform/examples/resources/sentry_notification_target/resource.tf @@ -0,0 +1,24 @@ +resource "sentry_notification_target" "ops_webhook" { + name = "Ops Webhook" + kind = "webhook" + webhook_url = "https://ops.example.com/hooks/sentry-alerts" + + # Optional. Sensitive -- not printed in plan/apply output, but note + # it's still stored in Terraform state in plaintext (alerting's own + # GET /targets/{id} returns it unredacted; see the provider README). + secret = var.ops_webhook_secret + + # Optional -- must be a JSON object string. + headers = jsonencode({ + "X-Team" = "platform" + }) +} + +variable "ops_webhook_secret" { + type = string + sensitive = true +} + +# Create/destroy only -- alerting has no PUT /targets/{id} today, so +# changing any attribute above destroys and recreates the target rather +# than updating it in place. See the provider README for why. diff --git a/terraform/internal/provider/client.go b/terraform/internal/provider/client.go index 75f0d01..48be006 100644 --- a/terraform/internal/provider/client.go +++ b/terraform/internal/provider/client.go @@ -200,3 +200,58 @@ func (c *client) getRule(ctx context.Context, id string) (*rule, error) { func (c *client) deleteRule(ctx context.Context, id string) error { return c.do(ctx, http.MethodDelete, "/rules/"+id, nil, nil) } + +// notificationTarget mirrors alerting/internal/notifystore.Target's +// JSON shape -- deliberately a local type, same "talk HTTP, not Go +// imports" posture as dashboard/rule above. Headers is left as raw +// JSON bytes (not decoded into a Go map) since this client has no +// opinion about its shape -- alerting's own Target type doesn't either +// (json.RawMessage), and the resource layer round-trips it as a plain +// JSON-text string a caller provides via Terraform's jsonencode(). +// +// Secret genuinely comes back from GET/List unredacted -- confirmed in +// notifystore/store.go's Get/List queries, which select the secret +// column with no redaction at either the store or handler layer. This +// is alerting's own existing behavior, not something this provider +// introduces or could fix from the client side; notificationTargetResource +// marks the corresponding attribute Sensitive so Terraform at least +// doesn't print it in plan/apply console output (it is still stored in +// Terraform state in plaintext -- a standard, disclosed Terraform +// limitation for any sensitive attribute, not specific to this one). +type notificationTarget struct { + ID string `json:"id,omitempty"` + TenantID string `json:"tenant_id,omitempty"` + Name string `json:"name"` + Kind string `json:"kind"` + WebhookURL string `json:"webhook_url"` + PayloadTemplate *string `json:"payload_template,omitempty"` + Headers json.RawMessage `json:"headers,omitempty"` + Secret *string `json:"secret,omitempty"` + CreatedBy string `json:"created_by,omitempty"` +} + +// createNotificationTarget and getNotificationTarget are the only +// mutating/reading calls this client makes against /targets -- same +// "no updateX method, alerting has no PUT /targets/{id} either" shape +// as rules above (rulestore.Store/notifystore.Store both only have +// Create/List/Get/Delete). notificationTargetResource is create/destroy +// only for the same reason alertRuleResource is. +func (c *client) createNotificationTarget(ctx context.Context, t *notificationTarget) (*notificationTarget, error) { + var out notificationTarget + if err := c.do(ctx, http.MethodPost, "/targets", t, &out); err != nil { + return nil, err + } + return &out, nil +} + +func (c *client) getNotificationTarget(ctx context.Context, id string) (*notificationTarget, error) { + var out notificationTarget + if err := c.do(ctx, http.MethodGet, "/targets/"+id, nil, &out); err != nil { + return nil, err + } + return &out, nil +} + +func (c *client) deleteNotificationTarget(ctx context.Context, id string) error { + return c.do(ctx, http.MethodDelete, "/targets/"+id, nil, nil) +} diff --git a/terraform/internal/provider/client_test.go b/terraform/internal/provider/client_test.go index 95e6edd..03ee334 100644 --- a/terraform/internal/provider/client_test.go +++ b/terraform/internal/provider/client_test.go @@ -241,3 +241,78 @@ func TestDeleteRuleSendsToCorrectPath(t *testing.T) { t.Fatal("expected the server to receive a DELETE request") } } + +func TestCreateNotificationTargetSendsExpectedRequest(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/targets" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + var body notificationTarget + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decoding request body: %v", err) + } + if body.Name != "Ops Webhook" || body.Kind != "webhook" { + t.Errorf("unexpected request body: %+v", body) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(notificationTarget{ + ID: "target-1", TenantID: "acme", Name: body.Name, Kind: body.Kind, WebhookURL: body.WebhookURL, + }) + })) + defer srv.Close() + + c := newClient(srv.URL, "") + out, err := c.createNotificationTarget(context.Background(), ¬ificationTarget{ + Name: "Ops Webhook", Kind: "webhook", WebhookURL: "https://example.com/hook", + }) + if err != nil { + t.Fatalf("createNotificationTarget: %v", err) + } + if out.ID != "target-1" { + t.Fatalf("unexpected response: %+v", out) + } +} + +func TestGetNotificationTargetReturnsSecretUnredacted(t *testing.T) { + // Documents real, existing alerting behavior (notifystore's Get + // query selects the secret column with no redaction) -- this test + // exists so a future change to alerting's redaction posture would + // be caught here too, not just discovered by surprise. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(notificationTarget{ID: "target-1", Name: "Ops Webhook", Kind: "webhook", Secret: strPtr("shh")}) + })) + defer srv.Close() + + c := newClient(srv.URL, "") + out, err := c.getNotificationTarget(context.Background(), "target-1") + if err != nil { + t.Fatalf("getNotificationTarget: %v", err) + } + if out.Secret == nil || *out.Secret != "shh" { + t.Fatalf("Secret = %v, want it echoed back unredacted", out.Secret) + } +} + +func TestDeleteNotificationTargetSendsToCorrectPath(t *testing.T) { + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + if r.Method != http.MethodDelete || r.URL.Path != "/targets/target-1" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + c := newClient(srv.URL, "") + if err := c.deleteNotificationTarget(context.Background(), "target-1"); err != nil { + t.Fatalf("deleteNotificationTarget: %v", err) + } + if !called { + t.Fatal("expected the server to receive a DELETE request") + } +} + +func strPtr(s string) *string { return &s } diff --git a/terraform/internal/provider/notification_target_resource.go b/terraform/internal/provider/notification_target_resource.go new file mode 100644 index 0000000..2df0ba6 --- /dev/null +++ b/terraform/internal/provider/notification_target_resource.go @@ -0,0 +1,248 @@ +package provider + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +var ( + _ resource.Resource = ¬ificationTargetResource{} + _ resource.ResourceWithConfigure = ¬ificationTargetResource{} + _ resource.ResourceWithImportState = ¬ificationTargetResource{} +) + +func newNotificationTargetResource() resource.Resource { + return ¬ificationTargetResource{} +} + +// notificationTargetResource implements sentry_notification_target +// against alerting/internal/httpapi's POST/GET/DELETE +// /targets[/{id}] endpoints. +// +// Deliberately create/destroy only, every attribute RequiresReplace -- +// same reasoning as alertRuleResource (see that file's doc comment): +// alerting has no PUT /targets/{id} at all, confirmed down to +// notifystore.Store, which has Create/List/Get/Delete but no Update. +type notificationTargetResource struct { + client *client +} + +type notificationTargetResourceModel struct { + ID types.String `tfsdk:"id"` + TenantID types.String `tfsdk:"tenant_id"` + Name types.String `tfsdk:"name"` + Kind types.String `tfsdk:"kind"` + WebhookURL types.String `tfsdk:"webhook_url"` + PayloadTemplate types.String `tfsdk:"payload_template"` + Headers types.String `tfsdk:"headers"` + Secret types.String `tfsdk:"secret"` + CreatedBy types.String `tfsdk:"created_by"` +} + +func (r *notificationTargetResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = req.ProviderTypeName + "_notification_target" +} + +func (r *notificationTargetResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + replace := []planmodifier.String{stringplanmodifier.RequiresReplace()} + resp.Schema = schema.Schema{ + Description: "A Sentry alert notification target. Create/destroy only -- alerting has no update " + + "endpoint for targets today (see this resource's Go doc comment), so every attribute below " + + "forces a destroy-and-recreate on change, never an in-place update.", + Attributes: map[string]schema.Attribute{ + "id": schema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, + Description: "Server-generated target ID -- reference this from a sentry_alert_rule's notification_target_id.", + }, + "tenant_id": schema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, + }, + "name": schema.StringAttribute{ + Required: true, + PlanModifiers: replace, + Description: "Target name. The API rejects an empty string.", + }, + "kind": schema.StringAttribute{ + Required: true, + PlanModifiers: replace, + Description: `One of "webhook", "slack", "pagerduty".`, + }, + "webhook_url": schema.StringAttribute{ + Required: true, + PlanModifiers: replace, + Description: "Destination URL. The API rejects an empty string, regardless of kind.", + }, + "payload_template": schema.StringAttribute{ + Optional: true, + PlanModifiers: replace, + Description: "Optional Go text/template string overriding the default payload shape for this target's kind.", + }, + "headers": schema.StringAttribute{ + Optional: true, + PlanModifiers: replace, + Description: `Optional extra HTTP headers, as a JSON object string -- e.g. jsonencode({"X-Custom" = "value"}). Stored and returned as opaque JSON; this provider does not interpret it.`, + }, + "secret": schema.StringAttribute{ + Optional: true, + Sensitive: true, + PlanModifiers: replace, + Description: "Optional shared secret (e.g. for HMAC-signing outgoing webhook payloads). " + + "alerting's GET /targets/{id} returns this back unredacted (confirmed in " + + "notifystore/store.go -- no redaction at the store or handler layer), so it is " + + "necessarily present in this resource's Terraform state in plaintext, the standard " + + "caveat for any Sensitive Terraform attribute: treat state files as sensitive, " + + "encrypt the backend, restrict who can read them.", + }, + "created_by": schema.StringAttribute{ + Computed: true, + PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}, + }, + }, + } +} + +func (r *notificationTargetResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + if req.ProviderData == nil { + return + } + data, ok := req.ProviderData.(*providerData) + if !ok { + resp.Diagnostics.AddError( + "Unexpected Resource Configure Type", + fmt.Sprintf("Expected *provider.providerData, got: %T. This is a provider bug -- please report it.", req.ProviderData), + ) + return + } + r.client = data.alerting +} + +func notificationTargetModelFromAPI(t *notificationTarget) notificationTargetResourceModel { + m := notificationTargetResourceModel{ + ID: types.StringValue(t.ID), + TenantID: types.StringValue(t.TenantID), + Name: types.StringValue(t.Name), + Kind: types.StringValue(t.Kind), + WebhookURL: types.StringValue(t.WebhookURL), + CreatedBy: types.StringValue(t.CreatedBy), + } + if t.PayloadTemplate != nil { + m.PayloadTemplate = types.StringValue(*t.PayloadTemplate) + } + if len(t.Headers) > 0 { + m.Headers = types.StringValue(string(t.Headers)) + } + if t.Secret != nil { + m.Secret = types.StringValue(*t.Secret) + } + return m +} + +func notificationTargetAPIFromModel(m notificationTargetResourceModel) (*notificationTarget, error) { + t := ¬ificationTarget{ + Name: m.Name.ValueString(), + Kind: m.Kind.ValueString(), + WebhookURL: m.WebhookURL.ValueString(), + } + if !m.PayloadTemplate.IsNull() { + v := m.PayloadTemplate.ValueString() + t.PayloadTemplate = &v + } + if !m.Headers.IsNull() { + raw := m.Headers.ValueString() + if !json.Valid([]byte(raw)) { + return nil, fmt.Errorf("headers must be valid JSON (use jsonencode(...) in the resource config), got: %s", raw) + } + t.Headers = json.RawMessage(raw) + } + if !m.Secret.IsNull() { + v := m.Secret.ValueString() + t.Secret = &v + } + return t, nil +} + +func (r *notificationTargetResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan notificationTargetResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + in, err := notificationTargetAPIFromModel(plan) + if err != nil { + resp.Diagnostics.AddError("Invalid Configuration", err.Error()) + return + } + out, err := r.client.createNotificationTarget(ctx, in) + if err != nil { + resp.Diagnostics.AddError("Creating Notification Target", err.Error()) + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, notificationTargetModelFromAPI(out))...) +} + +func (r *notificationTargetResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + var state notificationTargetResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + out, err := r.client.getNotificationTarget(ctx, state.ID.ValueString()) + if err != nil { + if isNotFound(err) { + resp.State.RemoveResource(ctx) + return + } + resp.Diagnostics.AddError("Reading Notification Target", err.Error()) + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, notificationTargetModelFromAPI(out))...) +} + +// Update should be unreachable in practice -- see alertRuleResource's +// Update doc comment for why this is a safe read-only passthrough +// rather than calling any mutating endpoint (alerting has none to call). +func (r *notificationTargetResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + var plan notificationTargetResourceModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } + + out, err := r.client.getNotificationTarget(ctx, plan.ID.ValueString()) + if err != nil { + resp.Diagnostics.AddError("Reading Notification Target", err.Error()) + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, notificationTargetModelFromAPI(out))...) +} + +func (r *notificationTargetResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + var state notificationTargetResourceModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() { + return + } + + if err := r.client.deleteNotificationTarget(ctx, state.ID.ValueString()); err != nil && !isNotFound(err) { + resp.Diagnostics.AddError("Deleting Notification Target", err.Error()) + } +} + +func (r *notificationTargetResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp) +} diff --git a/terraform/internal/provider/notification_target_resource_test.go b/terraform/internal/provider/notification_target_resource_test.go new file mode 100644 index 0000000..396fb55 --- /dev/null +++ b/terraform/internal/provider/notification_target_resource_test.go @@ -0,0 +1,74 @@ +package provider + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" +) + +// Same skip-gated-not-faked posture as TestAccDashboardResource_basic / +// TestAccAlertRuleResource_basic -- see those tests' doc comments. +func TestAccNotificationTargetResource_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: ` +provider "sentry" { + endpoint = "http://localhost:8080" + alerting_endpoint = "http://localhost:8081" +} + +resource "sentry_notification_target" "test" { + name = "Acceptance Test Target" + kind = "webhook" + webhook_url = "https://example.com/hook" + secret = "test-secret" +} +`, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("sentry_notification_target.test", "name", "Acceptance Test Target"), + resource.TestCheckResourceAttr("sentry_notification_target.test", "kind", "webhook"), + resource.TestCheckResourceAttr("sentry_notification_target.test", "secret", "test-secret"), + resource.TestCheckResourceAttrSet("sentry_notification_target.test", "id"), + resource.TestCheckResourceAttrSet("sentry_notification_target.test", "tenant_id"), + ), + }, + { + // Same create/destroy-only proof as + // TestAccAlertRuleResource_basic's second step -- + // notifystore.Store has no Update either. + Config: ` +provider "sentry" { + endpoint = "http://localhost:8080" + alerting_endpoint = "http://localhost:8081" +} + +resource "sentry_notification_target" "test" { + name = "Renamed Target" + kind = "webhook" + webhook_url = "https://example.com/hook" + secret = "test-secret" +} +`, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("sentry_notification_target.test", plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + Check: resource.TestCheckResourceAttr("sentry_notification_target.test", "name", "Renamed Target"), + }, + { + // No ImportStateVerifyIgnore for "secret" -- GET + // really does return it unredacted (see client.go's + // doc comment and TestGetNotificationTargetReturnsSecretUnredacted), + // so import-time equality is a real, meaningful + // assertion here, not one this test has to paper over. + ResourceName: "sentry_notification_target.test", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} diff --git a/terraform/internal/provider/provider.go b/terraform/internal/provider/provider.go index d985bbd..3cbf515 100644 --- a/terraform/internal/provider/provider.go +++ b/terraform/internal/provider/provider.go @@ -57,7 +57,7 @@ func (p *sentryProvider) Metadata(_ context.Context, _ provider.MetadataRequest, func (p *sentryProvider) Schema(_ context.Context, _ provider.SchemaRequest, resp *provider.SchemaResponse) { resp.Schema = schema.Schema{ - Description: "Manages Sentry log-aggregation-platform resources. Dashboards and alert rules for now -- notification targets and tenant/RBAC resources are real, disclosed future work, not built in this pass; see the provider README.", + Description: "Manages Sentry log-aggregation-platform resources. Dashboards, alert rules, and notification targets for now -- tenant/RBAC resources are real, disclosed future work, not built in this pass; see the provider README.", Attributes: map[string]schema.Attribute{ "endpoint": schema.StringAttribute{ Optional: true, @@ -130,6 +130,7 @@ func (p *sentryProvider) Resources(_ context.Context) []func() resource.Resource return []func() resource.Resource{ newDashboardResource, newAlertRuleResource, + newNotificationTargetResource, } } diff --git a/terraform/internal/provider/provider_test.go b/terraform/internal/provider/provider_test.go index 0cb97dd..895a00b 100644 --- a/terraform/internal/provider/provider_test.go +++ b/terraform/internal/provider/provider_test.go @@ -99,3 +99,37 @@ func TestAlertRuleResourceMetadataSetsTypeName(t *testing.T) { t.Fatalf("TypeName = %q, want sentry_alert_rule", resp.TypeName) } } + +func TestNotificationTargetResourceSchemaValid(t *testing.T) { + ctx := context.Background() + req := resource.SchemaRequest{} + resp := &resource.SchemaResponse{} + + newNotificationTargetResource().Schema(ctx, req, resp) + + if resp.Diagnostics.HasError() { + t.Fatalf("sentry_notification_target schema has errors: %v", resp.Diagnostics) + } + for _, attr := range []string{ + "id", "tenant_id", "name", "kind", "webhook_url", + "payload_template", "headers", "secret", "created_by", + } { + if _, ok := resp.Schema.Attributes[attr]; !ok { + t.Errorf("sentry_notification_target schema missing expected attribute %q", attr) + } + } + if !resp.Schema.Attributes["name"].IsRequired() { + t.Error(`"name" must be Required`) + } + if !resp.Schema.Attributes["secret"].IsSensitive() { + t.Error(`"secret" must be Sensitive`) + } +} + +func TestNotificationTargetResourceMetadataSetsTypeName(t *testing.T) { + resp := &resource.MetadataResponse{} + newNotificationTargetResource().Metadata(context.Background(), resource.MetadataRequest{ProviderTypeName: "sentry"}, resp) + if resp.TypeName != "sentry_notification_target" { + t.Fatalf("TypeName = %q, want sentry_notification_target", resp.TypeName) + } +}