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


--
Jan Cholasta

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

Reply via email to