From 8b51f4982463a11dd7814dab87d4bb1d2722da0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Thu, 5 Dec 2024 13:27:45 +0100 Subject: [PATCH] changes after review --- pkg/acceptance/helpers/account_client.go | 26 ------------ .../testenvs/testing_environment_variables.go | 7 ++-- pkg/sdk/testint/accounts_integration_test.go | 41 +++++++++++++++---- .../external_volumes_gen_integration_test.go | 2 +- pkg/sdk/testint/setup_test.go | 9 +--- .../testint/stages_gen_integration_test.go | 2 +- ...torage_integration_gen_integration_test.go | 2 +- 7 files changed, 41 insertions(+), 48 deletions(-) diff --git a/pkg/acceptance/helpers/account_client.go b/pkg/acceptance/helpers/account_client.go index c120a8dfbf..5bcef72bec 100644 --- a/pkg/acceptance/helpers/account_client.go +++ b/pkg/acceptance/helpers/account_client.go @@ -84,32 +84,6 @@ func (c *AccountClient) Alter(t *testing.T, opts *sdk.AlterAccountOptions) { require.NoError(t, err) } -func (c *AccountClient) UnsetPoliciesFunc(t *testing.T) func() { - t.Helper() - return func() { - _ = c.client().Alter(context.Background(), &sdk.AlterAccountOptions{ - Unset: &sdk.AccountUnset{ - PackagesPolicy: sdk.Bool(true), - }, - }) - _ = c.client().Alter(context.Background(), &sdk.AlterAccountOptions{ - Unset: &sdk.AccountUnset{ - PasswordPolicy: sdk.Bool(true), - }, - }) - _ = c.client().Alter(context.Background(), &sdk.AlterAccountOptions{ - Unset: &sdk.AccountUnset{ - SessionPolicy: sdk.Bool(true), - }, - }) - _ = c.client().Alter(context.Background(), &sdk.AlterAccountOptions{ - Unset: &sdk.AccountUnset{ - AuthenticationPolicy: sdk.Bool(true), - }, - }) - } -} - func (c *AccountClient) DropFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() { t.Helper() return func() { diff --git a/pkg/acceptance/testenvs/testing_environment_variables.go b/pkg/acceptance/testenvs/testing_environment_variables.go index 997d8cd8a8..07a69e6a34 100644 --- a/pkg/acceptance/testenvs/testing_environment_variables.go +++ b/pkg/acceptance/testenvs/testing_environment_variables.go @@ -13,9 +13,8 @@ type env string const ( BusinessCriticalAccount env = "SNOWFLAKE_BUSINESS_CRITICAL_ACCOUNT" - TestAccountCreate env = "TEST_SF_TF_TEST_ACCOUNT_CREATE" - TestFailoverGroups env = "TEST_SF_TF_TEST_FAILOVER_GROUPS" - ResourceMonitorNotifyUsers env = "TEST_SF_TF_RESOURCE_MONITOR_NOTIFY_USERS" + TestAccountCreate env = "TEST_SF_TF_TEST_ACCOUNT_CREATE" + TestFailoverGroups env = "TEST_SF_TF_TEST_FAILOVER_GROUPS" AwsExternalBucketUrl env = "TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL" AwsExternalKeyId env = "TEST_SF_TF_AWS_EXTERNAL_KEY_ID" @@ -24,7 +23,7 @@ const ( AzureExternalBucketUrl env = "TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL" AzureExternalTenantId env = "TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID" AzureExternalSasToken env = "TEST_SF_TF_AZURE_EXTERNAL_SAS_TOKEN" // #nosec G101 - GcsExternalBuckerUrl env = "TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL" + GcsExternalBucketUrl env = "TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL" EnableObjectRenamingTest env = "TEST_SF_TF_ENABLE_OBJECT_RENAMING" SkipManagedAccountTest env = "TEST_SF_TF_SKIP_MANAGED_ACCOUNT_TEST" diff --git a/pkg/sdk/testint/accounts_integration_test.go b/pkg/sdk/testint/accounts_integration_test.go index e1eb7e56c0..c4c864e445 100644 --- a/pkg/sdk/testint/accounts_integration_test.go +++ b/pkg/sdk/testint/accounts_integration_test.go @@ -3,6 +3,8 @@ package testint import ( "testing" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" @@ -18,9 +20,7 @@ import ( // - Shouldn't be any of the "main" accounts/admin users, because those tests alter the current account. func TestInt_Account(t *testing.T) { - if !testClientHelper().Context.IsRoleInSession(t, snowflakeroles.Orgadmin) { - t.Skip("ORGADMIN role is not in current session") - } + testenvs.GetOrSkipTest(t, testenvs.TestAccountCreate) client := testClient(t) ctx := testContext(t) @@ -405,9 +405,8 @@ func TestInt_Account(t *testing.T) { } func TestInt_Account_SelfAlter(t *testing.T) { - if !testClientHelper().Context.IsRoleInSession(t, snowflakeroles.Orgadmin) { - t.Skip("ORGADMIN role is not in current session") - } + t.Skip("TODO(SNOW-1844776): 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). // Cannot use a newly created account because ORGADMIN role is necessary, @@ -564,14 +563,19 @@ func TestInt_Account_SelfAlter(t *testing.T) { packagesPolicyId, packagesPolicyCleanup := testClientHelper().PackagesPolicy.Create(t) t.Cleanup(packagesPolicyCleanup) - t.Cleanup(testClientHelper().Account.UnsetPoliciesFunc(t)) - err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ Set: &sdk.AccountSet{ PackagesPolicy: packagesPolicyId, }, }) require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + Unset: &sdk.AccountUnset{ + PackagesPolicy: sdk.Bool(true), + }, + })) + }) err = client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ Set: &sdk.AccountSet{ @@ -579,6 +583,13 @@ func TestInt_Account_SelfAlter(t *testing.T) { }, }) require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + Unset: &sdk.AccountUnset{ + PasswordPolicy: sdk.Bool(true), + }, + })) + }) err = client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ Set: &sdk.AccountSet{ @@ -586,6 +597,13 @@ func TestInt_Account_SelfAlter(t *testing.T) { }, }) require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + Unset: &sdk.AccountUnset{ + SessionPolicy: sdk.Bool(true), + }, + })) + }) err = client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ Set: &sdk.AccountSet{ @@ -593,6 +611,13 @@ func TestInt_Account_SelfAlter(t *testing.T) { }, }) require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{ + Unset: &sdk.AccountUnset{ + AuthenticationPolicy: sdk.Bool(true), + }, + })) + }) assertPolicySet(t, authPolicy.ID()) assertPolicySet(t, passwordPolicy.ID()) diff --git a/pkg/sdk/testint/external_volumes_gen_integration_test.go b/pkg/sdk/testint/external_volumes_gen_integration_test.go index 83f663b188..4c40c1228f 100644 --- a/pkg/sdk/testint/external_volumes_gen_integration_test.go +++ b/pkg/sdk/testint/external_volumes_gen_integration_test.go @@ -20,7 +20,7 @@ func TestInt_ExternalVolumes(t *testing.T) { awsKmsKeyId := testenvs.GetOrSkipTest(t, testenvs.AwsExternalKeyId) awsExternalId := "123456789" - gcsBaseUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBuckerUrl) + gcsBaseUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBucketUrl) gcsKmsKeyId := "123456789" azureBaseUrl := testenvs.GetOrSkipTest(t, testenvs.AzureExternalBucketUrl) diff --git a/pkg/sdk/testint/setup_test.go b/pkg/sdk/testint/setup_test.go index 7f4838169a..ee949d3d67 100644 --- a/pkg/sdk/testint/setup_test.go +++ b/pkg/sdk/testint/setup_test.go @@ -130,13 +130,8 @@ func (itc *integrationTestContext) initialize() error { itc.client = c itc.ctx = context.Background() - currentRole, err := c.ContextFunctions.CurrentRole(context.Background()) - if err != nil { - return err - } - // TODO(SNOW-1842271): Adjust test setup to work properly with Accountadmin role for object tests and Orgadmin for account tests - if currentRole == snowflakeroles.Orgadmin { + if os.Getenv(string(testenvs.TestAccountCreate)) != "" { err = c.Sessions.UseRole(context.Background(), snowflakeroles.Accountadmin) if err != nil { return err @@ -215,7 +210,7 @@ func (itc *integrationTestContext) initialize() error { } // TODO(SNOW-1842271): Adjust test setup to work properly with Accountadmin role for object tests and Orgadmin for account tests - if currentRole != snowflakeroles.Orgadmin { + if os.Getenv(string(testenvs.TestAccountCreate)) == "" { err = helpers.EnsureScimProvisionerRolesExist(itc.client, itc.ctx) if err != nil { return err diff --git a/pkg/sdk/testint/stages_gen_integration_test.go b/pkg/sdk/testint/stages_gen_integration_test.go index 5bb3a94198..ff2a5a94e9 100644 --- a/pkg/sdk/testint/stages_gen_integration_test.go +++ b/pkg/sdk/testint/stages_gen_integration_test.go @@ -19,7 +19,7 @@ func TestInt_Stages(t *testing.T) { awsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.AwsExternalBucketUrl) awsKeyId := testenvs.GetOrSkipTest(t, testenvs.AwsExternalKeyId) awsSecretKey := testenvs.GetOrSkipTest(t, testenvs.AwsExternalSecretKey) - gcsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBuckerUrl) + gcsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBucketUrl) azureBucketUrl := testenvs.GetOrSkipTest(t, testenvs.AzureExternalBucketUrl) azureSasToken := testenvs.GetOrSkipTest(t, testenvs.AzureExternalSasToken) diff --git a/pkg/sdk/testint/storage_integration_gen_integration_test.go b/pkg/sdk/testint/storage_integration_gen_integration_test.go index 1ff3aab9d9..ddbd082d5b 100644 --- a/pkg/sdk/testint/storage_integration_gen_integration_test.go +++ b/pkg/sdk/testint/storage_integration_gen_integration_test.go @@ -18,7 +18,7 @@ func TestInt_StorageIntegrations(t *testing.T) { awsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.AwsExternalBucketUrl) awsRoleARN := testenvs.GetOrSkipTest(t, testenvs.AwsExternalRoleArn) - gcsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBuckerUrl) + gcsBucketUrl := testenvs.GetOrSkipTest(t, testenvs.GcsExternalBucketUrl) azureBucketUrl := testenvs.GetOrSkipTest(t, testenvs.AzureExternalBucketUrl) azureTenantId := testenvs.GetOrSkipTest(t, testenvs.AzureExternalTenantId)