[Freeipa-devel] [PATCH] [WIP] 172+173+175 Create per-type DNS API

[Rob Crittenden]

Martin Kosek wrote:
> On Thu, 2011-12-01 at 17:18 -0500, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Mon, 2011-11-28 at 17:35 +0100, Martin Kosek wrote:
>>>> I have prepared a working prototype of the new structured DNS API. It
>>>> may still have rough edges (and unit tests are not ready), but it will
>>>> provide a base for discussion and for WebUI folks - so that they can
>>>> start development of the new DNS WebUI API.
>>>>
>>>> The patch takes advantage of the DNS refactor I did in 172. For all
>>>> supported non-DNSSEC RR types, the following commands are available:
>>>>
>>>> dnsrecord<RRTYPE>-show ZONE NAME
>>>> dnsrecord<RRTYPE>-add ZONE NAME
>>>> dnsrecord<RRTYPE>-mod ZONE NAME VALUE
>>>>
>>>> This is an example of the new API in action:
>>>>
>>>> # ipa dnsrecord-show example.com foo
>>>>     Record name: foo
>>>>     A record: 10.0.0.1
>>>>
>>>> # ipa dnsrecordmx-add example.com foo --exchanger="foo.example.com."
>>>>     MX record: 0 foo.example.com.
>>>>     Preference: 0
>>>>     Exchanger: foo.example.com.
>>>> ----------------------------
>>>> Number of entries returned 1
>>>> ----------------------------
>>>>
>>>> # ipa dnsrecordmx-add example.com foo --preference=1 --exchanger="foo.example.com."
>>>>     MX record: 0 foo.example.com.
>>>>     Preference: 0
>>>>     Exchanger: foo.example.com.
>>>>
>>>>     MX record: 1 foo.example.com.
>>>>     Preference: 1
>>>>     Exchanger: foo.example.com.
>>>> ----------------------------
>>>> Number of entries returned 2
>>>> ----------------------------
>>>>
>>>> # ipa dnsrecordmx-show example.com foo
>>>>     MX record: 0 foo.example.com.
>>>>     Preference: 0
>>>>     Exchanger: foo.example.com.
>>>>
>>>>     MX record: 1 foo.example.com.
>>>>     Preference: 1
>>>>     Exchanger: foo.example.com.
>>>> ----------------------------
>>>> Number of entries returned 2
>>>> ----------------------------
>>>>
>>>>
>>>> There is an interactive wizard to help user modify a record without
>>>> specifying an updated value first. If there is just one (MX) record, no
>>>> wizard would be run.
>>>>
>>>> # ipa dnsrecordmx-mod example.com foo --preference=2
>>>> Which MX record would you like to modify?
>>>>
>>>> [1]: 0 foo.example.com.
>>>> [2]: 1 foo.example.com.
>>>>
>>>> DNS record number: 2
>>>>     MX record: 0 foo.example.com.
>>>>     Preference: 0
>>>>     Exchanger: foo.example.com.
>>>>
>>>>     MX record: 2 foo.example.com.
>>>>     Preference: 2
>>>>     Exchanger: foo.example.com.
>>>> ----------------------------
>>>> Number of entries returned 2
>>>> ----------------------------
>>>>
>>>> # ipa dnsrecordmx-mod example.com foo "2 foo.example.com." --preference=3
>>>>     MX record: 0 foo.example.com.
>>>>     Preference: 0
>>>>     Exchanger: foo.example.com.
>>>>
>>>>     MX record: 3 foo.example.com.
>>>>     Preference: 3
>>>>     Exchanger: foo.example.com.
>>>> ----------------------------
>>>> Number of entries returned 2
>>>> ----------------------------
>>>>
>>>>
>>>> There are few open questions I am still thinking about:
>>>>
>>>> 1) The commands return a list of structured records (just like *-find
>>>> commands) instead of returning just one record. I thought that it may be
>>>> more usable this way and consistent with dnsrecord-add/mod/show commands
>>>> behavior which returns all records too. Otherwise, we would have to
>>>> change the show command API and add VALUE argument, which would specify
>>>> a value to be displayed:
>>>> dnsrecord<RRTYPE>-show ZONE NAME VALUE
>>>>
>>>> 2) Raw DNS record value is in the output too. I though it would be
>>>> useful to see the raw DNS record value + its parts at one place.
>>>>
>>>> 3) The commands are in format dnsrecord<RRTYPE>-cmd, for example
>>>> dnsrecordmx-add. I think dnsrecord-mx-add may be more readable. If we
>>>> want to go this way, I would have to bend the server framework a little
>>>> which parses an LDAP object from the command name (LDAP object name is
>>>> dnsrecordmx in this case). This is doable, although I am not sure if
>>>> this does not have some implications in WebUI side.
>>>>
>>>> Martin
>>>
>>> I rebased both patches to the most recent master. Adding CSVs now works
>>> ok again (with the fix in 175):
>>>
>>> # ipa dnsrecord-mod example.com foo --a-rec=10.0.0.1,10.0.0.2
>>>     Record name: foo
>>>     A record: 10.0.0.1, 10.0.0.2
>>>
>>> Martin
>>
>
> Rob, thank you for the review! What do you think about the 3 open
> questions I posted above?
>
>> I think some abbreviations can be eliminated:
>>
>> siz ->  size
>> alt ->  altitude
>
> Sure, this can be fixed.
>
>>
>> For MX record (and probably KX) you can make preference required. It
>> should fill in without prompting since it has a default. This way it
>> will show as required in the UI.
>
> Right, we don't want to run into issues we had with user fields.
>
>>
>> PTRRecord doc I think would read better as "The hostname this reverse
>> record points to"
>
> Ok. Btw do you think it would be good to document this way all these new
> DNSRecord part parameters? As I checked with Petr, these would be shown
> in the UI - so the UI user would benefit from it. But it will require a
> lot of writing and RFC study :-)
>
>>
>> I'm not sure I follow the makeapi change. I see the new entry types in
>> API.txt but this makeapi seems to suggest the DNS api is not checked.
>
> This fix is in validate_doc(), which tries to check that all our
> commands have proper __doc__ string. It fails with the new DNS API
> commands (dnsrecordmx-add etc.) because it cannot find class definitions
> in dns.py. This is expected as I generate all these LDAP command classes
> on the fly from new DNSRecord parameters.
>
> Martin
>
>>
>> Otherwise generally looks good.
>>
>> rob
>
>

Build not working:

./make-lint
ipalib/parameters.py:718: [E1101, Param.normalize] Generator 
'__unicode_csv_reader' has no 'next' member

I found this works ok and adding records is definitely clearer but it 
seems odd to add with one command and delete/find with another. I could 
get used to it I suppose.

Help per command doesn't work:

$ ipa help dnsrecorda
ipa: ERROR: no command nor help topic 'dnsrecorda'

Not sure if we want specific help or simply point back to dnsrecord 
which might be a bit overwhelming.

$ ipa dnsrecordloc-add --help

This is is a significant improvement!

rob