s3 bucket - support switch for: PutPublicAccessBlock

[HerrmannHinz]

INFO

just trying out the example at:

https://github.com/crossplane/provider-aws/blob/master/examples/s3/bucket.yaml
in general this works fine, i did set the following acl permission:

spec:
  forProvider:
    acl: private

i expected the bucket to be not public, but when checking the aws console:
(see Block Public Access)

read more about this here:
https://docs.aws.amazon.com/AmazonS3/latest/dev/access-control-block-public-access.html#console-block-public-access-options

@hasheddan mentioned (on slack) it could be done via:
https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/s3#Client.PutPublicAccessBlock
.. but its not implemented yet.

original conversation with @hasheddan on slack:
https://crossplane.slack.com/archives/C01718T2476/p1605300583114500

TODO
please implement switch to enable feature PutPublicAccessBlock on s3 bucket

[JasP19]

I have not contributed before but I would like to try add this enhancement if possible. Of course, any assistance in terms of reviewing the updates I make would be much appreciated.

[muvaf]

Hi @JasP19 ! That would be a good first contribution! Thanks for chiming in!

I was thinking that would be another sub-resource we embed in Bucket object but it seems like Terraform community decided to make it a different API whereas CloudFormation supports it embedded.

I'm closer to sub-resource approach but curious about others' take on it. Do you think you would want to have a separate CRD for this call where you can limit people's RBAC separately form Bucket? i.e. do you think there are cases where I'd like to give Bucket permission to someone but not PutPublicAccessBlock?

[JasP19]

Thanks @muvaf! My initial thoughts were also a sub-resource embedded in Bucket (essentially a sub-object consisting of four Booleans as per the CloudFormation link you sent).

To me a sub-resource makes more sense since it aligns with the experience in the AWS Management Console. I understand the use case of having a separate CRD but, in my opinion, public access control is far more closely coupled to a Bucket than something like a Bucket Policy which gets attached to a Bucket. All in all, I think this setting fits in well on the Bucket CRD with the existing access controls such as grantRead, grantWrite, etc.

If @HerrmannHinz or anyone else has a different opinion then I'm definitely interested to hear it.

[muvaf]

@krishchow @hasheddan how do you guys feel? It's a v1beta1 resource so I want to make sure we're all onboard here since the change would be harder compared to v1alpha1 if we decide to separate later.

[krishchow]

Generally for the other related bucket resources (e.g., Server-side encryption or website configuration), we tried to mirror how to terraform is organizes the calls. I noticed that Terraform separated out resources that the bucket could have multiple of, like how an S3 bucket could have a variable amount of independent bucket policies.

So, just to confirm, an individual bucket can only have one PublicAccessBlock, and the value of the "ExpectedBucketOwner" should always be the same account id as the account used by the provider-aws?

If both of those are true, then I would say that it should be integrated as a sub-resource for the bucket.

[muvaf]

@krishchow looking at API, yes I believe these assumptions are true.

@JasP19 I think we can start working on it. GetPublicAccessBlock, DeletePublicAccessBlock and PutPublicAccessBlock are available in the API. Feel free to ping #dev channel in Crossplane Slack for any help 🙂

[JasP19]

Awesome, thanks! I'll start working on it as soon I get some time.

[publiusi]

Are there any updates on this?

[JasP19]

@publiusi unfortunately I've been quite busy over the past few weeks. I'm still happy to try add this functionality when I can but, if someone is available with more free time than me, I'm also happy to pass it over to them.