s3: raise an error if the bucket name is not correct (ansible-collect…

…ions#341)

s3: raise an error if the bucket name is not correct

Reviewed-by: https://github.com/apps/ansible-zuul

plugins/module_utils/s3.py

@@ -19,6 +19,9 @@
         HAS_MD5 = False
 
 
+import string
+
+
 def calculate_etag(module, filename, etag, s3, bucket, obj, version=None):
     if not HAS_MD5:
         return None
@@ -81,3 +84,19 @@ def calculate_etag_content(module, content, etag, s3, bucket, obj, version=None)
         return '"{0}-{1}"'.format(digest_squared.hexdigest(), len(digests))
     else:  # Compute the MD5 sum normally
         return '"{0}"'.format(md5(content).hexdigest())
+
+
+def validate_bucket_name(module, name):
+    # See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
+    if len(name) < 4:
+        module.fail_json(msg='the S3 bucket name is too short')
+    if len(name) > 63:
+        module.fail_json(msg='the length of an S3 bucket cannot exceed 63 characters')
+
+    legal_characters = string.ascii_lowercase + ".-" + string.digits
+    illegal_characters = [c for c in name if c not in legal_characters]
+    if illegal_characters:
+        module.fail_json(msg='invalid character(s) found in the bucket name')
+    if name[-1] not in string.ascii_lowercase + string.digits:
+        module.fail_json(msg='bucket names must begin and end with a letter or number')
+    return True

plugins/modules/aws_s3.py

@@ -323,6 +323,7 @@
 from ..module_utils.s3 import HAS_MD5
 from ..module_utils.s3 import calculate_etag
 from ..module_utils.s3 import calculate_etag_content
+from ..module_utils.s3 import validate_bucket_name
 
 IGNORE_S3_DROP_IN_EXCEPTIONS = ['XNotImplemented', 'NotImplemented']
 
@@ -731,6 +732,8 @@ def main():
     object_canned_acl = ["private", "public-read", "public-read-write", "aws-exec-read", "authenticated-read", "bucket-owner-read", "bucket-owner-full-control"]
     bucket_canned_acl = ["private", "public-read", "public-read-write", "authenticated-read"]
 
+    validate_bucket_name(module, bucket)
+
     if overwrite not in ['always', 'never', 'different']:
         if module.boolean(overwrite):
             overwrite = 'always'

plugins/modules/s3_bucket.py

@@ -258,6 +258,7 @@
 from ..module_utils.ec2 import compare_policies
 from ..module_utils.ec2 import get_aws_connection_info
 from ..module_utils.ec2 import snake_dict_to_camel_dict
+from ..module_utils.s3 import validate_bucket_name
 
 
 def create_or_update_bucket(s3_client, module, location):
@@ -912,6 +913,7 @@ def main():
     )
 
     region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True)
+    validate_bucket_name(module, module.params["name"])
 
     if region in ('us-east-1', '', None):
         # default to US Standard region

tests/integration/targets/aws_s3/tasks/main.yml

@@ -30,6 +30,17 @@
           - result is failed
           - "result.msg != 'MODULE FAILURE'"
 
+    - name: test create bucket with an invalid name
+      aws_s3:
+        bucket: "{{ bucket_name }}-"
+        mode: create
+      register: result
+      ignore_errors: yes
+
+    - assert:
+        that:
+          - result is failed
+
     - name: test create bucket
       aws_s3:
         bucket: "{{ bucket_name }}"
@@ -658,6 +669,6 @@
 
     - name: delete the dot bucket
       aws_s3:
-        bucket: "{{ bucket_name + '.bucket' }}"
+        bucket: "{{ bucket_name | hash('md5') + '.bucket' }}"
         mode: delete
       ignore_errors: yes

tests/unit/module_utils/test_s3.py

@@ -0,0 +1,40 @@
+#
+# (c) 2021 Red Hat Inc.
+#
+# This file is part of Ansible
+# 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
+
+from ansible_collections.amazon.aws.tests.unit.compat.mock import MagicMock
+import ansible_collections.amazon.aws.plugins.module_utils.s3 as s3
+
+
+def test_validate_bucket_name():
+    module = MagicMock()
+
+    assert s3.validate_bucket_name(module, "docexamplebucket1") is True
+    assert not module.fail_json.called
+    assert s3.validate_bucket_name(module, "log-delivery-march-2020") is True
+    assert not module.fail_json.called
+    assert s3.validate_bucket_name(module, "my-hosted-content") is True
+    assert not module.fail_json.called
+
+    assert s3.validate_bucket_name(module, "docexamplewebsite.com") is True
+    assert not module.fail_json.called
+    assert s3.validate_bucket_name(module, "www.docexamplewebsite.com") is True
+    assert not module.fail_json.called
+    assert s3.validate_bucket_name(module, "my.example.s3.bucket") is True
+    assert not module.fail_json.called
+
+    module.fail_json.reset_mock()
+    s3.validate_bucket_name(module, "doc_example_bucket")
+    assert module.fail_json.called
+
+    module.fail_json.reset_mock()
+    s3.validate_bucket_name(module, "DocExampleBucket")
+    assert module.fail_json.called
+    module.fail_json.reset_mock()
+    s3.validate_bucket_name(module, "doc-example-bucket-")
+    assert module.fail_json.called