Delete the whole DNS record with no parameters

Error out when deleting a nonexistent DNS record

Also fixes the DNS unit tests.

https://fedorahosted.org/freeipa/ticket/816
https://fedorahosted.org/freeipa/ticket/829

3 files changed, 71 insertions, 22 deletions

diff --git a/API.txt b/API.txt
index 703978f57..fd894a5db 100644
--- a/API.txt
+++ b/API.txt
@@ -580,9 +580,10 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), 'User-friendly
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('value', <type 'unicode'>, "The primary_key value of the entry, e.g. 'jdoe' for a user")
 command: dnsrecord_del
-args: 2,41,3
+args: 2,42,3
 arg: Str('dnszoneidnsname', cli_name='dnszone', label=Gettext('Zone name', domain='ipa', localedir=None), query=True, required=True)
 arg: Str('idnsname', attribute=True, cli_name='name', label=Gettext('Record name', domain='ipa', localedir=None), multivalue=False, primary_key=True, query=True, required=True)
+option: Flag('del_all', autofill=True, default=False, label=Gettext('Delete all associated records', domain='ipa', localedir=None))
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui', flags=['no_output'])
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui', flags=['no_output'])
 option: Str('version?', exclude='webui', flags=['no_option', 'no_output'])
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index af2ba4dd6..b4f56d4b1 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -120,6 +120,13 @@ _record_validators = {
     u'APL': _validate_ipnet,
 }
 
+def has_cli_options(entry, no_option_msg):
+    entry = dict((t, entry.get(t, [])) for t in _record_attributes)
+    numattr = reduce(lambda x,y: x+y,
+                     map(lambda x: len(x), entry.values()))
+    if numattr == 0:
+        raise errors.OptionError(no_option_msg)
+    return entry
 
 def dns_container_exists(ldap):
     try:
@@ -380,6 +387,13 @@ class dnsrecord(LDAPObject):
             return self.api.Object[self.parent_object].get_dn(*keys[:-1], **options)
         return super(dnsrecord, self).get_dn(*keys, **options)
 
+    def attr_to_cli(self, attr):
+        try:
+            cliname = attr[:-len('record')].upper()
+        except IndexError:
+            cliname = attr
+        return cliname
+
 api.register(dnsrecord)
 
 
@@ -407,7 +421,9 @@ class dnsrecord_cmd_w_record_options(Command):
                 )
 
     def record_options_2_entry(self, **options):
-        return dict((t, options.get(t, [])) for t in _record_attributes)
+        entries = dict((t, options.get(t, [])) for t in _record_attributes)
+        entries.update(dict((k, []) for (k,v) in entries.iteritems() if v == None ))
+        return entries
 
 
 class dnsrecord_mod_record(LDAPQuery, dnsrecord_cmd_w_record_options):
@@ -456,7 +472,9 @@ class dnsrecord_mod_record(LDAPQuery, dnsrecord_cmd_w_record_options):
         if self.obj.is_pkey_zone_record(*keys):
             entry_attrs[self.obj.primary_key.name] = [u'@']
 
-        self.post_callback(keys, entry_attrs)
+        retval = self.post_callback(keys, entry_attrs)
+        if retval:
+            return retval
 
         return dict(result=entry_attrs, value=keys[-1])
 
@@ -487,12 +505,18 @@ class dnsrecord_add(LDAPCreate, dnsrecord_cmd_w_record_options):
     """
     Add new DNS resource record.
     """
+    no_option_msg = 'No options to add a specific record provided.'
+
     def get_options(self):
         for option in super(dnsrecord_add, self).get_options():
             yield option
         for option in self.get_record_options():
             yield option
 
+    def args_options_2_entry(self, *keys, **options):
+        has_cli_options(options, self.no_option_msg)
+        return super(dnsrecord_add, self).args_options_2_entry(*keys, **options)
+
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
         if call_func.func_name == 'add_entry':
             if isinstance(exc, errors.DuplicateEntry):
@@ -509,6 +533,7 @@ class dnsrecord_delentry(LDAPDelete):
     """
     Delete DNS record entry.
     """
+    msg_summary = _('Deleted record "%(value)s"')
     NO_CLI = True
 
 api.register(dnsrecord_delentry)
@@ -518,6 +543,24 @@ class dnsrecord_del(dnsrecord_mod_record):
     """
     Delete DNS resource record.
     """
+    no_option_msg = 'Neither --del-all nor options to delete a specific record provided.'
+    takes_options = (
+            Flag('del_all',
+                default=False,
+                label=_('Delete all associated records'),
+            ),
+    )
+
+    def execute(self, *keys, **options):
+        if options.get('del_all', False):
+            return self.obj.methods.delentry(*keys)
+
+        return super(dnsrecord_del, self).execute(*keys, **options)
+
+    def record_options_2_entry(self, **options):
+        entry = super(dnsrecord_del, self).record_options_2_entry(**options)
+        return has_cli_options(entry, self.no_option_msg)
+
     def update_old_entry_callback(self, entry_attrs, old_entry_attrs):
         for (a, v) in entry_attrs.iteritems():
             if not isinstance(v, (list, tuple)):
