On Fri, 2011-08-26 at 09:31 +0200, Martin Kosek wrote:
> On Thu, 2011-08-25 at 09:28 -0400, Rob Crittenden wrote:
> > Martin Kosek wrote:
> > > 1) Add sudorule docstring headline
> > >
> > > 2) Fix naming inconsistency in Sudo plugins help and summaries,
> > >     especially capitalization of Sudo objects - Sudo Rule, Sudo
> > >     Command and Sudo Command Group
> > >
> > > 3) Add missing summaries for sudorule-add-option and
> > >     sudorule-remove-option
> > >
> > > https://fedorahosted.org/freeipa/ticket/1595
> > > https://fedorahosted.org/freeipa/ticket/1596
> > 
> > This breaks compatibility with old clients:
> > 
> > $ ipa sudorule-add-option test2
> > Sudo Option: foo
> > ipa: ERROR: non-public: ValueError: 
> > sudorule_add_option.validate_output(): unexpected keys ['summary'] in 
> > {'result': {'ipasudoopt': (u'foo',), 'cn': (u'test2',), 
> > 'ipaenabledflag': (u'TRUE',)}, 'summary': u'Added option "foo" to Sudo 
> > Rule "test2"'}
> > Traceback (most recent call last):
> >    File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 125, 
> > in execute
> >      result = self.Command[_name](*args, **options)
> >    File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 443, 
> > in __call__
> >      self.validate_output(ret)
> >    File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 895, 
> > in validate_output
> >      nice, extra, output)
> > ValueError: sudorule_add_option.validate_output(): unexpected keys 
> > ['summary'] in {'result': {'ipasudoopt': (u'foo',), 'cn': (u'test2',), 
> > 'ipaenabledflag': (u'TRUE',)}, 'summary': u'Added option "foo" to Sudo 
> > Rule "test2"'}
> > ipa: ERROR: an internal error has occurred
> > 
> 
> Thanks for catching this. I wonder if we should let output param
> validation skip unexpected keys in order to be able to do the change in
> Output + keep backwards compatibility in cases like this one.
> 
> I reworked the patch so that the summaries are printed via
> output_for_cli() - this solves this problem.
> 
> Martin

I had to rework the whole patch because of latest i18n patches. Here it
is, it can be applied to current master.

Martin
>From 6e54673829f464be4e0a10663f59bbac77833876 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 30 Aug 2011 17:10:27 +0200
Subject: [PATCH] Fix sudo help and summaries

1) Add sudorule docstring headline

2) Fix naming inconsistency in Sudo plugins help and summaries,
   especially capitalization of Sudo objects - Sudo Rule, Sudo
   Command and Sudo Command Group

3) Add missing summaries for sudorule-add-option and
   sudorule-remove-option. To keep backward compatibility with
   older clients, just print the missing summary with
   output_for_cli(), don't expand Output.

https://fedorahosted.org/freeipa/ticket/1595
https://fedorahosted.org/freeipa/ticket/1596
---
 ipalib/plugins/sudocmd.py                     |   18 +++++-----
 ipalib/plugins/sudocmdgroup.py                |   40 ++++++++++++------------
 ipalib/plugins/sudorule.py                    |   40 ++++++++++++++++---------
 tests/test_xmlrpc/test_sudocmd_plugin.py      |    8 ++--
 tests/test_xmlrpc/test_sudocmdgroup_plugin.py |   22 +++++++-------
 5 files changed, 70 insertions(+), 58 deletions(-)

diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py
index 18190f9d95361d15ab59d748dabcc13624a27e8e..42068edea3c51804be9ee5919934462afbee578f 100644
--- a/ipalib/plugins/sudocmd.py
+++ b/ipalib/plugins/sudocmd.py
@@ -97,36 +97,36 @@ class sudocmd(LDAPObject):
 api.register(sudocmd)
 
 class sudocmd_add(LDAPCreate):
-    __doc__ = _('Create new sudo command.')
+    __doc__ = _('Create new Sudo Command.')
 
-    msg_summary = _('Added sudo command "%(value)s"')
+    msg_summary = _('Added Sudo Command "%(value)s"')
 
 api.register(sudocmd_add)
 
 class sudocmd_del(LDAPDelete):
-    __doc__ = _('Delete sudo command.')
+    __doc__ = _('Delete Sudo Command.')
 
