Skip to content

Commit

Permalink
Merge branch 'main' into feature/2385/file-format-bug
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-asawicki authored Feb 7, 2024
2 parents 6de109b + 5865655 commit ca0cd13
Show file tree
Hide file tree
Showing 56 changed files with 894 additions and 1,485 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/team-jira-issues.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ jobs:
"labels": ["${{ secrets.JIRA_LABEL }}"],
"customfield_11401": {"id": "${{ secrets.JIRA_AREA }}"},
"customfield_10008": "${{ secrets.JIRA_EPIC }}",
"assignee": {"id": "${{ secrets.JIRA_ASSIGNEE }}"}
"assignee": {"id": "${{ secrets.JIRA_ASSIGNEE }}"},
"reporter": {"id": "${{ secrets.JIRA_ASSIGNEE }}"}
}
}
}'
22 changes: 20 additions & 2 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
# Migration guide

This document is meant to help you migrate your Terraform config to new newest version. In migration guides we will only
describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behaviour
This document is meant to help you migrate your Terraform config to the new newest version. In migration guides, we will only
describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior
across different versions.

## v0.85.0 ➞ v0.86.0
### snowflake_table_constraint resource changes

#### *(behavior change)* NOT NULL removed from possible types
The `type` of the constraint was limited back to `UNIQUE`, `PRIMARY KEY`, and `FOREIGN KEY`.
The reason for that is, that syntax for Out-of-Line constraint ([docs](https://docs.snowflake.com/en/sql-reference/sql/create-table-constraint#out-of-line-unique-primary-foreign-key)) does not contain `NOT NULL`.
It is noted as a behavior change but in some way it is not; with the previous implementation it did not work at all with `type` set to `NOT NULL` because the generated statement was not a valid Snowflake statement.

We will consider adding `NOT NULL` back because it can be set by `ALTER COLUMN columnX SET NOT NULL`, but first we want to revisit the whole resource design.

## vX.XX.X -> v0.85.0

### Migration from old (grant) resources to new ones

In recent changes, we introduced a new grant resources to replace the old ones.
To aid with the migration, we wrote a guide to show one of the possible ways to migrate deprecated resources to their new counter-parts.
As the guide is more general and applies to every version (and provider), we moved it [here](./docs/technical-documentation/resource_migration.md).

## v0.84.0 ➞ v0.85.0

### snowflake_notification_integration resource changes
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ test: test-client ## run unit and integration tests
go test -v -cover -timeout=30m ./...

test-acceptance: ## run acceptance tests
TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=30m ./...
TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=45m ./...

test-integration: ## run SDK integration tests
go test -run "^TestInt_" -v -cover -timeout=20m ./...
go test -run "^TestInt_" -v -cover -timeout=30m ./...

test-architecture: ## check architecture constraints between packages
go test ./pkg/architests/... -v
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Snowflake Terraform Provider

> ⚠️ **Please note**: If you believe you have found a security issue, _please responsibly disclose_ by contacting us at [team-cloud-foundation-tools-[email protected]](mailto:team-cloud-foundation-tools[email protected]).
> ⚠️ **Please note**: If you believe you have found a security issue, _please responsibly disclose_ by contacting us at [triage-terraformprovider-[email protected]](mailto:triage-terraformprovider[email protected]).
> ⚠️ **Disclaimer**: the project is still in the 0.x.x version, which means it’s still in the experimental phase (check [Go module versioning](https://go.dev/doc/modules/version-numbers#v0-number) for more details). It can be used in production but makes no stability or backward compatibility guarantees. We do not provide backward bug fixes and, therefore, always suggest using the newest version. We are providing only limited support for the provider; priorities will be assigned on a case-by-case basis.
>
Expand Down
14 changes: 7 additions & 7 deletions docs/resources/table_constraint.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,19 @@ resource "snowflake_table" "fk_t" {
resource "snowflake_table_constraint" "primary_key" {
name = "myconstraint"
type = "PRIMARY KEY"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col1"]
comment = "hello world"
}
resource "snowflake_table_constraint" "foreign_key" {
name = "myconstraintfk"
type = "FOREIGN KEY"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col2"]
foreign_key_properties {
references {
table_id = snowflake_table.fk_t.id
table_id = snowflake_table.fk_t.qualified_name
columns = ["fk_col1"]
}
}
Expand All @@ -92,7 +92,7 @@ resource "snowflake_table_constraint" "foreign_key" {
resource "snowflake_table_constraint" "unique" {
name = "unique"
type = "UNIQUE"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col3"]
comment = "hello unique"
}
Expand All @@ -105,12 +105,12 @@ resource "snowflake_table_constraint" "unique" {

- `columns` (List of String) Columns to use in constraint key
- `name` (String) Name of constraint
- `table_id` (String) Idenfifier for table to create constraint on. Must be of the form Note: format must follow: "<db_name>"."<schema_name>"."<table_name>" or "<db_name>.<schema_name>.<table_name>" or "<db_name>|<schema_name>.<table_name>" (snowflake_table.my_table.id)
- `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', 'FOREIGN KEY', or 'NOT NULL'
- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"<db_name>\".\"<schema_name>\".\"<table_name>\"" or "<db_name>.<schema_name>.<table_name>" (snowflake_table.my_table.id)
- `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY'

### Optional

- `comment` (String) Comment for the table constraint
- `comment` (String, Deprecated) Comment for the table constraint
- `deferrable` (Boolean) Whether the constraint is deferrable
- `enable` (Boolean) Specifies whether the constraint is enabled or disabled. These properties are provided for compatibility with Oracle.
- `enforced` (Boolean) Whether the constraint is enforced
Expand Down
163 changes: 163 additions & 0 deletions docs/technical-documentation/resource_migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@

# Resource migration

Here's a guide on how to migrate deprecated resources to their new counter-parts.
The migration can be done in two ways. Either you can remove old grant resources and replace them with new ones or perform
more complicated migration, but without revoking any grant (no downtime migration). We'll focus on the second one as the first approach
is pretty straight forward. As an example we'll take `snowflake_database_grant` to `snowflake_grant_privileges_to_account_role` migration with one privilege granted to two roles:

```terraform
resource "snowflake_database_grant" "old_resource" {
depends_on = [ snowflake_database.test, snowflake_role.a, snowflake_role.b ]
database_name = snowflake_database.test.name
privilege = "USAGE"
roles = [ snowflake_role.a.name, snowflake_role.b.name ]
}
```

#### 1. terraform list

First, we need to list all the grant resources that will need to be migrated.
We can do that by running the `terraform state list` command.

> **Tip:** for larger configurations, it's best to filter the results using the grep command. For example: `terraform state list | grep "snowflake_database_grant"`.
#### 2. terraform rm

Now choose which one you would to migrate next and remove it from the state, so when you remove the old resource,
no grant will be revoked. More specifically, the Terraform Delete operation won't be run for removed resource.
It will detach the resource block in your configuration from the actual Snowflake resource.
You can remove resources from the state with the `terraform state rm <resource_address>` command.
In our example, `terraform state rm snowflake_database_grant.old_resource`. After running the command, you can remove the resource from the configuration
(again, removing the state will "detach" the resource block from the Snowflake resource. That's why after removing it, the Terraform won't try to revoke USAGE from our roles).

#### 3. Two options from here

At this point, we have several options for creating new grant resources that will replace the old ones.
We will cover three options:
- Configuration + Terraform CLI
- Configuration + import block
- Generating the configuration with import block and `terraform plan -generate-config-out`

#### 3.1.1. Write a new grant resource that will be an equivalent of the older one

```terraform
resource "snowflake_grant_privileges_to_account_role" "new_resource" {
depends_on = [snowflake_database.test, snowflake_role.a, snowflake_role.b]
for_each = toset([snowflake_role.a.name, snowflake_role.b.name])
privileges = ["USAGE"]
role_name = each.key
on_account_object {
object_type = "DATABASE"
object_name = snowflake_database.test.name
}
}
```

#### 3.1.2. terraform import

Write the `terraform import` command with the ID so that the resource will be able to parse and fill the state correctly.
You can find import syntax in the documentation for a given resource, [here](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/grant_privileges_to_account_role#import)
is the one for `snowflake_grant_privileges_to_account_role`. In our case, the command will look like this:
```shell
terraform import 'snowflake_grant_privileges_to_account_role.new_resource["role_a_name"]' 'role_a_name|USAGE|false|false|OnAccountObject|DATABASE|database_name'
terraform import 'snowflake_grant_privileges_to_account_role.new_resource["role_b_name"]' 'role_b_name|USAGE|false|false|OnAccountObject|DATABASE|database_name'
```

[Hashicorp documentation reference on import command](https://developer.hashicorp.com/terraform/cli/commands/import)

#### 3.2.1 Write import block with new resource

This is similar to the first approach, but here we don't have to worry about importing each `for_each`
entry one by one. In the `locals` block, we're defining a map of resource name to ID. Then, we have
to write a new resource similar to the first approach. In the end, we have to define an import block
which will import defined IDs to a specified resource.

```terraform
locals {
migrations = {
"${snowflake_role.a.name}" = "\"${snowflake_role.a.name}\"|false|false|USAGE|OnAccountObject|DATABASE|\"${snowflake_database.test.name}\""
"${snowflake_role.b.name}" = "\"${snowflake_role.b.name}\"|false|false|USAGE|OnAccountObject|DATABASE|\"${snowflake_database.test.name}\""
}
}
resource "snowflake_grant_privileges_to_account_role" "new_resource" {
depends_on = [snowflake_database.test, snowflake_role.a, snowflake_role.b]
for_each = local.migrations
privileges = ["USAGE"]
account_role_name = "\"${each.key}\""
on_account_object {
object_type = "DATABASE"
object_name = "\"${snowflake_database.test.name}\""
}
}
import {
to = snowflake_grant_privileges_to_account_role.new_resource[each.key]
id = each.value
for_each = local.migrations
}
```

[Hashicorp documentation reference on import block](https://developer.hashicorp.com/terraform/language/import)

#### 3.2.2 Run terraform plan and apply

After running `terraform plan` you'll see if resources can be imported without any change. If that's the case
and nothing has to be adjusted, then we can perform `terraform apply` to import the state into our new grant resources.

#### 3.3.1. Write import block

Unfortunately, `for_each` cannot be used when generating with import blocks, so we have to define them one by one.
For simplicity, we'll define just one import block (the second one would look the same, only with a different role).

```terraform
import {
to = snowflake_grant_privileges_to_account_role.new_resource_role_a
id = "\"${snowflake_role.a.name}\"|false|false|USAGE|OnAccountObject|DATABASE|\"${snowflake_database.test.name}\""
}
```
[Hashicorp documentation reference on import block](https://developer.hashicorp.com/terraform/language/import)

#### 3.3.2. terraform plan -generate-config-out

After specifying the import block run the `terraform plan -generate-config-out=generated.tf` command,
which will scan your configuration files search for import blocks, and put the generated configurations inside the `generated.tf` file.

```terraform
# __generated__ by Terraform
# Please review these resources and move them into your main configuration files.
# __generated__ by Terraform
resource "snowflake_grant_privileges_to_account_role" "new_resource_role_a" {
account_role_name = "\"test_role_321123123\""
all_privileges = false
always_apply = false
always_apply_trigger = null
on_account = false
privileges = ["USAGE"]
with_grant_option = false
on_account_object {
object_name = "\"test_database_1231321\""
object_type = "DATABASE"
}
}
```

#### 3.3.3. terraform plan and apply

After running `terraform plan` you'll see if there are any changes we have to do before applying our generated configuration.
If no errors are appearing you can run `terraform apply` to import state into generated configurations.

#### 3.3.4. Limitations of Generating Configurations

Config generation may be a good solution for a few reasons, but it also comes with limitations:
- Manual review/fixing
- Half of the values could be removed because they're the same as the default values
- No `for_each` capabilities
- You cannot specify `for_each` in the import block like in the second approach which promotes incremental migration
- Generated configurations can't use `for_each` which results in much more configuration code
- No resource reference
- As you can see `account_role_name` and `object_name` are plain values, but the values most likely should be referenced by other resources' names.

[Hashicorp documentation reference on limitations of generating configurations](https://developer.hashicorp.com/terraform/language/import/generating-configuration)
8 changes: 4 additions & 4 deletions examples/resources/snowflake_table_constraint/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,19 @@ resource "snowflake_table" "fk_t" {
resource "snowflake_table_constraint" "primary_key" {
name = "myconstraint"
type = "PRIMARY KEY"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col1"]
comment = "hello world"
}

resource "snowflake_table_constraint" "foreign_key" {
name = "myconstraintfk"
type = "FOREIGN KEY"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col2"]
foreign_key_properties {
references {
table_id = snowflake_table.fk_t.id
table_id = snowflake_table.fk_t.qualified_name
columns = ["fk_col1"]
}
}
Expand All @@ -77,7 +77,7 @@ resource "snowflake_table_constraint" "foreign_key" {
resource "snowflake_table_constraint" "unique" {
name = "unique"
type = "UNIQUE"
table_id = snowflake_table.t.id
table_id = snowflake_table.t.qualified_name
columns = ["col3"]
comment = "hello unique"
}
16 changes: 10 additions & 6 deletions pkg/datasources/current_account.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package datasources

import (
"context"
"database/sql"
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

Expand Down Expand Up @@ -40,23 +41,26 @@ func CurrentAccount() *schema.Resource {
// ReadCurrentAccount read the current snowflake account information.
func ReadCurrentAccount(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
acc, err := snowflake.ReadCurrentAccount(db)
ctx := context.Background()
client := sdk.NewClientFromDB(db)

current, err := client.ContextFunctions.CurrentSessionDetails(ctx)
if err != nil {
log.Println("[DEBUG] current_account failed to decode")
d.SetId("")
return nil
}

d.SetId(fmt.Sprintf("%s.%s", acc.Account, acc.Region))
accountErr := d.Set("account", acc.Account)
d.SetId(fmt.Sprintf("%s.%s", current.Account, current.Region))
accountErr := d.Set("account", current.Account)
if accountErr != nil {
return accountErr
}
regionErr := d.Set("region", acc.Region)
regionErr := d.Set("region", current.Region)
if regionErr != nil {
return regionErr
}
url, err := acc.AccountURL()
url, err := current.AccountURL()
if err != nil {
log.Println("[DEBUG] generating snowflake url failed")
return nil
Expand Down
8 changes: 7 additions & 1 deletion pkg/datasources/current_account_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@ package datasources_test
import (
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
)

func TestAcc_CurrentAccount(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
Providers: providers(),
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Expand Down
Loading

0 comments on commit ca0cd13

Please sign in to comment.