diff --git a/CHANGELOG.md b/CHANGELOG.md index fe39f23f69..1a9ebcdab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## [0.81.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/compare/v0.80.0...v0.81.0) (2023-12-20) + + +### 🎉 **What's new:** + +* identifier validation ([#2269](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2269)) ([9687972](https://github.com/Snowflake-Labs/terraform-provider-snowflake/commit/9687972fc673bb7b768f9b4100a6d2b67fc46e48)) + + +### 🔧 **Misc** + +* Allow setting gosnowflake logging level from environment variable ([#2285](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2285)) ([843e8fc](https://github.com/Snowflake-Labs/terraform-provider-snowflake/commit/843e8fc8f8879fdb5b1148155e7e64174571e975)) +* Bump go to 1.21 ([#2267](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2267)) ([6b852c2](https://github.com/Snowflake-Labs/terraform-provider-snowflake/commit/6b852c2ba6b1348ba83ec2e2190e8175371524be)) + + +### 🐛 **Bug fixes:** + +* Fix small issues ([#2287](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2287)) ([4e2613c](https://github.com/Snowflake-Labs/terraform-provider-snowflake/commit/4e2613c82edc1e0f91fb0cb56dcaa977d412d785)) + ## [0.80.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/compare/v0.79.1...v0.80.0) (2023-12-13) diff --git a/Makefile b/Makefile index 7597519a45..44f3ec6b58 100644 --- a/Makefile +++ b/Makefile @@ -77,7 +77,7 @@ test-architecture: ## check architecture constraints between packages go test ./pkg/architests/... -v test-client: ## runs test that checks sdk.Client without instrumentedsql - SF_TF_NO_INSTRUMENTED_SQL=1 go test ./pkg/sdk/internal/client/... -v + SF_TF_NO_INSTRUMENTED_SQL=1 SF_TF_GOSNOWFLAKE_LOG_LEVEL=debug go test ./pkg/sdk/internal/client/... -v build-local: ## build the binary locally go build -o $(BASE_BINARY_NAME) . diff --git a/README.md b/README.md index 92ef72c870..5b4e49a116 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,18 @@ Set environment variable `SF_TF_ADDITIONAL_DEBUG_LOGGING` to a non-empty value. ## Additional SQL Client configuration Currently underlying sql [gosnowflake](https://github.com/snowflakedb/gosnowflake) driver is wrapped with [instrumentedsql](https://github.com/luna-duclos/instrumentedsql). In order to use raw [gosnowflake](https://github.com/snowflakedb/gosnowflake) driver, set environment variable `SF_TF_NO_INSTRUMENTED_SQL` to a non-empty value. +By default, the underlying driver is set to error level logging. It can be changed by setting `SF_TF_GOSNOWFLAKE_LOG_LEVEL` to one of: +- `panic` +- `fatal` +- `error` +- `warn` +- `warning` +- `info` +- `debug` +- `trace` + +*note*: It's possible it will be one of the provider config parameters in the future provider versions. + ## Contributing Cf. [Contributing](./CONTRIBUTING.md). diff --git a/pkg/resources/dynamic_table.go b/pkg/resources/dynamic_table.go index 3b452ba061..90cbdd1c22 100644 --- a/pkg/resources/dynamic_table.go +++ b/pkg/resources/dynamic_table.go @@ -65,6 +65,7 @@ var dynamicTableShema = map[string]*schema.Schema{ "query": { Type: schema.TypeString, Required: true, + ForceNew: true, Description: "Specifies the query to use to populate the dynamic table.", DiffSuppressFunc: DiffSuppressStatement, }, diff --git a/pkg/resources/dynamic_table_acceptance_test.go b/pkg/resources/dynamic_table_acceptance_test.go index 34edb137cb..a685561706 100644 --- a/pkg/resources/dynamic_table_acceptance_test.go +++ b/pkg/resources/dynamic_table_acceptance_test.go @@ -236,6 +236,54 @@ func TestAcc_DynamicTable_issue2134(t *testing.T) { }) } +// TestAcc_DynamicTable_issue2276 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2276 issue. +func TestAcc_DynamicTable_issue2276(t *testing.T) { + dynamicTableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + tableName := dynamicTableName + "_table" + query := fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName) + newQuery := fmt.Sprintf(`select "data" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName) + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(dynamicTableName), + "database": config.StringVariable(acc.TestDatabaseName), + "schema": config.StringVariable(acc.TestSchemaName), + "warehouse": config.StringVariable(acc.TestWarehouseName), + "query": config.StringVariable(query), + "comment": config.StringVariable("Terraform acceptance test for GH issue 2276"), + "table_name": config.StringVariable(tableName), + } + } + m2 := m() + m2["query"] = config.StringVariable(newQuery) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: testAccCheckDynamicTableDestroy, + Steps: []resource.TestStep{ + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: m(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "name", dynamicTableName), + resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "query", query), + ), + }, + { + ConfigDirectory: acc.ConfigurationSameAsStepN(1), + ConfigVariables: m2, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "name", dynamicTableName), + resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "query", newQuery), + ), + }, + }, + }) +} + func testAccCheckDynamicTableDestroy(s *terraform.State) error { db := acc.TestAccProvider.Meta().(*sql.DB) client := sdk.NewClientFromDB(db) diff --git a/pkg/resources/resource_monitor.go b/pkg/resources/resource_monitor.go index 0f8c51137d..59e416c69e 100644 --- a/pkg/resources/resource_monitor.go +++ b/pkg/resources/resource_monitor.go @@ -327,7 +327,9 @@ func UpdateResourceMonitor(d *schema.ResourceData, meta interface{}) error { for _, name := range userNames { users = append(users, sdk.NotifiedUser{Name: name}) } - opts.NotifyUsers.Users = users + opts.NotifyUsers = &sdk.NotifyUsers{ + Users: users, + } } if d.HasChange("credit_quota") { diff --git a/pkg/resources/resource_monitor_acceptance_test.go b/pkg/resources/resource_monitor_acceptance_test.go index c6e667897c..5318fa01f8 100644 --- a/pkg/resources/resource_monitor_acceptance_test.go +++ b/pkg/resources/resource_monitor_acceptance_test.go @@ -4,12 +4,14 @@ import ( "encoding/json" "fmt" "os" + "regexp" "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/stretchr/testify/require" ) func TestAcc_ResourceMonitor(t *testing.T) { @@ -216,6 +218,35 @@ resource "snowflake_resource_monitor" "test" { `, accName) } +// TestAcc_ResourceMonitor_issue2167 proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2167 issue. +// Second step is purposely error, because tests TestAcc_ResourceMonitorUpdateNotifyUsers and TestAcc_ResourceMonitorNotifyUsers are still skipped. +// It can be fixed with them. +func TestAcc_ResourceMonitor_issue2167(t *testing.T) { + name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + configNoUsers, err := resourceMonitorNotifyUsersConfig(name, []string{}) + require.NoError(t, err) + config, err := resourceMonitorNotifyUsersConfig(name, []string{"does not matter"}) + require.NoError(t, err) + + resource.Test(t, resource.TestCase{ + Providers: acc.TestAccProviders(), + PreCheck: func() { acc.TestAccPreCheck(t) }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: configNoUsers, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_resource_monitor.test", "name", name), + ), + }, + { + Config: config, + ExpectError: regexp.MustCompile(`.*exactly one of AlterResourceMonitorOptions fields \[Set NotifyUsers Triggers] must be set.*`), + }, + }, + }) +} + func TestAcc_ResourceMonitorNotifyUsers(t *testing.T) { userEnv := os.Getenv("RESOURCE_MONITOR_NOTIFY_USERS_TEST") if userEnv == "" { diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/test.tf new file mode 100644 index 0000000000..7942078c37 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/test.tf @@ -0,0 +1,27 @@ +resource "snowflake_table" "t" { + database = var.database + schema = var.schema + name = var.table_name + change_tracking = true + column { + name = "id" + type = "NUMBER(38,0)" + } + column { + name = "data" + type = "VARCHAR(16)" + } +} + +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 +} diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/variables.tf new file mode 100644 index 0000000000..5a6d6701d9 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_DynamicTable_issue2276/1/variables.tf @@ -0,0 +1,27 @@ +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/sdk/client.go b/pkg/sdk/client.go index 63a3f4a696..b8170d81e8 100644 --- a/pkg/sdk/client.go +++ b/pkg/sdk/client.go @@ -13,10 +13,14 @@ import ( "github.com/snowflakedb/gosnowflake" ) -var instrumentedSQL bool +var ( + instrumentedSQL bool + gosnowflakeLoggingLevel string +) func init() { instrumentedSQL = os.Getenv("SF_TF_NO_INSTRUMENTED_SQL") == "" + gosnowflakeLoggingLevel = os.Getenv("SF_TF_GOSNOWFLAKE_LOG_LEVEL") } type Client struct { @@ -121,6 +125,10 @@ func NewClient(cfg *gosnowflake.Config) (*Client, error) { driverName = "snowflake-instrumented" } + if gosnowflakeLoggingLevel != "" { + cfg.Tracing = gosnowflakeLoggingLevel + } + dsn, err := gosnowflake.DSN(cfg) if err != nil { return nil, err diff --git a/pkg/sdk/client_integration_test.go b/pkg/sdk/client_integration_test.go index 43f9de4608..fdc1ce666d 100644 --- a/pkg/sdk/client_integration_test.go +++ b/pkg/sdk/client_integration_test.go @@ -3,10 +3,11 @@ package sdk import ( "context" "database/sql" + "os" "testing" + "github.com/snowflakedb/gosnowflake" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) @@ -76,3 +77,28 @@ func TestClient_queryOne(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, row.One) } + +func TestClient_NewClientDriverLoggingLevel(t *testing.T) { + t.Run("get default gosnowflake driver logging level", func(t *testing.T) { + config := DefaultConfig() + _, err := NewClient(config) + require.NoError(t, err) + + var expected string + if os.Getenv("GITHUB_ACTIONS") != "" { + expected = "fatal" + } else { + expected = "error" + } + assert.Equal(t, expected, gosnowflake.GetLogger().GetLogLevel()) + }) + + t.Run("set gosnowflake driver logging level with config", func(t *testing.T) { + config := DefaultConfig() + config.Tracing = "trace" + _, err := NewClient(config) + require.NoError(t, err) + + assert.Equal(t, "trace", gosnowflake.GetLogger().GetLogLevel()) + }) +} diff --git a/pkg/sdk/internal/client/client_test.go b/pkg/sdk/internal/client/client_test.go index 9c2b87db52..6bcf5cbc50 100644 --- a/pkg/sdk/internal/client/client_test.go +++ b/pkg/sdk/internal/client/client_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/snowflakedb/gosnowflake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -27,3 +28,17 @@ func TestNewClientWithoutInstrumentedSQL(t *testing.T) { assert.Contains(t, sql.Drivers(), "snowflake") }) } + +func TestNewClientWithDebugLoggingSetFromEnv(t *testing.T) { + t.Run("set gosnowflake driver logging to debug", func(t *testing.T) { + if os.Getenv("SF_TF_GOSNOWFLAKE_LOG_LEVEL") == "" { + t.Skip("Skipping TestNewClientWithDebugLoggingSet, because SF_TF_GOSNOWFLAKE_LOG_LEVEL is not set") + } + + config := sdk.DefaultConfig() + _, err := sdk.NewClient(config) + require.NoError(t, err) + + assert.Equal(t, "debug", gosnowflake.GetLogger().GetLogLevel()) + }) +}