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

Fri May 10 11:30:25 UTC 2013

On 05/10/2013 10:58 AM, Petr Viktorin wrote:
> 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=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.
>
> 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.

Added - updated patch attached.
>
> 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.
>

I moved the string changes into a separate patch.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0027-03-Prompt-for-nameserver-IP-address-in-dnszone-add.patch
Type: text/x-patch
Size: 9139 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130510/bc6dd64d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0028-Improve-error-messages-in-dnszone-add.patch
Type: text/x-patch
Size: 1860 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130510/bc6dd64d/attachment-0001.bin>