chore: Fix env variables for tests (#2603)

- Introduce acceptance test context - Expose default config in integration tests - Remove not needed test envs - Expose envs to test jobs (because of [this](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsenv) and [that](https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#using-secrets-in-a-workflow)) - Skip the tests requiring business critical account
Snowflake-Labs · Mar 7, 2024 · 8bc2437 · 8bc2437
1 parent b2ce76e
commit 8bc2437
Show file tree

Hide file tree

Showing 12 changed files with 176 additions and 41 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -43,6 +43,16 @@ jobs:
 
       - run: make test
         if: steps.create_config.conclusion == 'success'
+        env:
+          SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT: ${{ secrets.SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT }}
+          TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL }}
+          TEST_SF_TF_AWS_EXTERNAL_KEY_ID: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_KEY_ID }}
+          TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN }}
+          TEST_SF_TF_AWS_EXTERNAL_SECRET_KEY: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_SECRET_KEY }}
+          TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL }}
+          TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN }}
+          TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID }}
+          TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL }}
 
       - name: Setup Terraform
         if: steps.create_config.conclusion == 'success'
@@ -53,6 +63,16 @@ jobs:
 
       - run: make test-acceptance
         if: steps.create_config.conclusion == 'success'
+        env:
+          SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT: ${{ secrets.SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT }}
+          TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL }}
+          TEST_SF_TF_AWS_EXTERNAL_KEY_ID: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_KEY_ID }}
+          TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN }}
+          TEST_SF_TF_AWS_EXTERNAL_SECRET_KEY: ${{ secrets.TEST_SF_TF_AWS_EXTERNAL_SECRET_KEY }}
+          TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL }}
+          TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN }}
+          TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID: ${{ secrets.TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID }}
+          TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL: ${{ secrets.TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL }}
 
       - name: sweepers cleanup
         if: ${{ always() }}

diff --git a/pkg/acceptance/testenvs/testenvs_test.go b/pkg/acceptance/testenvs/testenvs_test.go
@@ -18,14 +18,14 @@ func Test_GetOrSkipTest(t *testing.T) {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			env = testenvs.GetOrSkipTest(t, testenvs.User)
+			env = testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 		}()
 		wg.Wait()
 		return env
 	}
 
 	t.Run("skip test if missing", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "")
 
 		tut := &testing.T{}
 		env := runGetOrSkipInGoroutineAndWaitForCompletion(tut)
@@ -35,7 +35,7 @@ func Test_GetOrSkipTest(t *testing.T) {
 	})
 
 	t.Run("get env if exists", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "user")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "user")
 
 		tut := &testing.T{}
 		env := runGetOrSkipInGoroutineAndWaitForCompletion(tut)
@@ -60,19 +60,19 @@ func Test_SkipTestIfSet(t *testing.T) {
 	}
 
 	t.Run("skip test if env is set", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "1")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "1")
 
 		tut := &testing.T{}
-		runSkipTestIfSetInGoroutineAndWaitForCompletion(tut, testenvs.User)
+		runSkipTestIfSetInGoroutineAndWaitForCompletion(tut, testenvs.BusinessCriticalAccount)
 
 		require.True(t, tut.Skipped())
 	})
 
 	t.Run("do not skip if env not set", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "")
 
 		tut := &testing.T{}
-		runSkipTestIfSetInGoroutineAndWaitForCompletion(tut, testenvs.User)
+		runSkipTestIfSetInGoroutineAndWaitForCompletion(tut, testenvs.BusinessCriticalAccount)
 
 		require.False(t, tut.Skipped())
 	})
@@ -92,26 +92,26 @@ func Test_Assertions(t *testing.T) {
 	}
 
 	t.Run("test if env does not exist", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "")
 
 		tut1 := &testing.T{}
-		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvNotSet(tut1, string(testenvs.User)) })
+		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvNotSet(tut1, string(testenvs.BusinessCriticalAccount)) })
 
 		tut2 := &testing.T{}
-		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvSet(tut2, string(testenvs.User)) })
+		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvSet(tut2, string(testenvs.BusinessCriticalAccount)) })
 
 		require.False(t, tut1.Failed())
 		require.True(t, tut2.Failed())
 	})
 
 	t.Run("test if env exists", func(t *testing.T) {
-		t.Setenv(string(testenvs.User), "user")
+		t.Setenv(string(testenvs.BusinessCriticalAccount), "user")
 
 		tut1 := &testing.T{}
-		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvNotSet(tut1, string(testenvs.User)) })
+		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvNotSet(tut1, string(testenvs.BusinessCriticalAccount)) })
 
 		tut2 := &testing.T{}
-		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvSet(tut2, string(testenvs.User)) })
+		runAssertionInGoroutineAndWaitForCompletion(func() { testenvs.AssertEnvSet(tut2, string(testenvs.BusinessCriticalAccount)) })
 
 		require.True(t, tut1.Failed())
 		require.False(t, tut2.Failed())

