Do not use extra command options in the automount plugin

Allowing Commands to be called with ignored unknown options opens the
door to problems, for example with misspelled option names.
Before we start rejecting them, we need to make sure IPA itself does
not use them when it calls commands internally.

This patch does that for the automount plugin and its tests.

Part of the work for https://fedorahosted.org/freeipa/ticket/2509

2 files changed, 20 insertions, 23 deletions

diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 36672942..ac9c9769 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -788,6 +788,8 @@ class automountkey_add(LDAPCreate):
     msg_summary = _('Added automount key "%(value)s"')
 
     def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        options.pop('add_operation', None)
+        options.pop('description', None)
         self.obj.check_key_uniqueness(keys[-2], keys[-1], **options)
         return dn
 
@@ -827,39 +829,35 @@ class automountmap_add_indirect(LDAPCreate):
     )
 
     def execute(self, *keys, **options):
+        parentmap = options.pop('parentmap', None)
+        key = options.pop('key')
         result = self.api.Command['automountmap_add'](*keys, **options)
         try:
-            if options['parentmap'] != u'auto.master':
-                if options['key'].startswith('/'):
-                    raise errors.ValidationError(name='mount', error=_('mount point is relative to parent map, cannot begin with /'))
+            if parentmap != u'auto.master':
+                if key.startswith('/'):
+                    raise errors.ValidationError(name='mount',
+                        error=_('mount point is relative to parent map, '
+                            'cannot begin with /'))
                 location = keys[0]
                 map = keys[1]
                 options['automountinformation'] = map
 
                 # Ensure the referenced map exists
-                self.api.Command['automountmap_show'](
-                    location, options['parentmap']
-                )
+                self.api.Command['automountmap_show'](location, parentmap)
                 # Add a submount key
-                kw = dict(key=options['key'], automountinformation='-fstype=autofs ldap:%s' % map)
                 self.api.Command['automountkey_add'](
-                    location, options['parentmap'],
-                    automountkey=options['key'], **kw
-                )
+                    location, parentmap, automountkey=key, key=key,
+                    automountinformation='-fstype=autofs ldap:%s' % map)
             else: # adding to auto.master
                 # Ensure auto.master exists
-                self.api.Command['automountmap_show'](
-                    keys[0], options['parentmap']
-                )
-                options['automountinformation'] = keys[1]
+                self.api.Command['automountmap_show'](keys[0], parentmap)
                 self.api.Command['automountkey_add'](
-                    keys[0], u'auto.master',
-                    automountkey=options['key'], **options
-                )
-        except Exception, e:
+                    keys[0], u'auto.master', automountkey=key,
+                    automountinformation=keys[1])
+        except Exception:
             # The key exists, drop the map
-            self.api.Command['automountmap_del'](*keys, **options)
-            raise e
+            self.api.Command['automountmap_del'](*keys)
+            raise
         return result
 
 api.register(automountmap_add_indirect)
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index dedd2346..0dd65d87 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -145,7 +145,7 @@ class test_automount(XMLRPC_test):
         """
         Test the `xmlrpc.automountkey_del` method.
         """
-        delkey_kw={'automountkey': self.keyname_rename, 'automountinformation' : self.newinfo, 'raw': True}
+        delkey_kw={'automountkey': self.keyname_rename, 'automountinformation' : self.newinfo}
         res = api.Command['automountkey_del'](self.locname, self.mapname, **delkey_kw)['result']
         assert res
         assert_attr_equal(res, 'failed', '')
@@ -227,8 +227,7 @@ class test_automount_indirect(XMLRPC_test):
     mapname = u'auto.home'
     keyname = u'/home'
     parentmap = u'auto.master'
-    info = u'somehost:/homes'
-    map_kw = {'key': keyname, 'parentmap': parentmap, 'info': info, 'raw': True}
+    map_kw = {'key': keyname, 'parentmap': parentmap, 'raw': True}
     key_kw = {'automountkey': keyname, 'automountinformation': mapname}
 
     def test_0_automountlocation_add(self):