From 186930e645f638d3560b01e387b54996344b3e4a Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Tue, 19 Dec 2023 16:20:19 -0500 Subject: [PATCH 1/2] Alerting Contact Points: Refer by name Background: Currently, the contact point's ID is a collection of notifier UIDs. However, in Grafana (and in the resource's import ref), it's referred to by name This PR: Change the Terraform ID from semi-colon separated UIDs to the contact point name. This will be easier to test, and this will allow simplifying the code in the next major version (removing the code fetching notifiers by UID) Since the UID reading logic is moved to the read function, it also allows importing contact points in Crossplane where the import function is not used --- docs/resources/contact_point.md | 40 ++-- internal/common/errcheck.go | 6 +- .../resource_alerting_contact_point.go | 185 +++++++++--------- .../resource_alerting_contact_point_test.go | 42 +++- 4 files changed, 149 insertions(+), 124 deletions(-) diff --git a/docs/resources/contact_point.md b/docs/resources/contact_point.md index 6a3f3defa..97a34a419 100644 --- a/docs/resources/contact_point.md +++ b/docs/resources/contact_point.md @@ -42,26 +42,26 @@ resource "grafana_contact_point" "my_contact_point" { ### Optional -- `alertmanager` (Block List) A contact point that sends notifications to other Alertmanager instances. (see [below for nested schema](#nestedblock--alertmanager)) -- `dingding` (Block List) A contact point that sends notifications to DingDing. (see [below for nested schema](#nestedblock--dingding)) -- `discord` (Block List) A contact point that sends notifications as Discord messages (see [below for nested schema](#nestedblock--discord)) -- `email` (Block List) A contact point that sends notifications to an email address. (see [below for nested schema](#nestedblock--email)) -- `googlechat` (Block List) A contact point that sends notifications to Google Chat. (see [below for nested schema](#nestedblock--googlechat)) -- `kafka` (Block List) A contact point that publishes notifications to Apache Kafka topics. (see [below for nested schema](#nestedblock--kafka)) -- `line` (Block List) A contact point that sends notifications to LINE.me. (see [below for nested schema](#nestedblock--line)) -- `oncall` (Block List) A contact point that sends notifications to Grafana On-Call. (see [below for nested schema](#nestedblock--oncall)) -- `opsgenie` (Block List) A contact point that sends notifications to OpsGenie. (see [below for nested schema](#nestedblock--opsgenie)) -- `pagerduty` (Block List) A contact point that sends notifications to PagerDuty. (see [below for nested schema](#nestedblock--pagerduty)) -- `pushover` (Block List) A contact point that sends notifications to Pushover. (see [below for nested schema](#nestedblock--pushover)) -- `sensugo` (Block List) A contact point that sends notifications to SensuGo. (see [below for nested schema](#nestedblock--sensugo)) -- `slack` (Block List) A contact point that sends notifications to Slack. (see [below for nested schema](#nestedblock--slack)) -- `teams` (Block List) A contact point that sends notifications to Microsoft Teams. (see [below for nested schema](#nestedblock--teams)) -- `telegram` (Block List) A contact point that sends notifications to Telegram. (see [below for nested schema](#nestedblock--telegram)) -- `threema` (Block List) A contact point that sends notifications to Threema. (see [below for nested schema](#nestedblock--threema)) -- `victorops` (Block List) A contact point that sends notifications to VictorOps (now known as Splunk OnCall). (see [below for nested schema](#nestedblock--victorops)) -- `webex` (Block List) A contact point that sends notifications to Cisco Webex. (see [below for nested schema](#nestedblock--webex)) -- `webhook` (Block List) A contact point that sends notifications to an arbitrary webhook, using the Prometheus webhook format defined here: https://prometheus.io/docs/alerting/latest/configuration/#webhook_config (see [below for nested schema](#nestedblock--webhook)) -- `wecom` (Block List) A contact point that sends notifications to WeCom. (see [below for nested schema](#nestedblock--wecom)) +- `alertmanager` (Block Set) A contact point that sends notifications to other Alertmanager instances. (see [below for nested schema](#nestedblock--alertmanager)) +- `dingding` (Block Set) A contact point that sends notifications to DingDing. (see [below for nested schema](#nestedblock--dingding)) +- `discord` (Block Set) A contact point that sends notifications as Discord messages (see [below for nested schema](#nestedblock--discord)) +- `email` (Block Set) A contact point that sends notifications to an email address. (see [below for nested schema](#nestedblock--email)) +- `googlechat` (Block Set) A contact point that sends notifications to Google Chat. (see [below for nested schema](#nestedblock--googlechat)) +- `kafka` (Block Set) A contact point that publishes notifications to Apache Kafka topics. (see [below for nested schema](#nestedblock--kafka)) +- `line` (Block Set) A contact point that sends notifications to LINE.me. (see [below for nested schema](#nestedblock--line)) +- `oncall` (Block Set) A contact point that sends notifications to Grafana On-Call. (see [below for nested schema](#nestedblock--oncall)) +- `opsgenie` (Block Set) A contact point that sends notifications to OpsGenie. (see [below for nested schema](#nestedblock--opsgenie)) +- `pagerduty` (Block Set) A contact point that sends notifications to PagerDuty. (see [below for nested schema](#nestedblock--pagerduty)) +- `pushover` (Block Set) A contact point that sends notifications to Pushover. (see [below for nested schema](#nestedblock--pushover)) +- `sensugo` (Block Set) A contact point that sends notifications to SensuGo. (see [below for nested schema](#nestedblock--sensugo)) +- `slack` (Block Set) A contact point that sends notifications to Slack. (see [below for nested schema](#nestedblock--slack)) +- `teams` (Block Set) A contact point that sends notifications to Microsoft Teams. (see [below for nested schema](#nestedblock--teams)) +- `telegram` (Block Set) A contact point that sends notifications to Telegram. (see [below for nested schema](#nestedblock--telegram)) +- `threema` (Block Set) A contact point that sends notifications to Threema. (see [below for nested schema](#nestedblock--threema)) +- `victorops` (Block Set) A contact point that sends notifications to VictorOps (now known as Splunk OnCall). (see [below for nested schema](#nestedblock--victorops)) +- `webex` (Block Set) A contact point that sends notifications to Cisco Webex. (see [below for nested schema](#nestedblock--webex)) +- `webhook` (Block Set) A contact point that sends notifications to an arbitrary webhook, using the Prometheus webhook format defined here: https://prometheus.io/docs/alerting/latest/configuration/#webhook_config (see [below for nested schema](#nestedblock--webhook)) +- `wecom` (Block Set) A contact point that sends notifications to WeCom. (see [below for nested schema](#nestedblock--wecom)) ### Read-Only diff --git a/internal/common/errcheck.go b/internal/common/errcheck.go index 9cfe88196..3f7539b2d 100644 --- a/internal/common/errcheck.go +++ b/internal/common/errcheck.go @@ -25,6 +25,10 @@ func CheckReadError(resourceType string, d *schema.ResourceData, err error) (ret return diag.Errorf("error reading %s with ID`%s`: %v", resourceType, d.Id(), err), true } + return WarnMissing(resourceType, d), true +} + +func WarnMissing(resourceType string, d *schema.ResourceData) diag.Diagnostics { log.Printf("[WARN] removing %s with ID %q from state because it no longer exists in grafana", resourceType, d.Id()) var diags diag.Diagnostics diags = append(diags, diag.Diagnostic{ @@ -33,7 +37,7 @@ func CheckReadError(resourceType string, d *schema.ResourceData, err error) (ret Detail: fmt.Sprintf("%q will be recreated when you apply", d.Id()), }) d.SetId("") - return diags, true + return diags } func IsNotFoundError(err error) bool { diff --git a/internal/resources/grafana/resource_alerting_contact_point.go b/internal/resources/grafana/resource_alerting_contact_point.go index eb73ba480..c48132e7d 100644 --- a/internal/resources/grafana/resource_alerting_contact_point.go +++ b/internal/resources/grafana/resource_alerting_contact_point.go @@ -3,7 +3,6 @@ package grafana import ( "context" "fmt" - "log" "strings" "github.com/grafana/grafana-openapi-client-go/client/provisioning" @@ -53,7 +52,7 @@ This resource requires Grafana 9.1.0 or later. DeleteContext: deleteContactPoint, Importer: &schema.ResourceImporter{ - StateContext: importContactPoint, + StateContext: schema.ImportStatePassthroughContext, }, SchemaVersion: 0, @@ -74,7 +73,7 @@ This resource requires Grafana 9.1.0 or later. for _, n := range notifiers { resource.Schema[n.meta().field] = &schema.Schema{ - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Description: n.meta().desc, Elem: n.schema(), @@ -85,62 +84,55 @@ This resource requires Grafana 9.1.0 or later. return resource } -func importContactPoint(ctx context.Context, data *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - name := data.Id() - client := OAPIGlobalClient(meta) // TODO: Support org-scoped contact points - - params := provisioning.NewGetContactpointsParams().WithName(&name) - resp, err := client.Provisioning.GetContactpoints(params) - if err != nil { - return nil, err - } - ps := resp.Payload - - if len(ps) == 0 { - return nil, fmt.Errorf("no contact points with the given name were found to import") - } - - uids := make([]string, 0, len(ps)) - for _, p := range ps { - uids = append(uids, p.UID) - } - - data.SetId(packUIDs(uids)) - return []*schema.ResourceData{data}, nil -} - func readContactPoint(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics { client := OAPIGlobalClient(meta) // TODO: Support org-scoped contact points - uidsToFetch := unpackUIDs(data.Id()) - - resp, err := client.Provisioning.GetContactpoints(nil) + // First, try to fetch the contact point by name. + // If that fails, try to fetch it by the UID of its notifiers. + name := data.Id() + resp, err := client.Provisioning.GetContactpoints(provisioning.NewGetContactpointsParams().WithName(&name)) if err != nil { return diag.FromErr(err) } - contactPointByUID := map[string]*models.EmbeddedContactPoint{} - for _, p := range resp.Payload { - contactPointByUID[p.UID] = p - } + points := resp.Payload + if len(points) == 0 { + // If the contact point was not found by name, try to fetch it by UID. + // This is a deprecated ID format (uid;uid2;uid3) + // TODO: Remove on the next major version + uidsMap := map[string]bool{} + for _, uid := range strings.Split(data.Id(), ";") { + uidsMap[uid] = false + } + resp, err := client.Provisioning.GetContactpoints(provisioning.NewGetContactpointsParams()) + if err != nil { + return diag.FromErr(err) + } + for i, p := range resp.Payload { + if _, ok := uidsMap[p.UID]; !ok { + continue + } + uidsMap[p.UID] = true + points = append(points, p) + if i > 0 && p.Name != points[0].Name { + return diag.FromErr(fmt.Errorf("contact point with UID %s has a different name (%s) than the contact point with UID %s (%s)", p.UID, p.Name, points[0].UID, points[0].Name)) + } + } - points := []*models.EmbeddedContactPoint{} - for _, uid := range uidsToFetch { - p, ok := contactPointByUID[uid] - if !ok { - log.Printf("[WARN] removing contact point %s from state because it no longer exists in grafana", uid) - continue + for uid, found := range uidsMap { + if !found { + // Since this is an import, all UIDs should exist + return diag.FromErr(fmt.Errorf("contact point with UID %s was not found", uid)) + } } - points = append(points, p) + } + + if len(points) == 0 { + return common.WarnMissing("contact point", data) } if err := packContactPoints(points, data); err != nil { return diag.FromErr(err) } - uids := make([]string, 0, len(points)) - for _, p := range points { - uids = append(uids, p.UID) - } - data.SetId(packUIDs(uids)) return nil } @@ -151,42 +143,56 @@ func updateContactPoint(ctx context.Context, data *schema.ResourceData, meta int defer lock.Unlock() client := OAPIGlobalClient(meta) // TODO: Support org-scoped contact points - existingUIDs := unpackUIDs(data.Id()) ps := unpackContactPoints(data) - unprocessedUIDs := toUIDSet(existingUIDs) - newUIDs := make([]string, 0, len(ps)) - for i := range ps { - p := ps[i].gfState - delete(unprocessedUIDs, p.UID) - params := provisioning.NewPutContactpointParams().WithUID(p.UID).WithBody(p) - _, err := client.Provisioning.PutContactpoint(params) - if err != nil { - if common.IsNotFoundError(err) { - params := provisioning.NewPostContactpointsParams().WithBody(p) - resp, err := client.Provisioning.PostContactpoints(params) - ps[i].tfState["uid"] = resp.Payload.UID - newUIDs = append(newUIDs, resp.Payload.UID) - if err != nil { - return diag.FromErr(err) - } - continue - } + // If the contact point already exists, we need to fetch its current state so that we can compare it to the proposed state. + var currentPoints models.ContactPoints + if !data.IsNewResource() { + name := data.Id() + resp, err := client.Provisioning.GetContactpoints(provisioning.NewGetContactpointsParams().WithName(&name)) + if err != nil && !common.IsNotFoundError(err) { return diag.FromErr(err) } - newUIDs = append(newUIDs, p.UID) + if resp != nil { + currentPoints = resp.Payload + } } - // Any UIDs still left in the state that we haven't seen must map to deleted receivers. - // Delete them on the server and drop them from state. - for u := range unprocessedUIDs { - if _, err := client.Provisioning.DeleteContactpoints(u); err != nil { - return diag.FromErr(err) + processedUIDs := map[string]bool{} + for i := range ps { + p := ps[i] + var uid string + if uid = p.tfState["uid"].(string); uid != "" { + // If the contact point already has a UID, update it. + params := provisioning.NewPutContactpointParams().WithUID(uid).WithBody(p.gfState) + if _, err := client.Provisioning.PutContactpoint(params); err != nil { + return diag.FromErr(err) + } + } else { + // If the contact point does not have a UID, create it. + resp, err := client.Provisioning.PostContactpoints(provisioning.NewPostContactpointsParams().WithBody(p.gfState)) + if err != nil { + return diag.FromErr(err) + } + uid = resp.Payload.UID } + + // Since this is a new resource, the proposed state won't have a UID. + // We need the UID so that we can later associate it with the config returned in the api response. + ps[i].tfState["uid"] = uid + processedUIDs[uid] = true } - data.SetId(packUIDs(newUIDs)) + for _, p := range currentPoints { + if _, ok := processedUIDs[p.UID]; !ok { + // If the contact point is not in the proposed state, delete it. + if _, err := client.Provisioning.DeleteContactpoints(p.UID); err != nil { + return diag.Errorf("failed to remove contact point notifier with UID %s from contact point %s: %v", p.UID, p.Name, err) + } + } + } + data.SetId(data.Get("name").(string)) return readContactPoint(ctx, data, meta) } @@ -196,15 +202,19 @@ func deleteContactPoint(ctx context.Context, data *schema.ResourceData, meta int defer lock.Unlock() client := OAPIGlobalClient(meta) // TODO: Support org-scoped contact points - uids := unpackUIDs(data.Id()) + name := data.Id() + resp, err := client.Provisioning.GetContactpoints(provisioning.NewGetContactpointsParams().WithName(&name)) + if err, shouldReturn := common.CheckReadError("contact point", data, err); shouldReturn { + return err + } - for _, uid := range uids { - if _, err := client.Provisioning.DeleteContactpoints(uid); err != nil { + for _, cp := range resp.Payload { + if _, err := client.Provisioning.DeleteContactpoints(cp.UID); err != nil { return diag.FromErr(err) } } - return diag.Diagnostics{} + return nil } func unpackContactPoints(data *schema.ResourceData) []statePair { @@ -212,7 +222,7 @@ func unpackContactPoints(data *schema.ResourceData) []statePair { name := data.Get("name").(string) for _, n := range notifiers { if points, ok := data.GetOk(n.meta().field); ok { - for _, p := range points.([]interface{}) { + for _, p := range points.(*schema.Set).List() { result = append(result, statePair{ tfState: p.(map[string]interface{}), gfState: unpackPointConfig(n, p, name), @@ -240,6 +250,7 @@ func packContactPoints(ps []*models.EmbeddedContactPoint, data *schema.ResourceD pointsPerNotifier := map[notifier][]interface{}{} for _, p := range ps { data.Set("name", p.Name) + data.SetId(p.Name) for _, n := range notifiers { if *p.Type == n.meta().typeStr { @@ -307,24 +318,6 @@ func commonNotifierResource() *schema.Resource { } } -const UIDSeparator = ";" - -func packUIDs(uids []string) string { - return strings.Join(uids, UIDSeparator) -} - -func unpackUIDs(packed string) []string { - return strings.Split(packed, UIDSeparator) -} - -func toUIDSet(uids []string) map[string]bool { - set := map[string]bool{} - for _, uid := range uids { - set[uid] = true - } - return set -} - type notifier interface { meta() notifierMeta schema() *schema.Resource @@ -367,7 +360,7 @@ func unpackNotifierStringField(tfSettings, gfSettings *map[string]interface{}, t func getNotifierConfigFromStateWithUID(data *schema.ResourceData, n notifier, uid string) map[string]interface{} { if points, ok := data.GetOk(n.meta().field); ok { - for _, pt := range points.([]interface{}) { + for _, pt := range points.(*schema.Set).List() { config := pt.(map[string]interface{}) if config["uid"] == uid { return config diff --git a/internal/resources/grafana/resource_alerting_contact_point_test.go b/internal/resources/grafana/resource_alerting_contact_point_test.go index a8384a7fd..075bd3d17 100644 --- a/internal/resources/grafana/resource_alerting_contact_point_test.go +++ b/internal/resources/grafana/resource_alerting_contact_point_test.go @@ -3,6 +3,7 @@ package grafana_test import ( "fmt" "regexp" + "strings" "testing" gapi "github.com/grafana/grafana-api-golang-client" @@ -76,7 +77,6 @@ func TestAccContactPoint_compound(t *testing.T) { var points []gapi.ContactPoint // TODO: Make parallelizable - // Error: wrong number of contact points on the server, expected 2 but got []{..., ..., ...} (len=3) resource.Test(t, resource.TestCase{ ProviderFactories: testutils.ProviderFactories, // Implicitly tests deletion. @@ -91,6 +91,34 @@ func TestAccContactPoint_compound(t *testing.T) { resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.#", "2"), ), }, + // Test import by name + { + ResourceName: "grafana_contact_point.compound_contact_point", + ImportState: true, + ImportStateId: "Compound Contact Point", + ImportStateVerify: true, + }, + // Test import by UID + { + ResourceName: "grafana_contact_point.compound_contact_point", + ImportState: true, + ImportStateVerify: true, + ImportStateIdFunc: func(s *terraform.State) (string, error) { + resource, ok := s.RootModule().Resources["grafana_contact_point.compound_contact_point"] + if !ok { + return "", fmt.Errorf("resource not found") + } + firstUID, ok := resource.Primary.Attributes["email.0.uid"] + if !ok { + return "", fmt.Errorf("uid for first email notifier not found") + } + secondUID, ok := resource.Primary.Attributes["email.1.uid"] + if !ok { + return "", fmt.Errorf("uid for second email notifier not found") + } + return strings.Join([]string{firstUID, secondUID}, ";"), nil + }, + }, // Test update. { Config: testutils.TestAccExampleWithReplace(t, "resources/grafana_contact_point/_acc_compound_receiver.tf", map[string]string{ @@ -108,9 +136,9 @@ func TestAccContactPoint_compound(t *testing.T) { Check: resource.ComposeTestCheckFunc( testContactPointCheckExists("grafana_contact_point.compound_contact_point", &points, 3), resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.#", "3"), - resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.0.addresses.0", "one@company.org"), - resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.1.addresses.0", "three@company.org"), - resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.2.addresses.0", "five@company.org"), + resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.0.addresses.0", "five@company.org"), + resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.1.addresses.0", "one@company.org"), + resource.TestCheckResourceAttr("grafana_contact_point.compound_contact_point", "email.2.addresses.0", "three@company.org"), ), }, // Test removal of a point from a compound one does not leak. @@ -320,15 +348,15 @@ func TestAccContactPoint_notifiers(t *testing.T) { if !ok { return fmt.Errorf("resource not found: %s, resources: %#v", rname, s.RootModule().Resources) } - uid := rs.Primary.ID + name := rs.Primary.ID client := testutils.Provider.Meta().(*common.Client).DeprecatedGrafanaAPI - pt, err := client.ContactPoint(uid) + pt, err := client.ContactPointsByName(name) if err != nil { return fmt.Errorf("error getting resource: %w", err) } - if val, ok := pt.Settings["endpointUrl"]; ok { + if val, ok := pt[0].Settings["endpointUrl"]; ok { return fmt.Errorf("endpointUrl was still present in the settings when it should have been omitted. value: %#v", val) } From d9a2a4fdb640ddbae201f1cc246bd5d701dcb15d Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Thu, 21 Dec 2023 10:59:39 -0500 Subject: [PATCH 2/2] ForceNew on the name because it's now the primary ID --- internal/resources/grafana/resource_alerting_contact_point.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/resources/grafana/resource_alerting_contact_point.go b/internal/resources/grafana/resource_alerting_contact_point.go index c48132e7d..9b46495ee 100644 --- a/internal/resources/grafana/resource_alerting_contact_point.go +++ b/internal/resources/grafana/resource_alerting_contact_point.go @@ -59,6 +59,7 @@ This resource requires Grafana 9.1.0 or later. Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, + ForceNew: true, Required: true, Description: "The name of the contact point.", },