Skip to content

Commit 5305746

Browse files
authored
fix: ACL issue (#167)
removing ACL block since by default it's private Adding aws_s3_bucket_ownership_controls to block ACL modifications Keeping this change https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/pull/164/files since AWS has not yet released the feature so public block access is not enabled by default. <!-- Thank you for your contribution! ## Testing your PR You can pinpoint the pr changes as terraform module source with following format ``` source = "github.com/sysdiglabs/terraform-aws-secure-for-cloud//examples/organizational?ref=<BRANCH_NAME>" ``` ## General recommendations Check contribution guidelines at https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/blob/master/CONTRIBUTE.md#contribution-checklist For a cleaner PR make sure you follow these recommendations: - Review modified files and delete small changes that were not intended and maybe slip the commit. - Use Pull Request Drafts for visibility on Work-In-Progress branches and use them on daily mob/pairing for team review - Unless an external revision is desired, in order to validate or gather some feedback, you are free to merge as long as **validation checks are green-lighted** ## Checklist - [ ] If `test/fixtures/*/main.tf` files are modified, update: - [ ] the snippets in the README.md file under root folder. - [ ] the snippets in the README.md file for the corresponding example. - [ ] If `examples` folder are modified, update: - [ ] README.md file with pertinent changes. - [ ] `test/fixtures/*/main.tf` in case the snippet needs modifications. - [ ] If any architectural change has been made, update the diagrams. -->
1 parent bfcfc60 commit 5305746

File tree

6 files changed

+18
-16
lines changed

6 files changed

+18
-16
lines changed

modules/infrastructure/cloudtrail/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ No modules.
2626
| [aws_kms_alias.kms](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_alias) | resource |
2727
| [aws_kms_key.cloudtrail_kms](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_key) | resource |
2828
| [aws_s3_bucket.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) | resource |
29-
| [aws_s3_bucket_acl.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_acl) | resource |
3029
| [aws_s3_bucket_lifecycle_configuration.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_lifecycle_configuration) | resource |
30+
| [aws_s3_bucket_ownership_controls.owner_enforced](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_ownership_controls) | resource |
3131
| [aws_s3_bucket_policy.cloudtrail_s3](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource |
3232
| [aws_s3_bucket_public_access_block.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
3333
| [aws_s3_bucket_server_side_encryption_configuration.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |

modules/infrastructure/cloudtrail/s3.tf

+8-6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "cloudtrail" {
2121
}
2222
}
2323

24+
resource "aws_s3_bucket_ownership_controls" "owner_enforced" {
25+
bucket = aws_s3_bucket.cloudtrail.id
26+
27+
rule {
28+
object_ownership = "BucketOwnerEnforced"
29+
}
30+
}
31+
2432
resource "aws_s3_bucket_lifecycle_configuration" "cloudtrail" {
2533
bucket = aws_s3_bucket.cloudtrail.id
2634

@@ -34,12 +42,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "cloudtrail" {
3442
}
3543

3644

37-
resource "aws_s3_bucket_acl" "cloudtrail" {
38-
bucket = aws_s3_bucket.cloudtrail.id
39-
acl = "private"
40-
}
41-
42-
4345
# --------------------------
4446
# iam, acl
4547
# -------------------------

test/fixtures/organizational/variables.tf

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ variable "sysdig_secure_for_cloud_member_account_id" {
1414
variable "name" {
1515
type = string
1616
description = "Name is the prefix used in the resources will be created"
17-
default = "sfctest-org-ecs"
17+
default = "sfc-test-org-ecs"
1818
}
1919

2020
variable "region" {

test/fixtures/single-account-apprunner/variables.tf

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ variable "sysdig_secure_api_token" {
77
variable "name" {
88
type = string
99
description = "Name to be assigned to all child resources. A suffix may be added internally when required. Use default value unless you need to install multiple instances"
10-
default = "sfctest-single-app"
10+
default = "sfc-test-single-app"
1111
}
1212

1313
variable "sysdig_secure_url" {

use-cases/multiple-accounts-k8s-threat.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ module "cloudtrail_s3_sns_sqs" {
120120

121121
4. Kubernetes Multi-Account **AWS Permissions** to be able to handle S3/SQS operations
122122

123-
Helm Cloud-Connector chart requires specific AWS credentials to be passed by parameter, a new user + access key will
123+
Helm Cloud-Connector chart requires specific AWS credentials to be passed by parameter, a new user + access key will
124124
be created within account, to be able to fetch the events in the S3 bucket (1) or several S3 buckets (2)
125125
<br/><br/>
126126
WIP.

use-cases/org-three-way-ecs.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ This accountID will be required in the `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_I
106106

107107
#### 3.2 (Optional) S3 and Sysdig Workload are in different accounts
108108

109-
If `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID` is different to the account where the S3 is located, we need to allow
109+
If `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID` is different to the account where the S3 is located, we need to allow
110110
cross-account access through a role.
111111

112112
Permission setup for SysdigSecureForCloud-S3AccessRole
@@ -129,13 +129,13 @@ Permission setup for SysdigSecureForCloud-S3AccessRole
129129

130130
#### 3.3 Cloudtrail S3 ingestion through Event-Forward
131131

132-
When Cloudtrail-SNS is not available, or the Cloudtrail-S3 events are in an account different to the management
132+
When Cloudtrail-SNS is not available, or the Cloudtrail-S3 events are in an account different to the management
133133
account, we will rely on a S3 Event Forwarder, to allow the workload to ingest events more easily.
134134

135-
Secure for Cloud requires an SQS queue from which it can ingest events, and this will provide
135+
Secure for Cloud requires an SQS queue from which it can ingest events, and this will provide
136136
`CLOUDTRAIL_S3_SNS_SQS_ARN` and `CLOUDTRAIL_S3_SNS_SQS_URL` for later installation.
137137

138-
We provide a module to create this
138+
We provide a module to create this
139139
[Cloudtrail S3 bucket event-forwarder into an SNS>SQS](https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/tree/master/modules/infrastructure/cloudtrail_s3-sns-sqs)
140140
but you can do it manually too.
141141

@@ -167,7 +167,7 @@ Inspect `terraform state list` to gather these two values, `CLOUDTRAIL_S3_SNS_SQ
167167

168168
#### 4. Launch Terraform Manifest
169169

170-
Let's create the Terraform manifest module parametrization, based on `examples/organizational`.
170+
Let's create the Terraform manifest module parametrization, based on `examples/organizational`.
171171
<br/>Get detailed explanation of each variable bellow.
172172

173173
```terraform
@@ -221,7 +221,7 @@ module "sysdig-sfc" {
221221
existing_cloudtrail_config={
222222
cloudtrail_s3_sns_sqs_arn = "<CLOUDTRAIL_S3_SNS_SQS_ARN>"
223223
cloudtrail_s3_sns_sqs_url = "<CLOUDTRAIL_S3_SNS_SQS_URL>"
224-
224+
225225
# optional, only if CLOUDTRAIL_S3 and SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID are in different accounts
226226
cloudtrail_s3_role_arn = "<CLOUDTRAIL_S3_ROLE_ARN>"
227227
}

0 commit comments

Comments
 (0)