From abf58839374281cbe66ee4430b73a501de01cc63 Mon Sep 17 00:00:00 2001
From: Nick Venenga <nick@venenga.com>
Date: Wed, 19 Feb 2025 08:22:58 -0500
Subject: [PATCH] fix: external function varchar return type validation (#3400)

References https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3392

Signed-off-by: Nick Venenga <nick.venenga@15five.com>
Co-authored-by: Jakub Michalak <jakub.michalak@snowflake.com>
---
 MIGRATION_GUIDE.md                            |  5 ++
 pkg/resources/external_function.go            |  4 +-
 .../external_function_acceptance_test.go      | 80 +++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md
index fbc6729d1f..04e76a9423 100644
--- a/MIGRATION_GUIDE.md
+++ b/MIGRATION_GUIDE.md
@@ -7,6 +7,11 @@ across different versions.
 > [!TIP]
 > We highly recommend upgrading the versions one by one instead of bulk upgrades.
 
+## v1.0.3 ➞ v1.0.4
+
+### Fixed external_function VARCHAR return_type
+VARCHAR external_function return_type did not work correctly before ([#3392](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/3392)) but was fixed in this version.
+
 ## v1.0.2 ➞ v1.0.3
 
 ### Fixed METRIC_LEVEL parameter
diff --git a/pkg/resources/external_function.go b/pkg/resources/external_function.go
index ebd37b5c81..84a6a4358f 100644
--- a/pkg/resources/external_function.go
+++ b/pkg/resources/external_function.go
@@ -406,8 +406,8 @@ func ReadContextExternalFunction(ctx context.Context, d *schema.ResourceData, me
 			}
 		case "returns":
 			returnType := row.Value
-			// We first check for VARIANT or OBJECT
-			if returnType == "VARIANT" || returnType == "OBJECT" {
+			// We first check types that don't require size
+			if returnType == "VARIANT" || returnType == "OBJECT" || returnType == "VARCHAR" {
 				if err := d.Set("return_type", returnType); err != nil {
 					return diag.Errorf("error setting return_type: %v", err)
 				}
diff --git a/pkg/resources/external_function_acceptance_test.go b/pkg/resources/external_function_acceptance_test.go
index c53b90167a..e907444bf6 100644
--- a/pkg/resources/external_function_acceptance_test.go
+++ b/pkg/resources/external_function_acceptance_test.go
@@ -404,6 +404,54 @@ func TestAcc_ExternalFunction_issue2528(t *testing.T) {
 	})
 }
 
+func TestAcc_ExternalFunction_issue3392_returnVarchar(t *testing.T) {
+	accName := acc.TestClient().Ids.Alpha()
+
+	resourceName := "snowflake_external_function.f"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.ExternalFunction),
+		Steps: []resource.TestStep{
+			{
+				Config: externalFunctionConfigWithReturnType(acc.TestDatabaseName, acc.TestSchemaName, accName, "VARCHAR"),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "return_type", "VARCHAR"),
+				),
+			},
+		},
+	})
+}
+
+func TestAcc_ExternalFunction_issue3392_returnVarcharWithSize(t *testing.T) {
+	accName := acc.TestClient().Ids.Alpha()
+
+	resourceName := "snowflake_external_function.f"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.ExternalFunction),
+		Steps: []resource.TestStep{
+			{
+				Config: externalFunctionConfigWithReturnType(acc.TestDatabaseName, acc.TestSchemaName, accName, "VARCHAR(10)"),
+				Check: resource.ComposeTestCheckFunc(
+					// Snowflake drops the size from VARCHAR when it's specified
+					resource.TestCheckResourceAttr(resourceName, "return_type", "VARCHAR"),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 // Proves that header parsing handles values wrapped in curly braces, e.g. `value = "{1}"`
 func TestAcc_ExternalFunction_HeaderParsing(t *testing.T) {
 	id := acc.TestClient().Ids.RandomSchemaObjectIdentifier()
@@ -494,6 +542,38 @@ resource "snowflake_external_function" "f" {
 `, database, schema, name, returnNullAllowedText)
 }
 
+func externalFunctionConfigWithReturnType(database string, schema string, name string, returnType string) string {
+
+	return fmt.Sprintf(`
+resource "snowflake_api_integration" "test_api_int" {
+ name                 = "%[3]s"
+ api_provider         = "aws_api_gateway"
+ api_aws_role_arn     = "arn:aws:iam::000000000001:/role/test"
+ api_allowed_prefixes = ["https://123456.execute-api.us-west-2.amazonaws.com/prod/"]
+ enabled              = true
+}
+
+resource "snowflake_external_function" "f" {
+ name     = "%[3]s"
+ database = "%[1]s"
+ schema   = "%[2]s"
+ arg {
+   name = "ARG1"
+   type = "VARCHAR"
+ }
+ arg {
+   name = "ARG2"
+   type = "VARCHAR"
+ }
+ return_type               = "%[4]s"
+ 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"
+}
+
+`, database, schema, name, returnType)
+}
+
 func externalFunctionConfigIssue2528(database string, schema string, name string, schema2 string) string {
 	return fmt.Sprintf(`
 resource "snowflake_api_integration" "test_api_int" {