From 24b8f36f1e174b4b652a2411a23d26ed53779ae2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Moser?= <mail@renemoser.net>
Date: Mon, 5 Feb 2018 22:40:02 +0100
Subject: [PATCH] exo_dns_record: remove limitation for multiple records only
 for A type (#35173)

---
 lib/ansible/module_utils/exoscale.py          | 31 +-------
 .../net_tools/exoscale/exo_dns_record.py      | 77 ++++++++-----------
 .../roles/test_exoscale_dns/tasks/main.yml    | 29 +------
 3 files changed, 37 insertions(+), 100 deletions(-)

diff --git a/lib/ansible/module_utils/exoscale.py b/lib/ansible/module_utils/exoscale.py
index 5a18d591132..abc9f95d79f 100644
--- a/lib/ansible/module_utils/exoscale.py
+++ b/lib/ansible/module_utils/exoscale.py
@@ -1,31 +1,6 @@
 # -*- coding: utf-8 -*-
-#
-# (c) 2016, René Moser <mail@renemoser.net>
-#
-# This code is part of Ansible, but is an independent component.
-# This particular file snippet, and this file snippet only, is BSD licensed.
-# Modules you write using this snippet, which is embedded dynamically by Ansible
-# still belong to the author of the module, and may assign their own license
-# to the complete work.
-#
-# Redistribution and use in source and binary forms, with or without modification,
-# are permitted provided that the following conditions are met:
-#
-#    * Redistributions of source code must retain the above copyright
-#      notice, this list of conditions and the following disclaimer.
-#    * Redistributions in binary form must reproduce the above copyright notice,
-#      this list of conditions and the following disclaimer in the documentation
-#      and/or other materials provided with the distribution.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
-# IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
-# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
-# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
-# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
-# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
-# USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+# Copyright (c) 2016, René Moser <mail@renemoser.net>
+# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
 
 import os
 
@@ -43,7 +18,7 @@ def exo_dns_argument_spec():
         api_secret=dict(default=os.environ.get('CLOUDSTACK_SECRET'), no_log=True),
         api_timeout=dict(type='int', default=os.environ.get('CLOUDSTACK_TIMEOUT') or 10),
         api_region=dict(default=os.environ.get('CLOUDSTACK_REGION') or 'cloudstack'),
-        validate_certs=dict(default='yes', type='bool'),
+        validate_certs=dict(default=True, type='bool'),
     )
 
 
diff --git a/lib/ansible/modules/net_tools/exoscale/exo_dns_record.py b/lib/ansible/modules/net_tools/exoscale/exo_dns_record.py
index 618b0684a33..88d8dd510f8 100644
--- a/lib/ansible/modules/net_tools/exoscale/exo_dns_record.py
+++ b/lib/ansible/modules/net_tools/exoscale/exo_dns_record.py
@@ -34,12 +34,12 @@ options:
     description:
       - Type of the record.
     default: A
-    choices: ['A', 'ALIAS', 'CNAME', 'MX', 'SPF', 'URL', 'TXT', 'NS', 'SRV', 'NAPTR', 'PTR', 'AAAA', 'SSHFP', 'HINFO', 'POOL']
-    aliases: ['rtype', 'type']
+    choices: [ A, ALIAS, CNAME, MX, SPF, URL, TXT, NS, SRV, NAPTR, PTR, AAAA, SSHFP, HINFO, POOL ]
+    aliases: [ rtype, type ]
   content:
     description:
       - Content of the record.
-      - Required if C(state=present) or C(name="")
+      - Required if C(state=present) or C(multiple=yes).
     aliases: ['value', 'address']
   ttl:
     description:
@@ -48,19 +48,19 @@ options:
   prio:
     description:
       - Priority of the record.
-    aliases: ['priority']
+    aliases: [ priority ]
   multiple:
     description:
-      - Whether there are more than one records with similar C(name).
-      - Only allowed with C(record_type=A).
-      - C(content) will not be updated as it is used as key to find the record.
-    aliases: ['priority']
+      - Whether there are more than one records with similar C(name) and C(record_type).
+      - Only allowed for a few record types, e.g. C(record_type=A), C(record_type=NS) or C(record_type=MX).
+      - C(content) will not be updated, instead it is used as a key to find existing records.
+    type: bool
+    default: no
   state:
     description:
       - State of the record.
-    required: false
-    default: 'present'
-    choices: [ 'present', 'absent' ]
+    default: present
+    choices: [ present, absent ]
 extends_documentation_fragment: exoscale
 '''
 
@@ -95,23 +95,25 @@ EXAMPLES = '''
     record_type: CNAME
     content: web-vm-1
 
