Part of the work for https://fedorahosted.org/freeipa/ticket/2509
Validation doesn't complain about extra command options, which means
e.g. that misspelling an option's name will cause it to be ignored. This
allows errors in custom RPC clients (such as our test suite) to go
unnoticed.
Since I expect the review to take some time, and the master branch is a
moving target, I'll post this in several patches. This patch is for the
automount plugin; a patch for ACI and a patch to wrap up and enable the
validation will follow later.
--
Petr³
From 9a36039b3093e90da4766be4f9c0b7f4fa37a9c0 Mon Sep 17 00:00:00 2001
From: Petr Viktorin
Date: Mon, 16 Apr 2012 09:29:42 -0400
Subject: [PATCH] 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
---
ipalib/plugins/automount.py| 38 +--
tests/test_xmlrpc/test_automount_plugin.py |5 +--
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 366729425f733b74c6c2ffb3efb1545c39afac2d..ac9c9769f26b31631322e725721ecaf470b80f1c 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 dedd2346aaca52434074e028ff9ed1629d69f9d2..0dd65d8704cb7d2fbaa1e5127db4089ad243308e 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -145,7 +145,7 @@ def test_b_automountkey_del(self):
"""
Test the `xmlrpc.automountkey_del` method.
"""
-delkey_kw=
