[Freeipa-devel] [PATCH 0104-0109] DNS upgrade: change forwarding policy to "only" if private IPs are used

[Martin Basti]

On 03.05.2016 14:59, Petr Spacek wrote:
> Hello,
>
> DNS upgrade: change forwarding policy to "only" if private IPs are used.
>
> https://fedorahosted.org/freeipa/ticket/5710
>
> This is the upgrade part. I will add one more patch to print a warning in
> dnsforwardzone* commands to avoid surprises. Please do not close the ticket yet.
>
>
>

1)
Upgrade failed with 'BindInstance' object has no attribute 
'named_conf_get_directive'
IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run 
command ipa-server-upgrade manually.
('IPA upgrade failed.', 1)
The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for 
more information

2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object has 
no attribute 'named_conf_get_directive'
2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py", 
line 213, in __upgrade
     self.modified = (ld.update(self.files) or self.modified)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py", line 
917, in update
     self._run_updates(all_updates)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py", line 
889, in _run_updates
     self._run_update_plugin(update['plugin'])
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py", line 
862, in _run_update_plugin
     restart_ds, updates = self.api.Updater[plugin_name]()
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 
1418, in __call__
     return self.execute(**options)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py", 
line 547, in execute
     self.update_global_named_conf_forwarder(bind)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py", 
line 508, in update_global_named_conf_forwarder
     if bind.named_conf_get_directive(
AttributeError: 'BindInstance' object has no attribute 
'named_conf_get_directive'

2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", 
line 447, in start_creation
     run_step(full_msg, method)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", 
line 437, in run_step
     method()
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py", 
line 221, in __upgrade
     raise RuntimeError(e)
RuntimeError: 'BindInstance' object has no attribute 
'named_conf_get_directive'

PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new 
attribute *
2)
+        Int('ipadnsversion?',
+            label=_('IPA DNS version'),
+        ),

Shouldn't be this part of System: Read DNS Configuration permission?

3)
-    def postprocess_result(self, result):
+    def postprocess_result(self, result, show_version):
          if not any(param in result['result'] for param in self.params):
              result['summary'] = unicode(_('Global DNS configuration is 
empty'))

show_version param was added but I don't see it used in this patch.

4)
+        Int('ipadnsversion?',
+            label=_('IPA DNS version'),
+        ),

Could we add comment here that this option is accessible only from 
installers and upgrade?

5)
+        for config_option in container_entry.get("ipaConfigString", []):
+            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
+                               config_option, flags=re.I)
+            if matched:
+                version = int(matched.group("version"))

Shouldn't we print error if version cannot be parsed?

PATCH  * DNS upgrade: separate backup logic to make it reusable *

LGTM

PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *

7)
I'm curious why do you need to check superdomains?

PATCH * DNS upgrade: change forwarding policy to = only for conflicting 
forward zones*

8)
+            self.log.debug('Zone %s was sucessfully modified to use '
+                           'forward policy "only"', zone['idnsname'][0])
<---missing empty line---->
+    def execute(self, **options):

PATCH * DNS upgrade: change global forwarding policy in LDAP to "only" 
if private IPs are used *
9)
- dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
+                dnsutil.related_to_auto_empty_zone(
+                    dnsutil.DNSName(zone.get('idnsname')[0]))

Should be in previous commit

10)
-            return
+            return False, []
This should be fixed in the previous commit

PATCH * DNS upgrade: change global forwarding policy in named.conf to 
"only" if private IPs are used *
11)
IMO this is an upgrade of configuration and this should be in 
ipaserver/install/server/upgrade.py, upgrade plugins are used only for 
updating of LDAP values

Unless you really want to use this as precedence, but then it requires 
broader discussion.

12)

bind.named_conf_get_directive
should be
bindinstance.named_conf_get_directive

see 1)

Martin^2


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160511/d73c3676/attachment.htm>