From 70edd3e5e010dcd2d2a6aa1bd1dd735a252d22c6 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Thu, 21 Dec 2023 12:23:54 +0100 Subject: [PATCH] fix: Fix email notification integration (#2292) * Change allowed recipients to optional * Use my own email for email notification integration tests * Fix the docs * Add comment --- .../email_notification_integration.md | 2 +- .../email_notification_integration.go | 33 ++++++++-- ...otification_integration_acceptance_test.go | 61 ++++++++++++++++--- 3 files changed, 81 insertions(+), 15 deletions(-) diff --git a/docs/resources/email_notification_integration.md b/docs/resources/email_notification_integration.md index 4034ee5a33..741df1bc70 100644 --- a/docs/resources/email_notification_integration.md +++ b/docs/resources/email_notification_integration.md @@ -27,12 +27,12 @@ resource "snowflake_email_notification_integration" "email_int" { ### Required -- `allowed_recipients` (Set of String) List of email addresses that should receive notifications. - `enabled` (Boolean) - `name` (String) ### Optional +- `allowed_recipients` (Set of String) List of email addresses that should receive notifications. - `comment` (String) A comment for the email integration. ### Read-Only diff --git a/pkg/resources/email_notification_integration.go b/pkg/resources/email_notification_integration.go index 01dfc9ba5a..48f9f99133 100644 --- a/pkg/resources/email_notification_integration.go +++ b/pkg/resources/email_notification_integration.go @@ -4,6 +4,7 @@ import ( "database/sql" "fmt" "log" + "regexp" "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" @@ -24,7 +25,7 @@ var emailNotificationIntegrationSchema = map[string]*schema.Schema{ "allowed_recipients": { Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString}, - Required: true, + Optional: true, Description: "List of email addresses that should receive notifications.", }, "comment": { @@ -58,7 +59,10 @@ func CreateEmailNotificationIntegration(d *schema.ResourceData, meta interface{} stmt.SetString("TYPE", "EMAIL") stmt.SetBool(`ENABLED`, d.Get("enabled").(bool)) - stmt.SetStringList(`ALLOWED_RECIPIENTS`, expandStringList(d.Get("allowed_recipients").(*schema.Set).List())) + + if v, ok := d.GetOk("allowed_recipients"); ok { + stmt.SetStringList(`ALLOWED_RECIPIENTS`, expandStringList(v.(*schema.Set).List())) + } if v, ok := d.GetOk("comment"); ok { stmt.SetString(`COMMENT`, v.(string)) @@ -115,8 +119,18 @@ func ReadEmailNotificationIntegration(d *schema.ResourceData, meta interface{}) } switch k { case "ALLOWED_RECIPIENTS": - if err := d.Set("allowed_recipients", strings.Split(v.(string), ",")); err != nil { - return err + // Empty list returns strange string (it's empty on worksheet level). + // This is a quick workaround, should be fixed with moving the email integration to SDK. + r := regexp.MustCompile(`[[:print:]]`) + if r.MatchString(v.(string)) { + if err := d.Set("allowed_recipients", strings.Split(v.(string), ",")); err != nil { + return err + } + } else { + empty := make([]string, 0) + if err := d.Set("allowed_recipients", empty); err != nil { + return err + } } default: log.Printf("[WARN] unexpected property %v returned from Snowflake", k) @@ -142,7 +156,16 @@ func UpdateEmailNotificationIntegration(d *schema.ResourceData, meta interface{} } if d.HasChange("allowed_recipients") { - stmt.SetStringList(`ALLOWED_RECIPIENTS`, expandStringList(d.Get("allowed_recipients").(*schema.Set).List())) + if v, ok := d.GetOk("allowed_recipients"); ok { + stmt.SetStringList(`ALLOWED_RECIPIENTS`, expandStringList(v.(*schema.Set).List())) + } else { + // raw sql for now; will be updated with SDK rewrite + // https://docs.snowflake.com/en/sql-reference/sql/alter-notification-integration#syntax + unset := fmt.Sprintf(`ALTER NOTIFICATION INTEGRATION "%s" UNSET ALLOWED_RECIPIENTS`, id) + if err := snowflake.Exec(db, unset); err != nil { + return fmt.Errorf("error unsetting allowed recipients on email notification integration %v err = %w", id, err) + } + } } if err := snowflake.Exec(db, stmt.Statement()); err != nil { diff --git a/pkg/resources/email_notification_integration_acceptance_test.go b/pkg/resources/email_notification_integration_acceptance_test.go index 45086b5b75..ace8bcaf9c 100644 --- a/pkg/resources/email_notification_integration_acceptance_test.go +++ b/pkg/resources/email_notification_integration_acceptance_test.go @@ -2,7 +2,6 @@ package resources_test import ( "fmt" - "os" "strings" "testing" @@ -11,22 +10,21 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) +// TODO: use email of our service user func TestAcc_EmailNotificationIntegration(t *testing.T) { - env := os.Getenv("SKIP_EMAIL_INTEGRATION_TESTS") - if env != "" { - t.Skip("Skipping TestAcc_EmailNotificationIntegration") - } - emailIntegrationName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + verifiedEmail := "artur.sawicki@snowflake.com" + resource.Test(t, resource.TestCase{ Providers: acc.TestAccProviders(), PreCheck: func() { acc.TestAccPreCheck(t) }, CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: emailNotificationIntegrationConfig(emailIntegrationName), + Config: emailNotificationIntegrationConfig(emailIntegrationName, verifiedEmail), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "name", emailIntegrationName), + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients.0", verifiedEmail), ), }, { @@ -38,12 +36,12 @@ func TestAcc_EmailNotificationIntegration(t *testing.T) { }) } -func emailNotificationIntegrationConfig(name string) string { +func emailNotificationIntegrationConfig(name string, email string) string { s := ` resource "snowflake_email_notification_integration" "test" { name = "%s" enabled = true - allowed_recipients = ["joe@domain.com"] # Verified Email Addresses is required + allowed_recipients = ["%s"] # Verified Email Addresses is required comment = "test" /* Error: error creating notification integration: 394209 (22023): @@ -52,5 +50,50 @@ Either these email addresses are not yet validated or do not belong to any user */ } ` + return fmt.Sprintf(s, name, email) +} + +// TestAcc_EmailNotificationIntegration_issue2223 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2223 issue. +// Snowflake allowed empty allowed recipients in https://docs.snowflake.com/en/release-notes/2023/7_40#email-notification-integrations-allowed-recipients-no-longer-required. +func TestAcc_EmailNotificationIntegration_issue2223(t *testing.T) { + emailIntegrationName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + verifiedEmail := "artur.sawicki@snowflake.com" + + resource.Test(t, resource.TestCase{ + Providers: acc.TestAccProviders(), + PreCheck: func() { acc.TestAccPreCheck(t) }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: emailNotificationIntegrationWithoutRecipientsConfig(emailIntegrationName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "name", emailIntegrationName), + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients.#", "0"), + ), + }, + { + Config: emailNotificationIntegrationConfig(emailIntegrationName, verifiedEmail), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "name", emailIntegrationName), + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients.0", verifiedEmail), + ), + }, + { + Config: emailNotificationIntegrationWithoutRecipientsConfig(emailIntegrationName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "name", emailIntegrationName), + resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients.#", "0"), + ), + }, + }, + }) +} + +func emailNotificationIntegrationWithoutRecipientsConfig(name string) string { + s := ` +resource "snowflake_email_notification_integration" "test" { + name = "%s" + enabled = true +}` return fmt.Sprintf(s, name) }