Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: database tests and introduce a new parameter #2981

Merged
merged 4 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,9 @@ resource "snowflake_database" "test" {
}
```

If you had `from_database` set, it should migrate automatically.
If you had `from_database` set, you should follow our [resource migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md) to remove
the database from state to later import it in the newer version of the provider.
Otherwise, it may cause issues when migrating to v0.93.0.
For now, we're dropping the possibility to create a clone database from other databases.
The only way will be to clone a database manually and import it as `snowflake_database`, but if
cloned databases diverge in behavior from standard databases, it may cause issues.
Expand Down
1 change: 1 addition & 0 deletions docs/resources/database.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ resource "snowflake_database" "primary" {
- `comment` (String) Specifies a comment for the database.
- `data_retention_time_in_days` (Number) Specifies the number of days for which Time Travel actions (CLONE and UNDROP) can be performed on the database, as well as specifying the default Time Travel retention time for all schemas created in the database. For more details, see [Understanding & Using Time Travel](https://docs.snowflake.com/en/user-guide/data-time-travel).
- `default_ddl_collation` (String) Specifies a default collation specification for all schemas and tables added to the database. It can be overridden on schema or table level. For more information, see [collation specification](https://docs.snowflake.com/en/sql-reference/collation#label-collation-specification).
- `drop_public_schema_on_creation` (Boolean) Specifies whether to drop public schema on creation or not. Modifying the parameter after database is already created won't have any effect.
- `enable_console_output` (Boolean) If true, enables stdout/stderr fast path logging for anonymous stored procedures.
- `external_volume` (String) The database parameter that specifies the default external volume to use for Iceberg tables. For more information, see [EXTERNAL_VOLUME](https://docs.snowflake.com/en/sql-reference/parameters#external-volume).
- `is_transient` (Boolean) Specifies the database as transient. Transient databases do not have a Fail-safe period so they do not incur additional storage costs once they leave Time Travel; however, this means they are also not protected by Fail-safe in the event of a data loss.
Expand Down
10 changes: 10 additions & 0 deletions pkg/acceptance/helpers/database_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,13 @@ func (c *DatabaseClient) Show(t *testing.T, id sdk.AccountObjectIdentifier) (*sd

return c.client().ShowByID(ctx, id)
}

func (c *DatabaseClient) Describe(t *testing.T, id sdk.AccountObjectIdentifier) *sdk.DatabaseDetails {
t.Helper()
ctx := context.Background()

details, err := c.client().Describe(ctx, id)
require.NoError(t, err)

return details
}
21 changes: 21 additions & 0 deletions pkg/acceptance/snowflakechecks/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package snowflakechecks
import (
"errors"
"fmt"
"slices"
"testing"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
Expand All @@ -26,3 +27,23 @@ func CheckDatabaseDataRetentionTimeInDays(t *testing.T, databaseId sdk.AccountOb
return errors.Join(errs...)
}
}

func DoesNotContainPublicSchema(t *testing.T, id sdk.AccountObjectIdentifier) resource.TestCheckFunc {
t.Helper()
return func(state *terraform.State) error {
if slices.ContainsFunc(acc.TestClient().Database.Describe(t, id).Rows, func(row sdk.DatabaseDetailsRow) bool { return row.Name == "PUBLIC" && row.Kind == "SCHEMA" }) {
return fmt.Errorf("expected database %s to not contain public schema", id.FullyQualifiedName())
}
return nil
}
}

func ContainsPublicSchema(t *testing.T, id sdk.AccountObjectIdentifier) resource.TestCheckFunc {
t.Helper()
return func(state *terraform.State) error {
if !slices.ContainsFunc(acc.TestClient().Database.Describe(t, id).Rows, func(row sdk.DatabaseDetailsRow) bool { return row.Name == "PUBLIC" && row.Kind == "SCHEMA" }) {
return fmt.Errorf("expected database %s to contain public schema", id.FullyQualifiedName())
}
return nil
}
}
27 changes: 27 additions & 0 deletions pkg/resources/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"fmt"
"slices"
"strings"
"time"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/util"

"github.com/hashicorp/go-cty/cty"

Expand All @@ -22,6 +25,12 @@ var databaseSchema = map[string]*schema.Schema{
Required: true,
Description: "Specifies the identifier for the database; must be unique for your account. As a best practice for [Database Replication and Failover](https://docs.snowflake.com/en/user-guide/db-replication-intro), it is recommended to give each secondary database the same name as its primary database. This practice supports referencing fully-qualified objects (i.e. '<db>.<schema>.<object>') by other objects in the same database, such as querying a fully-qualified table name in a view. If a secondary database has a different name from the primary database, then these object references would break in the secondary database.",
},
"drop_public_schema_on_creation": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use DiffSuppressFunc: IgnoreAfterCreation,

Copy link
Collaborator Author

@sfc-gh-jcieslak sfc-gh-jcieslak Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, already used it after Kuba's comment

Type: schema.TypeBool,
Optional: true,
Description: "Specifies whether to drop public schema on creation or not. Modifying the parameter after database is already created won't have any effect.",
DiffSuppressFunc: IgnoreAfterCreation,
},
"is_transient": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -124,6 +133,24 @@ func CreateDatabase(ctx context.Context, d *schema.ResourceData, meta any) diag.

var diags diag.Diagnostics

if d.Get("drop_public_schema_on_creation").(bool) {
var dropSchemaErrs []error
err := util.Retry(3, time.Second, func() (error, bool) {
if err := client.Schemas.Drop(ctx, sdk.NewDatabaseObjectIdentifier(id.Name(), "PUBLIC"), &sdk.DropSchemaOptions{IfExists: sdk.Bool(true)}); err != nil {
dropSchemaErrs = append(dropSchemaErrs, err)
return nil, false
}
return nil, true
})
if err != nil {
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Failed to drop public schema on creation (failed after 3 attempts)",
Detail: fmt.Sprintf("The '%s' database was created successfully, but the provider was not able to remove public schema on creation. Please drop the public schema manually. Original errors: %s", id.Name(), errors.Join(dropSchemaErrs...)),
})
}
}

if v, ok := d.GetOk("replication"); ok {
replicationConfiguration := v.([]any)[0].(map[string]any)

Expand Down
218 changes: 32 additions & 186 deletions pkg/resources/database_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package resources_test

import (
"fmt"
"regexp"
"strconv"
"testing"

Expand All @@ -15,7 +14,6 @@ import (
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/planchecks"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/snowflakechecks"
"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/config"
Expand Down Expand Up @@ -1160,233 +1158,81 @@ resource "snowflake_database" "test" {
`, id.Name(), strconv.Quote(enableToAccount))
}

