On 04/13/2015 10:56 AM, Ludwig Krispenz wrote:
Hi,

in the attachment you find the latest state of the "topology plugin", it
implements what is defined in the design page:
http://www.freeipa.org/page/V4/Manage_replication_topology (which is
also waiting for a reviewer)

It contains the plugin itself and  a core of ipa commands to manage a
topology. to be really applicable, some work outside is required, eg the
management of the domain level and a decision where the binddn group
should be maintained.

Thanks,
Ludwig



I've looked at the python part, mostly because I want to start with POC of Web UI for topology.

topology.py is clearly still a work in progress. I've reflected following comments into a patch to speed things up.

What's in the patch:

1. git am complains about trailing whitespaces

2. pep8 check produces quite a lot of issues. New code should be almost with any (`E501 line too long` is not a hard rule)
   `git diff HEAD~1 -U0 | pep8 --diff`

3. some typos

4. A lot of unused imports

5. Option name --sname for 'Segment identifier' is not very friendly. I don't see any examples of command options in the design notes.

6. NO_UPG_MAGIC - leftover from other plugin?

7. suffix object has labels from segment

8. IPA framework has a support for nested object. Key is setting `parent_object = 'topologysuffix'` in topologysegment object.

9. repl_agmt_attrs could be in topologysegment takes_params.

10. missing various CRUD commands like topologysuffix-find and topologysuffix-show commands.

Whats missing, not fixed:
1. last 2 lines of VERSION file are not updated

2. Mixed terminology. Somewhere is used suffix and somewhere replication area or just area.

3. Validation
- suffix should check for dn
- existence of both ends of a segment

4. print of segments in suffix-show needs to be improved or removed

To discuss:
a) Do params in topologysegment have to have a maxlength set?

b) Terminology has to be united. Segments are nested in suffix but sometimes are called areas and suffix is 'the suffix'. User might be confused. E.g. shouldn't the object be named a topologyarea instead of topologysuffix?

c) I've added all missing CRUD commands. Are there any which we don't want there, or want to restrict them. E.g. I can imagine that deleting a suffix should be prevented if it contains any segments (or it has to be forced (--force option))

d) Do we want to print segments in suffix-show?

e) Mainly for Honza: I've added --show-segments option to suffix-show which defaults to True. I don't like the behavior of CLI, which asks to confirm the value all the time. My intention was to have it there by default, but also allow to disable it by --show-segments=False. I don't want to add it as Flag (--hide-segments) since it restricts versatility. I would like to see an optional flag which would be filled by default value if not explicitly defined and CLI would not ask for the option value.
--
Petr Vobornik
From 7da7466d0270568cb5481bc0234cdd524693b0c0 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Tue, 21 Apr 2015 12:53:18 +0200
Subject: [PATCH] topology plugin improvements

---
 ipalib/plugins/topology.py                         | 337 +++++++++------------
 .../install/plugins/fix_replica_agreements.py      |   2 +-
 ipaserver/install/replication.py                   |   6 +-
 3 files changed, 148 insertions(+), 197 deletions(-)

diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 5e6584761dc900e8b2952173fa5d2b5b368e49d0..c02e3237f2b17274abb108d0230bc21506da4074 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -17,30 +17,21 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from time import gmtime, strftime
 import string
-import posixpath
 import os
 
 from ipalib import api, errors
-from ipalib import Flag, Int, Password, Str, Bool, StrEnum, DateTime
+from ipalib import Int, Str, Bool, StrEnum
 from ipalib.plugable import Registry
 from ipalib.plugins.baseldap import *
 from ipalib.plugins import baseldap
-from ipalib.request import context
 from ipalib import _, ngettext
 from ipalib import output
 from ipaplatform.paths import paths
