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.

Sign up for GitHub

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

chore: Initialize the SDK client in fewer places #2710

Merged
merged 4 commits into from
Apr 16, 2024
Merged

chore: Initialize the SDK client in fewer places #2710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec18026
Reuse default sdk client throughout acceptance tests
sfc-gh-asawicki Apr 15, 2024
f96ff69
Add secondary client to acceptance tests setup
sfc-gh-asawicki Apr 15, 2024
14fc5ea
Inline unnecessary method
sfc-gh-asawicki Apr 15, 2024
3f93a9a
Fix secondary client
sfc-gh-asawicki Apr 15, 2024
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
20 changes: 18 additions & 2 deletions pkg/acceptance/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,22 @@ func init() {
log.Panicf("Cannot instantiate new client, err: %v", err)
}
atc.client = client

cfg, err := sdk.ProfileConfig(testprofiles.Secondary)
if err != nil {
log.Panicf("Config for the secondary client is needed to run acceptance tests, err: %v", err)
}
secondaryClient, err := sdk.NewClient(cfg)
if err != nil {
log.Panicf("Cannot instantiate new secondary client, err: %v", err)
}
atc.secondaryClient = secondaryClient
}

type acceptanceTestContext struct {
config *gosnowflake.Config
client *sdk.Client
config *gosnowflake.Config
client *sdk.Client
secondaryClient *sdk.Client
}

var TestAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){
Expand Down Expand Up @@ -144,6 +155,11 @@ func Client(t *testing.T) *sdk.Client {
return atc.client
}

func SecondaryClient(t *testing.T) *sdk.Client {
t.Helper()
return atc.secondaryClient
}

func DefaultConfig(t *testing.T) *gosnowflake.Config {
t.Helper()
return atc.config
Expand Down
20 changes: 3 additions & 17 deletions pkg/datasources/grants_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

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

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testprofiles"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-testing/config"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
Expand Down Expand Up @@ -373,18 +372,8 @@ func TestAcc_Grants_Of_Share(t *testing.T) {
getSecondaryAccountIdentifier := func(t *testing.T) *sdk.AccountIdentifier {
t.Helper()

client, err := sdk.NewDefaultClient()
if err != nil {
t.Fatal(err)
}
cfg, err := sdk.ProfileConfig(testprofiles.Secondary)
if err != nil {
t.Fatal(err)
}
secondaryClient, err := sdk.NewClient(cfg)
if err != nil {
t.Fatal(err)
}
client := acc.Client(t)
secondaryClient := acc.SecondaryClient(t)
ctx := context.Background()

replicationAccounts, err := client.ReplicationFunctions.ShowReplicationAccounts(ctx)
Expand Down Expand Up @@ -688,10 +677,7 @@ func checkAtLeastOneGrantPresentLimited() resource.TestCheckFunc {

func getCurrentUser(t *testing.T) string {
t.Helper()
client, err := sdk.NewDefaultClient()
if err != nil {
t.Fatal(err)
}
client := acc.Client(t)
user, err := client.ContextFunctions.CurrentUser(context.Background())
if err != nil {
t.Fatal(err)
Expand Down
36 changes: 10 additions & 26 deletions pkg/resources/database_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

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

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testprofiles"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-testing/config"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
Expand Down Expand Up @@ -198,8 +197,7 @@ func TestAcc_Database_DefaultDataRetentionTime(t *testing.T) {
return vars
}

client, err := sdk.NewDefaultClient()
require.NoError(t, err)
client := acc.Client(t)

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Expand Down Expand Up @@ -288,8 +286,7 @@ func TestAcc_Database_DefaultDataRetentionTime_SetOutsideOfTerraform(t *testing.
return vars
}

client, err := sdk.NewDefaultClient()
require.NoError(t, err)
client := acc.Client(t)

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Expand All @@ -310,7 +307,7 @@ func TestAcc_Database_DefaultDataRetentionTime_SetOutsideOfTerraform(t *testing.
},
{
PreConfig: func() {
err = client.Databases.Alter(context.Background(), id, &sdk.AlterDatabaseOptions{
err := client.Databases.Alter(context.Background(), id, &sdk.AlterDatabaseOptions{
Set: &sdk.DatabaseSet{
DataRetentionTimeInDays: sdk.Int(20),
},
Expand Down Expand Up @@ -382,23 +379,17 @@ resource "snowflake_database" "db" {
func dropDatabaseOutsideTerraform(t *testing.T, id string) {
t.Helper()

client, err := sdk.NewDefaultClient()
require.NoError(t, err)
client := acc.Client(t)
ctx := context.Background()

err = client.Databases.Drop(ctx, sdk.NewAccountObjectIdentifier(id), &sdk.DropDatabaseOptions{})
err := client.Databases.Drop(ctx, sdk.NewAccountObjectIdentifier(id), &sdk.DropDatabaseOptions{})
require.NoError(t, err)
}

func getSecondaryAccount(t *testing.T) string {
t.Helper()

secondaryConfig, err := sdk.ProfileConfig(testprofiles.Secondary)
require.NoError(t, err)

secondaryClient, err := sdk.NewClient(secondaryConfig)
require.NoError(t, err)

secondaryClient := acc.SecondaryClient(t)
ctx := context.Background()

account, err := secondaryClient.ContextFunctions.CurrentAccount(ctx)
Expand All @@ -410,11 +401,10 @@ func getSecondaryAccount(t *testing.T) string {
func testAccCheckDatabaseExistence(t *testing.T, id string, shouldExist bool) func(state *terraform.State) error {
t.Helper()
return func(state *terraform.State) error {
client, err := sdk.NewDefaultClient()
require.NoError(t, err)
client := acc.Client(t)
ctx := context.Background()

_, err = client.Databases.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id))
_, err := client.Databases.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id))
if shouldExist {
if err != nil {
return fmt.Errorf("error while retrieving database %s, err = %w", id, err)
Expand All @@ -431,10 +421,7 @@ func testAccCheckDatabaseExistence(t *testing.T, id string, shouldExist bool) fu
func testAccCheckIfDatabaseIsReplicated(t *testing.T, id string) func(state *terraform.State) error {
t.Helper()
return func(state *terraform.State) error {
client, err := sdk.NewDefaultClient()
if err != nil {
return err
}
client := acc.Client(t)

ctx := context.Background()
replicationDatabases, err := client.ReplicationFunctions.ShowReplicationDatabases(ctx, nil)
Expand Down Expand Up @@ -492,10 +479,7 @@ func checkAccountAndDatabaseDataRetentionTime(id sdk.AccountObjectIdentifier, ex

func createDatabaseOutsideTerraform(t *testing.T, name string) func() {
t.Helper()
client, err := sdk.NewDefaultClient()
if err != nil {
t.Fatal(err)
}
client := acc.Client(t)
ctx := context.Background()

if err := client.Databases.Create(ctx, sdk.NewAccountObjectIdentifier(name), new(sdk.CreateDatabaseOptions)); err != nil {
Expand Down
5 changes: 1 addition & 4 deletions pkg/resources/dynamic_table_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,7 @@ func testAccCheckDynamicTableDestroy(s *terraform.State) error {
// TODO [SNOW-926148]: currently this dynamic table is not cleaned in the test; it is removed when the whole database is removed - this currently happens in a sweeper
func createDynamicTableOutsideTerraform(t *testing.T, schemaName string, dynamicTableName string, query string) {
t.Helper()
client, err := sdk.NewDefaultClient()
if err != nil {
t.Fatal(err)
}
client := acc.Client(t)
ctx := context.Background()

dynamicTableId := sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, schemaName, dynamicTableName)
Expand Down
12 changes: 5 additions & 7 deletions pkg/resources/external_table_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestAcc_ExternalTable_basic(t *testing.T) {
},
{
PreConfig: func() {
publishExternalTablesTestData(sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, name), data)
publishExternalTablesTestData(t, sdk.NewSchemaObjectIdentifier(acc.TestDatabaseName, acc.TestSchemaName, name), data)
},
ConfigDirectory: config.TestStepDirectory(),
ConfigVariables: configVariables,
Expand Down Expand Up @@ -344,14 +344,12 @@ func externalTableContainsData(name string, contains func(rows []map[string]*any
}
}

func publishExternalTablesTestData(stageName sdk.SchemaObjectIdentifier, data []byte) {
client, err := sdk.NewDefaultClient()
if err != nil {
log.Fatal(err)
}
func publishExternalTablesTestData(t *testing.T, stageName sdk.SchemaObjectIdentifier, data []byte) {
t.Helper()
client := acc.Client(t)
ctx := context.Background()

_, err = client.ExecForTests(ctx, fmt.Sprintf(`copy into @%s/external_tables_test_data/test_data from (select parse_json('%s')) overwrite = true`, stageName.FullyQualifiedName(), string(data)))
_, err := client.ExecForTests(ctx, fmt.Sprintf(`copy into @%s/external_tables_test_data/test_data from (select parse_json('%s')) overwrite = true`, stageName.FullyQualifiedName(), string(data)))
if err != nil {
log.Fatal(err)
}
Expand Down
38 changes: 18 additions & 20 deletions pkg/resources/function_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package resources_test

import (
"context"
"errors"
"fmt"
"strings"
"testing"
Expand Down Expand Up @@ -44,7 +43,7 @@ func testAccFunction(t *testing.T, configDirectory string) {
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: testAccCheckFunctionDestroy,
CheckDestroy: testAccCheckFunctionDestroy(t),
Steps: []resource.TestStep{
{
ConfigDirectory: acc.ConfigurationDirectory(configDirectory),
Expand Down Expand Up @@ -131,7 +130,7 @@ func TestAcc_Function_complex(t *testing.T) {
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: testAccCheckFunctionDestroy,
CheckDestroy: testAccCheckFunctionDestroy(t),
Steps: []resource.TestStep{
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Function/complex"),
Expand Down Expand Up @@ -194,7 +193,7 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: testAccCheckFunctionDestroy,
CheckDestroy: testAccCheckFunctionDestroy(t),

// Using the string config because of the validation in teststep_validate.go:
// teststep.Config.HasConfigurationFiles() returns true both for ConfigFile and ConfigDirectory.
Expand Down Expand Up @@ -248,22 +247,21 @@ resource "snowflake_function" "f" {
`, database, schema, name)
}

func testAccCheckFunctionDestroy(s *terraform.State) error {
client, err := sdk.NewDefaultClient()
if err != nil {
return errors.New("client could not be instantiated")
}

for _, rs := range s.RootModule().Resources {
if rs.Type != "snowflake_function" {
continue
}
ctx := context.Background()
id := sdk.NewSchemaObjectIdentifier(rs.Primary.Attributes["database"], rs.Primary.Attributes["schema"], rs.Primary.Attributes["name"])
function, err := client.Functions.ShowByID(ctx, id)
if err == nil {
return fmt.Errorf("function %v still exists", function.Name)
func testAccCheckFunctionDestroy(t *testing.T) func(s *terraform.State) error {
t.Helper()
client := acc.Client(t)
return func(s *terraform.State) error {
for _, rs := range s.RootModule().Resources {
if rs.Type != "snowflake_function" {
continue
}
ctx := context.Background()
id := sdk.NewSchemaObjectIdentifier(rs.Primary.Attributes["database"], rs.Primary.Attributes["schema"], rs.Primary.Attributes["name"])
function, err := client.Functions.ShowByID(ctx, id)
if err == nil {
return fmt.Errorf("function %v still exists", function.Name)
}
}
return nil
}
return nil
}
25 changes: 8 additions & 17 deletions pkg/resources/grant_ownership_acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,14 +1078,12 @@ func TestAcc_GrantOwnership_OnAllTasks(t *testing.T) {

func createDatabaseWithRoleAsOwner(t *testing.T, roleName string, databaseName string) func() {
t.Helper()
client, err := sdk.NewDefaultClient()
assert.NoError(t, err)

client := acc.Client(t)
ctx := context.Background()
databaseId := sdk.NewAccountObjectIdentifier(databaseName)
assert.NoError(t, client.Databases.Create(ctx, databaseId, &sdk.CreateDatabaseOptions{}))

err = client.Grants.GrantOwnership(
err := client.Grants.GrantOwnership(
ctx,
sdk.OwnershipGrantOn{
Object: &sdk.Object{
Expand All @@ -1108,11 +1106,9 @@ func createDatabaseWithRoleAsOwner(t *testing.T, roleName string, databaseName s
func moveResourceOwnershipToAccountRole(t *testing.T, objectType sdk.ObjectType, objectName sdk.ObjectIdentifier, accountRoleName sdk.AccountObjectIdentifier) {
t.Helper()

client, err := sdk.NewDefaultClient()
assert.NoError(t, err)

client := acc.Client(t)
ctx := context.Background()
err = client.Grants.GrantOwnership(
err := client.Grants.GrantOwnership(
ctx,
sdk.OwnershipGrantOn{
Object: &sdk.Object{
Expand Down Expand Up @@ -1158,9 +1154,7 @@ func checkResourceOwnershipIsGranted(opts *sdk.ShowGrantOptions, grantOn sdk.Obj

func createAccountRole(t *testing.T, name string) func() {
t.Helper()
client, err := sdk.NewDefaultClient()
assert.NoError(t, err)

client := acc.Client(t)
ctx := context.Background()
roleId := sdk.NewAccountObjectIdentifier(name)
assert.NoError(t, client.Roles.Create(ctx, sdk.NewCreateRoleRequest(roleId)))
Expand All @@ -1172,8 +1166,7 @@ func createAccountRole(t *testing.T, name string) func() {

func createDatabase(t *testing.T, name string) func() {
t.Helper()
client, err := sdk.NewDefaultClient()
assert.NoError(t, err)
client := acc.Client(t)

ctx := context.Background()
roleId := sdk.NewAccountObjectIdentifier(name)
Expand All @@ -1186,8 +1179,7 @@ func createDatabase(t *testing.T, name string) func() {

func grantOwnershipToTheCurrentRole(t *testing.T, on sdk.OwnershipGrantOn) {
t.Helper()
client, err := sdk.NewDefaultClient()
assert.NoError(t, err)
client := acc.Client(t)

ctx := context.Background()
currentRole, err := client.ContextFunctions.CurrentRole(ctx)
Expand All @@ -1206,8 +1198,7 @@ func grantOwnershipToTheCurrentRole(t *testing.T, on sdk.OwnershipGrantOn) {

func getCurrentUser(t *testing.T) string {
t.Helper()
client, err := sdk.NewDefaultClient()
assert.NoError(t, err)
client := acc.Client(t)
currentUser, err := client.ContextFunctions.CurrentUser(context.Background())
assert.NoError(t, err)
return currentUser
Expand Down
Loading
Loading