-    msg_summary = _('Deleted sudo command "%(value)s"')
+    msg_summary = _('Deleted Sudo Command "%(value)s"')
 
 api.register(sudocmd_del)
 
 class sudocmd_mod(LDAPUpdate):
-    __doc__ = _('Modify command.')
+    __doc__ = _('Modify Sudo Command.')
 
-    msg_summary = _('Modified sudo command "%(value)s"')
+    msg_summary = _('Modified Sudo Command "%(value)s"')
 
 api.register(sudocmd_mod)
 
 class sudocmd_find(LDAPSearch):
-    __doc__ = _('Search for commands.')
+    __doc__ = _('Search for Sudo Commands.')
 
     msg_summary = ngettext(
-        '%(count)d sudo command matched', '%(count)d sudo command matched', 0
+        '%(count)d Sudo Command matched', '%(count)d Sudo Commands matched', 0
     )
 
 api.register(sudocmd_find)
 
 class sudocmd_show(LDAPRetrieve):
-    __doc__ = _('Display sudo command.')
+    __doc__ = _('Display Sudo Command.')
 
 api.register(sudocmd_show)
diff --git a/ipalib/plugins/sudocmdgroup.py b/ipalib/plugins/sudocmdgroup.py
index e7f1fd3be56c301de84d6d0389ebe7d1bc4d5d38..2124b84a530cdaa2f8cb9754de88bd04d9961dd0 100644
--- a/ipalib/plugins/sudocmdgroup.py
+++ b/ipalib/plugins/sudocmdgroup.py
@@ -23,25 +23,25 @@ from ipalib.plugins.baseldap import *
 from ipalib import _, ngettext
 
 __doc__ = _("""
-Groups of Sudo commands
+Groups of Sudo Commands
 
-Manage groups of Sudo commands.
+Manage groups of Sudo Commands.
 
 EXAMPLES:
 
- Add a new Sudo command group:
+ Add a new Sudo Command Group:
    ipa sudocmdgroup-add --desc='administrators commands' admincmds
 
- Remove a Sudo command group:
+ Remove a Sudo Command Group:
    ipa sudocmdgroup-del admincmds
 
- Manage Sudo command group membership, commands:
+ Manage Sudo Command Group membership, commands:
    ipa sudocmdgroup-add-member --sudocmds=/usr/bin/less,/usr/bin/vim admincmds
 
- Manage Sudo command group membership, commands:
+ Manage Sudo Command Group membership, commands:
    ipa group-remove-member --sudocmds=/usr/bin/less admincmds
 
- Show a Sudo command group:
+ Show a Sudo Command Group:
    ipa group-show localadmins
 """)
 
@@ -49,7 +49,7 @@ topic = ('sudo', _('commands for controlling sudo configuration'))
 
 class sudocmdgroup(LDAPObject):
     """
-    Sudo Group object.
+    Sudo Command Group object.
     """
     container_dn = api.env.container_sudocmdgroup
     object_name = _('sudo command group')
@@ -92,53 +92,53 @@ api.register(sudocmdgroup)
 
 
 class sudocmdgroup_add(LDAPCreate):
-    __doc__ = _('Create new sudo command group.')
+    __doc__ = _('Create new Sudo Command Group.')
 
-    msg_summary = _('Added sudo command group "%(value)s"')
+    msg_summary = _('Added Sudo Command Group "%(value)s"')
 
 api.register(sudocmdgroup_add)
 
 
 class sudocmdgroup_del(LDAPDelete):
-    __doc__ = _('Delete sudo command group.')
+    __doc__ = _('Delete Sudo Command Group.')
 
-    msg_summary = _('Deleted sudo command group "%(value)s"')
+    msg_summary = _('Deleted Sudo Command Group "%(value)s"')
 
 api.register(sudocmdgroup_del)
 
 
 class sudocmdgroup_mod(LDAPUpdate):
-    __doc__ = _('Modify group.')
+    __doc__ = _('Modify Sudo Command Group.')
 
-    msg_summary = _('Modified sudo command group "%(value)s"')
+    msg_summary = _('Modified Sudo Command Group "%(value)s"')
 
 api.register(sudocmdgroup_mod)
 
 
 class sudocmdgroup_find(LDAPSearch):
