fix: Fix failover group alter syntax and suppression for pipe statement

pkg/acceptance/testenvs/testing_environment_variables.go

@@ -14,6 +14,8 @@ const (
 	Account  env = "TEST_SF_TF_ACCOUNT"
 	Role     env = "TEST_SF_TF_ROLE"
 	Host     env = "TEST_SF_TF_HOST"
+
+	BusinessCriticalAccount env = "SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"
 )
 
 func GetOrSkipTest(t *testing.T, envName Env) string {

pkg/resources/failover_group.go

@@ -423,29 +423,6 @@ func UpdateFailoverGroup(d *schema.ResourceData, meta interface{}) error {
 		runSet = true
 	}
 
-	if d.HasChange("replication_schedule") {
-		_, n := d.GetChange("replication_schedule")
-		replicationSchedule := n.([]interface{})[0].(map[string]interface{})
-		c := replicationSchedule["cron"].([]interface{})
-		if len(c) > 0 {
-			if len(c) > 0 {
-				cron := c[0].(map[string]interface{})
-				cronExpression := cron["expression"].(string)
-				cronExpression = "USING CRON " + cronExpression
-				if v, ok := cron["time_zone"]; ok {
-					timeZone := v.(string)
-					if timeZone != "" {
-						cronExpression = cronExpression + " " + timeZone
-					}
-				}
-				opts.Set.ReplicationSchedule = &cronExpression
-			}
-		} else {
-			opts.Set.ReplicationSchedule = sdk.String(fmt.Sprintf("%d MINUTE", replicationSchedule["interval"].(int)))
-		}
-		runSet = true
-	}
-
 	if d.HasChange("allowed_integration_types") {
 		_, n := d.GetChange("allowed_integration_types")
 		newAllowedIntegrationTypes := expandStringList(n.(*schema.Set).List())
@@ -462,6 +439,36 @@ func UpdateFailoverGroup(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	if d.HasChange("replication_schedule") {
+		_, n := d.GetChange("replication_schedule")
+		replicationSchedule := n.([]interface{})[0].(map[string]interface{})
+		c := replicationSchedule["cron"].([]interface{})
+		var updatedReplicationSchedule string
+		if len(c) > 0 {
+			cron := c[0].(map[string]interface{})
+			cronExpression := cron["expression"].(string)
+			cronExpression = "USING CRON " + cronExpression
+			if v, ok := cron["time_zone"]; ok {
+				timeZone := v.(string)
+				if timeZone != "" {
+					cronExpression = cronExpression + " " + timeZone
+				}
+			}
+			updatedReplicationSchedule = cronExpression
+		} else {
+			updatedReplicationSchedule = fmt.Sprintf("%d MINUTE", replicationSchedule["interval"].(int))
+		}
+
+		err := client.FailoverGroups.AlterSource(ctx, id, &sdk.AlterSourceFailoverGroupOptions{
+			Set: &sdk.FailoverGroupSet{
+				ReplicationSchedule: sdk.String(updatedReplicationSchedule),
+			},
+		})
+		if err != nil {
+			return err
+		}
+	}
+
 	if d.HasChange("allowed_databases") {
 		o, n := d.GetChange("allowed_databases")
 		oad := expandStringList(o.(*schema.Set).List())

pkg/resources/failover_group_acceptance_test.go

@@ -2,12 +2,12 @@ package resources_test
 
 import (
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
@@ -16,10 +16,7 @@ import (
 func TestAcc_FailoverGroupBasic(t *testing.T) {
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
-	if _, ok := os.LookupEnv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"); !ok {
-		t.Skip("Skipping TestAcc_FailoverGroup since not a business critical account")
-	}
-	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
+	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -54,10 +51,7 @@ func TestAcc_FailoverGroupBasic(t *testing.T) {
 func TestAcc_FailoverGroupRemoveObjectTypes(t *testing.T) {
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
-	if _, ok := os.LookupEnv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"); !ok {
-		t.Skip("Skipping TestAcc_FailoverGroup since not a business critical account")
-	}
-	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
+	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -95,10 +89,7 @@ func TestAcc_FailoverGroupRemoveObjectTypes(t *testing.T) {
 func TestAcc_FailoverGroupInterval(t *testing.T) {
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
-	if _, ok := os.LookupEnv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"); !ok {
-		t.Skip("Skipping TestAcc_FailoverGroup since not a business critical account")
-	}
-	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
+	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -194,10 +185,7 @@ func TestAcc_FailoverGroupInterval(t *testing.T) {
 func TestAcc_FailoverGroup_issue2517(t *testing.T) {
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
-	if _, ok := os.LookupEnv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"); !ok {
-		t.Skip("Skipping TestAcc_FailoverGroup since not a business critical account")
-	}
-	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
+	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -222,6 +210,34 @@ func TestAcc_FailoverGroup_issue2517(t *testing.T) {
 	})
 }
 
+func TestAcc_FailoverGroup_issue2544(t *testing.T) {
+	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
+	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				Config: failoverGroupBasic(randomCharacters, accountName, acc.TestDatabaseName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "name", randomCharacters),
+				),
+			},
+			{
+				Config: failoverGroupWithChanges(randomCharacters, accountName, 20),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "name", randomCharacters),
+				),
+			},
+		},
+	})
+}
+
 func failoverGroupBasic(randomCharacters, accountName, databaseName string) string {
 	return fmt.Sprintf(`
 resource "snowflake_failover_group" "fg" {
@@ -304,3 +320,17 @@ resource "snowflake_failover_group" "fg" {
 }
 `, randomCharacters, accountName, databaseName)
 }
