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

[Petr Vobornik]

On 12/09/2011 08:02 AM, Endi Sukma Dewata wrote:
> On 11/28/2011 10:35 AM, 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:
>
> For the rest of the record types will there be new commands like below
> eventually or should we use the dnsrecord command?
>
>> dnsrecord<RRTYPE>-show ZONE NAME
>> dnsrecord<RRTYPE>-add ZONE NAME
>> dnsrecord<RRTYPE>-mod ZONE NAME VALUE
>
> Here's how the commands can be used in the DNS records list page:
>
> http://edewata.fedorapeople.org/freeipa/install/ui/#dns=dnszone&identity=dns&navigation=identity&dnszone-facet=default&dnszone-pkey=example.com
>
>
> As discussed in ticket #2094 we will add paging support for this page.
> This is done by calling dnsrecord-find --pkey-only to get all primary
> keys, then call a batch of dnsrecord-show to get full data of the
> records being displayed in the current page. I don't think we want to
> use the new dnsrecord<RRTYPE>-show here because we want to get all
> record types and since the space is limited we will just show the raw data.
>
> Click the Add link on the top of the page. The dialog box could be
> modified such that when you select a Record Type instead of showing the
> same Data field it will show the proper fields for the selected type.
> Then when you submit it will call the new dnsrecord<RRTYPE>-add command.
>
> Also discussed in #2094, we'll only have one checkbox for each record
> name instead of one for each value. So in this page we can only delete
> the entire record entry. This will be done using the existing
> dnsrecord-del command.
>
> And then the details page:
>
> http://edewata.fedorapeople.org/freeipa/install/ui/#dns=dnsrecord&identity=dns&navigation=identity&dnsrecord-facet=default&dnsrecord-pkey=testrec&dnszone-pkey=example.com
>
>
> Right now we use a single text field for each raw data. The first option
> is we could modify this page to use a table for each type, similar to
> HBAC/sudo rule. For example:
>
> SRV Records
>
> x | Priority | Weight | Port | Target [Delete] [Add]
> -------------------------------------------------------
> x | 0 | 100 | 88 | test
>
> The content of the table will be obtained using the new
> dnsrecord<RRTYPE>-show. The add dialog will show the fields for the type
> and it will call the new dnsrecord<RRTYPE>-add command. Deletion will be
> done using the existing dnsrecord-del command.

I like this option better than the second one. It seems clearer to me.

Just from UI perspective (no change in commands): I would show only the 
the tables which have some values - to let the page be shorter and 
clearer. This will require an add button in the facet header - similar 
dialog with type selection as in add in search facet except that the 
idnsname would be implied.
>
> The other option is to keep the existing style (without tables) but
> instead of a single text field for each record value we will show the
> values in separate fields. For example:
>
> SRV Records
>
> Target: [test ] Port: [88 ] [Delete]
> Priority: [0 ] Weight: [100 ]
>
> [Add]
>
> When you click Update, the UI will have to translate the changes into
> add, modify, and delete operations. The add and modify will be done
> using the new dnsrecord<RRTYPE>-add/mod command. The delete will be done
> using the existing dnsrecord-del command.
>
> Note, regardless of these options when you open the details page the UI
> could execute up to 20 dnsrecord<RRTYPE>-show operations since there are
> 20 possible record types currently listed in the UI.
To avoid this it would be nice to have a method like dnsrecord-find ZONE 
NAME (ie this one with new option) which would return the record with 
data structure similar to dnsrecord<RR>-show. For example (content of 
result):
[ //record array
     {
	idnsname: 'foo', //name
         arecord: [
             {
                 arecord: '10.10.10.10', //raw
                 ip_address: '10.10.10.10'
             }
         ],
	cnamerecord: [
             {
                 cnamerecord: 'bar.example.com.', //raw
                 hostname: 'bar.example.com.'
             },
             {
                 cnamerecord: 'bar2.example.com.', //raw
                 hostname: 'bar2.example.com.'
             },
         ]
     }
]

This way we would get all info we need without calling batch with 20 
commands or calling dnsrecord-find and then batch with 1-20 commands.
>
>> 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
>
> I think for consistency it would be better to call this command
> dnsrecord<RRTYPE>-find since it returns multiple entries. We might also
> be able to add filters in the future.
+1
>
>> 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.
>
> Yes, the raw value will be needed for calling dnsrecord<RRTYPE>-mod or
> dnsrecord-del command.
+1
>
>> 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.
>
> I agree, the extra dash would be better. When calling a command the UI
> concatenates entity name and method name with an '_'. In this case the
> entity name will remain dnsrecord and the RRTYPE will be prepended to
> the method name.

Or we can avoid specifying the entity for command and just fill whole 
command name.

Either way it is doable and the dash is definitely better for CLI - when 
I was playing with the commands I was inclined to wrote the dash.

>
> Btw, patch #175 causes a build break:
>
> ipalib/parameters.py:718: [E1101, Param.normalize] Generator
> '__unicode_csv_reader' has no 'next' member
>

-- 
Petr Vobornik