From 389db4c49e27708a1e7a05ed42b8fb9f97aaef67 Mon Sep 17 00:00:00 2001
From: Maish <maishsk@users.noreply.github.com>
Date: Thu, 3 May 2018 15:03:11 +0300
Subject: [PATCH] [aws] Added empty default for ec2_ami name parameter (#38585)

* removed additional check for name parameter

(cherry picked from commit 91357d07f4f9eb3714f582c1959ff54d25129500)

* Added empty default
Fix for issue https://github.com/ansible/ansible/issues/38482

(cherry picked from commit 60a32771039c190a7e9f6e0eb4d0f6bf4c356fd4)

* [ec2_ami] Ensure name or image_id is provided for state=present (#38972)

Add integration tests for backward compatibility and ensuring name or image_id is provided
(cherry picked from commit e2aa1155ba5889296d2f018fdf8284c33f5f4918)
---
 lib/ansible/modules/cloud/amazon/ec2_ami.py   |  6 ++++-
 .../targets/ec2_ami/tasks/main.yml            | 25 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/ansible/modules/cloud/amazon/ec2_ami.py b/lib/ansible/modules/cloud/amazon/ec2_ami.py
index 2f37a8dfd44..c37e2e68e34 100644
--- a/lib/ansible/modules/cloud/amazon/ec2_ami.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_ami.py
@@ -677,10 +677,14 @@ def main():
         argument_spec=argument_spec,
         required_if=[
             ['state', 'absent', ['image_id']],
-            ['state', 'present', ['name']],
         ]
     )
 
+    # Using a required_one_of=[['name', 'image_id']] overrides the message that should be provided by
+    # the required_if for state=absent, so check manually instead
+    if not any([module.params['image_id'], module.params['name']]):
+        module.fail_json(msg="one of the following is required: name, image_id")
+
     try:
         region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True)
         connection = boto3_conn(module, conn_type='client', resource='ec2', region=region, endpoint=ec2_url, **aws_connect_kwargs)
diff --git a/test/integration/targets/ec2_ami/tasks/main.yml b/test/integration/targets/ec2_ami/tasks/main.yml
index fb6e121431f..ec8b5126731 100644
--- a/test/integration/targets/ec2_ami/tasks/main.yml
+++ b/test/integration/targets/ec2_ami/tasks/main.yml
@@ -87,6 +87,30 @@
 
     # ============================================================
 
+    - name: test clean failure if not providing image_id or name with state=present
+      ec2_ami:
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        state: present
+        description: '{{ ec2_ami_description }}'
+        tags:
+          Name: '{{ ec2_ami_name }}_ami'
+        wait: yes
+        root_device_name: /dev/xvda
+      register: result
+      ignore_errors: yes
+
+    - name: assert error message is helpful
+      assert:
+        that:
+          - result.failed
+          - "result.msg == 'one of the following is required: name, image_id'"
+
+    # ============================================================
+
     - name: create an image from the instance
       ec2_ami:
         ec2_region: '{{ec2_region}}'
@@ -295,7 +319,6 @@
         security_token: '{{security_token}}'
         state: present
         image_id: '{{ result.image_id }}'
-        name: '{{ ec2_ami_name }}_ami'
         description: '{{ ec2_ami_description }}'
         tags:
           Name: '{{ ec2_ami_name }}_ami'