From bd8fc5d1c9ad049ab76dbf5d689f5beb833a40ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Thu, 7 Dec 2023 12:28:46 +0100 Subject: [PATCH] fix: schemas show by id (#2238) --- pkg/resources/schema_acceptance_test.go | 135 ++++++++++++++---- pkg/resources/testdata/TestAcc_Schema/test.tf | 5 + .../testdata/TestAcc_Schema/variables.tf | 11 ++ .../testdata/TestAcc_Schema_Rename/test.tf | 5 + .../TestAcc_Schema_Rename/variables.tf | 11 ++ .../1/test.tf | 4 + .../1/variables.tf | 7 + .../2/test.tf | 13 ++ .../2/variables.tf | 11 ++ pkg/sdk/schemas.go | 4 + 10 files changed, 178 insertions(+), 28 deletions(-) create mode 100644 pkg/resources/testdata/TestAcc_Schema/test.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_Rename/test.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_Rename/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/test.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/test.tf create mode 100644 pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/variables.tf diff --git a/pkg/resources/schema_acceptance_test.go b/pkg/resources/schema_acceptance_test.go index 8ae36610bb..435561c958 100644 --- a/pkg/resources/schema_acceptance_test.go +++ b/pkg/resources/schema_acceptance_test.go @@ -1,30 +1,45 @@ package resources_test import ( + "context" + "database/sql" "fmt" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + "github.com/hashicorp/terraform-plugin-testing/terraform" "strings" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" - "github.com/hashicorp/terraform-plugin-testing/helper/acctest" + "github.com/hashicorp/terraform-plugin-testing/config" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/tfversion" ) func TestAcc_Schema(t *testing.T) { - schemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + comment := "Terraform acceptance test" - resource.ParallelTest(t, resource.TestCase{ - Providers: acc.TestAccProviders(), - PreCheck: func() { acc.TestAccPreCheck(t) }, - CheckDestroy: nil, + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckSchemaDestroy, Steps: []resource.TestStep{ { - Config: schemaConfig(schemaName, acc.TestDatabaseName), + ConfigDirectory: config.TestNameDirectory(), + ConfigVariables: map[string]config.Variable{ + "name": config.StringVariable(name), + "database": config.StringVariable(acc.TestDatabaseName), + "comment": config.StringVariable(comment), + }, Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_schema.test", "name", schemaName), + resource.TestCheckResourceAttr("snowflake_schema.test", "name", name), resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"), - checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go + resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment), + checkBool("snowflake_schema.test", "is_transient", false), checkBool("snowflake_schema.test", "is_managed", false), ), }, @@ -32,32 +47,46 @@ func TestAcc_Schema(t *testing.T) { }) } -func TestAcc_SchemaRename(t *testing.T) { +func TestAcc_Schema_Rename(t *testing.T) { oldSchemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) newSchemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + comment := "Terraform acceptance test" - resource.ParallelTest(t, resource.TestCase{ - Providers: acc.TestAccProviders(), - PreCheck: func() { acc.TestAccPreCheck(t) }, - CheckDestroy: nil, + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckSchemaDestroy, Steps: []resource.TestStep{ { - Config: schemaConfig(oldSchemaName, acc.TestDatabaseName), + ConfigDirectory: config.TestNameDirectory(), + ConfigVariables: map[string]config.Variable{ + "name": config.StringVariable(oldSchemaName), + "database": config.StringVariable(acc.TestDatabaseName), + "comment": config.StringVariable(comment), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_schema.test", "name", oldSchemaName), resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"), - checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go + resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment), + checkBool("snowflake_schema.test", "is_transient", false), checkBool("snowflake_schema.test", "is_managed", false), ), }, { - Config: schemaConfig(newSchemaName, acc.TestDatabaseName), + ConfigDirectory: config.TestNameDirectory(), + ConfigVariables: map[string]config.Variable{ + "name": config.StringVariable(newSchemaName), + "database": config.StringVariable(acc.TestDatabaseName), + "comment": config.StringVariable(comment), + }, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_schema.test", "name", newSchemaName), resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"), - checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go + resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment), + checkBool("snowflake_schema.test", "is_transient", false), checkBool("snowflake_schema.test", "is_managed", false), ), }, @@ -65,12 +94,62 @@ func TestAcc_SchemaRename(t *testing.T) { }) } -func schemaConfig(schemaName string, databaseName string) string { - return fmt.Sprintf(` -resource "snowflake_schema" "test" { - name = "%v" - database = "%s" - comment = "Terraform acceptance test" +// TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases proves /~https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2209 issue. +func TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases(t *testing.T) { + name := "test_schema" + // It seems like Snowflake orders the output of SHOW command based on names, so they do matter + newDatabaseName := "SELDQBXEKC" + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckSchemaDestroy, + Steps: []resource.TestStep{ + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: map[string]config.Variable{ + "name": config.StringVariable(name), + "database": config.StringVariable(acc.TestDatabaseName), + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_schema.test", "name", name), + resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName), + ), + }, + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: map[string]config.Variable{ + "name": config.StringVariable(name), + "database": config.StringVariable(acc.TestDatabaseName), + "new_database": config.StringVariable(newDatabaseName), + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_schema.test", "name", name), + resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr("snowflake_schema.test_2", "name", name), + resource.TestCheckResourceAttr("snowflake_schema.test_2", "database", newDatabaseName), + ), + }, + }, + }) } -`, schemaName, databaseName) + +func testAccCheckSchemaDestroy(s *terraform.State) error { + db := acc.TestAccProvider.Meta().(*sql.DB) + client := sdk.NewClientFromDB(db) + for _, rs := range s.RootModule().Resources { + if rs.Type != "snowflake_schema" { + continue + } + ctx := context.Background() + id := sdk.NewDatabaseObjectIdentifier(rs.Primary.Attributes["database"], rs.Primary.Attributes["name"]) + schema, err := client.Schemas.ShowByID(ctx, id) + if err == nil { + return fmt.Errorf("schema %v still exists", schema.Name) + } + } + return nil } diff --git a/pkg/resources/testdata/TestAcc_Schema/test.tf b/pkg/resources/testdata/TestAcc_Schema/test.tf new file mode 100644 index 0000000000..5f8ca0abfc --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema/test.tf @@ -0,0 +1,5 @@ +resource "snowflake_schema" "test" { + name = var.name + database = var.database + comment = var.comment +} \ No newline at end of file diff --git a/pkg/resources/testdata/TestAcc_Schema/variables.tf b/pkg/resources/testdata/TestAcc_Schema/variables.tf new file mode 100644 index 0000000000..0c8231993a --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema/variables.tf @@ -0,0 +1,11 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "comment" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_Schema_Rename/test.tf b/pkg/resources/testdata/TestAcc_Schema_Rename/test.tf new file mode 100644 index 0000000000..5f8ca0abfc --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_Rename/test.tf @@ -0,0 +1,5 @@ +resource "snowflake_schema" "test" { + name = var.name + database = var.database + comment = var.comment +} \ No newline at end of file diff --git a/pkg/resources/testdata/TestAcc_Schema_Rename/variables.tf b/pkg/resources/testdata/TestAcc_Schema_Rename/variables.tf new file mode 100644 index 0000000000..0c8231993a --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_Rename/variables.tf @@ -0,0 +1,11 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "comment" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/test.tf b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/test.tf new file mode 100644 index 0000000000..991e3b38d5 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/test.tf @@ -0,0 +1,4 @@ +resource "snowflake_schema" "test" { + name = var.name + database = var.database +} \ No newline at end of file diff --git a/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/variables.tf b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/variables.tf new file mode 100644 index 0000000000..2cf13b7d6e --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/variables.tf @@ -0,0 +1,7 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/test.tf b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/test.tf new file mode 100644 index 0000000000..bc1af37dd8 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/test.tf @@ -0,0 +1,13 @@ +resource "snowflake_schema" "test" { + name = var.name + database = var.database +} + +resource "snowflake_database" "test" { + name = var.new_database +} + +resource "snowflake_schema" "test_2" { + name = var.name + database = snowflake_database.test.name +} diff --git a/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/variables.tf b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/variables.tf new file mode 100644 index 0000000000..f62a0992c8 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/variables.tf @@ -0,0 +1,11 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "new_database" { + type = string +} diff --git a/pkg/sdk/schemas.go b/pkg/sdk/schemas.go index 8e99c6dc61..57fbf1c18d 100644 --- a/pkg/sdk/schemas.go +++ b/pkg/sdk/schemas.go @@ -389,6 +389,10 @@ func (v *schemas) Show(ctx context.Context, opts *ShowSchemaOptions) ([]Schema, func (v *schemas) ShowByID(ctx context.Context, id DatabaseObjectIdentifier) (*Schema, error) { schemas, err := v.client.Schemas.Show(ctx, &ShowSchemaOptions{ + In: &SchemaIn{ + Database: Bool(true), + Name: NewAccountObjectIdentifier(id.DatabaseName()), + }, Like: &Like{ Pattern: String(id.Name()), },