fix: Fix several small issues (#2697)

Fixed a few issues (from GH and reported internally):
- Do not swallow error in access token setup (References: #2678)
- Fix import example after v0.85.0 changes (References: #993)
- Fix alter allowed values for tags (reported internally)
- Suppress diff for quoted on_table/on_view in stream resource
(References: #2672)
- Test primary key creation (References: #2674 #2629)
- Fix v0.85.0 state migrator for external functions (References: #2694)

docs/resources/external_function.md

@@ -84,6 +84,6 @@ Required:
 Import is supported using the following syntax:
 
 ```shell
-# format is database name | schema name | external function name | <list of function arg types, separated with '-'>
-terraform import snowflake_external_function.example 'dbName|schemaName|externalFunctionName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<external_function_name>(<arg types, separated with ','>)
+terraform import snowflake_external_function.example 'dbName.schemaName.externalFunctionName(varchar, varchar, varchar)'
 ```

docs/resources/function.md

@@ -129,6 +129,6 @@ Required:
 Import is supported using the following syntax:
 
 ```shell
-# format is database name | schema name | function name | <list of arg types, separated with '-'>
-terraform import snowflake_function.example 'dbName|schemaName|functionName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<function_name>(<arg types, separated with ','>)
+terraform import snowflake_function.example 'dbName.schemaName.functionName(varchar, varchar, varchar)'
 ```

docs/resources/procedure.md

@@ -90,6 +90,6 @@ Required:
 Import is supported using the following syntax:
 
 ```shell
-# format is database name | schema name | stored procedure name | <list of arg types, separated with '-'>
-terraform import snowflake_procedure.example 'dbName|schemaName|procedureName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<procedure_name>(<arg types, separated with ','>)
+terraform import snowflake_procedure.example 'dbName.schemaName.procedureName(varchar, varchar, varchar)'
 ```

examples/resources/snowflake_external_function/import.sh

@@ -1,2 +1,2 @@
-# format is database name | schema name | external function name | <list of function arg types, separated with '-'>
-terraform import snowflake_external_function.example 'dbName|schemaName|externalFunctionName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<external_function_name>(<arg types, separated with ','>)
+terraform import snowflake_external_function.example 'dbName.schemaName.externalFunctionName(varchar, varchar, varchar)'

examples/resources/snowflake_function/import.sh

@@ -1,2 +1,2 @@
-# format is database name | schema name | function name | <list of arg types, separated with '-'>
-terraform import snowflake_function.example 'dbName|schemaName|functionName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<function_name>(<arg types, separated with ','>)
+terraform import snowflake_function.example 'dbName.schemaName.functionName(varchar, varchar, varchar)'

examples/resources/snowflake_procedure/import.sh

@@ -1,2 +1,2 @@
-# format is database name | schema name | stored procedure name | <list of arg types, separated with '-'>
-terraform import snowflake_procedure.example 'dbName|schemaName|procedureName|varchar-varchar-varchar'
+# format is <database_name>.<schema_name>.<procedure_name>(<arg types, separated with ','>)
+terraform import snowflake_procedure.example 'dbName.schemaName.procedureName(varchar, varchar, varchar)'

pkg/provider/provider.go

@@ -720,7 +720,7 @@ func ConfigureProvider(s *schema.ResourceData) (interface{}, error) {
 			redirectURI := tokenAccessor["redirect_uri"].(string)
 			accessToken, err := GetAccessTokenWithRefreshToken(tokenEndpoint, clientID, clientSecret, refreshToken, redirectURI)
 			if err != nil {
-				return nil, fmt.Errorf("could not retrieve access token from refresh token")
+				return nil, fmt.Errorf("could not retrieve access token from refresh token, err = %w", err)
 			}
 			config.Token = accessToken
 			config.Authenticator = gosnowflake.AuthTypeOAuth

pkg/resources/common.go

@@ -3,6 +3,7 @@ package resources
 import (
 	"strings"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -23,3 +24,20 @@ func DiffSuppressStatement(_, old, new string, _ *schema.ResourceData) bool {
 func normalizeQuery(str string) string {
 	return strings.TrimSpace(space.ReplaceAllString(str, " "))
 }
+
+// TODO [SNOW-999049]: address during identifiers rework
+func suppressIdentifierQuoting(k, oldValue, newValue string, d *schema.ResourceData) bool {
+	if oldValue == "" || newValue == "" {
+		return false
+	} else {
+		oldId, err := helpers.DecodeSnowflakeParameterID(oldValue)
+		if err != nil {
+			return false
+		}
+		newId, err := helpers.DecodeSnowflakeParameterID(newValue)
+		if err != nil {
+			return false
+		}
+		return oldId.FullyQualifiedName() == newId.FullyQualifiedName()
+	}
+}

pkg/resources/common_test.go

@@ -0,0 +1,49 @@
+package resources
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func Test_suppressIdentifierQuoting(t *testing.T) {
+	firstId := "a.b.c"
+	firstIdQuoted := "\"a\".b.\"c\""
+	secondId := "d.e.f"
+	incorrectId := "a.b.c.d.e.f"
+
+	t.Run("old identifier with too many parts", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", incorrectId, firstId, nil)
+		require.False(t, result)
+	})
+
+	t.Run("new identifier with too many parts", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", firstId, incorrectId, nil)
+		require.False(t, result)
+	})
+
+	t.Run("old identifier empty", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", "", firstId, nil)
+		require.False(t, result)
+	})
+
+	t.Run("new identifier empty", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", firstId, "", nil)
+		require.False(t, result)
+	})
+
+	t.Run("identifiers the same", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", firstId, firstId, nil)
+		require.True(t, result)
+	})
+
+	t.Run("identifiers the same (but different quoting)", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", firstId, firstIdQuoted, nil)
+		require.True(t, result)
+	})
+
+	t.Run("identifiers different", func(t *testing.T) {
+		result := suppressIdentifierQuoting("", firstId, secondId, nil)
+		require.False(t, result)
+	})
+}

pkg/resources/external_function_acceptance_test.go

@@ -273,6 +273,97 @@ func TestAcc_ExternalFunction_migrateFromVersion085(t *testing.T) {
 	})
 }
 
+func TestAcc_ExternalFunction_migrateFromVersion085_issue2694_previousValuePresent(t *testing.T) {
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	resourceName := "snowflake_external_function.f"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.85.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: externalFunctionConfigWithReturnNullAllowed(acc.TestDatabaseName, acc.TestSchemaName, name, sdk.Bool(true)),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "return_null_allowed", "true"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   externalFunctionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()},
+				},
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "return_null_allowed", "true"),
+				),
+			},
+		},
+	})
+}
+
+func TestAcc_ExternalFunction_migrateFromVersion085_issue2694_previousValueRemoved(t *testing.T) {
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	resourceName := "snowflake_external_function.f"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: nil,
+
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.85.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: externalFunctionConfigWithReturnNullAllowed(acc.TestDatabaseName, acc.TestSchemaName, name, sdk.Bool(true)),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "return_null_allowed", "true"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.85.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: externalFunctionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckNoResourceAttr(resourceName, "return_null_allowed"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   externalFunctionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()},
+				},
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "return_null_allowed", "true"),
+				),
+			},
+		},
+	})
+}
+
 // Proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2528.
 // The problem originated from ShowById without IN clause. There was no IN clause in the docs at the time.
 // It was raised with the appropriate team in Snowflake.
