chore: Update documentation (#2931)

## Changes Applied changes from https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2046/files. Applied changes from https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/2854/files. Addressed comment #2912 (comment). Added a note about object cloning and design decisions about them. Assigned one issue to tables (it's a general one, but mostly points to the table's implementation; it's #1823). Added two common issues to our commonly known issues section in CREATING_ISSUES.md.
Snowflake-Labs · Jul 12, 2024 · da98bc3 · da98bc3
1 parent 32c7690
commit da98bc3
Show file tree

Hide file tree

Showing 9 changed files with 137 additions and 50 deletions.
diff --git a/CREATING_ISSUES.md b/CREATING_ISSUES.md
@@ -18,6 +18,8 @@
   * [How to set up the connection with the private key?](#how-to-set-up-the-connection-with-the-private-key)
   * [Incorrect identifier (index out of bounds) (even with the old error message)](#incorrect-identifier-index-out-of-bounds-even-with-the-old-error-message)
   * [Incorrect account identifier (snowflake_database.from_share)](#incorrect-account-identifier-snowflake_databasefrom_share)
+  * [Granting on Functions or Procedures](#granting-on-functions-or-procedures)
+  * [Infinite diffs, empty privileges, errors when revoking on grant resources](#infinite-diffs-empty-privileges-errors-when-revoking-on-grant-resources)
 
 This guide was made to aid with creating the GitHub issues, so you can maximize your chances of getting help as quickly as possible. 
 To correctly report the issue, we suggest going through the following steps.
@@ -161,3 +163,59 @@ panic: interface conversion: sdk.ObjectIdentifier is sdk.AccountObjectIdentifier
 [GitHub issue reference](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2590)
 
 **Solution:** As specified in the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#behavior-change-external-object-identifier-changes), use account locator instead.
+
+### Granting on Functions or Procedures
+**Problem:** Right now, when granting any privilege on Function or Procedure with this or similar configuration:
+
+```terraform
+resource "snowflake_grant_privileges_to_account_role" "grant_on_procedure" {
+  privileges        = ["USAGE"]
+  account_role_name = snowflake_account_role.name
+  on_schema_object {
+    object_type = "PROCEDURE"
+    object_name = "\"${snowflake_database.database.name}\".\"${snowflake_schema.schema.name}\".\"${snowflake_procedure.procedure.name}\""
+  }
+}
+```
+
+You may encounter the following error:
+```text
+│ Error: 090208 (42601): Argument types of function 'procedure_name' must be
+│ specified.
+```
+
+**Related issues:** [#2375](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2375), [#2922](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2922)
+
+**Solution:** Specify the arguments in the `object_name`: 
+
+```terraform
+resource "snowflake_grant_privileges_to_account_role" "grant_on_procedure" {
+  privileges        = ["USAGE"]
+  account_role_name = snowflake_account_role.name
+  on_schema_object {
+    object_type = "PROCEDURE"
+    object_name = "\"${snowflake_database.database.name}\".\"${snowflake_schema.schema.name}\".\"${snowflake_procedure.procedure.name}(NUMBER, VARCHAR)\""
+  }
+}
+```
+
+### Infinite diffs, empty privileges, errors when revoking on grant resources
+**Problem:** If you encountered one of the following issues:
+- Issue with revoking: `Error: An error occurred when revoking privileges from an account role.
+- Plan in every `terraform plan` run (mostly empty privileges)
+It's possible that the `object_type` you specified is "incorrect."
+Let's say you would like to grant `SELECT` on event table. In Snowflake, it's possible to specify
+`TABLE` object type instead of dedicated `EVENT TABLE` one. As `object_type` is one of the fields
+we filter on, it needs to exactly match with the output provided by `SHOW GRANTS` command.
+
+**Related issues:** [#2749](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2749), [#2803](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2803)
+
+**Solution:** Here's a list of things that may help with your issue:
+- Firstly, check if the privilege has been granted in Snowflake. If it is, it means the configuration is correct (or at least compliant with Snowflake syntax).
+- When granting `IMPORTED PRIVILEGES` on `SNOWFLAKE` database/application, use `object_type = "DATABASE"`.
+- Run `SHOW GRANTS` command with the right filters to find the granted privilege and check what is the object type returned of that command. If it doesn't match the one you have in your configuration, then follow those steps:
+  - Use state manipulation (no revoking)
+    - Remove the resource from your state using `terraform state rm`.
+    - Change the `object_type` to correct value.
+    - Import the state from Snowflake using `terraform import`.
+  - Remove the grant configuration and after `terraform apply` put it back with the correct `object_type` (requires revoking).
diff --git a/docs/index.md b/docs/index.md
@@ -18,6 +18,14 @@ Coverage is focused on part of Snowflake related to access control.
 ## Example Provider Configuration
 
 ```terraform
+terraform {
+  required_providers {
+    snowflake = {
+      source = "Snowflake-Labs/snowflake"
+    }
+  }
+}
+
 provider "snowflake" {
   account                = "..." # required if not using profile. Can also be set via SNOWFLAKE_ACCOUNT env var
   username               = "..." # required if not using profile or token. Can also be set via SNOWFLAKE_USER env var
@@ -43,7 +51,6 @@ provider "snowflake" {
   }
 }
 
-
 provider "snowflake" {
   profile = "securityadmin"
 }

diff --git a/docs/resources/table.md b/docs/resources/table.md
@@ -110,7 +110,7 @@ resource "snowflake_table" "table" {
 Required:
 
 - `name` (String) Column name
-- `type` (String) Column type, e.g. VARIANT
+- `type` (String) Column type, e.g. VARIANT. For a full list of column types, see [Summary of Data Types](https://docs.snowflake.com/en/sql-reference/intro-summary-data-types).
 
 Optional:
 

diff --git a/docs/technical-documentation/resource_migration.md b/docs/technical-documentation/resource_migration.md
@@ -120,8 +120,8 @@ import {
 }
 ```
 
-If you are using terraform 1.7 or above you could use a `for_each` to import multiple resources at once. See
-[Terraform 1.7 adds test mocking and config-driven remove > Import block for_each](https://www.hashicorp.com/blog/terraform-1-7-adds-test-mocking-and-config-driven-remove).
+If you are using terraform 1.7 or above you could use a `for_each` to import multiple resources at once.
+For more details, take a look at [importing multiple instances with for_each](https://developer.hashicorp.com/terraform/language/v1.7.x/import?product_intent=terraform#import-multiple-instances-with-for_each).
 
 [Hashicorp documentation reference on import block](https://developer.hashicorp.com/terraform/language/import)
 

diff --git a/examples/provider/provider.tf b/examples/provider/provider.tf
@@ -1,3 +1,11 @@
+terraform {
+  required_providers {
+    snowflake = {
+      source = "Snowflake-Labs/snowflake"
+    }
+  }
+}
+
 provider "snowflake" {
   account                = "..." # required if not using profile. Can also be set via SNOWFLAKE_ACCOUNT env var
   username               = "..." # required if not using profile or token. Can also be set via SNOWFLAKE_USER env var
@@ -23,7 +31,6 @@ provider "snowflake" {
   }
 }
 
-
 provider "snowflake" {
   profile = "securityadmin"
 }
diff --git a/pkg/resources/saml2_integration_acceptance_test.go b/pkg/resources/saml2_integration_acceptance_test.go
@@ -7,6 +7,8 @@ import (
 	"strings"
 	"testing"
 
+	r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources"
+
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
@@ -78,17 +80,17 @@ func TestAcc_Saml2Integration_basic(t *testing.T) {
 				ConfigVariables: m(issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, false, false),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "name", id.Name()),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_issuer", issuer),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sso_url", validUrl),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_provider", string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom)),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_x509_cert", cert),
 					resource.TestCheckNoResourceAttr("snowflake_saml2_integration.test", "saml2_sp_initiated_login_page_label"),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_enable_sp_initiated", "default"),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sign_request", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_enable_sp_initiated", r.BooleanDefault),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sign_request", r.BooleanDefault),
 					resource.TestCheckNoResourceAttr("snowflake_saml2_integration.test", "saml2_requested_nameid_format"),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", ""),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckNoResourceAttr("snowflake_saml2_integration.test", "saml2_snowflake_issuer_url"),
 					resource.TestCheckNoResourceAttr("snowflake_saml2_integration.test", "saml2_snowflake_acs_url"),
 					resource.TestCheckNoResourceAttr("snowflake_saml2_integration.test", "allowed_user_domains"),
@@ -316,17 +318,17 @@ func TestAcc_Saml2Integration_basic(t *testing.T) {
 				ConfigVariables: m(issuer, string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom), validUrl, cert, false, true),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "name", id.Name()),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_issuer", issuer),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sso_url", validUrl),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_provider", string(sdk.Saml2SecurityIntegrationSaml2ProviderCustom)),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_x509_cert", cert),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sp_initiated_login_page_label", "foo"),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_enable_sp_initiated", "default"),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sign_request", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_enable_sp_initiated", r.BooleanDefault),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_sign_request", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_requested_nameid_format", ""),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", ""),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_snowflake_issuer_url", issuerURL),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_snowflake_acs_url", acsURL),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "allowed_user_domains.#", "1"),
@@ -465,12 +467,12 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) {
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction("snowflake_saml2_integration.test", plancheck.ResourceActionUpdate),
 						planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("true"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("true"), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),
 					},
 				},
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"),
 				),
@@ -480,7 +482,7 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) {
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
 						planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("default"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),
 					},
 				},
@@ -501,12 +503,12 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) {
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectNonEmptyPlan(),
 						planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("false"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("false"), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),
 					},
 				},
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"),
 				),
@@ -522,12 +524,12 @@ func TestAcc_Saml2Integration_forceAuthn(t *testing.T) {
 					PreApply: []plancheck.PlanCheck{
 						plancheck.ExpectNonEmptyPlan(),
 						planchecks.PrintPlanDetails("snowflake_saml2_integration.test", "saml2_force_authn", "describe_output"),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("true"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("true"), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),
 					},
 				},
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.#", "1"),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "describe_output.0.saml2_force_authn.0.value", "false"),
 				),
@@ -976,16 +978,16 @@ func TestAcc_Saml2Integration_DefaultValues(t *testing.T) {
 				ConfigVariables: configVariables,
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String("false"), sdk.String("default")),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("false"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String("false"), sdk.String(r.BooleanDefault)),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("false"), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String(""), sdk.String("")),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "show_output", true),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),
 					},
 				},
 				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", "default"),
-					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", "default"),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "enabled", r.BooleanDefault),
+					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_force_authn", r.BooleanDefault),
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", ""),
 
 					resource.TestCheckResourceAttr("snowflake_saml2_integration.test", "show_output.#", "1"),
@@ -1002,8 +1004,8 @@ func TestAcc_Saml2Integration_DefaultValues(t *testing.T) {
 				ConfigVariables: configVariables,
 				ConfigPlanChecks: resource.ConfigPlanChecks{
 					PreApply: []plancheck.PlanCheck{
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String("default"), sdk.String("default")),
-						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String("default"), sdk.String("default")),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "enabled", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)),
+						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_force_authn", tfjson.ActionUpdate, sdk.String(r.BooleanDefault), sdk.String(r.BooleanDefault)),
 						planchecks.ExpectChange("snowflake_saml2_integration.test", "saml2_post_logout_redirect_url", tfjson.ActionUpdate, sdk.String(""), sdk.String("")),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "show_output", true),
 						planchecks.ExpectComputed("snowflake_saml2_integration.test", "describe_output", true),

diff --git a/pkg/resources/table.go b/pkg/resources/table.go
@@ -57,7 +57,7 @@ var tableSchema = map[string]*schema.Schema{
 				"type": {
 					Type:             schema.TypeString,
 					Required:         true,
-					Description:      "Column type, e.g. VARIANT",
+					Description:      "Column type, e.g. VARIANT. For a full list of column types, see [Summary of Data Types](https://docs.snowflake.com/en/sql-reference/intro-summary-data-types).",
 					ValidateFunc:     dataTypeValidateFunc,
 					DiffSuppressFunc: dataTypeDiffSuppressFunc,
 				},