-from ipapython.ipautil import ipa_generate_password
-from ipapython.ipavalidate import Email
-from ipalib.capabilities import client_has_capability
-from ipalib.util import (normalize_sshpubkey, validate_sshpubkey,
-    convert_sshpubkey_post)
-if api.env.in_server and api.env.context in ['lite', 'server']:
-    from ipaserver.plugins.ldap2 import ldap2
+
 
 __doc__ = _("""
-Toplogy
+Topology
 
 Manage replication topology.
 
@@ -48,60 +39,25 @@ Manage replication topology.
 
 register = Registry()
 
-NO_UPG_MAGIC = '__no_upg__'
-
-status_output_params = (
-    Str('server',
-        label=_('Server'),
-    ),
-   )
-
-repl_agmt_attrs = (
-    Str('nsds5replicastripattrs?',
-        cli_name='stripattrs',
-        label=_('Attributes to strip'),
-        normalizer=lambda value: value.lower(),
-    ),
-    Str('nsds5replicatedattributelist?',
-        cli_name='replattrs',
-        label='Attributes to replicate',
-    ),
-    Str('nsds5replicatedattributelisttotal?',
-        cli_name='replattrstotal',
-        label='Attributes for total update',
-    ),
-    Str('nsds5beginreplicarefresh?',
-        cli_name='init',
-        label='Initialize replica',
-    ),
-    Str('nsds5replicatimeout?',
-        cli_name='timeout',
-        label='Session timeout',
-    ),
-    Str('nsds5replicaenabled?',
-        cli_name='enabled',
-        label='Repliaction agreement enabled',
-    ),
-)
 
 @register()
 class topologysegment(LDAPObject):
     """
-    toplogy segment.
+    Topology segment.
     """
+    parent_object = 'topologysuffix'
     container_dn = api.env.container_topology
     object_name = _('segment')
     object_name_plural = _('segments')
     object_class = ['iparepltoposegment']
     default_attributes = [
-        'cn', 
-        'ipaReplTopoSegmentdirection', 'ipaReplTopoSegmentrightNode:',
-        'ipaReplTopoSegmentLeftNode'
-        'nsds5beginreplicarefresh', 'nsds5replicastripattrs', 
+        'cn',
+        'ipaReplTopoSegmentdirection', 'ipaReplTopoSegmentrightNode',
+        'ipaReplTopoSegmentLeftNode', 'nsds5beginreplicarefresh',
+        'nsds5replicastripattrs',
     ]
     search_display_attributes = [
-        'cn', 
-        'ipaReplTopoSegmentdirection', 'ipaReplTopoSegmentrightNode:',
+        'cn', 'ipaReplTopoSegmentdirection', 'ipaReplTopoSegmentrightNode',
         'ipaReplTopoSegmentLeftNode'
     ]
 
@@ -109,55 +65,83 @@ class topologysegment(LDAPObject):
     label_singular = _('Segment')
 
     takes_params = (
-        Str('cn',
+        Str(
+            'cn',
             maxlength=255,
-            cli_name='sname',
+            cli_name='name',
             primary_key=True,
-            label=_('Segment identifier'),
+            label=_('Segment name'),
+            default_from=lambda iparepltoposegmentleftnode, iparepltoposegmentrightnode:
+                '%s-%s' % (iparepltoposegmentleftnode, iparepltoposegmentrightnode),
             normalizer=lambda value: value.lower(),
         ),
-        Str('iparepltoposegmentleftnode',
+        Str(
+            'iparepltoposegmentleftnode',
             pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$',
             pattern_errmsg='may only include letters, numbers, -, . and $',
             maxlength=255,
             cli_name='lefthost',
-            label=_('Segment left host'),
+            label=_('Left host'),
             normalizer=lambda value: value.lower(),
         ),
-        Str('iparepltoposegmentrightnode',
+        Str(
+            'iparepltoposegmentrightnode',
             pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$',
             pattern_errmsg='may only include letters, numbers, -, . and $',
             maxlength=255,
             cli_name='righthost',
-            label=_('Segment right host'),
+            label=_('Right host'),
             normalizer=lambda value: value.lower(),
         ),
-        Str('iparepltoposegmentdirection?',
-            pattern='^[a-zA-Z][a-zA-Z-]{0,252}[a-zA-Z-]?$',
-            pattern_errmsg='may only include letters, and -',
-            maxlength=255,
+        StrEnum(
+            'iparepltoposegmentdirection',
             cli_name='direction',
-            label=_('Segment connectivity'),
+            label=_('Connectivity'),
+            values=(u'both', u'left-right', u'right-left', u'none'),
+            default=u'both',
+        ),
+        Str(
+            'nsds5replicastripattrs?',
+            cli_name='stripattrs',
+            label=_('Attributes to strip'),
             normalizer=lambda value: value.lower(),
         ),
-    ) + repl_agmt_attrs
+        Str(
+            'nsds5replicatedattributelist?',
+            cli_name='replattrs',
+            label='Attributes to replicate',
+        ),
+        Str(
+            'nsds5replicatedattributelisttotal?',
+            cli_name='replattrstotal',
+            label='Attributes for total update',
+        ),
+        Str(
+            'nsds5beginreplicarefresh?',
+            cli_name='init',
+            label='Initialize replica',
+        ),
+        Str(
+            'nsds5replicatimeout?',
+            cli_name='timeout',
+            label='Session timeout',
+        ),
+        Str(
+            'nsds5replicaenabled?',
+            cli_name='enabled',
+            label='Replication agreement enabled',
+        ),
+    )
 
-    def get_dn(self, *keys, **options):
-        parent_dn = self.backend.make_dn_from_attr(
-                             'cn', options.get('area', 'realm'),
-                             DN(self.container_dn, api.env.basedn)
-                             )
-        if keys:
-            return self.backend.make_dn_from_attr(
-                             'cn', keys[-1], parent_dn)
-        elif options.get('sname'):
-            return self.backend.make_dn_from_attr(
-                             'cn', options['sname'], parent_dn)
-        else:
-            full_name = '%s-%s' % (options['iparepltoposegmenlefthost'], options['iparepltoposegmentrighthost'])
-            return self.backend.make_dn_from_attr(
-                             'cn', full_name, parent_dn)
 
+@register()
+class topologysegment_find(LDAPSearch):
+    __doc__ = _('Search for topology segments.')
+
+    msg_summary = ngettext(
+        '%(count)d segment matched',
+        '%(count)d segments matched', 0
+    )
 
 
 @register()
@@ -166,96 +150,49 @@ class topologysegment_add(LDAPCreate):
 
     msg_summary = _('Added segment "%(value)s"')
 
-    has_output_params = LDAPCreate.has_output_params
-
-    takes_options = LDAPCreate.takes_options + (
-        Flag('whatfor',
-            cli_name='whatfor',
-            doc=_('connectivity direction: both, left-right, right-left, none'),
-        ),
-#        Str('sname?',
-#            maxlength=255,
-#            cli_name='sname',
-#            label=_('Segment identifier'),
-#            normalizer=lambda value: value.lower(),
-#        ),
-        Str('area?',
-            maxlength=255,
-            cli_name='area',
-            label=_('replica area for segment'),
-            normalizer=lambda value: value.lower(),
-        ),
-    )
-
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        assert isinstance(dn, DN)
-
-        config = ldap.get_ipa_config()
-        if options.get('sname'):
-            full_name = options['sname']
-        else:
-            full_name = '%s-%s' % (entry_attrs['iparepltoposegmentleftnode'], entry_attrs['iparepltoposegmentrightnode'])
-        entry_attrs.setdefault('cn', full_name)
-        if not entry_attrs.get('iparepltoposegmentdirection'):
-            entry_attrs.setdefault('iparepltoposegmentdirection', 'both')
-
-        return dn
 
 @register()
 class topologysegment_del(LDAPDelete):
     __doc__ = _('Delete a segment.')
 
+    msg_summary = _('Deleted segment "%(value)s"')
 
-    takes_options = LDAPDelete.takes_options + (
-        Str('area?',
-            maxlength=255,
-            cli_name='area',
-            label=_('replica area for segment'),
-            normalizer=lambda value: value.lower(),
-        ),
-    )
-
-    msg_summary = _('Deleted segement "%(value)s"')
 
 @register()
 class topologysegment_mod(LDAPUpdate):
     __doc__ = _('Modify a segment.')
 
+    msg_summary = _('Modified segment "%(value)s"')
 
-    takes_options = LDAPDelete.takes_options + (
-        Str('area?',
-            maxlength=255,
-            cli_name='area',
-            label=_('replica area for segment'),
-            normalizer=lambda value: value.lower(),
-        ),
-    )
 
-    msg_summary = _('Modified segement "%(value)s"')
+@register()
+class topologysegment_show(LDAPRetrieve):
+    __doc__ = _('Display a segment.')
+
 
 @register()
 class topologysuffix(LDAPObject):
     """
-    suffix managed by the topology plugin.
+    Suffix managed by the topology plugin.
     """
     container_dn = api.env.container_topology
     object_name = _('suffix')
     object_name_plural = _('suffices')
     object_class = ['iparepltopoconf']
-    default_attributes = [
-        'cn', 
-        'ipaReplTopoConfRoot'
-    ]
-    search_display_attributes = [
-        'cn', 
-        'ipaReplTopoConfRoot'
-    ]
-
-    label = _('Segments')
-    label_singular = _('Segment')
+    default_attributes = ['cn', 'ipaReplTopoConfRoot']
+    search_display_attributes = ['cn', 'ipaReplTopoConfRoot']
+    label = _('Suffices')
+    label_singular = _('Suffix')
 
     takes_params = (
-        Str('iparepltopoconfroot',
+        Str(
+            'cn',
+            cli_name='name',
+            primary_key=True,
+            label=_('Suffix name'),
+        ),
+        Str(
+            'iparepltopoconfroot',
             maxlength=255,
             cli_name='suffix',
             label=_('Suffix to be managed'),
@@ -263,81 +200,96 @@ class topologysuffix(LDAPObject):
         ),
     )
 
-    def get_dn(self, *keys, **options):
-        return self.backend.make_dn_from_attr(
-                             'cn', options['areaname'],
-                             DN(self.container_dn, api.env.basedn)
-                             )
 
+@register()
+class topologysuffix_find(LDAPSearch):
+    __doc__ = _('Search for topology suffices.')
+
+    msg_summary = ngettext(
+        '%(count)d topology suffix matched',
+        '%(count)d topology suffices matched', 0
+    )
+
+
+@register()
+class topologysuffix_del(LDAPDelete):
+    __doc__ = _('Delete a topology suffix.')
+
+    msg_summary = _('Deleted topology suffix "%(value)s"')
 
 
 @register()
 class topologysuffix_add(LDAPCreate):
-    __doc__ = _('Add a new suffix to be managed.')
+    __doc__ = _('Add a new topology suffix to be managed.')
 
     msg_summary = _('Added replication area "%(value)s"')
 
-    has_output_params = LDAPCreate.has_output_params
 
-    takes_options = LDAPCreate.takes_options + (
-        Str('areaname',
-            maxlength=255,
-            cli_name='area',
-            label=_('replica area for segment'),
-            normalizer=lambda value: value.lower(),
-        ),
-    )
+@register()
+class topologysuffix_mod(LDAPUpdate):
+    __doc__ = _('Modify a topology suffix.')
+
+    msg_summary = _('Modified topology suffix "%(value)s"')
 
 
 @register()
-class topologysuffix_show(LDAPMultiQuery):
+class topologysuffix_show(LDAPRetrieve):
     __doc__ = _('Show managed suffix.')
 
-    has_output = output.standard_list_of_entries
-    takes_options = LDAPSearch.takes_options + (
-        Str('areaname',
-            maxlength=255,
-            cli_name='area',
-            label=_('replica area for segment'),
-            normalizer=lambda value: value.lower(),
+    takes_options = LDAPRetrieve.takes_options + (
+        Bool(
+            'show_segments',
+            label=_('Show segments'),
+            default=True
         ),
     )
-    def execute(self, *keys, **options):
+
+    def get_segments(self, dn, options):
         ldap = self.obj.backend
-
-        dn = self.obj.get_dn(*keys, **options)
-
         try:
             (segs, truncated) = ldap.find_entries(
-                None, ['*'], DN(('cn', options['areaname']),('cn', 'topology'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn),
-                ldap.SCOPE_ONELEVEL
+                'objectclass=iparepltoposegment', ['*'], dn, ldap.SCOPE_ONELEVEL
             )
+
         except errors.NotFound:
-            # If this happens we have some pretty serious problems
-            self.error('No segments found!')
+            segs = []
             pass
 
         segments = []
+        print segs
         for s in segs:
             r = entry_to_dict(s, **options)
             segments.append(r)
+        return segments
 
-        return dict(result=segments,
-                    count=len(segments),
-                    truncated=False,
-                    summary=unicode(_('%d segments found' % len(segments))),
-        )
+    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        assert isinstance(dn, DN)
+
+        if options['show_segments']:
+            entry_attrs['segments'] = self.get_segments(dn, options)
+
+        return dn
 
     def output_for_cli(self, textui, result, *keys, **options):
-        segs = result['result']
 
-        textui.print_plain('Segments for replica %s: ' % options['areaname'])
-        for s in segs:
-            textui.print_plain('--------------------------------')
-            textui.print_plain('Segment %s: ' % s['cn'])
-            textui.print_plain('connecting:  %s <--> %s: ' % (s['iparepltoposegmentleftnode'],s['iparepltoposegmentrightnode']))
+        super(topologysuffix_show, self).output_for_cli(
+            textui, result, *keys, **options)
+
+        if not options['show_segments']:
+            return
+
+        result = result['result']
+        segments = result['segments']
+
+        if not segments:
+            textui.print_plain(u'Replica doesn\'t contain any segment')
+
+        for s in segments:
+            textui.print_plain(u'--------------------------------')
+            textui.print_plain(u'Segment %s: ' % s['cn'])
+            textui.print_plain(u'connecting:  %s <--> %s: ' % (s['iparepltoposegmentleftnode'], s['iparepltoposegmentrightnode']))
             for k in s:
-                textui.print_plain('Segment attr %s: %s ' % (k,s[k]))
+                textui.print_plain(u'Segment attr %s: %s ' % (k, s[k]))
 
 
 @register()
@@ -347,5 +299,4 @@ class topologysuffix_verify(Command):
     def execute(self, *args, **options):
         ldap = self.api.Backend.ldap2
 
-        location = self.api.Command['topology_show'](options['areaname'])
-
+        # location = self.api.Command['topology_show'](options['areaname'])
diff --git a/ipaserver/install/plugins/fix_replica_agreements.py b/ipaserver/install/plugins/fix_replica_agreements.py
index c56d39cda6c16bbe0f7e47cb19d12890a8946e11..1381c7cce46152f3768fdc64ee3ffd8b50de9b49 100644
--- a/ipaserver/install/plugins/fix_replica_agreements.py
+++ b/ipaserver/install/plugins/fix_replica_agreements.py
@@ -50,7 +50,7 @@ class update_replica_attribute_lists(Updater):
 
         for replica in ipa_replicas:
             for desc in replica.get('description', []):
-                 self.log.debug(desc)
+                self.log.debug(desc)
 
             self._update_attr(repl, replica,
                 'nsDS5ReplicatedAttributeList',
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 312eb41ecebb32bffc18ba29688c4c6ad58d49e0..83ed9529ddd5b0087c46fd2023c138442a38bcd3 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -390,7 +390,7 @@ class ReplicationManager(object):
         assert isinstance(replica_binddn, DN)
         dn = self.replica_dn()
         assert isinstance(dn, DN)
-        replica_groupdn = DN(('cn','replication managers'),('cn=etc'),self.suffix)
+        replica_groupdn = DN(('cn', 'replication managers'), ('cn=etc'), self.suffix)
 
         try:
             entry = conn.get_entry(dn)
@@ -733,7 +733,7 @@ class ReplicationManager(object):
         """
 
         rep_dn = self.replica_dn()
-        group_dn = DN(('cn','replication managers'),('cn','etc'),self.suffix)
+        group_dn = DN(('cn', 'replication managers'), ('cn', 'etc'), self.suffix)
         assert isinstance(rep_dn, DN)
         (a_dn, b_dn) = self.get_replica_principal_dns(a, b, retries=100)
         assert isinstance(a_dn, DN)
@@ -750,7 +750,7 @@ class ReplicationManager(object):
             b.modify_s(rep_dn, mod)
         except ldap.TYPE_OR_VALUE_EXISTS:
             pass
-        # Add kerberos principal DNs as valid bindDNs to bindDN group 
+        # Add kerberos principal DNs as valid bindDNs to bindDN group
         try:
             mod = [(ldap.MOD_ADD, "member", b_dn)]
             a.modify_s(group_dn, mod)
-- 
2.1.0

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