From e3ed26ea37be2d688d0ddef9bb8f3f7130fcddbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 26 Nov 2024 14:21:04 +0100 Subject: [PATCH 01/12] wip fix tests --- pkg/acceptance/helpers/random/certs.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/acceptance/helpers/random/certs.go b/pkg/acceptance/helpers/random/certs.go index c0e0142d7c..ca74761ec0 100644 --- a/pkg/acceptance/helpers/random/certs.go +++ b/pkg/acceptance/helpers/random/certs.go @@ -69,6 +69,15 @@ func GenerateRSAPublicKeyBasedOnPrivateKey(t *testing.T, key *rsa.PrivateKey) (s return encode(t, "RSA PUBLIC KEY", b), hash(t, b) } +func GenerateRSAPublicKeyBasedOnPrivateKey(t *testing.T, key *rsa.PrivateKey) (string, string) { + t.Helper() + + pub := key.Public() + b, err := x509.MarshalPKIXPublicKey(pub.(*rsa.PublicKey)) + require.NoError(t, err) + return encode(t, "RSA PUBLIC KEY", b), hash(t, b) +} + // GenerateRSAPrivateKey returns an RSA private key. func GenerateRSAPrivateKey(t *testing.T) *rsa.PrivateKey { t.Helper() From 4e65a6f2b2b122abca0eb2509beefc9f4851c2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 3 Dec 2024 13:18:29 +0100 Subject: [PATCH 02/12] wip --- pkg/acceptance/helpers/account_client.go | 2 +- pkg/resources/account.go | 598 +++++++++++-------- pkg/schemas/account_gen.go | 142 ++++- pkg/sdk/accounts.go | 52 +- pkg/sdk/accounts_test.go | 115 ++++ pkg/sdk/testint/accounts_integration_test.go | 29 +- 6 files changed, 658 insertions(+), 280 deletions(-) diff --git a/pkg/acceptance/helpers/account_client.go b/pkg/acceptance/helpers/account_client.go index 96605949ab..c9bbe245c6 100644 --- a/pkg/acceptance/helpers/account_client.go +++ b/pkg/acceptance/helpers/account_client.go @@ -69,7 +69,7 @@ func (c *AccountClient) Create(t *testing.T) (*sdk.Account, func()) { func (c *AccountClient) CreateWithRequest(t *testing.T, id sdk.AccountObjectIdentifier, opts *sdk.CreateAccountOptions) (*sdk.Account, func()) { t.Helper() - err := c.client().Create(context.Background(), id, opts) + _, err := c.client().Create(context.Background(), id, opts) require.NoError(t, err) account, err := c.client().ShowByID(context.Background(), id) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 8b687c7cd8..1d85acafca 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -2,18 +2,12 @@ package resources import ( "context" - "fmt" - "log" - "strings" - "time" - - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - + "errors" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/util" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" @@ -24,196 +18,323 @@ import ( // Note: no test case was created for account since we cannot actually delete them after creation, which is a critical part of the test suite. Instead, this resource // was manually tested +//var accountSchemaOld = map[string]*schema.Schema{ +// "name": { +// Type: schema.TypeString, +// Required: true, +// Description: "Specifies the identifier (i.e. name) for the account; must be unique within an organization, regardless of which Snowflake Region the account is in. In addition, the identifier 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 is automatically uppercase by Snowflake +// StateFunc: func(val interface{}) string { +// return strings.ToUpper(val.(string)) +// }, +// ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), +// }, +// "admin_name": { +// Type: schema.TypeString, +// Required: true, +// Description: "Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive.", +// // We have no way of assuming a role into this account to change the admin user name so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "admin_password": { +// Type: schema.TypeString, +// Optional: true, +// Sensitive: true, +// Description: "Password for the initial administrative user of the account. Optional if the `ADMIN_RSA_PUBLIC_KEY` parameter is specified. For more information about passwords in Snowflake, see [Snowflake-provided Password Policy](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=Snowflake%2Dprovided%20Password%20Policy).", +// AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, +// // We have no way of assuming a role into this account to change the password so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "admin_rsa_public_key": { +// Type: schema.TypeString, +// Optional: true, +// Sensitive: true, +// Description: "Assigns a public key to the initial administrative user of the account in order to implement [key pair authentication](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=key%20pair%20authentication) for the user. Optional if the `ADMIN_PASSWORD` parameter is specified.", +// AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, +// // We have no way of assuming a role into this account to change the admin rsa public key so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "email": { +// Type: schema.TypeString, +// Required: true, +// Sensitive: true, +// Description: "Email address of the initial administrative user of the account. This email address is used to send any notifications about the account.", +// // We have no way of assuming a role into this account to change the admin email so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "edition": { +// Type: schema.TypeString, +// Required: true, +// ForceNew: true, +// Description: "[Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values are: STANDARD | ENTERPRISE | BUSINESS_CRITICAL", +// ValidateFunc: validation.StringInSlice([]string{string(sdk.EditionStandard), string(sdk.EditionEnterprise), string(sdk.EditionBusinessCritical)}, false), +// }, +// "first_name": { +// Type: schema.TypeString, +// Optional: true, +// Sensitive: true, +// Description: "First name of the initial administrative user of the account", +// // We have no way of assuming a role into this account to change the admin first name so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "last_name": { +// Type: schema.TypeString, +// Optional: true, +// Sensitive: true, +// Description: "Last name of the initial administrative user of the account", +// // We have no way of assuming a role into this account to change the admin last name so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "must_change_password": { +// Type: schema.TypeBool, +// Optional: true, +// Default: false, +// Description: "Specifies whether the new user created to administer the account is forced to change their password upon first login into the account.", +// // We have no way of assuming a role into this account to change the admin password policy so this has to be ForceNew even though it's not ideal +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return old == "" +// }, +// }, +// "region_group": { +// Type: schema.TypeString, +// Optional: true, +// Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return new == "" +// }, +// }, +// "region": { +// Type: schema.TypeString, +// Optional: true, +// Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", +// ForceNew: true, +// DiffSuppressOnRefresh: true, +// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { +// // For new resources always show the diff +// if d.Id() == "" { +// return false +// } +// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value +// return new == "" +// }, +// }, +// "comment": { +// Type: schema.TypeString, +// Optional: true, +// Description: "Specifies a comment for the account.", +// ForceNew: true, +// }, +// "is_org_admin": { +// Type: schema.TypeBool, +// Computed: true, +// Description: "Indicates whether the ORGADMIN role is enabled in an account. If TRUE, the role is enabled.", +// }, +// "grace_period_in_days": { +// Type: schema.TypeInt, +// Optional: true, +// Default: 3, +// Description: "Specifies the number of days to wait before dropping the account. The default is 3 days.", +// }, +// FullyQualifiedNameAttributeName: schemas.FullyQualifiedNameSchema, +//} + var accountSchema = map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - Description: "Specifies the identifier (i.e. name) for the account; must be unique within an organization, regardless of which Snowflake Region the account is in. In addition, the identifier 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 is automatically uppercase by Snowflake - StateFunc: func(val interface{}) string { - return strings.ToUpper(val.(string)) - }, + Type: schema.TypeString, + Required: true, + Description: "TODO", ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), }, "admin_name": { - Type: schema.TypeString, - Required: true, - Description: "Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive.", - // We have no way of assuming a role into this account to change the admin user name so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Required: true, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, }, "admin_password": { - Type: schema.TypeString, - Optional: true, - Sensitive: true, - Description: "Password for the initial administrative user of the account. Optional if the `ADMIN_RSA_PUBLIC_KEY` parameter is specified. For more information about passwords in Snowflake, see [Snowflake-provided Password Policy](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=Snowflake%2Dprovided%20Password%20Policy).", - AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, - // We have no way of assuming a role into this account to change the password so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, + AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, }, "admin_rsa_public_key": { - Type: schema.TypeString, - Optional: true, - Sensitive: true, - Description: "Assigns a public key to the initial administrative user of the account in order to implement [key pair authentication](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=key%20pair%20authentication) for the user. Optional if the `ADMIN_PASSWORD` parameter is specified.", - AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, - // We have no way of assuming a role into this account to change the admin rsa public key so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, + AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, }, - "email": { - Type: schema.TypeString, - Required: true, - Sensitive: true, - Description: "Email address of the initial administrative user of the account. This email address is used to send any notifications about the account.", - // We have no way of assuming a role into this account to change the admin email so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, - }, - "edition": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - Description: "[Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values are: STANDARD | ENTERPRISE | BUSINESS_CRITICAL", - ValidateFunc: validation.StringInSlice([]string{string(sdk.EditionStandard), string(sdk.EditionEnterprise), string(sdk.EditionBusinessCritical)}, false), + "admin_user_type": { + Type: schema.TypeString, + Required: true, + // TODO: Valid options + Description: "TODO", + DiffSuppressFunc: SuppressIfAny(IgnoreAfterCreation, NormalizeAndCompare(sdk.ToUserType)), + ValidateDiagFunc: sdkValidation(sdk.ToUserType), }, "first_name": { - Type: schema.TypeString, - Optional: true, - Sensitive: true, - Description: "First name of the initial administrative user of the account", - // We have no way of assuming a role into this account to change the admin first name so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, }, "last_name": { - Type: schema.TypeString, - Optional: true, - Sensitive: true, - Description: "Last name of the initial administrative user of the account", - // We have no way of assuming a role into this account to change the admin last name so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Optional: true, + Sensitive: true, + Description: "", + DiffSuppressFunc: IgnoreAfterCreation, + }, + "email": { + Type: schema.TypeString, + Required: true, + Sensitive: true, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, }, "must_change_password": { - Type: schema.TypeBool, - Optional: true, - Default: false, - Description: "Specifies whether the new user created to administer the account is forced to change their password upon first login into the account.", - // We have no way of assuming a role into this account to change the admin password policy so this has to be ForceNew even though it's not ideal - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return old == "" - }, + Type: schema.TypeString, + Optional: true, + Default: BooleanDefault, + Description: "TODO", + DiffSuppressFunc: IgnoreAfterCreation, + ValidateDiagFunc: validateBooleanString, + }, + "edition": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + // TODO: Desc + Description: "[Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values are: STANDARD | ENTERPRISE | BUSINESS_CRITICAL", + // TODO: Valid options + ValidateFunc: validation.StringInSlice([]string{string(sdk.EditionStandard), string(sdk.EditionEnterprise), string(sdk.EditionBusinessCritical)}, false), }, "region_group": { - Type: schema.TypeString, - Optional: true, - Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return new == "" - }, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", }, "region": { - Type: schema.TypeString, - Optional: true, - Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", - ForceNew: true, - DiffSuppressOnRefresh: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - // For new resources always show the diff - if d.Id() == "" { - return false - } - // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value - return new == "" - }, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", }, "comment": { Type: schema.TypeString, Optional: true, - Description: "Specifies a comment for the account.", ForceNew: true, + Description: "Specifies a comment for the account.", }, "is_org_admin": { - Type: schema.TypeBool, - Computed: true, - Description: "Indicates whether the ORGADMIN role is enabled in an account. If TRUE, the role is enabled.", + Type: schema.TypeString, + Optional: true, + Default: BooleanDefault, + Description: "TODO", + ValidateDiagFunc: validateBooleanString, }, "grace_period_in_days": { - Type: schema.TypeInt, - Optional: true, - Default: 3, - Description: "Specifies the number of days to wait before dropping the account. The default is 3 days.", + Type: schema.TypeInt, + Required: true, + Description: "TODO", + ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(3)), }, FullyQualifiedNameAttributeName: schemas.FullyQualifiedNameSchema, + ShowOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Outputs the result of `SHOW ACCOUNTS` for the given account.", + Elem: &schema.Resource{ + Schema: schemas.ShowAccountSchema, + }, + }, + // TODO: This one will be pretty big (possibly over 200 parameters) + //ParametersAttributeName: { + // Type: schema.TypeList, + // Computed: true, + // Description: "Outputs the result of `SHOW PARAMETERS IN TASK` for the given task.", + // Elem: &schema.Resource{ + // Schema: schemas.ShowTaskParametersSchema, + // }, + //}, } func Account() *schema.Resource { return &schema.Resource{ + // TODO: Desc Description: "The account resource allows you to create and manage Snowflake accounts.", CreateContext: TrackingCreateWrapper(resources.Account, CreateAccount), ReadContext: TrackingReadWrapper(resources.Account, ReadAccount), @@ -231,144 +352,96 @@ func Account() *schema.Resource { } } -// CreateAccount implements schema.CreateFunc. -func CreateAccount(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client + id := sdk.NewAccountObjectIdentifier(d.Get("name").(string)) - name := d.Get("name").(string) - objectIdentifier := sdk.NewAccountObjectIdentifier(name) - - createOptions := &sdk.CreateAccountOptions{ + opts := &sdk.CreateAccountOptions{ AdminName: d.Get("admin_name").(string), Email: d.Get("email").(string), Edition: sdk.AccountEdition(d.Get("edition").(string)), } - // get optional fields. if v, ok := d.GetOk("admin_password"); ok { - createOptions.AdminPassword = sdk.String(v.(string)) + opts.AdminPassword = sdk.String(v.(string)) } if v, ok := d.GetOk("admin_rsa_public_key"); ok { - createOptions.AdminRSAPublicKey = sdk.String(v.(string)) + opts.AdminRSAPublicKey = sdk.String(v.(string)) + } + if v, ok := d.GetOk("admin_user_type"); ok { + userType, err := sdk.ToUserType(v.(string)) + if err != nil { + return diag.FromErr(err) + } + opts.AdminUserType = &userType } if v, ok := d.GetOk("first_name"); ok { - createOptions.FirstName = sdk.String(v.(string)) + opts.FirstName = sdk.String(v.(string)) } if v, ok := d.GetOk("last_name"); ok { - createOptions.LastName = sdk.String(v.(string)) + opts.LastName = sdk.String(v.(string)) } - - // Has default, don't fetch with GetOk because this can be falsey and valid - v := d.Get("must_change_password") - createOptions.MustChangePassword = sdk.Bool(v.(bool)) - - if v, ok := d.GetOk("region_group"); ok { - createOptions.RegionGroup = sdk.String(v.(string)) - } else { - // For organizations that have accounts in multiple region groups, returns . so we need to split on "." - currentRegion, err := client.ContextFunctions.CurrentRegion(ctx) + if v := d.Get("must_change_password"); v != BooleanDefault { + parsedBool, err := booleanStringToBool(v.(string)) if err != nil { return diag.FromErr(err) } - regionParts := strings.Split(currentRegion, ".") - if len(regionParts) == 2 { - createOptions.RegionGroup = sdk.String(regionParts[0]) - } + opts.MustChangePassword = &parsedBool + } + if v, ok := d.GetOk("region_group"); ok { + opts.RegionGroup = sdk.String(v.(string)) } if v, ok := d.GetOk("region"); ok { - createOptions.Region = sdk.String(v.(string)) - } else { - // For organizations that have accounts in multiple region groups, returns . so we need to split on "." - currentRegion, err := client.ContextFunctions.CurrentRegion(ctx) + opts.Region = sdk.String(v.(string)) + } + if v, ok := d.GetOk("comment"); ok { + opts.Comment = sdk.String(v.(string)) + } + if v := d.Get("polaris"); v != BooleanDefault { + parsedBool, err := booleanStringToBool(v.(string)) if err != nil { return diag.FromErr(err) } - regionParts := strings.Split(currentRegion, ".") - if len(regionParts) == 2 { - createOptions.Region = sdk.String(regionParts[1]) - } else { - createOptions.Region = sdk.String(currentRegion) - } - } - if v, ok := d.GetOk("comment"); ok { - createOptions.Comment = sdk.String(v.(string)) + opts.Polaris = &parsedBool } - err := client.Accounts.Create(ctx, objectIdentifier, createOptions) + createResponse, err := client.Accounts.Create(ctx, id, opts) if err != nil { return diag.FromErr(err) } - var account *sdk.Account - err = util.Retry(5, 3*time.Second, func() (error, bool) { - account, err = client.Accounts.ShowByID(ctx, objectIdentifier) - if err != nil { - log.Printf("[DEBUG] retryable operation resulted in error: %v\n", err) - return nil, false - } - return nil, true - }) - if err != nil { - return diag.FromErr(err) - } + d.SetId(helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(createResponse.OrganizationName, createResponse.AccountName))) - d.SetId(helpers.EncodeSnowflakeID(account.AccountLocator)) return ReadAccount(ctx, d, meta) } -// ReadAccount implements schema.ReadFunc. -func ReadAccount(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func ReadAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) - - var acc *sdk.Account - var err error - err = util.Retry(5, 3*time.Second, func() (error, bool) { - acc, err = client.Accounts.ShowByID(ctx, id) - if err != nil { - log.Printf("[DEBUG] retryable operation resulted in error: %v\n", err) - return nil, false - } - return nil, true - }) + id, err := sdk.ParseAccountIdentifier(d.Id()) if err != nil { return diag.FromErr(err) } - if err := d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()); err != nil { + account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id.AccountName())) + if err != nil { return diag.FromErr(err) } - if err = d.Set("name", acc.AccountName); err != nil { - return diag.FromErr(fmt.Errorf("error setting name: %w", err)) - } - - if err = d.Set("edition", acc.Edition); err != nil { - return diag.FromErr(fmt.Errorf("error setting edition: %w", err)) - } - - if err = d.Set("region_group", acc.RegionGroup); err != nil { - return diag.FromErr(fmt.Errorf("error setting region_group: %w", err)) - } - - if err = d.Set("region", acc.SnowflakeRegion); err != nil { - return diag.FromErr(fmt.Errorf("error setting region: %w", err)) - } - - if err = d.Set("comment", acc.Comment); err != nil { - return diag.FromErr(fmt.Errorf("error setting comment: %w", err)) + accountParameters, err := client.Accounts.ShowParameters(ctx) + if err != nil { + return diag.FromErr(err) } - if err = d.Set("is_org_admin", acc.IsOrgAdmin); err != nil { - return diag.FromErr(fmt.Errorf("error setting is_org_admin: %w", err)) + if errs := errors.Join( + d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), + ); errs != nil { + return diag.FromErr(errs) } return nil } -// UpdateAccount implements schema.UpdateFunc. -func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { /* todo: comments may eventually work again for accounts, so this can be uncommented when that happens client := meta.(*provider.Context).Client @@ -393,12 +466,21 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta interface{} return nil } -// DeleteAccount implements schema.DeleteFunc. -func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - gracePeriodInDays := d.Get("grace_period_in_days").(int) - err := client.Accounts.Drop(ctx, helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier), gracePeriodInDays, &sdk.DropAccountOptions{ + id, err := sdk.ParseAccountObjectIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } + + err = client.Accounts.Drop(ctx, id, d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{ IfExists: sdk.Bool(true), }) - return diag.FromErr(err) + if err != nil { + return diag.FromErr(err) + } + + d.SetId("") + + return nil } diff --git a/pkg/schemas/account_gen.go b/pkg/schemas/account_gen.go index 715e1fb9cf..5ef4c855c3 100644 --- a/pkg/schemas/account_gen.go +++ b/pkg/schemas/account_gen.go @@ -17,11 +17,11 @@ var ShowAccountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Computed: true, }, - "region_group": { + "snowflake_region": { Type: schema.TypeString, Computed: true, }, - "snowflake_region": { + "region_group": { Type: schema.TypeString, Computed: true, }, @@ -73,6 +73,58 @@ var ShowAccountSchema = map[string]*schema.Schema{ Type: schema.TypeBool, Computed: true, }, + "account_old_url_saved_on": { + Type: schema.TypeString, + Computed: true, + }, + "account_old_url_last_used": { + Type: schema.TypeString, + Computed: true, + }, + "organization_old_url": { + Type: schema.TypeString, + Computed: true, + }, + "organization_old_url_saved_on": { + Type: schema.TypeString, + Computed: true, + }, + "organization_old_url_last_used": { + Type: schema.TypeString, + Computed: true, + }, + "is_events_account": { + Type: schema.TypeBool, + Computed: true, + }, + "is_organization_account": { + Type: schema.TypeBool, + Computed: true, + }, + "dropped_on": { + Type: schema.TypeString, + Computed: true, + }, + "scheduled_deletion_time": { + Type: schema.TypeString, + Computed: true, + }, + "restored_on": { + Type: schema.TypeString, + Computed: true, + }, + "moved_to_organization": { + Type: schema.TypeString, + Computed: true, + }, + "moved_on": { + Type: schema.TypeString, + Computed: true, + }, + "organization_url_expiration_on": { + Type: schema.TypeString, + Computed: true, + }, } var _ = ShowAccountSchema @@ -81,20 +133,82 @@ func AccountToSchema(account *sdk.Account) map[string]any { accountSchema := make(map[string]any) accountSchema["organization_name"] = account.OrganizationName accountSchema["account_name"] = account.AccountName - accountSchema["region_group"] = account.RegionGroup accountSchema["snowflake_region"] = account.SnowflakeRegion - accountSchema["edition"] = account.Edition - accountSchema["account_url"] = account.AccountURL - accountSchema["created_on"] = account.CreatedOn.String() - accountSchema["comment"] = account.Comment + if account.RegionGroup != nil { + accountSchema["region_group"] = account.RegionGroup + } + if account.Edition != nil { + // Manually modified, please don't re-generate + accountSchema["edition"] = string(*account.Edition) + } + if account.AccountURL != nil { + accountSchema["account_url"] = account.AccountURL + } + if account.CreatedOn != nil { + accountSchema["created_on"] = account.CreatedOn.String() + } + if account.Comment != nil { + accountSchema["comment"] = account.Comment + } accountSchema["account_locator"] = account.AccountLocator - accountSchema["account_locator_url"] = account.AccountLocatorURL - accountSchema["managed_accounts"] = account.ManagedAccounts - accountSchema["consumption_billing_entity_name"] = account.ConsumptionBillingEntityName - accountSchema["marketplace_consumer_billing_entity_name"] = account.MarketplaceConsumerBillingEntityName - accountSchema["marketplace_provider_billing_entity_name"] = account.MarketplaceProviderBillingEntityName - accountSchema["old_account_url"] = account.OldAccountURL - accountSchema["is_org_admin"] = account.IsOrgAdmin + if account.AccountLocatorURL != nil { + accountSchema["account_locator_url"] = account.AccountLocatorURL + } + if account.ManagedAccounts != nil { + accountSchema["managed_accounts"] = account.ManagedAccounts + } + if account.ConsumptionBillingEntityName != nil { + accountSchema["consumption_billing_entity_name"] = account.ConsumptionBillingEntityName + } + if account.MarketplaceConsumerBillingEntityName != nil { + accountSchema["marketplace_consumer_billing_entity_name"] = account.MarketplaceConsumerBillingEntityName + } + if account.MarketplaceProviderBillingEntityName != nil { + accountSchema["marketplace_provider_billing_entity_name"] = account.MarketplaceProviderBillingEntityName + } + if account.OldAccountURL != nil { + accountSchema["old_account_url"] = account.OldAccountURL + } + if account.IsOrgAdmin != nil { + accountSchema["is_org_admin"] = account.IsOrgAdmin + } + if account.AccountOldUrlSavedOn != nil { + accountSchema["account_old_url_saved_on"] = account.AccountOldUrlSavedOn.String() + } + if account.AccountOldUrlLastUsed != nil { + accountSchema["account_old_url_last_used"] = account.AccountOldUrlLastUsed.String() + } + if account.OrganizationOldUrl != nil { + accountSchema["organization_old_url"] = account.OrganizationOldUrl + } + if account.OrganizationOldUrlSavedOn != nil { + accountSchema["organization_old_url_saved_on"] = account.OrganizationOldUrlSavedOn.String() + } + if account.OrganizationOldUrlLastUsed != nil { + accountSchema["organization_old_url_last_used"] = account.OrganizationOldUrlLastUsed.String() + } + if account.IsEventsAccount != nil { + accountSchema["is_events_account"] = account.IsEventsAccount + } + accountSchema["is_organization_account"] = account.IsOrganizationAccount + if account.DroppedOn != nil { + accountSchema["dropped_on"] = account.DroppedOn.String() + } + if account.ScheduledDeletionTime != nil { + accountSchema["scheduled_deletion_time"] = account.ScheduledDeletionTime.String() + } + if account.RestoredOn != nil { + accountSchema["restored_on"] = account.RestoredOn.String() + } + if account.MovedToOrganization != nil { + accountSchema["moved_to_organization"] = account.MovedToOrganization + } + if account.MovedOn != nil { + accountSchema["moved_on"] = account.MovedOn + } + if account.OrganizationUrlExpirationOn != nil { + accountSchema["organization_url_expiration_on"] = account.OrganizationUrlExpirationOn.String() + } return accountSchema } diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 00557cba7a..03e5e5a61d 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -3,7 +3,10 @@ package sdk import ( "context" "database/sql" + "encoding/json" "errors" + "github.com/snowflakedb/gosnowflake" + "strings" "time" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" @@ -16,7 +19,7 @@ var ( ) type Accounts interface { - Create(ctx context.Context, id AccountObjectIdentifier, opts *CreateAccountOptions) error + Create(ctx context.Context, id AccountObjectIdentifier, opts *CreateAccountOptions) (*AccountCreateResponse, error) Alter(ctx context.Context, opts *AlterAccountOptions) error Show(ctx context.Context, opts *ShowAccountOptions) ([]Account, error) ShowByID(ctx context.Context, id AccountObjectIdentifier) (*Account, error) @@ -81,12 +84,55 @@ func (opts *CreateAccountOptions) validate() error { return errors.Join(errs...) } -func (c *accounts) Create(ctx context.Context, id AccountObjectIdentifier, opts *CreateAccountOptions) error { +type AccountCreateResponse struct { + AccountLocator string `json:"accountLocator,omitempty"` + AccountLocatorUrl string `json:"accountLocatorUrl,omitempty"` + OrganizationName string + AccountName string `json:"accountName,omitempty"` + Url string `json:"url,omitempty"` + Edition AccountEdition `json:"edition,omitempty"` + RegionGroup string `json:"regionGroup,omitempty"` + Cloud string `json:"cloud,omitempty"` + Region string `json:"region,omitempty"` +} + +func ToAccountCreateResponse(v string) (*AccountCreateResponse, error) { + var res AccountCreateResponse + err := json.Unmarshal([]byte(v), &res) + if err != nil { + return nil, err + } + if len(res.Url) > 0 { + url := strings.TrimPrefix(res.Url, `https://`) + url = strings.TrimPrefix(url, `http://`) + parts := strings.SplitN(url, "-", 2) + if len(parts) == 2 { + res.OrganizationName = strings.ToUpper(parts[0]) + } + } + return &res, nil +} + +func (c *accounts) Create(ctx context.Context, id AccountObjectIdentifier, opts *CreateAccountOptions) (*AccountCreateResponse, error) { if opts == nil { opts = &CreateAccountOptions{} } opts.name = id - return validateAndExec(c.client, ctx, opts) + queryChanId := make(chan string, 1) + err := validateAndExec(c.client, gosnowflake.WithQueryIDChan(ctx, queryChanId), opts) + if err != nil { + return nil, err + } + + queryId := <-queryChanId + rows, err := c.client.QueryUnsafe(gosnowflake.WithFetchResultByID(ctx, queryId), "") + if len(rows) == 1 && rows[0]["status"] != nil { + if status, ok := (*rows[0]["status"]).(string); ok { + return ToAccountCreateResponse(status) + } + } + + return nil, nil } // AlterAccountOptions is based on https://docs.snowflake.com/en/sql-reference/sql/alter-account. diff --git a/pkg/sdk/accounts_test.go b/pkg/sdk/accounts_test.go index d275c82883..8faca70e9c 100644 --- a/pkg/sdk/accounts_test.go +++ b/pkg/sdk/accounts_test.go @@ -1,7 +1,9 @@ package sdk import ( + "encoding/json" "fmt" + "github.com/stretchr/testify/assert" "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -404,3 +406,116 @@ func TestAccountShow(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, `SHOW ACCOUNTS LIKE 'myaccount'`) }) } + +func TestToAccountCreateResponse(t *testing.T) { + testCases := []struct { + Name string + RawInput string + Input AccountCreateResponse + ExpectedOutput *AccountCreateResponse + Error string + }{ + { + Name: "validation: empty input", + RawInput: "", + Error: "unexpected end of JSON input", + }, + { + Name: "validation: only a few fields filled", + Input: AccountCreateResponse{ + AccountName: "acc_name", + Url: `https://org_name-acc_name.snowflakecomputing.com`, + Edition: EditionStandard, + RegionGroup: "region_group", + Cloud: "cloud", + Region: "region", + }, + ExpectedOutput: &AccountCreateResponse{ + AccountName: "acc_name", + Url: `https://org_name-acc_name.snowflakecomputing.com`, + OrganizationName: "ORG_NAME", + Edition: EditionStandard, + RegionGroup: "region_group", + Cloud: "cloud", + Region: "region", + }, + }, + { + Name: "validation: invalid url", + Input: AccountCreateResponse{ + Url: `https://org_name_acc_name.snowflake.computing.com`, + }, + ExpectedOutput: &AccountCreateResponse{ + Url: `https://org_name_acc_name.snowflake.computing.com`, + // OrganizationName is not filled + }, + }, + { + Name: "validation: valid url", + Input: AccountCreateResponse{ + Url: `https://org_name-acc_name.snowflakecomputing.com`, + }, + ExpectedOutput: &AccountCreateResponse{ + Url: `https://org_name-acc_name.snowflakecomputing.com`, + OrganizationName: "ORG_NAME", + }, + }, + { + Name: "validation: valid http url", + Input: AccountCreateResponse{ + Url: `http://org_name-acc_name.snowflakecomputing.com`, + }, + ExpectedOutput: &AccountCreateResponse{ + Url: `http://org_name-acc_name.snowflakecomputing.com`, + OrganizationName: "ORG_NAME", + }, + }, + { + Name: "complete", + Input: AccountCreateResponse{ + AccountLocator: "locator", + AccountLocatorUrl: "locator_url", + AccountName: "acc_name", + Url: `https://org_name-acc_name.snowflakecomputing.com`, + Edition: EditionBusinessCritical, + RegionGroup: "region_group", + Cloud: "cloud", + Region: "region", + }, + ExpectedOutput: &AccountCreateResponse{ + AccountLocator: "locator", + AccountLocatorUrl: "locator_url", + AccountName: "acc_name", + Url: `https://org_name-acc_name.snowflakecomputing.com`, + OrganizationName: "ORG_NAME", + Edition: EditionBusinessCritical, + RegionGroup: "region_group", + Cloud: "cloud", + Region: "region", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + input := tc.RawInput + if tc.Input != (AccountCreateResponse{}) { + bytes, err := json.Marshal(tc.Input) + if err != nil { + assert.Fail(t, err.Error()) + } + input = string(bytes) + } + + createResponse, err := ToAccountCreateResponse(input) + + if tc.Error != "" { + assert.EqualError(t, err, tc.Error) + assert.Nil(t, createResponse) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.ExpectedOutput, createResponse) + } + }) + } +} diff --git a/pkg/sdk/testint/accounts_integration_test.go b/pkg/sdk/testint/accounts_integration_test.go index c4c864e445..4b8bea8ba6 100644 --- a/pkg/sdk/testint/accounts_integration_test.go +++ b/pkg/sdk/testint/accounts_integration_test.go @@ -92,13 +92,30 @@ func TestInt_Account(t *testing.T) { assert.Nil(t, account.OrganizationUrlExpirationOn) } + 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) + // TODO: Rename to Url + 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) { id := testClientHelper().Ids.RandomAccountObjectIdentifier() name := testClientHelper().Ids.Alpha() password := random.Password() email := random.Email() - err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ + createResponse, err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ AdminName: name, AdminPassword: sdk.String(password), Email: email, @@ -110,6 +127,7 @@ func TestInt_Account(t *testing.T) { acc, err := client.Accounts.ShowByID(ctx, id) require.NoError(t, err) require.Equal(t, id, acc.ID()) + assertCreateResponse(t, createResponse, *acc) }) t.Run("create: user type service", func(t *testing.T) { @@ -118,7 +136,7 @@ func TestInt_Account(t *testing.T) { key, _ := random.GenerateRSAPublicKey(t) email := random.Email() - err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ + createResponse, err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ AdminName: name, AdminRSAPublicKey: sdk.String(key), AdminUserType: sdk.Pointer(sdk.UserTypeService), @@ -131,6 +149,7 @@ func TestInt_Account(t *testing.T) { acc, err := client.Accounts.ShowByID(ctx, id) require.NoError(t, err) require.Equal(t, id, acc.ID()) + assertCreateResponse(t, createResponse, *acc) }) t.Run("create: user type legacy service", func(t *testing.T) { @@ -139,7 +158,7 @@ func TestInt_Account(t *testing.T) { password := random.Password() email := random.Email() - err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ + createResponse, err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ AdminName: name, AdminPassword: sdk.String(password), AdminUserType: sdk.Pointer(sdk.UserTypeLegacyService), @@ -152,6 +171,7 @@ func TestInt_Account(t *testing.T) { acc, err := client.Accounts.ShowByID(ctx, id) require.NoError(t, err) require.Equal(t, id, acc.ID()) + assertCreateResponse(t, createResponse, *acc) }) t.Run("create: complete", func(t *testing.T) { @@ -167,7 +187,7 @@ func TestInt_Account(t *testing.T) { require.NoError(t, err) comment := random.Comment() - err = client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ + createResponse, err := client.Accounts.Create(ctx, id, &sdk.CreateAccountOptions{ AdminName: name, AdminPassword: sdk.String(password), FirstName: sdk.String("firstName"), @@ -187,6 +207,7 @@ func TestInt_Account(t *testing.T) { acc, err := client.Accounts.ShowByID(ctx, id) require.NoError(t, err) require.Equal(t, id, acc.ID()) + assertCreateResponse(t, createResponse, *acc) }) t.Run("alter: set / unset is org admin", func(t *testing.T) { From 36e9db8778f3acc18a2c1aa51d1c7db75e6f1606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Wed, 4 Dec 2024 07:16:45 +0100 Subject: [PATCH 03/12] wip --- pkg/resources/account.go | 41 ++++++++++++--- pkg/resources/resource_helpers_read.go | 11 ++++ pkg/schemas/account_parameters.go | 71 ++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 8 deletions(-) create mode 100644 pkg/schemas/account_parameters.go diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 1d85acafca..0b0b11e9c0 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -210,20 +210,22 @@ var accountSchema = map[string]*schema.Schema{ "name": { Type: schema.TypeString, Required: true, + // TODO: Sensitive? Description: "TODO", ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), }, "admin_name": { Type: schema.TypeString, Required: true, - Description: "TODO", + // TODO: Sensitive? + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, }, "admin_password": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, }, @@ -231,7 +233,7 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, }, @@ -239,7 +241,7 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, // TODO: Valid options - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: SuppressIfAny(IgnoreAfterCreation, NormalizeAndCompare(sdk.ToUserType)), ValidateDiagFunc: sdkValidation(sdk.ToUserType), }, @@ -247,28 +249,28 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, }, "last_name": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: "", + Description: externalChangesNotDetectedFieldDescription("TODO") DiffSuppressFunc: IgnoreAfterCreation, }, "email": { Type: schema.TypeString, Required: true, Sensitive: true, - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, }, "must_change_password": { Type: schema.TypeString, Optional: true, Default: BooleanDefault, - Description: "TODO", + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, ValidateDiagFunc: validateBooleanString, }, @@ -433,7 +435,30 @@ func ReadAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Dia } if errs := errors.Join( + attributeMappedValueReadOrDefault(d, "edition", account.Edition, func(edition *sdk.AccountEdition) (string, error) { + if edition != nil { + return string(*edition), nil + } + return "", nil + }, nil), + // TODO: use SHOW REGIONS? + // TODO: Should region group be read ? + // TODO: Should region be read ? + // TODO: There's default SNOWFLAKE comment + attributeMappedValueReadOrNil(d, "comment", account.Comment, func(comment *string) (string, error) { + if comment != nil { + return *comment, nil + } + return "", nil + }), + attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { + if isOrgAdmin != nil { + return booleanStringFromBool(*isOrgAdmin), nil + } + return BooleanDefault, nil + }), d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), + d.Set(ParametersAttributeName, []map[string]any{schemas.AccountParametersToSchema(accountParameters)}), ); errs != nil { return diag.FromErr(errs) } diff --git a/pkg/resources/resource_helpers_read.go b/pkg/resources/resource_helpers_read.go index b3dcfcebf1..72ee47daa6 100644 --- a/pkg/resources/resource_helpers_read.go +++ b/pkg/resources/resource_helpers_read.go @@ -50,6 +50,17 @@ func setBooleanStringFromBoolProperty(d *schema.ResourceData, key string, proper return nil } +func attributeMappedValueReadOrNil[T, R any](d *schema.ResourceData, key string, value *T, mapper func(*T) (R, error)) error { + if value != nil { + mappedValue, err := mapper(value) + if err != nil { + return err + } + return d.Set(key, mappedValue) + } + return d.Set(key, nil) +} + func attributeMappedValueReadOrDefault[T, R any](d *schema.ResourceData, key string, value *T, mapper func(*T) (R, error), defaultValue *R) error { if value != nil { mappedValue, err := mapper(value) diff --git a/pkg/schemas/account_parameters.go b/pkg/schemas/account_parameters.go new file mode 100644 index 0000000000..e5885b7967 --- /dev/null +++ b/pkg/schemas/account_parameters.go @@ -0,0 +1,71 @@ +package schemas + +import ( + "slices" + "strings" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +var ( + ShowAccountParametersSchema = make(map[string]*schema.Schema) + accountParameters = []sdk.AccountParameter{ + // TODO(SNOW-1348092 - next prs): Add parameters + // session parameters + sdk.AccountParameterAbortDetachedQuery, + sdk.AccountParameterAutocommit, + sdk.AccountParameterBinaryInputFormat, + sdk.AccountParameterBinaryOutputFormat, + sdk.AccountParameterClientMetadataRequestUseConnectionCtx, + sdk.AccountParameterClientResultColumnCaseInsensitive, + sdk.AccountParameterDateInputFormat, + sdk.AccountParameterDateOutputFormat, + sdk.AccountParameterErrorOnNondeterministicMerge, + sdk.AccountParameterErrorOnNondeterministicUpdate, + sdk.AccountParameterGeographyOutputFormat, + sdk.AccountParameterLockTimeout, + sdk.AccountParameterLogLevel, + sdk.AccountParameterMultiStatementCount, + sdk.AccountParameterQueryTag, + sdk.AccountParameterQuotedIdentifiersIgnoreCase, + sdk.AccountParameterRowsPerResultset, + sdk.AccountParameterS3StageVpceDnsName, + sdk.AccountParameterStatementQueuedTimeoutInSeconds, + sdk.AccountParameterStatementTimeoutInSeconds, + sdk.AccountParameterTimestampDayIsAlways24h, + sdk.AccountParameterTimestampInputFormat, + sdk.AccountParameterTimestampLtzOutputFormat, + sdk.AccountParameterTimestampNtzOutputFormat, + sdk.AccountParameterTimestampOutputFormat, + sdk.AccountParameterTimestampTypeMapping, + sdk.AccountParameterTimestampTzOutputFormat, + sdk.AccountParameterTimezone, + sdk.AccountParameterTimeInputFormat, + sdk.AccountParameterTimeOutputFormat, + sdk.AccountParameterTraceLevel, + sdk.AccountParameterTransactionAbortOnError, + sdk.AccountParameterTransactionDefaultIsolationLevel, + sdk.AccountParameterTwoDigitCenturyStart, + sdk.AccountParameterUnsupportedDdlAction, + sdk.AccountParameterUseCachedResult, + sdk.AccountParameterWeekOfYearPolicy, + sdk.AccountParameterWeekStart, + } +) + +func init() { + for _, param := range accountParameters { + ShowAccountParametersSchema[strings.ToLower(string(param))] = ParameterListSchema + } +} + +func AccountParametersToSchema(parameters []*sdk.Parameter) map[string]any { + accountParametersValue := make(map[string]any) + for _, param := range parameters { + if slices.Contains(accountParameters, sdk.AccountParameter(param.Key)) { + accountParametersValue[strings.ToLower(param.Key)] = []map[string]any{ParameterToSchema(param)} + } + } + return accountParametersValue +} From 99e499ab857f1ad837b665cc44b976191f94b4c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Wed, 4 Dec 2024 11:57:08 +0100 Subject: [PATCH 04/12] wip --- pkg/resources/account.go | 161 ++++++++++++----------- pkg/resources/account_acceptance_test.go | 122 ++++++++--------- pkg/schemas/account_gen.go | 1 + pkg/sdk/accounts.go | 11 ++ pkg/sdk/type_helpers.go | 11 ++ 5 files changed, 173 insertions(+), 133 deletions(-) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 0b0b11e9c0..47821954f8 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -208,15 +208,15 @@ import ( var accountSchema = map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, // TODO: Sensitive? Description: "TODO", ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), }, "admin_name": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, // TODO: Sensitive? Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, @@ -256,7 +256,7 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO") + Description: externalChangesNotDetectedFieldDescription("TODO"), DiffSuppressFunc: IgnoreAfterCreation, }, "email": { @@ -275,25 +275,23 @@ var accountSchema = map[string]*schema.Schema{ ValidateDiagFunc: validateBooleanString, }, "edition": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - // TODO: Desc - Description: "[Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values are: STANDARD | ENTERPRISE | BUSINESS_CRITICAL", - // TODO: Valid options - ValidateFunc: validation.StringInSlice([]string{string(sdk.EditionStandard), string(sdk.EditionEnterprise), string(sdk.EditionBusinessCritical)}, false), + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "TODO", + ValidateDiagFunc: sdkValidation(sdk.ToAccountEdition), }, "region_group": { Type: schema.TypeString, Optional: true, ForceNew: true, - Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", + Description: "TODO", }, "region": { Type: schema.TypeString, Optional: true, ForceNew: true, - Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", + Description: "TODO", }, "comment": { Type: schema.TypeString, @@ -323,15 +321,6 @@ var accountSchema = map[string]*schema.Schema{ Schema: schemas.ShowAccountSchema, }, }, - // TODO: This one will be pretty big (possibly over 200 parameters) - //ParametersAttributeName: { - // Type: schema.TypeList, - // Computed: true, - // Description: "Outputs the result of `SHOW PARAMETERS IN TASK` for the given task.", - // Elem: &schema.Resource{ - // Schema: schemas.ShowTaskParametersSchema, - // }, - //}, } func Account() *schema.Resource { @@ -339,18 +328,21 @@ func Account() *schema.Resource { // TODO: Desc Description: "The account resource allows you to create and manage Snowflake accounts.", CreateContext: TrackingCreateWrapper(resources.Account, CreateAccount), - ReadContext: TrackingReadWrapper(resources.Account, ReadAccount), + ReadContext: TrackingReadWrapper(resources.Account, ReadAccount(true)), UpdateContext: TrackingUpdateWrapper(resources.Account, UpdateAccount), DeleteContext: TrackingDeleteWrapper(resources.Account, DeleteAccount), CustomizeDiff: TrackingCustomDiffWrapper(resources.Account, customdiff.All( ComputedIfAnyAttributeChanged(accountSchema, FullyQualifiedNameAttributeName, "name"), + ComputedIfAnyAttributeChanged(accountSchema, ShowOutputAttributeName, "name", "is_org_admin"), )), Schema: accountSchema, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, + + // TODO: State upgrader and import } } @@ -399,13 +391,15 @@ func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D if v, ok := d.GetOk("comment"); ok { opts.Comment = sdk.String(v.(string)) } - if v := d.Get("polaris"); v != BooleanDefault { - parsedBool, err := booleanStringToBool(v.(string)) - if err != nil { - return diag.FromErr(err) - } - opts.Polaris = &parsedBool - } + + // TODO(TODO): next prs + //if v := d.Get("polaris"); v != BooleanDefault { + // parsedBool, err := booleanStringToBool(v.(string)) + // if err != nil { + // return diag.FromErr(err) + // } + // opts.Polaris = &parsedBool + //} createResponse, err := client.Accounts.Create(ctx, id, opts) if err != nil { @@ -414,56 +408,75 @@ func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D d.SetId(helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(createResponse.OrganizationName, createResponse.AccountName))) - return ReadAccount(ctx, d, meta) + return ReadAccount(false)(ctx, d, meta) } -func ReadAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*provider.Context).Client - id, err := sdk.ParseAccountIdentifier(d.Id()) - if err != nil { - return diag.FromErr(err) - } - - account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id.AccountName())) - if err != nil { - return diag.FromErr(err) - } +func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { + return func(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + client := meta.(*provider.Context).Client + id, err := sdk.ParseAccountIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } - accountParameters, err := client.Accounts.ShowParameters(ctx) - if err != nil { - return diag.FromErr(err) - } + account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id.AccountName())) + if err != nil { + return diag.FromErr(err) + } - if errs := errors.Join( - attributeMappedValueReadOrDefault(d, "edition", account.Edition, func(edition *sdk.AccountEdition) (string, error) { - if edition != nil { - return string(*edition), nil - } - return "", nil - }, nil), - // TODO: use SHOW REGIONS? - // TODO: Should region group be read ? - // TODO: Should region be read ? - // TODO: There's default SNOWFLAKE comment - attributeMappedValueReadOrNil(d, "comment", account.Comment, func(comment *string) (string, error) { - if comment != nil { - return *comment, nil + if withExternalChangesMarking { + if err = handleExternalChangesToObjectInShow(d, + outputMapping{"region_group", "region_group", account.RegionGroup, sdk.DerefIfNotNil(account.RegionGroup), sdk.DerefIfNotNil}, + outputMapping{"snowflake_region", "region", account.SnowflakeRegion, account.SnowflakeRegion, nil}, + outputMapping{"comment", "comment", account.Comment, sdk.DerefIfNotNil(account.Comment), sdk.DerefIfNotNil}, + ); err != nil { + return diag.FromErr(err) } - return "", nil - }), - attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { - if isOrgAdmin != nil { - return booleanStringFromBool(*isOrgAdmin), nil + } else { + if err = setStateToValuesFromConfig(d, taskSchema, []string{ + "allow_overlapping_execution", + }); err != nil { + return diag.FromErr(err) } - return BooleanDefault, nil - }), - d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), - d.Set(ParametersAttributeName, []map[string]any{schemas.AccountParametersToSchema(accountParameters)}), - ); errs != nil { - return diag.FromErr(errs) - } + } - return nil + if errs := errors.Join( + attributeMappedValueReadOrDefault(d, "edition", account.Edition, func(edition *sdk.AccountEdition) (string, error) { + if edition != nil { + return string(*edition), nil + } + return "", nil + }, nil), + // TODO: Region group is only returned when org is span on multiple region groups, but you can explicitly set it (e.g. PUBLIC) + //attributeMappedValueReadOrNil(d, "region_group", account.RegionGroup, func(regionGroup *string) (string, error) { + // if regionGroup != nil { + // return *regionGroup, nil + // } + // return "", nil + //}), + // TODO: Can be left empty and it will be populated with current account's region + //d.Set("region", account.SnowflakeRegion), + // TODO: Default comment is "SNOWFLAKE" + //attributeMappedValueReadOrNil(d, "comment", account.Comment, func(comment *string) (string, error) { + // if comment != nil { + // return *comment, nil + // } + // return "", nil + //}), + attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { + if isOrgAdmin != nil { + return booleanStringFromBool(*isOrgAdmin), nil + } + return BooleanDefault, nil + }), + d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), + d.Set(ShowOutputAttributeName, []map[string]any{schemas.AccountToSchema(account)}), + ); errs != nil { + return diag.FromErr(errs) + } + + return nil + } } func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index ceb1a5df64..af7ff0de21 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -1,7 +1,13 @@ package resources_test import ( - "fmt" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" @@ -13,10 +19,29 @@ import ( ) func TestAcc_Account_complete(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) id := acc.TestClient().Ids.RandomAccountObjectIdentifier() - password := acc.TestClient().Ids.AlphaContaining("123ABC") + firstName := acc.TestClient().Ids.Alpha() + lastName := acc.TestClient().Ids.Alpha() + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + region := acc.TestClient().Context.CurrentRegion(t) + comment := random.Comment() + + configModel := model.Account("test", id.Name(), string(sdk.EditionStandard), email, name). + // TODO: WithAdminUserType() + WithAdminRsaPublicKey(key). + WithFirstName(firstName). + WithLastName(lastName). + WithMustChangePassword(true). + WithRegionGroup("PUBLIC"). + WithRegion(region). + WithComment(comment). + WithIsOrgAdmin(true). + WithGracePeriodInDays(3) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -25,68 +50,47 @@ func TestAcc_Account_complete(t *testing.T) { tfversion.RequireAbove(tfversion.Version1_5_0), }, CheckDestroy: acc.CheckDestroy(t, resources.Account), - // this errors with: Error running post-test destroy, there may be dangling resources: exit status 1 - // unless we change the resource to return nil on destroy then this is unavoidable Steps: []resource.TestStep{ { - Config: accountConfig(id.Name(), password, "Terraform acceptance test", 3), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_account.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_account.test", "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr("snowflake_account.test", "admin_name", "someadmin"), - resource.TestCheckResourceAttr("snowflake_account.test", "first_name", "Ad"), - resource.TestCheckResourceAttr("snowflake_account.test", "last_name", "Min"), - resource.TestCheckResourceAttr("snowflake_account.test", "email", "admin@example.com"), - resource.TestCheckResourceAttr("snowflake_account.test", "must_change_password", "false"), - resource.TestCheckResourceAttr("snowflake_account.test", "edition", "BUSINESS_CRITICAL"), - resource.TestCheckResourceAttr("snowflake_account.test", "comment", "Terraform acceptance test"), - resource.TestCheckResourceAttr("snowflake_account.test", "grace_period_in_days", "3"), + Config: config.FromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModel.ResourceReference()). + HasNameString(id.Name()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasAdminNameString(name). + HasAdminRsaPublicKeyString(key). + HasEmailString(email). + HasFirstNameString(firstName). + HasLastNameString(lastName). + HasMustChangePasswordString(r.BooleanTrue). + HasRegionGroupString("PUBLIC"). + HasRegionString(region). + HasCommentString(comment). + HasIsOrgAdminString(r.BooleanTrue). + HasGracePeriodInDaysString("3"), + // TODO: Show output ), - Destroy: false, - }, - // Change Grace Period In Days - { - Config: accountConfig(id.Name(), password, "Terraform acceptance test", 4), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_account.test", "grace_period_in_days", "4"), - ), - }, - // IMPORT - { - ResourceName: "snowflake_account.test", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "admin_name", - "admin_password", - "admin_rsa_public_key", - "email", - "must_change_password", - "first_name", - "last_name", - "grace_period_in_days", - }, }, }, }) } -func accountConfig(name string, password string, comment string, gracePeriodInDays int) string { - return fmt.Sprintf(` -data "snowflake_current_account" "current" {} - -resource "snowflake_account" "test" { - name = "%s" - admin_name = "someadmin" - admin_password = "%s" - first_name = "Ad" - last_name = "Min" - email = "admin@example.com" - must_change_password = false - edition = "BUSINESS_CRITICAL" - comment = "%s" - region = data.snowflake_current_account.current.region - grace_period_in_days = %d -} -`, name, password, comment, gracePeriodInDays) -} +//func accountConfig(name string, password string, comment string, gracePeriodInDays int) string { +// return fmt.Sprintf(` +//data "snowflake_current_account" "current" {} +// +//resource "snowflake_account" "test" { +// name = "%s" +// admin_name = "someadmin" +// admin_password = "%s" +// first_name = "Ad" +// last_name = "Min" +// email = "admin@example.com" +// must_change_password = false +// edition = "BUSINESS_CRITICAL" +// comment = "%s" +// region = data.snowflake_current_account.current.region +// grace_period_in_days = %d +//} +//`, name, password, comment, gracePeriodInDays) +//} diff --git a/pkg/schemas/account_gen.go b/pkg/schemas/account_gen.go index 5ef4c855c3..03be3a920e 100644 --- a/pkg/schemas/account_gen.go +++ b/pkg/schemas/account_gen.go @@ -134,6 +134,7 @@ func AccountToSchema(account *sdk.Account) map[string]any { accountSchema["organization_name"] = account.OrganizationName accountSchema["account_name"] = account.AccountName accountSchema["snowflake_region"] = account.SnowflakeRegion + // TODO: Check if populated or have to deref if account.RegionGroup != nil { accountSchema["region_group"] = account.RegionGroup } diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 03e5e5a61d..02a22a6049 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "errors" + "fmt" "github.com/snowflakedb/gosnowflake" "strings" "time" @@ -42,6 +43,16 @@ var ( EditionBusinessCritical AccountEdition = "BUSINESS_CRITICAL" ) +// TODO: test +func ToAccountEdition(edition string) (AccountEdition, error) { + switch typedEdition := AccountEdition(strings.ToUpper(edition)); typedEdition { + case EditionStandard, EditionEnterprise, EditionBusinessCritical: + return typedEdition, nil + default: + return "", fmt.Errorf("unknown account edition: %s", edition) + } +} + // CreateAccountOptions is based on https://docs.snowflake.com/en/sql-reference/sql/create-account. type CreateAccountOptions struct { create bool `ddl:"static" sql:"CREATE"` diff --git a/pkg/sdk/type_helpers.go b/pkg/sdk/type_helpers.go index e1c7034b9c..d1a13a57a3 100644 --- a/pkg/sdk/type_helpers.go +++ b/pkg/sdk/type_helpers.go @@ -55,3 +55,14 @@ func ToFloat64(s string) float64 { func Pointer[K any](v K) *K { return &v } + +// TODO: Test +// DerefIfNotNil is a generic function that returns a dereferenced value if a given value is not nil +func DerefIfNotNil(v any) any { + if v != nil { + if pointerValue, ok := v.(*any); ok { + return *pointerValue + } + } + return nil +} From 886cad6f9e24d13dbdec558339984d994ece9ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Thu, 5 Dec 2024 08:10:19 +0100 Subject: [PATCH 05/12] wip --- .../config/model/account_model_gen.go | 27 +++- .../helpers/random/random_helpers.go | 3 +- pkg/resources/account.go | 100 ++++++++++-- pkg/resources/account_acceptance_test.go | 153 ++++++++++++++---- pkg/resources/resource_helpers_read.go | 11 -- pkg/sdk/type_helpers.go | 11 -- 6 files changed, 234 insertions(+), 71 deletions(-) diff --git a/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go index 87ddf58d4e..8b616f90de 100644 --- a/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go +++ b/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go @@ -13,6 +13,7 @@ type AccountModel struct { AdminName tfconfig.Variable `json:"admin_name,omitempty"` AdminPassword tfconfig.Variable `json:"admin_password,omitempty"` AdminRsaPublicKey tfconfig.Variable `json:"admin_rsa_public_key,omitempty"` + AdminUserType tfconfig.Variable `json:"admin_user_type,omitempty"` Comment tfconfig.Variable `json:"comment,omitempty"` Edition tfconfig.Variable `json:"edition,omitempty"` Email tfconfig.Variable `json:"email,omitempty"` @@ -36,28 +37,36 @@ type AccountModel struct { func Account( resourceName string, adminName string, + adminUserType string, edition string, email string, + gracePeriodInDays int, name string, ) *AccountModel { a := &AccountModel{ResourceModelMeta: config.Meta(resourceName, resources.Account)} a.WithAdminName(adminName) + a.WithAdminUserType(adminUserType) a.WithEdition(edition) a.WithEmail(email) + a.WithGracePeriodInDays(gracePeriodInDays) a.WithName(name) return a } func AccountWithDefaultMeta( adminName string, + adminUserType string, edition string, email string, + gracePeriodInDays int, name string, ) *AccountModel { a := &AccountModel{ResourceModelMeta: config.DefaultMeta(resources.Account)} a.WithAdminName(adminName) + a.WithAdminUserType(adminUserType) a.WithEdition(edition) a.WithEmail(email) + a.WithGracePeriodInDays(gracePeriodInDays) a.WithName(name) return a } @@ -81,6 +90,11 @@ func (a *AccountModel) WithAdminRsaPublicKey(adminRsaPublicKey string) *AccountM return a } +func (a *AccountModel) WithAdminUserType(adminUserType string) *AccountModel { + a.AdminUserType = tfconfig.StringVariable(adminUserType) + return a +} + func (a *AccountModel) WithComment(comment string) *AccountModel { a.Comment = tfconfig.StringVariable(comment) return a @@ -111,8 +125,8 @@ func (a *AccountModel) WithGracePeriodInDays(gracePeriodInDays int) *AccountMode return a } -func (a *AccountModel) WithIsOrgAdmin(isOrgAdmin bool) *AccountModel { - a.IsOrgAdmin = tfconfig.BoolVariable(isOrgAdmin) +func (a *AccountModel) WithIsOrgAdmin(isOrgAdmin string) *AccountModel { + a.IsOrgAdmin = tfconfig.StringVariable(isOrgAdmin) return a } @@ -121,8 +135,8 @@ func (a *AccountModel) WithLastName(lastName string) *AccountModel { return a } -func (a *AccountModel) WithMustChangePassword(mustChangePassword bool) *AccountModel { - a.MustChangePassword = tfconfig.BoolVariable(mustChangePassword) +func (a *AccountModel) WithMustChangePassword(mustChangePassword string) *AccountModel { + a.MustChangePassword = tfconfig.StringVariable(mustChangePassword) return a } @@ -160,6 +174,11 @@ func (a *AccountModel) WithAdminRsaPublicKeyValue(value tfconfig.Variable) *Acco return a } +func (a *AccountModel) WithAdminUserTypeValue(value tfconfig.Variable) *AccountModel { + a.AdminUserType = value + return a +} + func (a *AccountModel) WithCommentValue(value tfconfig.Variable) *AccountModel { a.Comment = value return a diff --git a/pkg/acceptance/helpers/random/random_helpers.go b/pkg/acceptance/helpers/random/random_helpers.go index fcfb5b7208..6e11e7cd05 100644 --- a/pkg/acceptance/helpers/random/random_helpers.go +++ b/pkg/acceptance/helpers/random/random_helpers.go @@ -3,6 +3,7 @@ package random import ( "github.com/brianvoe/gofakeit/v6" "github.com/hashicorp/go-uuid" + "strings" ) func UUID() string { @@ -22,7 +23,7 @@ func Password() string { // 090088 (22000): ADMIN_NAME can only contain letters, numbers and underscores. // 090089 (22000): ADMIN_NAME must start with a letter. func AdminName() string { - return AlphaN(1) + AlphanumericN(11) + return strings.ToUpper(AlphaN(1) + AlphanumericN(11)) } func Bool() bool { diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 47821954f8..96ec958e1d 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -3,6 +3,7 @@ package resources import ( "context" "errors" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -298,13 +299,20 @@ var accountSchema = map[string]*schema.Schema{ Optional: true, ForceNew: true, Description: "Specifies a comment for the account.", + DiffSuppressFunc: SuppressIfAny( + IgnoreChangeToCurrentSnowflakeValueInShow("comment"), + func(k, oldValue, newValue string, d *schema.ResourceData) bool { + return oldValue == "SNOWFLAKE" && newValue == "" + }, + ), }, "is_org_admin": { Type: schema.TypeString, Optional: true, Default: BooleanDefault, - Description: "TODO", + DiffSuppressFunc: IgnoreChangeToCurrentSnowflakeValueInShow("is_org_admin"), ValidateDiagFunc: validateBooleanString, + Description: "TODO", }, "grace_period_in_days": { Type: schema.TypeInt, @@ -348,6 +356,15 @@ func Account() *schema.Resource { func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client + + isOrgAdmin, err := client.ContextFunctions.IsRoleInSession(ctx, snowflakeroles.Orgadmin) + if err != nil { + return diag.FromErr(err) + } + if !isOrgAdmin { + return diag.FromErr(errors.New("current user doesn't have the orgadmin role in session")) + } + id := sdk.NewAccountObjectIdentifier(d.Get("name").(string)) opts := &sdk.CreateAccountOptions{ @@ -408,12 +425,33 @@ func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D d.SetId(helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(createResponse.OrganizationName, createResponse.AccountName))) + if v, ok := d.GetOk("is_org_admin"); ok && v == BooleanTrue { + err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: id, + OrgAdmin: true, + }, + }) + if err != nil { + return diag.FromErr(err) + } + } + return ReadAccount(false)(ctx, d, meta) } func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { return func(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client + + isOrgAdmin, err := client.ContextFunctions.IsRoleInSession(ctx, snowflakeroles.Orgadmin) + if err != nil { + return diag.FromErr(err) + } + if !isOrgAdmin { + return diag.FromErr(errors.New("current user doesn't have the orgadmin role in session")) + } + id, err := sdk.ParseAccountIdentifier(d.Id()) if err != nil { return diag.FromErr(err) @@ -425,16 +463,35 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { } if withExternalChangesMarking { + var regionGroup string + if account.RegionGroup != nil { + regionGroup = *account.RegionGroup + } if err = handleExternalChangesToObjectInShow(d, - outputMapping{"region_group", "region_group", account.RegionGroup, sdk.DerefIfNotNil(account.RegionGroup), sdk.DerefIfNotNil}, + outputMapping{"is_org_admin", "is_org_admin", *account.IsOrgAdmin, booleanStringFromBool(*account.IsOrgAdmin), nil}, + outputMapping{"region_group", "region_group", regionGroup, regionGroup, nil}, outputMapping{"snowflake_region", "region", account.SnowflakeRegion, account.SnowflakeRegion, nil}, - outputMapping{"comment", "comment", account.Comment, sdk.DerefIfNotNil(account.Comment), sdk.DerefIfNotNil}, + outputMapping{"comment", "comment", *account.Comment, *account.Comment, nil}, ); err != nil { return diag.FromErr(err) } } else { if err = setStateToValuesFromConfig(d, taskSchema, []string{ - "allow_overlapping_execution", + "name", + "admin_name", + "admin_password", + "admin_rsa_public_key", + "admin_user_type", + "first_name", + "last_name", + "email", + "must_change_password", + "edition", + "region_group", + "region", + "comment", + "is_org_admin", + "grace_period_in_days", }); err != nil { return diag.FromErr(err) } @@ -463,12 +520,12 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { // } // return "", nil //}), - attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { - if isOrgAdmin != nil { - return booleanStringFromBool(*isOrgAdmin), nil - } - return BooleanDefault, nil - }), + //attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { + // if isOrgAdmin != nil { + // return booleanStringFromBool(*isOrgAdmin), nil + // } + // return BooleanDefault, nil + //}), d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), d.Set(ShowOutputAttributeName, []map[string]any{schemas.AccountToSchema(account)}), ); errs != nil { @@ -480,6 +537,16 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { } func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + client := meta.(*provider.Context).Client + + isOrgAdmin, err := client.ContextFunctions.IsRoleInSession(ctx, snowflakeroles.Orgadmin) + if err != nil { + return diag.FromErr(err) + } + if !isOrgAdmin { + return diag.FromErr(errors.New("current user doesn't have the orgadmin role in session")) + } + /* todo: comments may eventually work again for accounts, so this can be uncommented when that happens client := meta.(*provider.Context).Client @@ -506,12 +573,21 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - id, err := sdk.ParseAccountObjectIdentifier(d.Id()) + + isOrgAdmin, err := client.ContextFunctions.IsRoleInSession(ctx, snowflakeroles.Orgadmin) + if err != nil { + return diag.FromErr(err) + } + if !isOrgAdmin { + return diag.FromErr(errors.New("current user doesn't have the orgadmin role in session")) + } + + id, err := sdk.ParseAccountIdentifier(d.Id()) if err != nil { return diag.FromErr(err) } - err = client.Accounts.Drop(ctx, id, d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{ + err = client.Accounts.Drop(ctx, sdk.NewAccountObjectIdentifier(id.AccountName()), d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{ IfExists: sdk.Bool(true), }) if err != nil { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index af7ff0de21..f945da6db4 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -3,6 +3,7 @@ package resources_test import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -18,11 +19,86 @@ import ( "github.com/hashicorp/terraform-plugin-testing/tfversion" ) +func TestAcc_Account_minimal(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() + id := random.AdminName() + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + region := acc.TestClient().Context.CurrentRegion(t) + + configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + Config: config.FromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModel.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(sdk.NewAccountIdentifier(organizationName, id).FullyQualifiedName()). + HasAdminNameString(name). + HasAdminRsaPublicKeyString(key). + HasEmailString(email). + HasNoFirstName(). + HasNoLastName(). + HasMustChangePasswordString(r.BooleanDefault). + HasNoRegionGroup(). + HasNoRegion(). + HasNoComment(). + HasIsOrgAdminString(r.BooleanDefault). + HasGracePeriodInDaysString("3"), + resourceshowoutputassert.AccountShowOutput(t, configModel.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasSnowflakeRegion(region). + HasRegionGroup(""). + HasEdition(sdk.EditionStandard). + //HasAccountURL(). + //HasCreatedOn(). + HasComment("SNOWFLAKE"). + //HasAccountLocator(). + //HasAccountLocatorURL(). + HasManagedAccounts(0). + //HasConsumptionBillingEntityName(). + //HasMarketplaceConsumerBillingEntityName(). + //HasMarketplaceProviderBillingEntityName(). + //HasOldAccountURL(). + HasIsOrgAdmin(false). + //HasAccountOldUrlSavedOn(). + //HasAccountOldUrlLastUsed(). + //HasOrganizationOldUrl(). + //HasOrganizationOldUrlSavedOn(). + //HasOrganizationOldUrlLastUsed(). + HasIsEventsAccount(false). + HasIsOrganizationAccount(false), + //HasDroppedOn(). + //HasScheduledDeletionTime(). + //HasRestoredOn(). + //HasMovedToOrganization(). + //HasMovedOn(). + //HasOrganizationUrlExpirationOn(), + ), + }, + }, + }) +} + func TestAcc_Account_complete(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) - id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() + id := random.AdminName() firstName := acc.TestClient().Ids.Alpha() lastName := acc.TestClient().Ids.Alpha() email := random.Email() @@ -31,21 +107,18 @@ func TestAcc_Account_complete(t *testing.T) { region := acc.TestClient().Context.CurrentRegion(t) comment := random.Comment() - configModel := model.Account("test", id.Name(), string(sdk.EditionStandard), email, name). - // TODO: WithAdminUserType() + configModel := model.Account("test", name, string(sdk.UserTypePerson), string(sdk.EditionStandard), email, 3, id). WithAdminRsaPublicKey(key). WithFirstName(firstName). WithLastName(lastName). - WithMustChangePassword(true). - WithRegionGroup("PUBLIC"). + WithMustChangePassword(r.BooleanTrue). + //WithRegionGroup("PUBLIC"). WithRegion(region). WithComment(comment). - WithIsOrgAdmin(true). - WithGracePeriodInDays(3) + WithIsOrgAdmin(r.BooleanFalse) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, - PreCheck: func() { acc.TestAccPreCheck(t) }, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), }, @@ -55,42 +128,58 @@ func TestAcc_Account_complete(t *testing.T) { Config: config.FromModel(t, configModel), Check: assert.AssertThat(t, resourceassert.AccountResource(t, configModel.ResourceReference()). - HasNameString(id.Name()). - HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasNameString(id). + HasFullyQualifiedNameString(sdk.NewAccountIdentifier(organizationName, id).FullyQualifiedName()). HasAdminNameString(name). HasAdminRsaPublicKeyString(key). HasEmailString(email). HasFirstNameString(firstName). HasLastNameString(lastName). HasMustChangePasswordString(r.BooleanTrue). - HasRegionGroupString("PUBLIC"). + HasNoRegionGroup(). // TODO HasRegionString(region). HasCommentString(comment). - HasIsOrgAdminString(r.BooleanTrue). + HasIsOrgAdminString(r.BooleanFalse). HasGracePeriodInDaysString("3"), - // TODO: Show output + resourceshowoutputassert.AccountShowOutput(t, configModel.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasSnowflakeRegion(region). + HasRegionGroup(""). + HasEdition(sdk.EditionStandard). + //HasAccountURL(). + //HasCreatedOn(). + HasComment(comment). + //HasAccountLocator(). + //HasAccountLocatorURL(). + HasManagedAccounts(0). + //HasConsumptionBillingEntityName(). + //HasMarketplaceConsumerBillingEntityName(). + //HasMarketplaceProviderBillingEntityName(). + //HasOldAccountURL(). + HasIsOrgAdmin(false). + //HasAccountOldUrlSavedOn(). + //HasAccountOldUrlLastUsed(). + //HasOrganizationOldUrl(). + //HasOrganizationOldUrlSavedOn(). + //HasOrganizationOldUrlLastUsed(). + HasIsEventsAccount(false). + HasIsOrganizationAccount(false), + //HasDroppedOn(). + //HasScheduledDeletionTime(). + //HasRestoredOn(). + //HasMovedToOrganization(). + //HasMovedOn(). + //HasOrganizationUrlExpirationOn(), ), }, }, }) } -//func accountConfig(name string, password string, comment string, gracePeriodInDays int) string { -// return fmt.Sprintf(` -//data "snowflake_current_account" "current" {} -// -//resource "snowflake_account" "test" { -// name = "%s" -// admin_name = "someadmin" -// admin_password = "%s" -// first_name = "Ad" -// last_name = "Min" -// email = "admin@example.com" -// must_change_password = false -// edition = "BUSINESS_CRITICAL" -// comment = "%s" -// region = data.snowflake_current_account.current.region -// grace_period_in_days = %d -//} -//`, name, password, comment, gracePeriodInDays) -//} +// TODO: All show outputs in minimal and complete +// TODO: Imports +// TODO: Alters +// TODO: Not orgadmin role +// TODO: Invalid values +// TODO: State upgrader diff --git a/pkg/resources/resource_helpers_read.go b/pkg/resources/resource_helpers_read.go index 72ee47daa6..b3dcfcebf1 100644 --- a/pkg/resources/resource_helpers_read.go +++ b/pkg/resources/resource_helpers_read.go @@ -50,17 +50,6 @@ func setBooleanStringFromBoolProperty(d *schema.ResourceData, key string, proper return nil } -func attributeMappedValueReadOrNil[T, R any](d *schema.ResourceData, key string, value *T, mapper func(*T) (R, error)) error { - if value != nil { - mappedValue, err := mapper(value) - if err != nil { - return err - } - return d.Set(key, mappedValue) - } - return d.Set(key, nil) -} - func attributeMappedValueReadOrDefault[T, R any](d *schema.ResourceData, key string, value *T, mapper func(*T) (R, error), defaultValue *R) error { if value != nil { mappedValue, err := mapper(value) diff --git a/pkg/sdk/type_helpers.go b/pkg/sdk/type_helpers.go index d1a13a57a3..e1c7034b9c 100644 --- a/pkg/sdk/type_helpers.go +++ b/pkg/sdk/type_helpers.go @@ -55,14 +55,3 @@ func ToFloat64(s string) float64 { func Pointer[K any](v K) *K { return &v } - -// TODO: Test -// DerefIfNotNil is a generic function that returns a dereferenced value if a given value is not nil -func DerefIfNotNil(v any) any { - if v != nil { - if pointerValue, ok := v.(*any); ok { - return *pointerValue - } - } - return nil -} From fd03f33f850de52968056ee24b215f827109a2b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Thu, 5 Dec 2024 12:59:15 +0100 Subject: [PATCH 06/12] wip --- .../account_show_output_ext.go | 80 +++ pkg/resources/account.go | 341 ++++--------- pkg/resources/account_acceptance_test.go | 467 ++++++++++++++++-- pkg/resources/common.go | 11 +- pkg/sdk/identifier_helpers.go | 4 + 5 files changed, 607 insertions(+), 296 deletions(-) create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/account_show_output_ext.go diff --git a/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/account_show_output_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/account_show_output_ext.go new file mode 100644 index 0000000000..66a7a98a42 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/account_show_output_ext.go @@ -0,0 +1,80 @@ +package resourceshowoutputassert + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +func (a *AccountShowOutputAssert) HasAccountUrlNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("account_url")) + return a +} + +func (a *AccountShowOutputAssert) HasCreatedOnNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("created_on")) + return a +} + +func (a *AccountShowOutputAssert) HasAccountLocatorNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("account_locator")) + return a +} + +func (a *AccountShowOutputAssert) HasAccountLocatorUrlNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("account_locator_url")) + return a +} + +func (a *AccountShowOutputAssert) HasConsumptionBillingEntityNameNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("consumption_billing_entity_name")) + return a +} + +func (a *AccountShowOutputAssert) HasMarketplaceProviderBillingEntityNameNotEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValuePresent("marketplace_provider_billing_entity_name")) + return a +} + +func (a *AccountShowOutputAssert) HasAccountOldUrlSavedOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("account_old_url_saved_on", "")) + return a +} + +func (a *AccountShowOutputAssert) HasAccountOldUrlLastUsedEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("account_old_url_last_used", "")) + return a +} + +func (a *AccountShowOutputAssert) HasOrganizationOldUrlSavedOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("organization_old_url_saved_on", "")) + return a +} + +func (a *AccountShowOutputAssert) HasOrganizationOldUrlLastUsedEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("organization_old_url_last_used", "")) + return a +} + +func (a *AccountShowOutputAssert) HasDroppedOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("dropped_on", "")) + return a +} + +func (a *AccountShowOutputAssert) HasScheduledDeletionTimeEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("scheduled_deletion_time", "")) + return a +} + +func (a *AccountShowOutputAssert) HasRestoredOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("restored_on", "")) + return a +} + +func (a *AccountShowOutputAssert) HasMovedOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("moved_on", "")) + return a +} + +func (a *AccountShowOutputAssert) HasOrganizationUrlExpirationOnEmpty() *AccountShowOutputAssert { + a.AddAssertion(assert.ResourceShowOutputValueSet("organization_url_expiration_on", "")) + return a +} diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 96ec958e1d..86c1058d40 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -6,6 +6,7 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" @@ -16,197 +17,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) -// Note: no test case was created for account since we cannot actually delete them after creation, which is a critical part of the test suite. Instead, this resource -// was manually tested - -//var accountSchemaOld = map[string]*schema.Schema{ -// "name": { -// Type: schema.TypeString, -// Required: true, -// Description: "Specifies the identifier (i.e. name) for the account; must be unique within an organization, regardless of which Snowflake Region the account is in. In addition, the identifier 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 is automatically uppercase by Snowflake -// StateFunc: func(val interface{}) string { -// return strings.ToUpper(val.(string)) -// }, -// ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), -// }, -// "admin_name": { -// Type: schema.TypeString, -// Required: true, -// Description: "Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive.", -// // We have no way of assuming a role into this account to change the admin user name so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "admin_password": { -// Type: schema.TypeString, -// Optional: true, -// Sensitive: true, -// Description: "Password for the initial administrative user of the account. Optional if the `ADMIN_RSA_PUBLIC_KEY` parameter is specified. For more information about passwords in Snowflake, see [Snowflake-provided Password Policy](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=Snowflake%2Dprovided%20Password%20Policy).", -// AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, -// // We have no way of assuming a role into this account to change the password so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "admin_rsa_public_key": { -// Type: schema.TypeString, -// Optional: true, -// Sensitive: true, -// Description: "Assigns a public key to the initial administrative user of the account in order to implement [key pair authentication](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=key%20pair%20authentication) for the user. Optional if the `ADMIN_PASSWORD` parameter is specified.", -// AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, -// // We have no way of assuming a role into this account to change the admin rsa public key so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "email": { -// Type: schema.TypeString, -// Required: true, -// Sensitive: true, -// Description: "Email address of the initial administrative user of the account. This email address is used to send any notifications about the account.", -// // We have no way of assuming a role into this account to change the admin email so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "edition": { -// Type: schema.TypeString, -// Required: true, -// ForceNew: true, -// Description: "[Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values are: STANDARD | ENTERPRISE | BUSINESS_CRITICAL", -// ValidateFunc: validation.StringInSlice([]string{string(sdk.EditionStandard), string(sdk.EditionEnterprise), string(sdk.EditionBusinessCritical)}, false), -// }, -// "first_name": { -// Type: schema.TypeString, -// Optional: true, -// Sensitive: true, -// Description: "First name of the initial administrative user of the account", -// // We have no way of assuming a role into this account to change the admin first name so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "last_name": { -// Type: schema.TypeString, -// Optional: true, -// Sensitive: true, -// Description: "Last name of the initial administrative user of the account", -// // We have no way of assuming a role into this account to change the admin last name so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "must_change_password": { -// Type: schema.TypeBool, -// Optional: true, -// Default: false, -// Description: "Specifies whether the new user created to administer the account is forced to change their password upon first login into the account.", -// // We have no way of assuming a role into this account to change the admin password policy so this has to be ForceNew even though it's not ideal -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return old == "" -// }, -// }, -// "region_group": { -// Type: schema.TypeString, -// Optional: true, -// Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return new == "" -// }, -// }, -// "region": { -// Type: schema.TypeString, -// Optional: true, -// Description: "ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", -// ForceNew: true, -// DiffSuppressOnRefresh: true, -// DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { -// // For new resources always show the diff -// if d.Id() == "" { -// return false -// } -// // This suppresses the diff if the old value is empty. This would happen in the event of importing existing accounts since we have no way of reading this value -// return new == "" -// }, -// }, -// "comment": { -// Type: schema.TypeString, -// Optional: true, -// Description: "Specifies a comment for the account.", -// ForceNew: true, -// }, -// "is_org_admin": { -// Type: schema.TypeBool, -// Computed: true, -// Description: "Indicates whether the ORGADMIN role is enabled in an account. If TRUE, the role is enabled.", -// }, -// "grace_period_in_days": { -// Type: schema.TypeInt, -// Optional: true, -// Default: 3, -// Description: "Specifies the number of days to wait before dropping the account. The default is 3 days.", -// }, -// FullyQualifiedNameAttributeName: schemas.FullyQualifiedNameSchema, -//} - var accountSchema = map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -347,11 +157,55 @@ func Account() *schema.Resource { Schema: accountSchema, Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, + StateContext: TrackingImportWrapper(resources.Account, ImportAccount), }, - // TODO: State upgrader and import + // TODO: State upgrader + } +} + +func ImportAccount(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + client := meta.(*provider.Context).Client + + isOrgAdmin, err := client.ContextFunctions.IsRoleInSession(ctx, snowflakeroles.Orgadmin) + if err != nil { + return nil, err + } + if !isOrgAdmin { + // TODO: + return nil, errors.New("current user doesn't have the orgadmin role in session") + } + + id, err := sdk.ParseAccountIdentifier(d.Id()) + if err != nil { + return nil, err + } + + account, err := client.Accounts.ShowByID(ctx, id.AccountId()) + if err != nil { + return nil, err } + + if _, err := ImportName[sdk.AccountIdentifier](context.Background(), d, nil); err != nil { + return nil, err + } + + if account.RegionGroup != nil { + if err = d.Set("region_group", *account.RegionGroup); err != nil { + return nil, err + } + } + + if err := errors.Join( + d.Set("edition", string(*account.Edition)), + d.Set("region", account.SnowflakeRegion), + d.Set("comment", *account.Comment), + d.Set("is_org_admin", booleanStringFromBool(*account.IsOrgAdmin)), + ); err != nil { + return nil, err + } + + return []*schema.ResourceData{d}, nil } func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { @@ -457,7 +311,7 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { return diag.FromErr(err) } - account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id.AccountName())) + account, err := client.Accounts.ShowByID(ctx, id.AccountId()) if err != nil { return diag.FromErr(err) } @@ -468,6 +322,7 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { regionGroup = *account.RegionGroup } if err = handleExternalChangesToObjectInShow(d, + outputMapping{"edition", "edition", *account.Edition, *account.Edition, nil}, outputMapping{"is_org_admin", "is_org_admin", *account.IsOrgAdmin, booleanStringFromBool(*account.IsOrgAdmin), nil}, outputMapping{"region_group", "region_group", regionGroup, regionGroup, nil}, outputMapping{"snowflake_region", "region", account.SnowflakeRegion, account.SnowflakeRegion, nil}, @@ -476,7 +331,7 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { return diag.FromErr(err) } } else { - if err = setStateToValuesFromConfig(d, taskSchema, []string{ + if err = setStateToValuesFromConfig(d, accountSchema, []string{ "name", "admin_name", "admin_password", @@ -498,34 +353,6 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { } if errs := errors.Join( - attributeMappedValueReadOrDefault(d, "edition", account.Edition, func(edition *sdk.AccountEdition) (string, error) { - if edition != nil { - return string(*edition), nil - } - return "", nil - }, nil), - // TODO: Region group is only returned when org is span on multiple region groups, but you can explicitly set it (e.g. PUBLIC) - //attributeMappedValueReadOrNil(d, "region_group", account.RegionGroup, func(regionGroup *string) (string, error) { - // if regionGroup != nil { - // return *regionGroup, nil - // } - // return "", nil - //}), - // TODO: Can be left empty and it will be populated with current account's region - //d.Set("region", account.SnowflakeRegion), - // TODO: Default comment is "SNOWFLAKE" - //attributeMappedValueReadOrNil(d, "comment", account.Comment, func(comment *string) (string, error) { - // if comment != nil { - // return *comment, nil - // } - // return "", nil - //}), - //attributeMappedValueReadOrNil(d, "is_org_admin", account.IsOrgAdmin, func(isOrgAdmin *bool) (string, error) { - // if isOrgAdmin != nil { - // return booleanStringFromBool(*isOrgAdmin), nil - // } - // return BooleanDefault, nil - //}), d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()), d.Set(ShowOutputAttributeName, []map[string]any{schemas.AccountToSchema(account)}), ); errs != nil { @@ -547,28 +374,56 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D return diag.FromErr(errors.New("current user doesn't have the orgadmin role in session")) } - /* - todo: comments may eventually work again for accounts, so this can be uncommented when that happens - client := meta.(*provider.Context).Client - client := sdk.NewClientFromDB(db) - ctx := context.Background() - - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.AccountObjectIdentifier) - - // Change comment - if d.HasChange("comment") { - // changing comment isn't supported for accounts - err := client.Comments.Set(ctx, &sdk.SetCommentOptions{ - ObjectType: sdk.ObjectTypeAccount, - ObjectName: sdk.NewAccountObjectIdentifier(d.Get("name").(string)), - Value: sdk.String(d.Get("comment").(string)), - }) + id, err := sdk.ParseAccountIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } + + if d.HasChange("name") { + newId := sdk.NewAccountIdentifier(id.OrganizationName(), d.Get("name").(string)) + + err = client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + Rename: &sdk.AccountRename{ + Name: id.AccountId(), + NewName: newId.AccountId(), + }, + }) + if err != nil { + return diag.FromErr(err) + } + + d.SetId(helpers.EncodeResourceIdentifier(newId)) + id = newId + } + + if d.HasChange("is_org_admin") { + if v := d.Get("is_org_admin").(string); v != BooleanDefault { + parsed, err := booleanStringToBool(v) if err != nil { - return err + return diag.FromErr(err) + } + if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: id.AccountId(), + OrgAdmin: parsed, + }, + }); err != nil { + return diag.FromErr(err) + } + } else { + // No unset available for this field (setting Snowflake default) + if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: id.AccountId(), + OrgAdmin: false, + }, + }); err != nil && !strings.Contains(err.Error(), "already has ORGADMIN disabled") { // TODO: What to do about this error? + return diag.FromErr(err) } } - */ - return nil + } + + return ReadAccount(false)(ctx, d, meta) } func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { @@ -587,7 +442,7 @@ func DeleteAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D return diag.FromErr(err) } - err = client.Accounts.Drop(ctx, sdk.NewAccountObjectIdentifier(id.AccountName()), d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{ + err = client.Accounts.Drop(ctx, id.AccountId(), d.Get("grace_period_in_days").(int), &sdk.DropAccountOptions{ IfExists: sdk.Bool(true), }) if err != nil { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index f945da6db4..cc07fc8a17 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -1,30 +1,35 @@ package resources_test import ( + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeenvs" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "regexp" "testing" - acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) -func TestAcc_Account_minimal(t *testing.T) { +func TestAcc_Account_Minimal(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() id := random.AdminName() + accountId := sdk.NewAccountIdentifier(organizationName, id) email := random.Email() name := random.AdminName() key, _ := random.GenerateRSAPublicKey(t) @@ -45,7 +50,7 @@ func TestAcc_Account_minimal(t *testing.T) { Check: assert.AssertThat(t, resourceassert.AccountResource(t, configModel.ResourceReference()). HasNameString(id). - HasFullyQualifiedNameString(sdk.NewAccountIdentifier(organizationName, id).FullyQualifiedName()). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasAdminNameString(name). HasAdminRsaPublicKeyString(key). HasEmailString(email). @@ -63,42 +68,65 @@ func TestAcc_Account_minimal(t *testing.T) { HasSnowflakeRegion(region). HasRegionGroup(""). HasEdition(sdk.EditionStandard). - //HasAccountURL(). - //HasCreatedOn(). + HasAccountUrlNotEmpty(). + HasCreatedOnNotEmpty(). HasComment("SNOWFLAKE"). - //HasAccountLocator(). - //HasAccountLocatorURL(). + HasAccountLocatorNotEmpty(). + HasAccountLocatorUrlNotEmpty(). HasManagedAccounts(0). - //HasConsumptionBillingEntityName(). - //HasMarketplaceConsumerBillingEntityName(). - //HasMarketplaceProviderBillingEntityName(). - //HasOldAccountURL(). + HasConsumptionBillingEntityNameNotEmpty(). + HasMarketplaceConsumerBillingEntityName(""). + HasMarketplaceProviderBillingEntityNameNotEmpty(). + HasOldAccountURL(""). HasIsOrgAdmin(false). - //HasAccountOldUrlSavedOn(). - //HasAccountOldUrlLastUsed(). - //HasOrganizationOldUrl(). - //HasOrganizationOldUrlSavedOn(). - //HasOrganizationOldUrlLastUsed(). + HasAccountOldUrlSavedOnEmpty(). + HasAccountOldUrlLastUsedEmpty(). + HasOrganizationOldUrl(""). + HasOrganizationOldUrlSavedOnEmpty(). + HasOrganizationOldUrlLastUsedEmpty(). HasIsEventsAccount(false). - HasIsOrganizationAccount(false), - //HasDroppedOn(). - //HasScheduledDeletionTime(). - //HasRestoredOn(). - //HasMovedToOrganization(). - //HasMovedOn(). - //HasOrganizationUrlExpirationOn(), + HasIsOrganizationAccount(false). + HasDroppedOnEmpty(). + HasScheduledDeletionTimeEmpty(). + HasRestoredOnEmpty(). + HasMovedToOrganization(""). + HasMovedOn(""). + HasOrganizationUrlExpirationOnEmpty(), + ), + }, + { + ResourceName: configModel.ResourceReference(), + Config: config.FromModel(t, configModel), + ImportState: true, + ImportStateCheck: assert.AssertThatImport(t, + resourceassert.ImportedAccountResource(t, helpers.EncodeResourceIdentifier(accountId)). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasNoAdminName(). + HasNoAdminRsaPublicKey(). + HasNoEmail(). + HasNoFirstName(). + HasNoLastName(). + HasNoMustChangePassword(). + HasEditionString(string(sdk.EditionStandard)). + HasNoRegionGroup(). + HasRegionString(region). + HasCommentString("SNOWFLAKE"). + HasIsOrgAdminString(r.BooleanFalse). + HasNoGracePeriodInDays(), ), }, }, }) } -func TestAcc_Account_complete(t *testing.T) { +func TestAcc_Account_Complete(t *testing.T) { _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() id := random.AdminName() + accountId := sdk.NewAccountIdentifier(organizationName, id) firstName := acc.TestClient().Ids.Alpha() lastName := acc.TestClient().Ids.Alpha() email := random.Email() @@ -112,7 +140,7 @@ func TestAcc_Account_complete(t *testing.T) { WithFirstName(firstName). WithLastName(lastName). WithMustChangePassword(r.BooleanTrue). - //WithRegionGroup("PUBLIC"). + WithRegionGroup("PUBLIC"). WithRegion(region). WithComment(comment). WithIsOrgAdmin(r.BooleanFalse) @@ -136,7 +164,7 @@ func TestAcc_Account_complete(t *testing.T) { HasFirstNameString(firstName). HasLastNameString(lastName). HasMustChangePasswordString(r.BooleanTrue). - HasNoRegionGroup(). // TODO + HasRegionGroupString("PUBLIC"). HasRegionString(region). HasCommentString(comment). HasIsOrgAdminString(r.BooleanFalse). @@ -147,39 +175,374 @@ func TestAcc_Account_complete(t *testing.T) { HasSnowflakeRegion(region). HasRegionGroup(""). HasEdition(sdk.EditionStandard). - //HasAccountURL(). - //HasCreatedOn(). + HasAccountUrlNotEmpty(). + HasCreatedOnNotEmpty(). HasComment(comment). - //HasAccountLocator(). - //HasAccountLocatorURL(). + HasAccountLocatorNotEmpty(). + HasAccountLocatorUrlNotEmpty(). HasManagedAccounts(0). - //HasConsumptionBillingEntityName(). - //HasMarketplaceConsumerBillingEntityName(). - //HasMarketplaceProviderBillingEntityName(). - //HasOldAccountURL(). + HasConsumptionBillingEntityNameNotEmpty(). + HasMarketplaceConsumerBillingEntityName(""). + HasMarketplaceProviderBillingEntityNameNotEmpty(). + HasOldAccountURL(""). HasIsOrgAdmin(false). - //HasAccountOldUrlSavedOn(). - //HasAccountOldUrlLastUsed(). - //HasOrganizationOldUrl(). - //HasOrganizationOldUrlSavedOn(). - //HasOrganizationOldUrlLastUsed(). + HasAccountOldUrlSavedOnEmpty(). + HasAccountOldUrlLastUsedEmpty(). + HasOrganizationOldUrl(""). + HasOrganizationOldUrlSavedOnEmpty(). + HasOrganizationOldUrlLastUsedEmpty(). HasIsEventsAccount(false). - HasIsOrganizationAccount(false), - //HasDroppedOn(). - //HasScheduledDeletionTime(). - //HasRestoredOn(). - //HasMovedToOrganization(). - //HasMovedOn(). - //HasOrganizationUrlExpirationOn(), + HasIsOrganizationAccount(false). + HasDroppedOnEmpty(). + HasScheduledDeletionTimeEmpty(). + HasRestoredOnEmpty(). + HasMovedToOrganization(""). + HasMovedOn(""). + HasOrganizationUrlExpirationOnEmpty(), + ), + }, + { + ResourceName: configModel.ResourceReference(), + Config: config.FromModel(t, configModel), + ImportState: true, + ImportStateCheck: assert.AssertThatImport(t, + resourceassert.ImportedAccountResource(t, helpers.EncodeResourceIdentifier(accountId)). + HasNameString(id). + HasFullyQualifiedNameString(sdk.NewAccountIdentifier(organizationName, id).FullyQualifiedName()). + HasNoAdminName(). + HasNoAdminRsaPublicKey(). + HasNoEmail(). + HasNoFirstName(). + HasNoLastName(). + HasNoMustChangePassword(). + HasEditionString(string(sdk.EditionStandard)). + HasNoRegionGroup(). + HasRegionString(region). + HasCommentString(comment). + HasIsOrgAdminString(r.BooleanFalse). + HasNoGracePeriodInDays(), + ), + }, + }, + }) +} + +func TestAcc_Account_Rename(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() + id := random.AdminName() + accountId := sdk.NewAccountIdentifier(organizationName, id) + + newId := random.AdminName() + newAccountId := sdk.NewAccountIdentifier(organizationName, newId) + + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + + configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key) + newConfigModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, newId). + WithAdminRsaPublicKey(key) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + Config: config.FromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModel.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()), + resourceshowoutputassert.AccountShowOutput(t, configModel.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id), + ), + }, + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(newConfigModel.ResourceReference(), plancheck.ResourceActionUpdate), + }, + }, + Config: config.FromModel(t, newConfigModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, newConfigModel.ResourceReference()). + HasNameString(newId). + HasFullyQualifiedNameString(newAccountId.FullyQualifiedName()), + resourceshowoutputassert.AccountShowOutput(t, newConfigModel.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(newId), + ), + }, + }, + }) +} + +func TestAcc_Account_IsOrgAdmin(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() + id := random.AdminName() + accountId := sdk.NewAccountIdentifier(organizationName, id) + + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + + configModelWithOrgAdminTrue := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key). + WithIsOrgAdmin(r.BooleanTrue) + + configModelWithOrgAdminFalse := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key). + WithIsOrgAdmin(r.BooleanFalse) + + configModelWithoutOrgAdmin := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + // Create with ORGADMIN enabled + { + Config: config.FromModel(t, configModelWithOrgAdminTrue), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModelWithOrgAdminTrue.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasIsOrgAdminString(r.BooleanTrue), + resourceshowoutputassert.AccountShowOutput(t, configModelWithOrgAdminTrue.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasIsOrgAdmin(true), + ), + }, + // Disable ORGADMIN + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(configModelWithOrgAdminFalse.ResourceReference(), plancheck.ResourceActionUpdate), + }, + }, + Config: config.FromModel(t, configModelWithOrgAdminFalse), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModelWithOrgAdminFalse.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasIsOrgAdminString(r.BooleanFalse), + resourceshowoutputassert.AccountShowOutput(t, configModelWithOrgAdminFalse.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasIsOrgAdmin(false), ), }, + // Remove is_org_admin from the config and go back to default (disabled) + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(configModelWithoutOrgAdmin.ResourceReference(), plancheck.ResourceActionUpdate), + }, + }, + Config: config.FromModel(t, configModelWithoutOrgAdmin), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModelWithoutOrgAdmin.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasIsOrgAdminString(r.BooleanDefault), + resourceshowoutputassert.AccountShowOutput(t, configModelWithoutOrgAdmin.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasIsOrgAdmin(false), + ), + }, + // External change (enable ORGADMIN) + { + PreConfig: func() { + acc.TestClient().Account.Alter(t, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: accountId.AccountId(), + OrgAdmin: true, + }, + }) + }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(configModelWithoutOrgAdmin.ResourceReference(), plancheck.ResourceActionUpdate), + }, + }, + Config: config.FromModel(t, configModelWithoutOrgAdmin), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModelWithoutOrgAdmin.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasIsOrgAdminString(r.BooleanDefault), + resourceshowoutputassert.AccountShowOutput(t, configModelWithoutOrgAdmin.ResourceReference()). + HasOrganizationName(organizationName). + HasAccountName(id). + HasIsOrgAdmin(false), + ), + }, + }, + }) +} + +func TestAcc_Account_IgnoreUpdateAfterCreationOnCertainFields(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + organizationName := acc.TestClient().Context.CurrentAccountId(t).OrganizationName() + id := random.AdminName() + accountId := sdk.NewAccountIdentifier(organizationName, id) + + firstName := random.AdminName() + lastName := random.AdminName() + email := random.Email() + name := random.AdminName() + pass := random.Password() + + newFirstName := random.AdminName() + newLastName := random.AdminName() + newEmail := random.Email() + newName := random.AdminName() + newPass := random.Password() + + configModel := model.Account("test", name, string(sdk.UserTypePerson), string(sdk.EditionStandard), email, 3, id). + WithFirstName(firstName). + WithLastName(lastName). + WithMustChangePassword(r.BooleanTrue). + WithAdminPassword(pass) + + newConfigModel := model.Account("test", newName, string(sdk.UserTypeService), string(sdk.EditionStandard), newEmail, 3, id). + WithAdminPassword(newPass). + WithFirstName(newFirstName). + WithLastName(newLastName) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + Config: config.FromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModel.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminNameString(name). + HasAdminPasswordString(pass). + //HasAdminUserType(). TODO + HasEmailString(email). + HasFirstNameString(firstName). + HasLastNameString(lastName). + HasMustChangePasswordString(r.BooleanTrue), + ), + }, + { + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(newConfigModel.ResourceReference(), plancheck.ResourceActionNoop), + }, + }, + Config: config.FromModel(t, newConfigModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, newConfigModel.ResourceReference()). + HasNameString(id). + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminNameString(name). + HasAdminPasswordString(pass). + HasEmailString(email). + HasFirstNameString(firstName). + HasLastNameString(lastName). + HasMustChangePasswordString(r.BooleanTrue), + ), + }, + }, + }) +} + +func TestAcc_Account_TryToCreateWithoutOrgadmin(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + id := random.AdminName() + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + + t.Setenv(string(testenvs.ConfigureClientOnce), "") + t.Setenv(snowflakeenvs.Role, snowflakeroles.Accountadmin.Name()) + + configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + Config: config.FromModel(t, configModel), + ExpectError: regexp.MustCompile("Error: current user doesn't have the orgadmin role in session"), + }, + }, + }) +} + +func TestAcc_Account_InvalidValues(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + id := random.AdminName() + email := random.Email() + name := random.AdminName() + key, _ := random.GenerateRSAPublicKey(t) + + configModelInvalidUserType := model.Account("test", name, "invalid_user_type", string(sdk.EditionStandard), email, 3, id). + WithAdminRsaPublicKey(key) + + configModelInvalidAccountEdition := model.Account("test", name, string(sdk.UserTypeService), "invalid_account_edition", email, 3, id). + WithAdminRsaPublicKey(key) + + configModelInvalidGracePeriodInDays := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 2, id). + WithAdminRsaPublicKey(key) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + Config: config.FromModel(t, configModelInvalidUserType), + ExpectError: regexp.MustCompile("invalid user type: invalid_user_type"), + }, + { + Config: config.FromModel(t, configModelInvalidAccountEdition), + ExpectError: regexp.MustCompile("unknown account edition: invalid_account_edition"), + }, + { + Config: config.FromModel(t, configModelInvalidGracePeriodInDays), + ExpectError: regexp.MustCompile("Error: expected grace_period_in_days to be at least \\(3\\), got 2"), + }, }, }) } -// TODO: All show outputs in minimal and complete -// TODO: Imports -// TODO: Alters -// TODO: Not orgadmin role -// TODO: Invalid values // TODO: State upgrader diff --git a/pkg/resources/common.go b/pkg/resources/common.go index 643524f9d9..4c84ac1c4c 100644 --- a/pkg/resources/common.go +++ b/pkg/resources/common.go @@ -60,7 +60,7 @@ func ctyValToSliceString(valueElems []cty.Value) []string { return elems } -func ImportName[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier](ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { +func ImportName[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier | sdk.AccountIdentifier](ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { switch any(new(T)).(type) { case *sdk.AccountObjectIdentifier: id, err := sdk.ParseAccountObjectIdentifier(d.Id()) @@ -101,6 +101,15 @@ func ImportName[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | s if err := d.Set("schema", id.SchemaName()); err != nil { return nil, err } + case *sdk.AccountIdentifier: + id, err := sdk.ParseAccountIdentifier(d.Id()) + if err != nil { + return nil, err + } + + if err := d.Set("name", id.AccountName()); err != nil { + return nil, err + } } return []*schema.ResourceData{d}, nil diff --git a/pkg/sdk/identifier_helpers.go b/pkg/sdk/identifier_helpers.go index 90d1acdf44..8ce7a23850 100644 --- a/pkg/sdk/identifier_helpers.go +++ b/pkg/sdk/identifier_helpers.go @@ -124,6 +124,10 @@ func (i AccountIdentifier) AccountName() string { return i.accountName } +func (i AccountIdentifier) AccountId() AccountObjectIdentifier { + return NewAccountObjectIdentifier(i.accountName) +} + func (i AccountIdentifier) Name() string { if i.organizationName != "" && i.accountName != "" { return fmt.Sprintf("%s.%s", i.organizationName, i.accountName) From 2bcec115e7a38ce5709eed54b5cdfea6baa005c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 6 Dec 2024 08:16:22 +0100 Subject: [PATCH 07/12] wip --- pkg/resources/account.go | 24 ++++++ pkg/resources/account_acceptance_test.go | 103 +++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 86c1058d40..72758965bd 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "strings" @@ -161,7 +162,30 @@ func Account() *schema.Resource { }, // TODO: State upgrader + SchemaVersion: 1, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + // setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject + Type: cty.EmptyObject, + Upgrade: v0_99_0_AccountStateUpgrader, + }, + }, + } +} + +func v0_99_0_AccountStateUpgrader(ctx context.Context, state map[string]any, meta any) (map[string]any, error) { + 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)) + account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(state["name"].(string))) + if err != nil { + return nil, err } + + state["id"] = helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(account.OrganizationName, account.AccountName)) + + return state, nil } func ImportAccount(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index cc07fc8a17..b64bb3ca52 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -1,6 +1,7 @@ package resources_test import ( + "fmt" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" @@ -545,4 +546,106 @@ func TestAcc_Account_InvalidValues(t *testing.T) { }) } +func TestAcc_Account_UpgradeFrom_v0_99_0(t *testing.T) { + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + _ = testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) + + email := random.Email() + name := random.AdminName() + adminName := random.AdminName() + adminPassword := random.Password() + firstName := random.AdminName() + lastName := random.AdminName() + region := acc.TestClient().Context.CurrentRegion(t) + comment := random.Comment() + + configModel := model.Account("test", adminName, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, name). + WithAdminPassword(adminPassword). + WithFirstName(firstName). + WithLastName(lastName). + WithMustChangePassword(r.BooleanTrue). + WithRegion(region). + WithIsOrgAdmin(r.BooleanFalse). + WithComment(comment) + + resource.Test(t, resource.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.Account), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.99.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: accountConfig_v0_99_0(name, adminName, adminPassword, email, sdk.EditionStandard, firstName, lastName, true, region, 3, comment), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: config.FromModel(t, configModel), + Check: assert.AssertThat(t, + resourceassert.AccountResource(t, configModel.ResourceReference()). + HasNameString(name). + HasAdminNameString(adminName). + HasAdminPasswordString(adminPassword). + HasEmailString(email). + HasFirstNameString(firstName). + HasLastNameString(lastName). + HasMustChangePasswordString(r.BooleanTrue). + HasRegionGroupString(""). + HasRegionString(region). + HasCommentString(comment). + HasIsOrgAdminString(r.BooleanFalse). + HasGracePeriodInDaysString("3"), + ), + }, + }, + }) +} + +func accountConfig_v0_99_0( + name string, + adminName string, + adminPassword string, + email string, + edition sdk.AccountEdition, + firstName string, + lastName string, + mustChangePassword bool, + region string, + gracePeriodInDays int, + comment string, +) string { + return fmt.Sprintf(` +resource "snowflake_account" "test" { + name = "%[1]s" + admin_name = "%[2]s" + admin_password = "%[3]s" + email = "%[4]s" + edition = "%[5]s" + first_name = "%[6]s" + last_name = "%[7]s" + must_change_password = %[8]t + region = "%[9]s" + grace_period_in_days = %[10]d + comment = "%[11]s" +} +`, + name, + adminName, + adminPassword, + email, + edition, + firstName, + lastName, + mustChangePassword, + region, + gracePeriodInDays, + comment, + ) +} + // TODO: State upgrader From b61d78cc27659847b58ef3511c994fbded618869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 6 Dec 2024 13:22:54 +0100 Subject: [PATCH 08/12] wip --- docs/resources/account.md | 88 ++++++++++++------- .../resources/snowflake_account/resource.tf | 22 ++--- pkg/acceptance/helpers/random/certs.go | 18 ---- .../helpers/random/random_helpers.go | 3 +- pkg/resources/account.go | 80 ++++++----------- pkg/resources/account_acceptance_test.go | 11 ++- pkg/resources/account_state_upgraders.go | 24 +++++ pkg/sdk/accounts.go | 11 ++- pkg/sdk/accounts_test.go | 3 +- 9 files changed, 136 insertions(+), 124 deletions(-) create mode 100644 pkg/resources/account_state_upgraders.go diff --git a/docs/resources/account.md b/docs/resources/account.md index 4d3a8fea48..1caf592f99 100644 --- a/docs/resources/account.md +++ b/docs/resources/account.md @@ -2,12 +2,12 @@ page_title: "snowflake_account Resource - terraform-provider-snowflake" subcategory: "" description: |- - The account resource allows you to create and manage Snowflake accounts. + The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role. --- # snowflake_account (Resource) -The account resource allows you to create and manage Snowflake accounts. +The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role. !> **Warning** This resource cannot be destroyed!!! The only way to delete accounts is to go through [Snowflake Support](https://docs.snowflake.com/en/user-guide/organizations-manage-accounts.html#deleting-an-account) @@ -16,23 +16,14 @@ The account resource allows you to create and manage Snowflake accounts. ## Example Usage ```terraform -provider "snowflake" { - role = "ORGADMIN" - alias = "orgadmin" +## TODO: + +## Minimal +resource "snowflake_account" "minimal" { } -resource "snowflake_account" "ac1" { - provider = snowflake.orgadmin - name = "SNOWFLAKE_TEST_ACCOUNT" - admin_name = "John Doe" - admin_password = "Abcd1234!" - email = "john.doe@snowflake.com" - first_name = "John" - last_name = "Doe" - must_change_password = true - edition = "STANDARD" - comment = "Snowflake Test Account" - region = "AWS_US_WEST_2" +## Complete (with every optional set) +resource "snowflake_account" "complete" { } ``` -> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources). @@ -43,28 +34,65 @@ resource "snowflake_account" "ac1" { ### Required -- `admin_name` (String) Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive. -- `edition` (String) [Snowflake Edition](https://docs.snowflake.com/en/user-guide/intro-editions.html) of the account. Valid values 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. -- `name` (String) Specifies the identifier (i.e. name) for the account; must be unique within an organization, regardless of which Snowflake Region the account is in. In addition, the identifier 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` (String, Sensitive) Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive. 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". +- `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. ### Optional -- `admin_password` (String, Sensitive) Password for the initial administrative user of the account. Optional if the `ADMIN_RSA_PUBLIC_KEY` parameter is specified. For more information about passwords in Snowflake, see [Snowflake-provided Password Policy](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=Snowflake%2Dprovided%20Password%20Policy). -- `admin_rsa_public_key` (String, Sensitive) Assigns a public key to the initial administrative user of the account in order to implement [key pair authentication](https://docs.snowflake.com/en/sql-reference/sql/create-account.html#:~:text=key%20pair%20authentication) for the user. Optional if the `ADMIN_PASSWORD` parameter is specified. +- `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. 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_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 -- `grace_period_in_days` (Number) Specifies the number of days to wait before dropping the account. The default is 3 days. -- `last_name` (String, Sensitive) Last name of the initial administrative user of the account -- `must_change_password` (Boolean) Specifies whether the new user created to administer the account is forced to change their password upon first login into the account. -- `region` (String) ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.) -- `region_group` (String) ID of the Snowflake Region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.) +- `first_name` (String, Sensitive) First name of the initial administrative user of 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". +- `is_org_admin` (String) Sets an account property that determines whether the ORGADMIN role is enabled in the account. Only an organization administrator (i.e. user with the ORGADMIN role) can set the property. +- `last_name` (String, Sensitive) Last name of the initial administrative user of 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". +- `must_change_password` (String) Specifies whether the new user created to administer the account is forced to change their password upon first login into 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". +- `region` (String) [Snowflake Region ID](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-snowflake-region-ids) of the region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.) +- `region_group` (String) ID of the region group where the account is created. To retrieve the region group ID for existing accounts in your organization, execute the [SHOW REGIONS](https://docs.snowflake.com/en/sql-reference/sql/show-regions) command. For information about when you might need to specify region group, see [Region groups](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-region-groups). ### Read-Only - `fully_qualified_name` (String) Fully qualified name of the resource. For more information, see [object name resolution](https://docs.snowflake.com/en/sql-reference/name-resolution). - `id` (String) The ID of this resource. -- `is_org_admin` (Boolean) Indicates whether the ORGADMIN role is enabled in an account. If TRUE, the role is enabled. +- `show_output` (List of Object) Outputs the result of `SHOW ACCOUNTS` for the given account. (see [below for nested schema](#nestedatt--show_output)) + + +### Nested Schema for `show_output` + +Read-Only: + +- `account_locator` (String) +- `account_locator_url` (String) +- `account_name` (String) +- `account_old_url_last_used` (String) +- `account_old_url_saved_on` (String) +- `account_url` (String) +- `comment` (String) +- `consumption_billing_entity_name` (String) +- `created_on` (String) +- `dropped_on` (String) +- `edition` (String) +- `is_events_account` (Boolean) +- `is_org_admin` (Boolean) +- `is_organization_account` (Boolean) +- `managed_accounts` (Number) +- `marketplace_consumer_billing_entity_name` (String) +- `marketplace_provider_billing_entity_name` (String) +- `moved_on` (String) +- `moved_to_organization` (String) +- `old_account_url` (String) +- `organization_name` (String) +- `organization_old_url` (String) +- `organization_old_url_last_used` (String) +- `organization_old_url_saved_on` (String) +- `organization_url_expiration_on` (String) +- `region_group` (String) +- `restored_on` (String) +- `scheduled_deletion_time` (String) +- `snowflake_region` (String) ## Import diff --git a/examples/resources/snowflake_account/resource.tf b/examples/resources/snowflake_account/resource.tf index 3de2897d40..6b53dacf8e 100644 --- a/examples/resources/snowflake_account/resource.tf +++ b/examples/resources/snowflake_account/resource.tf @@ -1,18 +1,10 @@ -provider "snowflake" { - role = "ORGADMIN" - alias = "orgadmin" + +## TODO: + +## Minimal +resource "snowflake_account" "minimal" { } -resource "snowflake_account" "ac1" { - provider = snowflake.orgadmin - name = "SNOWFLAKE_TEST_ACCOUNT" - admin_name = "John Doe" - admin_password = "Abcd1234!" - email = "john.doe@snowflake.com" - first_name = "John" - last_name = "Doe" - must_change_password = true - edition = "STANDARD" - comment = "Snowflake Test Account" - region = "AWS_US_WEST_2" +## Complete (with every optional set) +resource "snowflake_account" "complete" { } diff --git a/pkg/acceptance/helpers/random/certs.go b/pkg/acceptance/helpers/random/certs.go index ca74761ec0..b314a0cbfa 100644 --- a/pkg/acceptance/helpers/random/certs.go +++ b/pkg/acceptance/helpers/random/certs.go @@ -60,24 +60,6 @@ func GenerateRSAPublicKeyFromPrivateKey(t *testing.T, key *rsa.PrivateKey) (stri return encode(t, "RSA PUBLIC KEY", b), hash(t, b) } -func GenerateRSAPublicKeyBasedOnPrivateKey(t *testing.T, key *rsa.PrivateKey) (string, string) { - t.Helper() - - pub := key.Public() - b, err := x509.MarshalPKIXPublicKey(pub.(*rsa.PublicKey)) - require.NoError(t, err) - return encode(t, "RSA PUBLIC KEY", b), hash(t, b) -} - -func GenerateRSAPublicKeyBasedOnPrivateKey(t *testing.T, key *rsa.PrivateKey) (string, string) { - t.Helper() - - pub := key.Public() - b, err := x509.MarshalPKIXPublicKey(pub.(*rsa.PublicKey)) - require.NoError(t, err) - return encode(t, "RSA PUBLIC KEY", b), hash(t, b) -} - // GenerateRSAPrivateKey returns an RSA private key. func GenerateRSAPrivateKey(t *testing.T) *rsa.PrivateKey { t.Helper() diff --git a/pkg/acceptance/helpers/random/random_helpers.go b/pkg/acceptance/helpers/random/random_helpers.go index 6e11e7cd05..e9176206a9 100644 --- a/pkg/acceptance/helpers/random/random_helpers.go +++ b/pkg/acceptance/helpers/random/random_helpers.go @@ -1,9 +1,10 @@ package random import ( + "strings" + "github.com/brianvoe/gofakeit/v6" "github.com/hashicorp/go-uuid" - "strings" ) func UUID() string { diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 72758965bd..22a620fc09 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -3,11 +3,15 @@ package resources import ( "context" "errors" + "fmt" + "strings" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider/docs" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" @@ -20,24 +24,23 @@ import ( var accountSchema = map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - // TODO: Sensitive? - Description: "TODO", + 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.", ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), }, "admin_name": { - Type: schema.TypeString, - Required: true, - // TODO: Sensitive? - Description: externalChangesNotDetectedFieldDescription("TODO"), + Type: schema.TypeString, + Required: true, + Sensitive: true, + Description: externalChangesNotDetectedFieldDescription("Login name of the initial administrative user of the account. A new user is created in the new account with this name and password and granted the ACCOUNTADMIN role in the account. A login name can be any string consisting of letters, numbers, and underscores. Login names are always case-insensitive."), DiffSuppressFunc: IgnoreAfterCreation, }, "admin_password": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO"), + Description: externalChangesNotDetectedFieldDescription("Password for 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"}, }, @@ -45,15 +48,14 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO"), + 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"}, }, "admin_user_type": { - Type: schema.TypeString, - Required: true, - // TODO: Valid options - Description: externalChangesNotDetectedFieldDescription("TODO"), + Type: schema.TypeString, + Optional: true, + Description: externalChangesNotDetectedFieldDescription(fmt.Sprintf("Used for setting the type of the first user that is assigned the ACCOUNTADMIN role during account creation. Valid options are: %s", docs.PossibleValuesListed(sdk.AllUserTypes))), DiffSuppressFunc: SuppressIfAny(IgnoreAfterCreation, NormalizeAndCompare(sdk.ToUserType)), ValidateDiagFunc: sdkValidation(sdk.ToUserType), }, @@ -61,28 +63,28 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO"), + Description: externalChangesNotDetectedFieldDescription("First name of the initial administrative user of the account."), DiffSuppressFunc: IgnoreAfterCreation, }, "last_name": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO"), + Description: externalChangesNotDetectedFieldDescription("Last name of the initial administrative user of the account."), DiffSuppressFunc: IgnoreAfterCreation, }, "email": { Type: schema.TypeString, Required: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("TODO"), + Description: externalChangesNotDetectedFieldDescription("Email address of the initial administrative user of the account. This email address is used to send any notifications about the account."), DiffSuppressFunc: IgnoreAfterCreation, }, "must_change_password": { Type: schema.TypeString, Optional: true, Default: BooleanDefault, - Description: externalChangesNotDetectedFieldDescription("TODO"), + Description: externalChangesNotDetectedFieldDescription("Specifies whether the new user created to administer the account is forced to change their password upon first login into the account."), DiffSuppressFunc: IgnoreAfterCreation, ValidateDiagFunc: validateBooleanString, }, @@ -90,20 +92,20 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: "TODO", + Description: fmt.Sprintf("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: %s", docs.PossibleValuesListed(sdk.AllAccountEditions)), ValidateDiagFunc: sdkValidation(sdk.ToAccountEdition), }, "region_group": { Type: schema.TypeString, Optional: true, ForceNew: true, - Description: "TODO", + Description: "ID of the region group where the account is created. To retrieve the region group ID for existing accounts in your organization, execute the [SHOW REGIONS](https://docs.snowflake.com/en/sql-reference/sql/show-regions) command. For information about when you might need to specify region group, see [Region groups](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-region-groups).", }, "region": { Type: schema.TypeString, Optional: true, ForceNew: true, - Description: "TODO", + Description: "[Snowflake Region ID](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-snowflake-region-ids) of the region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.)", }, "comment": { Type: schema.TypeString, @@ -123,12 +125,12 @@ var accountSchema = map[string]*schema.Schema{ Default: BooleanDefault, DiffSuppressFunc: IgnoreChangeToCurrentSnowflakeValueInShow("is_org_admin"), ValidateDiagFunc: validateBooleanString, - Description: "TODO", + Description: "Sets an account property that determines whether the ORGADMIN role is enabled in the account. Only an organization administrator (i.e. user with the ORGADMIN role) can set the property.", }, "grace_period_in_days": { Type: schema.TypeInt, Required: true, - Description: "TODO", + Description: "Specifies the number of days during which the account can be restored (“undropped”). The minimum is 3 days and the maximum is 90 days.", ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(3)), }, FullyQualifiedNameAttributeName: schemas.FullyQualifiedNameSchema, @@ -144,8 +146,7 @@ var accountSchema = map[string]*schema.Schema{ func Account() *schema.Resource { return &schema.Resource{ - // TODO: Desc - Description: "The account resource allows you to create and manage Snowflake accounts.", + Description: "The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role.", CreateContext: TrackingCreateWrapper(resources.Account, CreateAccount), ReadContext: TrackingReadWrapper(resources.Account, ReadAccount(true)), UpdateContext: TrackingUpdateWrapper(resources.Account, UpdateAccount), @@ -161,7 +162,6 @@ func Account() *schema.Resource { StateContext: TrackingImportWrapper(resources.Account, ImportAccount), }, - // TODO: State upgrader SchemaVersion: 1, StateUpgraders: []schema.StateUpgrader{ { @@ -174,20 +174,6 @@ func Account() *schema.Resource { } } -func v0_99_0_AccountStateUpgrader(ctx context.Context, state map[string]any, meta any) (map[string]any, error) { - 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)) - account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(state["name"].(string))) - if err != nil { - return nil, err - } - - state["id"] = helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(account.OrganizationName, account.AccountName)) - - return state, nil -} - func ImportAccount(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { client := meta.(*provider.Context).Client @@ -287,15 +273,6 @@ func CreateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D opts.Comment = sdk.String(v.(string)) } - // TODO(TODO): next prs - //if v := d.Get("polaris"); v != BooleanDefault { - // parsedBool, err := booleanStringToBool(v.(string)) - // if err != nil { - // return diag.FromErr(err) - // } - // opts.Polaris = &parsedBool - //} - createResponse, err := client.Accounts.Create(ctx, id, opts) if err != nil { return diag.FromErr(err) @@ -441,7 +418,8 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D Name: id.AccountId(), OrgAdmin: false, }, - }); err != nil && !strings.Contains(err.Error(), "already has ORGADMIN disabled") { // TODO: What to do about this error? + // This error may happen when a user removes is_org_admin, and previously it was explicitly set to false. + }); err != nil && !strings.Contains(err.Error(), "already has ORGADMIN disabled") { return diag.FromErr(err) } } diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index b64bb3ca52..94afd44903 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -2,6 +2,9 @@ package resources_test import ( "fmt" + "regexp" + "testing" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" @@ -15,8 +18,6 @@ import ( r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/hashicorp/terraform-plugin-testing/plancheck" - "regexp" - "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" @@ -445,7 +446,7 @@ func TestAcc_Account_IgnoreUpdateAfterCreationOnCertainFields(t *testing.T) { HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasAdminNameString(name). HasAdminPasswordString(pass). - //HasAdminUserType(). TODO + // HasAdminUserType(). TODO HasEmailString(email). HasFirstNameString(firstName). HasLastNameString(lastName). @@ -540,7 +541,7 @@ func TestAcc_Account_InvalidValues(t *testing.T) { }, { Config: config.FromModel(t, configModelInvalidGracePeriodInDays), - ExpectError: regexp.MustCompile("Error: expected grace_period_in_days to be at least \\(3\\), got 2"), + ExpectError: regexp.MustCompile(`Error: expected grace_period_in_days to be at least \(3\), got 2`), }, }, }) @@ -647,5 +648,3 @@ resource "snowflake_account" "test" { comment, ) } - -// TODO: State upgrader diff --git a/pkg/resources/account_state_upgraders.go b/pkg/resources/account_state_upgraders.go new file mode 100644 index 0000000000..63965731c8 --- /dev/null +++ b/pkg/resources/account_state_upgraders.go @@ -0,0 +1,24 @@ +package resources + +import ( + "context" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +func v0_99_0_AccountStateUpgrader(ctx context.Context, state map[string]any, meta any) (map[string]any, error) { + 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)) + account, err := client.Accounts.ShowByID(ctx, sdk.NewAccountObjectIdentifier(state["name"].(string))) + if err != nil { + return nil, err + } + + state["id"] = helpers.EncodeResourceIdentifier(sdk.NewAccountIdentifier(account.OrganizationName, account.AccountName)) + + return state, nil +} diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 02a22a6049..94a4dd8666 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -6,10 +6,11 @@ import ( "encoding/json" "errors" "fmt" - "github.com/snowflakedb/gosnowflake" "strings" "time" + "github.com/snowflakedb/gosnowflake" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" ) @@ -43,6 +44,12 @@ var ( EditionBusinessCritical AccountEdition = "BUSINESS_CRITICAL" ) +var AllAccountEditions = []AccountEdition{ + EditionStandard, + EditionEnterprise, + EditionBusinessCritical, +} + // TODO: test func ToAccountEdition(edition string) (AccountEdition, error) { switch typedEdition := AccountEdition(strings.ToUpper(edition)); typedEdition { @@ -137,7 +144,7 @@ func (c *accounts) Create(ctx context.Context, id AccountObjectIdentifier, opts queryId := <-queryChanId rows, err := c.client.QueryUnsafe(gosnowflake.WithFetchResultByID(ctx, queryId), "") - if len(rows) == 1 && rows[0]["status"] != nil { + if err != nil && len(rows) == 1 && rows[0]["status"] != nil { if status, ok := (*rows[0]["status"]).(string); ok { return ToAccountCreateResponse(status) } diff --git a/pkg/sdk/accounts_test.go b/pkg/sdk/accounts_test.go index 8faca70e9c..3b5e48e5bd 100644 --- a/pkg/sdk/accounts_test.go +++ b/pkg/sdk/accounts_test.go @@ -3,9 +3,10 @@ package sdk import ( "encoding/json" "fmt" - "github.com/stretchr/testify/assert" "testing" + "github.com/stretchr/testify/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" ) From ad871181d6e2ffda271e939980066b1857ff6f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 6 Dec 2024 15:06:47 +0100 Subject: [PATCH 09/12] wip --- MIGRATION_GUIDE.md | 11 +++- .../resources/snowflake_account/import.sh | 1 + .../resources/snowflake_account/resource.tf | 40 ++++++++++-- .../resourceassert/account_resource_ext.go | 11 ++++ .../resourceassert/account_resource_gen.go | 10 +++ .../config/model/account_model_ext.go | 11 ++++ .../config/model/account_model_gen.go | 4 -- pkg/resources/account.go | 16 ++--- pkg/resources/account_acceptance_test.go | 61 +++++++++++++------ pkg/sdk/accounts.go | 2 +- templates/resources/account.md.tmpl | 15 +++-- 11 files changed, 140 insertions(+), 42 deletions(-) create mode 100644 examples/resources/snowflake_account/import.sh create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_ext.go create mode 100644 pkg/acceptance/bettertestspoc/config/model/account_model_ext.go diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index dbfef7d7f6..3afa12500e 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -9,9 +9,18 @@ across different versions. ## v0.99.0 ➞ v0.100.0 +### snowflake_account resource changes + +Changes: +- 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. +- The underlying resource identifier was changed from `` to `.`. Migration will be done automatically. Notice this introduces changes in how `snowflake_account` resource is imported. + + ### snowflake_tag_association resource changes #### *(behavior change)* new id format -In order to provide more functionality for tagging objects, we have changed the resource id from `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"` to `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"|TAG_VALUE|OBJECT_TYPE`. This allows to group tags associations per tag ID, tag value and object type in one resource. +To provide more functionality for tagging objects, we have changed the resource id from `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"` to `"TAG_DATABASE"."TAG_SCHEMA"."TAG_NAME"|TAG_VALUE|OBJECT_TYPE`. This allows to group tags associations per tag ID, tag value and object type in one resource. ``` resource "snowflake_tag_association" "gold_warehouses" { object_identifiers = [snowflake_warehouse.w1.fully_qualified_name, snowflake_warehouse.w2.fully_qualified_name] diff --git a/examples/resources/snowflake_account/import.sh b/examples/resources/snowflake_account/import.sh new file mode 100644 index 0000000000..8076279421 --- /dev/null +++ b/examples/resources/snowflake_account/import.sh @@ -0,0 +1 @@ +terraform import snowflake_account.example '"".""' diff --git a/examples/resources/snowflake_account/resource.tf b/examples/resources/snowflake_account/resource.tf index 6b53dacf8e..054e05c097 100644 --- a/examples/resources/snowflake_account/resource.tf +++ b/examples/resources/snowflake_account/resource.tf @@ -1,10 +1,42 @@ - -## TODO: - ## Minimal resource "snowflake_account" "minimal" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + email = "admin@email.com" + edition = "STANDARD" + grace_period_in_days = 3 +} + +## Complete (with SERVICE user type) +resource "snowflake_account" "complete" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_rsa_public_key = "" + admin_user_type = "SERVICE" + email = "admin@email.com" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" + grace_period_in_days = 3 } -## Complete (with every optional set) +## Complete (with PERSON user type) resource "snowflake_account" "complete" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + admin_user_type = "PERSON" + first_name = "first_name" + last_name = "last_name" + email = "admin@email.com" + must_change_password = "false" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" + grace_period_in_days = 3 } diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_ext.go new file mode 100644 index 0000000000..daf6dd018a --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_ext.go @@ -0,0 +1,11 @@ +package resourceassert + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +func (a *AccountResourceAssert) HasAdminUserType(expected sdk.UserType) *AccountResourceAssert { + a.AddAssertion(assert.ValueSet("admin_user_type", string(expected))) + return a +} diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_gen.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_gen.go index c68f6424c1..5d6f9d2d0a 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_gen.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/account_resource_gen.go @@ -47,6 +47,11 @@ func (a *AccountResourceAssert) HasAdminRsaPublicKeyString(expected string) *Acc return a } +func (a *AccountResourceAssert) HasAdminUserTypeString(expected string) *AccountResourceAssert { + a.AddAssertion(assert.ValueSet("admin_user_type", expected)) + return a +} + func (a *AccountResourceAssert) HasCommentString(expected string) *AccountResourceAssert { a.AddAssertion(assert.ValueSet("comment", expected)) return a @@ -126,6 +131,11 @@ func (a *AccountResourceAssert) HasNoAdminRsaPublicKey() *AccountResourceAssert return a } +func (a *AccountResourceAssert) HasNoAdminUserType() *AccountResourceAssert { + a.AddAssertion(assert.ValueNotSet("admin_user_type")) + return a +} + func (a *AccountResourceAssert) HasNoComment() *AccountResourceAssert { a.AddAssertion(assert.ValueNotSet("comment")) return a diff --git a/pkg/acceptance/bettertestspoc/config/model/account_model_ext.go b/pkg/acceptance/bettertestspoc/config/model/account_model_ext.go new file mode 100644 index 0000000000..4d81e2e589 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/config/model/account_model_ext.go @@ -0,0 +1,11 @@ +package model + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + tfconfig "github.com/hashicorp/terraform-plugin-testing/config" +) + +func (a *AccountModel) WithAdminUserTypeEnum(adminUserType sdk.UserType) *AccountModel { + a.AdminUserType = tfconfig.StringVariable(string(adminUserType)) + return a +} diff --git a/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go index 8b616f90de..b1000b7931 100644 --- a/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go +++ b/pkg/acceptance/bettertestspoc/config/model/account_model_gen.go @@ -37,7 +37,6 @@ type AccountModel struct { func Account( resourceName string, adminName string, - adminUserType string, edition string, email string, gracePeriodInDays int, @@ -45,7 +44,6 @@ func Account( ) *AccountModel { a := &AccountModel{ResourceModelMeta: config.Meta(resourceName, resources.Account)} a.WithAdminName(adminName) - a.WithAdminUserType(adminUserType) a.WithEdition(edition) a.WithEmail(email) a.WithGracePeriodInDays(gracePeriodInDays) @@ -55,7 +53,6 @@ func Account( func AccountWithDefaultMeta( adminName string, - adminUserType string, edition string, email string, gracePeriodInDays int, @@ -63,7 +60,6 @@ func AccountWithDefaultMeta( ) *AccountModel { a := &AccountModel{ResourceModelMeta: config.DefaultMeta(resources.Account)} a.WithAdminName(adminName) - a.WithAdminUserType(adminUserType) a.WithEdition(edition) a.WithEmail(email) a.WithGracePeriodInDays(gracePeriodInDays) diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 22a620fc09..5144f94d17 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -24,10 +24,9 @@ import ( 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.", - ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), + 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.", }, "admin_name": { Type: schema.TypeString, @@ -40,7 +39,7 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("Password for the initial administrative user of the account. Either admin_password or admin_rsa_public_key has to be specified."), + Description: externalChangesNotDetectedFieldDescription("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."), DiffSuppressFunc: IgnoreAfterCreation, AtLeastOneOf: []string{"admin_password", "admin_rsa_public_key"}, }, @@ -63,14 +62,14 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("First name of the initial administrative user of the account."), + Description: externalChangesNotDetectedFieldDescription("First name of the initial administrative user of the account. This field cannot be used whenever admin_user_type is set to SERVICE."), DiffSuppressFunc: IgnoreAfterCreation, }, "last_name": { Type: schema.TypeString, Optional: true, Sensitive: true, - Description: externalChangesNotDetectedFieldDescription("Last name of the initial administrative user of the account."), + Description: externalChangesNotDetectedFieldDescription("Last name of the initial administrative user of the account. This field cannot be used whenever admin_user_type is set to SERVICE."), DiffSuppressFunc: IgnoreAfterCreation, }, "email": { @@ -84,7 +83,7 @@ var accountSchema = map[string]*schema.Schema{ Type: schema.TypeString, Optional: true, Default: BooleanDefault, - Description: externalChangesNotDetectedFieldDescription("Specifies whether the new user created to administer the account is forced to change their password upon first login into the account."), + Description: externalChangesNotDetectedFieldDescription("Specifies whether the new user created to administer the account is forced to change their password upon first login into the account. This field cannot be used whenever admin_user_type is set to SERVICE."), DiffSuppressFunc: IgnoreAfterCreation, ValidateDiagFunc: validateBooleanString, }, @@ -93,6 +92,7 @@ var accountSchema = map[string]*schema.Schema{ Required: true, ForceNew: true, Description: fmt.Sprintf("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: %s", docs.PossibleValuesListed(sdk.AllAccountEditions)), + DiffSuppressFunc: NormalizeAndCompare(sdk.ToAccountEdition), ValidateDiagFunc: sdkValidation(sdk.ToAccountEdition), }, "region_group": { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index 94afd44903..5758b683ae 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -2,6 +2,7 @@ package resources_test import ( "fmt" + tfconfig "github.com/hashicorp/terraform-plugin-testing/config" "regexp" "testing" @@ -37,7 +38,7 @@ func TestAcc_Account_Minimal(t *testing.T) { key, _ := random.GenerateRSAPublicKey(t) region := acc.TestClient().Context.CurrentRegion(t) - configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). WithAdminRsaPublicKey(key) resource.Test(t, resource.TestCase{ @@ -55,6 +56,7 @@ func TestAcc_Account_Minimal(t *testing.T) { HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasAdminNameString(name). HasAdminRsaPublicKeyString(key). + HasNoAdminUserType(). HasEmailString(email). HasNoFirstName(). HasNoLastName(). @@ -106,6 +108,7 @@ func TestAcc_Account_Minimal(t *testing.T) { HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasNoAdminName(). HasNoAdminRsaPublicKey(). + HasNoAdminUserType(). HasNoEmail(). HasNoFirstName(). HasNoLastName(). @@ -137,7 +140,8 @@ func TestAcc_Account_Complete(t *testing.T) { region := acc.TestClient().Context.CurrentRegion(t) comment := random.Comment() - configModel := model.Account("test", name, string(sdk.UserTypePerson), string(sdk.EditionStandard), email, 3, id). + configModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypePerson). WithAdminRsaPublicKey(key). WithFirstName(firstName). WithLastName(lastName). @@ -162,6 +166,7 @@ func TestAcc_Account_Complete(t *testing.T) { HasFullyQualifiedNameString(sdk.NewAccountIdentifier(organizationName, id).FullyQualifiedName()). HasAdminNameString(name). HasAdminRsaPublicKeyString(key). + HasAdminUserType(sdk.UserTypePerson). HasEmailString(email). HasFirstNameString(firstName). HasLastNameString(lastName). @@ -216,6 +221,7 @@ func TestAcc_Account_Complete(t *testing.T) { HasNoEmail(). HasNoFirstName(). HasNoLastName(). + HasNoAdminUserType(). HasNoMustChangePassword(). HasEditionString(string(sdk.EditionStandard)). HasNoRegionGroup(). @@ -244,9 +250,11 @@ func TestAcc_Account_Rename(t *testing.T) { name := random.AdminName() key, _ := random.GenerateRSAPublicKey(t) - configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) - newConfigModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, newId). + newConfigModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, newId). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) resource.Test(t, resource.TestCase{ @@ -261,7 +269,8 @@ func TestAcc_Account_Rename(t *testing.T) { Check: assert.AssertThat(t, resourceassert.AccountResource(t, configModel.ResourceReference()). HasNameString(id). - HasFullyQualifiedNameString(accountId.FullyQualifiedName()), + HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService), resourceshowoutputassert.AccountShowOutput(t, configModel.ResourceReference()). HasOrganizationName(organizationName). HasAccountName(id), @@ -277,7 +286,8 @@ func TestAcc_Account_Rename(t *testing.T) { Check: assert.AssertThat(t, resourceassert.AccountResource(t, newConfigModel.ResourceReference()). HasNameString(newId). - HasFullyQualifiedNameString(newAccountId.FullyQualifiedName()), + HasFullyQualifiedNameString(newAccountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService), resourceshowoutputassert.AccountShowOutput(t, newConfigModel.ResourceReference()). HasOrganizationName(organizationName). HasAccountName(newId), @@ -299,15 +309,18 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) { name := random.AdminName() key, _ := random.GenerateRSAPublicKey(t) - configModelWithOrgAdminTrue := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModelWithOrgAdminTrue := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key). WithIsOrgAdmin(r.BooleanTrue) - configModelWithOrgAdminFalse := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModelWithOrgAdminFalse := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key). WithIsOrgAdmin(r.BooleanFalse) - configModelWithoutOrgAdmin := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModelWithoutOrgAdmin := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) resource.Test(t, resource.TestCase{ @@ -324,6 +337,7 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) { resourceassert.AccountResource(t, configModelWithOrgAdminTrue.ResourceReference()). HasNameString(id). HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService). HasIsOrgAdminString(r.BooleanTrue), resourceshowoutputassert.AccountShowOutput(t, configModelWithOrgAdminTrue.ResourceReference()). HasOrganizationName(organizationName). @@ -343,6 +357,7 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) { resourceassert.AccountResource(t, configModelWithOrgAdminFalse.ResourceReference()). HasNameString(id). HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService). HasIsOrgAdminString(r.BooleanFalse), resourceshowoutputassert.AccountShowOutput(t, configModelWithOrgAdminFalse.ResourceReference()). HasOrganizationName(organizationName). @@ -362,6 +377,7 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) { resourceassert.AccountResource(t, configModelWithoutOrgAdmin.ResourceReference()). HasNameString(id). HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService). HasIsOrgAdminString(r.BooleanDefault), resourceshowoutputassert.AccountShowOutput(t, configModelWithoutOrgAdmin.ResourceReference()). HasOrganizationName(organizationName). @@ -389,6 +405,7 @@ func TestAcc_Account_IsOrgAdmin(t *testing.T) { resourceassert.AccountResource(t, configModelWithoutOrgAdmin.ResourceReference()). HasNameString(id). HasFullyQualifiedNameString(accountId.FullyQualifiedName()). + HasAdminUserType(sdk.UserTypeService). HasIsOrgAdminString(r.BooleanDefault), resourceshowoutputassert.AccountShowOutput(t, configModelWithoutOrgAdmin.ResourceReference()). HasOrganizationName(organizationName). @@ -420,13 +437,15 @@ func TestAcc_Account_IgnoreUpdateAfterCreationOnCertainFields(t *testing.T) { newName := random.AdminName() newPass := random.Password() - configModel := model.Account("test", name, string(sdk.UserTypePerson), string(sdk.EditionStandard), email, 3, id). + configModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypePerson). WithFirstName(firstName). WithLastName(lastName). WithMustChangePassword(r.BooleanTrue). WithAdminPassword(pass) - newConfigModel := model.Account("test", newName, string(sdk.UserTypeService), string(sdk.EditionStandard), newEmail, 3, id). + newConfigModel := model.Account("test", newName, string(sdk.EditionStandard), newEmail, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminPassword(newPass). WithFirstName(newFirstName). WithLastName(newLastName) @@ -446,7 +465,7 @@ func TestAcc_Account_IgnoreUpdateAfterCreationOnCertainFields(t *testing.T) { HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasAdminNameString(name). HasAdminPasswordString(pass). - // HasAdminUserType(). TODO + HasAdminUserType(sdk.UserTypePerson). HasEmailString(email). HasFirstNameString(firstName). HasLastNameString(lastName). @@ -466,6 +485,7 @@ func TestAcc_Account_IgnoreUpdateAfterCreationOnCertainFields(t *testing.T) { HasFullyQualifiedNameString(accountId.FullyQualifiedName()). HasAdminNameString(name). HasAdminPasswordString(pass). + HasAdminUserType(sdk.UserTypePerson). HasEmailString(email). HasFirstNameString(firstName). HasLastNameString(lastName). @@ -488,7 +508,8 @@ func TestAcc_Account_TryToCreateWithoutOrgadmin(t *testing.T) { t.Setenv(string(testenvs.ConfigureClientOnce), "") t.Setenv(snowflakeenvs.Role, snowflakeroles.Accountadmin.Name()) - configModel := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, id). + configModel := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) resource.Test(t, resource.TestCase{ @@ -515,13 +536,16 @@ func TestAcc_Account_InvalidValues(t *testing.T) { name := random.AdminName() key, _ := random.GenerateRSAPublicKey(t) - configModelInvalidUserType := model.Account("test", name, "invalid_user_type", string(sdk.EditionStandard), email, 3, id). + configModelInvalidUserType := model.Account("test", name, string(sdk.EditionStandard), email, 3, id). + WithAdminUserType("invalid_user_type"). WithAdminRsaPublicKey(key) - configModelInvalidAccountEdition := model.Account("test", name, string(sdk.UserTypeService), "invalid_account_edition", email, 3, id). + configModelInvalidAccountEdition := model.Account("test", name, "invalid_account_edition", email, 3, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) - configModelInvalidGracePeriodInDays := model.Account("test", name, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 2, id). + configModelInvalidGracePeriodInDays := model.Account("test", name, string(sdk.EditionStandard), email, 2, id). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminRsaPublicKey(key) resource.Test(t, resource.TestCase{ @@ -560,11 +584,12 @@ func TestAcc_Account_UpgradeFrom_v0_99_0(t *testing.T) { region := acc.TestClient().Context.CurrentRegion(t) comment := random.Comment() - configModel := model.Account("test", adminName, string(sdk.UserTypeService), string(sdk.EditionStandard), email, 3, name). + configModel := model.Account("test", adminName, string(sdk.EditionStandard), email, 3, name). + WithAdminUserTypeEnum(sdk.UserTypeService). WithAdminPassword(adminPassword). WithFirstName(firstName). WithLastName(lastName). - WithMustChangePassword(r.BooleanTrue). + WithMustChangePasswordValue(tfconfig.BoolVariable(true)). WithRegion(region). WithIsOrgAdmin(r.BooleanFalse). WithComment(comment) diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 94a4dd8666..33ad208a64 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -144,7 +144,7 @@ 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 && len(rows) == 1 && rows[0]["status"] != nil { if status, ok := (*rows[0]["status"]).(string); ok { return ToAccountCreateResponse(status) } diff --git a/templates/resources/account.md.tmpl b/templates/resources/account.md.tmpl index 973e844784..c05e6ff4bc 100644 --- a/templates/resources/account.md.tmpl +++ b/templates/resources/account.md.tmpl @@ -9,26 +9,29 @@ description: |- {{- end }} --- +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0990--v01000) to use it. + # {{.Name}} ({{.Type}}) {{ .Description | trimspace }} -!> **Warning** This resource cannot be destroyed!!! The only way to delete accounts is to go through [Snowflake Support](https://docs.snowflake.com/en/user-guide/organizations-manage-accounts.html#deleting-an-account) - -~> **Note** ORGADMIN priviliges are required for this resource +~> **Note** To use this resource you have to use an account with a privilege to use the ORGADMIN role. +{{ if .HasExample -}} ## Example Usage {{ tffile (printf "examples/resources/%s/resource.tf" .Name)}} -> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources). +{{- end }} + {{ .SchemaMarkdown | trimspace }} +{{- if .HasImport }} ## Import Import is supported using the following syntax: -```shell -terraform import snowflake_account.account -``` +{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}} +{{- end }} From bbcbc2f8235d6e3e22c3c1312c232bfdd773ad12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 6 Dec 2024 15:42:35 +0100 Subject: [PATCH 10/12] wip --- MIGRATION_GUIDE.md | 2 +- docs/resources/account.md | 59 +++++++++++---- .../resources/snowflake_account/resource.tf | 52 ++++++------- pkg/resources/account.go | 75 ++++++++++++------- pkg/resources/account_acceptance_test.go | 3 +- pkg/schemas/account_gen.go | 1 - pkg/sdk/accounts.go | 1 - pkg/sdk/accounts_test.go | 40 ++++++++++ 8 files changed, 165 insertions(+), 68 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 3afa12500e..095643f1c8 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -16,7 +16,7 @@ Changes: - `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. - The underlying resource identifier was changed from `` to `.`. 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)). ### snowflake_tag_association resource changes #### *(behavior change)* new id format diff --git a/docs/resources/account.md b/docs/resources/account.md index 1caf592f99..eb9ae228bd 100644 --- a/docs/resources/account.md +++ b/docs/resources/account.md @@ -2,28 +2,61 @@ page_title: "snowflake_account Resource - terraform-provider-snowflake" subcategory: "" description: |- - The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role. + The account resource allows you to create and manage Snowflake accounts. --- -# snowflake_account (Resource) +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0990--v01000) to use it. -The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role. +# snowflake_account (Resource) -!> **Warning** This resource cannot be destroyed!!! The only way to delete accounts is to go through [Snowflake Support](https://docs.snowflake.com/en/user-guide/organizations-manage-accounts.html#deleting-an-account) +The account resource allows you to create and manage Snowflake accounts. -~> **Note** ORGADMIN priviliges are required for this resource +~> **Note** To use this resource you have to use an account with a privilege to use the ORGADMIN role. ## Example Usage ```terraform -## TODO: - ## Minimal resource "snowflake_account" "minimal" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + email = "admin@email.com" + edition = "STANDARD" + grace_period_in_days = 3 +} + +## Complete (with SERVICE user type) +resource "snowflake_account" "complete" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_rsa_public_key = "" + admin_user_type = "SERVICE" + email = "admin@email.com" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" + grace_period_in_days = 3 } -## Complete (with every optional set) +## Complete (with PERSON user type) resource "snowflake_account" "complete" { + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + admin_user_type = "PERSON" + first_name = "first_name" + last_name = "last_name" + email = "admin@email.com" + must_change_password = "false" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" + grace_period_in_days = 3 } ``` -> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources). @@ -42,14 +75,14 @@ resource "snowflake_account" "complete" { ### 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. 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_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_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. 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". +- `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". - `is_org_admin` (String) Sets an account property that determines whether the ORGADMIN role is enabled in the account. Only an organization administrator (i.e. user with the ORGADMIN role) can set the property. -- `last_name` (String, Sensitive) Last name of the initial administrative user of 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". -- `must_change_password` (String) Specifies whether the new user created to administer the account is forced to change their password upon first login into 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". +- `last_name` (String, Sensitive) Last 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". +- `must_change_password` (String) Specifies whether the new user created to administer the account is forced to change their password upon first login into 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". - `region` (String) [Snowflake Region ID](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-snowflake-region-ids) of the region where the account is created. If no value is provided, Snowflake creates the account in the same Snowflake Region as the current account (i.e. the account in which the CREATE ACCOUNT statement is executed.) - `region_group` (String) ID of the region group where the account is created. To retrieve the region group ID for existing accounts in your organization, execute the [SHOW REGIONS](https://docs.snowflake.com/en/sql-reference/sql/show-regions) command. For information about when you might need to specify region group, see [Region groups](https://docs.snowflake.com/en/user-guide/admin-account-identifier.html#label-region-groups). @@ -99,5 +132,5 @@ Read-Only: Import is supported using the following syntax: ```shell -terraform import snowflake_account.account +terraform import snowflake_account.example '"".""' ``` diff --git a/examples/resources/snowflake_account/resource.tf b/examples/resources/snowflake_account/resource.tf index 054e05c097..a9e61e2d3f 100644 --- a/examples/resources/snowflake_account/resource.tf +++ b/examples/resources/snowflake_account/resource.tf @@ -1,42 +1,42 @@ ## Minimal resource "snowflake_account" "minimal" { - name = "ACCOUNT_NAME" - admin_name = "ADMIN_NAME" - admin_password = "ADMIN_PASSWORD" - email = "admin@email.com" - edition = "STANDARD" + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + email = "admin@email.com" + edition = "STANDARD" grace_period_in_days = 3 } ## Complete (with SERVICE user type) resource "snowflake_account" "complete" { - name = "ACCOUNT_NAME" - admin_name = "ADMIN_NAME" + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" admin_rsa_public_key = "" - admin_user_type = "SERVICE" - email = "admin@email.com" - edition = "STANDARD" - region_group = "PUBLIC" - region = "AWS_US_WEST_2" - comment = "some comment" - is_org_admin = "true" + admin_user_type = "SERVICE" + email = "admin@email.com" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" grace_period_in_days = 3 } ## Complete (with PERSON user type) resource "snowflake_account" "complete" { - name = "ACCOUNT_NAME" - admin_name = "ADMIN_NAME" - admin_password = "ADMIN_PASSWORD" - admin_user_type = "PERSON" - first_name = "first_name" - last_name = "last_name" - email = "admin@email.com" + name = "ACCOUNT_NAME" + admin_name = "ADMIN_NAME" + admin_password = "ADMIN_PASSWORD" + admin_user_type = "PERSON" + first_name = "first_name" + last_name = "last_name" + email = "admin@email.com" must_change_password = "false" - edition = "STANDARD" - region_group = "PUBLIC" - region = "AWS_US_WEST_2" - comment = "some comment" - is_org_admin = "true" + edition = "STANDARD" + region_group = "PUBLIC" + region = "AWS_US_WEST_2" + comment = "some comment" + is_org_admin = "true" grace_period_in_days = 3 } diff --git a/pkg/resources/account.go b/pkg/resources/account.go index 5144f94d17..cfe0d31a17 100644 --- a/pkg/resources/account.go +++ b/pkg/resources/account.go @@ -146,7 +146,7 @@ var accountSchema = map[string]*schema.Schema{ func Account() *schema.Resource { return &schema.Resource{ - Description: "The account resource allows you to create and manage Snowflake accounts. To use this resource, make sure you use an account with the ORGADMIN role.", + Description: "The account resource allows you to create and manage Snowflake accounts.", CreateContext: TrackingCreateWrapper(resources.Account, CreateAccount), ReadContext: TrackingReadWrapper(resources.Account, ReadAccount(true)), UpdateContext: TrackingUpdateWrapper(resources.Account, UpdateAccount), @@ -182,7 +182,6 @@ func ImportAccount(ctx context.Context, d *schema.ResourceData, meta any) ([]*sc return nil, err } if !isOrgAdmin { - // TODO: return nil, errors.New("current user doesn't have the orgadmin role in session") } @@ -314,6 +313,16 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { account, err := client.Accounts.ShowByID(ctx, id.AccountId()) if err != nil { + if errors.Is(err, sdk.ErrObjectNotFound) { + d.SetId("") + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Failed to query account. Marking the resource as removed.", + Detail: fmt.Sprintf("Account: %s, Err: %s", id.FullyQualifiedName(), err), + }, + } + } return diag.FromErr(err) } @@ -321,6 +330,12 @@ func ReadAccount(withExternalChangesMarking bool) schema.ReadContextFunc { var regionGroup string if account.RegionGroup != nil { regionGroup = *account.RegionGroup + + // For organizations that have accounts in multiple region groups, returns . so we need to split on "." + parts := strings.Split(regionGroup, ".") + if len(parts) == 2 { + regionGroup = parts[0] + } } if err = handleExternalChangesToObjectInShow(d, outputMapping{"edition", "edition", *account.Edition, *account.Edition, nil}, @@ -398,29 +413,39 @@ func UpdateAccount(ctx context.Context, d *schema.ResourceData, meta any) diag.D } if d.HasChange("is_org_admin") { - if v := d.Get("is_org_admin").(string); v != BooleanDefault { - parsed, err := booleanStringToBool(v) - if err != nil { - return diag.FromErr(err) - } - if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ - SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ - Name: id.AccountId(), - OrgAdmin: parsed, - }, - }); err != nil { - return diag.FromErr(err) - } - } else { - // No unset available for this field (setting Snowflake default) - if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ - SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ - Name: id.AccountId(), - OrgAdmin: false, - }, - // This error may happen when a user removes is_org_admin, and previously it was explicitly set to false. - }); err != nil && !strings.Contains(err.Error(), "already has ORGADMIN disabled") { - return diag.FromErr(err) + oldIsOrgAdmin, newIsOrgAdmin := d.GetChange("is_org_admin") + + // Setting from default to false and vice versa is not allowed because Snowflake throws an error on already disabled IsOrgAdmin + canUpdate := true + if (oldIsOrgAdmin.(string) == BooleanFalse && newIsOrgAdmin.(string) == BooleanDefault) || + (oldIsOrgAdmin.(string) == BooleanDefault && newIsOrgAdmin.(string) == BooleanFalse) { + canUpdate = false + } + + if canUpdate { + if newIsOrgAdmin.(string) != BooleanDefault { + parsed, err := booleanStringToBool(newIsOrgAdmin.(string)) + if err != nil { + return diag.FromErr(err) + } + if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: id.AccountId(), + OrgAdmin: parsed, + }, + }); err != nil { + return diag.FromErr(err) + } + } else { + // No unset available for this field (setting Snowflake default) + if err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + SetIsOrgAdmin: &sdk.AccountSetIsOrgAdmin{ + Name: id.AccountId(), + OrgAdmin: false, + }, + }); err != nil { + return diag.FromErr(err) + } } } } diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index 5758b683ae..3ea105f44a 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/pkg/resources/account_acceptance_test.go @@ -2,10 +2,11 @@ package resources_test import ( "fmt" - tfconfig "github.com/hashicorp/terraform-plugin-testing/config" "regexp" "testing" + tfconfig "github.com/hashicorp/terraform-plugin-testing/config" + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" diff --git a/pkg/schemas/account_gen.go b/pkg/schemas/account_gen.go index 03be3a920e..5ef4c855c3 100644 --- a/pkg/schemas/account_gen.go +++ b/pkg/schemas/account_gen.go @@ -134,7 +134,6 @@ func AccountToSchema(account *sdk.Account) map[string]any { accountSchema["organization_name"] = account.OrganizationName accountSchema["account_name"] = account.AccountName accountSchema["snowflake_region"] = account.SnowflakeRegion - // TODO: Check if populated or have to deref if account.RegionGroup != nil { accountSchema["region_group"] = account.RegionGroup } diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 33ad208a64..6f42530bb6 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -50,7 +50,6 @@ var AllAccountEditions = []AccountEdition{ EditionBusinessCritical, } -// TODO: test func ToAccountEdition(edition string) (AccountEdition, error) { switch typedEdition := AccountEdition(strings.ToUpper(edition)); typedEdition { case EditionStandard, EditionEnterprise, EditionBusinessCritical: diff --git a/pkg/sdk/accounts_test.go b/pkg/sdk/accounts_test.go index 3b5e48e5bd..df48048ef4 100644 --- a/pkg/sdk/accounts_test.go +++ b/pkg/sdk/accounts_test.go @@ -3,6 +3,7 @@ package sdk import ( "encoding/json" "fmt" + "github.com/stretchr/testify/require" "testing" "github.com/stretchr/testify/assert" @@ -520,3 +521,42 @@ func TestToAccountCreateResponse(t *testing.T) { }) } } + +func TestToAccountEdition(t *testing.T) { + type test struct { + input string + want AccountEdition + } + + valid := []test{ + // case insensitive. + {input: "standard", want: EditionStandard}, + + // Supported Values + {input: "STANDARD", want: EditionStandard}, + {input: "ENTERPRISE", want: EditionEnterprise}, + {input: "BUSINESS_CRITICAL", want: EditionBusinessCritical}, + } + + invalid := []test{ + // bad values + {input: ""}, + {input: "foo"}, + {input: "businesscritical"}, + } + + for _, tc := range valid { + t.Run(tc.input, func(t *testing.T) { + got, err := ToAccountEdition(tc.input) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range invalid { + t.Run(tc.input, func(t *testing.T) { + _, err := ToAccountEdition(tc.input) + require.Error(t, err) + }) + } +} From 9893fabbfeba275fadc4e7d979611d9cdf771634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 6 Dec 2024 15:45:06 +0100 Subject: [PATCH 11/12] wip --- .../assert/objectassert/account_snowflake_gen.go | 6 +++--- pkg/acceptance/helpers/account_client.go | 2 +- pkg/datasources/accounts.go | 2 +- pkg/schemas/account_gen.go | 4 ++-- pkg/sdk/accounts.go | 4 ++-- pkg/sdk/accounts_test.go | 5 ++--- pkg/sdk/testint/accounts_integration_test.go | 7 +++---- 7 files changed, 14 insertions(+), 16 deletions(-) diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/account_snowflake_gen.go b/pkg/acceptance/bettertestspoc/assert/objectassert/account_snowflake_gen.go index d394c1ae39..6ba8eceb11 100644 --- a/pkg/acceptance/bettertestspoc/assert/objectassert/account_snowflake_gen.go +++ b/pkg/acceptance/bettertestspoc/assert/objectassert/account_snowflake_gen.go @@ -148,11 +148,11 @@ func (a *AccountAssert) HasAccountLocator(expected string) *AccountAssert { func (a *AccountAssert) HasAccountLocatorURL(expected string) *AccountAssert { a.AddAssertion(func(t *testing.T, o *sdk.Account) error { t.Helper() - if o.AccountLocatorURL == nil { + if o.AccountLocatorUrl == nil { return fmt.Errorf("expected account locator url to have value; got: nil") } - if *o.AccountLocatorURL != expected { - return fmt.Errorf("expected account locator url: %v; got: %v", expected, *o.AccountLocatorURL) + if *o.AccountLocatorUrl != expected { + return fmt.Errorf("expected account locator url: %v; got: %v", expected, *o.AccountLocatorUrl) } return nil }) diff --git a/pkg/acceptance/helpers/account_client.go b/pkg/acceptance/helpers/account_client.go index c9bbe245c6..b91d10579a 100644 --- a/pkg/acceptance/helpers/account_client.go +++ b/pkg/acceptance/helpers/account_client.go @@ -141,7 +141,7 @@ func (c *AccountClient) CreateAndLogIn(t *testing.T) (*sdk.Account, *sdk.Client, newClient, err := sdk.NewClient(&gosnowflake.Config{ Account: fmt.Sprintf("%s-%s", account.OrganizationName, account.AccountName), User: name, - Host: strings.TrimPrefix(*account.AccountLocatorURL, `https://`), + Host: strings.TrimPrefix(*account.AccountLocatorUrl, `https://`), Authenticator: gosnowflake.AuthTypeJwt, PrivateKey: privateKey, Role: snowflakeroles.Accountadmin.Name(), diff --git a/pkg/datasources/accounts.go b/pkg/datasources/accounts.go index d5aff6abec..89efac8d7e 100644 --- a/pkg/datasources/accounts.go +++ b/pkg/datasources/accounts.go @@ -153,7 +153,7 @@ func ReadAccounts(ctx context.Context, d *schema.ResourceData, meta any) diag.Di m["created_on"] = account.CreatedOn.String() m["comment"] = account.Comment m["account_locator"] = account.AccountLocator - m["account_locator_url"] = account.AccountLocatorURL + m["account_locator_url"] = account.AccountLocatorUrl m["managed_accounts"] = account.ManagedAccounts m["consumption_billing_entity_name"] = account.ConsumptionBillingEntityName m["marketplace_consumer_billing_entity_name"] = account.MarketplaceConsumerBillingEntityName diff --git a/pkg/schemas/account_gen.go b/pkg/schemas/account_gen.go index 5ef4c855c3..e6f4413875 100644 --- a/pkg/schemas/account_gen.go +++ b/pkg/schemas/account_gen.go @@ -151,8 +151,8 @@ func AccountToSchema(account *sdk.Account) map[string]any { accountSchema["comment"] = account.Comment } accountSchema["account_locator"] = account.AccountLocator - if account.AccountLocatorURL != nil { - accountSchema["account_locator_url"] = account.AccountLocatorURL + if account.AccountLocatorUrl != nil { + accountSchema["account_locator_url"] = account.AccountLocatorUrl } if account.ManagedAccounts != nil { accountSchema["managed_accounts"] = account.ManagedAccounts diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index 6f42530bb6..a80912ccfb 100644 --- a/pkg/sdk/accounts.go +++ b/pkg/sdk/accounts.go @@ -362,7 +362,7 @@ type Account struct { CreatedOn *time.Time Comment *string AccountLocator string - AccountLocatorURL *string + AccountLocatorUrl *string ManagedAccounts *int ConsumptionBillingEntityName *string MarketplaceConsumerBillingEntityName *string @@ -450,7 +450,7 @@ func (row accountDBRow) convert() *Account { acc.Comment = &row.Comment.String } if row.AccountLocatorURL.Valid { - acc.AccountLocatorURL = &row.AccountLocatorURL.String + acc.AccountLocatorUrl = &row.AccountLocatorURL.String } if row.ManagedAccounts.Valid { acc.ManagedAccounts = Int(int(row.ManagedAccounts.Int32)) diff --git a/pkg/sdk/accounts_test.go b/pkg/sdk/accounts_test.go index df48048ef4..e072eabd71 100644 --- a/pkg/sdk/accounts_test.go +++ b/pkg/sdk/accounts_test.go @@ -3,12 +3,11 @@ package sdk import ( "encoding/json" "fmt" - "github.com/stretchr/testify/require" "testing" - "github.com/stretchr/testify/assert" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAccountCreate(t *testing.T) { diff --git a/pkg/sdk/testint/accounts_integration_test.go b/pkg/sdk/testint/accounts_integration_test.go index 4b8bea8ba6..7b052ab95f 100644 --- a/pkg/sdk/testint/accounts_integration_test.go +++ b/pkg/sdk/testint/accounts_integration_test.go @@ -37,7 +37,7 @@ func TestInt_Account(t *testing.T) { assert.NotEmpty(t, *account.CreatedOn) assert.Equal(t, "SNOWFLAKE", *account.Comment) assert.NotEmpty(t, account.AccountLocator) - assert.NotEmpty(t, *account.AccountLocatorURL) + assert.NotEmpty(t, *account.AccountLocatorUrl) assert.Zero(t, *account.ManagedAccounts) assert.NotEmpty(t, *account.ConsumptionBillingEntityName) assert.Nil(t, account.MarketplaceConsumerBillingEntityName) @@ -65,7 +65,7 @@ func TestInt_Account(t *testing.T) { assert.Nil(t, account.AccountURL) assert.Nil(t, account.CreatedOn) assert.Nil(t, account.Comment) - assert.Nil(t, account.AccountLocatorURL) + assert.Nil(t, account.AccountLocatorUrl) assert.Nil(t, account.ManagedAccounts) assert.Nil(t, account.ConsumptionBillingEntityName) assert.Nil(t, account.MarketplaceConsumerBillingEntityName) @@ -97,8 +97,7 @@ func TestInt_Account(t *testing.T) { assert.NotNil(t, response) if response != nil { assert.Equal(t, account.AccountLocator, response.AccountLocator) - // TODO: Rename to Url - assert.Equal(t, *account.AccountLocatorURL, response.AccountLocatorUrl) + 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) From d2b6f6fade16d8d9e2c37c18846e28964e234879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Tue, 10 Dec 2024 09:51:01 +0100 Subject: [PATCH 12/12] changes after review --- MIGRATION_GUIDE.md | 4 +++- docs/resources/account.md | 4 ++-- pkg/resources/account.go | 17 +++++++-------- pkg/resources/account_acceptance_test.go | 2 +- pkg/resources/account_state_upgraders.go | 4 ++++ pkg/sdk/accounts.go | 7 ++++++- pkg/sdk/identifier_helpers.go | 2 +- pkg/sdk/testint/accounts_integration_test.go | 22 +++++++++----------- 8 files changed, 35 insertions(+), 27 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 095643f1c8..f9c746e6e6 100644 --- a/MIGRATION_GUIDE.md +++ b/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 `` to `.`. 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)). diff --git a/docs/resources/account.md b/docs/resources/account.md index eb9ae228bd..6597e1e855 100644 --- a/docs/resources/account.md +++ b/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". diff --git a/pkg/resources/account.go b/pkg/resources/account.go index cfe0d31a17..2f5a1e0249 100644 --- a/pkg/resources/account.go +++ b/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 { diff --git a/pkg/resources/account_acceptance_test.go b/pkg/resources/account_acceptance_test.go index 3ea105f44a..3b2e699d9f 100644 --- a/pkg/resources/account_acceptance_test.go +++ b/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, }, }) diff --git a/pkg/resources/account_state_upgraders.go b/pkg/resources/account_state_upgraders.go index 63965731c8..bfd0b60bba 100644 --- a/pkg/resources/account_state_upgraders.go +++ b/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)) diff --git a/pkg/sdk/accounts.go b/pkg/sdk/accounts.go index a80912ccfb..997c5f2086 100644 --- a/pkg/sdk/accounts.go +++ b/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) } diff --git a/pkg/sdk/identifier_helpers.go b/pkg/sdk/identifier_helpers.go index 8ce7a23850..b45932b3a7 100644 --- a/pkg/sdk/identifier_helpers.go +++ b/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) } diff --git a/pkg/sdk/testint/accounts_integration_test.go b/pkg/sdk/testint/accounts_integration_test.go index 7b052ab95f..6ed6b1ac5d 100644 --- a/pkg/sdk/testint/accounts_integration_test.go +++ b/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) {