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

@@ -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'])

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:

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)
-            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['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']
+        if 'newautomountkey' in options:
-        entry_attrs['description'] = self.obj.get_pk(
+            entry_attrs['automountkey'] = options['newautomountkey']
-                                            options['automountkey'],
+        if 'newautomountinformation' in options:
-                                            options['newautomountinformation'])
+            entry_attrs['automountinformation'] = options['newautomountinformation']
         return dn
 
     def execute(self, *keys, **options):
-        keys += (self.obj.get_pk(options['automountkey'],
+        ldap = self.api.Backend.ldap2
-                                 options.get('automountinformation', None)), )
+        key = options['automountkey']
-        options[self.obj.primary_key.name] = self.obj.get_pk(
+        info = options.get('automountinformation', None)
-                                            options['automountkey'],
+        keys += (self.obj.get_pk(key, info), )
-                                            options.get('automountinformation', None))
+
+        # 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

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', '')