From 7e91ec03fd381647ecf44a855b93812d433eeabd Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Wed, 20 Dec 2023 18:31:15 +0100 Subject: [PATCH 1/4] Change allowed recipients to optional --- .../email_notification_integration.go | 31 ++++++++++++--- ...otification_integration_acceptance_test.go | 38 +++++++++++++++++-- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/pkg/resources/email_notification_integration.go b/pkg/resources/email_notification_integration.go index 01dfc9ba5a..d5355c9b84 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,16 @@ 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 + 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 +154,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..29db46e422 100644 --- a/pkg/resources/email_notification_integration_acceptance_test.go +++ b/pkg/resources/email_notification_integration_acceptance_test.go @@ -18,15 +18,18 @@ func TestAcc_EmailNotificationIntegration(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: 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 +41,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 +55,34 @@ 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)) + 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.TestCheckNoResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients"), + ), + }, + }, + }) +} + +func emailNotificationIntegrationWithoutRecipientsConfig(name string) string { + s := ` +resource "snowflake_email_notification_integration" "test" { + name = "%s" + enabled = true +}` return fmt.Sprintf(s, name) } From 72e94c47fe0f455cc8c7a30c7fe4790c5524a5bd Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Wed, 20 Dec 2023 18:48:36 +0100 Subject: [PATCH 2/4] Use my own email for email notification integration tests --- ...otification_integration_acceptance_test.go | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/resources/email_notification_integration_acceptance_test.go b/pkg/resources/email_notification_integration_acceptance_test.go index 29db46e422..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,12 +10,8 @@ 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" @@ -62,6 +57,8 @@ Either these email addresses are not yet validated or do not belong to any user // 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) }, @@ -71,7 +68,21 @@ func TestAcc_EmailNotificationIntegration_issue2223(t *testing.T) { Config: emailNotificationIntegrationWithoutRecipientsConfig(emailIntegrationName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_email_notification_integration.test", "name", emailIntegrationName), - resource.TestCheckNoResourceAttr("snowflake_email_notification_integration.test", "allowed_recipients"), + 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"), ), }, }, From 13d77cac67b8c7b360ab18e17795c05d1a4c8140 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Wed, 20 Dec 2023 18:56:20 +0100 Subject: [PATCH 3/4] Fix the docs --- docs/resources/email_notification_integration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 57bc4189ffd6b4b2a04bb4337203a42ef748e701 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Thu, 21 Dec 2023 11:27:32 +0100 Subject: [PATCH 4/4] Add comment --- pkg/resources/email_notification_integration.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/resources/email_notification_integration.go b/pkg/resources/email_notification_integration.go index d5355c9b84..48f9f99133 100644 --- a/pkg/resources/email_notification_integration.go +++ b/pkg/resources/email_notification_integration.go @@ -119,6 +119,8 @@ func ReadEmailNotificationIntegration(d *schema.ResourceData, meta interface{}) } switch k { case "ALLOWED_RECIPIENTS": + // 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 {