[Freeipa-devel] [PATCH] [WIP] 172+173+175 Create per-type DNS API
Jan Cholasta
jcholast at redhat.com
Wed Dec 14 10:36:40 UTC 2011
Dne 14.12.2011 07:53, Martin Kosek napsal(a):
> On Fri, 2011-12-09 at 15:55 -0500, Rob Crittenden wrote:
>> 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 must be using older version of pylint - it didn't occurred on my box.
> I will add a pylint hint to ignore this one.
Regarding patch 175, I think a fix like this would be somewhat nicer:
def normalize(self, value):
...
if self.multivalue:
if type(value) not in (tuple, list):
value = (value,)
if self.csv:
newval = ()
for v in value:
if isinstance(v, basestring):
csvreader = self.__unicode_csv_reader([unicode(v)])
newval += tuple(csvreader.next()) #pylint:
disable=E1101
else:
newval += (v,)
value = newval
...
>
>>
>> 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.
>
> Hm, we could add dnsrecord-<rr>-del ZONE RECORD VALUE command, but this
> would increase the already high number of DNS commands.
>
> Endi and Petr had an idea to add a new --structured option for
> dnsrecord-find/dnsrecord-show which would show structured DNS records
> instead of raw DNS records. But I think our current framework is not
> ready for this. While a raw DNS record is basically one entry item
> (label : value), structured DNS record is an entry on its own
> ({label1:value1, label2:value2, ...}):
> - dnsrecord-find's output is ListOfEntries, but with use of
> --structured option it would have to be changed to something like
> ListOfListsOfEntries
> - dnsrecord-show's output would have to be changed with a use of
> --structured option from Entry to ListOfEntries
>
>>
>> Help per command doesn't work:
>>
>> $ ipa help dnsrecorda
>> ipa: ERROR: no command nor help topic 'dnsrecorda'
>
> Hm, help on LDAP objects (dnsrecord, dnsrecordloc) never worked. We can
> only get help for topics (python module name - "dns" in this case) or
> commands - dnsrecorda-add/mod/show. It worked for me:
>
> ipa help dnsrecorda-mod
> Purpose: Modify a record of resource record type A
> ...
>
>>
>> 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!
>
> Yeah, I think it will help users construct even complex DNS records
> without having to know all the RFCs with record format.
>
> Martin
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Jan Cholasta
More information about the Freeipa-devel
mailing list