Prove and fix issue #2517

Add to migration guide Remove comment
Snowflake-Labs · Feb 19, 2024 · 25024e8 · 25024e8
1 parent a523a6b
commit 25024e8
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 15 deletions.
diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md
@@ -4,6 +4,12 @@ This document is meant to help you migrate your Terraform config to the new newe
 describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior
 across different versions.
 
+## v0.86.0 ➞ v0.87.0
+### snowflake_failover_group resource changes
+#### *(bug fix)* ACCOUNT PARAMETERS is returned as PARAMETERS from SHOW FAILOVER GROUPS
+Longer context in [#2517](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2517).
+After this change, one apply may be required to update the state correctly for failover group resources using `ACCOUNT PARAMETERS`.
+
 ## v0.85.0 ➞ v0.86.0
 ### snowflake_table_constraint resource changes
 

diff --git a/pkg/datasources/failover_groups_acceptance_test.go b/pkg/datasources/failover_groups_acceptance_test.go
@@ -6,8 +6,11 @@ import (
 	"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_FailoverGroups(t *testing.T) {
@@ -18,18 +21,21 @@ func TestAcc_FailoverGroups(t *testing.T) {
 
 	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	resource.ParallelTest(t, resource.TestCase{
-		Providers:    providers(),
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
 		CheckDestroy: nil,
 		Steps: []resource.TestStep{
 			{
 				Config: failoverGroupsConfig(name, accountName),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.#", "1"),
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.0.name", name),
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.0.object_types.#", "1"),
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.0.object_types.0", "ROLES"),
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.0.allowed_accounts.#", "1"),
-					resource.TestCheckResourceAttr("snowflake_failover_groups.d", "failover_groups.0.allowed_accounts.0", accountName),
+					resource.TestCheckResourceAttr("data.snowflake_failover_groups.d", "failover_groups.#", "1"),
+					resource.TestCheckResourceAttr("data.snowflake_failover_groups.d", "failover_groups.0.object_types.#", "1"),
+					resource.TestCheckResourceAttr("data.snowflake_failover_groups.d", "failover_groups.0.object_types.0", "ROLES"),
+					resource.TestCheckResourceAttr("data.snowflake_failover_groups.d", "failover_groups.0.allowed_accounts.#", "1"),
+					resource.TestCheckResourceAttr("data.snowflake_failover_groups.d", "failover_groups.0.allowed_accounts.0", accountName),
 				),
 			},
 		},

diff --git a/pkg/resources/failover_group_acceptance_test.go b/pkg/resources/failover_group_acceptance_test.go
@@ -7,8 +7,10 @@ import (
 	"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_FailoverGroupBasic(t *testing.T) {
@@ -19,8 +21,11 @@ func TestAcc_FailoverGroupBasic(t *testing.T) {
 	}
 	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
 	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{
 			{
@@ -54,8 +59,11 @@ func TestAcc_FailoverGroupRemoveObjectTypes(t *testing.T) {
 	}
 	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
 	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{
 			{
@@ -92,8 +100,11 @@ func TestAcc_FailoverGroupInterval(t *testing.T) {
 	}
 	accountName := os.Getenv("SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT")
 	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{
 			{
@@ -180,11 +191,42 @@ 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")
+	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: failoverGroupWithAccountParameters(randomCharacters, accountName, acc.TestDatabaseName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "name", randomCharacters),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "object_types.#", "5"),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "allowed_accounts.#", "1"),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "allowed_databases.#", "1"),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "allowed_integration_types.#", "1"),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "replication_schedule.0.cron.0.expression", "0 0 10-20 * TUE,THU"),
+					resource.TestCheckResourceAttr("snowflake_failover_group.fg", "replication_schedule.0.cron.0.time_zone", "UTC"),
+				),
+			},
+		},
+	})
+}
+
 func failoverGroupBasic(randomCharacters, accountName, databaseName string) string {
 	return fmt.Sprintf(`
 resource "snowflake_failover_group" "fg" {
 	name = "%s"
-	object_types = ["WAREHOUSES","DATABASES", "INTEGRATIONS", "ROLES"]
+	object_types = ["WAREHOUSES", "DATABASES", "INTEGRATIONS", "ROLES"]
 	allowed_accounts= ["%s"]
 	allowed_databases = ["%s"]
 	allowed_integration_types = ["SECURITY INTEGRATIONS"]
@@ -244,3 +286,21 @@ resource "snowflake_failover_group" "fg" {
 }
 `, randomCharacters, accountName, databaseName, expression)
 }
+
+func failoverGroupWithAccountParameters(randomCharacters, accountName, databaseName string) string {
+	return fmt.Sprintf(`
+resource "snowflake_failover_group" "fg" {
+	name = "%s"
+	object_types = ["ACCOUNT PARAMETERS", "WAREHOUSES", "DATABASES", "INTEGRATIONS", "ROLES"]
+	allowed_accounts= ["%s"]
+	allowed_databases = ["%s"]
+	allowed_integration_types = ["SECURITY INTEGRATIONS"]
+	replication_schedule {
+		cron {
+			expression = "0 0 10-20 * TUE,THU"
+			time_zone = "UTC"
+		}
+	}
+}
+`, randomCharacters, accountName, databaseName)
+}
diff --git a/pkg/sdk/failover_groups.go b/pkg/sdk/failover_groups.go
@@ -410,7 +410,12 @@ func (row failoverGroupDBRow) convert() *FailoverGroup {
 	ots := strings.Split(row.ObjectTypes, ",")
 	pluralObjectTypes := make([]PluralObjectType, 0, len(ots))
 	for _, ot := range ots {
-		pluralObjectTypes = append(pluralObjectTypes, PluralObjectType(strings.TrimSpace(ot)))
+		pot := PluralObjectType(strings.TrimSpace(ot))
+		if pot == PluralObjectTypeParameters {
+			pluralObjectTypes = append(pluralObjectTypes, PluralObjectType("ACCOUNT PARAMETERS"))
+		} else {
+			pluralObjectTypes = append(pluralObjectTypes, pot)
+		}
 	}
 	its := strings.Split(row.AllowedIntegrationTypes, ",")
 	allowedIntegrationTypes := make([]IntegrationType, 0, len(its))