Merge branch 'main' into functions-sdk

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)
 
 

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) .

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).

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,
 	},

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)

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") {

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 == "" {

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
+}

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
+}

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

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())
+	})
+}

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())
+	})
+}