On Wed, 2011-07-27 at 10:41 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > Fix automountkey-mod so that automountkey attribute is correctly
> > updated. Add this test case to the unit tests.
> >
> > https://fedorahosted.org/freeipa/ticket/1528
> 
> It fixes the problem but I've found another: --key isn't required so if 
> you don't pass it in then a backtrace will occur:
> 
> Traceback (most recent call last):
>    File "/home/rcrit/redhat/freeipa-master/ipaserver/rpcserver.py", line 
> 220, in wsgi_execute
>      result = self.Command[name](*args, **options)
>    File "/home/rcrit/redhat/freeipa-master/ipalib/frontend.py", line 
> 425, in __call__
>      ret = self.run(*args, **options)
>    File "/home/rcrit/redhat/freeipa-master/ipalib/frontend.py", line 
> 731, in run
>      return self.execute(*args, **options)
>    File "/home/rcrit/redhat/freeipa-master/ipalib/plugins/automount.py", 
> line 873, in execute
>      keys += (self.obj.get_pk(options['automountkey'],
> KeyError: 'automountkey'
> 
> Also, automountinformation is already required. This may be a leftover 
> from when we used it in description, this can probably be lifted too.
> 
> rob

Good catch. I fixed this bug too and I also made --newinfo optional so
that automountkey may be just renamed without changing its info
attribute.

I didn't bump up API VERSION as these are either compatible changes or
they caused server internal error.

Martin
>From 5059b9c8dbafbc3158eb5d3a6b4f956dd9bba112 Mon Sep 17 00:00:00 2001
From: Martin Kosek <[email protected]>
Date: Wed, 27 Jul 2011 15:26:57 +0200
Subject: [PATCH] Fix automountkey-mod

Fix automountkey-mod so that automountkey attribute is correctly
updated. Add this test case to the unit tests.

Make automountkey required for automountkey-mod, otherwise it would
cause internal server error.

Make --newinfo optional so that automountkey may be just renamed
without changing its info attribute.

https://fedorahosted.org/freeipa/ticket/1528
---
 API.txt                                    |    8 ++++----
 ipalib/crud.py                             |    4 ++++
 ipalib/plugins/automount.py                |   11 ++++++++---
 tests/test_xmlrpc/test_automount_plugin.py |    7 +++++--
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/API.txt b/API.txt
index 42a212b1ebda0f8e1f45b016c5ba421f16ffb24c..cdbf111eb32c950d29a5d1bc9791e7b14e401c3c 100644
--- a/API.txt
+++ b/API.txt
@@ -103,7 +103,7 @@ command: automountkey_add
 args: 2,7,3
 arg: Str('automountlocationcn', cli_name='automountlocation', label=Gettext('Location', domain='ipa', localedir=None), query=True, required=True)
 arg: IA5Str('automountmapautomountmapname', cli_name='automountmap', label=Gettext('Map', domain='ipa', localedir=None), query=True, required=True)
-option: IA5Str('automountkey', attribute=True, cli_name='key', label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, required=True)
+option: IA5Str('automountkey', attribute=True, cli_name='key', flags=('req_update',), label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, required=True)
 option: IA5Str('automountinformation', attribute=True, cli_name='info', label=Gettext('Mount information', domain='ipa', localedir=None), multivalue=False, required=True)
 option: Str('addattr*', validate_add_attribute, cli_name='addattr', exclude='webui')
 option: Str('setattr*', validate_set_attribute, cli_name='setattr', exclude='webui')
@@ -128,7 +128,7 @@ args: 3,7,4
 arg: Str('automountlocationcn', cli_name='automountlocation', label=Gettext('Location', domain='ipa', localedir=None), query=True, required=True)
 arg: IA5Str('automountmapautomountmapname', cli_name='automountmap', label=Gettext('Map', domain='ipa', localedir=None), query=True, required=True)
 arg: Str('criteria?', noextrawhitespace=False)
-option: IA5Str('automountkey', attribute=True, autofill=False, cli_name='key', label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, query=True, required=False)
+option: IA5Str('automountkey', attribute=True, autofill=False, cli_name='key', flags=('req_update',), label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, query=True, required=False)
 option: IA5Str('automountinformation', attribute=True, autofill=False, cli_name='info', label=Gettext('Mount information', domain='ipa', localedir=None), multivalue=False, query=True, required=False)
 option: Int('timelimit?', autofill=False, flags=['no_display'], label=Gettext('Time Limit', domain='ipa', localedir=None), minvalue=0)
 option: Int('sizelimit?', autofill=False, flags=['no_display'], label=Gettext('Size Limit', domain='ipa', localedir=None), minvalue=0)
@@ -143,12 +143,12 @@ command: automountkey_mod
 args: 2,10,3
 arg: Str('automountlocationcn', cli_name='automountlocation', label=Gettext('Location', domain='ipa', localedir=None), query=True, required=True)
 arg: IA5Str('automountmapautomountmapname', cli_name='automountmap', label=Gettext('Map', domain='ipa', localedir=None), query=True, required=True)
-option: IA5Str('automountkey', attribute=True, autofill=False, cli_name='key', label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, required=False)
+option: IA5Str('automountkey', alwaysask=False, attribute=True, cli_name='key', flags=('req_update',), label=Gettext('Key', domain='ipa', localedir=None), multivalue=False, required=True)
 option: IA5Str('automountinformation', attribute=True, autofill=False, cli_name='info', label=Gettext('Mount information', domain='ipa', localedir=None), multivalue=False, required=False)
 option: Str('addattr*', validate_add_attribute, cli_name='addattr', exclude='webui')
 option: Str('setattr*', validate_set_attribute, cli_name='setattr', exclude='webui')
 option: Flag('rights', autofill=True, default=False, label=Gettext('Rights', domain='ipa', localedir=None))
-option: IA5Str('newautomountinformation', cli_name='newinfo', label=Gettext('New mount information', domain='ipa', localedir=None))
+option: IA5Str('newautomountinformation?', cli_name='newinfo', label=Gettext('New mount information', domain='ipa', localedir=None))
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui', flags=['no_output'])
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui', flags=['no_output'])
 option: Str('version?', exclude='webui', flags=['no_option', 'no_output'])
diff --git a/ipalib/crud.py b/ipalib/crud.py
index b7a665361804b853da4f00de5ab2b2030c9de86c..97d6430d70f74cacdb7d78d5157a4b1a62ea9405 100644
--- a/ipalib/crud.py
+++ b/ipalib/crud.py
@@ -190,6 +190,10 @@ class Update(PKQuery):
                     attribute=True, query=True, required=False,
                     autofill=False, alwaysask=True
                 )
+            elif 'req_update' in option.flags:
+                yield option.clone(
+                    attribute=True, required=True, alwaysask=False,
+                )
             else:
                 yield option.clone(attribute=True, required=False, autofill=False)
         if not self.extra_options_first:
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index d692b2c866f787f46623abb01dad87e560a9c797..8f3bf4d4d7b23e4a46f1782944636cf23253fc0d 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -612,6 +612,7 @@ class automountkey(LDAPObject):
                cli_name='key',
                label=_('Key'),
                doc=_('Automount key name.'),
+               flags=('req_update',),
         ),
         IA5Str('automountinformation',
                cli_name='info',
@@ -841,7 +842,7 @@ class automountkey_mod(LDAPUpdate):
     msg_summary = _('Modified automount key "%(value)s"')
 
     takes_options = LDAPUpdate.takes_options + (
-        IA5Str('newautomountinformation',
+        IA5Str('newautomountinformation?',
                cli_name='newinfo',
                label=_('New mount information'),
         ),
@@ -852,10 +853,14 @@ class automountkey_mod(LDAPUpdate):
             yield key
 
     def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        entry_attrs['automountinformation'] = options['newautomountinformation']
+        if 'newautomountinformation' in options:
+            entry_attrs['automountinformation'] = options['newautomountinformation']
+
         entry_attrs['description'] = self.obj.get_pk(
                                             options['automountkey'],
-                                            options['newautomountinformation'])
+                                            options.get('automountinformation', None))
+        if 'rename' in options:
+            entry_attrs['automountkey'] = options['rename']
         return dn
 
     def execute(self, *keys, **options):
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index c5dd619e254e45739f1abad4f78e7337dc46b096..9866c78f4631afac31a45d45daecd058dc24dc18 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -34,6 +34,7 @@ class test_automount(XMLRPC_test):
     locname = u'testlocation'
     mapname = u'testmap'
     keyname = u'testkey'
+    keyname_rename = u'testkey_rename'
     keyname2 = u'testkey2'
     description = u'description of map'
     info = u'ro'
@@ -127,9 +128,11 @@ class test_automount(XMLRPC_test):
         Test the `xmlrpc.automountkey_mod` method.
         """
         self.key_kw['newautomountinformation'] = self.newinfo
+        self.key_kw['rename'] = self.keyname_rename
         res = api.Command['automountkey_mod'](self.locname, self.mapname, **self.key_kw)['result']
         assert res
-        assert_attr_equal(res, 'automountinformation', 'rw')
+        assert_attr_equal(res, 'automountinformation', self.newinfo)
+        assert_attr_equal(res, 'automountkey', self.keyname_rename)
 
     def test_a_automountmap_mod(self):
         """
@@ -144,7 +147,7 @@ class test_automount(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_del` method.
         """
-        delkey_kw={'automountkey': self.keyname, 'automountinformation' : self.newinfo, 'raw': True}
+        delkey_kw={'automountkey': self.keyname_rename, 'automountinformation' : self.newinfo, 'raw': True}
         res = api.Command['automountkey_del'](self.locname, self.mapname, **delkey_kw)['result']
         assert res
         assert_attr_equal(res, 'failed', '')
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to