From caaa49475a94ce4a798d34707314c9384d42c9b4 Mon Sep 17 00:00:00 2001 From: Alexander Braverman Masis Date: Fri, 21 Jun 2024 09:58:48 -0400 Subject: [PATCH] s3_logging add bucket policy as default access method to target bucket --- plugins/modules/s3_logging.py | 115 ++++++++++++++++-- .../targets/s3_logging/defaults/main.yml | 1 + .../targets/s3_logging/tasks/main.yml | 83 ++++++++++++- 3 files changed, 186 insertions(+), 13 deletions(-) diff --git a/plugins/modules/s3_logging.py b/plugins/modules/s3_logging.py index 3a78749945f..1d3458ab60a 100644 --- a/plugins/modules/s3_logging.py +++ b/plugins/modules/s3_logging.py @@ -34,6 +34,12 @@ - "The prefix that should be prepended to the generated log files written to the target_bucket." default: "" type: str + target_access: + description: + - "Permissions type for log delivery" + default: policy + choices: [ 'policy', 'acl' ] + type: str extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -58,6 +64,7 @@ state: absent """ +import json try: import botocore except ImportError: @@ -85,8 +92,16 @@ def compare_bucket_logging(bucket_logging, target_bucket, target_prefix): return False -def verify_acls(connection, module, target_bucket): +def verify_acls(connection, module, bucket_name, target_access, target_bucket): + current_obj_ownership = "BucketOwnerEnforced" try: + current_ownership = connection.get_bucket_ownership_controls(aws_retry=True, Bucket=target_bucket) + rules = current_ownership["OwnershipControls"]["Rules"] + for rule in rules: + if "ObjectOwnership" in rule: + current_obj_ownership = rule["ObjectOwnership"] + if target_access == "acl" and current_obj_ownership == "BucketOwnerEnforced": + module.fail_json_aws(msg="The access type is set to ACL but it is disabled on target bucket") current_acl = connection.get_bucket_acl(aws_retry=True, Bucket=target_bucket) current_grants = current_acl["Grants"] except is_boto3_error_code("NoSuchBucket"): @@ -95,25 +110,36 @@ def verify_acls(connection, module, target_bucket): botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError, ) as e: # pylint: disable=duplicate-except - module.fail_json_aws(e, msg="Failed to fetch target bucket ACL") + module.fail_json_aws(e, msg="Failed to fetch target bucket ownership") required_grant = { "Grantee": {"URI": "http://acs.amazonaws.com/groups/s3/LogDelivery", "Type": "Group"}, "Permission": "FULL_CONTROL", } - for grant in current_grants: - if grant == required_grant: + updated_grants = [] + if target_access == "acl": + if required_grant not in list(current_grants): + updated_grants = list(current_grants) + updated_grants.append(required_grant) + else: + return False + else: + if required_grant in list(current_grants): + for grant in current_grants: + if grant != required_grant: + updated_grants.append(grant) + else: return False - - if module.check_mode: - return True updated_acl = dict(current_acl) - updated_grants = list(current_grants) - updated_grants.append(required_grant) updated_acl["Grants"] = updated_grants del updated_acl["ResponseMetadata"] + module.warn(json.dumps(updated_acl)) + + if module.check_mode: + return True + try: connection.put_bucket_acl(aws_retry=True, Bucket=target_bucket, AccessControlPolicy=updated_acl) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: @@ -122,10 +148,75 @@ def verify_acls(connection, module, target_bucket): return True -def enable_bucket_logging(connection, module): +def verify_policy(connection, module, bucket_name, target_access, target_bucket, target_prefix): + policy = { + "Version": "2012-10-17", + "Statement": [] + } + policy_statement = { + "Sid": bucket_name, + "Effect": "Allow", + "Principal": { + "Service": "logging.s3.amazonaws.com" + }, + "Action": "s3:PutObject", + "Resource": f"arn:aws:s3:::{target_bucket}/{target_prefix}*", + } + + try: + current_policy_raw = connection.get_bucket_policy(aws_retry=True, Bucket=target_bucket) + except is_boto3_error_code("NoSuchBucket"): + module.fail_json(msg=f"Target Bucket '{target_bucket}' not found") + except is_boto3_error_code("NoSuchBucketPolicy"): + current_policy_raw = {"Policy": json.dumps(policy)} + except ( + botocore.exceptions.BotoCoreError, + botocore.exceptions.ClientError, + ) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg="Failed to fetch target bucket policy") + + try: + current_policy = json.loads(current_policy_raw["Policy"]) + except json.JSONDecodeError as e: + module.fail_json(e, msg="Unable to parse policy") + + new_policy_statements = [] + new_policy = current_policy + for p in current_policy.get("Statement", []): + if p == policy_statement and target_access == "policy": + return False + if target_access == "acl": + for p in current_policy.get("Statement", []): + if p != policy_statement: + new_policy_statements.append(p) + new_policy["Statement"] = new_policy_statements + if new_policy == current_policy: + return False + else: + new_policy = current_policy + if new_policy["Statement"] is None: + new_policy["Statement"] = [policy_statement] + else: + new_policy["Statement"].append(policy_statement) + + if module.check_mode: + return True + + try: + connection.put_bucket_policy(aws_retry=True, Bucket=target_bucket, Policy=json.dumps(new_policy)) + except is_boto3_error_code("MalformedPolicy"): + module.fail_json(msg=f"Bad policy:\n {new_policy}") + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg="Failed to update target bucket policy to allow log delivery") + + return True + + +def enable_bucket_logging(connection, module: AnsibleAWSModule): bucket_name = module.params.get("name") target_bucket = module.params.get("target_bucket") target_prefix = module.params.get("target_prefix") + target_access = module.params.get("target_access") changed = False try: @@ -139,7 +230,8 @@ def enable_bucket_logging(connection, module): module.fail_json_aws(e, msg="Failed to fetch current logging status") try: - changed |= verify_acls(connection, module, target_bucket) + changed |= verify_acls(connection, module, bucket_name, target_access, target_bucket) + changed |= verify_policy(connection, module, bucket_name, target_access, target_bucket, target_prefix) if not compare_bucket_logging(bucket_logging, target_bucket, target_prefix): bucket_logging = camel_dict_to_snake_dict(bucket_logging) @@ -196,6 +288,7 @@ def main(): name=dict(required=True), target_bucket=dict(required=False, default=None), target_prefix=dict(required=False, default=""), + target_access=dict(required=False, default="policy", choices=["policy", "acl"]), state=dict(required=False, default="present", choices=["present", "absent"]), ) diff --git a/tests/integration/targets/s3_logging/defaults/main.yml b/tests/integration/targets/s3_logging/defaults/main.yml index f8f1d758746..f9751bd8d33 100644 --- a/tests/integration/targets/s3_logging/defaults/main.yml +++ b/tests/integration/targets/s3_logging/defaults/main.yml @@ -2,3 +2,4 @@ test_bucket: '{{ tiny_prefix }}-s3-logging' log_bucket_1: '{{ tiny_prefix }}-logs-1' log_bucket_2: '{{ tiny_prefix }}-logs-2' +log_bucket_3: '{{ tiny_prefix }}-logs-3' diff --git a/tests/integration/targets/s3_logging/tasks/main.yml b/tests/integration/targets/s3_logging/tasks/main.yml index e9a7b220b52..18be8a576f6 100644 --- a/tests/integration/targets/s3_logging/tasks/main.yml +++ b/tests/integration/targets/s3_logging/tasks/main.yml @@ -46,7 +46,6 @@ s3_bucket: state: present name: '{{ log_bucket_1 }}' - object_ownership: BucketOwnerPreferred register: output - assert: that: @@ -57,13 +56,23 @@ s3_bucket: state: present name: '{{ log_bucket_2 }}' - object_ownership: BucketOwnerPreferred register: output - assert: that: - output is changed - output.name == log_bucket_2 + - name: Create simple s3_bucket as third target for logs + s3_bucket: + state: present + name: '{{ log_bucket_3 }}' + object_ownership: BucketOwnerPreferred + register: output + - assert: + that: + - output is changed + - output.name == log_bucket_3 + # ============================================================ - name: Enable logging (check_mode) @@ -152,6 +161,76 @@ that: - result is not changed +# ============================================================ + + - name: ACL logging bucket (check_mode) + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: acl + register: result + check_mode: True + - assert: + that: + - result is changed + + - name: ACL logging bucket + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: acl + register: result + - assert: + that: + - result is changed + + - name: ACL logging bucket idempotency (check_mode) + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: acl + register: result + check_mode: True + - assert: + that: + - result is not changed + + - name: ACL logging bucket idempotency + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: acl + register: result + - assert: + that: + - result is not changed + + - name: ACL logging bucket convert to policy + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: policy + register: result + - assert: + that: + - result is changed + + - name: policy logging bucket convert to ACL + s3_logging: + state: present + name: '{{ test_bucket }}' + target_bucket: '{{ log_bucket_3 }}' + target_access: acl + register: result + - assert: + that: + - result is changed + # ============================================================ - name: Change logging prefix (check_mode)