Skip to content
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.

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

S3 Logging add option to disable ACL setup #2136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 25 additions & 3 deletions plugins/modules/s3_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@
- "The prefix that should be prepended to the generated log files written to the target_bucket."
default: ""
type: str
acl:
description:
- "Setup target bucket ACLs to grant AWS special log delivery account to write server access logs."
abraverm marked this conversation as resolved.
Show resolved Hide resolved
- "Setting to False will remove the ACL for log delivery on the target bucket."
default: True
type: bool
abraverm marked this conversation as resolved.
Show resolved Hide resolved
version_added: 8.3.0
extends_documentation_fragment:
- amazon.aws.common.modules
- amazon.aws.region.modules
Expand Down Expand Up @@ -95,23 +102,37 @@ def verify_acls(connection, module, target_bucket):
botocore.exceptions.BotoCoreError,
botocore.exceptions.ClientError,
) as e: # pylint: disable=duplicate-except
if not module.params.get("acl"):
module.warn(f"Unable to fetch Bucket ACLs ({e})")
return False
module.fail_json_aws(e, msg="Failed to fetch target bucket ACL")

required_grant = {
"Grantee": {"URI": "http://acs.amazonaws.com/groups/s3/LogDelivery", "Type": "Group"},
"Permission": "FULL_CONTROL",
abraverm marked this conversation as resolved.
Show resolved Hide resolved
}

grant_present = False
for grant in current_grants:
if grant == required_grant:
return False
grant_present = True

if module.params.get("acl") == grant_present:
return False

if module.check_mode:
return True

updated_acl = dict(current_acl)
updated_grants = list(current_grants)
updated_grants.append(required_grant)
updated_grants = []
if module.params.get("acl"):
updated_grants = list(current_grants)
updated_grants.append(required_grant)
else:
for grant in current_grants:
if grant != required_grant:
updated_grants.append(grant)

updated_acl["Grants"] = updated_grants
del updated_acl["ResponseMetadata"]
try:
Expand Down Expand Up @@ -197,6 +218,7 @@ def main():
target_bucket=dict(required=False, default=None),
target_prefix=dict(required=False, default=""),
state=dict(required=False, default="present", choices=["present", "absent"]),
acl=dict(type="bool", default=True),
)

module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True)
Expand Down
1 change: 1 addition & 0 deletions tests/integration/targets/s3_logging/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
102 changes: 102 additions & 0 deletions tests/integration/targets/s3_logging/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@
- 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)
Expand Down Expand Up @@ -152,6 +163,97 @@
that:
- result is not changed

# ============================================================

- name: Disable ACL on logging bucket (check_mode)
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
acl: False
register: result
check_mode: True
- assert:
that:
- result is changed

- name: Disable ACL logging bucket
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
acl: False
register: result
- assert:
that:
- result is changed

- name: Disable ACL on logging bucket idempotency (check_mode)
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
acl: False
register: result
check_mode: True
- assert:
that:
- result is not changed

- name: Disable ACL on logging bucket idempotency
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
acl: False
register: result
- assert:
that:
- result is not changed

- name: Re-Enable ACL on logging bucket (check_mode)
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
register: result
check_mode: True
- assert:
that:
- result is changed

- name: Re-Enable ACL logging bucket
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
register: result
- assert:
that:
- result is changed

- name: Re-Enable ACL on logging bucket idempotency (check_mode)
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
register: result
check_mode: True
- assert:
that:
- result is not changed

- name: Re-Enable ACL on logging bucket idempotency
s3_logging:
state: present
name: '{{ test_bucket }}'
target_bucket: '{{ log_bucket_2 }}'
register: result
- assert:
that:
- result is not changed


# ============================================================

- name: Change logging prefix (check_mode)
Expand Down
Loading