-    __doc__ = _('Search for sudo command groups.')
+    __doc__ = _('Search for Sudo Command Groups.')
 
     msg_summary = ngettext(
-        '%(count)d sudo command group matched',
-        '%(count)d sudo command groups matched', 0
+        '%(count)d Sudo Command Group matched',
+        '%(count)d Sudo Command Groups matched', 0
     )
 
 api.register(sudocmdgroup_find)
 
 
 class sudocmdgroup_show(LDAPRetrieve):
-    __doc__ = _('Display sudo command group.')
+    __doc__ = _('Display Sudo Command Group.')
 
 api.register(sudocmdgroup_show)
 
 
 class sudocmdgroup_add_member(LDAPAddMember):
-    __doc__ = _('Add members to sudo command group.')
+    __doc__ = _('Add members to Sudo Command Group.')
 
 api.register(sudocmdgroup_add_member)
 
 
 class sudocmdgroup_remove_member(LDAPRemoveMember):
-    __doc__ = _('Remove members from sudo command group.')
+    __doc__ = _('Remove members from Sudo Command Group.')
 
 api.register(sudocmdgroup_remove_member)
diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py
index a169452a726e4408f6c5c6a0350a88dc9205bdfa..3ecb2e7dcb8faba075eded97869810c3f30317ae 100644
--- a/ipalib/plugins/sudorule.py
+++ b/ipalib/plugins/sudorule.py
@@ -23,6 +23,8 @@ from ipalib.plugins.baseldap import *
 from ipalib import _, ngettext
 
 __doc__ = _("""
+Sudo Rules
+
 Sudo (su "do") allows a system administrator to delegate authority to
 give certain users (or groups of users) the ability to run some (or all)
 commands as root or another user while providing an audit trail of the
@@ -64,7 +66,7 @@ def validate_runasextgroup(ugettext, value):
 
 class sudorule(LDAPObject):
     """
-    Sudo Rule management
+    Sudo Rule object.
     """
     container_dn = api.env.container_sudorule
     object_name = _('sudo rule')
@@ -208,11 +210,11 @@ class sudorule_add(LDAPCreate):
     __doc__ = _('Create new Sudo Rule.')
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        # Sudo rules are enabled by default
+        # Sudo Rules are enabled by default
         entry_attrs['ipaenabledflag'] = 'TRUE'
         return dn
 
-    msg_summary = _('Added sudo rule "%(value)s"')
+    msg_summary = _('Added Sudo Rule "%(value)s"')
 
 api.register(sudorule_add)
 
@@ -220,7 +222,7 @@ api.register(sudorule_add)
 class sudorule_del(LDAPDelete):
     __doc__ = _('Delete Sudo Rule.')
 
-    msg_summary = _('Deleted sudo rule "%(value)s"')
+    msg_summary = _('Deleted Sudo Rule "%(value)s"')
 
 api.register(sudorule_del)
 
@@ -228,7 +230,7 @@ api.register(sudorule_del)
 class sudorule_mod(LDAPUpdate):
     __doc__ = _('Modify Sudo Rule.')
 
-    msg_summary = _('Modified sudo rule "%(value)s"')
+    msg_summary = _('Modified Sudo Rule "%(value)s"')
 
 api.register(sudorule_mod)
 
@@ -237,7 +239,7 @@ class sudorule_find(LDAPSearch):
     __doc__ = _('Search for Sudo Rule.')
 
     msg_summary = ngettext(
-        '%(count)d sudo rule matched', '%(count)d sudo rules matched', 0
+        '%(count)d Sudo Rule matched', '%(count)d Sudo Rules matched', 0
     )
 
 api.register(sudorule_find)
@@ -250,7 +252,7 @@ api.register(sudorule_show)
 
 
 class sudorule_enable(LDAPQuery):
-    __doc__ = _('Enable a Sudo rule.')
+    __doc__ = _('Enable a Sudo Rule.')
 
     def execute(self, cn):
         ldap = self.obj.backend
@@ -268,14 +270,13 @@ class sudorule_enable(LDAPQuery):
         return dict(result=True)
 
     def output_for_cli(self, textui, result, cn):
-        textui.print_name(self.name)
-        textui.print_dashed('Enabled Sudo rule "%s".' % cn)
+        textui.print_dashed(_('Enabled Sudo Rule "%s"') % cn)
 
 api.register(sudorule_enable)
 
 
 class sudorule_disable(LDAPQuery):
-    __doc__ = _('Disable a Sudo rule.')
+    __doc__ = _('Disable a Sudo Rule.')
 
     def execute(self, cn):
         ldap = self.obj.backend
@@ -293,8 +294,7 @@ class sudorule_disable(LDAPQuery):
         return dict(result=True)
 
     def output_for_cli(self, textui, result, cn):
-        textui.print_name(self.name)
-        textui.print_dashed('Disabled Sudo rule "%s".' % cn)
+        textui.print_dashed(_('Disabled Sudo Rule "%s"') % cn)
 
 api.register(sudorule_disable)
 
@@ -609,7 +609,7 @@ api.register(sudorule_remove_runasgroup)
 
 
 class sudorule_add_option(LDAPQuery):
-    __doc__ = _('Add an option to the Sudo rule.')
+    __doc__ = _('Add an option to the Sudo Rule.')
 
     takes_options = (
         Str('ipasudoopt',
@@ -650,11 +650,18 @@ class sudorule_add_option(LDAPQuery):
 
         return dict(result=entry_attrs)
 
+    def output_for_cli(self, textui, result, cn, **options): 
+        textui.print_dashed(_('Added option "%s" to Sudo Rule "%s"') % \
+                (options['ipasudoopt'], cn))
+        super(sudorule_add_option, self).output_for_cli(textui, result, cn, options)
+
+
+
 api.register(sudorule_add_option)
 
 
 class sudorule_remove_option(LDAPQuery):
-    __doc__ = _('Remove an option from Sudo rule.')
+    __doc__ = _('Remove an option from Sudo Rule.')
 
     takes_options = (
         Str('ipasudoopt',
@@ -698,4 +705,9 @@ class sudorule_remove_option(LDAPQuery):
 
         return dict(result=entry_attrs)
 
+    def output_for_cli(self, textui, result, cn, **options): 
+        textui.print_dashed(_('Removed option "%s" from Sudo Rule "%s"') % \
+                (options['ipasudoopt'], cn))
+        super(sudorule_remove_option, self).output_for_cli(textui, result, cn, options)
+
 api.register(sudorule_remove_option)
diff --git a/tests/test_xmlrpc/test_sudocmd_plugin.py b/tests/test_xmlrpc/test_sudocmd_plugin.py
index a19fefc735d5e1f01c5f2365564eb26c465f67d1..cbbd26cd7061ee73c44f691692072daaf6f84f50 100644
--- a/tests/test_xmlrpc/test_sudocmd_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmd_plugin.py
@@ -67,7 +67,7 @@ class test_sudocmd(Declarative):
             ),
             expected=dict(
                 value=sudocmd1,
-                summary=u'Added sudo command "%s"' % sudocmd1,
+                summary=u'Added Sudo Command "%s"' % sudocmd1,
                 result=dict(
                     dn=lambda x: DN(x) == \
                         DN(('sudocmd',sudocmd1),('cn','sudocmds'),('cn','sudo'),
@@ -115,7 +115,7 @@ class test_sudocmd(Declarative):
             expected=dict(
                 count=1,
                 truncated=False,
-                summary=u'1 sudo command matched',
+                summary=u'1 Sudo Command matched',
                 result=[
                     dict(
                         dn=lambda x: DN(x) == \
@@ -135,7 +135,7 @@ class test_sudocmd(Declarative):
                 description=u'Updated sudo command 1')),
             expected=dict(
                 value=sudocmd1,
-                summary=u'Modified sudo command "%s"' % sudocmd1,
+                summary=u'Modified Sudo Command "%s"' % sudocmd1,
                 result=dict(
                     sudocmd=[sudocmd1],
                     description=[u'Updated sudo command 1'],
@@ -166,7 +166,7 @@ class test_sudocmd(Declarative):
             command=('sudocmd_del', [sudocmd1], {}),
             expected=dict(
                 value=sudocmd1,
-                summary=u'Deleted sudo command "%s"' % sudocmd1,
+                summary=u'Deleted Sudo Command "%s"' % sudocmd1,
                 result=dict(failed=u''),
             ),
         ),
diff --git a/tests/test_xmlrpc/test_sudocmdgroup_plugin.py b/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
index dd89c5d17aab2201f454a0bf5bbd2596cf558e43..8a534b2bf9f8f73c6304555a2bef3c52a367e626 100644
--- a/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmdgroup_plugin.py
@@ -47,7 +47,7 @@ class test_sudocmdgroup(Declarative):
             ),
             expected=dict(
                 value=sudocmd1,
-                summary=u'Added sudo command "%s"' % sudocmd1,
+                summary=u'Added Sudo Command "%s"' % sudocmd1,
                 result=dict(
                     objectclass=objectclasses.sudocmd,
                     sudocmd=[u'/usr/bin/sudotestcmd1'],
@@ -110,7 +110,7 @@ class test_sudocmdgroup(Declarative):
             ),
             expected=dict(
                 value=sudocmdgroup1,
-                summary=u'Added sudo command group "testsudocmdgroup1"',
+                summary=u'Added Sudo Command Group "testsudocmdgroup1"',
                 result=dict(
                     cn=[sudocmdgroup1],
                     description=[u'Test desc 1'],
@@ -162,7 +162,7 @@ class test_sudocmdgroup(Declarative):
                     cn=[sudocmdgroup1],
                     description=[u'New desc 1'],
                 ),
-                summary=u'Modified sudo command group "testsudocmdgroup1"',
+                summary=u'Modified Sudo Command Group "testsudocmdgroup1"',
                 value=sudocmdgroup1,
             ),
         ),
@@ -200,7 +200,7 @@ class test_sudocmdgroup(Declarative):
                         description=[u'New desc 1'],
                     ),
                 ],
-                summary=u'1 sudo command group matched',
+                summary=u'1 Sudo Command Group matched',
             ),
         ),
 
@@ -238,7 +238,7 @@ class test_sudocmdgroup(Declarative):
             ),
             expected=dict(
                 value=sudocmdgroup2,
-                summary=u'Added sudo command group "testsudocmdgroup2"',
+                summary=u'Added Sudo Command Group "testsudocmdgroup2"',
                 result=dict(
                     cn=[sudocmdgroup2],
                     description=[u'Test desc 2'],
@@ -290,7 +290,7 @@ class test_sudocmdgroup(Declarative):
                     cn=[sudocmdgroup2],
                     description=[u'New desc 2'],
                 ),
-                summary=u'Modified sudo command group "testsudocmdgroup2"',
+                summary=u'Modified Sudo Command Group "testsudocmdgroup2"',
                 value=sudocmdgroup2,
             ),
         ),
@@ -328,7 +328,7 @@ class test_sudocmdgroup(Declarative):
                         description=[u'New desc 2'],
                     ),
                 ],
-                summary=u'1 sudo command group matched',
+                summary=u'1 Sudo Command Group matched',
             ),
         ),
 
@@ -337,7 +337,7 @@ class test_sudocmdgroup(Declarative):
             desc='Search for all sudocmdgroups',
             command=('sudocmdgroup_find', [], {}),
             expected=dict(
-                summary=u'2 sudo command groups matched',
+                summary=u'2 Sudo Command Groups matched',
                 count=2,
                 truncated=False,
                 result=[
@@ -482,7 +482,7 @@ class test_sudocmdgroup(Declarative):
             expected=dict(
                 result=dict(failed=u''),
                 value=sudocmdgroup1,
-                summary=u'Deleted sudo command group "testsudocmdgroup1"',
+                summary=u'Deleted Sudo Command Group "testsudocmdgroup1"',
             )
         ),
 
@@ -517,7 +517,7 @@ class test_sudocmdgroup(Declarative):
             expected=dict(
                 result=dict(failed=u''),
                 value=sudocmdgroup2,
-                summary=u'Deleted sudo command group "testsudocmdgroup2"',
+                summary=u'Deleted Sudo Command Group "testsudocmdgroup2"',
             )
         ),
 
@@ -552,7 +552,7 @@ class test_sudocmdgroup(Declarative):
             expected=dict(
                 result=dict(failed=u''),
                 value=sudocmd1,
-                summary=u'Deleted sudo command "%s"' % sudocmd1,
+                summary=u'Deleted Sudo Command "%s"' % sudocmd1,
             )
         ),
 
-- 
1.7.6

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

Reply via email to