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

Petr Viktorin pviktori at redhat.com
Fri May 10 08:58:24 UTC 2013

On 05/10/2013 09:30 AM, Petr Spacek wrote:
> 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=
>>>> --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]:
>>>> 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
>>> Martin
>> Thanks all for the reviews. I addressed all comments in the attached
>> updated patch.

Thanks, it works nicely now.
Could you add some tests? See tests/test_cmdline/test_cli.py for some 
examples of tests of interactive prompting.

The Web UI works, but it'd be nice if a JS expert looked over the code.

> 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.

Is this going into this release? If yes, it's pretty late for string 
changes, both these and the "for for" typo fix in this patch, so we 
should leave them out.


More information about the Freeipa-devel mailing list