@@ -301,6 +392,15 @@ func TestAcc_ExternalFunction_issue2528(t *testing.T) {
 }
 
 func externalFunctionConfig(database string, schema string, name string) string {
+	return externalFunctionConfigWithReturnNullAllowed(database, schema, name, nil)
+}
+
+func externalFunctionConfigWithReturnNullAllowed(database string, schema string, name string, returnNullAllowed *bool) string {
+	returnNullAllowedText := ""
+	if returnNullAllowed != nil {
+		returnNullAllowedText = fmt.Sprintf("return_null_allowed = \"%t\"", *returnNullAllowed)
+	}
+
 	return fmt.Sprintf(`
 resource "snowflake_api_integration" "test_api_int" {
   name                 = "%[3]s"
@@ -326,9 +426,10 @@ resource "snowflake_external_function" "f" {
   return_behavior           = "IMMUTABLE"
   api_integration           = snowflake_api_integration.test_api_int.name
   url_of_proxy_and_resource = "https://123456.execute-api.us-west-2.amazonaws.com/prod/test_func"
+  %[4]s
 }
 
-`, database, schema, name)
+`, database, schema, name, returnNullAllowedText)
 }
 
 func externalFunctionConfigIssue2528(database string, schema string, name string, schema2 string) string {

pkg/resources/external_function_state_upgraders.go

@@ -69,7 +69,7 @@ func v085ExternalFunctionStateUpgrader(ctx context.Context, rawState map[string]
 	rawState["database"] = strings.Trim(oldDatabase, "\"")
 	rawState["schema"] = strings.Trim(oldSchema, "\"")
 
-	if old, isPresent := rawState["return_null_allowed"]; !isPresent || old == nil || old.(string) == "" {
+	if old, isPresent := rawState["return_null_allowed"]; !isPresent || old == nil {
 		rawState["return_null_allowed"] = "true"
 	}
 

pkg/resources/masking_policy_acceptance_test.go

@@ -2,14 +2,14 @@ package resources_test
 
 import (
 	"fmt"
-	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"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/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 

pkg/resources/stream.go

@@ -38,18 +38,20 @@ var streamSchema = map[string]*schema.Schema{
 		Description: "Specifies a comment for the stream.",
 	},
 	"on_table": {
-		Type:         schema.TypeString,
-		Optional:     true,
-		ForceNew:     true,
-		Description:  "Specifies an identifier for the table the stream will monitor.",
-		ExactlyOneOf: []string{"on_table", "on_view", "on_stage"},
+		Type:             schema.TypeString,
+		Optional:         true,
+		ForceNew:         true,
+		Description:      "Specifies an identifier for the table the stream will monitor.",
+		ExactlyOneOf:     []string{"on_table", "on_view", "on_stage"},
+		DiffSuppressFunc: suppressIdentifierQuoting,
 	},
 	"on_view": {
-		Type:         schema.TypeString,
-		Optional:     true,
-		ForceNew:     true,
-		Description:  "Specifies an identifier for the view the stream will monitor.",
-		ExactlyOneOf: []string{"on_table", "on_view", "on_stage"},
+		Type:             schema.TypeString,
+		Optional:         true,
+		ForceNew:         true,
+		Description:      "Specifies an identifier for the view the stream will monitor.",
+		ExactlyOneOf:     []string{"on_table", "on_view", "on_stage"},
+		DiffSuppressFunc: suppressIdentifierQuoting,
 	},
 	"on_stage": {
 		Type:         schema.TypeString,