From 3f0c47196eb7be6e12a641e929431e72b8a2dc49 Mon Sep 17 00:00:00 2001
From: Sloane Hertel <shertel@redhat.com>
Date: Mon, 12 Jun 2017 13:52:25 -0400
Subject: [PATCH] [cloud] s3: deleting a nonexistent bucket should not give a
 traceback Fixes #25445 (#25487)

* trying to delete a nonexistent bucket should not fail

* Improve error handling for deleting s3 bucket

* Allow successful deletion

* Add test for deleting a nonexistent bucket

rename integration test target from s3 to aws_s3
---
 lib/ansible/modules/cloud/amazon/s3.py        | 27 ++++++++++++++++---
 .../targets/{s3 => aws_s3}/aliases            |  0
 .../targets/{s3 => aws_s3}/defaults/main.yml  |  0
 .../targets/{s3 => aws_s3}/meta/main.yml      |  0
 .../targets/{s3 => aws_s3}/tasks/main.yml     | 13 +++++++++
 5 files changed, 36 insertions(+), 4 deletions(-)
 rename test/integration/targets/{s3 => aws_s3}/aliases (100%)
 rename test/integration/targets/{s3 => aws_s3}/defaults/main.yml (100%)
 rename test/integration/targets/{s3 => aws_s3}/meta/main.yml (100%)
 rename test/integration/targets/{s3 => aws_s3}/tasks/main.yml (94%)

diff --git a/lib/ansible/modules/cloud/amazon/s3.py b/lib/ansible/modules/cloud/amazon/s3.py
index 42be44740bb..03006d87873 100644
--- a/lib/ansible/modules/cloud/amazon/s3.py
+++ b/lib/ansible/modules/cloud/amazon/s3.py
@@ -350,13 +350,32 @@ def delete_bucket(module, s3, bucket):
     if module.check_mode:
         module.exit_json(msg="DELETE operation skipped - running in check mode", changed=True)
     try:
-        bucket = s3.lookup(bucket)
+        bucket = s3.lookup(bucket, validate=False)
         bucket_contents = bucket.list()
         bucket.delete_keys([key.name for key in bucket_contents])
+    except s3.provider.storage_response_error as e:
+        if e.status == 404:
+            # bucket doesn't appear to exist
+            return False
+        elif e.status == 403:
+            # bucket appears to exist but user doesn't have list bucket permission; may still be able to delete bucket
+            pass
+        else:
+            module.fail_json(msg=str(e), exception=traceback.format_exc())
+    try:
         bucket.delete()
         return True
     except s3.provider.storage_response_error as e:
-        module.fail_json(msg= str(e))
+        if e.status == 403:
+            module.exit_json(msg="Unable to complete DELETE operation. Check you have have s3:DeleteBucket "
+                             "permission. Error: {0}.".format(e.message),
+                             exception=traceback.format_exc())
+        elif e.status == 409:
+            module.exit_json(msg="Unable to complete DELETE operation. It appears there are contents in the "
+                             "bucket that you don't have permission to delete. Error: {0}.".format(e.message),
+                             exception=traceback.format_exc())
+        else:
+            module.fail_json(msg=str(e), exception=traceback.format_exc())
 
 def delete_key(module, s3, bucket, obj, validate=True):
     if module.check_mode:
@@ -676,8 +695,8 @@ def main():
     if mode == 'delete':
         if bucket:
             deletertn = delete_bucket(module, s3, bucket)
-            if deletertn is True:
-                module.exit_json(msg="Bucket %s and all keys have been deleted."%bucket, changed=True)
+            message = "Bucket {0} and all keys have been deleted.".format(bucket)
+            module.exit_json(msg=message, changed=deletertn)
         else:
             module.fail_json(msg="Bucket parameter is required.")
 
diff --git a/test/integration/targets/s3/aliases b/test/integration/targets/aws_s3/aliases
similarity index 100%
rename from test/integration/targets/s3/aliases
rename to test/integration/targets/aws_s3/aliases
diff --git a/test/integration/targets/s3/defaults/main.yml b/test/integration/targets/aws_s3/defaults/main.yml
similarity index 100%
rename from test/integration/targets/s3/defaults/main.yml
rename to test/integration/targets/aws_s3/defaults/main.yml
diff --git a/test/integration/targets/s3/meta/main.yml b/test/integration/targets/aws_s3/meta/main.yml
similarity index 100%
rename from test/integration/targets/s3/meta/main.yml
rename to test/integration/targets/aws_s3/meta/main.yml
diff --git a/test/integration/targets/s3/tasks/main.yml b/test/integration/targets/aws_s3/tasks/main.yml
similarity index 94%
rename from test/integration/targets/s3/tasks/main.yml
rename to test/integration/targets/aws_s3/tasks/main.yml
index 2e6750cecdf..8a9d6fe75cc 100644
--- a/test/integration/targets/s3/tasks/main.yml
+++ b/test/integration/targets/aws_s3/tasks/main.yml
@@ -215,3 +215,16 @@
     that:
       - result.changed == True
 # ============================================================
+- name: test delete a nonexistent bucket
+  s3:
+    bucket: "{{ bucket.stdout + '.bucket' }}"
+    mode: delete
+    security_token: "{{security_token}}"
+    aws_access_key: "{{ ec2_access_key }}"
+    aws_secret_key: "{{ ec2_secret_key }}"
+  register: result
+- name: assert that changed is False
+  assert:
+    that:
+      - result.changed == False
+# ============================================================