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

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

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 {

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 := "[email protected]"
+
 	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 = ["[email protected]"] # 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 := "[email protected]"
+
+	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)
 }