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. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
From d76e10e2e1d0474d5ff5815660bd2cd11a6d4199 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic <akriv...@redhat.com> Date: Thu, 9 May 2013 18:47:12 +0200 Subject: [PATCH] Prompt for nameserver IP address in dnszone-add Prompt for nameserver IP address in interactive mode of dnszone-add. Add a corresponding field to dnszone creation dialog in the web UI. This parameter is required if and only if: * New zone is a forward zone * Nameserver is defined inside the new zone https://fedorahosted.org/freeipa/ticket/3603 --- install/ui/src/freeipa/dns.js | 52 +++++++++++++++++++++++++++++ install/ui/test/data/ipa_init_commands.json | 11 ++++++ ipalib/plugins/dns.py | 23 ++++++++++++- 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js index 5024e8b768ea46c86eb4db5901e71b02866432ff..25c3340b4df6be974342926e2bd13ce1b7c3b11f 100644 --- a/install/ui/src/freeipa/dns.js +++ b/install/ui/src/freeipa/dns.js @@ -300,6 +300,11 @@ return { fields: [ 'idnssoamname', { + name: 'ip_address', + validators: [ 'ip_address' ], + metadata: '@mc-opt:dnszone_add:ip_address' + }, + { name: 'idnssoarname', required: false }, @@ -576,11 +581,58 @@ IPA.dnszone_adder_dialog = function(spec) { var that = IPA.entity_adder_dialog(spec); + var init = function() { + var zone_w = that.fields.get_field('idnsname').widget; + var reverse_zone_w = that.fields.get_field('name_from_ip').widget; + var ns_w = that.fields.get_field('idnssoamname').widget; + + zone_w.value_changed.attach(that.check_ns_ip); + reverse_zone_w.value_changed.attach(that.check_ns_ip); + ns_w.value_changed.attach(that.check_ns_ip); + }; + + that.check_ns_ip = function() { + var ip_address_f = that.fields.get_field('ip_address'); + var zone_w = that.fields.get_field('idnsname').widget; + var ns_w = that.fields.get_field('idnssoamname').widget; + + var zone = $('input', zone_w.container).val(); + var ns = $('input', ns_w.container).val(); + + var zone_is_reverse = zone_w.input.prop('disabled'); + var relative_ns = true; + var ns_in_zone = false; + + if (ns && ns[ns.length-1] === '.') { + relative_ns = false; + ns = ns.slice(0, -1); + } + + if (zone && zone[zone.length-1] === '.') { + zone = zone.slice(0, -1); + } + + if (ns && zone && ns.indexOf(zone, ns.length - zone.length) !== -1) { + ns_in_zone = true; + } + + if (!zone_is_reverse && (relative_ns || ns_in_zone)) { + ip_address_f.set_enabled(true); + ip_address_f.set_required(true); + } else { + ip_address_f.reset(); + ip_address_f.set_required(false); + ip_address_f.set_enabled(false); + } + }; + that.create = function() { that.entity_adder_dialog_create(); that.container.addClass('dnszone-adder-dialog'); }; + init(); + return that; }; diff --git a/install/ui/test/data/ipa_init_commands.json b/install/ui/test/data/ipa_init_commands.json index a7e00ba55209a987b9f5684a738c99803ecb7e28..b66ae4dd1338dca3beb68ef4b34125e8c4516145 100644 --- a/install/ui/test/data/ipa_init_commands.json +++ b/install/ui/test/data/ipa_init_commands.json @@ -7654,6 +7654,17 @@ { "attribute": true, "class": "Str", + "doc": "Add forward record for nameserver located in the created zone", + "flags": [], + "label": "Nameserver IP address", + "name": "ip_address", + "noextrawhitespace": true, + "required": true, + "type": "unicode" + }, + { + "attribute": true, + "class": "Str", "doc": "Administrator e-mail address", "flags": [], "label": "Administrator e-mail address", diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 3ad03402d5a3b66b0f64545ff8812e9201258d6e..f34dbd062f14ade4bd60fd0570ddf25bf4167142 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -1781,9 +1781,30 @@ class dnszone_add(LDAPCreate): ), Str('ip_address?', _validate_ipaddr, doc=_('Add forward record for nameserver located in the created zone'), + label=_('Nameserver IP address'), ), ) + def interactive_prompt_callback(self, kw): + """ + Interactive mode should prompt for nameserver IP address only if all + of the following conditions are true: + * New zone is a forward zone + * NS is defined inside the new zone (NS can be given either in the + form of absolute or relative name) + """ + if kw.get('ip_address', None): + return + + zone = normalize_zone(kw['idnsname']) + ns = kw['idnssoamname'] + relative_ns = not ns.endswith('.') + ns_in_zone = self.obj.get_name_in_zone(zone, ns) + + if not zone_is_reverse(zone) and (relative_ns or ns_in_zone): + ip_address = self.Backend.textui.prompt(_(u'Nameserver IP address')) + kw['ip_address'] = ip_address + def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): assert isinstance(dn, DN) if not dns_container_exists(self.api.Backend.ldap2): @@ -1815,7 +1836,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): elif nameserver_ip_address: raise errors.ValidationError(name='ip_address', error=_("Nameserver DNS record is created for " - "for forward zones only")) + "forward zones only")) elif nameserver_ip_address and nameserver.endswith('.') and not record_in_zone: raise errors.ValidationError(name='ip_address', error=_("Nameserver DNS record is created only for " -- 1.8.1.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