func TestAcc_Database_UpgradeFromShare(t *testing.T) {
_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)

func TestAcc_Database_WithoutPublicSchema(t *testing.T) {
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
secondaryClientLocator := acc.SecondaryClient(t).GetAccountLocator()

shareExternalId := createShareableDatabase(t)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDestroy(t, resources.Database),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
VersionConstraint: "=0.92.0",
Source: "Snowflake-Labs/snowflake",
},
},
Config: databaseStateUpgraderFromShareOld(id, secondaryClientLocator, shareExternalId),
Config: databaseWithDropPublicSchemaConfig(id, true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_database.test", "id", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "name", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "from_share.provider", secondaryClientLocator),
resource.TestCheckResourceAttr("snowflake_database.test", "from_share.share", shareExternalId.Name()),
snowflakechecks.DoesNotContainPublicSchema(t, id),
),
},
// Change in parameter shouldn't change the state Snowflake
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromShareNewAfterUpgrade(id),
ExpectError: regexp.MustCompile("failed to upgrade the state with database created from share, please use snowflake_shared_database or deprecated snowflake_database_old instead"),
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromShareNew(id, shareExternalId),
ResourceName: "snowflake_shared_database.test",
ImportStateId: id.FullyQualifiedName(),
ImportState: true,
},
},
})
}

func databaseStateUpgraderFromShareOld(id sdk.AccountObjectIdentifier, secondaryClientLocator string, externalShare sdk.ExternalObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0 # to avoid in-place update to -1
from_share = {
provider = "%s"
share = "%s"
}
}
`, id.Name(), secondaryClientLocator, externalShare.Name())
}

func databaseStateUpgraderFromShareNewAfterUpgrade(id sdk.AccountObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0 # to avoid in-place update to -1
}
`, id.Name())
}

