Skip to content

Commit

Permalink
fix: unset network policy should not have single quotes (#2584)
Browse files Browse the repository at this point in the history 
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 <artur.sawicki@snowflake.com>
  • Loading branch information
@raulbonet @sfc-gh-asawicki
raulbonet and sfc-gh-asawicki committed May 6, 2024
1 parent b0bf28f commit 8f2a363
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 8 deletions.
76 changes: 70 additions & 6 deletions pkg/resources/network_policy_attachment_acceptance_test.go
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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"),
),
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions pkg/sdk/parameters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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"`
Expand Down
29 changes: 29 additions & 0 deletions pkg/sdk/parameters_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
})
}

0 comments on commit 8f2a363

Please sign in to comment.