Skip to content

Commit

Permalink
fix: Fix failover group alter syntax and suppression for pipe stateme…
Browse files Browse the repository at this point in the history 
…nt (#2562)

- Run two separate alters in failover group resource
- Handle `.` in failover ids
- Trim leading and trailing space for pipe statement diff suppression
- Unskip part of the integration tests for failover groups

References: #2544 #2560
  • Loading branch information
@sfc-gh-asawicki
sfc-gh-asawicki authored Feb 27, 2024
1 parent d1e8912 commit 24d76c3
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 63 deletions.
2 changes: 2 additions & 0 deletions pkg/acceptance/testenvs/testing_environment_variables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
53 changes: 30 additions & 23 deletions pkg/resources/failover_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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())
Expand Down
64 changes: 47 additions & 17 deletions pkg/resources/failover_group_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) },
Expand Down Expand Up @@ -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) },
Expand Down Expand Up @@ -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) },
Expand Down Expand Up @@ -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) },
Expand All @@ -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" {
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions pkg/resources/pipe.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 9 additions & 7 deletions pkg/resources/pipe_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand All @@ -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" {
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions pkg/sdk/failover_groups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit 24d76c3

Please sign in to comment.