Skip to content

Commit

Permalink
chore: Update documentation (#2931)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
sfc-gh-jcieslak authored Jul 12, 2024
1 parent 32c7690 commit da98bc3
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 50 deletions.
58 changes: 58 additions & 0 deletions CREATING_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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).
9 changes: 8 additions & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -43,7 +51,6 @@ provider "snowflake" {
}
}
provider "snowflake" {
profile = "securityadmin"
}
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/table.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
4 changes: 2 additions & 2 deletions docs/technical-documentation/resource_migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 8 additions & 1 deletion examples/provider/provider.tf
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -23,7 +31,6 @@ provider "snowflake" {
}
}


provider "snowflake" {
profile = "securityadmin"
}
44 changes: 23 additions & 21 deletions pkg/resources/saml2_integration_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
),
Expand All @@ -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),
},
},
Expand All @@ -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"),
),
Expand All @@ -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"),
),
Expand Down Expand Up @@ -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"),
Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion pkg/resources/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Loading

0 comments on commit da98bc3

Please sign in to comment.