AWS S3 : Fixup sanity test issues and deprecate 2 unused options (#63989)

* S3 module Sanity check fixes.

* s3_lifecycle: deprecate unused 'requester_pays' option

* s3_sync: deprecate unused retries option

* Add changelog fragment

changelogs/fragments/63989-deprecate-unused.yml

@@ -0,0 +1,3 @@
+deprecated_features:
+- "s3_sync - The ``retries`` option had no effect and will be removed in Ansible 2.14"
+- "s3_lifecycle - The ``requester_pays`` option had no effect and will be removed in Ansible 2.14"

docs/docsite/rst/porting_guides/porting_guide_2.10.rst

@@ -54,6 +54,8 @@ The following functionality will be removed in Ansible 2.14. Please update updat
 * The :ref:`openssl_csr <openssl_csr_module>` module's option ``version`` no longer supports values other than ``1`` (the current only standardized CSR version).
 * :ref:`docker_container <docker_container_module>`: the ``trust_image_content`` option will be removed. It has always been ignored by the module.
 * :ref:`iam_managed_policy <iam_managed_policy_module>`: the ``fail_on_delete`` option wil be removed.  It has always been ignored by the module.
+* :ref:`s3_lifecycle <s3_lifecycle_module>`: the ``requester_pays`` option will be removed. It has always been ignored by the module.
+* :ref:`s3_sync <s3_sync_module>`: the ``retries`` option will be removed. It has always been ignored by the module.
 
 
 Noteworthy module changes

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

@@ -2,6 +2,10 @@
 # Copyright (c) 2017 Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 #
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
                     'supported_by': 'community'}
@@ -19,14 +23,17 @@ options:
     description:
       - Name of the s3 bucket
     required: true
+    type: str
   rules:
     description:
       - Cors rules to put on the s3 bucket
+    type: list
   state:
     description:
       - Create or remove cors on the s3 bucket
     required: true
     choices: [ 'present', 'absent' ]
+    type: str
 extends_documentation_fragment:
     - aws
     - ec2

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

@@ -13,6 +13,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['stableinterface'],
                     'supported_by': 'core'}
@@ -38,15 +41,18 @@ options:
     description:
       - Name of the s3 bucket
     required: true
+    type: str
   policy:
     description:
       - The JSON policy as a string.
+    type: json
   s3_url:
     description:
       - S3 URL endpoint for usage with DigitalOcean, Ceph, Eucalyptus and fakes3 etc.
       - Assumes AWS if not specified.
       - For Walrus, use FQDN of the endpoint without scheme nor path.
     aliases: [ S3_URL ]
+    type: str
   ceph:
     description:
       - Enable API compatibility with Ceph. It takes into account the S3 API subset working
@@ -65,9 +71,11 @@ options:
     required: false
     default: present
     choices: [ 'present', 'absent' ]
+    type: str
   tags:
     description:
       - tags dict to apply to bucket
+    type: dict
   purge_tags:
     description:
       - whether to remove tags that aren't present in the C(tags) parameter
@@ -84,10 +92,12 @@ options:
         In order to remove the server-side encryption, the encryption needs to be set to 'none' explicitly.
     choices: [ 'none', 'AES256', 'aws:kms' ]
     version_added: "2.9"
+    type: str
   encryption_key_id:
     description: KMS master key ID to use for the default encryption. This parameter is allowed if encryption is aws:kms. If
                  not specified then it will default to the AWS provided KMS key.
     version_added: "2.9"
+    type: str
 extends_documentation_fragment:
     - aws
     - ec2

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

@@ -29,17 +29,21 @@ options:
     description:
       - "Name of the s3 bucket"
     required: true
+    type: str
   expiration_date:
     description:
       - >
         Indicates the lifetime of the objects that are subject to the rule by the date they will expire. The value must be ISO-8601 format, the time must
         be midnight and a GMT timezone must be specified.
+    type: str
   expiration_days:
     description:
       - "Indicates the lifetime, in days, of the objects that are subject to the rule. The value must be a non-zero positive integer."
+    type: int
   prefix:
     description:
       - "Prefix identifying one or more objects to which the rule applies.  If no prefix is specified, the rule will apply to the whole bucket."
+    type: str
   purge_transitions:
     description:
       - >
@@ -54,6 +58,7 @@ options:
       - 'Delete noncurrent versions this many days after they become noncurrent'
     required: false
     version_added: 2.6
+    type: int
   noncurrent_version_storage_class:
     description:
       - 'Transition noncurrent versions to this storage class'
@@ -61,11 +66,13 @@ options:
     choices: ['glacier', 'onezone_ia', 'standard_ia']
     required: false
     version_added: 2.6
+    type: str
   noncurrent_version_transition_days:
     description:
       - 'Transition noncurrent versions this many days after they become noncurrent'
     required: false
     version_added: 2.6
+    type: int
   noncurrent_version_transitions:
     description:
       - >
@@ -74,34 +81,41 @@ options:
           I(transition_days)
           I(storage_class)
     version_added: 2.6
+    type: list
   rule_id:
     description:
       - "Unique identifier for the rule. The value cannot be longer than 255 characters. A unique value for the rule will be generated if no value is provided."
+    type: str
   state:
     description:
       - "Create or remove the lifecycle rule"
     default: present
     choices: [ 'present', 'absent' ]
+    type: str
   status:
     description:
       - "If 'enabled', the rule is currently being applied. If 'disabled', the rule is not currently being applied."
     default: enabled
     choices: [ 'enabled', 'disabled' ]
+    type: str
   storage_class:
     description:
       - "The storage class to transition to. Currently there are two supported values - 'glacier',  'onezone_ia', or 'standard_ia'."
       - "The 'standard_ia' class is only being available from Ansible version 2.2."
     default: glacier
     choices: [ 'glacier', 'onezone_ia', 'standard_ia']
+    type: str
   transition_date:
     description:
       - >
         Indicates the lifetime of the objects that are subject to the rule by the date they will transition to a different storage class.
         The value must be ISO-8601 format, the time must be midnight and a GMT timezone must be specified. If transition_days is not specified,
         this parameter is required."
+    type: str
   transition_days:
     description:
       - "Indicates when, in days, an object transitions to a different storage class. If transition_date is not specified, this parameter is required."
+    type: int
   transitions:
     description:
       - A list of transition behaviors to be applied to the rule. Each storage class may be used only once. Each transition
@@ -110,6 +124,11 @@ options:
           I(transition_date)
           I(storage_class)
     version_added: 2.6
+    type: list
+  requester_pays:
+    description:
+      - The I(requester_pays) option does nothing and will be removed in Ansible 2.14.
+    type: bool
 extends_documentation_fragment:
     - aws
     - ec2
@@ -435,7 +454,7 @@ def main():
         noncurrent_version_transition_days=dict(type='int'),
         noncurrent_version_transitions=dict(type='list'),
         prefix=dict(),
-        requester_pays=dict(default='no', type='bool'),
+        requester_pays=dict(type='bool', removed_in_version='2.14'),
         rule_id=dict(),
         state=dict(default='present', choices=['present', 'absent']),
         status=dict(default='enabled', choices=['enabled', 'disabled']),

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

@@ -14,6 +14,9 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
                     'supported_by': 'community'}
@@ -32,7 +35,8 @@ options:
     description:
     - sync direction.
     default: 'push'
-    choices: [ push ]
+    choices: [ 'push' ]
+    type: str
   file_change_strategy:
     description:
     - Difference determination method to allow changes-only syncing. Unlike rsync, files are not patched- they are fully skipped or fully uploaded.
@@ -41,32 +45,45 @@ options:
     - force will always upload all files.
     required: false
     default: 'date_size'
-    choices: [ force, checksum, date_size ]
+    choices: [ 'force', 'checksum', 'date_size' ]
+    type: str
   bucket:
     description:
     - Bucket name.
     required: true
+    type: str
   key_prefix:
     description:
     - In addition to file path, prepend s3 path with this prefix. Module will add slash at end of prefix if necessary.
     required: false
+    type: str
   file_root:
     description:
     - File/directory path for synchronization. This is a local path.
     - This root path is scrubbed from the key name, so subdirectories will remain as keys.
     required: true
+    type: path
   permission:
     description:
     - Canned ACL to apply to synced files.
     - Changing this ACL only changes newly synced files, it does not trigger a full reupload.
     required: false