+
+func failoverGroupWithChanges(randomCharacters string, accountName string, interval int) string {
+	return fmt.Sprintf(`
+resource "snowflake_failover_group" "fg" {
+	name = "%[1]s"
+	object_types = ["DATABASES", "INTEGRATIONS"]
+	allowed_accounts= ["%[2]s"]
+	allowed_integration_types = ["NOTIFICATION INTEGRATIONS"]
+	replication_schedule {
+		interval = %d
+	}
+}
+`, randomCharacters, accountName, interval)
+}

pkg/resources/pipe.go

@@ -101,8 +101,8 @@ func pipeCopyStatementDiffSuppress(_, o, n string, _ *schema.ResourceData) bool
 	o = strings.ReplaceAll(o, "\r\n", "\n")
 	n = strings.ReplaceAll(n, "\r\n", "\n")
 
-	// trim off any trailing line endings
-	return strings.TrimRight(o, ";\r\n") == strings.TrimRight(n, ";\r\n")
+	// trim off any trailing line endings and leading/trailing whitespace
+	return strings.TrimSpace(strings.TrimRight(o, ";\r\n")) == strings.TrimSpace(strings.TrimRight(n, ";\r\n"))
 }
 
 // CreatePipe implements schema.CreateFunc.

pkg/resources/pipe_acceptance_test.go

@@ -2,24 +2,25 @@ package resources_test
 
 import (
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
+
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 
 func TestAcc_Pipe(t *testing.T) {
-	if _, ok := os.LookupEnv("SKIP_PIPE_TESTS"); ok {
-		t.Skip("Skipping TestAcc_Pipe")
-	}
 	accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	resource.Test(t, resource.TestCase{
-		Providers:    acc.TestAccProviders(),
-		PreCheck:     func() { acc.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
 		CheckDestroy: nil,
 		Steps: []resource.TestStep{
 			{
@@ -37,6 +38,7 @@ func TestAcc_Pipe(t *testing.T) {
 	})
 }
 
+// whitespace in copy_statement matters for the tests, change with caution!
 func pipeConfig(name string, databaseName string, schemaName string) string {
 	s := `
 resource "snowflake_table" "test" {
@@ -69,7 +71,7 @@ resource "snowflake_pipe" "test" {
   name           = "%s"
   comment        = "Terraform acceptance test"
   copy_statement = <<CMD
-COPY INTO "${snowflake_table.test.database}"."${snowflake_table.test.schema}"."${snowflake_table.test.name}"
+  	COPY INTO "${snowflake_table.test.database}"."${snowflake_table.test.schema}"."${snowflake_table.test.name}"
   FROM @"${snowflake_stage.test.database}"."${snowflake_stage.test.schema}"."${snowflake_stage.test.name}"
   FILE_FORMAT = (TYPE = CSV)
 CMD

pkg/sdk/failover_groups.go

@@ -183,8 +183,8 @@ func (opts *AlterSourceFailoverGroupOptions) validate() error {
 
 type FailoverGroupSet struct {
 	ObjectTypes             []PluralObjectType `ddl:"parameter" sql:"OBJECT_TYPES"`
-	ReplicationSchedule     *string            `ddl:"parameter,single_quotes" sql:"REPLICATION_SCHEDULE"`
 	AllowedIntegrationTypes []IntegrationType  `ddl:"parameter" sql:"ALLOWED_INTEGRATION_TYPES"`
+	ReplicationSchedule     *string            `ddl:"parameter,single_quotes" sql:"REPLICATION_SCHEDULE"`
 }
 
 func (v *FailoverGroupSet) validate() error {
@@ -574,15 +574,17 @@ func (v *failoverGroups) ShowShares(ctx context.Context, id AccountObjectIdentif
 		return nil, err
 	}
 	dest := []struct {
-		Name string `db:"name"`
+		Name         string `db:"name"`
+		OwnerAccount string `db:"owner_account"`
 	}{}
 	err = v.client.query(ctx, &dest, sql)
 	if err != nil {
 		return nil, err
 	}
 	resultList := make([]AccountObjectIdentifier, len(dest))
-	for i, row := range dest {
-		resultList[i] = NewExternalObjectIdentifierFromFullyQualifiedName(row.Name).objectIdentifier.(AccountObjectIdentifier)
+	for i, r := range dest {
+		// TODO [SNOW-999049]: this was not working correctly with identifiers containing `.` character
+		resultList[i] = NewExternalObjectIdentifier(NewAccountIdentifierFromFullyQualifiedName(r.OwnerAccount), NewAccountObjectIdentifier(r.Name)).objectIdentifier.(AccountObjectIdentifier)
 	}
 	return resultList, nil
 }