Apply suggested changes

Signed-off-by: Alina Buzachis <[email protected]>

changelogs/fragments/s3_bucket-object-retention.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - s3_bucket - Add ``object_lock_default_retention`` to set Object Lock default retention configuration for S3 buckets (https://github.com/ansible-collections/amazon.aws/pull/2062).

plugins/modules/s3_bucket.py

@@ -170,23 +170,23 @@
     description:
       - Default Object Lock configuration that will be applied by default to
         every new object placed in the specified bucket.
-      - I(object_lock_enabled) must be included and set to C(True)
+      - O(object_lock_enabled) must be included and set to V(True).
       - Object lock retention policy can't be removed.
     suboptions:
       mode:
-        description: Type of retention modes
-        choices: [ 'GOVERNANCE', "['COMPLIANCE']"]
+        description: Type of retention modes.
+        choices: [ "GOVERNANCE", "COMPLIANCE"]
         required: true
         type: str
       days:
         description:
             - The number of days that you want to specify for the default retention period.
-            - Mutually exclusive with I(years).
+            - Mutually exclusive with O(object_lock_default_retention.years).
         type: int
       years:
         description:
             - The number of years that you want to specify for the default retention period.
-            - Mutually exclusive with I(days).
+            - Mutually exclusive with O(object_lock_default_retention.days).
         type: int
     type: dict
     version_added: 8.1.0
@@ -342,7 +342,7 @@
 object_lock_default_retention:
     description: S3 bucket's object lock retention policy.
     type: dict
-    returned: I(state=present)
+    returned: when O(state=present)
     sample: {
         "Days": 1,
         "Mode": "GOVERNANCE",
@@ -938,10 +938,10 @@ def handle_bucket_object_lock_retention(s3_client, module: AnsibleAWSModule, nam
             object_lock_configuration_status = {}
     except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e:
         if object_lock_default_retention is not None:
-            module.fail_json(msg="Fetching bucket object lock default retention is not supported")
+            module.fail_json_aws(e, msg="Fetching bucket object lock default retention is not supported")
     except is_boto3_error_code("AccessDenied") as e:  # pylint: disable=duplicate-except
         if object_lock_default_retention is not None:
-            module.fail_json(msg="Permission denied fetching object lock default retention for bucket")
+            module.fail_json_aws(e, msg="Permission denied fetching object lock default retention for bucket")
     except (
         botocore.exceptions.BotoCoreError,
         botocore.exceptions.ClientError,
@@ -954,7 +954,7 @@ def handle_bucket_object_lock_retention(s3_client, module: AnsibleAWSModule, nam
             conf = snake_dict_to_camel_dict(object_lock_default_retention, capitalize_first=True)
             conf = {k: v for k, v in conf.items() if v}  # remove keys with None value
             try:
-                if object_lock_default_retention and object_lock_configuration_status != conf:                    
+                if object_lock_default_retention and object_lock_configuration_status != conf:
                     put_object_lock_configuration(s3_client, name, conf)
                     object_lock_default_retention_changed = True
                     object_lock_default_retention_result = object_lock_default_retention
@@ -1043,7 +1043,9 @@ def create_or_update_bucket(s3_client, module: AnsibleAWSModule):
     result["object_lock_enabled"] = bucket_object_lock_result
 
     # -- Object Lock Default Retention
-    bucket_object_lock_retention_changed, bucket_object_lock_retention_result = handle_bucket_object_lock_retention(s3_client, module, name)
+    bucket_object_lock_retention_changed, bucket_object_lock_retention_result = handle_bucket_object_lock_retention(
+        s3_client, module, name
+    )
     result["object_lock_default_retention"] = bucket_object_lock_retention_result
 
     # Module exit
@@ -1111,18 +1113,34 @@ def create_bucket(s3_client, bucket_name: str, location: str, object_lock_enable
         # method. However, the AWS Api sometimes fails to report bucket presence, so we catch this exception
         return False
 
+
 @AWSRetry.exponential_backoff(max_delay=120, catch_extra_error_codes=["NoSuchBucket", "OperationAborted"])
 def get_object_lock_configuration(s3_client, bucket_name):
+    """
+    Get the object lock default retention configuration for an S3 bucket.
+    Parameters:
+        s3_client (boto3.client): The Boto3 S3 client object.
+        bucket_name (str): The name of the S3 bucket.
+    Returns:
+        Object lock default retention configuration dictionary.
+    """
     result = s3_client.get_object_lock_configuration(Bucket=bucket_name)
-    return result.get("ObjectLockConfiguration",{}).get("Rule", {}).get("DefaultRetention", {})
+    return result.get("ObjectLockConfiguration", {}).get("Rule", {}).get("DefaultRetention", {})
 
 
 @AWSRetry.exponential_backoff(max_delay=120, catch_extra_error_codes=["NoSuchBucket", "OperationAborted"])
 def put_object_lock_configuration(s3_client, bucket_name, object_lock_default_retention):
-    conf = {
-        "ObjectLockEnabled": "Enabled",
-        "Rule": { "DefaultRetention": object_lock_default_retention }
-    }
+    """
+    Set tags for an S3 bucket.
+    Parameters:
+        s3_client (boto3.client): The Boto3 S3 client object.
+        bucket_name (str): The name of the S3 bucket.
+        object_lock_default_retention (dict): A dictionary containing the object
+        lock default retention configuration to be set on the bucket.
+    Returns:
+        None
+    """
+    conf = {"ObjectLockEnabled": "Enabled", "Rule": {"DefaultRetention": object_lock_default_retention}}
     s3_client.put_object_lock_configuration(Bucket=bucket_name, ObjectLockConfiguration=conf)
 
 
@@ -1889,13 +1907,13 @@ def main():
         object_lock_default_retention=dict(
             type="dict",
             options=dict(
-                mode=dict(type="str", choices=["GOVERNANCE", ["COMPLIANCE"]], required=True),
+                mode=dict(type="str", choices=["GOVERNANCE", "COMPLIANCE"], required=True),
                 years=dict(type="int"),
                 days=dict(type="int"),
             ),
             mutually_exclusive=[("days", "years")],
-            required_one_of=[('days', 'years')],
-        )
+            required_one_of=[("days", "years")],
+        ),
     )
 
     required_by = dict(

tests/integration/targets/s3_bucket/inventory

@@ -1,5 +1,4 @@
 [tests]
-default_retention
 ownership_controls
 missing
 simple
@@ -12,6 +11,7 @@ encryption_sse
 public_access
 acl
 object_lock
+default_retention
 
 [all:vars]
 ansible_connection=local

tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/default_retention.yml

@@ -66,7 +66,7 @@
           - output.changed
           - output.object_lock_enabled
           - output.object_lock_default_retention
-    
+
     - name: Touch bucket with object lock enabled (idempotency)
       amazon.aws.s3_bucket:
         name: "{{ local_bucket_name }}-2"