-    choices: [ '', private, public-read, public-read-write, authenticated-read, aws-exec-read, bucket-owner-read, bucket-owner-full-control ]
+    choices:
+    - 'private'
+    - 'public-read'
+    - 'public-read-write'
+    - 'authenticated-read'
+    - 'aws-exec-read'
+    - 'bucket-owner-read'
+    - 'bucket-owner-full-control'
+    type: str
   mime_map:
     description:
     - >
       Dict entry from extension to MIME type. This will override any default/sniffed MIME type.
       For example C({".txt": "application/text", ".yml": "application/text"})
     required: false
+    type: dict
   include:
     description:
     - Shell pattern-style file matching.
@@ -74,6 +91,7 @@ options:
     - For multiple patterns, comma-separate them.
     required: false
     default: "*"
+    type: str
   exclude:
     description:
     - Shell pattern-style file matching.
@@ -81,13 +99,14 @@ options:
     - For multiple patterns, comma-separate them.
     required: false
     default: ".*"
+    type: str
   cache_control:
     description:
-    - This is a string.
     - Cache-Control header set on uploaded objects.
     - Directives are separated by commas.
     required: false
     version_added: "2.4"
+    type: str
   delete:
     description:
     - Remove remote files that exist in bucket but are not present in the file root.
@@ -95,6 +114,10 @@ options:
     default: no
     version_added: "2.4"
     type: bool
+  retries:
+    description:
+      - The I(retries) option does nothing and will be removed in Ansible 2.14.
+    type: str
 
 requirements:
   - boto3 >= 1.4.4
@@ -356,7 +379,7 @@ def determine_mimetypes(filelist, override_map):
         localfile = fileentry['fullpath']
 
         # reminder: file extension is '.txt', not 'txt'.
-        _, file_extension = os.path.splitext(localfile)
+        file_extension = os.path.splitext(localfile)[1]
         if override_map and override_map.get(file_extension):
             # override? use it.
             retentry['mime_type'] = override_map[file_extension]
@@ -489,7 +512,7 @@ def main():
         file_root=dict(required=True, type='path'),
         permission=dict(required=False, choices=['private', 'public-read', 'public-read-write', 'authenticated-read',
                                                  'aws-exec-read', 'bucket-owner-read', 'bucket-owner-full-control']),
-        retries=dict(required=False),
+        retries=dict(required=False, removed_in_version='2.14'),
         mime_map=dict(required=False, type='dict'),
         exclude=dict(required=False, default=".*"),
         include=dict(required=False, default="*"),

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

@@ -25,23 +25,21 @@ options:
     description:
       - "Name of the s3 bucket"
     required: true
+    type: str
   error_key:
     description:
       - "The object key name to use when a 4XX class error occurs. To remove an error key, set to None."
+    type: str
   redirect_all_requests:
     description:
       - "Describes the redirect behavior for every request to this s3 bucket website endpoint"
-  region:
+    type: str
-    description:
-     - >
-       AWS region to create the bucket in. If not set then the value of the AWS_REGION and EC2_REGION environment variables are checked,
-       followed by the aws_region and ec2_region settings in the Boto config file.  If none of those are set the region defaults to the
-       S3 Location: US Standard.
   state:
     description:
       - "Add or remove s3 website configuration"
-    default: present
     choices: [ 'present', 'absent' ]
+    required: true
+    type: str
   suffix:
     description:
       - >
@@ -49,6 +47,7 @@ options:
         samplebucket/images/ the data that is returned will be for the object with the key name images/index.html). The suffix must not include a slash
         character.
     default: index.html
+    type: str
 
 extends_documentation_fragment:
   - aws
@@ -109,30 +108,59 @@ redirect_all_requests_to:
             returned: when redirect all requests parameter set
             type: str
             sample: ansible.com
+        protocol:
+            description: protocol to use when redirecting requests.
+            returned: when redirect all requests parameter set
+            type: str
+            sample: https
 routing_rules:
     description: routing rules
-    type: complex
+    type: list
     returned: always
     contains:
-        routing_rule:
-            host_name:
-                description: name of the host where requests will be redirected.
-                returned: when host name set as part of redirect rule
-                type: str
-                sample: ansible.com
         condition:
