[Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add

[Petr Spacek]

On 9.5.2013 19:03, Ana Krivokapic wrote:
> On 05/09/2013 02:10 PM, Martin Kosek wrote:
>> On 05/09/2013 12:45 PM, Petr Viktorin wrote:
>>> On 05/07/2013 07:50 PM, Ana Krivokapic wrote:
>>>> Prompt for nameserver IP address in dnszone-add
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/3603
>>> See Petr Špaček's mail for normal zones.
>>> Also when adding a reverse zone we should not ask:
>>>
>>> $ ipa dnszone-add --name-from-ip=80.142.15.0/24 --name-server=`hostname`.
>>> Zone name [15.142.80.in-addr.arpa.]:
>>> Administrator e-mail address [hostmaster.15.142.80.in-addr.arpa.]:
>>> [Nameserver IP address]: 1.2.3.4
>>> ipa: ERROR: invalid 'ip_address': Nameserver DNS record is created for for
>>> forward zones only
>>>
>>> The Web UI also asks for the NS IP address for reverse zones and fails when
>>> it's given.
>>>
>>>
>>> (Also, looking at the output above, asking for the zone name isn't useful for
>>> reverse zones, but I guess that's a different usability issue.)
>>>
>>>
>>> I think the easiest way to selectively ask in CLI is a custom
>>> interactive_prompt_callback (or we could have a separate dnszone-add-reverse
>>> command). As for the UI I don't know.
>>> The question is, do we want to go that far for this bug?
>> I do not like the alwaysask approach. I think it rather complicates things.
>>
>> I think we will indeed need to do the interactive_prompt_callback and in case
>> when we detect the following cases (as per Petr Spacek's mail):
>>
>> new zone = example.com.
>> a) NS = ns.example.com. (i.e. absolute name)
>> => IP address is required because NS is defined inside the new zone
>>
>> b) NS = ns (i.e. relative name)
>> => IP address is required because NS is defined inside the new zone
>>
>> we need to ask for IP address. As in this case, this is really not an option,
>> but a required parameter. If possible, the same logic would be great to have
>> for the UI.
>>
>> When deciding the question above, you can take advantage of get_name_in_zone
>> function which will also work with cases like
>> ipa dnszone-add example.com --name-server=@ --ip-address 10.16.78.47
>>
>> Martin
>
> Thanks all for the reviews. I addressed all comments in the attached
> updated patch.

I went quickly through the patch and the logic seems okay.

As a part of the patch I would recommend to change the error messages below to 
something more meaningful/accurate:

 >         if zone_is_reverse(normalized_zone):
 >             if not nameserver.endswith('.'):
 >                 raise errors.ValidationError(name='name-server',
 >                         error=_("Nameserver for reverse zone cannot be "
 >                                 "a relative DNS name"))
 >             elif nameserver_ip_address:
 >                 raise errors.ValidationError(name='ip_address',
 >                         error=_("Nameserver DNS record is created for "
 >                                 "for forward zones only"))

"Nameserver for reverse zone cannot be relative DNS name"
=> "Relative names in nameserver records for reverse zone are not supported"

"Nameserver DNS record is created for for forward zones only"
=> "A/AAAA records in reverse zone are not supported"

The problem is that both cases are completely legal from DNS point of view, 
but IPA interface do not allow to create such zones.


The other option is to drop this 'if' branch completely, but it definitely 
should be in a separate patch or even a ticket.

-- 
Petr^2 Spacek