diff --git a/pkg/acceptance/testenvs/testing_environment_variables.go b/pkg/acceptance/testenvs/testing_environment_variables.go
@@ -9,12 +9,6 @@ import (
 type env string
 
 const (
-	User     env = "TEST_SF_TF_USER"
-	Password env = "TEST_SF_TF_PASSWORD" // #nosec G101
-	Account  env = "TEST_SF_TF_ACCOUNT"
-	Role     env = "TEST_SF_TF_ROLE"
-	Host     env = "TEST_SF_TF_HOST"
-
 	BusinessCriticalAccount env = "SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT"
 
 	TestAccountCreate          env = "TEST_SF_TF_TEST_ACCOUNT_CREATE"
@@ -33,7 +27,8 @@ const (
 	SkipManagedAccountTest  env = "TEST_SF_TF_SKIP_MANAGED_ACCOUNT_TEST"
 	SkipSamlIntegrationTest env = "TEST_SF_TF_SKIP_SAML_INTEGRATION_TEST"
 
-	EnableSweep env = "TEST_SF_TF_ENABLE_SWEEP"
+	EnableSweep         env = "TEST_SF_TF_ENABLE_SWEEP"
+	ConfigureClientOnce env = "SF_TF_ACC_TEST_CONFIGURE_CLIENT_ONCE"
 )
 
 func GetOrSkipTest(t *testing.T, envName Env) string {

diff --git a/pkg/acceptance/testing.go b/pkg/acceptance/testing.go
@@ -8,13 +8,15 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testprofiles"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 	"github.com/hashicorp/terraform-plugin-mux/tf5to6server"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-testing/config"
+	"github.com/snowflakedb/gosnowflake"
 )
 
 const (
@@ -28,6 +30,7 @@ var (
 	TestAccProvider *schema.Provider
 	v5Server        tfprotov5.ProviderServer
 	v6Server        tfprotov6.ProviderServer
+	atc             acceptanceTestContext
 )
 
 func init() {
@@ -44,6 +47,26 @@ func init() {
 		log.Panicf("Cannot upgrade server from proto v5 to proto v6, failing, err: %v", err)
 	}
 	_ = testAccProtoV6ProviderFactoriesNew
+
+	defaultConfig, err := sdk.ProfileConfig(testprofiles.Default)
+	if err != nil {
+		log.Panicf("Cannot load default config, err: %v", err)
+	}
+	if defaultConfig == nil {
+		log.Panic("Config is required to run acceptance tests")
+	}
+	atc.config = defaultConfig
+
+	client, err := sdk.NewClient(defaultConfig)
+	if err != nil {
+		log.Panicf("Cannot instantiate new client, err: %v", err)
+	}
+	atc.client = client
+}
+
+type acceptanceTestContext struct {
+	config *gosnowflake.Config
+	client *sdk.Client
 }
 
 var TestAccProtoV6ProviderFactories = map[string]func() (tfprotov6.ProviderServer, error){
@@ -68,35 +91,31 @@ var once sync.Once
 func TestAccPreCheck(t *testing.T) {
 	// use singleton design pattern to ensure we only create these resources once
 	once.Do(func() {
-		client, err := sdk.NewDefaultClient()
-		if err != nil {
-			t.Fatal(err)
-		}
 		ctx := context.Background()
 
 		dbId := sdk.NewAccountObjectIdentifier(TestDatabaseName)
-		if err := client.Databases.Create(ctx, dbId, &sdk.CreateDatabaseOptions{
+		if err := atc.client.Databases.Create(ctx, dbId, &sdk.CreateDatabaseOptions{
 			IfNotExists: sdk.Bool(true),
 		}); err != nil {
 			t.Fatal(err)
 		}
 
 		schemaId := sdk.NewDatabaseObjectIdentifier(TestDatabaseName, TestSchemaName)
-		if err := client.Schemas.Create(ctx, schemaId, &sdk.CreateSchemaOptions{
+		if err := atc.client.Schemas.Create(ctx, schemaId, &sdk.CreateSchemaOptions{
 			IfNotExists: sdk.Bool(true),
 		}); err != nil {
 			t.Fatal(err)
 		}
 
 		warehouseId := sdk.NewAccountObjectIdentifier(TestWarehouseName)
-		if err := client.Warehouses.Create(ctx, warehouseId, &sdk.CreateWarehouseOptions{
+		if err := atc.client.Warehouses.Create(ctx, warehouseId, &sdk.CreateWarehouseOptions{
 			IfNotExists: sdk.Bool(true),
 		}); err != nil {
 			t.Fatal(err)
 		}
 
 		warehouseId2 := sdk.NewAccountObjectIdentifier(TestWarehouseName2)
-		if err := client.Warehouses.Create(ctx, warehouseId2, &sdk.CreateWarehouseOptions{
+		if err := atc.client.Warehouses.Create(ctx, warehouseId2, &sdk.CreateWarehouseOptions{
 			IfNotExists: sdk.Bool(true),
 		}); err != nil {
 			t.Fatal(err)
@@ -119,3 +138,13 @@ func ConfigurationDirectory(directory string) func(config.TestStepConfigRequest)
 		return filepath.Join("testdata", directory)
 	}
 }
+
+func Client(t *testing.T) *sdk.Client {
+	t.Helper()
+	return atc.client
+}
+
+func DefaultConfig(t *testing.T) *gosnowflake.Config {
+	t.Helper()
+	return atc.config
+}
diff --git a/pkg/datasources/failover_groups_acceptance_test.go b/pkg/datasources/failover_groups_acceptance_test.go
@@ -14,6 +14,9 @@ import (
 )
 
 func TestAcc_FailoverGroups(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 
 	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))

diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go
@@ -782,11 +782,20 @@ func ConfigureProvider(s *schema.ResourceData) (interface{}, error) {
 		}
 	}
 
-	configuredClient, configureClientError = sdk.NewClient(config)
+	cl, clErr := sdk.NewClient(config)
 
-	if configureClientError != nil {
-		return nil, configureClientError
+	// needed for tests verifying different provider setups
+	if os.Getenv("TF_ACC") != "" && os.Getenv("SF_TF_ACC_TEST_CONFIGURE_CLIENT_ONCE") == "true" {
+		configuredClient = cl
+		configureClientError = clErr
+	} else {
+		configuredClient = nil
+		configureClientError = nil
+	}
+
+	if clErr != nil {
+		return nil, clErr
 	}
 
-	return &provider.Context{Client: configuredClient}, nil
+	return &provider.Context{Client: cl}, nil
 }
diff --git a/pkg/provider/provider_acceptance_test.go b/pkg/provider/provider_acceptance_test.go
@@ -17,11 +17,13 @@ import (
 )
 
 func TestAcc_Provider_configHierarchy(t *testing.T) {
-	user := testenvs.GetOrSkipTest(t, testenvs.User)
-	pass := testenvs.GetOrSkipTest(t, testenvs.Password)
-	account := testenvs.GetOrSkipTest(t, testenvs.Account)
-	role := testenvs.GetOrSkipTest(t, testenvs.Role)
-	host := testenvs.GetOrSkipTest(t, testenvs.Host)
+	t.Setenv(string(testenvs.ConfigureClientOnce), "")
+
+	user := acc.DefaultConfig(t).User
+	pass := acc.DefaultConfig(t).Password
+	account := acc.DefaultConfig(t).Account
+	role := acc.DefaultConfig(t).Role
+	host := acc.DefaultConfig(t).Host
 
 	nonExistingUser := "non-existing-user"
 
@@ -119,6 +121,39 @@ func TestAcc_Provider_configHierarchy(t *testing.T) {
 	})
 }
 
+func TestAcc_Provider_configureClientOnceSwitching(t *testing.T) {
+	t.Setenv(string(testenvs.ConfigureClientOnce), "")
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck: func() {
+			acc.TestAccPreCheck(t)
+			testenvs.AssertEnvNotSet(t, snowflakeenvs.User)
+			testenvs.AssertEnvNotSet(t, snowflakeenvs.Password)
+		},
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		Steps: []resource.TestStep{
+			// client setup is incorrect
+			{
+				Config:      providerConfig(testprofiles.IncorrectUserAndPassword),
+				ExpectError: regexp.MustCompile("Incorrect username or password was specified"),
+			},
+			// in this step we simulate the situation when we want to use client configured once, but it was faulty last time
+			{
+				PreConfig: func() {
+					t.Setenv(string(testenvs.ConfigureClientOnce), "true")
+				},
+				Config: emptyProviderConfig(),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("data.snowflake_database.t", "name", acc.TestDatabaseName),
+				),
+			},
+		},
+	})
+}
+
 func emptyProviderConfig() string {
 	return `
 provider "snowflake" {

diff --git a/pkg/resources/failover_group_acceptance_test.go b/pkg/resources/failover_group_acceptance_test.go
@@ -14,6 +14,9 @@ import (
 )
 
 func TestAcc_FailoverGroupBasic(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
@@ -49,6 +52,9 @@ func TestAcc_FailoverGroupBasic(t *testing.T) {
 }
 
 func TestAcc_FailoverGroupRemoveObjectTypes(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
@@ -87,6 +93,9 @@ func TestAcc_FailoverGroupRemoveObjectTypes(t *testing.T) {
 }
 
 func TestAcc_FailoverGroupInterval(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
@@ -183,6 +192,9 @@ func TestAcc_FailoverGroupInterval(t *testing.T) {
 }
 
 func TestAcc_FailoverGroup_issue2517(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
@@ -211,6 +223,9 @@ func TestAcc_FailoverGroup_issue2517(t *testing.T) {
 }
 
 func TestAcc_FailoverGroup_issue2544(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	randomCharacters := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)

diff --git a/pkg/resources/failover_group_grant_acceptance_test.go b/pkg/resources/failover_group_grant_acceptance_test.go
@@ -14,6 +14,9 @@ import (
 )
 
 func TestAcc_FailoverGroupGrant(t *testing.T) {
+	// TODO [SNOW-1002023]: Unskip; Business Critical Snowflake Edition needed
+	_ = testenvs.GetOrSkipTest(t, testenvs.TestFailoverGroups)
+
 	accountName := testenvs.GetOrSkipTest(t, testenvs.BusinessCriticalAccount)
 	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))