From 6aa0b57440ee83ef771c23c5bf81a5edc26e2e67 Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Mon, 1 Aug 2011 16:41:28 +0200 Subject: 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 | 53 ++++++++++++++++++++---------- tests/test_xmlrpc/test_automount_plugin.py | 7 ++-- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/API.txt b/API.txt index 48be5911c..24f763406 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 b7a665361..97d6430d7 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 e0408033d..b27df4fd7 100644 --- a/ipalib/plugins/automount.py +++ b/ipalib/plugins/automount.py @@ -177,6 +177,7 @@ from ipalib import _, ngettext import ldap as _ldap import os +DIRECT_MAP_KEY = u'/-' class automountlocation(LDAPObject): """ @@ -213,7 +214,7 @@ class automountlocation_add(LDAPCreate): # create auto.master for the new location self.api.Command['automountmap_add'](keys[-1], u'auto.master') self.api.Command['automountmap_add_indirect']( - keys[-1], u'auto.direct', key=u'/-' + keys[-1], u'auto.direct', key=DIRECT_MAP_KEY ) return dn @@ -612,6 +613,7 @@ class automountkey(LDAPObject): cli_name='key', label=_('Key'), doc=_('Automount key name.'), + flags=('req_update',), ), IA5Str('automountinformation', cli_name='info', @@ -714,7 +716,7 @@ class automountkey(LDAPObject): ) def get_pk(self, key, info=None): - if info: + if key == DIRECT_MAP_KEY and info: return self.rdn_separator.join((key,info)) else: return key @@ -727,7 +729,7 @@ class automountkey(LDAPObject): entries = self.methods.find(location, map, automountkey=key)['result'] if len(entries) > 0: - if key == u'/-': + if key == DIRECT_MAP_KEY: info = keykw.get('automountinformation') entries = self.methods.find(location, map, **keykw)['result'] if len(entries) > 0: @@ -756,10 +758,7 @@ class automountkey_add(LDAPCreate): def execute(self, *keys, **options): key = options['automountkey'] info = options.get('automountinformation', None) - if key == '/-': - options[self.obj.primary_key.name] = self.obj.get_pk(key, info) - else: - options[self.obj.primary_key.name] = self.obj.get_pk(key, None) + options[self.obj.primary_key.name] = self.obj.get_pk(key, info) options['add_operation'] = True result = super(automountkey_add, self).execute(*keys, **options) result['value'] = options['automountkey'] @@ -858,7 +857,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'), ), @@ -869,18 +868,38 @@ class automountkey_mod(LDAPUpdate): yield key def pre_callback(self, ldap, dn, entry_attrs, *keys, **options): - entry_attrs['automountinformation'] = options['newautomountinformation'] - entry_attrs['description'] = self.obj.get_pk( - options['automountkey'], - options['newautomountinformation']) + if 'newautomountkey' in options: + entry_attrs['automountkey'] = options['newautomountkey'] + if 'newautomountinformation' in options: + entry_attrs['automountinformation'] = options['newautomountinformation'] return dn def execute(self, *keys, **options): - keys += (self.obj.get_pk(options['automountkey'], - options.get('automountinformation', None)), ) - options[self.obj.primary_key.name] = self.obj.get_pk( - options['automountkey'], - options.get('automountinformation', None)) + ldap = self.api.Backend.ldap2 + key = options['automountkey'] + info = options.get('automountinformation', None) + keys += (self.obj.get_pk(key, info), ) + + # handle RDN changes + if 'rename' in options or 'newautomountinformation' in options: + new_key = options.get('rename', key) + new_info = options.get('newautomountinformation', info) + + if new_key == DIRECT_MAP_KEY and not new_info: + # automountinformation attribute of existing LDAP object needs + # to be retrieved so that RDN can be generated + dn = self.obj.get_dn(*keys, **options) + (dn_, entry_attrs_) = ldap.get_entry(dn, ['automountinformation']) + new_info = entry_attrs_.get('automountinformation', [])[0] + + # automounkey attribute cannot be overwritten so that get_dn() + # still works right + options['newautomountkey'] = new_key + + new_rdn = self.obj.get_pk(new_key, new_info) + if new_rdn != keys[-1]: + options['rename'] = new_rdn + result = super(automountkey_mod, self).execute(*keys, **options) result['value'] = options['automountkey'] return result diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py index 0face2ef0..cfea61270 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', '') -- cgit