From dfa52b2c46e5596184969da96aeb30471c74b492 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= <jan.cieslak@snowflake.com>
Date: Thu, 15 Feb 2024 10:31:07 +0100
Subject: [PATCH] fix: user error handling (#2486)

Fix for
https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2481
Adjusted error handling in the User's Read operation. Now when an error
happens and we get the "object does not exist or not authorized" error
then we're marking the resource as removed and return nil. Otherwise, we
return the error. Also, added a test case that proves no error happens
when the user is removed outside of the Terraform (it failed with the
previous implementation).
---
 pkg/resources/user.go                 |  5 +-
 pkg/resources/user_acceptance_test.go | 69 +++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/pkg/resources/user.go b/pkg/resources/user.go
index d77313162f..36471f8d84 100644
--- a/pkg/resources/user.go
+++ b/pkg/resources/user.go
@@ -246,9 +246,8 @@ func ReadUser(d *schema.ResourceData, meta interface{}) error {
 	ctx := context.Background()
 	user, err := client.Users.Describe(ctx, objectIdentifier)
 	if err != nil {
-		if errors.Is(err, sql.ErrNoRows) {
-			// If not found, mark resource to be removed from state file during apply or refresh
-			log.Printf("[DEBUG] user (%s) not found or we are not authorized.Err:\n%s", d.Id(), err.Error())
+		if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) {
+			log.Printf("[DEBUG] user (%s) not found or we are not authorized. Err: %s", d.Id(), err)
 			d.SetId("")
 			return nil
 		}
diff --git a/pkg/resources/user_acceptance_test.go b/pkg/resources/user_acceptance_test.go
index 7300bd876d..986023bf1c 100644
--- a/pkg/resources/user_acceptance_test.go
+++ b/pkg/resources/user_acceptance_test.go
@@ -1,12 +1,19 @@
 package resources_test
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"log"
 	"strconv"
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/testhelpers"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
@@ -112,6 +119,68 @@ func TestAcc_User(t *testing.T) {
 	})
 }
 
+// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2481 has been fixed
+func TestAcc_User_RemovedOutsideOfTerraform(t *testing.T) {
+	userName := sdk.NewAccountObjectIdentifier(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	config := fmt.Sprintf(`
+resource "snowflake_user" "test" {
+	name = "%s"
+}
+`, userName.Name())
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		Steps: []resource.TestStep{
+			{
+				Config: config,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+			{
+				PreConfig: removeUserOutsideOfTerraform(t, userName),
+				Config:    config,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectNonEmptyPlan(),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+				Check: resource.ComposeTestCheckFunc(
+					func(state *terraform.State) error {
+						if len(state.RootModule().Resources) != 1 {
+							return errors.New("user should be created again and present in the state")
+						}
+						return nil
+					},
+				),
+			},
+		},
+	})
+}
+
+func removeUserOutsideOfTerraform(t *testing.T, name sdk.AccountObjectIdentifier) func() {
+	t.Helper()
+	return func() {
+		client, err := sdk.NewDefaultClient()
+		if err != nil {
+			t.Fatal(err)
+		}
+		ctx := context.Background()
+		if err := client.Users.Drop(ctx, name); err != nil {
+			t.Fatalf("failed to drop user: %s", name.FullyQualifiedName())
+		}
+	}
+}
+
 func uConfig(prefix, key1, key2 string) string {
 	s := `
 resource "snowflake_user" "w" {