On 05/14/2013 12:05 PM, Ana Krivokapic wrote:
> On 05/13/2013 02:50 PM, Petr Vobornik wrote:
>> On 05/12/2013 03:10 PM, Ana Krivokapic wrote:
>>> On 05/10/2013 10:37 PM, Endi Sukma Dewata wrote:
>>>> On 5/10/2013 9:38 AM, Petr Viktorin wrote:
>>>>> On 05/10/2013 03:57 PM, Ana Krivokapic wrote:
>>>>> [...]
>>>>>> Thanks for catching the bugs. Updated patches are attached.
>>>>> Thanks! It works nicely.
>>>>> Endi is doing a quick check of the Javascript, if he doesn't find an
>>>>> issue then ACK.
>>>>>
>>>>> If this still makes it into 3.2.0, please only push the first patch
>>>>> there.
>>>> I tried this in the UI:
>>>>
>>>>    Zone name: test.com
>>>>    Authoritative nameserver: ns.sometest.com.
>>>>
>>>> The 'Nameserver IP address' field is still enabled. This is because
>>>> the name
>>>> server is considered in the zone although it's actually not.
>>>>
>>>> The CLI seems to work fine, it didn't ask for IP address.
>>>>
>>>> The UI probably could be fixed using endsWith(ns, '.' + zone).
>>>> Everything else
>>>> looks fine. ACK with the fix.
>>>>
>>> Fixed, updated patch attached.
>>>
>> A nitpick for UI part which is not a blocker(nack) because we don't
>> have any strict rules for following topic:
>>
>> We should avoid depending on widget's html output outside of the
>> widget code.
>>
>> So we should use:
>>    zone_w.save()[0]
>> instead of:
>>   $('input', zone_w.container).val();
>>
>> same for `ns`.
> Thanks, fixed.
>> Unfortunately there is no text_widget.is_enabled() method  so
>> `zone_w.input.prop('disabled')` can't be replaced.
> I implemented the `text_widget.is_enabled()` method, and replaced
> `zone_w.input.prop('disabled')` with `!zone_w.is_enabled()`.
>
> Updated patch attached.

Petr caught another bug: due to the return value of
`text_widget.save()`, an exception was raised in the case of empty zone.
This has been fixed in the attached patch.

I also changed the name of the endsWith() function to ends_with(), to
conform to our coding standard.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 160e5a46e9a21c702d61c457343e2f0408ede2d4 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

Add a new unit test to cover this functionality.

https://fedorahosted.org/freeipa/ticket/3603
---
 install/ui/src/freeipa/dns.js               | 58 +++++++++++++++++++++++++
 install/ui/src/freeipa/widget.js            |  4 ++
 install/ui/test/data/ipa_init_commands.json | 11 +++++
 ipalib/plugins/dns.py                       | 21 +++++++++
 tests/test_cmdline/test_cli.py              | 67 +++++++++++++++++++++++++++++
 5 files changed, 161 insertions(+)

diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index 5024e8b768ea46c86eb4db5901e71b02866432ff..52cbb81f36f863afd61bf3b6cc04affe59809a48 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,64 @@ IPA.dnszone_adder_dialog = function(spec) {
 
     var that = IPA.entity_adder_dialog(spec);
 
+    function ends_with(str, suffix) {
+        return str.indexOf(suffix, str.length - suffix.length) !== -1;
+    }
+
+    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 = zone_w.save()[0] || '';
+        var ns = ns_w.save()[0] || '';
+
+        var zone_is_reverse = !zone_w.is_enabled() ||
+                              ends_with(zone, '.in-addr.arpa.') ||
+                              ends_with(zone, '.ip6.arpa.');
+        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 && ends_with(ns, '.' + zone)) {
+            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/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 8f1208e0b12ed26a97c1f0c83a228f192f61e0ec..4570c9033e39361e3827674c2e022026bdc6d883 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -303,6 +303,10 @@ IPA.text_widget = function(spec) {
         }
     };
 
+    that.is_enabled = function(value) {
+        return !that.input.prop('disabled');
+    };
+
     that.set_enabled = function(value) {
 
         that.input.prop('disabled', !value);
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..fbc445215dac583cac91de6cb9b33ee82e1aeffd 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):
diff --git a/tests/test_cmdline/test_cli.py b/tests/test_cmdline/test_cli.py
index f66906c6aa48137c83445147253599ae437621d1..bd1281e1d682b055ede9685a10a9cec91a3c76fd 100644
--- a/tests/test_cmdline/test_cli.py
+++ b/tests/test_cmdline/test_cli.py
@@ -258,3 +258,70 @@ def test_dnsrecord_del_comma(self):
                     version=API_VERSION)
         finally:
             self.run_command('dnszone_del', idnsname=u'test-example.com')
+
+    def test_dnszone_add(self):
+        """
+        Test dnszone-add with nameserver IP passed interatively
+        """
+        # Pass IP of nameserver interactively for nameserver in zone
+        # (absolute name)
+        with self.fake_stdin('1.1.1.1\n'):
+            self.check_command(
+                'dnszone_add example.com --name-server=ns.example.com. '
+                '--admin-email=ad...@example.com',
+                'dnszone_add',
+                idnsname=u'example.com',
+                idnssoamname=u'ns.example.com.',
+                idnssoarname=u'ad...@example.com',
+                ip_address=u'1.1.1.1',
+                idnssoaexpire=util.Fuzzy(type=int),
+                idnssoaserial=util.Fuzzy(type=int),
+                idnssoaretry=util.Fuzzy(type=int),
+                idnssoaminimum=util.Fuzzy(type=int),
+                idnssoarefresh=util.Fuzzy(type=int),
+                all=False,
+                raw=False,
+                force=False,
+                version=API_VERSION
+            )
+
+        # Pass IP of nameserver interactively for nameserver in zone
+        # (relative name)
+        with self.fake_stdin('1.1.1.1\n'):
+            self.check_command(
+                'dnszone_add example.com --name-server=ns '
+                '--admin-email=ad...@example.com',
+                'dnszone_add',
+                idnsname=u'example.com',
+                idnssoamname=u'ns',
+                idnssoarname=u'ad...@example.com',
+                ip_address=u'1.1.1.1',
+                idnssoaexpire=util.Fuzzy(type=int),
+                idnssoaserial=util.Fuzzy(type=int),
+                idnssoaretry=util.Fuzzy(type=int),
+                idnssoaminimum=util.Fuzzy(type=int),
+                idnssoarefresh=util.Fuzzy(type=int),
+                all=False,
+                raw=False,
+                force=False,
+                version=API_VERSION
+            )
+
+        # Nameserver is outside the zone - no need to pass the IP
+        self.check_command(
+            'dnszone_add example.com --name-server=ns.example.net. '
+            '--admin-email=ad...@example.com',
+            'dnszone_add',
+            idnsname=u'example.com',
+            idnssoamname=u'ns.example.net.',
+            idnssoarname=u'ad...@example.com',
+            idnssoaexpire=util.Fuzzy(type=int),
+            idnssoaserial=util.Fuzzy(type=int),
+            idnssoaretry=util.Fuzzy(type=int),
+            idnssoaminimum=util.Fuzzy(type=int),
+            idnssoarefresh=util.Fuzzy(type=int),
+            all=False,
+            raw=False,
+            force=False,
+            version=API_VERSION
+        )
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to