-            key_prefix_equals:
+            type: complex
-            description: object key name prefix when the redirect is applied. For example, to redirect requests for ExamplePage.html, the key prefix will be
+            description: A container for describing a condition that must be met for the specified redirect to apply.
-                     ExamplePage.html
+            contains:
-            returned: when routing rule present
+                http_error_code_returned_equals:
-            type: str
+                    description: The HTTP error code when the redirect is applied.
-            sample: docs/
+                    returned: always
+                    type: str
+                key_prefix_equals:
+                    description: object key name prefix when the redirect is applied. For example, to redirect
+                                 requests for ExamplePage.html, the key prefix will be ExamplePage.html
+                    returned: when routing rule present
+                    type: str
+                    sample: docs/
         redirect:
-            replace_key_prefix_with:
+            type: complex
-                description: object key prefix to use in the redirect request
+            description: Container for redirect information.
-                returned: when routing rule present
+            returned: always
-                type: str
+            contains:
-                sample: documents/
+                host_name:
+                    description: name of the host where requests will be redirected.
+                    returned: when host name set as part of redirect rule
+                    type: str
+                    sample: ansible.com
+                http_redirect_code:
+                    description: The HTTP redirect code to use on the response.
+                    returned: when routing rule present
+                    type: str
+                protocol:
+                    description: Protocol to use when redirecting requests.
+                    returned: when routing rule present
+                    type: str
+                    sample: http
+                replace_key_prefix_with:
+                    description: object key prefix to use in the redirect request
+                    returned: when routing rule present
+                    type: str
+                    sample: documents/
+                replace_key_with:
+                    description: object key prefix to use in the redirect request
+                    returned: when routing rule present
+                    type: str
+                    sample: documents/
 '''
 
 import time

test/sanity/ignore.txt

@@ -606,9 +606,6 @@ lib/ansible/modules/cloud/amazon/aws_s3.py validate-modules:undocumented-paramet
 lib/ansible/modules/cloud/amazon/aws_s3.py validate-modules:doc-default-does-not-match-spec
 lib/ansible/modules/cloud/amazon/aws_s3.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/cloud/amazon/aws_s3.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/aws_s3_cors.py future-import-boilerplate
-lib/ansible/modules/cloud/amazon/aws_s3_cors.py metaclass-boilerplate
-lib/ansible/modules/cloud/amazon/aws_s3_cors.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/cloud/amazon/aws_secret.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/cloud/amazon/aws_secret.py validate-modules:doc-missing-type
 lib/ansible/modules/cloud/amazon/aws_ses_identity.py future-import-boilerplate
@@ -1020,23 +1017,6 @@ lib/ansible/modules/cloud/amazon/route53_info.py validate-modules:doc-missing-ty
 lib/ansible/modules/cloud/amazon/route53_zone.py future-import-boilerplate
 lib/ansible/modules/cloud/amazon/route53_zone.py metaclass-boilerplate
 lib/ansible/modules/cloud/amazon/route53_zone.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/s3_bucket.py future-import-boilerplate
-lib/ansible/modules/cloud/amazon/s3_bucket.py metaclass-boilerplate
-lib/ansible/modules/cloud/amazon/s3_bucket.py validate-modules:parameter-type-not-in-doc
-lib/ansible/modules/cloud/amazon/s3_bucket.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:undocumented-parameter
-lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:parameter-type-not-in-doc
-lib/ansible/modules/cloud/amazon/s3_lifecycle.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/s3_sync.py future-import-boilerplate
-lib/ansible/modules/cloud/amazon/s3_sync.py metaclass-boilerplate
-lib/ansible/modules/cloud/amazon/s3_sync.py pylint:blacklisted-name
-lib/ansible/modules/cloud/amazon/s3_sync.py validate-modules:undocumented-parameter
-lib/ansible/modules/cloud/amazon/s3_sync.py validate-modules:doc-choices-do-not-match-spec
-lib/ansible/modules/cloud/amazon/s3_sync.py validate-modules:parameter-type-not-in-doc
-lib/ansible/modules/cloud/amazon/s3_sync.py validate-modules:doc-missing-type
-lib/ansible/modules/cloud/amazon/s3_website.py validate-modules:doc-default-does-not-match-spec
-lib/ansible/modules/cloud/amazon/s3_website.py validate-modules:parameter-type-not-in-doc
-lib/ansible/modules/cloud/amazon/s3_website.py validate-modules:return-syntax-error
 lib/ansible/modules/cloud/amazon/sns.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/cloud/amazon/sns.py validate-modules:doc-missing-type
 lib/ansible/modules/cloud/amazon/sns_topic.py validate-modules:parameter-type-not-in-doc