From bdc3959ac3761327c15f0c4fe1b15d7f9973f446 Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Mon, 29 Apr 2024 16:06:35 +0200 Subject: [PATCH 01/11] chore: Minimise permissions when not needed. --- .../cloudbackupschedule/resource_cloud_backup_schedule_test.go | 2 +- .../resource_cloud_provider_access_authorization_test.go | 2 +- .../encryptionatrest/resource_encryption_at_rest_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/cloudbackupschedule/resource_cloud_backup_schedule_test.go b/internal/service/cloudbackupschedule/resource_cloud_backup_schedule_test.go index 6a8220f730..b6dcc05229 100644 --- a/internal/service/cloudbackupschedule/resource_cloud_backup_schedule_test.go +++ b/internal/service/cloudbackupschedule/resource_cloud_backup_schedule_test.go @@ -790,7 +790,7 @@ func configExportPolicies(info *acc.ClusterInfo, policyName, roleName, awsAccess "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/cloudprovideraccess/resource_cloud_provider_access_authorization_test.go b/internal/service/cloudprovideraccess/resource_cloud_provider_access_authorization_test.go index bf1791afba..c1be438cd5 100644 --- a/internal/service/cloudprovideraccess/resource_cloud_provider_access_authorization_test.go +++ b/internal/service/cloudprovideraccess/resource_cloud_provider_access_authorization_test.go @@ -75,7 +75,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/encryptionatrest/resource_encryption_at_rest_test.go b/internal/service/encryptionatrest/resource_encryption_at_rest_test.go index 639253046d..4392b8d8cc 100644 --- a/internal/service/encryptionatrest/resource_encryption_at_rest_test.go +++ b/internal/service/encryptionatrest/resource_encryption_at_rest_test.go @@ -46,7 +46,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } From 9738976824077bd557b7709076c36efe2d8cdc0e Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Mon, 29 Apr 2024 17:52:41 +0200 Subject: [PATCH 02/11] adds remaining changes. --- .../data_source_federated_database_instance_test.go | 2 +- .../data_source_federated_database_instances_test.go | 2 +- .../resource_federated_database_instance_test.go | 2 +- .../federatedquerylimit/resource_federated_query_limit_test.go | 2 +- internal/service/pushbasedlogexport/resource_test.go | 2 +- scripts/generate-credentials-with-sts-assume-role.sh | 1 - 6 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go index 57b6e1b42d..718d0a2441 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go @@ -117,7 +117,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go index 049988fd6c..719ef1d4c6 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go @@ -51,7 +51,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index 7abb408ed2..5213fb726b 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -281,7 +281,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/federatedquerylimit/resource_federated_query_limit_test.go b/internal/service/federatedquerylimit/resource_federated_query_limit_test.go index a4b667d18a..b5483f48d3 100644 --- a/internal/service/federatedquerylimit/resource_federated_query_limit_test.go +++ b/internal/service/federatedquerylimit/resource_federated_query_limit_test.go @@ -81,7 +81,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/internal/service/pushbasedlogexport/resource_test.go b/internal/service/pushbasedlogexport/resource_test.go index 2d74b95885..247926c1cf 100644 --- a/internal/service/pushbasedlogexport/resource_test.go +++ b/internal/service/pushbasedlogexport/resource_test.go @@ -196,7 +196,7 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Allow", + "Effect": "Deny", "Action": "*", "Resource": "*" } diff --git a/scripts/generate-credentials-with-sts-assume-role.sh b/scripts/generate-credentials-with-sts-assume-role.sh index ace65dc596..38de0bce99 100755 --- a/scripts/generate-credentials-with-sts-assume-role.sh +++ b/scripts/generate-credentials-with-sts-assume-role.sh @@ -4,7 +4,6 @@ set -Eeou pipefail # This script uses aws sts assume-role to generate temporary credentials # and outputs them in $GITHUB_OUTPUT so those can be used in other workflow jobs. -# role-arn = arn:aws:iam::358363220050:role/terraform-provider-mongodbatlas-acceptancetests # Define a function to convert a string to lowercase function to_lowercase() { From 194db90ba822eec531b8f4ea75c682dcf48ca38b Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 07:44:47 +0200 Subject: [PATCH 03/11] fixes TestAccFederatedDatabaseInstance_s3bucket. --- .../resource_federated_database_instance_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index 5213fb726b..f0b67ff867 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -77,7 +77,6 @@ func TestAccFederatedDatabaseInstance_s3bucket(t *testing.T) { policyName = acc.RandomName() roleName = acc.RandomIAMRole() testS3Bucket = os.Getenv("AWS_S3_BUCKET") - region = "VIRGINIA_USA" ) resource.ParallelTest(t, resource.TestCase{ @@ -87,7 +86,7 @@ func TestAccFederatedDatabaseInstance_s3bucket(t *testing.T) { { ExternalProviders: acc.ExternalProvidersOnlyAWS(), ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - Config: configWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket, region), + Config: configWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet(resourceName, "project_id"), resource.TestCheckResourceAttr(resourceName, "name", name), @@ -269,7 +268,7 @@ func importStateIDFunc(resourceName string) resource.ImportStateIdFunc { } } -func configWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket, dataLakeRegion string) string { +func configWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket string) string { stepConfig := configFirstStepS3Bucket(name, testS3Bucket) return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { @@ -281,9 +280,9 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Deny", - "Action": "*", - "Resource": "*" + "Effect": "Allow", + "Action": "s3:*", + "Resource": "arn:aws:s3:::%[6]q" } ] } @@ -334,8 +333,8 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } } -%s - `, policyName, roleName, projectName, orgID, stepConfig) +%[5]s + `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) } func configFirstStepS3Bucket(name, testS3Bucket string) string { From e5f0f69f037f308fb5f70644e6add47f126109aa Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 08:16:46 +0200 Subject: [PATCH 04/11] fixes policy definition. --- .../resource_federated_database_instance_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index f0b67ff867..dd563ffe0e 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -270,6 +270,7 @@ func importStateIDFunc(resourceName string) resource.ImportStateIdFunc { func configWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket string) string { stepConfig := configFirstStepS3Bucket(name, testS3Bucket) + bucketResourceName := "arn:aws:s3:::" + testS3Bucket return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { name = %[1]q @@ -282,7 +283,7 @@ resource "aws_iam_role_policy" "test_policy" { { "Effect": "Allow", "Action": "s3:*", - "Resource": "arn:aws:s3:::%[6]q" + "Resource": %[6]q } ] } @@ -334,7 +335,7 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } %[5]s - `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) + `, policyName, roleName, projectName, orgID, stepConfig, bucketResourceName) } func configFirstStepS3Bucket(name, testS3Bucket string) string { From 3f6216d04212193c62e73a87944a8452d648eada Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 09:17:15 +0200 Subject: [PATCH 05/11] fixes permission error. --- .../resource_federated_database_instance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index dd563ffe0e..bf2d3be63b 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -282,7 +282,7 @@ resource "aws_iam_role_policy" "test_policy" { "Statement": [ { "Effect": "Allow", - "Action": "s3:*", + "Action": "*", "Resource": %[6]q } ] From 7bd98a030aab02b07f7abfdf27b08edfb84fb7c7 Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 12:02:40 +0200 Subject: [PATCH 06/11] tries new policy. --- ...source_federated_database_instance_test.go | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index bf2d3be63b..c1b99d9b72 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -280,11 +280,32 @@ resource "aws_iam_role_policy" "test_policy" { { "Version": "2012-10-17", "Statement": [ - { - "Effect": "Allow", - "Action": "*", - "Resource": %[6]q - } + { + "Effect": "Allow", + "Action": [ + "s3:ListAccessPointsForObjectLambda", + "s3:GetAccessPoint", + "s3:PutAccountPublicAccessBlock", + "s3:ListAccessPoints", + "s3:CreateStorageLensGroup", + "s3:ListJobs", + "s3:PutStorageLensConfiguration", + "s3:ListMultiRegionAccessPoints", + "s3:ListStorageLensGroups", + "s3:ListStorageLensConfigurations", + "s3:GetAccountPublicAccessBlock", + "s3:ListAllMyBuckets", + "s3:ListAccessGrantsInstances", + "s3:PutAccessPointPublicAccessBlock", + "s3:CreateJob" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": %[6]q + } ] } EOF From 8d1c1e52766754e317de72caf8b7f2b7e37fa235 Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 14:28:39 +0200 Subject: [PATCH 07/11] adds more permissions. --- .../resource_federated_database_instance_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index c1b99d9b72..4b78566c40 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -297,7 +297,10 @@ resource "aws_iam_role_policy" "test_policy" { "s3:ListAllMyBuckets", "s3:ListAccessGrantsInstances", "s3:PutAccessPointPublicAccessBlock", - "s3:CreateJob" + "s3:CreateJob", + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" ], "Resource": "*" }, From 642caf90ef08d5086172e5a3f1277399fc54a13b Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Tue, 30 Apr 2024 16:56:09 +0200 Subject: [PATCH 08/11] fixes permissions on other tests. --- ...source_federated_database_instance_test.go | 28 ++++++++++------- ...ource_federated_database_instances_test.go | 30 ++++++++++++------- ...source_federated_database_instance_test.go | 15 ---------- 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go index 718d0a2441..1472793c68 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go @@ -54,7 +54,6 @@ func TestAccFederatedDatabaseInstanceDS_s3Bucket(t *testing.T) { policyName = acc.RandomName() roleName = acc.RandomIAMRole() testS3Bucket = os.Getenv("AWS_S3_BUCKET") - region = "VIRGINIA_USA" federatedInstance = admin.DataLakeTenant{} ) @@ -65,7 +64,7 @@ func TestAccFederatedDatabaseInstanceDS_s3Bucket(t *testing.T) { { ExternalProviders: acc.ExternalProvidersOnlyAWS(), ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - Config: configDSWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket, region), + Config: configDSWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket), Check: resource.ComposeTestCheckFunc( checkExists(resourceName, &federatedInstance), checkAttributes(&federatedInstance, name), @@ -105,7 +104,7 @@ func checkAttributes(dataFederatedInstance *admin.DataLakeTenant, name string) r } } -func configDSWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket, dataLakeRegion string) string { +func configDSWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket string) string { stepConfig := configDSFirstStepS3Bucket(name, testS3Bucket) return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { @@ -116,11 +115,20 @@ resource "aws_iam_role_policy" "test_policy" { { "Version": "2012-10-17", "Statement": [ - { - "Effect": "Deny", - "Action": "*", - "Resource": "*" - } + { + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": %[6]q + } ] } EOF @@ -170,8 +178,8 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } } -%s - `, policyName, roleName, projectName, orgID, stepConfig) +%[5]s + `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) } func configDSFirstStepS3Bucket(name, testS3Bucket string) string { return fmt.Sprintf(` diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go index 719ef1d4c6..e0ab2a58f8 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go @@ -19,7 +19,6 @@ func TestAccFederatedDatabaseInstanceDSPlural_basic(t *testing.T) { policyName = acc.RandomName() roleName = acc.RandomIAMRole() testS3Bucket = os.Getenv("AWS_S3_BUCKET") - region = "VIRGINIA_USA" ) resource.ParallelTest(t, resource.TestCase{ @@ -29,7 +28,7 @@ func TestAccFederatedDatabaseInstanceDSPlural_basic(t *testing.T) { { ExternalProviders: acc.ExternalProvidersOnlyAWS(), ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - Config: configDSPlural(policyName, roleName, projectName, orgID, firstName, secondName, testS3Bucket, region), + Config: configDSPlural(policyName, roleName, projectName, orgID, firstName, secondName, testS3Bucket), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet(resourceName, "project_id"), resource.TestCheckResourceAttrSet(resourceName, "results.#"), @@ -39,7 +38,7 @@ func TestAccFederatedDatabaseInstanceDSPlural_basic(t *testing.T) { }) } -func configDSPlural(policyName, roleName, projectName, orgID, firstName, secondName, testS3Bucket, dataLakeRegion string) string { +func configDSPlural(policyName, roleName, projectName, orgID, firstName, secondName, testS3Bucket string) string { stepConfig := configDSPluralFirstStep(firstName, secondName, testS3Bucket) return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { @@ -49,12 +48,21 @@ resource "aws_iam_role_policy" "test_policy" { policy = <<-EOF { "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Deny", - "Action": "*", - "Resource": "*" - } + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": %[6]q + } ] } EOF @@ -104,8 +112,8 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } } -%s - `, policyName, roleName, projectName, orgID, stepConfig) +%[5]s + `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) } func configDSPluralFirstStep(firstName, secondName, testS3Bucket string) string { return fmt.Sprintf(` diff --git a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go index 4b78566c40..91f642aec7 100644 --- a/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/resource_federated_database_instance_test.go @@ -283,21 +283,6 @@ resource "aws_iam_role_policy" "test_policy" { { "Effect": "Allow", "Action": [ - "s3:ListAccessPointsForObjectLambda", - "s3:GetAccessPoint", - "s3:PutAccountPublicAccessBlock", - "s3:ListAccessPoints", - "s3:CreateStorageLensGroup", - "s3:ListJobs", - "s3:PutStorageLensConfiguration", - "s3:ListMultiRegionAccessPoints", - "s3:ListStorageLensGroups", - "s3:ListStorageLensConfigurations", - "s3:GetAccountPublicAccessBlock", - "s3:ListAllMyBuckets", - "s3:ListAccessGrantsInstances", - "s3:PutAccessPointPublicAccessBlock", - "s3:CreateJob", "s3:GetObject", "s3:ListBucket", "s3:GetObjectVersion" From 24df72d35002074731576566cebe56528933677a Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Thu, 2 May 2024 07:44:01 +0200 Subject: [PATCH 09/11] fixes format. --- ...source_federated_database_instance_test.go | 3 +- ...ource_federated_database_instances_test.go | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go index 1472793c68..a5372ac97f 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instance_test.go @@ -106,6 +106,7 @@ func checkAttributes(dataFederatedInstance *admin.DataLakeTenant, name string) r func configDSWithS3Bucket(policyName, roleName, projectName, orgID, name, testS3Bucket string) string { stepConfig := configDSFirstStepS3Bucket(name, testS3Bucket) + bucketResourceName := "arn:aws:s3:::" + testS3Bucket return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { name = %[1]q @@ -179,7 +180,7 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } %[5]s - `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) + `, policyName, roleName, projectName, orgID, stepConfig, bucketResourceName) } func configDSFirstStepS3Bucket(name, testS3Bucket string) string { return fmt.Sprintf(` diff --git a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go index e0ab2a58f8..5d2d7ff9c0 100644 --- a/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go +++ b/internal/service/federateddatabaseinstance/data_source_federated_database_instances_test.go @@ -40,6 +40,7 @@ func TestAccFederatedDatabaseInstanceDSPlural_basic(t *testing.T) { func configDSPlural(policyName, roleName, projectName, orgID, firstName, secondName, testS3Bucket string) string { stepConfig := configDSPluralFirstStep(firstName, secondName, testS3Bucket) + bucketResourceName := "arn:aws:s3:::" + testS3Bucket return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { name = %[1]q @@ -48,21 +49,21 @@ resource "aws_iam_role_policy" "test_policy" { policy = <<-EOF { "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:GetObject", - "s3:ListBucket", - "s3:GetObjectVersion" - ], - "Resource": "*" - }, - { - "Effect": "Allow", - "Action": "s3:*", - "Resource": %[6]q - } + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": %[6]q + } ] } EOF @@ -113,7 +114,7 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } %[5]s - `, policyName, roleName, projectName, orgID, stepConfig, testS3Bucket) + `, policyName, roleName, projectName, orgID, stepConfig, bucketResourceName) } func configDSPluralFirstStep(firstName, secondName, testS3Bucket string) string { return fmt.Sprintf(` From cbd6929ba8f918f09d7de3babae0c78c45f6c9ef Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Thu, 2 May 2024 11:57:59 +0200 Subject: [PATCH 10/11] fixes S3 push-based logging test. --- .../pushbasedlogexport/resource_test.go | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/internal/service/pushbasedlogexport/resource_test.go b/internal/service/pushbasedlogexport/resource_test.go index 247926c1cf..caf4882f7d 100644 --- a/internal/service/pushbasedlogexport/resource_test.go +++ b/internal/service/pushbasedlogexport/resource_test.go @@ -122,7 +122,7 @@ func configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, aw %[8]s `, projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, - awsIAMroleAuthAndS3Config(), pushBasedLogExportConfig(false, usePrefixPath, prefixPath)) + awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(false, usePrefixPath, prefixPath)) return test } @@ -141,7 +141,7 @@ func configBasicUpdated(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyN %[8]s `, projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, - awsIAMroleAuthAndS3Config(), pushBasedLogExportConfig(true, usePrefixPath, prefixPath)) // updating the S3 bucket to use for push-based log config + awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(true, usePrefixPath, prefixPath)) // updating the S3 bucket to use for push-based log config return test } @@ -184,8 +184,10 @@ func pushBasedLogExportDataSourceConfig() string { // awsIAMroleAuthAndS3Config returns config for required IAM roles and authorizes them (sets up cloud provider access) with a mongodbatlas_project // This method also creates two S3 buckets and sets up required access policy for them -func awsIAMroleAuthAndS3Config() string { - return ` +func awsIAMroleAuthAndS3Config(firstBucketName, secondBucketName string) string { + firstBucketResourceName := "arn:aws:s3:::" + firstBucketName + secondBucketResourceName := "arn:aws:s3:::" + secondBucketName + return fmt.Sprintf(` // Create IAM role & policy to authorize with Atlas resource "aws_iam_role_policy" "test_policy" { name = local.aws_iam_role_policy_name @@ -196,9 +198,21 @@ resource "aws_iam_role_policy" "test_policy" { "Version": "2012-10-17", "Statement": [ { - "Effect": "Deny", - "Action": "*", - "Resource": "*" + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": [ + %[1]q, + %[2]q + ] } ] } @@ -291,7 +305,7 @@ resource "aws_iam_role_policy" "s3_bucket_policy" { } EOF } - ` + `, firstBucketResourceName, secondBucketResourceName) } func checkDestroy(state *terraform.State) error { From b0e29751216a486b187fb3094e8f6cfeddf87770 Mon Sep 17 00:00:00 2001 From: Marco Suma Date: Thu, 2 May 2024 12:03:51 +0200 Subject: [PATCH 11/11] fixes other policy. --- .../resource_federated_query_limit_test.go | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/internal/service/federatedquerylimit/resource_federated_query_limit_test.go b/internal/service/federatedquerylimit/resource_federated_query_limit_test.go index b5483f48d3..d973afd0c6 100644 --- a/internal/service/federatedquerylimit/resource_federated_query_limit_test.go +++ b/internal/service/federatedquerylimit/resource_federated_query_limit_test.go @@ -71,6 +71,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { func configBasic(policyName, roleName, projectName, orgID, name, testS3Bucket string) string { stepConfig := configFirstStep(name, testS3Bucket) + bucketResourceName := "arn:aws:s3:::" + testS3Bucket return fmt.Sprintf(` resource "aws_iam_role_policy" "test_policy" { name = %[1]q @@ -80,11 +81,20 @@ resource "aws_iam_role_policy" "test_policy" { { "Version": "2012-10-17", "Statement": [ - { - "Effect": "Deny", - "Action": "*", - "Resource": "*" - } + { + "Effect": "Allow", + "Action": [ + "s3:GetObject", + "s3:ListBucket", + "s3:GetObjectVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": "s3:*", + "Resource": %[6]q + } ] } EOF @@ -134,8 +144,8 @@ resource "mongodbatlas_cloud_provider_access_authorization" "auth_role" { } } -%s - `, policyName, roleName, projectName, orgID, stepConfig) +%[5]s + `, policyName, roleName, projectName, orgID, stepConfig, bucketResourceName) } func configFirstStep(name, testS3Bucket string) string { @@ -214,7 +224,7 @@ data "mongodbatlas_federated_query_limits" "test" { project_id = mongodbatlas_project.test_project.id tenant_name = mongodbatlas_federated_database_instance.db_instance.name } - `, name, testS3Bucket) + `, name, testS3Bucket) } func importStateIDFunc(resourceName string) resource.ImportStateIdFunc {