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.
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.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
