Re: [Freeipa-devel] [PATCH 0050-0051] Topology fixes for CA suffix

2016-06-24 Thread Petr Vobornik 
On 06/24/2016 12:47 PM, Stanislav Laznicka wrote:
> 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.
> 

ACK (after consultation with Standa, I've removed accidental removal of
doc string).

master:
* 5b5258b01081aa9ad4bf83907941c1c2d8a47722 Fix topologysuffix-verify
failing connections
* 13328bc7518a9e536d26562a738b4591c0494b75 topo segment-add: validate
that both masters support target suffix


-- 
Petr Vobornik

-- 
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

Re: [Freeipa-devel] [PATCH 0050-0051] Topology fixes for CA suffix

2016-06-24 Thread Stanislav Laznicka 

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 
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 
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):

Re: [Freeipa-devel] [PATCH 0050-0051] Topology fixes for CA suffix

2016-06-24 Thread Petr Vobornik 
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.

-- 
Petr Vobornik
From c89512b00f584337cb51be51a239d482661e64e5 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Thu, 23 Jun 2016 16:07:18 +0200
Subject: [PATCH] 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index 216b5964e1063545b90564bbf6df316aef592fd4..6b65d16ed4ab4f67a4dbda144e3e2db84e209a93 100644
--- a/ipaserver/plugins/topology.py
+++ b/ipaserver/plugins/topology.py
@@ -498,6 +498,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)
-- 
2.5.5

From 7d57f3479272c42b69c74066dc984a87a5a1fb8e Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Thu, 23 Jun 2016 16:04:04 +0200
Subject: [PATCH] topo segment-add: validate that both master supports 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, 26 insertions(+), 4 deletions(-)

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index c1848f0cc699f84b40be3623e956780d65de8619..216b5964e1063545b90564bbf6df316aef592fd4 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:
@@ -203,7 +204,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')
 
@@ -245,6 +246,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):
@@ -265,7 +287,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
 
 
@@ -290,7 +312,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

Re: [Freeipa-devel] [PATCH 0050-0051] Topology fixes for CA suffix

2016-06-23 Thread Petr Vobornik 
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.

-- 
Petr Vobornik

-- 
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

[Freeipa-devel] [PATCH 0050-0051] Topology fixes for CA suffix

2016-06-23 Thread Stanislav Laznicka 

Hello,

attached are patches fixing the logic mentioned in 
https://fedorahosted.org/freeipa/ticket/5967.
From 7d833bf0018f4b3e85bae88cbe383568f6d9c3f4 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Thu, 23 Jun 2016 16:04:04 +0200
Subject: [PATCH 1/2] Raise exception on incorrect segment addition

This patch removes the ability to add CA segment between hosts where
either is not yet managed by the CA suffix.

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

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index c1848f0cc699f84b40be3623e956780d65de8619..1c54d187342bfdc830fd650f24bf5720a5135ea1 100644
--- a/ipaserver/plugins/topology.py
+++ b/ipaserver/plugins/topology.py
@@ -89,6 +89,37 @@ def validate_domain_level(api):
 )
 
 
+def validate_ca_connectivity(api, left_node, right_node):
+"""
+Validate whether two nodes may create a CA replication agreement
+"""
+ca_hosts = api.Command.topologysegment_find(u'ca', sizelimit=0)['result']
+left_found = False
+right_found = False
+for ca_host in ca_hosts:
+if (not left_found and left_node in
+(ca_host['iparepltoposegmentleftnode'][0],
+ ca_host['iparepltoposegmentrightnode'][0])):
+left_found = True
+if (not right_found and right_node in
+(ca_host['iparepltoposegmentleftnode'][0],
+ ca_host['iparepltoposegmentrightnode'][0])):
+right_found = True
+
+if left_found and right_found:
+return
+
+invalid_nodes = {}
+if not left_found:
+invalid_nodes['leftnode'] = left_node
+if not right_found:
+invalid_nodes['rightnode'] = right_node
+raise errors.ValidationError(
+name=', '.join(invalid_nodes.keys()),
+error=_("The following nodes are not yet in the 'ca' suffix: {hosts}"
+.format(hosts=', '.join(invalid_nodes.values()
+
+
 @register()
 class topologysegment(LDAPObject):
 """
@@ -266,6 +297,12 @@ class topologysegment_add(LDAPCreate):
 assert isinstance(dn, DN)
 validate_domain_level(self.api)
 self.obj.validate_nodes(ldap, dn, entry_attrs)
+if keys[0] == 'ca':
+validate_ca_connectivity(
+self.api,
+entry_attrs['iparepltoposegmentleftnode'],
+entry_attrs['iparepltoposegmentrightnode']
+)
 return dn
 
 
-- 
2.5.5

From b146c6bec3e99b06ce86d931a22ce038b1807a02 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Thu, 23 Jun 2016 16:07:18 +0200
Subject: [PATCH 2/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 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/topology.py b/ipaserver/plugins/topology.py
index 1c54d187342bfdc830fd650f24bf5720a5135ea1..5cf2927c6da36dbb93b241450c05a0bb87d759aa 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:
@@ -513,6 +514,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)
-- 
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