Skip to content

feat: add additional bucket policy #8194

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

Open
jjfrench wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: add additional bucket policy #8194

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e7539b
feat: add additional bucket policy
jjfrench Apr 28, 2026
619724d
Document __BUCKET_ARN__
yuvipanda May 11, 2026
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
11 changes: 11 additions & 0 deletions terraform/aws/buckets.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ locals {
# ref: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document
data "aws_iam_policy_document" "bucket_access" {
for_each = { for bp in local.bucket_permissions : "${bp.hub_name}.${bp.bucket_name}" => bp }
source_policy_documents = compact([
try(
replace(
var.user_buckets[each.value.bucket_name].bucket_policy,
"__BUCKET_ARN__",
aws_s3_bucket.user_buckets[each.value.bucket_name].arn
),
null
)
])

statement {
effect = "Allow"
actions = ["s3:*"]
Expand Down
28 changes: 28 additions & 0 deletions terraform/aws/projects/nasa-veda.tfvars
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,34 @@ user_buckets = {
},
"scratch" : {
"delete_after" : 7,
"bucket_policy" : <<-EOT
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "MAAPHubAllowReadOnly",
"Effect": "Allow",
"Principal": {
"AWS": [
"arn:aws:iam::916098889494:role/maap-staging",
"arn:aws:iam::916098889494:role/maap-prod"
]
},
"Action": [
"s3:GetObject",
"s3:GetObjectTagging",
"s3:ListBucketVersions",
"s3:ListBucket",
"s3:GetBucketLocation"
],
"Resource": [
"__BUCKET_ARN__",

@yuvipanda yuvipanda Apr 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs the actual ARN right?

@jjfrench jjfrench Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should, yeah. I try to do this with a replace using the value from below

aws_s3_bucket.user_buckets[each.value.bucket_name].arn

@yuvipanda yuvipanda May 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Can we just hardcode it instead? This terraform is used for a lot of hubs (outside of the veda related projects too https://github.com/2i2c-org/infrastructure/tree/main/terraform/aws/projects), and I want to keep changes minimal if we can. (sorry, forgot to post this earlier)

@jjfrench jjfrench May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcode the ARN? Yeah, I can make that change, it would just be removing the try/replace. It would still modify the "user_buckets" object in variables.tf which is probably why it's showing so many changes?

@yuvipanda yuvipanda May 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had misread where this was being applied. I think this is fine. I just documented BUCKET_ARN here.

Thanks for the PR, and sorry about the delay!

"__BUCKET_ARN__/*"
]
}
]
}
EOT,
"tags" : { "2i2c:hub-name" : "prod" },
},
"scratch-binder" : {
Expand Down
7 changes: 5 additions & 2 deletions terraform/aws/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ variable "user_buckets" {
object({
delete_after : optional(number, null),
archival_storageclass_after : optional(number, null),
bucket_policy : optional(string, null),
tags : optional(map(string), {}),
})
)
Expand All @@ -41,7 +42,9 @@ variable "user_buckets" {
2. `archival_storageclass_after` - number of days after *creation* an
object in this bucket will be automatically transitioned to a cheaper,
slower storageclass for cost savings. Set to null to not transition.
3. `tags` - bucket specific tags to be merged into the general tags variable.
3. `bucket_policy` - optional AWS IAM policy document JSON to be merged into
this bucket's policy.
4. `tags` - bucket specific tags to be merged into the general tags variable.
EOT
}

Expand Down Expand Up @@ -313,4 +316,4 @@ variable "enable_nfs_backup" {
description = <<-EOT
Enable backup of NFS home directories on EBS using Data Lifecycle Manager (DLM).
EOT
}
}
Loading