func databaseStateUpgraderFromShareNew(id sdk.AccountObjectIdentifier, externalShare sdk.ExternalObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_shared_database" "test" {
name = "%s"
from_share = %s
}
`, id.Name(), strconv.Quote(externalShare.FullyQualifiedName()))
}

func TestAcc_Database_UpgradeFromReplica(t *testing.T) {
_ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance)

id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
_, primaryDatabaseId, databaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{
acc.TestClient().Account.GetAccountIdentifier(t),
})
t.Cleanup(databaseCleanup)

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
VersionConstraint: "=0.92.0",
Source: "Snowflake-Labs/snowflake",
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("snowflake_database.test", plancheck.ResourceActionNoop),
},
},
Config: databaseStateUpgraderFromReplicaOld(id, primaryDatabaseId),
Config: databaseWithDropPublicSchemaConfig(id, false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_database.test", "id", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "name", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "from_replica", primaryDatabaseId.FullyQualifiedName()),
snowflakechecks.DoesNotContainPublicSchema(t, id),
),
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromReplicaNewAfterUpgrade(id),
ExpectError: regexp.MustCompile("failed to upgrade the state with database created from replica, please use snowflake_secondary_database or deprecated snowflake_database_old instead"),
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromReplicaNew(id, primaryDatabaseId),
ResourceName: "snowflake_secondary_database.test",
ImportStateId: id.FullyQualifiedName(),
ImportState: true,
},
},
})
}

func databaseStateUpgraderFromReplicaOld(id sdk.AccountObjectIdentifier, primaryDatabaseId sdk.ExternalObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0 # to avoid in-place update to -1
from_replica = %s
}
`, id.Name(), strconv.Quote(primaryDatabaseId.FullyQualifiedName()))
}

func databaseStateUpgraderFromReplicaNewAfterUpgrade(id sdk.AccountObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0
}
`, id.Name())
}

func databaseStateUpgraderFromReplicaNew(id sdk.AccountObjectIdentifier, primaryDatabaseId sdk.ExternalObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_secondary_database" "test" {
name = "%s"
as_replica_of = %s
}
`, id.Name(), strconv.Quote(id.FullyQualifiedName()))
}

func TestAcc_Database_UpgradeFromClonedDatabase(t *testing.T) {
func TestAcc_Database_WithPublicSchema(t *testing.T) {
id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
cloneId := acc.TestClient().Ids.RandomAccountObjectIdentifier()

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDestroy(t, resources.Database),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
VersionConstraint: "=0.92.0",
Source: "Snowflake-Labs/snowflake",
},
},
Config: databaseStateUpgraderFromDatabaseOld(id, cloneId),
Config: databaseWithDropPublicSchemaConfig(id, false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_database.cloned", "id", cloneId.Name()),
resource.TestCheckResourceAttr("snowflake_database.cloned", "name", cloneId.Name()),
resource.TestCheckResourceAttr("snowflake_database.cloned", "from_database", id.Name()),
resource.TestCheckResourceAttr("snowflake_database.test", "id", id.Name()),
snowflakechecks.ContainsPublicSchema(t, id),
),
},
// Change in parameter shouldn't change the state Snowflake
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromDatabaseNewAfterUpgrade(id, cloneId),
ExpectError: regexp.MustCompile("failed to upgrade the state with database created from database, please use snowflake_database or deprecated snowflake_database_old instead"),
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: databaseStateUpgraderFromDatabaseNew(id, cloneId),
ResourceName: "snowflake_database.cloned",
ImportStateId: cloneId.FullyQualifiedName(),
ImportState: true,
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("snowflake_database.test", plancheck.ResourceActionNoop),
},
},
Config: databaseWithDropPublicSchemaConfig(id, true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_database.test", "id", id.Name()),
snowflakechecks.ContainsPublicSchema(t, id),
),
},
},
})
}

func databaseStateUpgraderFromDatabaseOld(id sdk.AccountObjectIdentifier, secondId sdk.AccountObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0 # to avoid in-place update to -1
}

resource "snowflake_database" "cloned" {
name = "%s"
data_retention_time_in_days = 0 # to avoid in-place update to -1
from_database = snowflake_database.test.name
}
`, id.Name(), secondId.Name())
}

func databaseStateUpgraderFromDatabaseNewAfterUpgrade(id sdk.AccountObjectIdentifier, secondId sdk.AccountObjectIdentifier) string {
func databaseWithDropPublicSchemaConfig(id sdk.AccountObjectIdentifier, withDropPublicSchema bool) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
data_retention_time_in_days = 0
}

resource "snowflake_database" "cloned" {
name = "%s"
data_retention_time_in_days = 0
}
`, id.Name(), secondId.Name())
}

func databaseStateUpgraderFromDatabaseNew(id sdk.AccountObjectIdentifier, secondId sdk.AccountObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_database" "test" {
name = "%s"
}

resource "snowflake_database" "cloned" {
name = "%s"
drop_public_schema_on_creation = %s
}
`, id.Name(), secondId.Name())
`, id.Name(), strconv.FormatBool(withDropPublicSchema))
}
Loading
Loading