Provide support for AWS S3 Public Access Blocking

[zeten30]

SUMMARY

Provide support to configure public access block on S3 bucket (including tests).
Fixes #144

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

s3_bucket

ADDITIONAL INFORMATION

• https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.put_public_access_block
• https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.get_public_access_block

[tremble]

Hi Milan,

thanks for taking the time to submit this. In general it's better to use options/suboptions rather than manually validating things later. It would also be good if you could test that you don't change values when you haven't intentionally set them.

[tremble]

This would be better handled as "suboptions" on the parameter, will show how below.

[tremble]

Suggested change

	public_access=dict(type='dict')
	public_access=dict(type='dict', options=dict(
	block_public_acls=dict(type='bool'),
	ignore_public_acls=dict(type='bool'),
	block_public_policy=dict(type='bool'),
	restrict_public_buckets=dict(type='bool')),

This can be converted to the format used by boto3 using snake_dict_to_camel_dict (and back again using camel_dict_to_snake_dict) these functions can be found in ansible.module_utils.common.dict_transformations

[zeten30]

Sure, that will be much better. I was not aware about this 'dict in dict'.

[tremble]

Please also add a test to the 'basic' suite to make sure your PR doesn't change the default behaviour.

[zeten30]

I don't think I get this. Can you please explain in further detail? I don't see any file named basic in tests/. I've followed what's already in place for 'encryption_kms' test. It's in inventory and in tasks/. Thanks in advance.

[tremble]

Simplest version would be to tweak https://github.com/ansible-collections/amazon.aws/blob/main/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/simple.yml and make sure that the returned values for public_access match those of Amazon's defaults if you didn't pass public_access.

The intention is to ensure that in adding the new public_access option you've not changed the default behaviour when someone creates a bucket.

[zeten30]

Ok, makes perfect sense now:) TY.

[zeten30]

@tremble Please check the latest changes. I hope I've adapted all requested changes.

I'm stuck with failing tests. All T=aws/X.Y/1 are throwing:

ClientError: An error occurred (AccessDenied) when calling the GetPublicAccessBlock operation: Access Denied

Isn't it a testing user permissions problem? Similar to: https://aws.amazon.com/premiumsupport/knowledge-center/s3-access-denied-bucket-policy/

I have no issues when running unit test locally.

[zeten30]

I think that testing account is probably missing:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "VisualEditor0",
            "Effect": "Allow",
            "Action": [
                "s3:GetBucketPublicAccessBlock",
                "s3:PutBucketPublicAccessBlock"
            ],
            "Resource": "*"
        }
    ]
}

[zeten30]

# Before applying policy
$ aws --profile testing_profile s3api get-public-access-block --bucket testing-bucket-ansible-7

An error occurred (AccessDenied) when calling the GetPublicAccessBlock operation: Access Denied

# After applying policy
$ aws --profile testing_profile  s3api get-public-access-block --bucket testing-bucket-ansible-7
{
    "PublicAccessBlockConfiguration": {
        "BlockPublicAcls": true,
        "IgnorePublicAcls": true,
        "BlockPublicPolicy": true,
        "RestrictPublicBuckets": true
    }
}

# Other S3 API calls (not defined in policy)
aws --profile testing_profile s3api get-bucket-policy --bucket testing-bucket-ansible-7

An error occurred (AccessDenied) when calling the GetBucketPolicy operation: Access Denied

[zeten30]

Got rid of unnecessary API public block calls .. but it's still failing due to permission in public_access test.

Local tests are ok.

complex                    : ok=24   changed=6    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
dotted                     : ok=18   changed=4    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
encryption_kms             : ok=21   changed=6    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
encryption_sse             : ok=21   changed=6    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
missing                    : ok=12   changed=2    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
public_access              : ok=22   changed=6    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
simple                     : ok=18   changed=4    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
tags                       : ok=38   changed=9    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

AWS ACTIONS: ['s3:CreateBucket', 's3:DeleteBucket', 's3:DeleteBucketEncryption', 's3:DeleteBucketTagging', 's3:DeletePublicAccessBlock', 's3:GetBucketEncryption', 's3:GetBucketPolicy', 's3:GetBucketRequestPayment', 's3:GetBucketTagging', 's3:GetBucketVersioning', 's3:GetPublicAccessBlock', 's3:HeadBucket', 's3:ListBuckets', 's3:PutBucketEncryption', 's3:PutBucketPolicy', 's3:PutBucketRequestPayment', 's3:PutBucketTagging', 's3:PutBucketVersioning', 's3:PutPublicAccessBlock']

[tremble]

Please submit a PR against the CI policy:

https://github.com/mattclay/aws-terminator/blob/master/aws/policy/storage-services.yaml

[tremble]

@zeten30

I've realised there's one small thing I forgot to request: please add a changelog entry. CI policy has now been updated so we're good to merge once we have a changelog entry

[tremble]

Many thanks for your efforts on this. I'm sorry it's taken so long.

Given how long you've been waiting on us for this I've added a changelog fragment and some missing version_added parameters to the documentation.

[zeten30]

@tremble Thank you very much. I was about to add changelog later today, but you were faster;)