fix: Fix some bugs (#2234)

* Test alert datasource (basic) * Add more test cases * Fix bad config params combination * Fix database behavior * Add test for show by schema * Add test for empty in * Fix after review * Fix merge conflicts
Snowflake-Labs · Dec 8, 2023 · 774a7db · 774a7db
1 parent c1700de
commit 774a7db
Show file tree

Hide file tree

Showing 16 changed files with 378 additions and 54 deletions.
diff --git a/pkg/datasources/alerts.go b/pkg/datasources/alerts.go
@@ -87,24 +87,36 @@ func ReadAlerts(d *schema.ResourceData, meta interface{}) error {
 	ctx := context.Background()
 
 	d.SetId("alerts_read")
-	databaseName := d.Get("database").(string)
-	schemaName := d.Get("schema").(string)
-	alertPattern := d.Get("pattern").(string)
-	var like sdk.Like
-	if alertPattern != "" {
-		like = sdk.Like{Pattern: &alertPattern}
+
+	opts := sdk.ShowAlertOptions{}
+
+	if v, ok := d.GetOk("pattern"); ok {
+		alertPattern := v.(string)
+		opts.Like = &sdk.Like{Pattern: &alertPattern}
 	}
-	listAlerts, err := client.Alerts.Show(ctx, &sdk.ShowAlertOptions{
-		In: &sdk.In{
-			Schema: sdk.NewDatabaseObjectIdentifier(databaseName, schemaName),
-		},
-		Like: &like,
-	})
+
+	if v, ok := d.GetOk("database"); ok {
+		databaseName := v.(string)
+
+		if v, ok := d.GetOk("schema"); ok {
+			schemaName := v.(string)
+			opts.In = &sdk.In{
+				Schema: sdk.NewDatabaseObjectIdentifier(databaseName, schemaName),
+			}
+		} else {
+			opts.In = &sdk.In{
+				Database: sdk.NewAccountObjectIdentifier(databaseName),
+			}
+		}
+	}
+
+	listAlerts, err := client.Alerts.Show(ctx, &opts)
 	if err != nil {
 		log.Printf("[DEBUG] failed to list alerts in schema (%s)", d.Id())
 		d.SetId("")
 		return err
 	}
+
 	alerts := make([]map[string]any, 0, len(listAlerts))
 	for _, alert := range listAlerts {
 		alertMap := map[string]any{}

diff --git a/pkg/datasources/alerts_acceptance_test.go b/pkg/datasources/alerts_acceptance_test.go
@@ -0,0 +1,118 @@
+package datasources_test
+
+import (
+	"fmt"
+	"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/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
+)
+
+func TestAcc_Alerts(t *testing.T) {
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				Config: alertsResourceConfig(name) + alertsDatasourceConfigNoOptionals(),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("data.snowflake_alerts.test_datasource_alert", "alerts.#"),
+				),
+			},
+			{
+				Config: alertsResourceConfig(name) + alertsDatasourceConfigDbOnly(),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("data.snowflake_alerts.test_datasource_alert", "alerts.#"),
+				),
+			},
+			{
+				Config: alertsResourceConfig(name) + alertsDatasourceConfigDbAndSchema(),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("data.snowflake_alerts.test_datasource_alert", "alerts.#"),
+					resource.TestCheckResourceAttr("data.snowflake_alerts.test_datasource_alert", "alerts.0.name", name),
+				),
+			},
+			{
+				Config: alertsResourceConfig(name) + alertsDatasourceConfigAllOptionals(name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("data.snowflake_alerts.test_datasource_alert", "alerts.#"),
+					resource.TestCheckResourceAttr("data.snowflake_alerts.test_datasource_alert", "alerts.0.name", name),
+				),
+			},
+			{
+				Config: alertsResourceConfig(name) + alertsDatasourceConfigSchemaOnly(),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("data.snowflake_alerts.test_datasource_alert", "alerts.#"),
+				),
+			},
+		},
+	})
+}
+
+func alertsResourceConfig(name string) string {
+	return fmt.Sprintf(`
+resource "snowflake_alert" "test_resource_alert" {
+	name     	      = "%s"
+	database  	      = "%s"
+	schema   	      = "%s"
+	warehouse 	      = "%s"
+	condition         = "select 0 as c"
+	action            = "select 0 as c"
+	enabled  	      = false
+	comment           = "some comment"
+	alert_schedule 	  {
+		interval = "60"
+	}
+}
+`, name, acc.TestDatabaseName, acc.TestSchemaName, acc.TestWarehouseName)
+}
+
+func alertsDatasourceConfigNoOptionals() string {
+	return `
+data "snowflake_alerts" "test_datasource_alert" {}
+`
+}
+
+func alertsDatasourceConfigDbOnly() string {
+	return fmt.Sprintf(`
+data "snowflake_alerts" "test_datasource_alert" {
+	database  	      = "%s"
+}
+`, acc.TestDatabaseName)
+}
+
+func alertsDatasourceConfigDbAndSchema() string {
+	return fmt.Sprintf(`
+data "snowflake_alerts" "test_datasource_alert" {
+	database  	      = "%s"
+	schema  	      = "%s"
+}
+`, acc.TestDatabaseName, acc.TestSchemaName)
+}
+
+func alertsDatasourceConfigAllOptionals(name string) string {
+	return fmt.Sprintf(`
+data "snowflake_alerts" "test_datasource_alert" {
+	database  	      = "%s"
+	schema  	      = "%s"
+	pattern  	      = "%s"
+}
+`, acc.TestDatabaseName, acc.TestSchemaName, name)
+}
+
+func alertsDatasourceConfigSchemaOnly() string {
+	return fmt.Sprintf(`
+data "snowflake_alerts" "test_datasource_alert" {
+	schema  	      = "%s"
+}
+`, acc.TestSchemaName)
+}
diff --git a/pkg/datasources/dynamic_tables.go b/pkg/datasources/dynamic_tables.go
@@ -33,22 +33,22 @@ var dynamicTablesSchema = map[string]*schema.Schema{
 		Elem: &schema.Resource{
 			Schema: map[string]*schema.Schema{
 				"account": {
-					Type:          schema.TypeBool,
-					Optional:      true,
-					Description:   "Returns records for the entire account.",
-					ConflictsWith: []string{"in.database", "in.schema"},
+					Type:         schema.TypeBool,
+					Optional:     true,
+					Description:  "Returns records for the entire account.",
+					ExactlyOneOf: []string{"in.0.account", "in.0.database", "in.0.schema"},
 				},
 				"database": {
-					Type:          schema.TypeString,
-					Optional:      true,
-					Description:   "Returns records for the current database in use or for a specified database (db_name).",
-					ConflictsWith: []string{"in.account", "in.schema"},
+					Type:         schema.TypeString,
+					Optional:     true,
+					Description:  "Returns records for the current database in use or for a specified database (db_name).",
+					ExactlyOneOf: []string{"in.0.account", "in.0.database", "in.0.schema"},
 				},
 				"schema": {
-					Type:          schema.TypeString,
-					Optional:      true,
-					Description:   "Returns records for the current schema in use or a specified schema (schema_name).",
-					ConflictsWith: []string{"in.account", "in.database"},
+					Type:         schema.TypeString,
+					Optional:     true,
+					Description:  "Returns records for the current schema in use or a specified schema (schema_name).",
+					ExactlyOneOf: []string{"in.0.account", "in.0.database", "in.0.schema"},
 				},
 			},
 		},

diff --git a/pkg/datasources/dynamic_tables_acceptance_test.go b/pkg/datasources/dynamic_tables_acceptance_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -79,10 +80,71 @@ func TestAcc_DynamicTables_complete(t *testing.T) {
 					resource.TestCheckResourceAttrSet(dataSourceName, "records.0.data_timestamp"),
 				),
 			},