@@ -526,14 +569,15 @@ class dnsrecord_del(dnsrecord_mod_record):
                 try:
                     old_entry_attrs[a].remove(val)
                 except (KeyError, ValueError):
-                    pass
+                    raise errors.NotFound(reason='%s record with value %s not found' %
+                                          (self.obj.attr_to_cli(a), val))
 
     def post_callback(self, keys, entry_attrs):
         if not self.obj.is_pkey_zone_record(*keys):
             for a in _record_attributes:
                 if a in entry_attrs and entry_attrs[a]:
                     return
-            self.obj.methods.delentry(*keys)
+            return self.obj.methods.delentry(*keys)
 
 api.register(dnsrecord_del)
 
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 5fb08fdf5..0be29ff86 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -45,7 +45,7 @@ class test_dns(Declarative):
 
     cleanup_commands = [
         ('dnszone_del', [dnszone1], {}),
-        ('dnsrecord_del', [dnszone1, dnsres1], {}),
+        ('dnsrecord_del', [dnszone1, dnsres1], {'del_all' : True}),
     ]
 
     tests = [
@@ -86,7 +86,8 @@ class test_dns(Declarative):
                     'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'TRUE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [fuzzy_digits],
@@ -122,7 +123,8 @@ class test_dns(Declarative):
                     'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'TRUE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [fuzzy_digits],
@@ -143,7 +145,8 @@ class test_dns(Declarative):
                 'result': {
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'TRUE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [u'5478'],
@@ -157,8 +160,8 @@ class test_dns(Declarative):
 
 
         dict(
-            desc='Search for zones with name server %r' % (u'ns1.%s' % dnszone1),
-            command=('dnszone_find', [], {'idnssoamname': u'ns1.%s' % dnszone1}),
+            desc='Search for zones with name server %r' % (u'ns1.%s.' % dnszone1),
+            command=('dnszone_find', [], {'idnssoamname': u'ns1.%s.' % dnszone1}),
             expected={
                 'summary': None,
                 'count': 1,
@@ -167,7 +170,8 @@ class test_dns(Declarative):
                     'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'TRUE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [u'5478'],
@@ -200,7 +204,8 @@ class test_dns(Declarative):
                     'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'FALSE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [fuzzy_digits],
@@ -233,7 +238,8 @@ class test_dns(Declarative):
                     'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
                     'idnsname': [dnszone1],
                     'idnszoneactive': [u'TRUE'],
-                    'idnssoamname': [u'ns1.%s' % dnszone1],
+                    'nsrecord': [u'ns1.%s.' % dnszone1],
+                    'idnssoamname': [u'ns1.%s.' % dnszone1],
                     'idnssoarname': [u'root.%s.' % dnszone1],
                     'idnssoaserial': [fuzzy_digits],
                     'idnssoarefresh': [fuzzy_digits],
@@ -254,7 +260,7 @@ class test_dns(Declarative):
 
         dict(
             desc='Try to delete non-existent record %r in zone %r' % (dnsres1, dnszone1),
-            command=('dnsrecord_del', [dnszone1, dnsres1], {}),
+            command=('dnsrecord_del', [dnszone1, dnsres1], {'del_all' : True}),
             expected=errors.NotFound(reason='DNS resource record not found'),
         ),
 
@@ -285,6 +291,7 @@ class test_dns(Declarative):
                 'result': [
                     {
                         'dn': u'idnsname=%s,cn=dns,%s' % (dnszone1, api.env.basedn),
+                        'nsrecord': (u'ns1.dnszone.test.',),
                         'idnsname': [u'@'],
                     },
                     {
@@ -329,14 +336,11 @@ class test_dns(Declarative):
 
         dict(
             desc='Delete record %r in zone %r' % (dnsres1, dnszone1),
-            command=('dnsrecord_del', [dnszone1, dnsres1], {}),
+            command=('dnsrecord_del', [dnszone1, dnsres1], {'del_all': True }),
             expected={
                 'value': dnsres1,
-                'summary': None,
-                'result': {
-                    'idnsname': [dnsres1],
-                    'arecord': [u'10.10.0.1'],
-                }
+                'summary': u'Deleted record "%s"' % dnsres1,
+                'result': {'failed': u''},
             },
         ),
 
@@ -347,7 +351,7 @@ class test_dns(Declarative):
             expected={
                 'value': dnszone1,
                 'summary': None,
-                'result': True,
+                'result': {'failed': u''},
             },
         ),