changes after review

MIGRATION_GUIDE.md

@@ -12,9 +12,11 @@ across different versions.
 ### snowflake_account resource changes
 
 Changes:
+- `admin_user_type` is now supported. No action required during the migration.
+- `grace_period_in_days` is now required. The field should be explicitly set in the following versions.
 - Account renaming is now supported.
 - `is_org_admin` is a settable field (previously it was read-only field). Changing its value is also supported.
-- `must_change_password` and `is_org_admin` type was changed from `bool` to bool-string (more on that [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/CHANGES_BEFORE_V1.md#empty-values)). No action should be required during the migration.
+- `must_change_password` and `is_org_admin` type was changed from `bool` to bool-string (more on that [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/CHANGES_BEFORE_V1.md#empty-values)). No action required during the migration. 
 - The underlying resource identifier was changed from `<account_locator>` to `<organization_name>.<account_name>`. Migration will be done automatically. Notice this introduces changes in how `snowflake_account` resource is imported.
 - New `show_output` field was added (see [raw Snowflake output](./v1-preparations/CHANGES_BEFORE_V1.md#raw-snowflake-output)).
 

docs/resources/account.md

@@ -71,12 +71,12 @@ resource "snowflake_account" "complete" {
 - `edition` (String) Snowflake Edition of the account. See more about Snowflake Editions in the [official documentation](https://docs.snowflake.com/en/user-guide/intro-editions). Valid options are: `STANDARD` | `ENTERPRISE` | `BUSINESS_CRITICAL`
 - `email` (String, Sensitive) Email address of the initial administrative user of the account. This email address is used to send any notifications about the account. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".
 - `grace_period_in_days` (Number) Specifies the number of days during which the account can be restored (“undropped”). The minimum is 3 days and the maximum is 90 days.
-- `name` (String) Specifies the identifier (i.e. name) for the account. It be unique within an organization, regardless of which Snowflake Region the account is in and must start with an alphabetic character and cannot contain spaces or special characters except for underscores (_). Note that if the account name includes underscores, features that do not accept account names with underscores (e.g. Okta SSO or SCIM) can reference a version of the account name that substitutes hyphens (-) for the underscores.
+- `name` (String) Specifies the identifier (i.e. name) for the account. It must be unique within an organization, regardless of which Snowflake Region the account is in and must start with an alphabetic character and cannot contain spaces or special characters except for underscores (_). Note that if the account name includes underscores, features that do not accept account names with underscores (e.g. Okta SSO or SCIM) can reference a version of the account name that substitutes hyphens (-) for the underscores.
 
 ### Optional
 
 - `admin_password` (String, Sensitive) Password for the initial administrative user of the account. Either admin_password or admin_rsa_public_key has to be specified. This field cannot be used whenever admin_user_type is set to SERVICE. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".
-- `admin_rsa_public_key` (String, Sensitive) Assigns a public key to the initial administrative user of the account. Either admin_password or admin_rsa_public_key has to be specified. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".
+- `admin_rsa_public_key` (String) Assigns a public key to the initial administrative user of the account. Either admin_password or admin_rsa_public_key has to be specified. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".
 - `admin_user_type` (String) Used for setting the type of the first user that is assigned the ACCOUNTADMIN role during account creation. Valid options are: `PERSON` | `SERVICE` | `LEGACY_SERVICE` External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".
 - `comment` (String) Specifies a comment for the account.
 - `first_name` (String, Sensitive) First name of the initial administrative user of the account. This field cannot be used whenever admin_user_type is set to SERVICE. External changes for this field won't be detected. In case you want to apply external changes, you can re-create the resource manually using "terraform taint".

pkg/resources/account.go

@@ -26,7 +26,7 @@ var accountSchema = map[string]*schema.Schema{
 	"name": {
 		Type:        schema.TypeString,
 		Required:    true,
-		Description: "Specifies the identifier (i.e. name) for the account. It be unique within an organization, regardless of which Snowflake Region the account is in and must start with an alphabetic character and cannot contain spaces or special characters except for underscores (_). Note that if the account name includes underscores, features that do not accept account names with underscores (e.g. Okta SSO or SCIM) can reference a version of the account name that substitutes hyphens (-) for the underscores.",
+		Description: "Specifies the identifier (i.e. name) for the account. It must be unique within an organization, regardless of which Snowflake Region the account is in and must start with an alphabetic character and cannot contain spaces or special characters except for underscores (_). Note that if the account name includes underscores, features that do not accept account names with underscores (e.g. Okta SSO or SCIM) can reference a version of the account name that substitutes hyphens (-) for the underscores.",
 	},
 	"admin_name": {
 		Type:             schema.TypeString,
@@ -46,7 +46,6 @@ var accountSchema = map[string]*schema.Schema{
 	"admin_rsa_public_key": {
 		Type:             schema.TypeString,
 		Optional:         true,
-		Sensitive:        true,
 		Description:      externalChangesNotDetectedFieldDescription("Assigns a public key to the initial administrative user of the account. Either admin_password or admin_rsa_public_key has to be specified."),
 		DiffSuppressFunc: IgnoreAfterCreation,
 		AtLeastOneOf:     []string{"admin_password", "admin_rsa_public_key"},
@@ -190,7 +189,7 @@ func ImportAccount(ctx context.Context, d *schema.ResourceData, meta any) ([]*sc
 		return nil, err
 	}
 
-	account, err := client.Accounts.ShowByID(ctx, id.AccountId())
+	account, err := client.Accounts.ShowByID(ctx, id.AsAccountObjectIdentifier())
 	if err != nil {
 		return nil, err
 	}
@@ -311,7 +310,7 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc {
 			return diag.FromErr(err)
 		}
 
-		account, err := client.Accounts.ShowByID(ctx, id.AccountId())
+		account, err := client.Accounts.ShowByID(ctx, id.AsAccountObjectIdentifier())
 		if err != nil {
 			if errors.Is(err, sdk.ErrObjectNotFound) {
 				d.SetId("")
@@ -400,8 +399,8 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D
 
 		err = client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{
 			Rename: &sdk.AccountRename{
-				Name:    id.AccountId(),
-				NewName: newId.AccountId(),
+				Name:    id.AsAccountObjectIdentifier(),
+				NewName: newId.AsAccountObjectIdentifier(),
 			},
 		})
 		if err != nil {
@@ -430,7 +429,7 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D
 				}
 				if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{
 					SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{
-						Name:     id.AccountId(),
+						Name:     id.AsAccountObjectIdentifier(),
 						OrgAdmin: parsed,
 					},
 				}); err != nil {
@@ -440,7 +439,7 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D
 				// No unset available for this field (setting Snowflake default)
 				if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{
 					SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{
-						Name:     id.AccountId(),
+						Name:     id.AsAccountObjectIdentifier(),
 						OrgAdmin: false,
 					},
 				}); err != nil {
@@ -469,7 +468,7 @@ func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D
 		return diag.FromErr(err)
 	}
 
-	err = client.Accounts.Drop(ctx, id.AccountId(), d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{
+	err = client.Accounts.Drop(ctx, id.AsAccountObjectIdentifier(), d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{
 		IfExists: sdk.Bool(true),
 	})
 	if err != nil {

pkg/resources/account_acceptance_test.go

@@ -391,7 +391,7 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) {
 				PreConfig: func() {
 					acc.TestClient().Account.Alter(t, &sdk.AlterAccountOptions{
 						SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{
-							Name:     accountId.AccountId(),
+							Name:     accountId.AsAccountObjectIdentifier(),
 							OrgAdmin: true,
 						},
 					})

pkg/resources/account_state_upgraders.go

@@ -10,6 +10,10 @@ import (
 )
 
 func v0_99_0_AccountStateUpgrader(ctx context.Context, state map[string]any, meta any) (map[string]any, error) {
+	if state == nil {
+		return state, nil
+	}
+
 	client := meta.(*provider.Context).Client
 	state["must_change_password"] = booleanStringFromBool(state["must_change_password"].(bool))
 	state["is_org_admin"] = booleanStringFromBool(state["is_org_admin"].(bool))

pkg/sdk/accounts.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log"
 	"strings"
 	"time"
 
@@ -143,7 +144,11 @@ func (c *accounts) Create(ctx context.Context, id AccountObjectIdentifier, opts
 
 	queryId := <-queryChanId
 	rows, err := c.client.QueryUnsafe(gosnowflake.WithFetchResultByID(ctx, queryId), "")
-	if err == nil && len(rows) == 1 && rows[0]["status"] != nil {
+	if err != nil {
+		log.Printf("[WARN] Unable to retrieve create account output, err = %v", err)
+	}
+
+	if len(rows) == 1 && rows[0]["status"] != nil {
 		if status, ok := (*rows[0]["status"]).(string); ok {
 			return ToAccountCreateResponse(status)
 		}

pkg/sdk/identifier_helpers.go

@@ -124,7 +124,7 @@ func (i AccountIdentifier) AccountName() string {
 	return i.accountName
 }
 
-func (i AccountIdentifier) AccountId() AccountObjectIdentifier {
+func (i AccountIdentifier) AsAccountObjectIdentifier() AccountObjectIdentifier {
 	return NewAccountObjectIdentifier(i.accountName)
 }
 

pkg/sdk/testint/accounts_integration_test.go

@@ -94,18 +94,16 @@ func TestInt_Account(t *testing.T) {
 
 	assertCreateResponse := func(t *testing.T, response *sdk.AccountCreateResponse, account sdk.Account) {
 		t.Helper()
-		assert.NotNil(t, response)
-		if response != nil {
-			assert.Equal(t, account.AccountLocator, response.AccountLocator)
-			assert.Equal(t, *account.AccountLocatorUrl, response.AccountLocatorUrl)
-			assert.Equal(t, account.AccountName, response.AccountName)
-			assert.Equal(t, *account.AccountURL, response.Url)
-			assert.Equal(t, account.OrganizationName, response.OrganizationName)
-			assert.Equal(t, *account.Edition, response.Edition)
-			assert.NotEmpty(t, response.RegionGroup)
-			assert.NotEmpty(t, response.Cloud)
-			assert.NotEmpty(t, response.Region)
-		}
+		require.NotNil(t, response)
+		assert.Equal(t, account.AccountLocator, response.AccountLocator)
+		assert.Equal(t, *account.AccountLocatorUrl, response.AccountLocatorUrl)
+		assert.Equal(t, account.AccountName, response.AccountName)
+		assert.Equal(t, *account.AccountURL, response.Url)
+		assert.Equal(t, account.OrganizationName, response.OrganizationName)
+		assert.Equal(t, *account.Edition, response.Edition)
+		assert.NotEmpty(t, response.RegionGroup)
+		assert.NotEmpty(t, response.Cloud)
+		assert.NotEmpty(t, response.Region)
 	}
 
 	t.Run("create: minimal", func(t *testing.T) {