+			{
+				ConfigDirectory: config.TestStepDirectory(),
+				ConfigVariables: variableSet1,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(dataSourceName, "records.#", "1"),
+				),
+			},
+		},
+	})
+}
+
+func TestAcc_DynamicTables_badCombination(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				Config:      dynamicTablesDatasourceConfigDbAndSchema(),
+				ExpectError: regexp.MustCompile("Invalid combination of arguments"),
+			},
 		},
 	})
 }
 
+func TestAcc_DynamicTables_emptyIn(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+		Steps: []resource.TestStep{
+			{
+				Config:      dynamicTablesDatasourceEmptyIn(),
+				ExpectError: regexp.MustCompile("Invalid combination of arguments"),
+			},
+		},
+	})
+}
+
+func dynamicTablesDatasourceConfigDbAndSchema() string {
+	return fmt.Sprintf(`
+data "snowflake_dynamic_tables" "dts" {
+  in {
+    database = "%s"
+    schema   = "%s"
+  }
+}
+`, acc.TestDatabaseName, acc.TestSchemaName)
+}
+
+func dynamicTablesDatasourceEmptyIn() string {
+	return `
+data "snowflake_dynamic_tables" "dts" {
+  in {
+  }
+}
+`
+}
+
 func testAccCheckDynamicTableDestroy(s *terraform.State) error {
 	db := acc.TestAccProvider.Meta().(*sql.DB)
 	client := sdk.NewClientFromDB(db)

diff --git a/pkg/datasources/testdata/TestAcc_DynamicTables_complete/1/test.tf b/pkg/datasources/testdata/TestAcc_DynamicTables_complete/1/test.tf
@@ -1,5 +1,3 @@
-
-
 resource "snowflake_table" "t" {
   database        = var.database
   schema          = var.schema

diff --git a/pkg/datasources/testdata/TestAcc_DynamicTables_complete/2/test.tf b/pkg/datasources/testdata/TestAcc_DynamicTables_complete/2/test.tf
@@ -0,0 +1,36 @@
+resource "snowflake_table" "t" {
+  database        = var.database
+  schema          = var.schema
+  name            = var.table_name
+  change_tracking = true
+  column {
+    name = "id"
+    type = "NUMBER(38,0)"
+  }
+}
+
+resource "snowflake_dynamic_table" "dt" {
+  depends_on = [snowflake_table.t]
+  name       = var.name
+  database   = var.database
+  schema     = var.schema
+  target_lag {
+    maximum_duration = "2 minutes"
+  }
+  warehouse = var.warehouse
+  query     = var.query
+  comment   = var.comment
+}
+
+data "snowflake_dynamic_tables" "dts" {
+  like {
+    pattern = snowflake_dynamic_table.dt.name
+  }
+  in {
+    schema = "\"${snowflake_dynamic_table.dt.database}\".\"${snowflake_dynamic_table.dt.schema}\""
+  }
+  starts_with = snowflake_dynamic_table.dt.name
+  limit {
+    rows = 1
+  }
+}
diff --git a/pkg/datasources/testdata/TestAcc_DynamicTables_complete/2/variables.tf b/pkg/datasources/testdata/TestAcc_DynamicTables_complete/2/variables.tf
@@ -0,0 +1,29 @@
+
+
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "schema" {
+  type = string
+}
+
+variable "warehouse" {
+  type = string
+}
+
+variable "query" {
+  type = string
+}
+
+variable "comment" {
+  type = string
+}
+
+variable "table_name" {
+  type = string
+}
diff --git a/pkg/resources/database.go b/pkg/resources/database.go
@@ -190,8 +190,10 @@ func ReadDatabase(d *schema.ResourceData, meta interface{}) error {
 
 	database, err := client.Databases.ShowByID(ctx, id)
 	if err != nil {
-		return err
+		d.SetId("")
+		return nil
 	}
+
 	if err := d.Set("name", database.Name); err != nil {
 		return err
 	}