[Freeipa-devel] [PATCH] 039 Delete the whole DNS record with no parameters

[Jakub Hrozek]

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

On 01/21/2011 05:54 PM, Rob Crittenden wrote:
> Jakub Hrozek wrote:
>> -----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<dpal at 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).
> 
> nack, this isn't working properly for me.
> 
> Here is how I tested:
> 
> - add a new zone, newzone1
> - ipa dnsrecord-add newzone1 as --a-rec 3.4.5.6
> - ipa dnsrecord-add newzone1 as
>   Record name: as
>   A record: 3.4.5.6
> - ipa dnsrecord-show newzone1 as
>   Record name: as
>   A record: 3.4.5.6
> - ipa dnsrecord-del newzone1 as --del-all
> [ no output ]
> - ipa dnsrecord-show newzone1 as
> ipa: ERROR: as: DNS resource record not found
> 
> So a couple of problems:
> 
> 1. An error should have been thrown when I tried a delete without a
> specific record type.

I agree but I was reluctant to do this because it was perfectly OK to
call "dnsrecord-add" with no options. That would create an empty DNS
record. The interface was orthogonal so "dnsrecord-del" with no options
would remove the record if it was empty. But I don't think an empty DNS
record makes any sense.

I changed the behaviour such that:
* dnsrecord-add with no attributes is no longer allowed. You have to
specify at least one RR type.
* dnsrecord-del with no attributes is no longer allowed. You have to
either specify a RR type or --del-all.

> 2. Some output should be displayed when I delete all records, at least a
> summary.
> 

Agreed and fixed.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk09kfIACgkQHsardTLnvCWyBgCeIos2bWGps/FxL7of6BkuiU8U
AzEAn1Bp/uuoNKB2Qlm2XGGwdDL4dAjl
=I13I
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jhrozek-freeipa-039-03-dns-record-del.patch
Type: text/x-patch
Size: 12557 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110124/94fd07cc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jhrozek-freeipa-039-03-dns-record-del.patch.sig
Type: application/pgp-signature
Size: 72 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110124/94fd07cc/attachment.sig>