-- name: Create or update a MX record
+- name: Create another MX record
   local_action:
     module: exo_dns_record
     domain: example.com
     record_type: MX
     content: mx1.example.com
     prio: 10
+    multiple: yes
 
-- name: Delete a MX record
+- name: Delete one MX record out of multiple
   local_action:
     module: exo_dns_record
     domain: example.com
     record_type: MX
     content: mx1.example.com
+    multiple: yes
     state: absent
 
-- name: Remove a record
+- name: Remove a single A record
   local_action:
     module: exo_dns_record
     name: www
@@ -223,12 +225,7 @@ class ExoDnsRecord(ExoDns):
 
         self.multiple = self.module.params.get('multiple')
         self.record_type = self.module.params.get('record_type')
-        if self.multiple and self.record_type != 'A':
-            self.module.fail_json(msg="Multiple is only usable with record_type A")
-
         self.content = self.module.params.get('content')
-        if self.content and self.record_type != 'TXT':
-            self.content = self.content.lower()
 
     def _create_record(self, record):
         self.result['changed'] = True
@@ -265,31 +262,26 @@ class ExoDnsRecord(ExoDns):
         domain = self.module.params.get('domain')
         records = self.api_query("/domains/%s/records" % domain, "GET")
 
-        record = None
+        result = {}
         for r in records:
-            found_record = None
-            if r['record']['record_type'] == self.record_type:
-                r_name = r['record']['name'].lower()
-                r_content = r['record']['content'].lower()
 
-                # there are multiple A records but we found an exact match
-                if self.multiple and self.name == r_name and self.content == r_content:
-                    record = r
-                    break
+            if r['record']['record_type'] != self.record_type:
+                continue
 
-                # We do not expect to found more then one record with that content
-                if not self.multiple and not self.name and self.content == r_content:
-                    found_record = r
+            r_name = r['record']['name'].lower()
+            r_content = r['record']['content']
 
-                # We do not expect to found more then one record with that name
-                elif not self.multiple and self.name and self.name == r_name:
-                    found_record = r
+            if r_name == self.name:
+                if not self.multiple:
+                    if result:
+                        self.module.fail_json(msg="More than one record with record_type=%s and name=%s params. "
+                                                  "Use multiple=yes for more than one record." % (self.record_type, self.name))
+                    else:
+                        result = r
+                elif r_content == self.content:
+                    return r
 
-                if record and found_record:
-                    self.module.fail_json(msg="More than one record with your params. Use multiple=yes for more than one A record.")
-                if found_record:
-                    record = found_record
-        return record
+        return result
 
     def present_record(self):
         record = self.get_record()
@@ -332,11 +324,8 @@ def main():
         argument_spec=argument_spec,
         required_together=exo_dns_required_together(),
         required_if=[
-            ['state', 'present', ['content']],
-            ['name', '', ['content']],
-        ],
-        required_one_of=[
-            ['content', 'name'],
+            ('state', 'present', ['content']),
+            ('multiple', True, ['content']),
         ],
         supports_check_mode=True,
     )
diff --git a/test/legacy/roles/test_exoscale_dns/tasks/main.yml b/test/legacy/roles/test_exoscale_dns/tasks/main.yml
index 6bfe0227b5f..bf78ebad396 100644
--- a/test/legacy/roles/test_exoscale_dns/tasks/main.yml
+++ b/test/legacy/roles/test_exoscale_dns/tasks/main.yml
@@ -54,19 +54,6 @@
     - result is failed
     - 'result.msg == "missing required arguments: domain"'
 
-- name: test fail if missing required params
-  local_action:
-    module: exo_dns_record
-    domain: "{{ exo_dns_domain_name }}"
-    state: absent
-  register: result
-  ignore_errors: true
-- name: verify results of test fail if missing required params
-  assert:
-    that:
-    - result is failed
-    - 'result.msg == "name is  but the following are missing: content"'
-
 - name: test fail if missing required params state=present
   local_action:
     module: exo_dns_record
@@ -78,21 +65,7 @@
   assert:
     that:
     - result is failed
-    - 'result.msg == "state is present but the following are missing: content"'
-
-- name: test fail if missing required params state=absent
-  local_action:
-    module: exo_dns_record
-    domain: "{{ exo_dns_domain_name }}"
-    name: ""
-    state: absent
-  register: result
-  ignore_errors: true
-- name: verify results of test fail if missing required params state=absent
-  assert:
-    that:
-    - result is failed
-    - 'result.msg == "name is  but the following are missing: content"'
+    - 'result.msg == "state is present but all of the following are missing: content"'
 
 - name: test create a record
   local_action: