From 0dbac7a748f143811d5d3d82e5dd38b5e3aba3f0 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Wed, 21 Aug 2024 17:43:14 +0200
Subject: [PATCH] s3_bucket - remove default for accelerate_enabled (#2222)

SUMMARY
fixes #2220
Removes the default for accelerate_enabled:

having the default explicitly set to false breaks idempotency when only passed partial configuration
having the default explicitly set results in the module always failing on non-AWS platforms

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
s3_bucket
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
---
 .../2220-accelerate_enabled-unsupported.yml   |  2 +
 plugins/modules/s3_bucket.py                  | 42 ++++++++++---------
 2 files changed, 25 insertions(+), 19 deletions(-)
 create mode 100644 changelogs/fragments/2220-accelerate_enabled-unsupported.yml

diff --git a/changelogs/fragments/2220-accelerate_enabled-unsupported.yml b/changelogs/fragments/2220-accelerate_enabled-unsupported.yml
new file mode 100644
index 00000000000..778b4b1c1fd
--- /dev/null
+++ b/changelogs/fragments/2220-accelerate_enabled-unsupported.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- s3_bucket - change the default behaviour of the new ``accelerate_enabled`` option to only update the configuration if explicitly passed (https://github.com/ansible-collections/amazon.aws/issues/2220).
diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py
index ca42044a439..b25e176b46f 100644
--- a/plugins/modules/s3_bucket.py
+++ b/plugins/modules/s3_bucket.py
@@ -172,7 +172,6 @@
       - Transfer Acceleration is not available in AWS GovCloud (US).
       - See U(https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html#govcloud-S3-diffs).
     type: bool
-    default: false
     version_added: 8.1.0
   object_lock_default_retention:
     description:
@@ -969,17 +968,18 @@ def handle_bucket_accelerate(s3_client, module: AnsibleAWSModule, name: str) ->
     ) as e:  # pylint: disable=duplicate-except
         module.fail_json_aws(e, msg="Failed to fetch bucket transfer acceleration state")
     else:
-        try:
-            if not accelerate_enabled and accelerate_status:
-                delete_bucket_accelerate_configuration(s3_client, name)
-                accelerate_enabled_changed = True
-                accelerate_enabled_result = False
-            if accelerate_enabled and not accelerate_status:
-                put_bucket_accelerate_configuration(s3_client, name)
-                accelerate_enabled_changed = True
-                accelerate_enabled_result = True
-        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
-            module.fail_json_aws(e, msg="Failed to update bucket transfer acceleration")
+        if accelerate_enabled is not None:
+            try:
+                if not accelerate_enabled and accelerate_status:
+                    delete_bucket_accelerate_configuration(s3_client, name)
+                    accelerate_enabled_changed = True
+                    accelerate_enabled_result = False
+                if accelerate_enabled and not accelerate_status:
+                    put_bucket_accelerate_configuration(s3_client, name)
+                    accelerate_enabled_changed = True
+                    accelerate_enabled_result = True
+            except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+                module.fail_json_aws(e, msg="Failed to update bucket transfer acceleration")
     return accelerate_enabled_changed, accelerate_enabled_result
 
 
@@ -1989,15 +1989,21 @@ def destroy_bucket(s3_client, module: AnsibleAWSModule) -> None:
 
 def main():
     argument_spec = dict(
-        force=dict(default=False, type="bool"),
-        policy=dict(type="json"),
         name=dict(required=True),
-        requester_pays=dict(type="bool"),
+        validate_bucket_name=dict(type="bool", default=True),
+        dualstack=dict(default=False, type="bool"),
         state=dict(default="present", choices=["present", "absent"]),
+        ceph=dict(default=False, type="bool", aliases=["rgw"]),
+        # ** Warning **
+        # we support non-AWS implementations, only force/purge options should have a
+        # default set for any top-level option.  We need to be able to identify
+        # unset options where we can ignore NotImplemented exceptions.
         tags=dict(type="dict", aliases=["resource_tags"]),
         purge_tags=dict(type="bool", default=True),
+        force=dict(default=False, type="bool"),
+        policy=dict(type="json"),
         versioning=dict(type="bool"),
-        ceph=dict(default=False, type="bool", aliases=["rgw"]),
+        requester_pays=dict(type="bool"),
         encryption=dict(choices=["none", "AES256", "aws:kms"]),
         encryption_key_id=dict(),
         bucket_key_enabled=dict(type="bool"),
@@ -2014,9 +2020,7 @@ def main():
         object_ownership=dict(type="str", choices=["BucketOwnerEnforced", "BucketOwnerPreferred", "ObjectWriter"]),
         delete_object_ownership=dict(type="bool", default=False),
         acl=dict(type="str", choices=["private", "public-read", "public-read-write", "authenticated-read"]),
-        validate_bucket_name=dict(type="bool", default=True),
-        dualstack=dict(default=False, type="bool"),
-        accelerate_enabled=dict(default=False, type="bool"),
+        accelerate_enabled=dict(type="bool"),
         object_lock_enabled=dict(type="bool"),
         object_lock_default_retention=dict(
             type="dict",