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. > > 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@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel