From 8f2a363db7ea0bcc747ef7c3576d24255f4bf208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BCl=20Bonet?= <64003978+raulbonet@users.noreply.github.com> Date: Mon, 6 May 2024 12:07:07 +0200 Subject: [PATCH] fix: unset network policy should not have single quotes (#2584) ref #2582 When unsetting, `NETWORK_POLICY` should not be enclosed between single quotes. This was not detected in the acceptance tests because these only covered user attachments; I added a test for an account attachment. --------- Co-authored-by: Artur Sawicki --- ...twork_policy_attachment_acceptance_test.go | 76 +++++++++++++++++-- pkg/sdk/parameters.go | 4 +- pkg/sdk/parameters_test.go | 29 +++++++ 3 files changed, 101 insertions(+), 8 deletions(-) diff --git a/pkg/resources/network_policy_attachment_acceptance_test.go b/pkg/resources/network_policy_attachment_acceptance_test.go index 20fb58a0c9..326e41e878 100644 --- a/pkg/resources/network_policy_attachment_acceptance_test.go +++ b/pkg/resources/network_policy_attachment_acceptance_test.go @@ -1,19 +1,26 @@ package resources_test import ( + "context" "fmt" + "strings" "testing" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func TestAcc_NetworkPolicyAttachment(t *testing.T) { +func TestAcc_NetworkPolicyAttachmentUser(t *testing.T) { user1 := acc.TestClient().Ids.Alpha() user2 := acc.TestClient().Ids.Alpha() - policyName := acc.TestClient().Ids.Alpha() + policyNameUser := acc.TestClient().Ids.Alpha() resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -24,17 +31,17 @@ func TestAcc_NetworkPolicyAttachment(t *testing.T) { CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: networkPolicyAttachmentConfigSingle(user1, policyName), + Config: networkPolicyAttachmentConfigSingle(user1, policyNameUser), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "network_policy_name", policyName), + resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "network_policy_name", policyNameUser), resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "set_for_account", "false"), resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "users.#", "1"), ), }, { - Config: networkPolicyAttachmentConfig(user1, user2, policyName), + Config: networkPolicyAttachmentConfig(user1, user2, policyNameUser), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "network_policy_name", policyName), + resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "network_policy_name", policyNameUser), resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "set_for_account", "false"), resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "users.#", "2"), ), @@ -47,6 +54,49 @@ func TestAcc_NetworkPolicyAttachment(t *testing.T) { }, }, }) + +} + +func TestAcc_NetworkPolicyAttachmentAccount(t *testing.T) { + policyNameAccount := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckNetworkPolicyAttachmentDestroy, + Steps: []resource.TestStep{ + { + Config: networkPolicyAttachmentConfigAccount(policyNameAccount), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "network_policy_name", policyNameAccount), + resource.TestCheckResourceAttr("snowflake_network_policy_attachment.test", "set_for_account", "true"), + ), + }, + }, + }) +} + +func testAccCheckNetworkPolicyAttachmentDestroy(s *terraform.State) error { + client := acc.TestAccProvider.Meta().(*provider.Context).Client + + for _, rs := range s.RootModule().Resources { + if rs.Type != "snowflake_network_policy_attachment" { + continue + } + ctx := context.Background() + parameter, err := client.Parameters.ShowAccountParameter(ctx, sdk.AccountParameterNetworkPolicy) + if err != nil { + fmt.Printf("[WARN] network policy (%s) not found on account", rs.Primary.Attributes["Id"]) + return nil + } + if err == nil && parameter.Level == "ACCOUNT" && parameter.Key == "NETWORK_POLICY" && parameter.Value == rs.Primary.Attributes["network_policy_name"] { + return fmt.Errorf("network policy attachment %v still exists", rs.Primary.Attributes["Id"]) + } + } + return nil } func networkPolicyAttachmentConfigSingle(user1, policyName string) string { @@ -90,3 +140,17 @@ resource "snowflake_network_policy_attachment" "test" { } `, user1, user2, policyName) } + +func networkPolicyAttachmentConfigAccount(policyName string) string { + return fmt.Sprintf(` +resource "snowflake_network_policy" "test" { + name = "%v" + allowed_ip_list = ["0.0.0.0/0"] +} + +resource "snowflake_network_policy_attachment" "test" { + network_policy_name = snowflake_network_policy.test.name + set_for_account = true +} +`, policyName) +} diff --git a/pkg/sdk/parameters.go b/pkg/sdk/parameters.go index fcff205eb7..64004f37c4 100644 --- a/pkg/sdk/parameters.go +++ b/pkg/sdk/parameters.go @@ -556,7 +556,7 @@ type AccountParametersUnset struct { ExternalOAuthAddPrivilegedRolesToBlockedList *bool `ddl:"keyword" sql:"EXTERNAL_OAUTH_ADD_PRIVILEGED_ROLES_TO_BLOCKED_LIST"` InitialReplicationSizeLimitInTB *bool `ddl:"keyword" sql:"INITIAL_REPLICATION_SIZE_LIMIT_IN_TB"` MinDataRetentionTimeInDays *bool `ddl:"keyword" sql:"MIN_DATA_RETENTION_TIME_IN_DAYS"` - NetworkPolicy *bool `ddl:"keyword,single_quotes" sql:"NETWORK_POLICY"` + NetworkPolicy *bool `ddl:"keyword" sql:"NETWORK_POLICY"` PeriodicDataRekeying *bool `ddl:"keyword" sql:"PERIODIC_DATA_REKEYING"` PreventUnloadToInlineURL *bool `ddl:"keyword" sql:"PREVENT_UNLOAD_TO_INLINE_URL"` PreventUnloadToInternalStages *bool `ddl:"keyword" sql:"PREVENT_UNLOAD_TO_INTERNAL_STAGES"` @@ -816,7 +816,7 @@ type ObjectParametersUnset struct { PipeExecutionPaused *bool `ddl:"keyword" sql:"PIPE_EXECUTION_PAUSED"` PreventUnloadToInternalStages *bool `ddl:"keyword" sql:"PREVENT_UNLOAD_TO_INTERNAL_STAGES"` StatementQueuedTimeoutInSeconds *bool `ddl:"keyword" sql:"STATEMENT_QUEUED_TIMEOUT_IN_SECONDS"` - NetworkPolicy *bool `ddl:"keyword,single_quotes" sql:"NETWORK_POLICY"` + NetworkPolicy *bool `ddl:"keyword" sql:"NETWORK_POLICY"` ShareRestrictions *bool `ddl:"keyword" sql:"SHARE_RESTRICTIONS"` SuspendTaskAfterNumFailures *bool `ddl:"keyword" sql:"SUSPEND_TASK_AFTER_NUM_FAILURES"` TraceLevel *bool `ddl:"keyword" sql:"TRACE_LEVEL"` diff --git a/pkg/sdk/parameters_test.go b/pkg/sdk/parameters_test.go index 24129bc585..f7f1a88233 100644 --- a/pkg/sdk/parameters_test.go +++ b/pkg/sdk/parameters_test.go @@ -22,3 +22,32 @@ func TestSetObjectParameterOnObject(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, "ALTER USER %s SET ENABLE_UNREDACTED_QUERY_SYNTAX_ERROR = TRUE", id.FullyQualifiedName()) }) } + +func TestUnSetObjectParameterNetworkPolicyOnAccount(t *testing.T) { + opts := &AlterAccountOptions{ + Unset: &AccountUnset{ + Parameters: &AccountLevelParametersUnset{ + ObjectParameters: &ObjectParametersUnset{ + NetworkPolicy: Bool(true), + }, + }, + }, + } + t.Run("Unset Account Network Policy", func(t *testing.T) { + assertOptsValidAndSQLEquals(t, opts, "ALTER ACCOUNT UNSET NETWORK_POLICY") + }) +} + +func TestUnSetObjectParameterNetworkPolicyOnUser(t *testing.T) { + opts := &AlterUserOptions{ + name: NewAccountObjectIdentifierFromFullyQualifiedName("TEST_USER"), + Unset: &UserUnset{ + ObjectParameters: &UserObjectParametersUnset{ + NetworkPolicy: Bool(true), + }, + }, + } + t.Run("Unset User Network Policy", func(t *testing.T) { + assertOptsValidAndSQLEquals(t, opts, `ALTER USER "TEST_USER" UNSET NETWORK_POLICY`) + }) +}