-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/20/2011 11:53 PM, Simo Sorce wrote:
> On Thu, 20 Jan 2011 17:27:37 -0500
> Dmitri Pal <d...@redhat.com> wrote:
> 
>> Michael Gregg wrote:
>>> Jakub Hrozek wrote:
>>> Hi,
>>>
>>> as discussed in https://bugzilla.redhat.com/show_bug.cgi?id=671019
>>> to delete a DNS RR one has to remove its record types one by one.
>>>
>>> This patch modifies the behaviour so that if the user runs
>>> dnsrecord-del <zone> <record-name> with no other parameters, the
>>> whole record is removed.
>>>
>>> Alternative solutions might be to expose the internal command that
>>> is able to delete the record (although I think it is
>>> counterintuitive to have one command to remove record types and one
>>> for the whole record) or have a special flag (--del-all?) to remove
>>> the whole record.
>>>
>>> The patch also fixes the unit tests as they didn't reflect all the
>>> recent changes.
>>
>>> Going with this patch sounds good, but to make sure, I polled
>>> several
>> people here, and they all seemed to think that having to add a
>> --del-all or --del-record flag at the end would be better as it would
>> be less prone to failure where admins would accidentally delete a
>> entire record because they didn't specify anything after the "<zone>
>> <record>"
>>
>>> So, maybe we do need a --del-all or --del-record operator.
>>
>> Agree.
> 
> +1
> Someone may simply push enter accidentally while checking what to write
> after the command. It would be rather unfortunate.
> 
> Simo.
> 
> 

Attached is a new version of the patch that implements --del-all. It
also reports failure when deleting a nonexistent RR (new ticket 829).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk05oPgACgkQHsardTLnvCUqNwCfQ1BOTgx7L2lVcG5a7a6oP8lW
sX4AoIDw5xVidcKblzWueO5OwlzkZ6kZ
=YkBR
-----END PGP SIGNATURE-----
From b34a673c3015dd562a96d7f30973fa10a91b7d8d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 20 Jan 2011 07:54:14 -0500
Subject: [PATCH] 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
---
 ipalib/plugins/dns.py                |   22 +++++++++++++++++++++-
 tests/test_xmlrpc/test_dns_plugin.py |   32 ++++++++++++++++++--------------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 5b7eb55..900db0e 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -380,6 +380,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)
 
 
@@ -518,6 +525,18 @@ class dnsrecord_del(dnsrecord_mod_record):
     """
     Delete DNS resource record.
     """
+    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 update_old_entry_callback(self, entry_attrs, old_entry_attrs):
         for (a, v) in entry_attrs.iteritems():
             if not isinstance(v, (list, tuple)):
@@ -526,7 +545,8 @@ 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):
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 5fb08fd..6282e95 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -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],
@@ -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'],
-                }
+                'result': {'failed': u''},
             },
         ),
 
@@ -347,7 +351,7 @@ class test_dns(Declarative):
             expected={
                 'value': dnszone1,
                 'summary': None,
-                'result': True,
+                'result': {'failed': u''},
             },
         ),
 
-- 
1.7.3.4

Attachment: jhrozek-freeipa-039-02-dns-record-del.patch.sig
Description: PGP signature

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to