Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix managed account and adjust account ticket numbers #3395

Merged
merged 4 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,8 @@ var allResourceSchemaDefs = []ResourceSchemaDef{
name: "ProcedureSql",
schema: resources.ProcedureSql().Schema,
},
{
name: "ManagedAccount",
schema: resources.ManagedAccount().Schema,
},
}
52 changes: 52 additions & 0 deletions pkg/resources/managed_account_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-testing/plancheck"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
Expand Down Expand Up @@ -54,6 +56,56 @@ func TestAcc_ManagedAccount(t *testing.T) {
})
}

func TestAcc_ManagedAccount_HandleShowOutputChanges_BCR_2024_08(t *testing.T) {
// TODO [SNOW-1011985]: unskip the tests
testenvs.SkipTestIfSet(t, testenvs.SkipManagedAccountTest, "error: 090337 (23001): Number of managed accounts allowed exceeded the limit. Please contact Snowflake support")

id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
adminName := acc.TestClient().Ids.Alpha()
adminPass := acc.TestClient().Ids.AlphaWithPrefix("ABC_abc_123!!!")

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDestroy(t, resources.ManagedAccount),
Steps: []resource.TestStep{
{
Config: managedAccountConfig(id.Name(), adminName, adminPass),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_managed_account.test", "name", id.Name()),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "fully_qualified_name", id.FullyQualifiedName()),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "admin_name", adminName),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "admin_password", adminPass),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "comment", managedAccountComment),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "type", "READER"),
),
},
{
PreConfig: func() {
acc.TestClient().BcrBundles.EnableBcrBundle(t, "2024_08")
},
Config: managedAccountConfig(id.Name(), adminName, adminPass),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("snowflake_managed_account.test", plancheck.ResourceActionNoop),
},
},
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_managed_account.test", "name", id.Name()),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "fully_qualified_name", id.FullyQualifiedName()),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "admin_name", adminName),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "admin_password", adminPass),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "comment", managedAccountComment),
resource.TestCheckResourceAttr("snowflake_managed_account.test", "type", "READER"),
),
},
},
})
}

func managedAccountConfig(accName, aName, aPass string) string {
return fmt.Sprintf(`
resource "snowflake_managed_account" "test" {
Expand Down
9 changes: 6 additions & 3 deletions pkg/sdk/managed_accounts_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import g "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/poc/gen
//go:generate go run ./poc/main.go

var managedAccountDbRow = g.DbStruct("managedAccountDBRow").
Text("name").
OptionalText("name").
OptionalText("account_name").
Text("cloud").
Text("region").
Text("locator").
OptionalText("locator").
OptionalText("account_locator").
Text("created_on").
Text("url").
OptionalText("url").
OptionalText("account_url").
Text("account_locator_url").
Bool("is_reader").
OptionalText("comment")
Expand Down
19 changes: 15 additions & 4 deletions pkg/sdk/managed_accounts_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ type ShowManagedAccountOptions struct {
}

type managedAccountDBRow struct {
Name string `db:"name"`
Name sql.NullString `db:"name"`
AccountName sql.NullString `db:"account_name"`
Cloud string `db:"cloud"`
Region string `db:"region"`
Locator string `db:"locator"`
Locator sql.NullString `db:"locator"`
AccountLocator sql.NullString `db:"account_locator"`
CreatedOn string `db:"created_on"`
Url string `db:"url"`
Url sql.NullString `db:"url"`
AccountUrl sql.NullString `db:"account_url"`
AccountLocatorUrl string `db:"account_locator_url"`
IsReader bool `db:"is_reader"`
Comment sql.NullString `db:"comment"`
Expand All @@ -62,5 +65,13 @@ type ManagedAccount struct {
URL string
AccountLocatorURL string
IsReader bool
Comment string
Comment *string
}

func (v *ManagedAccount) ID() AccountObjectIdentifier {
return NewAccountObjectIdentifier(v.Name)
}

func (v *ManagedAccount) ObjectType() ObjectType {
return ObjectTypeManagedAccount
}
25 changes: 21 additions & 4 deletions pkg/sdk/managed_accounts_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,34 @@ func (r *ShowManagedAccountRequest) toOpts() *ShowManagedAccountOptions {

func (r managedAccountDBRow) convert() *ManagedAccount {
managedAccount := &ManagedAccount{
Name: r.Name,
Cloud: r.Cloud,
Region: r.Region,
Locator: r.Locator,
CreatedOn: r.CreatedOn,
URL: r.Url,
AccountLocatorURL: r.AccountLocatorUrl,
IsReader: r.IsReader,
}

if r.AccountName.Valid {
managedAccount.Name = r.AccountName.String
} else if r.Name.Valid {
managedAccount.Name = r.Name.String
}

if r.AccountLocator.Valid {
managedAccount.Locator = r.AccountLocator.String
} else if r.Locator.Valid {
managedAccount.Locator = r.Locator.String
}

if r.AccountUrl.Valid {
managedAccount.URL = r.AccountUrl.String
} else if r.Url.Valid {
managedAccount.URL = r.Url.String
}

if r.Comment.Valid {
managedAccount.Comment = r.Comment.String
managedAccount.Comment = &r.Comment.String
}

return managedAccount
}
5 changes: 2 additions & 3 deletions pkg/sdk/testint/accounts_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/stretchr/testify/require"
)

// TODO(SNOW-1920887): Some of the account features cannot be currently tested as they require two Snowflake organizations
// TODO(SNOW-1342761): Adjust the tests, so they can be run in their own pipeline
// For now, those tests should be run manually. The account/admin user running those tests is required to:
// - Be privileged with ORGADMIN and ACCOUNTADMIN roles.
Expand Down Expand Up @@ -333,8 +334,6 @@ func TestInt_Account(t *testing.T) {
require.Empty(t, acc.OldAccountURL)
})

// TODO(SNOW-1844776): This cannot be tested as it requires capabilities of moving accounts between organizations.

t.Run("drop: without options", func(t *testing.T) {
err := client.Accounts.Drop(ctx, sdk.NewAccountObjectIdentifier("non-existing-account"), 3, &sdk.DropAccountOptions{})
require.Error(t, err)
Expand Down Expand Up @@ -423,7 +422,7 @@ func TestInt_Account(t *testing.T) {
}

func TestInt_Account_SelfAlter(t *testing.T) {
t.Skip("TODO(SNOW-1844776): Adjust the test so that self alters will be done on newly created account - not the main test one")
t.Skip("TODO(SNOW-1920881): Adjust the test so that self alters will be done on newly created account - not the main test one")
testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate)

// This client should be operating on a different account than the "main" one (because it will be altered here).
Expand Down
21 changes: 21 additions & 0 deletions pkg/sdk/testint/managed_accounts_gen_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,25 @@ func TestInt_ManagedAccounts(t *testing.T) {
assert.Contains(t, returnedManagedAccounts, *managedAccount1)
assert.NotContains(t, returnedManagedAccounts, *managedAccount2)
})

// proves https://docs.snowflake.com/en/release-notes/bcr-bundles/2024_08/bcr-1738 is supported (column renames)
t.Run("show managed account: before and after BCR 2024_08", func(t *testing.T) {
managedAccount := createManagedAccount(t)

returnedManagedAccounts, err := client.ManagedAccounts.ShowByID(ctx, managedAccount.ID())
require.NoError(t, err)

assert.Equal(t, managedAccount.Name, returnedManagedAccounts.Name)
assert.Equal(t, managedAccount.Locator, returnedManagedAccounts.Locator)
assert.Equal(t, managedAccount.URL, returnedManagedAccounts.URL)

testClientHelper().BcrBundles.EnableBcrBundle(t, "2024_08")

returnedManagedAccounts, err = client.ManagedAccounts.ShowByID(ctx, managedAccount.ID())
require.NoError(t, err)

assert.Equal(t, managedAccount.Name, returnedManagedAccounts.Name)
assert.Equal(t, managedAccount.Locator, returnedManagedAccounts.Locator)
assert.Equal(t, managedAccount.URL, returnedManagedAccounts.URL)
})
}
Loading