On 06/24/2016 11:52 AM, Martin Babinsky wrote:
On 06/24/2016 11:30 AM, Petr Vobornik wrote:
On 06/23/2016 05:30 PM, Stanislav Laznicka wrote:
On 06/23/2016 04:38 PM, Petr Vobornik wrote:
On 06/23/2016 04:20 PM, Stanislav Laznicka wrote:
Hello,

attached are patches fixing the logic mentioned in
https://fedorahosted.org/freeipa/ticket/5967.


If server supports the suffix can be verified in validate_nodes call
where masters are already fetched.

Thank you for the suggestion, modified patch 50 attached.


Maybe it's just me, but the code is hard to ready. Check the attached
version - speeding up review process.

I've also change the first commit message line it was too generic.




If you intend to use that internal function in other modules, please remove the leading underscore from its name. Otherwise pylint/IDEs may complain about import of private module member.

Thank you for the review/update. You went the other way around it which indeed does seem much more readable. Not sure if you meant to change the order of the patches which in order 50 -> 51 would make 50 alone not work because of missing import but I did change the order and fixed that.

I also included the objection from Martin in the patches and removed the leading underscore from the imported function.

From e0a21a40cd0c8871ebb19fdce4ce1882ad674bed Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Thu, 23 Jun 2016 16:07:18 +0200
Subject: [PATCH 1/2] Fix topologysuffix-verify failing connections

topologysuffix-verify would have checked connectivity even between hosts that
are not managed by the given suffix.

https://fedorahosted.org/freeipa/ticket/5967
---
 ipaserver/plugins/topology.py | 4 +++-
 ipaserver/topology.py         | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index c1848f0cc699f84b40be3623e956780d65de8619..0d0b3c0841558c5adce05996fecb297b998bce66 100644
--- a/ipaserver/plugins/topology.py
+++ b/ipaserver/plugins/topology.py
@@ -14,7 +14,8 @@ from ipalib import _, ngettext
 from ipalib import output
 from ipalib.constants import DOMAIN_LEVEL_1
 from ipaserver.topology import (
-    create_topology_graph, get_topology_connection_errors)
+    create_topology_graph, get_topology_connection_errors,
+    map_masters_to_suffixes)
 from ipapython.dn import DN
 
 if six.PY3:
@@ -476,6 +477,7 @@ Checks done:
 
         masters = self.api.Command.server_find(
             '', sizelimit=0, no_members=False)['result']
+        masters = map_masters_to_suffixes(masters).get(keys[0], [])
         segments = self.api.Command.topologysegment_find(
             keys[0], sizelimit=0)['result']
         graph = create_topology_graph(masters, segments)
diff --git a/ipaserver/topology.py b/ipaserver/topology.py
index 27c3b29a4d3c2fe477e6e519b4006b3d96f0eeae..385da29a66fb7276c55e9aac5c8c266b897721a7 100644
--- a/ipaserver/topology.py
+++ b/ipaserver/topology.py
@@ -70,7 +70,7 @@ def get_topology_connection_errors(graph):
     return connect_errors
 
 
-def _map_masters_to_suffixes(masters):
+def map_masters_to_suffixes(masters):
     masters_to_suffix = {}
 
     for master in masters:
@@ -97,7 +97,7 @@ def _create_topology_graphs(api_instance):
     masters = api_instance.Command.server_find(
         u'', sizelimit=0, no_members=False)['result']
 
-    suffix_to_masters = _map_masters_to_suffixes(masters)
+    suffix_to_masters = map_masters_to_suffixes(masters)
 
     topology_graphs = {}
 
-- 
2.5.5

From 9cd466fcc32399907b231250e78523fa305cb68a Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Thu, 23 Jun 2016 16:04:04 +0200
Subject: [PATCH 2/2] topo segment-add: validate that both masters support
 target suffix

This patch removes the ability to add segment between hosts where
either does not support the requested suffix.

https://fedorahosted.org/freeipa/ticket/5967
---
 ipaserver/plugins/topology.py | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index 0d0b3c0841558c5adce05996fecb297b998bce66..4676c48891c7a6b97ec1f97ad9a7c51400daf06c 100644
--- a/ipaserver/plugins/topology.py
+++ b/ipaserver/plugins/topology.py
@@ -43,9 +43,6 @@ B to A. Creation of unidirectional segments is not allowed.
 """) + _("""
 To verify that no server is disconnected in the topology of the given suffix,
 use:
-  ipa topologysuffix-verify $suffix
-""") + _("""
-
 Examples:
   Find all IPA servers:
     ipa server-find
@@ -204,7 +201,7 @@ class topologysegment(LDAPObject):
         ),
     )
 
-    def validate_nodes(self, ldap, dn, entry_attrs):
+    def validate_nodes(self, ldap, dn, entry_attrs, suffix):
         leftnode = entry_attrs.get('iparepltoposegmentleftnode')
         rightnode = entry_attrs.get('iparepltoposegmentrightnode')
 
@@ -246,6 +243,27 @@ class topologysegment(LDAPObject):
                 error=_('left node and right node must not be the same')
             )
 
+        # don't allow segment between nodes where both don't have the suffix
+        masters_to_suffix = map_masters_to_suffixes(masters)
+        suffix_masters = masters_to_suffix.get(suffix, [])
+        suffix_m_hostnames = [m['cn'][0].lower() for m in suffix_masters]
+
+        if leftnode not in suffix_m_hostnames:
+            raise errors.ValidationError(
+                name='leftnode',
+                error=_("left node ({host}) does not support "
+                        "suffix '{suff}'"
+                        .format(host=leftnode, suff=suffix))
+            )
+
+        if rightnode not in suffix_m_hostnames:
+            raise errors.ValidationError(
+                name='rightnode',
+                error=_("right node ({host}) does not support "
+                        "suffix '{suff}'"
+                        .format(host=rightnode, suff=suffix))
+            )
+
 
 @register()
 class topologysegment_find(LDAPSearch):
@@ -266,7 +284,7 @@ class topologysegment_add(LDAPCreate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
         validate_domain_level(self.api)
-        self.obj.validate_nodes(ldap, dn, entry_attrs)
+        self.obj.validate_nodes(ldap, dn, entry_attrs, keys[0])
         return dn
 
 
@@ -291,7 +309,7 @@ class topologysegment_mod(LDAPUpdate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
         validate_domain_level(self.api)
-        self.obj.validate_nodes(ldap, dn, entry_attrs)
+        self.obj.validate_nodes(ldap, dn, entry_attrs, keys[0])
         return dn
 
 
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to