From 1ad80d8088ec47636ba7066e0a61cd503a231d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Mon, 8 Jul 2024 11:46:28 +0200 Subject: [PATCH 1/7] wip --- MIGRATION_GUIDE.md | 21 + docs/data-sources/network_policies.md | 111 ++++ docs/resources/network_policy.md | 77 +-- .../snowflake_network_policies/data-source.tf | 49 ++ .../snowflake_network_policy/import.sh | 2 +- .../snowflake_network_policy/resource.tf | 43 +- .../helpers/network_policy_client.go | 10 +- pkg/acceptance/helpers/network_rule_client.go | 53 ++ pkg/acceptance/helpers/test_client.go | 2 + pkg/datasources/network_policies.go | 102 ++++ .../network_policies_acceptance_test.go | 200 ++++++++ pkg/provider/provider.go | 1 + pkg/resources/network_policy.go | 287 ++++++----- .../network_policy_acceptance_test.go | 479 ++++++++++++------ pkg/resources/saml2_integration.go | 8 +- pkg/resources/validators.go | 22 + pkg/resources/validators_test.go | 48 ++ pkg/schemas/network_policy_properties.go | 46 ++ pkg/sdk/network_policies_def.go | 11 +- pkg/sdk/network_policies_dto_builders_gen.go | 81 +-- pkg/sdk/network_policies_dto_gen.go | 4 +- pkg/sdk/network_policies_gen.go | 9 +- pkg/sdk/network_policies_gen_test.go | 5 +- pkg/sdk/network_policies_impl_gen.go | 14 +- pkg/sdk/parsers.go | 24 +- pkg/sdk/parsers_test.go | 4 +- .../network_policies_gen_integration_test.go | 46 +- .../data-sources/network_policies.md.tmpl | 24 + templates/resources/network_policy.md.tmpl | 32 ++ 29 files changed, 1385 insertions(+), 430 deletions(-) create mode 100644 docs/data-sources/network_policies.md create mode 100644 examples/data-sources/snowflake_network_policies/data-source.tf create mode 100644 pkg/acceptance/helpers/network_rule_client.go create mode 100644 pkg/datasources/network_policies.go create mode 100644 pkg/datasources/network_policies_acceptance_test.go create mode 100644 pkg/schemas/network_policy_properties.go create mode 100644 templates/data-sources/network_policies.md.tmpl create mode 100644 templates/resources/network_policy.md.tmpl diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index bbd93e1eeb..499f7d79c5 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -20,6 +20,27 @@ They are all described in short in the [changes before v1 doc](./v1-preparations ### old grant resources removal Following the [announcement](https://github.com/Snowflake-Labs/terraform-provider-snowflake/discussions/2736) we have removed the old grant resources. The two resources [snowflake_role_ownership_grant](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/role_ownership_grant) and [snowflake_user_ownership_grant](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/user_ownership_grant) were not listed in the announcement, but they were also marked as deprecated ones. We are removing them too to conclude the grants redesign saga. +### *(new feature)* refactored snowflake_network_policy resource + +No migration required. + +New behavior: +- `name` is no longer marked as ForceNew parameter. When changed, now it will perform ALTER RENAME operation, instead of re-creating with the new name. +- Additional validation was added to `blocked_ip_list` to inform about specifying `0.0.0.0/0` ip. More details in the [official documentation](https://docs.snowflake.com/en/sql-reference/sql/create-network-policy#usage-notes). + +New fields: +- `show_output` and `describe_output` added to hold the results returned by `SHOW` and `DESCRIBE` commands. + +### *(new feature)* snowflake_network_policies datasource + +Added a new datasource enabling querying and filtering network policies. Notes: +- all results are stored in `network_policies` field. +- `like` field enables filtering. +- SHOW NETWORK POLICIES output is enclosed in `show_output` field inside `network_policies`. +- Output from **DESC NETWORK POLICY** (which can be turned off by declaring `with_describe = false`, **it's turned on by default**) is enclosed in `describe_output` field inside `network_policies`. + The additional parameters call **DESC NETWORK POLICY** (with `with_describe` turned on) **per network policy** returned by **SHOW NETWORK POLICIES**. + It's important to limit the records and calls to Snowflake to the minimum. That's why we recommend assessing which information you need from the data source and then providing strong filters and turning off additional fields for better plan performance. + ### *(new feature)* snowflake_security_integrations datasource Added a new datasource enabling querying and filtering all types of security integrations. Notes: - all results are stored in `security_integrations` field. diff --git a/docs/data-sources/network_policies.md b/docs/data-sources/network_policies.md new file mode 100644 index 0000000000..f2e8c505ce --- /dev/null +++ b/docs/data-sources/network_policies.md @@ -0,0 +1,111 @@ +--- +page_title: "snowflake_network_policies Data Source - terraform-provider-snowflake" +subcategory: "" +description: |- + Datasource used to get details of filtered network policies. Filtering is aligned with the current possibilities for SHOW NETWORK POLICIES https://docs.snowflake.com/en/sql-reference/sql/show-network-policies query (like is supported). The results of SHOW and DESCRIBE are encapsulated in one output collection. +--- + +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it. + +# snowflake_network_policies (Data Source) + +Datasource used to get details of filtered network policies. Filtering is aligned with the current possibilities for [SHOW NETWORK POLICIES](https://docs.snowflake.com/en/sql-reference/sql/show-network-policies) query (`like` is supported). The results of SHOW and DESCRIBE are encapsulated in one output collection. + +## Example Usage + +```terraform +# Simple usage +data "snowflake_network_policies" "simple" { +} + +output "simple_output" { + value = data.snowflake_network_policies.simple.network_policies +} + +# Filtering (like) +data "snowflake_network_policies" "like" { + like = "network-policy-name" +} + +output "like_output" { + value = data.snowflake_network_policies.like.network_policies +} + +# Without additional data (to limit the number of calls make for every found network policy) +data "snowflake_network_policies" "only_show" { + # with_describe is turned on by default and it calls DESCRIBE NETWORK POLICY for every network policy found and attaches its output to network_policies.*.describe_output field + with_describe = false +} + +output "only_show_output" { + value = data.snowflake_network_policies.only_show.network_policies +} + +# Ensure the number of network policies is equal to at least one element (with the use of postcondition) +data "snowflake_network_policies" "assert_with_postcondition" { + starts_with = "network-policy-name" + lifecycle { + postcondition { + condition = length(self.network_policies) > 0 + error_message = "there should be at least one network policy" + } + } +} + +# Ensure the number of network policies is equal to at exactly one element (with the use of check block) +check "network_policy_check" { + data "snowflake_network_policies" "assert_with_check_block" { + like = "network-policy-name" + } + + assert { + condition = length(data.snowflake_network_policies.assert_with_check_block.network_policies) == 1 + error_message = "Network policies filtered by '${data.snowflake_network_policies.assert_with_check_block.like}' returned ${length(data.snowflake_network_policies.assert_with_check_block.network_policies)} network policies where one was expected" + } +} +``` + + +## Schema + +### Optional + +- `like` (String) Filters the output with **case-insensitive** pattern, with support for SQL wildcard characters (`%` and `_`). +- `with_describe` (Boolean) Runs DESC NETWORK POLICY for each network policy returned by SHOW NETWORK POLICIES. The output of describe is saved to the description field. By default this value is set to true. + +### Read-Only + +- `id` (String) The ID of this resource. +- `network_policies` (List of Object) Holds the aggregated output of all network policies details queries. (see [below for nested schema](#nestedatt--network_policies)) + + +### Nested Schema for `network_policies` + +Read-Only: + +- `describe_output` (List of Object) (see [below for nested schema](#nestedobjatt--network_policies--describe_output)) +- `show_output` (List of Object) (see [below for nested schema](#nestedobjatt--network_policies--show_output)) + + +### Nested Schema for `network_policies.describe_output` + +Read-Only: + +- `allowed_ip_list` (String) +- `allowed_network_rule_list` (String) +- `blocked_ip_list` (String) +- `blocked_network_rule_list` (String) + + + +### Nested Schema for `network_policies.show_output` + +Read-Only: + +- `comment` (String) +- `created_on` (String) +- `entries_in_allowed_ip_list` (Number) +- `entries_in_allowed_network_rules` (Number) +- `entries_in_blocked_ip_list` (Number) +- `entries_in_blocked_network_rules` (Number) +- `name` (String) diff --git a/docs/resources/network_policy.md b/docs/resources/network_policy.md index 27a789944c..fbfd54c596 100644 --- a/docs/resources/network_policy.md +++ b/docs/resources/network_policy.md @@ -2,48 +2,31 @@ page_title: "snowflake_network_policy Resource - terraform-provider-snowflake" subcategory: "" description: |- - + Resource used to control network traffic. For more information, check an official guide https://docs.snowflake.com/en/user-guide/network-policies on controlling network traffic with network policies. --- -# snowflake_network_policy (Resource) +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it. +# snowflake_network_policy (Resource) +Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies. ## Example Usage ```terraform -################################## -### using network rules -################################## - -resource "snowflake_network_rule" "rule" { - name = "rule" - database = "EXAMPLE_DB" - schema = "EXAMPLE_SCHEMA" - comment = "A rule." - type = "IPV4" - mode = "INGRESS" - value_list = ["192.168.0.100/24", "29.254.123.20"] -} - -resource "snowflake_network_policy" "policy" { - name = "policy" - comment = "A policy." - - allowed_network_rule_list = [snowflake_network_rule.rule.qualified_name] +## Minimal +resource "snowflake_network_policy" "basic" { + name = "network_policy_name" } - -################################## -### using ip lists -################################## - -resource "snowflake_network_policy" "policy" { - name = "policy" - comment = "A policy." - - allowed_ip_list = ["192.168.0.100/24"] - blocked_ip_list = ["192.168.0.101"] +## Complete (with every optional set) +resource "snowflake_network_policy" "basic" { + name = "network_policy_name" + allowed_network_rule_list = [""] + blocked_network_rule_list = [""] + allowed_ip_list = ["192.168.1.0/24"] + blocked_ip_list = ["192.168.1.99"] + comment = "my network policy" } ``` @@ -58,18 +41,44 @@ resource "snowflake_network_policy" "policy" { - `allowed_ip_list` (Set of String) Specifies one or more IPv4 addresses (CIDR notation) that are allowed access to your Snowflake account. - `allowed_network_rule_list` (Set of String) Specifies a list of fully qualified network rules that contain the network identifiers that are allowed access to Snowflake. -- `blocked_ip_list` (Set of String) Specifies one or more IPv4 addresses (CIDR notation) that are denied access to your Snowflake account

**Do not** add `0.0.0.0/0` to `blocked_ip_list`. +- `blocked_ip_list` (Set of String) Specifies one or more IPv4 addresses (CIDR notation) that are denied access to your Snowflake account. **Do not** add `0.0.0.0/0` to `blocked_ip_list`, in order to block all IP addresses except a select list, you only need to add IP addresses to `allowed_ip_list`. - `blocked_network_rule_list` (Set of String) Specifies a list of fully qualified network rules that contain the network identifiers that are denied access to Snowflake. - `comment` (String) Specifies a comment for the network policy. ### Read-Only +- `describe_output` (List of Object) Outputs the result of `DESCRIBE NETWORK POLICY` for the given network policy. (see [below for nested schema](#nestedatt--describe_output)) - `id` (String) The ID of this resource. +- `show_output` (List of Object) Outputs the result of `SHOW NETWORK POLICIES` for the given network policy. (see [below for nested schema](#nestedatt--show_output)) + + +### Nested Schema for `describe_output` + +Read-Only: + +- `allowed_ip_list` (String) +- `allowed_network_rule_list` (String) +- `blocked_ip_list` (String) +- `blocked_network_rule_list` (String) + + + +### Nested Schema for `show_output` + +Read-Only: + +- `comment` (String) +- `created_on` (String) +- `entries_in_allowed_ip_list` (Number) +- `entries_in_allowed_network_rules` (Number) +- `entries_in_blocked_ip_list` (Number) +- `entries_in_blocked_network_rules` (Number) +- `name` (String) ## Import Import is supported using the following syntax: ```shell -terraform import snowflake_network_policy.example policyname +terraform import snowflake_network_policy.example "name" ``` diff --git a/examples/data-sources/snowflake_network_policies/data-source.tf b/examples/data-sources/snowflake_network_policies/data-source.tf new file mode 100644 index 0000000000..55b7ce844a --- /dev/null +++ b/examples/data-sources/snowflake_network_policies/data-source.tf @@ -0,0 +1,49 @@ +# Simple usage +data "snowflake_network_policies" "simple" { +} + +output "simple_output" { + value = data.snowflake_network_policies.simple.network_policies +} + +# Filtering (like) +data "snowflake_network_policies" "like" { + like = "network-policy-name" +} + +output "like_output" { + value = data.snowflake_network_policies.like.network_policies +} + +# Without additional data (to limit the number of calls make for every found network policy) +data "snowflake_network_policies" "only_show" { + # with_describe is turned on by default and it calls DESCRIBE NETWORK POLICY for every network policy found and attaches its output to network_policies.*.describe_output field + with_describe = false +} + +output "only_show_output" { + value = data.snowflake_network_policies.only_show.network_policies +} + +# Ensure the number of network policies is equal to at least one element (with the use of postcondition) +data "snowflake_network_policies" "assert_with_postcondition" { + starts_with = "network-policy-name" + lifecycle { + postcondition { + condition = length(self.network_policies) > 0 + error_message = "there should be at least one network policy" + } + } +} + +# Ensure the number of network policies is equal to at exactly one element (with the use of check block) +check "network_policy_check" { + data "snowflake_network_policies" "assert_with_check_block" { + like = "network-policy-name" + } + + assert { + condition = length(data.snowflake_network_policies.assert_with_check_block.network_policies) == 1 + error_message = "Network policies filtered by '${data.snowflake_network_policies.assert_with_check_block.like}' returned ${length(data.snowflake_network_policies.assert_with_check_block.network_policies)} network policies where one was expected" + } +} diff --git a/examples/resources/snowflake_network_policy/import.sh b/examples/resources/snowflake_network_policy/import.sh index 2db8882cd7..e9f2b372ac 100644 --- a/examples/resources/snowflake_network_policy/import.sh +++ b/examples/resources/snowflake_network_policy/import.sh @@ -1 +1 @@ -terraform import snowflake_network_policy.example policyname +terraform import snowflake_network_policy.example "name" diff --git a/examples/resources/snowflake_network_policy/resource.tf b/examples/resources/snowflake_network_policy/resource.tf index c8fb894245..17c0e70fcb 100644 --- a/examples/resources/snowflake_network_policy/resource.tf +++ b/examples/resources/snowflake_network_policy/resource.tf @@ -1,33 +1,14 @@ -################################## -### using network rules -################################## - -resource "snowflake_network_rule" "rule" { - name = "rule" - database = "EXAMPLE_DB" - schema = "EXAMPLE_SCHEMA" - comment = "A rule." - type = "IPV4" - mode = "INGRESS" - value_list = ["192.168.0.100/24", "29.254.123.20"] -} - -resource "snowflake_network_policy" "policy" { - name = "policy" - comment = "A policy." - - allowed_network_rule_list = [snowflake_network_rule.rule.qualified_name] +## Minimal +resource "snowflake_network_policy" "basic" { + name = "network_policy_name" } - -################################## -### using ip lists -################################## - -resource "snowflake_network_policy" "policy" { - name = "policy" - comment = "A policy." - - allowed_ip_list = ["192.168.0.100/24"] - blocked_ip_list = ["192.168.0.101"] -} +## Complete (with every optional set) +resource "snowflake_network_policy" "basic" { + name = "network_policy_name" + allowed_network_rule_list = [""] + blocked_network_rule_list = [""] + allowed_ip_list = ["192.168.1.0/24"] + blocked_ip_list = ["192.168.1.99"] + comment = "my network policy" +} \ No newline at end of file diff --git a/pkg/acceptance/helpers/network_policy_client.go b/pkg/acceptance/helpers/network_policy_client.go index 45d2239794..d3a3d57db6 100644 --- a/pkg/acceptance/helpers/network_policy_client.go +++ b/pkg/acceptance/helpers/network_policy_client.go @@ -42,12 +42,20 @@ func (c *NetworkPolicyClient) CreateNetworkPolicyWithRequest(t *testing.T, reque return networkPolicy, c.DropNetworkPolicyFunc(t, request.GetName()) } +func (c *NetworkPolicyClient) Update(t *testing.T, request *sdk.AlterNetworkPolicyRequest) { + t.Helper() + ctx := context.Background() + + err := c.client().Alter(ctx, request) + require.NoError(t, err) +} + func (c *NetworkPolicyClient) DropNetworkPolicyFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() { t.Helper() ctx := context.Background() return func() { - err := c.client().Drop(ctx, sdk.NewDropNetworkPolicyRequest(id).WithIfExists(sdk.Bool(true))) + err := c.client().Drop(ctx, sdk.NewDropNetworkPolicyRequest(id).WithIfExists(true)) require.NoError(t, err) } } diff --git a/pkg/acceptance/helpers/network_rule_client.go b/pkg/acceptance/helpers/network_rule_client.go new file mode 100644 index 0000000000..5093fc2277 --- /dev/null +++ b/pkg/acceptance/helpers/network_rule_client.go @@ -0,0 +1,53 @@ +package helpers + +import ( + "context" + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/stretchr/testify/require" +) + +type NetworkRuleClient struct { + context *TestClientContext + ids *IdsGenerator +} + +func NewNetworkRuleClient(context *TestClientContext, idsGenerator *IdsGenerator) *NetworkRuleClient { + return &NetworkRuleClient{ + context: context, + ids: idsGenerator, + } +} + +func (c *NetworkRuleClient) client() sdk.NetworkRules { + return c.context.client.NetworkRules +} + +func (c *NetworkRuleClient) Create(t *testing.T) *sdk.NetworkRule { + t.Helper() + return c.CreateWithName(t, c.ids.Alpha()) +} + +func (c *NetworkRuleClient) CreateWithName(t *testing.T, name string) *sdk.NetworkRule { + t.Helper() + return c.CreateWithIdentifier(t, c.ids.NewSchemaObjectIdentifier(name)) +} + +func (c *NetworkRuleClient) CreateWithIdentifier(t *testing.T, id sdk.SchemaObjectIdentifier) *sdk.NetworkRule { + t.Helper() + ctx := context.Background() + + err := c.client().Create(ctx, sdk.NewCreateNetworkRuleRequest(id, sdk.NetworkRuleTypeIpv4, []sdk.NetworkRuleValue{}, sdk.NetworkRuleModeIngress)) + require.NoError(t, err) + + t.Cleanup(func() { + _ = c.client().Drop(ctx, sdk.NewDropNetworkRuleRequest(id).WithIfExists(sdk.Bool(true))) + }) + + networkRule, err := c.client().ShowByID(ctx, id) + require.NoError(t, err) + require.NotNil(t, networkRule) + + return networkRule +} diff --git a/pkg/acceptance/helpers/test_client.go b/pkg/acceptance/helpers/test_client.go index 45ad486791..c446f6225a 100644 --- a/pkg/acceptance/helpers/test_client.go +++ b/pkg/acceptance/helpers/test_client.go @@ -26,6 +26,7 @@ type TestClient struct { MaskingPolicy *MaskingPolicyClient MaterializedView *MaterializedViewClient NetworkPolicy *NetworkPolicyClient + NetworkRule *NetworkRuleClient Parameter *ParameterClient PasswordPolicy *PasswordPolicyClient Pipe *PipeClient @@ -76,6 +77,7 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri MaskingPolicy: NewMaskingPolicyClient(context, idsGenerator), MaterializedView: NewMaterializedViewClient(context, idsGenerator), NetworkPolicy: NewNetworkPolicyClient(context, idsGenerator), + NetworkRule: NewNetworkRuleClient(context, idsGenerator), Parameter: NewParameterClient(context), PasswordPolicy: NewPasswordPolicyClient(context, idsGenerator), Pipe: NewPipeClient(context, idsGenerator), diff --git a/pkg/datasources/network_policies.go b/pkg/datasources/network_policies.go new file mode 100644 index 0000000000..653316c02b --- /dev/null +++ b/pkg/datasources/network_policies.go @@ -0,0 +1,102 @@ +package datasources + +import ( + "context" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +var networkPoliciesSchema = map[string]*schema.Schema{ + "with_describe": { + Type: schema.TypeBool, + Optional: true, + Default: true, + Description: "Runs DESC NETWORK POLICY for each network policy returned by SHOW NETWORK POLICIES. The output of describe is saved to the description field. By default this value is set to true.", + }, + "like": { + Type: schema.TypeString, + Optional: true, + Description: "Filters the output with **case-insensitive** pattern, with support for SQL wildcard characters (`%` and `_`).", + }, + "network_policies": { + Type: schema.TypeList, + Computed: true, + Description: "Holds the aggregated output of all network policies details queries.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + resources.ShowOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Holds the output of SHOW NETWORK POLICIES.", + Elem: &schema.Resource{ + Schema: schemas.ShowNetworkPolicySchema, + }, + }, + resources.DescribeOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Holds the output of DESCRIBE NETWORK POLICIES.", + Elem: &schema.Resource{ + Schema: schemas.DescribeNetworkPolicySchema, + }, + }, + }, + }, + }, +} + +func NetworkPolicies() *schema.Resource { + return &schema.Resource{ + ReadContext: ReadNetworkPolicies, + Schema: networkPoliciesSchema, + Description: "Datasource used to get details of filtered network policies. Filtering is aligned with the current possibilities for [SHOW NETWORK POLICIES](https://docs.snowflake.com/en/sql-reference/sql/show-network-policies) query (`like` is supported). The results of SHOW and DESCRIBE are encapsulated in one output collection.", + } +} + +func ReadNetworkPolicies(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + client := meta.(*provider.Context).Client + req := sdk.NewShowNetworkPolicyRequest() + + if likePattern, ok := d.GetOk("like"); ok { + req.WithLike(sdk.Like{ + Pattern: sdk.String(likePattern.(string)), + }) + } + + networkPolicies, err := client.NetworkPolicies.Show(ctx, req) + if err != nil { + return diag.FromErr(err) + } + + d.SetId("network_policies_read") + + flattenedNetworkPolicies := make([]map[string]any, len(networkPolicies)) + for i, networkPolicy := range networkPolicies { + networkPolicy := networkPolicy + + var networkPolicyDescribeOutput []map[string]any + if d.Get("with_describe").(bool) { + describeResult, err := client.NetworkPolicies.Describe(ctx, sdk.NewAccountObjectIdentifier(networkPolicy.Name)) + if err != nil { + return diag.FromErr(err) + } + networkPolicyDescribeOutput = []map[string]any{schemas.NetworkPolicyPropertiesToSchema(describeResult)} + } + + flattenedNetworkPolicies[i] = map[string]any{ + resources.ShowOutputAttributeName: []map[string]any{schemas.NetworkPolicyToSchema(&networkPolicy)}, + resources.DescribeOutputAttributeName: networkPolicyDescribeOutput, + } + } + + if err = d.Set("network_policies", flattenedNetworkPolicies); err != nil { + return diag.FromErr(err) + } + + return nil +} diff --git a/pkg/datasources/network_policies_acceptance_test.go b/pkg/datasources/network_policies_acceptance_test.go new file mode 100644 index 0000000000..131d592d6e --- /dev/null +++ b/pkg/datasources/network_policies_acceptance_test.go @@ -0,0 +1,200 @@ +package datasources_test + +import ( + "context" + "encoding/json" + "fmt" + "regexp" + "testing" + + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/tfversion" + "github.com/stretchr/testify/require" +) + +func TestAcc_NetworkPolicies_Complete(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) + + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + id2 := acc.TestClient().Ids.RandomAccountObjectIdentifier() + comment := random.Comment() + + databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier() + schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId) + + allowedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) + blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), + Steps: []resource.TestStep{ + { + PreConfig: func() { + db, _ := acc.TestClient().Database.CreateDatabaseWithIdentifier(t, databaseId) + t.Cleanup(func() { + require.NoError(t, acc.Client(t).Databases.Drop(context.Background(), db.ID(), &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)})) + }) + acc.TestClient().Schema.CreateSchemaWithIdentifier(t, schemaId) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId2) + }, + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + id2.Name(), + ), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.#", "1"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.show_output.0.created_on"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_ip_list", "2"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_ip_list", "2"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_network_rules", "2"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_network_rules", "2"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.comment", comment), + + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.#", "1"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_ip_list"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_ip_list"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_network_rule_list"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_network_rule_list"), + ), + }, + { + Config: networkPolicyConfigBasic(id.Name(), true), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.#", "1"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.show_output.0.created_on"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.comment", ""), + + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.#", "1"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_ip_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_ip_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_network_rule_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_network_rule_list"), + ), + }, + { + Config: networkPolicyConfigBasic(id.Name(), false), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.#", "1"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.show_output.0.created_on"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.comment", ""), + + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.#", "0"), + ), + }, + }, + }) +} + +func networkPolicyConfigBasic(name string, withDescribe bool) string { + return fmt.Sprintf(` + resource "snowflake_network_policy" "test" { + name = "%v" + } + + data "snowflake_network_policies" "test" { + with_describe = %t + like = snowflake_network_policy.test.name + } +`, name, withDescribe) +} + +func networkPolicyConfigComplete( + name string, + allowedRuleList []string, + blockedRuleList []string, + allowedIpList []string, + blockedIpList []string, + comment string, + name2 string, +) string { + allowedRuleListBytes, _ := json.Marshal(allowedRuleList) + blockedRuleListBytes, _ := json.Marshal(blockedRuleList) + allowedIpListBytes, _ := json.Marshal(allowedIpList) + blockedIpListBytes, _ := json.Marshal(blockedIpList) + + return fmt.Sprintf(` + resource "snowflake_network_policy" "test" { + name = "%[1]s" + allowed_network_rule_list = %[2]s + blocked_network_rule_list = %[3]s + allowed_ip_list = %[4]s + blocked_ip_list = %[5]s + comment = "%[6]s" + } + + resource "snowflake_network_policy" "test2" { + name = "%[7]s" + } + + data "snowflake_network_policies" "test" { + like = snowflake_network_policy.test.name + } +`, + name, + string(allowedRuleListBytes), + string(blockedRuleListBytes), + string(allowedIpListBytes), + string(blockedIpListBytes), + comment, + name2, + ) +} + +func TestAcc_NetworkPolicies_NetworkPolicyNotFound_WithPostConditions(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + Config: networkPolicyConfigBasicWithPostConditions(), + ExpectError: regexp.MustCompile("there should be at least one network policy"), + }, + }, + }) +} + +func networkPolicyConfigBasicWithPostConditions() string { + return ` + data "snowflake_network_policies" "test" { + like = "non_existing_network_policy" + lifecycle { + postcondition { + condition = length(self.network_policies) > 0 + error_message = "there should be at least one network policy" + } + } + } + ` +} diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 8c98fdd90a..2c65640897 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -508,6 +508,7 @@ func getDataSources() map[string]*schema.Resource { "snowflake_grants": datasources.Grants(), "snowflake_masking_policies": datasources.MaskingPolicies(), "snowflake_materialized_views": datasources.MaterializedViews(), + "snowflake_network_policies": datasources.NetworkPolicies(), "snowflake_parameters": datasources.Parameters(), "snowflake_pipes": datasources.Pipes(), "snowflake_procedures": datasources.Procedures(), diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index e6f74dfde4..966ffc8c16 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -5,10 +5,14 @@ import ( "encoding/json" "errors" "fmt" - "strings" + "log" + "reflect" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -20,7 +24,6 @@ var networkPolicySchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, Description: "Specifies the identifier for the network policy; must be unique for the account in which the network policy is created.", - ForceNew: true, }, "allowed_network_rule_list": { Type: schema.TypeSet, @@ -46,30 +49,66 @@ var networkPolicySchema = map[string]*schema.Schema{ Optional: true, Description: "Specifies one or more IPv4 addresses (CIDR notation) that are allowed access to your Snowflake account.", }, - // TODO: Add a ValidationFunc to ensure 0.0.0.0/0 is not in blocked_ip_list - // See: https://docs.snowflake.com/en/user-guide/network-policies.html#create-an-account-level-network-policy "blocked_ip_list": { - Type: schema.TypeSet, - Elem: &schema.Schema{Type: schema.TypeString}, + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: isNotEqualTo("0.0.0.0/0", "**Do not** add `0.0.0.0/0` to `blocked_ip_list`, in order to block all IP addresses except a select list, you only need to add IP addresses to `allowed_ip_list`."), + }, Optional: true, - Description: "Specifies one or more IPv4 addresses (CIDR notation) that are denied access to your Snowflake account

**Do not** add `0.0.0.0/0` to `blocked_ip_list`.", + Description: "Specifies one or more IPv4 addresses (CIDR notation) that are denied access to your Snowflake account. **Do not** add `0.0.0.0/0` to `blocked_ip_list`, in order to block all IP addresses except a select list, you only need to add IP addresses to `allowed_ip_list`.", }, "comment": { Type: schema.TypeString, Optional: true, Description: "Specifies a comment for the network policy.", }, + ShowOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Outputs the result of `SHOW NETWORK POLICIES` for the given network policy.", + Elem: &schema.Resource{ + Schema: schemas.ShowNetworkPolicySchema, + }, + }, + DescribeOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Outputs the result of `DESCRIBE NETWORK POLICY` for the given network policy.", + Elem: &schema.Resource{ + Schema: schemas.DescribeNetworkPolicySchema, + }, + }, } -// NetworkPolicy returns a pointer to the resource representing a network policy. func NetworkPolicy() *schema.Resource { return &schema.Resource{ + Schema: networkPolicySchema, + CreateContext: CreateContextNetworkPolicy, ReadContext: ReadContextNetworkPolicy, UpdateContext: UpdateContextNetworkPolicy, DeleteContext: DeleteContextNetworkPolicy, + Description: "Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies.", + + CustomizeDiff: customdiff.All( + ComputedIfAnyAttributeChanged( + ShowOutputAttributeName, + "allowed_network_rule_list", + "blocked_network_rule_list", + "allowed_ip_list", + "blocked_ip_list", + "comment", + ), + ComputedIfAnyAttributeChanged( + DescribeOutputAttributeName, + "allowed_network_rule_list", + "blocked_network_rule_list", + "allowed_ip_list", + "blocked_ip_list", + ), + ), - Schema: networkPolicySchema, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, @@ -77,31 +116,27 @@ func NetworkPolicy() *schema.Resource { } func CreateContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - name := d.Get("name").(string) - req := sdk.NewCreateNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(name)) + id := sdk.NewAccountObjectIdentifier(d.Get("name").(string)) + req := sdk.NewCreateNetworkPolicyRequest(id) if v, ok := d.GetOk("comment"); ok { - req = req.WithComment(sdk.String(v.(string))) + req.WithComment(v.(string)) } if v, ok := d.GetOk("allowed_network_rule_list"); ok { - networkRuleIdentifiers := parseNetworkRulesList(v) - req = req.WithAllowedNetworkRuleList(networkRuleIdentifiers) + req.WithAllowedNetworkRuleList(parseNetworkRulesList(v)) } if v, ok := d.GetOk("blocked_network_rule_list"); ok { - networkRuleIdentifiers := parseNetworkRulesList(v) - req = req.WithBlockedNetworkRuleList(networkRuleIdentifiers) + req.WithBlockedNetworkRuleList(parseNetworkRulesList(v)) } if v, ok := d.GetOk("allowed_ip_list"); ok { - ipRequests := parseIPList(v) - req = req.WithAllowedIpList(ipRequests) + req.WithAllowedIpList(parseIPList(v)) } if v, ok := d.GetOk("blocked_ip_list"); ok { - ipRequests := parseIPList(v) - req = req.WithAllowedIpList(ipRequests) + req.WithBlockedIpList(parseIPList(v)) } client := meta.(*provider.Context).Client @@ -111,27 +146,22 @@ func CreateContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, met diag.Diagnostic{ Severity: diag.Error, Summary: "Error creating network policy", - Detail: fmt.Sprintf("error creating network policy %v err = %v", name, err), + Detail: fmt.Sprintf("error creating network policy %v err = %v", id.Name(), err), }, } } - d.SetId(name) - return ReadContextNetworkPolicy(ctx, d, meta) -} + d.SetId(helpers.EncodeSnowflakeID(id)) -// NetworkRulesSnowflakeDTO is needed to unpack the applied network rules from the JSON response from Snowflake -type NetworkRulesSnowflakeDTO struct { - FullyQualifiedRuleName string + return ReadContextNetworkPolicy(ctx, d, meta) } func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - diags := diag.Diagnostics{} - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) client := meta.(*provider.Context).Client + id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) networkPolicy, err := client.NetworkPolicies.ShowByID(ctx, id) - if networkPolicy == nil || err != nil { + if err != nil { if errors.Is(err, sdk.ErrObjectNotFound) { d.SetId("") return diag.Diagnostics{ @@ -151,7 +181,7 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta } } - if err = d.Set("name", networkPolicy.Name); err != nil { + if err = d.Set("name", sdk.NewAccountObjectIdentifier(networkPolicy.Name).Name()); err != nil { return diag.FromErr(err) } @@ -159,141 +189,160 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta return diag.FromErr(err) } - policyDescriptions, err := client.NetworkPolicies.Describe(ctx, id) + policyProperties, err := client.NetworkPolicies.Describe(ctx, id) if err != nil { return diag.FromErr(err) } - for _, desc := range policyDescriptions { - switch desc.Name { - case "ALLOWED_IP_LIST": - if err = d.Set("allowed_ip_list", strings.Split(desc.Value, ",")); err != nil { - return diag.FromErr(err) - } - case "BLOCKED_IP_LIST": - if err = d.Set("blocked_ip_list", strings.Split(desc.Value, ",")); err != nil { - return diag.FromErr(err) - } - case "ALLOWED_NETWORK_RULE_LIST": - var networkRules []NetworkRulesSnowflakeDTO - err := json.Unmarshal([]byte(desc.Value), &networkRules) - if err != nil { - return diag.FromErr(err) - } - networkRulesFullyQualified := make([]string, len(networkRules)) - for i, ele := range networkRules { - networkRulesFullyQualified[i] = sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(ele.FullyQualifiedRuleName).FullyQualifiedName() - } - if err = d.Set("allowed_network_rule_list", networkRulesFullyQualified); err != nil { - return diag.FromErr(err) - } - case "BLOCKED_NETWORK_RULE_LIST": - var networkRules []NetworkRulesSnowflakeDTO - err := json.Unmarshal([]byte(desc.Value), &networkRules) - if err != nil { - return diag.FromErr(err) - } - networkRulesFullyQualified := make([]string, len(networkRules)) - for i, ele := range networkRules { - networkRulesFullyQualified[i] = sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(ele.FullyQualifiedRuleName).FullyQualifiedName() - } + log.Println("PROPS:", policyProperties) - if err = d.Set("blocked_network_rule_list", networkRulesFullyQualified); err != nil { - return diag.FromErr(err) - } + if allowedIpList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_IP_LIST" }); err == nil { + if err = d.Set("allowed_ip_list", sdk.ParseCommaSeparatedStringArray(allowedIpList.Value, false)); err != nil { + return diag.FromErr(err) + } + } else { + if err = d.Set("allowed_ip_list", []any{}); err != nil { + return diag.FromErr(err) + } + } + + if blockedIpList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "BLOCKED_IP_LIST" }); err == nil { + if err = d.Set("blocked_ip_list", sdk.ParseCommaSeparatedStringArray(blockedIpList.Value, false)); err != nil { + return diag.FromErr(err) + } + } else { + if err = d.Set("blocked_ip_list", []any{}); err != nil { + return diag.FromErr(err) + } + } + + allowedNetworkRules := make([]string, 0) + if allowedNetworkRuleList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_NETWORK_RULE_LIST" }); err == nil { + var networkRules []sdk.NetworkRulesSnowflakeDTO + err := json.Unmarshal([]byte(allowedNetworkRuleList.Value), &networkRules) + if err != nil { + return diag.FromErr(err) + } + + for _, networkRule := range networkRules { + allowedNetworkRules = append(allowedNetworkRules, sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(networkRule.FullyQualifiedRuleName).FullyQualifiedName()) } } + if err = d.Set("allowed_network_rule_list", allowedNetworkRules); err != nil { + return diag.FromErr(err) + } - return diags + blockedNetworkRules := make([]string, 0) + if blockedNetworkRuleList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "BLOCKED_NETWORK_RULE_LIST" }); err == nil { + var networkRules []sdk.NetworkRulesSnowflakeDTO + err := json.Unmarshal([]byte(blockedNetworkRuleList.Value), &networkRules) + if err != nil { + return diag.FromErr(err) + } + + for _, networkRule := range networkRules { + blockedNetworkRules = append(blockedNetworkRules, sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(networkRule.FullyQualifiedRuleName).FullyQualifiedName()) + } + } + if err = d.Set("blocked_network_rule_list", blockedNetworkRules); err != nil { + return diag.FromErr(err) + } + + if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.NetworkPolicyToSchema(networkPolicy)}); err != nil { + return diag.FromErr(err) + } + + if err = d.Set(DescribeOutputAttributeName, []map[string]any{schemas.NetworkPolicyPropertiesToSchema(policyProperties)}); err != nil { + return diag.FromErr(err) + } + + return nil } func UpdateContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) client := meta.(*provider.Context).Client + id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) + set, unset := sdk.NewNetworkPolicySetRequest(), sdk.NewNetworkPolicyUnsetRequest() + + if d.HasChange("name") { + helpers.EncodeSnowflakeID() + newId := sdk.NewAccountObjectIdentifier(d.Get("name").(string)) + + err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(id).WithRenameTo(newId)) + if err != nil { + return diag.FromErr(err) + } + + d.SetId(helpers.EncodeSnowflakeID(newId)) + id = newId + } if d.HasChange("comment") { - comment := d.Get("comment").(string) - baseReq := sdk.NewAlterNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())) - - if comment == "" { - unsetReq := sdk.NewNetworkPolicyUnsetRequest().WithComment(sdk.Bool(true)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithUnset(unsetReq)) - if err != nil { - return getUpdateContextDiag("unsetting comment", id.Name(), err) - } + if v, ok := d.GetOk("comment"); ok { + set.WithComment(v.(string)) } else { - setReq := sdk.NewNetworkPolicySetRequest().WithComment(sdk.String(comment)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithSet(setReq)) - if err != nil { - return getUpdateContextDiag("updating comment", id.Name(), err) - } + unset.WithComment(true) } } if d.HasChange("allowed_network_rule_list") { - baseReq := sdk.NewAlterNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())) - networkRuleIdentifiers := parseNetworkRulesList(d.Get("allowed_network_rule_list")) - setReq := sdk.NewNetworkPolicySetRequest().WithAllowedNetworkRuleList(sdk.NewAllowedNetworkRuleListRequest().WithAllowedNetworkRuleList(networkRuleIdentifiers)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithSet(setReq)) - if err != nil { - return getUpdateContextDiag("updating ALLOWED_NETWORK_RULE_LIST", id.Name(), err) + if v, ok := d.GetOk("allowed_network_rule_list"); ok { + set.WithAllowedNetworkRuleList(*sdk.NewAllowedNetworkRuleListRequest().WithAllowedNetworkRuleList(parseNetworkRulesList(v))) + } else { + unset.WithAllowedNetworkRuleList(true) } } if d.HasChange("blocked_network_rule_list") { - baseReq := sdk.NewAlterNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())) - networkRuleIdentifiers := parseNetworkRulesList(d.Get("blocked_network_rule_list")) - setReq := sdk.NewNetworkPolicySetRequest().WithBlockedNetworkRuleList(sdk.NewBlockedNetworkRuleListRequest().WithBlockedNetworkRuleList(networkRuleIdentifiers)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithSet(setReq)) - if err != nil { - return getUpdateContextDiag("updating BLOCKED_NETWORK_RULE_LIST", id.Name(), err) + if v, ok := d.GetOk("blocked_network_rule_list"); ok { + set.WithBlockedNetworkRuleList(*sdk.NewBlockedNetworkRuleListRequest().WithBlockedNetworkRuleList(parseNetworkRulesList(v))) + } else { + unset.WithBlockedNetworkRuleList(true) } } if d.HasChange("allowed_ip_list") { - baseReq := sdk.NewAlterNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())) - ipRequests := parseIPList(d.Get("allowed_ip_list")) - setReq := sdk.NewNetworkPolicySetRequest().WithAllowedIpList(sdk.NewAllowedIPListRequest().WithAllowedIPList(ipRequests)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithSet(setReq)) - if err != nil { - return getUpdateContextDiag("updating ALLOWED_IP_LIST", id.Name(), err) + if v, ok := d.GetOk("allowed_ip_list"); ok { + set.WithAllowedIpList(*sdk.NewAllowedIPListRequest().WithAllowedIPList(parseIPList(v))) + } else { + unset.WithAllowedIpList(true) } } if d.HasChange("blocked_ip_list") { - baseReq := sdk.NewAlterNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())) - ipRequests := parseIPList(d.Get("blocked_ip_list")) - setReq := sdk.NewNetworkPolicySetRequest().WithBlockedIpList(sdk.NewBlockedIPListRequest().WithBlockedIPList(ipRequests)) - err := client.NetworkPolicies.Alter(ctx, baseReq.WithSet(setReq)) - if err != nil { - return getUpdateContextDiag("updating BLOCKED_IP_LIST", id.Name(), err) + if v, ok := d.GetOk("blocked_ip_list"); ok { + set.WithBlockedIpList(*sdk.NewBlockedIPListRequest().WithBlockedIPList(parseIPList(v))) + } else { + unset.WithBlockedIpList(true) } } - return ReadContextNetworkPolicy(ctx, d, meta) -} + if !reflect.DeepEqual(*set, *sdk.NewNetworkPolicySetRequest()) { + if err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(id).WithSet(*set)); err != nil { + return diag.FromErr(err) + } + } -func getUpdateContextDiag(action string, name string, err error) diag.Diagnostics { - return diag.Diagnostics{ - diag.Diagnostic{ - Severity: diag.Error, - Summary: "Error updating network policy", - Detail: fmt.Sprintf("error %v for network policy %v err = %v", action, name, err), - }, + if !reflect.DeepEqual(*unset, *sdk.NewNetworkPolicyUnsetRequest()) { + if err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(id).WithUnset(*unset)); err != nil { + return diag.FromErr(err) + } } + + return ReadContextNetworkPolicy(ctx, d, meta) } func DeleteContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) client := meta.(*provider.Context).Client - err := client.NetworkPolicies.Drop(ctx, sdk.NewDropNetworkPolicyRequest(sdk.NewAccountObjectIdentifier(id.Name())).WithIfExists(sdk.Bool(true))) + err := client.NetworkPolicies.Drop(ctx, sdk.NewDropNetworkPolicyRequest(id).WithIfExists(true)) if err != nil { return diag.Diagnostics{ diag.Diagnostic{ Severity: diag.Error, Summary: "Error deleting network policy", - Detail: fmt.Sprintf("error deleting network policy %v err = %v", id.Name(), err), + Detail: fmt.Sprintf("Error deleting network policy %v, err = %v", id.Name(), err), }, } } diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index 8991c00325..28c776818e 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -1,10 +1,16 @@ package resources_test import ( + "encoding/json" "fmt" "regexp" "testing" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" @@ -12,14 +18,14 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -const ( - networkPolicyComment = "CREATED BY A TERRAFORM ACCEPTANCE TEST" -) +func TestAcc_NetworkPolicy_Basic(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + comment := random.Comment() -func TestAcc_NetworkPolicy(t *testing.T) { - name := acc.TestClient().Ids.Alpha() - nameRule1 := acc.TestClient().Ids.Alpha() - nameRule2 := acc.TestClient().Ids.Alpha() + allowedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -29,80 +35,206 @@ func TestAcc_NetworkPolicy(t *testing.T) { }, CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), Steps: []resource.TestStep{ + // create with empty optionals { - Config: networkPolicyConfig(name), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", name), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", networkPolicyComment), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "2"), + Config: networkPolicyConfigBasic(id.Name()), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", ""), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "show_output.0.created_on"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.comment", ""), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_ip_list", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_ip_list", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "0"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.allowed_ip_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.blocked_ip_list"), ), }, - // CHANGE PROPERTIES - add to and remove from ip lists + // import - without optionals { - Config: networkPolicyConfig2(name), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", name), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", networkPolicyComment), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "0"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "0"), + Config: networkPolicyConfigBasic(id.Name()), + ResourceName: "snowflake_network_policy.test", + ImportState: true, + ImportStateCheck: importchecks.ComposeAggregateImportStateCheck( + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()), + importchecks.TestCheckResourceAttrNotInInstanceState(id.Name(), "allowed_ip_list"), + importchecks.TestCheckResourceAttrNotInInstanceState(id.Name(), "blocked_ip_list"), + importchecks.TestCheckResourceAttrNotInInstanceState(id.Name(), "allowed_network_rule_list"), + importchecks.TestCheckResourceAttrNotInInstanceState(id.Name(), "blocked_network_rule_list"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", ""), ), }, + // set optionals { - Config: networkPolicyConfigNetworkRules(name, nameRule1, nameRule2, acc.TestDatabaseName, acc.TestSchemaName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", name), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", networkPolicyComment), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "0"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "0"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "1"), + PreConfig: func() { + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + }, + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_network_policy.test", plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", comment), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "show_output.0.created_on"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.comment", comment), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_ip_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_ip_list"), ), }, + // import - complete { - Config: networkPolicyConfigIPsAndRules(name, nameRule1, nameRule2, acc.TestDatabaseName, acc.TestSchemaName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", name), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", networkPolicyComment), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "1"), + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + ), + ResourceName: "snowflake_network_policy.test", + ImportState: true, + ImportStateCheck: importchecks.ComposeAggregateImportStateCheck( + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "allowed_ip_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "blocked_ip_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "allowed_network_rule_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "blocked_network_rule_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", comment), ), }, - // IMPORT - all fields are non-empty + // change externally { - ResourceName: "snowflake_network_policy.test", - ImportState: true, - ImportStateVerify: true, + PreConfig: func() { + acc.TestClient().NetworkPolicy.Update(t, sdk.NewAlterNetworkPolicyRequest(id).WithUnset( + *sdk.NewNetworkPolicyUnsetRequest(). + WithAllowedIpList(true). + WithBlockedIpList(true). + WithAllowedNetworkRuleList(true). + WithBlockedNetworkRuleList(true). + WithComment(true), + )) + }, + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_network_policy.test", plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", comment), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "show_output.0.created_on"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.comment", comment), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_ip_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_ip_list"), + ), }, + // unset { - Config: networkPolicyConfigAllEmpty(name), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", name), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", networkPolicyComment), + Config: networkPolicyConfigBasic(id.Name()), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_network_policy.test", plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "0"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", ""), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "show_output.0.created_on"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.comment", ""), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_ip_list", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_ip_list", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "0"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "0"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.allowed_ip_list"), + resource.TestCheckNoResourceAttr("snowflake_network_policy.test", "describe_output.0.blocked_ip_list"), ), }, - // IMPORT - incomplete - { - ResourceName: "snowflake_network_policy.test", - ImportState: true, - ImportStateVerify: true, - }, }, }) } -func TestAcc_NetworkPolicyBadNetworkRuleNames(t *testing.T) { - name := acc.TestClient().Ids.Alpha() +func TestAcc_NetworkPolicy_Complete(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + comment := random.Comment() + + allowedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -112,119 +244,164 @@ func TestAcc_NetworkPolicyBadNetworkRuleNames(t *testing.T) { }, CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), Steps: []resource.TestStep{ - // Checks the case when a network rule name, which is not a schema object identifier, is passed to a network policy { - Config: networkPolicyConfigBadNetworkRule(name), - ExpectError: regexp.MustCompile(`.*Invalid identifier type.*`), + PreConfig: func() { + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + }, + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + ), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_ip_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "comment", comment), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "show_output.0.created_on"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.comment", comment), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_ip_list", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_ip_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_ip_list"), + ), + }, + { + Config: networkPolicyConfigComplete( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{"1.1.1.1", "2.2.2.2"}, + []string{"3.3.3.3", "4.4.4.4"}, + comment, + ), + ResourceName: "snowflake_network_policy.test", + ImportState: true, + ImportStateCheck: importchecks.ComposeAggregateImportStateCheck( + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "name", id.Name()), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "allowed_ip_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "blocked_ip_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "allowed_network_rule_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "blocked_network_rule_list.#", "2"), + importchecks.TestCheckResourceAttrInstanceState(id.Name(), "comment", comment), + ), }, }, }) } -func networkPolicyConfig(name string) string { - return fmt.Sprintf(` -resource "snowflake_network_policy" "test" { - name = "%v" - comment = "%v" - allowed_ip_list = ["192.168.0.100/24", "29.254.123.20"] -} -`, name, networkPolicyComment) -} +func TestAcc_NetworkPolicy_Rename(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + newId := acc.TestClient().Ids.RandomAccountObjectIdentifier() -func networkPolicyConfig2(name string) string { - return fmt.Sprintf(` -resource "snowflake_network_policy" "test" { - name = "%v" - comment = "%v" - allowed_ip_list = ["192.168.0.100/24"] - blocked_ip_list = ["192.168.0.101"] -} -`, name, networkPolicyComment) -} - -func networkPolicyConfigNetworkRules(name string, nameRule1 string, nameRule2 string, database string, schema string) string { - return fmt.Sprintf(` -resource "snowflake_network_rule" "test1" { - name = "%[2]v" - database = "%[4]v" - schema = "%[5]v" - comment = "%[6]v" - type = "IPV4" - mode = "INGRESS" - value_list = ["192.168.0.100/24", "29.254.123.20"] + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), + Steps: []resource.TestStep{ + { + Config: networkPolicyConfigBasic(id.Name()), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "id", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", id.Name()), + ), + }, + { + Config: networkPolicyConfigBasic(newId.Name()), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_network_policy.test", plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "id", newId.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", newId.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.name", newId.Name()), + ), + }, + }, + }) } -resource "snowflake_network_rule" "test2" { - name = "%[3]v" - database = "%[4]v" - schema = "%[5]v" - comment = "%[6]v" - type = "HOST_PORT" - mode = "EGRESS" - value_list = ["example.com", "company.com:443"] -} +func TestAcc_NetworkPolicy_InvalidBlockedIpListValue(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() -resource "snowflake_network_policy" "test" { - name = "%[1]v" - comment = "%[6]v" - allowed_network_rule_list = [snowflake_network_rule.test1.qualified_name] - blocked_network_rule_list = [snowflake_network_rule.test2.qualified_name] -} -`, name, nameRule1, nameRule2, database, schema, networkPolicyComment) -} - -func networkPolicyConfigIPsAndRules(name string, nameRule1 string, nameRule2 string, database string, schema string) string { - return fmt.Sprintf(` -resource "snowflake_network_rule" "test1" { - name = "%[2]v" - database = "%[4]v" - schema = "%[5]v" - comment = "%[6]v" - type = "IPV4" - mode = "INGRESS" - value_list = ["192.168.0.100/24", "29.254.123.20"] + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), + Steps: []resource.TestStep{ + { + Config: networkPolicyConfigInvalidBlockedIpListValue(id.Name()), + ExpectError: regexp.MustCompile(`invalid value \(0.0.0.0/0\) set for a field \[{{} blocked_ip_list} {{} {{{{}`), + }, + }, + }) } -resource "snowflake_network_rule" "test2" { - name = "%[3]v" - database = "%[4]v" - schema = "%[5]v" - comment = "%[6]v" - type = "HOST_PORT" - mode = "EGRESS" - value_list = ["example.com", "company.com:443"] +func networkPolicyConfigBasic(name string) string { + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%v" + }`, name) } -resource "snowflake_network_policy" "test" { - name = "%[1]v" - comment = "%[6]v" - allowed_ip_list = ["192.168.0.100/24"] - blocked_ip_list = ["192.168.0.101"] - allowed_network_rule_list = [snowflake_network_rule.test1.qualified_name] - blocked_network_rule_list = [snowflake_network_rule.test2.qualified_name] -} -`, name, nameRule1, nameRule2, database, schema, networkPolicyComment) +func networkPolicyConfigInvalidBlockedIpListValue(name string) string { + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%v" + blocked_ip_list = ["1.1.1.1", "0.0.0.0/0"] + }`, name) } -func networkPolicyConfigAllEmpty(name string) string { - return fmt.Sprintf(` -resource "snowflake_network_policy" "test" { - name = "%v" - comment = "%v" - allowed_ip_list = [] - blocked_ip_list = [] - allowed_network_rule_list = [] - blocked_network_rule_list = [] -} -`, name, networkPolicyComment) -} +func networkPolicyConfigComplete( + name string, + allowedRuleList []string, + blockedRuleList []string, + allowedIpList []string, + blockedIpList []string, + comment string, +) string { + allowedRuleListBytes, _ := json.Marshal(allowedRuleList) + blockedRuleListBytes, _ := json.Marshal(blockedRuleList) + allowedIpListBytes, _ := json.Marshal(allowedIpList) + blockedIpListBytes, _ := json.Marshal(blockedIpList) -func networkPolicyConfigBadNetworkRule(name string) string { - return fmt.Sprintf(` -resource "snowflake_network_policy" "test" { - name = "%v" - comment = "%v" - allowed_network_rule_list = ["FOO"] -} -`, name, networkPolicyComment) + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%[1]s" + allowed_network_rule_list = %[2]s + blocked_network_rule_list = %[3]s + allowed_ip_list = %[4]s + blocked_ip_list = %[5]s + comment = "%[6]s" + }`, + name, + string(allowedRuleListBytes), + string(blockedRuleListBytes), + string(allowedIpListBytes), + string(blockedIpListBytes), + comment, + ) } diff --git a/pkg/resources/saml2_integration.go b/pkg/resources/saml2_integration.go index 259c8847df..6b03ef3c75 100644 --- a/pkg/resources/saml2_integration.go +++ b/pkg/resources/saml2_integration.go @@ -320,7 +320,7 @@ func ImportSaml2Integration(ctx context.Context, d *schema.ResourceData, meta an if err != nil { return nil, fmt.Errorf("failed to find allowed user domains, err = %w", err) } - if err := d.Set("allowed_user_domains", sdk.ParseCommaSeparatedStringArray(allowedUserDomains.Value)); err != nil { + if err := d.Set("allowed_user_domains", sdk.ParseCommaSeparatedStringArray(allowedUserDomains.Value, false)); err != nil { return nil, err } @@ -330,7 +330,7 @@ func ImportSaml2Integration(ctx context.Context, d *schema.ResourceData, meta an if err != nil { return nil, fmt.Errorf("failed to find allowed email patterns, err = %w", err) } - if err := d.Set("allowed_email_patterns", sdk.ParseCommaSeparatedStringArray(allowedEmailDomains.Value)); err != nil { + if err := d.Set("allowed_email_patterns", sdk.ParseCommaSeparatedStringArray(allowedEmailDomains.Value, false)); err != nil { return nil, err } @@ -525,7 +525,7 @@ func ReadContextSAML2Integration(withExternalChangesMarking bool) schema.ReadCon if err != nil { return diag.FromErr(fmt.Errorf("failed to find allowed user domains, err = %w", err)) } - if err := d.Set("allowed_user_domains", sdk.ParseCommaSeparatedStringArray(allowedUserDomains.Value)); err != nil { + if err := d.Set("allowed_user_domains", sdk.ParseCommaSeparatedStringArray(allowedUserDomains.Value, false)); err != nil { return diag.FromErr(err) } @@ -535,7 +535,7 @@ func ReadContextSAML2Integration(withExternalChangesMarking bool) schema.ReadCon if err != nil { return diag.FromErr(fmt.Errorf("failed to find allowed email patterns, err = %w", err)) } - if err := d.Set("allowed_email_patterns", sdk.ParseCommaSeparatedStringArray(allowedEmailDomains.Value)); err != nil { + if err := d.Set("allowed_email_patterns", sdk.ParseCommaSeparatedStringArray(allowedEmailDomains.Value, false)); err != nil { return diag.FromErr(err) } diff --git a/pkg/resources/validators.go b/pkg/resources/validators.go index d719de90de..52eaf50b6a 100644 --- a/pkg/resources/validators.go +++ b/pkg/resources/validators.go @@ -182,3 +182,25 @@ func sdkValidation[T any](normalize func(string) (T, error)) schema.SchemaValida return nil } } + +func isNotEqualTo(notExpectedValue string, errorMessage string) schema.SchemaValidateDiagFunc { + return func(value any, path cty.Path) diag.Diagnostics { + if value != nil { + if stringValue, ok := value.(string); ok { + if stringValue == notExpectedValue { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "Invalid value set", + Detail: fmt.Sprintf("invalid value (%s) set for a field %v. %s", notExpectedValue, path, errorMessage), + }, + } + } + } else { + return diag.Errorf("isNotEqualTo validator: expected string type, got %T", value) + } + } + + return nil + } +} diff --git a/pkg/resources/validators_test.go b/pkg/resources/validators_test.go index f10226e30d..6146d3c603 100644 --- a/pkg/resources/validators_test.go +++ b/pkg/resources/validators_test.go @@ -307,3 +307,51 @@ func Test_IsValidAccountIdentifier(t *testing.T) { }) } } + +func Test_isNotEqualTo(t *testing.T) { + testCases := []struct { + Name string + Value any + NotExpectedValue string + ExpectedError string + }{ + { + Name: "nil value", + Value: nil, + NotExpectedValue: "123", + }, + { + Name: "int value", + Value: 123, + NotExpectedValue: "123", + ExpectedError: "isNotEqualTo validator: expected string type, got int", + }, + { + Name: "value equal to invalid one", + Value: "123", + NotExpectedValue: "123", + ExpectedError: "invalid value (123) set for a field [{{} path}]. error message.", + }, + { + Name: "value equal to valid one", + Value: "456", + NotExpectedValue: "123", + }, + } + + for _, tt := range testCases { + t.Run(tt.Name, func(t *testing.T) { + diag := isNotEqualTo(tt.NotExpectedValue, "error message.")(tt.Value, cty.GetAttrPath("path")) + if tt.ExpectedError != "" { + assert.Len(t, diag, 1) + if diag[0].Detail != "" { + assert.Contains(t, diag[0].Detail, tt.ExpectedError) + } else { + assert.Contains(t, diag[0].Summary, tt.ExpectedError) + } + } else { + assert.Len(t, diag, 0) + } + }) + } +} diff --git a/pkg/schemas/network_policy_properties.go b/pkg/schemas/network_policy_properties.go new file mode 100644 index 0000000000..e8ab040cd6 --- /dev/null +++ b/pkg/schemas/network_policy_properties.go @@ -0,0 +1,46 @@ +package schemas + +import ( + "strings" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +// DescribeNetworkPolicySchema represents output of DESCRIBE query for the single NetworkPolicy. +var DescribeNetworkPolicySchema = map[string]*schema.Schema{ + "allowed_ip_list": { + Type: schema.TypeString, + Computed: true, + }, + "blocked_ip_list": { + Type: schema.TypeString, + Computed: true, + }, + "allowed_network_rule_list": { + Type: schema.TypeString, + Computed: true, + }, + "blocked_network_rule_list": { + Type: schema.TypeString, + Computed: true, + }, +} + +var _ = DescribeNetworkPolicySchema + +func NetworkPolicyPropertiesToSchema(networkPolicyProperties []sdk.NetworkPolicyProperty) map[string]any { + networkPolicySchema := make(map[string]any) + for _, property := range networkPolicyProperties { + switch property.Name { + case "ALLOWED_IP_LIST", + "BLOCKED_IP_LIST", + "ALLOWED_NETWORK_RULE_LIST", + "BLOCKED_NETWORK_RULE_LIST": + networkPolicySchema[strings.ToLower(property.Name)] = property.Value + } + } + return networkPolicySchema +} + +var _ = NetworkPolicyPropertiesToSchema diff --git a/pkg/sdk/network_policies_def.go b/pkg/sdk/network_policies_def.go index d50ead0a4f..608b679325 100644 --- a/pkg/sdk/network_policies_def.go +++ b/pkg/sdk/network_policies_def.go @@ -4,6 +4,11 @@ import g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/gen //go:generate go run ./poc/main.go +// NetworkRulesSnowflakeDTO is needed to unpack the applied network rules from the JSON response from Snowflake +type NetworkRulesSnowflakeDTO struct { + FullyQualifiedRuleName string +} + var ( ip = g.NewQueryStruct("IP"). Text("IP", g.KeywordOptions().SingleQuotes().Required()) @@ -122,15 +127,17 @@ var ( Field("EntriesInBlockedNetworkRules", "int"), g.NewQueryStruct("ShowNetworkPolicies"). Show(). - SQL("NETWORK POLICIES"), + SQL("NETWORK POLICIES"). + OptionalLike(), ). + ShowByIdOperation(). DescribeOperation( g.DescriptionMappingKindSlice, "https://docs.snowflake.com/en/sql-reference/sql/desc-network-policy", g.DbStruct("describeNetworkPolicyDBRow"). Field("name", "string"). Field("value", "string"), - g.PlainStruct("NetworkPolicyDescription"). + g.PlainStruct("NetworkPolicyProperty"). Field("Name", "string"). Field("Value", "string"), g.NewQueryStruct("DescribeNetworkPolicy"). diff --git a/pkg/sdk/network_policies_dto_builders_gen.go b/pkg/sdk/network_policies_dto_builders_gen.go index ee41eea29d..6d6f01fd2c 100644 --- a/pkg/sdk/network_policies_dto_builders_gen.go +++ b/pkg/sdk/network_policies_dto_builders_gen.go @@ -12,8 +12,8 @@ func NewCreateNetworkPolicyRequest( return &s } -func (s *CreateNetworkPolicyRequest) WithOrReplace(OrReplace *bool) *CreateNetworkPolicyRequest { - s.OrReplace = OrReplace +func (s *CreateNetworkPolicyRequest) WithOrReplace(OrReplace bool) *CreateNetworkPolicyRequest { + s.OrReplace = &OrReplace return s } @@ -37,8 +37,8 @@ func (s *CreateNetworkPolicyRequest) WithBlockedIpList(BlockedIpList []IPRequest return s } -func (s *CreateNetworkPolicyRequest) WithComment(Comment *string) *CreateNetworkPolicyRequest { - s.Comment = Comment +func (s *CreateNetworkPolicyRequest) WithComment(Comment string) *CreateNetworkPolicyRequest { + s.Comment = &Comment return s } @@ -58,33 +58,33 @@ func NewAlterNetworkPolicyRequest( return &s } -func (s *AlterNetworkPolicyRequest) WithIfExists(IfExists *bool) *AlterNetworkPolicyRequest { - s.IfExists = IfExists +func (s *AlterNetworkPolicyRequest) WithIfExists(IfExists bool) *AlterNetworkPolicyRequest { + s.IfExists = &IfExists return s } -func (s *AlterNetworkPolicyRequest) WithSet(Set *NetworkPolicySetRequest) *AlterNetworkPolicyRequest { - s.Set = Set +func (s *AlterNetworkPolicyRequest) WithSet(Set NetworkPolicySetRequest) *AlterNetworkPolicyRequest { + s.Set = &Set return s } -func (s *AlterNetworkPolicyRequest) WithUnset(Unset *NetworkPolicyUnsetRequest) *AlterNetworkPolicyRequest { - s.Unset = Unset +func (s *AlterNetworkPolicyRequest) WithUnset(Unset NetworkPolicyUnsetRequest) *AlterNetworkPolicyRequest { + s.Unset = &Unset return s } -func (s *AlterNetworkPolicyRequest) WithAdd(Add *AddNetworkRuleRequest) *AlterNetworkPolicyRequest { - s.Add = Add +func (s *AlterNetworkPolicyRequest) WithAdd(Add AddNetworkRuleRequest) *AlterNetworkPolicyRequest { + s.Add = &Add return s } -func (s *AlterNetworkPolicyRequest) WithRemove(Remove *RemoveNetworkRuleRequest) *AlterNetworkPolicyRequest { - s.Remove = Remove +func (s *AlterNetworkPolicyRequest) WithRemove(Remove RemoveNetworkRuleRequest) *AlterNetworkPolicyRequest { + s.Remove = &Remove return s } -func (s *AlterNetworkPolicyRequest) WithRenameTo(RenameTo *AccountObjectIdentifier) *AlterNetworkPolicyRequest { - s.RenameTo = RenameTo +func (s *AlterNetworkPolicyRequest) WithRenameTo(RenameTo AccountObjectIdentifier) *AlterNetworkPolicyRequest { + s.RenameTo = &RenameTo return s } @@ -92,28 +92,28 @@ func NewNetworkPolicySetRequest() *NetworkPolicySetRequest { return &NetworkPolicySetRequest{} } -func (s *NetworkPolicySetRequest) WithAllowedNetworkRuleList(AllowedNetworkRuleList *AllowedNetworkRuleListRequest) *NetworkPolicySetRequest { - s.AllowedNetworkRuleList = AllowedNetworkRuleList +func (s *NetworkPolicySetRequest) WithAllowedNetworkRuleList(AllowedNetworkRuleList AllowedNetworkRuleListRequest) *NetworkPolicySetRequest { + s.AllowedNetworkRuleList = &AllowedNetworkRuleList return s } -func (s *NetworkPolicySetRequest) WithBlockedNetworkRuleList(BlockedNetworkRuleList *BlockedNetworkRuleListRequest) *NetworkPolicySetRequest { - s.BlockedNetworkRuleList = BlockedNetworkRuleList +func (s *NetworkPolicySetRequest) WithBlockedNetworkRuleList(BlockedNetworkRuleList BlockedNetworkRuleListRequest) *NetworkPolicySetRequest { + s.BlockedNetworkRuleList = &BlockedNetworkRuleList return s } -func (s *NetworkPolicySetRequest) WithAllowedIpList(AllowedIpList *AllowedIPListRequest) *NetworkPolicySetRequest { - s.AllowedIpList = AllowedIpList +func (s *NetworkPolicySetRequest) WithAllowedIpList(AllowedIpList AllowedIPListRequest) *NetworkPolicySetRequest { + s.AllowedIpList = &AllowedIpList return s } -func (s *NetworkPolicySetRequest) WithBlockedIpList(BlockedIpList *BlockedIPListRequest) *NetworkPolicySetRequest { - s.BlockedIpList = BlockedIpList +func (s *NetworkPolicySetRequest) WithBlockedIpList(BlockedIpList BlockedIPListRequest) *NetworkPolicySetRequest { + s.BlockedIpList = &BlockedIpList return s } -func (s *NetworkPolicySetRequest) WithComment(Comment *string) *NetworkPolicySetRequest { - s.Comment = Comment +func (s *NetworkPolicySetRequest) WithComment(Comment string) *NetworkPolicySetRequest { + s.Comment = &Comment return s } @@ -157,28 +157,28 @@ func NewNetworkPolicyUnsetRequest() *NetworkPolicyUnsetRequest { return &NetworkPolicyUnsetRequest{} } -func (s *NetworkPolicyUnsetRequest) WithAllowedNetworkRuleList(AllowedNetworkRuleList *bool) *NetworkPolicyUnsetRequest { - s.AllowedNetworkRuleList = AllowedNetworkRuleList +func (s *NetworkPolicyUnsetRequest) WithAllowedNetworkRuleList(AllowedNetworkRuleList bool) *NetworkPolicyUnsetRequest { + s.AllowedNetworkRuleList = &AllowedNetworkRuleList return s } -func (s *NetworkPolicyUnsetRequest) WithBlockedNetworkRuleList(BlockedNetworkRuleList *bool) *NetworkPolicyUnsetRequest { - s.BlockedNetworkRuleList = BlockedNetworkRuleList +func (s *NetworkPolicyUnsetRequest) WithBlockedNetworkRuleList(BlockedNetworkRuleList bool) *NetworkPolicyUnsetRequest { + s.BlockedNetworkRuleList = &BlockedNetworkRuleList return s } -func (s *NetworkPolicyUnsetRequest) WithAllowedIpList(AllowedIpList *bool) *NetworkPolicyUnsetRequest { - s.AllowedIpList = AllowedIpList +func (s *NetworkPolicyUnsetRequest) WithAllowedIpList(AllowedIpList bool) *NetworkPolicyUnsetRequest { + s.AllowedIpList = &AllowedIpList return s } -func (s *NetworkPolicyUnsetRequest) WithBlockedIpList(BlockedIpList *bool) *NetworkPolicyUnsetRequest { - s.BlockedIpList = BlockedIpList +func (s *NetworkPolicyUnsetRequest) WithBlockedIpList(BlockedIpList bool) *NetworkPolicyUnsetRequest { + s.BlockedIpList = &BlockedIpList return s } -func (s *NetworkPolicyUnsetRequest) WithComment(Comment *bool) *NetworkPolicyUnsetRequest { - s.Comment = Comment +func (s *NetworkPolicyUnsetRequest) WithComment(Comment bool) *NetworkPolicyUnsetRequest { + s.Comment = &Comment return s } @@ -218,8 +218,8 @@ func NewDropNetworkPolicyRequest( return &s } -func (s *DropNetworkPolicyRequest) WithIfExists(IfExists *bool) *DropNetworkPolicyRequest { - s.IfExists = IfExists +func (s *DropNetworkPolicyRequest) WithIfExists(IfExists bool) *DropNetworkPolicyRequest { + s.IfExists = &IfExists return s } @@ -227,6 +227,11 @@ func NewShowNetworkPolicyRequest() *ShowNetworkPolicyRequest { return &ShowNetworkPolicyRequest{} } +func (s *ShowNetworkPolicyRequest) WithLike(Like Like) *ShowNetworkPolicyRequest { + s.Like = &Like + return s +} + func NewDescribeNetworkPolicyRequest( name AccountObjectIdentifier, ) *DescribeNetworkPolicyRequest { diff --git a/pkg/sdk/network_policies_dto_gen.go b/pkg/sdk/network_policies_dto_gen.go index 27ef520dab..a7ea9ce292 100644 --- a/pkg/sdk/network_policies_dto_gen.go +++ b/pkg/sdk/network_policies_dto_gen.go @@ -81,7 +81,9 @@ type DropNetworkPolicyRequest struct { name AccountObjectIdentifier // required } -type ShowNetworkPolicyRequest struct{} +type ShowNetworkPolicyRequest struct { + Like *Like +} type DescribeNetworkPolicyRequest struct { name AccountObjectIdentifier // required diff --git a/pkg/sdk/network_policies_gen.go b/pkg/sdk/network_policies_gen.go index e972a6cbc4..b5947c42cb 100644 --- a/pkg/sdk/network_policies_gen.go +++ b/pkg/sdk/network_policies_gen.go @@ -8,7 +8,7 @@ type NetworkPolicies interface { Drop(ctx context.Context, request *DropNetworkPolicyRequest) error Show(ctx context.Context, request *ShowNetworkPolicyRequest) ([]NetworkPolicy, error) ShowByID(ctx context.Context, id AccountObjectIdentifier) (*NetworkPolicy, error) - Describe(ctx context.Context, id AccountObjectIdentifier) ([]NetworkPolicyDescription, error) + Describe(ctx context.Context, id AccountObjectIdentifier) ([]NetworkPolicyProperty, error) } // CreateNetworkPolicyOptions is based on https://docs.snowflake.com/en/sql-reference/sql/create-network-policy. @@ -97,8 +97,9 @@ type DropNetworkPolicyOptions struct { // ShowNetworkPolicyOptions is based on https://docs.snowflake.com/en/sql-reference/sql/show-network-policies. type ShowNetworkPolicyOptions struct { - show bool `ddl:"static" sql:"SHOW"` - networkPolicies bool `ddl:"static" sql:"NETWORK POLICIES"` + show bool `ddl:"static" sql:"SHOW"` + networkPolicies bool `ddl:"static" sql:"NETWORK POLICIES"` + Like *Like `ddl:"keyword" sql:"LIKE"` } type showNetworkPolicyDBRow struct { @@ -133,7 +134,7 @@ type describeNetworkPolicyDBRow struct { Value string `db:"value"` } -type NetworkPolicyDescription struct { +type NetworkPolicyProperty struct { Name string Value string } diff --git a/pkg/sdk/network_policies_gen_test.go b/pkg/sdk/network_policies_gen_test.go index 4d87d79902..cd552f2b9d 100644 --- a/pkg/sdk/network_policies_gen_test.go +++ b/pkg/sdk/network_policies_gen_test.go @@ -279,7 +279,10 @@ func TestNetworkPolicies_Show(t *testing.T) { t.Run("all options", func(t *testing.T) { opts := defaultOpts() - assertOptsValidAndSQLEquals(t, opts, "SHOW NETWORK POLICIES") + opts.Like = &Like{ + Pattern: String("some pattern"), + } + assertOptsValidAndSQLEquals(t, opts, "SHOW NETWORK POLICIES LIKE 'some pattern'") }) } diff --git a/pkg/sdk/network_policies_impl_gen.go b/pkg/sdk/network_policies_impl_gen.go index 4099de87a6..8021781c5b 100644 --- a/pkg/sdk/network_policies_impl_gen.go +++ b/pkg/sdk/network_policies_impl_gen.go @@ -38,7 +38,7 @@ func (v *networkPolicies) Show(ctx context.Context, request *ShowNetworkPolicyRe } func (v *networkPolicies) ShowByID(ctx context.Context, id AccountObjectIdentifier) (*NetworkPolicy, error) { - networkPolicies, err := v.Show(ctx, NewShowNetworkPolicyRequest()) + networkPolicies, err := v.Show(ctx, NewShowNetworkPolicyRequest().WithLike(Like{Pattern: String(id.Name())})) if err != nil { return nil, err } @@ -46,7 +46,7 @@ func (v *networkPolicies) ShowByID(ctx context.Context, id AccountObjectIdentifi return collections.FindOne(networkPolicies, func(r NetworkPolicy) bool { return r.Name == id.Name() }) } -func (v *networkPolicies) Describe(ctx context.Context, id AccountObjectIdentifier) ([]NetworkPolicyDescription, error) { +func (v *networkPolicies) Describe(ctx context.Context, id AccountObjectIdentifier) ([]NetworkPolicyProperty, error) { opts := &DescribeNetworkPolicyOptions{ name: id, } @@ -54,7 +54,7 @@ func (v *networkPolicies) Describe(ctx context.Context, id AccountObjectIdentifi if err != nil { return nil, err } - return convertRows[describeNetworkPolicyDBRow, NetworkPolicyDescription](rows), nil + return convertRows[describeNetworkPolicyDBRow, NetworkPolicyProperty](rows), nil } func (r *CreateNetworkPolicyRequest) toOpts() *CreateNetworkPolicyOptions { @@ -158,7 +158,9 @@ func (r *DropNetworkPolicyRequest) toOpts() *DropNetworkPolicyOptions { } func (r *ShowNetworkPolicyRequest) toOpts() *ShowNetworkPolicyOptions { - opts := &ShowNetworkPolicyOptions{} + opts := &ShowNetworkPolicyOptions{ + Like: r.Like, + } return opts } @@ -181,8 +183,8 @@ func (r *DescribeNetworkPolicyRequest) toOpts() *DescribeNetworkPolicyOptions { return opts } -func (r describeNetworkPolicyDBRow) convert() *NetworkPolicyDescription { - return &NetworkPolicyDescription{ +func (r describeNetworkPolicyDBRow) convert() *NetworkPolicyProperty { + return &NetworkPolicyProperty{ Name: r.Name, Value: r.Value, } diff --git a/pkg/sdk/parsers.go b/pkg/sdk/parsers.go index 8e415373f3..26dc8b4a07 100644 --- a/pkg/sdk/parsers.go +++ b/pkg/sdk/parsers.go @@ -22,18 +22,18 @@ func ParseTimestampWithOffset(s string, dateTimeFormat string) (string, error) { // 1. The list is enclosed by [] brackets, and they shouldn't be a part of any item's value // 2. Items are separated by commas, and they shouldn't be a part of any item's value // 3. Items can have as many spaces in between, but after separation they will be trimmed and shouldn't be a part of any item's value -func ParseCommaSeparatedStringArray(value string) []string { - if strings.HasPrefix(value, "[") && strings.HasSuffix(value, "]") { - if value == "[]" { - return make([]string, 0) - } - list := strings.Trim(value, "[]") - listItems := strings.Split(list, ",") - trimmedListItems := make([]string, len(listItems)) - for i, item := range listItems { - trimmedListItems[i] = strings.TrimSpace(item) +func ParseCommaSeparatedStringArray(value string, trimQuotes bool) []string { + value = strings.Trim(value, "[]") + if value == "" { + return make([]string, 0) + } + listItems := strings.Split(value, ",") + trimmedListItems := make([]string, len(listItems)) + for i, item := range listItems { + trimmedListItems[i] = strings.TrimSpace(item) + if trimQuotes { + trimmedListItems[i] = strings.Trim(trimmedListItems[i], "'") } - return trimmedListItems } - return make([]string, 0) + return trimmedListItems } diff --git a/pkg/sdk/parsers_test.go b/pkg/sdk/parsers_test.go index 6ff3a59c16..425dbf7bcf 100644 --- a/pkg/sdk/parsers_test.go +++ b/pkg/sdk/parsers_test.go @@ -45,13 +45,13 @@ func TestParseCommaSeparatedStringArray(t *testing.T) { { Name: "list without brackets", Value: "one,two,three", - Result: []string{}, + Result: []string{"one", "two", "three"}, }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - assert.Equal(t, tc.Result, ParseCommaSeparatedStringArray(tc.Value)) + assert.Equal(t, tc.Result, ParseCommaSeparatedStringArray(tc.Value, false)) }) } } diff --git a/pkg/sdk/testint/network_policies_gen_integration_test.go b/pkg/sdk/testint/network_policies_gen_integration_test.go index aa2969afd6..38a31eb16f 100644 --- a/pkg/sdk/testint/network_policies_gen_integration_test.go +++ b/pkg/sdk/testint/network_policies_gen_integration_test.go @@ -35,10 +35,10 @@ func TestInt_NetworkPolicies(t *testing.T) { id := testClientHelper().Ids.RandomAccountObjectIdentifier() comment := "some_comment" return sdk.NewCreateNetworkPolicyRequest(id). - WithOrReplace(sdk.Bool(true)). + WithOrReplace(true). WithAllowedIpList([]sdk.IPRequest{*allowedIP}). WithBlockedIpList([]sdk.IPRequest{*blockedIP, *blockedIP2}). - WithComment(&comment) + WithComment(comment) } findNetworkPolicy := func(nps []sdk.NetworkPolicy, name string) (*sdk.NetworkPolicy, error) { @@ -75,7 +75,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithAllowedIpList(sdk.NewAllowedIPListRequest().WithAllowedIPList([]sdk.IPRequest{{IP: "123.0.0.1"}, {IP: "125.0.0.1"}})))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithAllowedIpList(*sdk.NewAllowedIPListRequest().WithAllowedIPList([]sdk.IPRequest{{IP: "123.0.0.1"}, {IP: "125.0.0.1"}})))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -92,7 +92,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithAllowedIpList(sdk.NewAllowedIPListRequest().WithAllowedIPList(nil)))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithAllowedIpList(*sdk.NewAllowedIPListRequest().WithAllowedIPList(nil)))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -109,7 +109,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithUnset(sdk.NewNetworkPolicyUnsetRequest().WithAllowedIpList(sdk.Bool(true)))) + WithUnset(*sdk.NewNetworkPolicyUnsetRequest().WithAllowedIpList(true))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -126,7 +126,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithBlockedIpList(sdk.NewBlockedIPListRequest().WithBlockedIPList([]sdk.IPRequest{{IP: "123.0.0.1"}})))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithBlockedIpList(*sdk.NewBlockedIPListRequest().WithBlockedIPList([]sdk.IPRequest{{IP: "123.0.0.1"}})))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -143,7 +143,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithBlockedIpList(sdk.NewBlockedIPListRequest()))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithBlockedIpList(*sdk.NewBlockedIPListRequest()))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -160,7 +160,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithUnset(sdk.NewNetworkPolicyUnsetRequest().WithBlockedIpList(sdk.Bool(true)))) + WithUnset(*sdk.NewNetworkPolicyUnsetRequest().WithBlockedIpList(true))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -179,7 +179,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithAllowedNetworkRuleList(sdk.NewAllowedNetworkRuleListRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule, allowedNetworkRule2})))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithAllowedNetworkRuleList(*sdk.NewAllowedNetworkRuleListRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule, allowedNetworkRule2})))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -194,7 +194,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithAllowedNetworkRuleList(sdk.NewAllowedNetworkRuleListRequest()))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithAllowedNetworkRuleList(*sdk.NewAllowedNetworkRuleListRequest()))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -209,7 +209,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithUnset(sdk.NewNetworkPolicyUnsetRequest().WithAllowedNetworkRuleList(sdk.Bool(true)))) + WithUnset(*sdk.NewNetworkPolicyUnsetRequest().WithAllowedNetworkRuleList(true))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -225,7 +225,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithBlockedNetworkRuleList(sdk.NewBlockedNetworkRuleListRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule})))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithBlockedNetworkRuleList(*sdk.NewBlockedNetworkRuleListRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule})))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -240,7 +240,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithBlockedNetworkRuleList(sdk.NewBlockedNetworkRuleListRequest()))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithBlockedNetworkRuleList(*sdk.NewBlockedNetworkRuleListRequest()))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -255,7 +255,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithUnset(sdk.NewNetworkPolicyUnsetRequest().WithBlockedNetworkRuleList(sdk.Bool(true)))) + WithUnset(*sdk.NewNetworkPolicyUnsetRequest().WithBlockedNetworkRuleList(true))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -271,7 +271,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithAdd(sdk.NewAddNetworkRuleRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule}))) + WithAdd(*sdk.NewAddNetworkRuleRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule}))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -279,7 +279,7 @@ func TestInt_NetworkPolicies(t *testing.T) { assert.Equal(t, 1, np.EntriesInAllowedNetworkRules) err = client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithRemove(sdk.NewRemoveNetworkRuleRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule}))) + WithRemove(*sdk.NewRemoveNetworkRuleRequest().WithAllowedNetworkRuleList([]sdk.SchemaObjectIdentifier{allowedNetworkRule}))) require.NoError(t, err) np, err = client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -295,7 +295,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithAdd(sdk.NewAddNetworkRuleRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule}))) + WithAdd(*sdk.NewAddNetworkRuleRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule}))) require.NoError(t, err) np, err := client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -303,7 +303,7 @@ func TestInt_NetworkPolicies(t *testing.T) { assert.Equal(t, 1, np.EntriesInBlockedNetworkRules) err = client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithRemove(sdk.NewRemoveNetworkRuleRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule}))) + WithRemove(*sdk.NewRemoveNetworkRuleRequest().WithBlockedNetworkRuleList([]sdk.SchemaObjectIdentifier{blockedNetworkRule}))) require.NoError(t, err) np, err = client.NetworkPolicies.ShowByID(ctx, req.GetName()) @@ -318,7 +318,7 @@ func TestInt_NetworkPolicies(t *testing.T) { alteredComment := "altered_comment" err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()). - WithSet(sdk.NewNetworkPolicySetRequest().WithComment(&alteredComment))) + WithSet(*sdk.NewNetworkPolicySetRequest().WithComment(alteredComment))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -334,7 +334,7 @@ func TestInt_NetworkPolicies(t *testing.T) { _, dropNetworkPolicy := testClientHelper().NetworkPolicy.CreateNetworkPolicyWithRequest(t, req) t.Cleanup(dropNetworkPolicy) - err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()).WithUnset(sdk.NewNetworkPolicyUnsetRequest().WithComment(sdk.Bool(true)))) + err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()).WithUnset(*sdk.NewNetworkPolicyUnsetRequest().WithComment(true))) require.NoError(t, err) nps, err := client.NetworkPolicies.Show(ctx, sdk.NewShowNetworkPolicyRequest()) @@ -351,7 +351,7 @@ func TestInt_NetworkPolicies(t *testing.T) { t.Cleanup(dropNetworkPolicy) newID := testClientHelper().Ids.RandomAccountObjectIdentifier() - err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()).WithRenameTo(&newID)) + err := client.NetworkPolicies.Alter(ctx, sdk.NewAlterNetworkPolicyRequest(req.GetName()).WithRenameTo(newID)) require.NoError(t, err) t.Cleanup(testClientHelper().NetworkPolicy.DropNetworkPolicyFunc(t, newID)) @@ -375,7 +375,7 @@ func TestInt_NetworkPolicies(t *testing.T) { require.NoError(t, err) assert.Equal(t, 2, len(desc)) - assert.Contains(t, desc, sdk.NetworkPolicyDescription{Name: "ALLOWED_IP_LIST", Value: allowedIP.IP}) - assert.Contains(t, desc, sdk.NetworkPolicyDescription{Name: "BLOCKED_IP_LIST", Value: fmt.Sprintf("%s,%s", blockedIP.IP, blockedIP2.IP)}) + assert.Contains(t, desc, sdk.NetworkPolicyProperty{Name: "ALLOWED_IP_LIST", Value: allowedIP.IP}) + assert.Contains(t, desc, sdk.NetworkPolicyProperty{Name: "BLOCKED_IP_LIST", Value: fmt.Sprintf("%s,%s", blockedIP.IP, blockedIP2.IP)}) }) } diff --git a/templates/data-sources/network_policies.md.tmpl b/templates/data-sources/network_policies.md.tmpl new file mode 100644 index 0000000000..18e5fffd7a --- /dev/null +++ b/templates/data-sources/network_policies.md.tmpl @@ -0,0 +1,24 @@ +--- +page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}" +subcategory: "" +description: |- +{{ if gt (len (split .Description "")) 1 -}} +{{ index (split .Description "") 1 | plainmarkdown | trimspace | prefixlines " " }} +{{- else -}} +{{ .Description | plainmarkdown | trimspace | prefixlines " " }} +{{- end }} +--- + +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it. + +# {{.Name}} ({{.Type}}) + +{{ .Description | trimspace }} + +{{ if .HasExample -}} +## Example Usage + +{{ tffile (printf "examples/data-sources/%s/data-source.tf" .Name)}} +{{- end }} + +{{ .SchemaMarkdown | trimspace }} diff --git a/templates/resources/network_policy.md.tmpl b/templates/resources/network_policy.md.tmpl new file mode 100644 index 0000000000..e7f66bbf91 --- /dev/null +++ b/templates/resources/network_policy.md.tmpl @@ -0,0 +1,32 @@ +--- +page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}" +subcategory: "" +description: |- +{{ if gt (len (split .Description "")) 1 -}} +{{ index (split .Description "") 1 | plainmarkdown | trimspace | prefixlines " " }} +{{- else -}} +{{ .Description | plainmarkdown | trimspace | prefixlines " " }} +{{- end }} +--- + +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it. + +# {{.Name}} ({{.Type}}) + +{{ .Description | trimspace }} + +{{ if .HasExample -}} +## Example Usage + +{{ tffile (printf "examples/resources/%s/resource.tf" .Name)}} +{{- end }} + +{{ .SchemaMarkdown | trimspace }} +{{- if .HasImport }} + +## Import + +Import is supported using the following syntax: + +{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}} +{{- end }} From e545ec3ae41aace52ca3cea6a3de23fc565e5968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Thu, 11 Jul 2024 14:05:54 +0200 Subject: [PATCH 2/7] wip --- .../network_policies_acceptance_test.go | 22 ++++- pkg/resources/network_policy.go | 3 - .../network_policy_acceptance_test.go | 86 +++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/pkg/datasources/network_policies_acceptance_test.go b/pkg/datasources/network_policies_acceptance_test.go index 131d592d6e..b88e7de076 100644 --- a/pkg/datasources/network_policies_acceptance_test.go +++ b/pkg/datasources/network_policies_acceptance_test.go @@ -9,7 +9,6 @@ import ( acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -18,7 +17,7 @@ import ( ) func TestAcc_NetworkPolicies_Complete(t *testing.T) { - _ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) + //_ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) id := acc.TestClient().Ids.RandomAccountObjectIdentifier() id2 := acc.TestClient().Ids.RandomAccountObjectIdentifier() @@ -96,6 +95,25 @@ func TestAcc_NetworkPolicies_Complete(t *testing.T) { resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_network_rule_list"), ), }, + { + Config: networkPolicyConfigBasic(id2.Name(), true), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.#", "1"), + resource.TestCheckResourceAttrSet("data.snowflake_network_policies.test", "network_policies.0.show_output.0.created_on"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.name", id2.Name()), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_ip_list", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_allowed_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.entries_in_blocked_network_rules", "0"), + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.show_output.0.comment", ""), + + resource.TestCheckResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.#", "1"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_ip_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_ip_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.allowed_network_rule_list"), + resource.TestCheckNoResourceAttr("data.snowflake_network_policies.test", "network_policies.0.describe_output.0.blocked_network_rule_list"), + ), + }, { Config: networkPolicyConfigBasic(id.Name(), false), Check: resource.ComposeTestCheckFunc( diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index 966ffc8c16..40c806b6bf 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "log" "reflect" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" @@ -194,8 +193,6 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta return diag.FromErr(err) } - log.Println("PROPS:", policyProperties) - if allowedIpList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_IP_LIST" }); err == nil { if err = d.Set("allowed_ip_list", sdk.ParseCommaSeparatedStringArray(allowedIpList.Value, false)); err != nil { return diag.FromErr(err) diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index 28c776818e..cebf3e1f56 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -405,3 +405,89 @@ func networkPolicyConfigComplete( comment, ) } + +// TODO: Test upgrade + +func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + //allowedNetworkRuleId1 := acc.TestClient().Ids.NewSchemaObjectIdentifier() + allowedNetworkRuleId1 := sdk.NewSchemaObjectIdentifier(acc.TestClient().Ids.DatabaseId().Name(), acc.TestClient().Ids.SchemaId().Name(), "INTERNAL_TRAFFIC") + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.NetworkPolicy), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.92.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + PreConfig: func() { + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) + acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + }, + Config: networkPolicyConfigWithNetworkRules( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + ), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), + + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + // + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: networkPolicyConfigWithNetworkRules( + id.Name(), + []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, + []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + ), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), + + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + // + //resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + ), + }, + }, + }) +} + +func networkPolicyConfigWithNetworkRules(name string, allowedNetworkRules []string, blockedNetworkRules []string) string { + allowedRuleListBytes, _ := json.Marshal(allowedNetworkRules) + blockedRuleListBytes, _ := json.Marshal(blockedNetworkRules) + + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%[1]s" + allowed_network_rule_list = %[2]s + blocked_network_rule_list = %[3]s + }`, name, string(allowedRuleListBytes), string(blockedRuleListBytes)) +} From ca16218731a447b639488799d662016c5d3f1f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 12 Jul 2024 10:10:38 +0200 Subject: [PATCH 3/7] Changes after review --- pkg/acceptance/helpers/network_rule_client.go | 7 +---- .../network_policies_acceptance_test.go | 22 ++++----------- pkg/resources/network_policy.go | 28 ++++++++----------- .../network_policy_acceptance_test.go | 24 ++++++++-------- 4 files changed, 30 insertions(+), 51 deletions(-) diff --git a/pkg/acceptance/helpers/network_rule_client.go b/pkg/acceptance/helpers/network_rule_client.go index 5093fc2277..348df52b16 100644 --- a/pkg/acceptance/helpers/network_rule_client.go +++ b/pkg/acceptance/helpers/network_rule_client.go @@ -26,12 +26,7 @@ func (c *NetworkRuleClient) client() sdk.NetworkRules { func (c *NetworkRuleClient) Create(t *testing.T) *sdk.NetworkRule { t.Helper() - return c.CreateWithName(t, c.ids.Alpha()) -} - -func (c *NetworkRuleClient) CreateWithName(t *testing.T, name string) *sdk.NetworkRule { - t.Helper() - return c.CreateWithIdentifier(t, c.ids.NewSchemaObjectIdentifier(name)) + return c.CreateWithIdentifier(t, c.ids.RandomSchemaObjectIdentifier()) } func (c *NetworkRuleClient) CreateWithIdentifier(t *testing.T, id sdk.SchemaObjectIdentifier) *sdk.NetworkRule { diff --git a/pkg/datasources/network_policies_acceptance_test.go b/pkg/datasources/network_policies_acceptance_test.go index b88e7de076..2c9806a2d8 100644 --- a/pkg/datasources/network_policies_acceptance_test.go +++ b/pkg/datasources/network_policies_acceptance_test.go @@ -1,7 +1,6 @@ package datasources_test import ( - "context" "encoding/json" "fmt" "regexp" @@ -10,29 +9,23 @@ import ( acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/tfversion" - "github.com/stretchr/testify/require" ) func TestAcc_NetworkPolicies_Complete(t *testing.T) { - //_ = testenvs.GetOrSkipTest(t, testenvs.ConfigureClientOnce) - id := acc.TestClient().Ids.RandomAccountObjectIdentifier() id2 := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() - databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier() - schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId) - - allowedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) - allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) - blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) - blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierInSchema(schemaId) + allowedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, @@ -40,11 +33,6 @@ func TestAcc_NetworkPolicies_Complete(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() { - db, _ := acc.TestClient().Database.CreateDatabaseWithIdentifier(t, databaseId) - t.Cleanup(func() { - require.NoError(t, acc.Client(t).Databases.Drop(context.Background(), db.ID(), &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)})) - }) - acc.TestClient().Schema.CreateSchemaWithIdentifier(t, schemaId) acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index 40c806b6bf..eb015da9b6 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -193,24 +193,20 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta return diag.FromErr(err) } - if allowedIpList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_IP_LIST" }); err == nil { - if err = d.Set("allowed_ip_list", sdk.ParseCommaSeparatedStringArray(allowedIpList.Value, false)); err != nil { - return diag.FromErr(err) - } - } else { - if err = d.Set("allowed_ip_list", []any{}); err != nil { - return diag.FromErr(err) - } + allowedIpList := make([]string, 0) + if allowedIpListProperty, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_IP_LIST" }); err == nil { + allowedIpList = append(allowedIpList, sdk.ParseCommaSeparatedStringArray(allowedIpListProperty.Value, false)...) + } + if err = d.Set("allowed_ip_list", allowedIpList); err != nil { + return diag.FromErr(err) } - if blockedIpList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "BLOCKED_IP_LIST" }); err == nil { - if err = d.Set("blocked_ip_list", sdk.ParseCommaSeparatedStringArray(blockedIpList.Value, false)); err != nil { - return diag.FromErr(err) - } - } else { - if err = d.Set("blocked_ip_list", []any{}); err != nil { - return diag.FromErr(err) - } + blockedIpList := make([]string, 0) + if blockedIpListProperty, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "BLOCKED_IP_LIST" }); err == nil { + blockedIpList = append(blockedIpList, sdk.ParseCommaSeparatedStringArray(blockedIpListProperty.Value, false)...) + } + if err = d.Set("blocked_ip_list", blockedIpList); err != nil { + return diag.FromErr(err) } allowedNetworkRules := make([]string, 0) diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index cebf3e1f56..f151aba8aa 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -79,10 +79,10 @@ func TestAcc_NetworkPolicy_Basic(t *testing.T) { // set optionals { PreConfig: func() { - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId2) }, Config: networkPolicyConfigComplete( id.Name(), @@ -246,10 +246,10 @@ func TestAcc_NetworkPolicy_Complete(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() { - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId2) }, Config: networkPolicyConfigComplete( id.Name(), @@ -432,10 +432,10 @@ func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { }, }, PreConfig: func() { - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, allowedNetworkRuleId2.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId1.Name()) - acc.TestClient().NetworkRule.CreateWithName(t, blockedNetworkRuleId2.Name()) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId2) }, Config: networkPolicyConfigWithNetworkRules( id.Name(), From e9bce1568e0e7c2c110b319a6446be4aba9fd71c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 12 Jul 2024 15:19:02 +0200 Subject: [PATCH 4/7] Changes after review --- MIGRATION_GUIDE.md | 2 +- pkg/helpers/helpers.go | 23 +++++ pkg/helpers/helpers_test.go | 91 +++++++++++++++++++ pkg/resources/diff_suppressions.go | 22 +++++ pkg/resources/diff_suppressions_test.go | 70 ++++++++++++++ pkg/resources/network_policy.go | 34 +++---- .../network_policy_acceptance_test.go | 58 ++++++------ pkg/sdk/network_policies_def.go | 18 +++- 8 files changed, 266 insertions(+), 52 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 97b74ee5ff..6aa77e7f58 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -15,7 +15,7 @@ New behavior: - Additional validation was added to `blocked_ip_list` to inform about specifying `0.0.0.0/0` ip. More details in the [official documentation](https://docs.snowflake.com/en/sql-reference/sql/create-network-policy#usage-notes). New fields: -- `show_output` and `describe_output` added to hold the results returned by `SHOW` and `DESCRIBE` commands. +- `show_output` and `describe_output` added to hold the results returned by `SHOW` and `DESCRIBE` commands. Those fields will only be recomputed when specified fields change ### *(new feature)* snowflake_network_policies datasource diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index c190f4f3d9..09b7c15a61 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -160,3 +160,26 @@ func ConcatSlices[T any](slices ...[]T) []T { } return tmp } + +func ContainsIdentifierIgnoreQuotes(ids []string, valueToCompare string) bool { + if len(ids) == 0 || len(valueToCompare) == 0 { + return false + } + + idToCompare, err := DecodeSnowflakeParameterID(valueToCompare) + if err != nil { + return false + } + + for _, idString := range ids { + id, err := DecodeSnowflakeParameterID(idString) + if err != nil { + return false + } + if idToCompare.FullyQualifiedName() == id.FullyQualifiedName() { + return true + } + } + + return false +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index f6198b6fb6..114a8993cf 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -2,6 +2,7 @@ package helpers import ( "fmt" + "github.com/stretchr/testify/assert" "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -221,3 +222,93 @@ func Test_DecodeSnowflakeAccountIdentifier(t *testing.T) { require.ErrorContains(t, err, fmt.Sprintf("unable to read identifier: %s", id)) }) } + +func Test_ContainsIdentifierIgnoreQuotes(t *testing.T) { + testCases := []struct { + Name string + Ids []string + Id string + ShouldContain bool + }{ + { + Name: "validation: nil Ids", + Id: "id", + }, + { + Name: "validation: empty Id", + Ids: []string{"id"}, + Id: "", + }, + { + Name: "validation: Ids with too many parts", + Ids: []string{"this.id.has.too.many.parts"}, + Id: "id", + }, + { + Name: "validation: Id with too many parts", + Ids: []string{"id"}, + Id: "this.id.has.too.many.parts", + }, + { + Name: "validation: account object identifier in Ids ignore quotes with upper cased Id", + Ids: []string{"object", "db.schema", "db.schema.object"}, + Id: "\"OBJECT\"", + }, + { + Name: "validation: account object identifier in Ids ignore quotes with upper cased id in Ids", + Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, + Id: "\"object\"", + }, + { + Name: "validation: account object identifier in Ids ignore quotes with upper cased id in Ids", + Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, + Id: "\"object\"", + }, + { + // TODO: In cases like these the original quoting should be retrieved to avoid situations like "object" == "\"object\"" (true) + // where that should not be a valid comparison (different ids). Right now, we assume the cases covered by the suppress diff are + // cases where the only different parts are upper-cased and returned without quotes by snowflake, e.g. "OBJECT" == "\"OBJECT\"" + // and this is valid comparison (the same ids). + Name: "account object identifier in Ids", + Ids: []string{"object", "db.schema", "db.schema.object"}, + Id: "\"object\"", + ShouldContain: true, + }, + { + Name: "database object identifier in Ids", + Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, + Id: "\"db\".\"schema\"", + ShouldContain: true, + }, + { + Name: "schema object identifier in Ids", + Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, + Id: "\"db\".\"schema\".\"object\"", + ShouldContain: true, + }, + { + Name: "account object identifier in Ids upper-cased", + Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, + Id: "\"OBJECT\"", + ShouldContain: true, + }, + { + Name: "database object identifier in Ids upper-cased", + Ids: []string{"object", "DB.SCHEMA", "db.schema.object"}, + Id: "\"DB\".\"SCHEMA\"", + ShouldContain: true, + }, + { + Name: "schema object identifier in Ids upper-cased", + Ids: []string{"object", "db.schema", "DB.SCHEMA.OBJECT"}, + Id: "\"DB\".\"SCHEMA\".\"OBJECT\"", + ShouldContain: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + assert.Equal(t, tc.ShouldContain, ContainsIdentifierIgnoreQuotes(tc.Ids, tc.Id)) + }) + } +} diff --git a/pkg/resources/diff_suppressions.go b/pkg/resources/diff_suppressions.go index f990094c0d..a401b9bcea 100644 --- a/pkg/resources/diff_suppressions.go +++ b/pkg/resources/diff_suppressions.go @@ -116,3 +116,25 @@ func IgnoreValuesFromSetIfParamSet(key, param string, values []string) schema.Sc return slices.Contains(values, old) } } + +func NormalizeAndCompareIdentifiersInSet(key string) schema.SchemaDiffSuppressFunc { + return func(k, oldValue, newValue string, d *schema.ResourceData) bool { + if strings.HasSuffix(k, ".#") { + return false + } + + if oldValue == "" && !d.GetRawState().IsNull() { + if helpers.ContainsIdentifierIgnoreQuotes(ctyValToSliceString(d.GetRawState().AsValueMap()[key].AsValueSet().Values()), newValue) { + return true + } + } + + if newValue == "" { + if helpers.ContainsIdentifierIgnoreQuotes(expandStringList(d.Get(key).(*schema.Set).List()), oldValue) { + return true + } + } + + return false + } +} diff --git a/pkg/resources/diff_suppressions_test.go b/pkg/resources/diff_suppressions_test.go index 9bf249d338..818f867381 100644 --- a/pkg/resources/diff_suppressions_test.go +++ b/pkg/resources/diff_suppressions_test.go @@ -80,3 +80,73 @@ func Test_IgnoreAfterCreation(t *testing.T) { assert.True(t, result) }) } + +func Test_NormalizeAndCompareIdentifiersSet(t *testing.T) { + rawDataWithValues := func(values []any) *schema.ResourceData { + return schema.TestResourceDataRaw(t, map[string]*schema.Schema{ + "value": { + Required: true, + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, map[string]any{ + "value": values, + }) + } + emptyResourceData := rawDataWithValues([]any{}) + + t.Run("validation: size key", func(t *testing.T) { + assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.#", "1", "2", emptyResourceData)) + }) + + t.Run("validation: case mismatch", func(t *testing.T) { + resourceData := rawDataWithValues([]any{"SCHEMA.OBJECT.IDENTIFIER"}) + assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run("validation: case mismatch quoted identifier in the state", func(t *testing.T) { + resourceData := rawDataWithValues([]any{`"SCHEMA"."OBJECT"."IDENTIFIER"`}) + assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run(`change detected from schema.object.identifier to "schema"."object"."identifier" with schema.object.identifier in state`, func(t *testing.T) { + resourceData := rawDataWithValues([]any{"schema.object.identifier"}) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run(`change detected from schema.object.identifier to "schema"."object"."identifier" with "schema"."object"."identifier" in state`, func(t *testing.T) { + resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with schema.object.identifier in state`, func(t *testing.T) { + resourceData := rawDataWithValues([]any{"schema.object.identifier"}) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state`, func(t *testing.T) { + resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + }) + + t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state uppercased`, func(t *testing.T) { + resourceData := rawDataWithValues([]any{`"SCHEMA"."OBJECT"."IDENTIFIER"`}) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "SCHEMA.OBJECT.IDENTIFIER", "", resourceData)) + // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"SCHEMA"."OBJECT"."IDENTIFIER"`, resourceData)) + }) +} diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index eb015da9b6..cbacb900e6 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -2,16 +2,14 @@ package resources import ( "context" - "encoding/json" "errors" "fmt" - "reflect" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" + "reflect" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -30,8 +28,9 @@ var networkPolicySchema = map[string]*schema.Schema{ Type: schema.TypeString, ValidateDiagFunc: IsValidIdentifier[sdk.SchemaObjectIdentifier](), }, - Optional: true, - Description: "Specifies a list of fully qualified network rules that contain the network identifiers that are allowed access to Snowflake.", + DiffSuppressFunc: NormalizeAndCompareIdentifiersInSet("allowed_network_rule_list"), + Optional: true, + Description: "Specifies a list of fully qualified network rules that contain the network identifiers that are allowed access to Snowflake.", }, "blocked_network_rule_list": { Type: schema.TypeSet, @@ -39,8 +38,9 @@ var networkPolicySchema = map[string]*schema.Schema{ Type: schema.TypeString, ValidateDiagFunc: IsValidIdentifier[sdk.SchemaObjectIdentifier](), }, - Optional: true, - Description: "Specifies a list of fully qualified network rules that contain the network identifiers that are denied access to Snowflake.", + DiffSuppressFunc: NormalizeAndCompareIdentifiersInSet("blocked_network_rule_list"), + Optional: true, + Description: "Specifies a list of fully qualified network rules that contain the network identifiers that are denied access to Snowflake.", }, "allowed_ip_list": { Type: schema.TypeSet, @@ -91,18 +91,22 @@ func NetworkPolicy() *schema.Resource { Description: "Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies.", CustomizeDiff: customdiff.All( + // TODO: For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented and the implementation + // for ComputedIfAnyAttributeChanged has to be adjusted. The main issue lays in fields that have diff suppression. + // When the value in state and the value in config are different (which is notmal with diff suppressions) show + // and decribe outputs are constantly recomputed (which will appear in every terraform plan). ComputedIfAnyAttributeChanged( ShowOutputAttributeName, - "allowed_network_rule_list", - "blocked_network_rule_list", + //"allowed_network_rule_list", + //"blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", "comment", ), ComputedIfAnyAttributeChanged( DescribeOutputAttributeName, - "allowed_network_rule_list", - "blocked_network_rule_list", + //"allowed_network_rule_list", + //"blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", ), @@ -211,12 +215,10 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta allowedNetworkRules := make([]string, 0) if allowedNetworkRuleList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "ALLOWED_NETWORK_RULE_LIST" }); err == nil { - var networkRules []sdk.NetworkRulesSnowflakeDTO - err := json.Unmarshal([]byte(allowedNetworkRuleList.Value), &networkRules) + networkRules, err := sdk.ParseNetworkRulesSnowflakeDto(allowedNetworkRuleList.Value) if err != nil { return diag.FromErr(err) } - for _, networkRule := range networkRules { allowedNetworkRules = append(allowedNetworkRules, sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(networkRule.FullyQualifiedRuleName).FullyQualifiedName()) } @@ -227,12 +229,10 @@ func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta blockedNetworkRules := make([]string, 0) if blockedNetworkRuleList, err := collections.FindOne(policyProperties, func(prop sdk.NetworkPolicyProperty) bool { return prop.Name == "BLOCKED_NETWORK_RULE_LIST" }); err == nil { - var networkRules []sdk.NetworkRulesSnowflakeDTO - err := json.Unmarshal([]byte(blockedNetworkRuleList.Value), &networkRules) + networkRules, err := sdk.ParseNetworkRulesSnowflakeDto(blockedNetworkRuleList.Value) if err != nil { return diag.FromErr(err) } - for _, networkRule := range networkRules { blockedNetworkRules = append(blockedNetworkRules, sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(networkRule.FullyQualifiedRuleName).FullyQualifiedName()) } diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index f151aba8aa..6f568dba60 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -406,16 +406,12 @@ func networkPolicyConfigComplete( ) } -// TODO: Test upgrade - func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() - - //allowedNetworkRuleId1 := acc.TestClient().Ids.NewSchemaObjectIdentifier() - allowedNetworkRuleId1 := sdk.NewSchemaObjectIdentifier(acc.TestClient().Ids.DatabaseId().Name(), acc.TestClient().Ids.SchemaId().Name(), "INTERNAL_TRAFFIC") - allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() - blockedNetworkRuleId1 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() - blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + allowedNetworkRuleId := acc.TestClient().Ids.RandomSchemaObjectIdentifierWithPrefix("ALLOWED") + allowedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierWithPrefix("ALLOWED") + blockedNetworkRuleId := acc.TestClient().Ids.RandomSchemaObjectIdentifierWithPrefix("BLOCKED") + blockedNetworkRuleId2 := acc.TestClient().Ids.RandomSchemaObjectIdentifierWithPrefix("BLOCKED") resource.Test(t, resource.TestCase{ PreCheck: func() { acc.TestAccPreCheck(t) }, @@ -431,50 +427,50 @@ func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { Source: "Snowflake-Labs/snowflake", }, }, + // Identifier quoting mismatch (no diff suppression) + ExpectNonEmptyPlan: true, PreConfig: func() { - acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId) acc.TestClient().NetworkRule.CreateWithIdentifier(t, allowedNetworkRuleId2) - acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId1) + acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId) acc.TestClient().NetworkRule.CreateWithIdentifier(t, blockedNetworkRuleId2) }, Config: networkPolicyConfigWithNetworkRules( id.Name(), - []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, - []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId.DatabaseName(), allowedNetworkRuleId.SchemaName(), allowedNetworkRuleId.Name())}, + []string{fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId.DatabaseName(), blockedNetworkRuleId.SchemaName(), blockedNetworkRuleId.Name())}, ), Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), - - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), - // - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), - //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), - //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "1"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "1"), ), }, { ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, Config: networkPolicyConfigWithNetworkRules( id.Name(), - []string{allowedNetworkRuleId1.FullyQualifiedName(), allowedNetworkRuleId2.FullyQualifiedName()}, - []string{blockedNetworkRuleId1.FullyQualifiedName(), blockedNetworkRuleId2.FullyQualifiedName()}, + []string{ + fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId.DatabaseName(), allowedNetworkRuleId.SchemaName(), allowedNetworkRuleId.Name()), + fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId2.DatabaseName(), allowedNetworkRuleId2.SchemaName(), allowedNetworkRuleId2.Name()), + }, + []string{ + fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId.DatabaseName(), blockedNetworkRuleId.SchemaName(), blockedNetworkRuleId.Name()), + fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId2.DatabaseName(), blockedNetworkRuleId2.SchemaName(), blockedNetworkRuleId2.Name()), + }, ), Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), - // - //resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), - //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), - //resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.#", "1"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_allowed_network_rules", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "show_output.0.entries_in_blocked_network_rules", "2"), + + resource.TestCheckResourceAttr("snowflake_network_policy.test", "describe_output.#", "1"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.allowed_network_rule_list"), + resource.TestCheckResourceAttrSet("snowflake_network_policy.test", "describe_output.0.blocked_network_rule_list"), ), }, }, diff --git a/pkg/sdk/network_policies_def.go b/pkg/sdk/network_policies_def.go index 608b679325..ea928e7833 100644 --- a/pkg/sdk/network_policies_def.go +++ b/pkg/sdk/network_policies_def.go @@ -1,14 +1,26 @@ package sdk -import g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator" +import ( + "encoding/json" + g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator" +) //go:generate go run ./poc/main.go -// NetworkRulesSnowflakeDTO is needed to unpack the applied network rules from the JSON response from Snowflake -type NetworkRulesSnowflakeDTO struct { +// NetworkRulesSnowflakeDto is needed to unpack the applied network rules from the JSON response from Snowflake +type NetworkRulesSnowflakeDto struct { FullyQualifiedRuleName string } +func ParseNetworkRulesSnowflakeDto(networkRulesStringValue string) ([]NetworkRulesSnowflakeDto, error) { + var networkRules []NetworkRulesSnowflakeDto + err := json.Unmarshal([]byte(networkRulesStringValue), &networkRules) + if err != nil { + return nil, err + } + return networkRules, nil +} + var ( ip = g.NewQueryStruct("IP"). Text("IP", g.KeywordOptions().SingleQuotes().Required()) From 59fbe5a16ffe67cbcc9cab159f3156bffbaad05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 12 Jul 2024 16:12:48 +0200 Subject: [PATCH 5/7] Changes after review --- pkg/helpers/helpers.go | 19 +++++++++++++------ pkg/helpers/helpers_test.go | 7 ++----- pkg/resources/diff_suppressions.go | 9 +++++++++ pkg/resources/diff_suppressions_test.go | 8 ++++---- pkg/resources/network_policy.go | 17 +++++++++-------- pkg/sdk/network_policies_def.go | 1 + 6 files changed, 38 insertions(+), 23 deletions(-) diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 09b7c15a61..aa15623bd5 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -161,22 +161,29 @@ func ConcatSlices[T any](slices ...[]T) []T { return tmp } -func ContainsIdentifierIgnoreQuotes(ids []string, valueToCompare string) bool { - if len(ids) == 0 || len(valueToCompare) == 0 { +// ContainsIdentifierIgnoreQuotes takes ids (a slice of Snowflake identifiers represented as strings), and +// id (a string representing Snowflake id). It checks if id is contained within ids ignoring quotes around identifier parts. +// +// The original quoting should be retrieved to avoid situations like "object" == "\"object\"" (true) +// where that should not be a truthful comparison (different ids). Right now, we assume this case won't happen because the quoting difference would only appear +// in cases where the identifier parts are upper-cased and returned without quotes by snowflake, e.g. "OBJECT" == "\"OBJECT\"" (true) +// which is correct (the same ids). +func ContainsIdentifierIgnoreQuotes(ids []string, id string) bool { + if len(ids) == 0 || len(id) == 0 { return false } - idToCompare, err := DecodeSnowflakeParameterID(valueToCompare) + idToCompare, err := DecodeSnowflakeParameterID(id) if err != nil { return false } - for _, idString := range ids { - id, err := DecodeSnowflakeParameterID(idString) + for _, stringId := range ids { + objectIdentifier, err := DecodeSnowflakeParameterID(stringId) if err != nil { return false } - if idToCompare.FullyQualifiedName() == id.FullyQualifiedName() { + if idToCompare.FullyQualifiedName() == objectIdentifier.FullyQualifiedName() { return true } } diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index 114a8993cf..d67888d505 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -2,9 +2,10 @@ package helpers import ( "fmt" - "github.com/stretchr/testify/assert" "testing" + "github.com/stretchr/testify/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/stretchr/testify/require" ) @@ -265,10 +266,6 @@ func Test_ContainsIdentifierIgnoreQuotes(t *testing.T) { Id: "\"object\"", }, { - // TODO: In cases like these the original quoting should be retrieved to avoid situations like "object" == "\"object\"" (true) - // where that should not be a valid comparison (different ids). Right now, we assume the cases covered by the suppress diff are - // cases where the only different parts are upper-cased and returned without quotes by snowflake, e.g. "OBJECT" == "\"OBJECT\"" - // and this is valid comparison (the same ids). Name: "account object identifier in Ids", Ids: []string{"object", "db.schema", "db.schema.object"}, Id: "\"object\"", diff --git a/pkg/resources/diff_suppressions.go b/pkg/resources/diff_suppressions.go index a401b9bcea..dc8ea75fc3 100644 --- a/pkg/resources/diff_suppressions.go +++ b/pkg/resources/diff_suppressions.go @@ -117,6 +117,15 @@ func IgnoreValuesFromSetIfParamSet(key, param string, values []string) schema.Sc } } +// NormalizeAndCompareIdentifiersInSet is a diff suppression function that should be used at top-level TypeSet fields that +// hold identifiers to avoid diffs like: +// - "DATABASE"."SCHEMA"."OBJECT" +// + DATABASE.SCHEMA.OBJECT +// where both identifiers are pointing to the same object, but have different structure. When a diff occurs in the +// list or set, we have to handle two suppressions (one that prevents adding and one that prevents the removal). +// It's handled by the two statements with the help of helpers.ContainsIdentifierIgnoreQuotes and by getting +// the current state of ids to compare against. The dff suppressions for lists and sets are running for each element one by one +// and the first diff is usually .# referring to the collection length (we skip those). func NormalizeAndCompareIdentifiersInSet(key string) schema.SchemaDiffSuppressFunc { return func(k, oldValue, newValue string, d *schema.ResourceData) bool { if strings.HasSuffix(k, ".#") { diff --git a/pkg/resources/diff_suppressions_test.go b/pkg/resources/diff_suppressions_test.go index 818f867381..e67a8a0555 100644 --- a/pkg/resources/diff_suppressions_test.go +++ b/pkg/resources/diff_suppressions_test.go @@ -126,27 +126,27 @@ func Test_NormalizeAndCompareIdentifiersSet(t *testing.T) { resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with schema.object.identifier in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{"schema.object.identifier"}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state uppercased`, func(t *testing.T) { resourceData := rawDataWithValues([]any{`"SCHEMA"."OBJECT"."IDENTIFIER"`}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "SCHEMA.OBJECT.IDENTIFIER", "", resourceData)) // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - //assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"SCHEMA"."OBJECT"."IDENTIFIER"`, resourceData)) + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"SCHEMA"."OBJECT"."IDENTIFIER"`, resourceData)) }) } diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index cbacb900e6..0c264e5fe4 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -4,12 +4,13 @@ import ( "context" "errors" "fmt" + "reflect" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" - "reflect" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -91,22 +92,22 @@ func NetworkPolicy() *schema.Resource { Description: "Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies.", CustomizeDiff: customdiff.All( - // TODO: For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented and the implementation + // For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented and the implementation // for ComputedIfAnyAttributeChanged has to be adjusted. The main issue lays in fields that have diff suppression. - // When the value in state and the value in config are different (which is notmal with diff suppressions) show - // and decribe outputs are constantly recomputed (which will appear in every terraform plan). + // When the value in state and the value in config are different (which is normal with diff suppressions) show + // and describe outputs are constantly recomputed (which will appear in every terraform plan). ComputedIfAnyAttributeChanged( ShowOutputAttributeName, - //"allowed_network_rule_list", - //"blocked_network_rule_list", + // "allowed_network_rule_list", + // "blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", "comment", ), ComputedIfAnyAttributeChanged( DescribeOutputAttributeName, - //"allowed_network_rule_list", - //"blocked_network_rule_list", + // "allowed_network_rule_list", + // "blocked_network_rule_list", "allowed_ip_list", "blocked_ip_list", ), diff --git a/pkg/sdk/network_policies_def.go b/pkg/sdk/network_policies_def.go index ea928e7833..77692c4ef5 100644 --- a/pkg/sdk/network_policies_def.go +++ b/pkg/sdk/network_policies_def.go @@ -2,6 +2,7 @@ package sdk import ( "encoding/json" + g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/generator" ) From 933ebc315e4068a565b22c5ca11922dabac41d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Mon, 22 Jul 2024 14:46:29 +0200 Subject: [PATCH 6/7] Changes after review --- MIGRATION_GUIDE.md | 4 +-- pkg/helpers/helpers.go | 4 +-- pkg/helpers/helpers_test.go | 7 +--- pkg/resources/diff_suppressions.go | 8 ++--- pkg/resources/diff_suppressions_test.go | 34 +++++++++---------- pkg/resources/network_policy.go | 2 +- .../network_policy_acceptance_test.go | 16 ++++++--- 7 files changed, 38 insertions(+), 37 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 603410a5a1..03538cce1e 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -21,8 +21,6 @@ Changes: - `pattern` was renamed to `like` - output of SHOW is enclosed in `show_output`, so before, e.g. `roles.0.comment` is now `roles.0.show_output.0.comment` -## v0.92.0 ➞ v0.93.0 - ### *(new feature)* refactored snowflake_network_policy resource No migration required. @@ -44,6 +42,8 @@ Added a new datasource enabling querying and filtering network policies. Notes: The additional parameters call **DESC NETWORK POLICY** (with `with_describe` turned on) **per network policy** returned by **SHOW NETWORK POLICIES**. It's important to limit the records and calls to Snowflake to the minimum. That's why we recommend assessing which information you need from the data source and then providing strong filters and turning off additional fields for better plan performance. +## v0.92.0 ➞ v0.93.0 + ### general changes With this change we introduce the first resources redesigned for the V1. We have made a few design choices that will be reflected in these and in the further reworked resources. This includes: diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index aa15623bd5..b64dec4bfb 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -161,14 +161,14 @@ func ConcatSlices[T any](slices ...[]T) []T { return tmp } -// ContainsIdentifierIgnoreQuotes takes ids (a slice of Snowflake identifiers represented as strings), and +// ContainsIdentifierIgnoringQuotes takes ids (a slice of Snowflake identifiers represented as strings), and // id (a string representing Snowflake id). It checks if id is contained within ids ignoring quotes around identifier parts. // // The original quoting should be retrieved to avoid situations like "object" == "\"object\"" (true) // where that should not be a truthful comparison (different ids). Right now, we assume this case won't happen because the quoting difference would only appear // in cases where the identifier parts are upper-cased and returned without quotes by snowflake, e.g. "OBJECT" == "\"OBJECT\"" (true) // which is correct (the same ids). -func ContainsIdentifierIgnoreQuotes(ids []string, id string) bool { +func ContainsIdentifierIgnoringQuotes(ids []string, id string) bool { if len(ids) == 0 || len(id) == 0 { return false } diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index d67888d505..5a6c64d350 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -260,11 +260,6 @@ func Test_ContainsIdentifierIgnoreQuotes(t *testing.T) { Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, Id: "\"object\"", }, - { - Name: "validation: account object identifier in Ids ignore quotes with upper cased id in Ids", - Ids: []string{"OBJECT", "db.schema", "db.schema.object"}, - Id: "\"object\"", - }, { Name: "account object identifier in Ids", Ids: []string{"object", "db.schema", "db.schema.object"}, @@ -305,7 +300,7 @@ func Test_ContainsIdentifierIgnoreQuotes(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - assert.Equal(t, tc.ShouldContain, ContainsIdentifierIgnoreQuotes(tc.Ids, tc.Id)) + assert.Equal(t, tc.ShouldContain, ContainsIdentifierIgnoringQuotes(tc.Ids, tc.Id)) }) } } diff --git a/pkg/resources/diff_suppressions.go b/pkg/resources/diff_suppressions.go index dc8ea75fc3..36d3e391fb 100644 --- a/pkg/resources/diff_suppressions.go +++ b/pkg/resources/diff_suppressions.go @@ -123,8 +123,8 @@ func IgnoreValuesFromSetIfParamSet(key, param string, values []string) schema.Sc // + DATABASE.SCHEMA.OBJECT // where both identifiers are pointing to the same object, but have different structure. When a diff occurs in the // list or set, we have to handle two suppressions (one that prevents adding and one that prevents the removal). -// It's handled by the two statements with the help of helpers.ContainsIdentifierIgnoreQuotes and by getting -// the current state of ids to compare against. The dff suppressions for lists and sets are running for each element one by one +// It's handled by the two statements with the help of helpers.ContainsIdentifierIgnoringQuotes and by getting +// the current state of ids to compare against. The diff suppressions for lists and sets are running for each element one by one, // and the first diff is usually .# referring to the collection length (we skip those). func NormalizeAndCompareIdentifiersInSet(key string) schema.SchemaDiffSuppressFunc { return func(k, oldValue, newValue string, d *schema.ResourceData) bool { @@ -133,13 +133,13 @@ func NormalizeAndCompareIdentifiersInSet(key string) schema.SchemaDiffSuppressFu } if oldValue == "" && !d.GetRawState().IsNull() { - if helpers.ContainsIdentifierIgnoreQuotes(ctyValToSliceString(d.GetRawState().AsValueMap()[key].AsValueSet().Values()), newValue) { + if helpers.ContainsIdentifierIgnoringQuotes(ctyValToSliceString(d.GetRawState().AsValueMap()[key].AsValueSet().Values()), newValue) { return true } } if newValue == "" { - if helpers.ContainsIdentifierIgnoreQuotes(expandStringList(d.Get(key).(*schema.Set).List()), oldValue) { + if helpers.ContainsIdentifierIgnoringQuotes(expandStringList(d.Get(key).(*schema.Set).List()), oldValue) { return true } } diff --git a/pkg/resources/diff_suppressions_test.go b/pkg/resources/diff_suppressions_test.go index e67a8a0555..f7ff8cc131 100644 --- a/pkg/resources/diff_suppressions_test.go +++ b/pkg/resources/diff_suppressions_test.go @@ -104,49 +104,49 @@ func Test_NormalizeAndCompareIdentifiersSet(t *testing.T) { t.Run("validation: case mismatch", func(t *testing.T) { resourceData := rawDataWithValues([]any{"SCHEMA.OBJECT.IDENTIFIER"}) assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below // assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) t.Run("validation: case mismatch quoted identifier in the state", func(t *testing.T) { resourceData := rawDataWithValues([]any{`"SCHEMA"."OBJECT"."IDENTIFIER"`}) assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below // assert.False(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) - t.Run(`change detected from schema.object.identifier to "schema"."object"."identifier" with schema.object.identifier in state`, func(t *testing.T) { + t.Run(`change suppressed from schema.object.identifier to "schema"."object"."identifier" with schema.object.identifier in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{"schema.object.identifier"}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) - t.Run(`change detected from schema.object.identifier to "schema"."object"."identifier" with "schema"."object"."identifier" in state`, func(t *testing.T) { + t.Run(`change suppressed from schema.object.identifier to "schema"."object"."identifier" with "schema"."object"."identifier" in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) }) - t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with schema.object.identifier in state`, func(t *testing.T) { + t.Run(`change suppressed from "schema"."object"."identifier" to schema.object.identifier with schema.object.identifier in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{"schema.object.identifier"}) - assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", `"schema"."object"."identifier"`, "", resourceData)) + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", "schema.object.identifier", resourceData)) }) - t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state`, func(t *testing.T) { + t.Run(`change suppressed from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{`"schema"."object"."identifier"`}) - assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "schema.object.identifier", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"schema"."object"."identifier"`, resourceData)) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", `"schema"."object"."identifier"`, "", resourceData)) + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", "schema.object.identifier`, resourceData)) }) - t.Run(`change detected from "schema"."object"."identifier" to schema.object.identifier with "schema"."object"."identifier" in state uppercased`, func(t *testing.T) { + t.Run(`change suppressed from "SCHEMA"."OBJECT"."IDENTIFIER" to SCHEMA.OBJECT.IDENTIFIER with "SCHEMA"."OBJECT"."IDENTIFIER" in state`, func(t *testing.T) { resourceData := rawDataWithValues([]any{`"SCHEMA"."OBJECT"."IDENTIFIER"`}) - assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "SCHEMA.OBJECT.IDENTIFIER", "", resourceData)) - // TODO: Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below - // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", `"SCHEMA"."OBJECT"."IDENTIFIER"`, resourceData)) + assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", `"SCHEMA"."OBJECT"."IDENTIFIER"`, "", resourceData)) + // TODO(SNOW-1511594): Cannot be tested with schema.TestResourceDataRaw because it doesn't populate raw state which is used in the cases below + // assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", "SCHEMA.OBJECT.IDENTIFIER", resourceData)) }) } diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index 0c264e5fe4..5d84e96dd2 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -160,7 +160,7 @@ func CreateContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, met return ReadContextNetworkPolicy(ctx, d, meta) } -func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func ReadContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index 6f568dba60..638b58dfbd 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -423,7 +423,7 @@ func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { { ExternalProviders: map[string]resource.ExternalProvider{ "snowflake": { - VersionConstraint: "=0.92.0", + VersionConstraint: "=0.93.0", Source: "Snowflake-Labs/snowflake", }, }, @@ -437,13 +437,19 @@ func TestAcc_NetworkPolicy_Issue2236(t *testing.T) { }, Config: networkPolicyConfigWithNetworkRules( id.Name(), - []string{fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId.DatabaseName(), allowedNetworkRuleId.SchemaName(), allowedNetworkRuleId.Name())}, - []string{fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId.DatabaseName(), blockedNetworkRuleId.SchemaName(), blockedNetworkRuleId.Name())}, + []string{ + fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId.DatabaseName(), allowedNetworkRuleId.SchemaName(), allowedNetworkRuleId.Name()), + fmt.Sprintf("\"%s\".\"%s\".%s", allowedNetworkRuleId2.DatabaseName(), allowedNetworkRuleId2.SchemaName(), allowedNetworkRuleId2.Name()), + }, + []string{ + fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId.DatabaseName(), blockedNetworkRuleId.SchemaName(), blockedNetworkRuleId.Name()), + fmt.Sprintf("\"%s\".\"%s\".%s", blockedNetworkRuleId2.DatabaseName(), blockedNetworkRuleId2.SchemaName(), blockedNetworkRuleId2.Name()), + }, ), Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr("snowflake_network_policy.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "1"), - resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "1"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "allowed_network_rule_list.#", "2"), + resource.TestCheckResourceAttr("snowflake_network_policy.test", "blocked_network_rule_list.#", "2"), ), }, { From 40290ec7cc89c3cb4ee4a95078ab9308bca0fe17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Mon, 22 Jul 2024 14:56:29 +0200 Subject: [PATCH 7/7] Merge and resolve conflicts --- pkg/acceptance/helpers/network_rule_client.go | 16 +++++++++++----- pkg/acceptance/helpers/test_client.go | 5 ++--- pkg/datasources/streamlits_acceptance_test.go | 2 +- pkg/helpers/helpers_test.go | 2 -- pkg/resources/streamlit_acceptance_test.go | 4 ++-- pkg/sdk/network_rule_def.go | 2 +- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkg/acceptance/helpers/network_rule_client.go b/pkg/acceptance/helpers/network_rule_client.go index dcfa0c97c8..c7632104e8 100644 --- a/pkg/acceptance/helpers/network_rule_client.go +++ b/pkg/acceptance/helpers/network_rule_client.go @@ -24,16 +24,22 @@ func (c *NetworkRuleClient) client() sdk.NetworkRules { return c.context.client.NetworkRules } -func (c *NetworkRuleClient) CreateNetworkRule(t *testing.T) (*sdk.NetworkRule, func()) { +func (c *NetworkRuleClient) Create(t *testing.T) (*sdk.NetworkRule, func()) { t.Helper() - return c.CreateNetworkRuleWithRequest(t, sdk.NewCreateNetworkRuleRequest(c.ids.RandomSchemaObjectIdentifier(), + return c.CreateWithIdentifier(t, c.ids.RandomSchemaObjectIdentifier()) +} + +func (c *NetworkRuleClient) CreateWithIdentifier(t *testing.T, id sdk.SchemaObjectIdentifier) (*sdk.NetworkRule, func()) { + t.Helper() + return c.CreateWithRequest(t, sdk.NewCreateNetworkRuleRequest( + id, sdk.NetworkRuleTypeHostPort, []sdk.NetworkRuleValue{}, sdk.NetworkRuleModeEgress, )) } -func (c *NetworkRuleClient) CreateNetworkRuleWithRequest(t *testing.T, request *sdk.CreateNetworkRuleRequest) (*sdk.NetworkRule, func()) { +func (c *NetworkRuleClient) CreateWithRequest(t *testing.T, request *sdk.CreateNetworkRuleRequest) (*sdk.NetworkRule, func()) { t.Helper() ctx := context.Background() @@ -43,10 +49,10 @@ func (c *NetworkRuleClient) CreateNetworkRuleWithRequest(t *testing.T, request * networkRule, err := c.client().ShowByID(ctx, request.GetName()) require.NoError(t, err) - return networkRule, c.DropNetworkRuleFunc(t, request.GetName()) + return networkRule, c.DropFunc(t, request.GetName()) } -func (c *NetworkRuleClient) DropNetworkRuleFunc(t *testing.T, id sdk.SchemaObjectIdentifier) func() { +func (c *NetworkRuleClient) DropFunc(t *testing.T, id sdk.SchemaObjectIdentifier) func() { t.Helper() ctx := context.Background() diff --git a/pkg/acceptance/helpers/test_client.go b/pkg/acceptance/helpers/test_client.go index b74d697402..ad1b2a982b 100644 --- a/pkg/acceptance/helpers/test_client.go +++ b/pkg/acceptance/helpers/test_client.go @@ -27,7 +27,7 @@ type TestClient struct { MaskingPolicy *MaskingPolicyClient MaterializedView *MaterializedViewClient NetworkPolicy *NetworkPolicyClient - NetworkRule *NetworkRuleClient + NetworkRule *NetworkRuleClient Parameter *ParameterClient PasswordPolicy *PasswordPolicyClient Pipe *PipeClient @@ -79,9 +79,8 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri FileFormat: NewFileFormatClient(context, idsGenerator), MaskingPolicy: NewMaskingPolicyClient(context, idsGenerator), MaterializedView: NewMaterializedViewClient(context, idsGenerator), - NetworkRule: NewNetworkRuleClient(context, idsGenerator), NetworkPolicy: NewNetworkPolicyClient(context, idsGenerator), - NetworkRule: NewNetworkRuleClient(context, idsGenerator), + NetworkRule: NewNetworkRuleClient(context, idsGenerator), Parameter: NewParameterClient(context), PasswordPolicy: NewPasswordPolicyClient(context, idsGenerator), Pipe: NewPipeClient(context, idsGenerator), diff --git a/pkg/datasources/streamlits_acceptance_test.go b/pkg/datasources/streamlits_acceptance_test.go index 2149dd5c28..37070e2e89 100644 --- a/pkg/datasources/streamlits_acceptance_test.go +++ b/pkg/datasources/streamlits_acceptance_test.go @@ -33,7 +33,7 @@ func TestAcc_Streamlits(t *testing.T) { // TODO(SNOW-1541938): use a default warehouse after fix on snowflake side warehouse, warehouseCleanup := acc.TestClient().Warehouse.CreateWarehouse(t) t.Cleanup(warehouseCleanup) - networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.CreateNetworkRule(t) + networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.Create(t) t.Cleanup(networkRuleCleanup) externalAccessIntegrationId, externalAccessIntegrationCleanup := acc.TestClient().ExternalAccessIntegration.CreateExternalAccessIntegration(t, networkRule.ID()) t.Cleanup(externalAccessIntegrationCleanup) diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index 99ffbd0bde..fac9ae1858 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -4,8 +4,6 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/assert" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/resources/streamlit_acceptance_test.go b/pkg/resources/streamlit_acceptance_test.go index b634513a6b..83194703c1 100644 --- a/pkg/resources/streamlit_acceptance_test.go +++ b/pkg/resources/streamlit_acceptance_test.go @@ -35,7 +35,7 @@ func TestAcc_Streamlit_basic(t *testing.T) { // TODO(SNOW-1541938): use a default warehouse after fix on snowflake side warehouse, warehouseCleanup := acc.TestClient().Warehouse.CreateWarehouse(t) t.Cleanup(warehouseCleanup) - networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.CreateNetworkRule(t) + networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.Create(t) t.Cleanup(networkRuleCleanup) externalAccessIntegrationId, externalAccessIntegrationCleanup := acc.TestClient().ExternalAccessIntegration.CreateExternalAccessIntegration(t, networkRule.ID()) t.Cleanup(externalAccessIntegrationCleanup) @@ -264,7 +264,7 @@ func TestAcc_Streamlit_complete(t *testing.T) { // TODO(SNOW-1541938): use a default warehouse after fix on snowflake side warehouse, warehouseCleanup := acc.TestClient().Warehouse.CreateWarehouse(t) t.Cleanup(warehouseCleanup) - networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.CreateNetworkRule(t) + networkRule, networkRuleCleanup := acc.TestClient().NetworkRule.Create(t) t.Cleanup(networkRuleCleanup) externalAccessIntegrationId, externalAccessIntegrationCleanup := acc.TestClient().ExternalAccessIntegration.CreateExternalAccessIntegration(t, networkRule.ID()) t.Cleanup(externalAccessIntegrationCleanup) diff --git a/pkg/sdk/network_rule_def.go b/pkg/sdk/network_rule_def.go index 0a90f495c0..6e5bf918e7 100644 --- a/pkg/sdk/network_rule_def.go +++ b/pkg/sdk/network_rule_def.go @@ -28,7 +28,7 @@ var NetworkRuleDef = g.NewInterface( ). CreateOperation( "https://docs.snowflake.com/en/sql-reference/sql/create-network-rule", - g.NewQueryStruct("CreateNetworkRule"). + g.NewQueryStruct("Create"). Create(). OrReplace(). SQL("NETWORK RULE").