Skip to content

Remove unused embargo buckets #249

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

Merged
mvandenburgh merged 5 commits into from
Oct 24, 2025
Merged

Remove unused embargo buckets #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
195b80a
Remove embargo buckets
mvandenburgh Oct 2, 2025
d19d69c
Remove `public` variable from `dandiset_bucket`
mvandenburgh Oct 2, 2025
2032db6
Remove `versioning` variable from `dandiset_bucket`
mvandenburgh Oct 2, 2025
de0a654
Remove `allow_heroku_put_object` variable from `dandiset_bucket`
mvandenburgh Oct 2, 2025
455c222
Remove `allow_cross_account_heroku_put_object` variable from `dandise…
mvandenburgh Oct 2, 2025
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
288 changes: 125 additions & 163 deletions terraform/modules/dandiset_bucket/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "dandiset_bucket"
resource "aws_s3_bucket_public_access_block" "dandiset_bucket" {
bucket = aws_s3_bucket.dandiset_bucket.id

# Allows public access to the bucket (if applicable)
# S3's default is to block all public access
block_public_policy = !var.public
restrict_public_buckets = !var.public
block_public_acls = !var.public
ignore_public_acls = !var.public
# Allows public access to the bucket. S3's default is to block all public access
block_public_policy = false
restrict_public_buckets = false
block_public_acls = false
ignore_public_acls = false
}

resource "aws_s3_bucket_cors_configuration" "dandiset_bucket" {
Expand Down Expand Up @@ -72,8 +71,6 @@ resource "aws_s3_bucket_logging" "dandiset_bucket" {
}

resource "aws_s3_bucket_versioning" "dandiset_bucket" {
count = var.versioning ? 1 : 0

bucket = aws_s3_bucket.dandiset_bucket.id

versioning_configuration {
Expand Down Expand Up @@ -115,17 +112,13 @@ data "aws_iam_policy_document" "dandiset_bucket_owner" {
]
}

dynamic "statement" {
for_each = (var.allow_cross_account_heroku_put_object || var.allow_heroku_put_object) ? [1] : []
content {

resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
statement {
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]

actions = ["s3:PutObject", "s3:PutObjectTagging"]
}
actions = ["s3:PutObject", "s3:PutObjectTagging"]
}

statement {
Expand All @@ -148,83 +141,71 @@ resource "aws_s3_bucket_policy" "dandiset_bucket_policy" {
data "aws_iam_policy_document" "dandiset_bucket_policy" {
version = "2008-10-17"

dynamic "statement" {
for_each = var.public ? [1] : []

content {
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
statement {
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]

actions = [
"s3:Get*",
"s3:List*",
]
actions = [
"s3:Get*",
"s3:List*",
]

principals {
identifiers = ["*"]
type = "*"
}
principals {
identifiers = ["*"]
type = "*"
}
}

# Disallow access to embargoed objects, unless using the heroku user arn, or
# an extra, authorized embargo reader account.
dynamic "statement" {
for_each = var.public ? [1] : []

content {
effect = "Deny"
principals {
identifiers = ["*"]
type = "*"
}
actions = ["s3:*"]
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
condition {
test = "StringEquals"
variable = "s3:ExistingObjectTag/embargoed"
values = ["true"]
}
condition {
test = "ArnNotEquals"
variable = "aws:PrincipalArn"
values = flatten([
var.heroku_user.arn,
[for user in var.embargo_readers : user.arn],
])
}
statement {
effect = "Deny"
principals {
identifiers = ["*"]
type = "*"
}
actions = ["s3:*"]
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
condition {
test = "StringEquals"
variable = "s3:ExistingObjectTag/embargoed"
values = ["true"]
}
condition {
test = "ArnNotEquals"
variable = "aws:PrincipalArn"
values = flatten([
var.heroku_user.arn,
[for user in var.embargo_readers : user.arn],
])
}
}

dynamic "statement" {
for_each = var.allow_cross_account_heroku_put_object ? [1] : []

content {
sid = "S3PolicyStmt-DO-NOT-MODIFY-1569973164923"
principals {
identifiers = ["s3.amazonaws.com"]
type = "Service"
}
actions = [
"s3:PutObject",
]
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.sponsored_account.account_id]
}
condition {
test = "ArnLike"
variable = "aws:SourceArn"
values = [aws_s3_bucket.dandiset_bucket.arn]
}
statement {
sid = "S3PolicyStmt-DO-NOT-MODIFY-1569973164923"
principals {
identifiers = ["s3.amazonaws.com"]
type = "Service"
}
actions = [
"s3:PutObject",
]
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
condition {
test = "StringEquals"
variable = "aws:SourceAccount"
values = [data.aws_caller_identity.sponsored_account.account_id]
}
condition {
test = "ArnLike"
variable = "aws:SourceArn"
values = [aws_s3_bucket.dandiset_bucket.arn]
}
}

Expand Down Expand Up @@ -260,43 +241,36 @@ data "aws_iam_policy_document" "dandiset_bucket_policy" {
}
}

dynamic "statement" {
for_each = var.allow_cross_account_heroku_put_object ? [1] : []
content {
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]
statement {
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}",
"${aws_s3_bucket.dandiset_bucket.arn}/*",
]

actions = ["s3:PutObjectTagging"]
actions = ["s3:PutObjectTagging"]

principals {
type = "AWS"
identifiers = [var.heroku_user.arn]
}
principals {
type = "AWS"
identifiers = [var.heroku_user.arn]
}
}

dynamic "statement" {
for_each = var.versioning ? [1] : []

content {
sid = "PreventDeletionOfObjectVersions"
statement {
sid = "PreventDeletionOfObjectVersions"

resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*"
]
resources = [
"${aws_s3_bucket.dandiset_bucket.arn}/*"
]

actions = [
"s3:DeleteObjectVersion",
]
actions = [
"s3:DeleteObjectVersion",
]

effect = "Deny"
effect = "Deny"

principals {
identifiers = ["*"]
type = "*"
}
principals {
identifiers = ["*"]
type = "*"
}
}
}
Expand All @@ -306,65 +280,53 @@ resource "aws_s3_bucket_lifecycle_configuration" "dandiset_bucket" {
# Must have bucket versioning enabled first
depends_on = [aws_s3_bucket_versioning.dandiset_bucket]

count = var.versioning ? 1 : 0

bucket = aws_s3_bucket.dandiset_bucket.id

# S3 lifecycle policy that permanently deletes objects with delete markers
# after 30 days. Note, this only applies to objects with the `blobs/` prefix.
# Based on https://docs.aws.amazon.com/AmazonS3/latest/userguide/lifecycle-configuration-examples.html#lifecycle-config-conceptual-ex7
dynamic "rule" {
# Only create this rule if versioning is enabled on the bucket
for_each = var.versioning ? [1] : []

content {
id = "ExpireOldDeleteMarkers"
filter {
# We only want to expire objects with the `blobs/` prefix, i.e. Asset Blobs.
# Other objects in this bucket are not subject to this lifecycle policy.
prefix = "blobs/"
}

# Expire objects with delete markers after 30 days
noncurrent_version_expiration {
noncurrent_days = 30
}

# Also delete any delete markers associated with the expired object
expiration {
expired_object_delete_marker = true
}

status = "Enabled"
rule {
id = "ExpireOldDeleteMarkers"
filter {
# We only want to expire objects with the `blobs/` prefix, i.e. Asset Blobs.
# Other objects in this bucket are not subject to this lifecycle policy.
prefix = "blobs/"
}

# Expire objects with delete markers after 30 days
noncurrent_version_expiration {
noncurrent_days = 30
}

# Also delete any delete markers associated with the expired object
expiration {
expired_object_delete_marker = true
}

status = "Enabled"
}

# S3 lifecycle policy that garbage collects old manifest file versions
dynamic "rule" {
# Only create this rule if versioning is enabled and we want to expire old manifest file versions
for_each = var.versioning ? [1] : []

content {
id = "ExpireOldManifestFileVersions"
filter {
# We only want to expire objects with the `dandisets/` prefix, i.e. manifest files.
# Other objects in this bucket are not subject to this lifecycle policy.
prefix = "dandisets/"
}

noncurrent_version_expiration {
# keep most recent noncurrent version indefinitely
newer_noncurrent_versions = 1
# delete all other noncurrent versions after 1 day
noncurrent_days = 1
}

# Also delete any delete markers associated with the expired object
expiration {
expired_object_delete_marker = true
}

status = "Enabled"
rule {
id = "ExpireOldManifestFileVersions"
filter {
# We only want to expire objects with the `dandisets/` prefix, i.e. manifest files.
# Other objects in this bucket are not subject to this lifecycle policy.
prefix = "dandisets/"
}

noncurrent_version_expiration {
# keep most recent noncurrent version indefinitely
newer_noncurrent_versions = 1
# delete all other noncurrent versions after 1 day
noncurrent_days = 1
}

# Also delete any delete markers associated with the expired object
expiration {
expired_object_delete_marker = true
}

status = "Enabled"
}
}
23 changes: 0 additions & 23 deletions terraform/modules/dandiset_bucket/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,8 @@
variable "public" {
type = bool
description = "Whether or not the contents of the bucket should be public."
default = false
}

variable "bucket_name" {
type = string
description = "The name of the bucket."
}

# TODO: remove after migration
variable "allow_heroku_put_object" {
type = bool
default = false
}

# TODO: refactor after migration
variable "allow_cross_account_heroku_put_object" {
type = bool
default = false
}

variable "versioning" {
type = bool
description = "Whether or not versioning should be enabled on the bucket."
}

variable "heroku_user" {
description = "The Heroku API IAM user who will have write access to the bucket."
}
Expand Down
Loading