[cloud] Ensure that s3_bucket module always enables/disables versioning if it is specified (#21320)

If versioning is not enabled, then `get_bucket_versioning`
can return an empty dict.

If that happens, the code to enable versioning should still
run!

The logic for suspending versioning was also incorrect, so
have updated that too.

Fixes #20491

lib/ansible/modules/cloud/amazon/s3_bucket.py

@@ -49,11 +49,13 @@ options:
     aliases: [ S3_URL ]
   ceph:
     description:
-      - Enable API compatibility with Ceph. It takes into account the S3 API subset working with Ceph in order to provide the same module behaviour where possible.
+      - Enable API compatibility with Ceph. It takes into account the S3 API subset working
+        with Ceph in order to provide the same module behaviour where possible.
     version_added: "2.2"
   requester_pays:
     description:
-      - With Requester Pays buckets, the requester instead of the bucket owner pays the cost of the request and the data download from the bucket.
+      - With Requester Pays buckets, the requester instead of the bucket owner pays the cost
+        of the request and the data download from the bucket.
     required: false
     default: no
     choices: [ 'yes', 'no' ]
@@ -110,16 +112,19 @@ EXAMPLES = '''
 
 '''
 
+import json
 import os
+import traceback
 import xml.etree.ElementTree as ET
-import ansible.module_utils.six.moves.urllib.parse as urlparse
 
-from ansible.module_utils.basic import *
+import ansible.module_utils.six.moves.urllib.parse as urlparse
-from ansible.module_utils.ec2 import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ec2 import get_aws_connection_info, ec2_argument_spec
+from ansible.module_utils.ec2 import sort_json_policy_dict
 
 try:
     import boto.ec2
-    from boto.s3.connection import OrdinaryCallingFormat, Location
+    from boto.s3.connection import OrdinaryCallingFormat, Location, S3Connection
     from boto.s3.tagging import Tags, TagSet
     from boto.exception import BotoServerError, S3CreateError, S3ResponseError
     HAS_BOTO = True
@@ -168,22 +173,21 @@ def _create_or_update_bucket(connection, module, location):
 
     # Versioning
     versioning_status = bucket.get_versioning_status()
-    if versioning_status:
+    if versioning is not None:
-        if versioning is not None:
+        if versioning and versioning_status.get('Versioning') != "Enabled":
-            if versioning and versioning_status['Versioning'] != "Enabled":
+            try:
-                try:
+                bucket.configure_versioning(versioning)
-                    bucket.configure_versioning(versioning)
+                changed = True
-                    changed = True
+                versioning_status = bucket.get_versioning_status()
-                    versioning_status = bucket.get_versioning_status()
+            except S3ResponseError as e:
-                except S3ResponseError as e:
+                module.fail_json(msg=e.message, exception=traceback.format_exc())
-                    module.fail_json(msg=e.message)
+        elif not versioning and versioning_status.get('Versioning') == "Enabled":
-            elif not versioning and versioning_status['Versioning'] != "Enabled":
+            try:
-                try:
+                bucket.configure_versioning(versioning)
-                    bucket.configure_versioning(versioning)
+                changed = True
-                    changed = True
+                versioning_status = bucket.get_versioning_status()
-                    versioning_status = bucket.get_versioning_status()
+            except S3ResponseError as e:
-                except S3ResponseError as e:
+                module.fail_json(msg=e.message, exception=traceback.format_exc())
-                    module.fail_json(msg=e.message)
 
     # Requester pays
     requester_pays_status = get_request_payment_status(bucket)
@@ -247,7 +251,8 @@ def _create_or_update_bucket(connection, module, location):
             except S3ResponseError as e:
                 module.fail_json(msg=e.message)
 
-    module.exit_json(changed=changed, name=bucket.name, versioning=versioning_status, requester_pays=requester_pays_status, policy=current_policy, tags=current_tags_dict)
+    module.exit_json(changed=changed, name=bucket.name, versioning=versioning_status,
+                     requester_pays=requester_pays_status, policy=current_policy, tags=current_tags_dict)
 
 
 def _destroy_bucket(connection, module):

test/sanity/pep8/legacy-files.txt

@@ -64,7 +64,6 @@ lib/ansible/modules/cloud/amazon/redshift.py
 lib/ansible/modules/cloud/amazon/route53.py
 lib/ansible/modules/cloud/amazon/route53_health_check.py
 lib/ansible/modules/cloud/amazon/s3.py
-lib/ansible/modules/cloud/amazon/s3_bucket.py
 lib/ansible/modules/cloud/amazon/s3_lifecycle.py
 lib/ansible/modules/cloud/amazon/s3_sync.py
 lib/ansible/modules/cloud/amazon/s3_website.py