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) + } +}