feat: Support S3 bucket ABAC (Supersedes #370) - #393
Conversation
|
@bryantbiggs could you let us know if there are any show stoppers from getting this PR out ? Happy to help if there is some more work to do :) |
|
This PR has been automatically marked as stale because it has been open 30 days |
|
Not stale :/ |
cf6c057 to
b9fb7cc
Compare
|
Thanks for keeping this alive, @RafPe! |
|
Hi @milosveljkovic and @magreenbaum just giving this a quick bump! I've just rebased this branch against the latest this is a lightweight, non-breaking addition that fully resolves #370 and #372. All CI checks are completely green and it's ready for a final review whenever you have a cycle! |
e5a7e72 to
7396340
Compare
|
Just rebased this against the fresh "v5.15.3" master to keep everything completely up to date and conflict free. All CI checks are green. |
| } | ||
|
|
||
| resource "aws_s3_bucket_abac" "this" { | ||
| count = local.create_bucket && length(keys(var.abac_status)) > 0 && !var.is_directory_bucket ? 1 : 0 |
There was a problem hiding this comment.
did you try length(var.abac_status) instead of 'length(keys(var.abac_status))' , should work I think
| default = false | ||
| } | ||
|
|
||
| variable "abac_status" { |
There was a problem hiding this comment.
Should we be more restrictive about this one? With this setup, it can be whatevery (any). Can we set it to enable 'string' 'Enabled' 'Disabled' and as default null?
something like this:
variable "abac_status" {
description = "ABAC (Attribute Based Access Control) status for a general-purpose S3 bucket."
type = object({
status = optional(string, "Enabled")
})
default = null
validation {
condition = (
var.abac_status == null ||
contains(["Enabled", "Disabled"], var.abac_status.status)
)
error_message = "The abac_status.status must be 'Enabled' or 'Disabled'."
}
}This is just a suggestion, the current setup is working as expected
33205d8 to
d365563
Compare
d365563 to
110c01c
Compare
|
Thanks for the review, @milosveljkovic I completely agree that strict typing is better here. I've applied both of your suggestions:
I also rebased against the latest |
|
@antonbabenko please take a look here :) |
Description
This PR implements Attribute-Based Access Control (ABAC) for general-purpose S3 buckets via the
aws_s3_bucket_abacresource.This directly supersedes and closes #370, which was unfortunately locked from maintainer edits. I have brought the original implementation into alignment with the module's standards by:
.idinstead of.bucketfor internal references.regionarguments from the sub-resource.terraform-docspre-commit hook rather than manually modifying the READMEs.Motivation and Context
Closes #372. Closes #370. Enables automatic permission management based on tags attached to buckets.
Breaking Changes
None.
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull request