Thanks for the review. New patch attached to be applied on top of all previous patches. Please see comments below.

On 3/6/2015 3:53 PM, Jan Cholasta wrote:
Patch 353:

1) Please follow PEP8 in new code.

The pep8 tool reports these errors in existing files:

./ipalib/constants.py:98:80: E501 line too long (84 > 79 characters)
./ipalib/plugins/baseldap.py:1527:80: E501 line too long (81 > 79
characters)
./ipalib/plugins/user.py:915:80: E501 line too long (80 > 79 characters)

as well as many errors in the files this patch adds.

For some reason pylint keeps crashing during build so I cannot run it for all files. I'm fixing the errors that I can see. If you see other errors please let me know while I'm still trying to figure out the problem.

Is there an existing ticket for fixing PEP8 errors? Let's use that for fixing the errors in the existing code.

2) Pylint reports the following error:

ipatests/test_xmlrpc/test_vault_plugin.py:153:
[E0102(function-redefined), test_vault] class already defined line 27)

Fixed.

3) The container_vault config option should be renamed to
container_vaultcontainer, as it is used in the vaultcontainer plugin,
not the vault plugin.

It was named container_vault because it defines the DN for of the subtree that contains all vault-related entries. I moved the base_dn variable from vaultcontainer object to the vault object for clarity.

4) The vault object should be child of the vaultcontainer object.

Not only is this correct from the object model perspective, but it would
also make all the container_id hacks go away.

It's a bit difficult because it will affect how the container & vault ID's are represented on the CLI.

In the design the container ID would be a single value like this:

  $ ipa vault-add /services/server.example.com/HTTP

And if the vault ID is relative (without initial slash), it will be appended to the user's private container (i.e. /users/<username>/):

  $ ipa vault-add PrivateVault

The implementation is not complete yet. Currently it accepts this format:

  $ ipa vault-add <vault name> [--container <container ID>]

and I'm still planning to add this:

  $ ipa vault-add <vault ID>

If the vault must be a child of vaultcontainer, and the vaultcontainer must be a child of a vaultcontainer, does it mean the vault ID would have to be split into separate arguments like this?

  $ ipa vaultcontainer-add services server.example.com HTTP

If that's the case we'd lose the ability to specify a relative vault ID.

5) When specifying param flags, use set literals.

This is especially wrong, because it's not a tuple, but a string in
parentheses:

+            flags=('virtual_attribute'),

Fixed.

6) The `container` param of vault should actually be an option in
vault_* commands.

Also it should be renamed to `container_id`, for consistency with
vaultcontainer.

Fixed. It was actually made to be consistent with the 'parent' attribute in the vaultcontainer class. Now the 'parent' has been renamed to 'parent_id' as well.

7) The `vault_id` param of vault should have "no_option" in flags, since
it is output-only.

Fixed.

8) Don't translate docstrings where not necessary:

+    def get_dn(self, *keys, **options):
+        __doc__ = _("""
+        Generates vault DN from vault ID.
+        """)

Only plugin modules and classes should have translated docstrings.

Fixed.

9) This looks wrong in vault.get_dn() and vaultcontainer.get_dn():

+        name = None
+        if keys:
+            name = keys[0]

Primary key of the object should always be set, so the if statement
should not be there.

Fixed.

Also, primary key of any given object is always last in "keys", so use
keys[-1] instead of keys[0].

Fixed.

10) Use "self.api" instead of "api" to access the API in plugins.

Fixed.

11) No clever optimizations like this please:

+        # vault DN cannot be the container base DN
+        if len(dn) == len(api.Object.vaultcontainer.base_dn):
+            raise ValueError('Invalid vault DN: %s' % dn)

Compare the DNs by value instead.

Actually the DN values have already been compared in the code right above it:

    # make sure the DN is a vault DN
    if not dn.endswith(self.api.Object.vaultcontainer.base_dn):
        raise ValueError('Invalid vault DN: %s' % dn)

This code confirms that the incoming vault DN is within the vault subtree. After that, the DN length comparison above is just to make sure the incoming vault DN is not the root of the vault subtree itself. It doesn't need to compare the values again.

12) vault.split_id() is not used anywhere.

Removed.

13) Bytes is not base64-encoded data:

+        Bytes('data?',
+            cli_name='data',
+            doc=_('Base-64 encoded binary data to archive'),
+        ),

It is base64-encoded in the CLI, but on the API level it is not. The doc
should say just "Binary data to archive".

Fixed.

14) Use File instead of Str for input files:

+        Str('in?',
+            cli_name='in',
+            doc=_('File containing data to archive'),
+        ),

The File type doesn't work with binary files because it tries to decode the content.

15) Use MutuallyExclusiveError instead of ValidationError when there are
mutually exclusive options specified.

Fixed.

16) You do way too much stuff in vault_add.forward(). Only code that
must be done on the client needs to be there, i.e. handling of the
"data", "text" and "in" options.

The vault_archive call must be in vault_add.execute(), otherwise a) we
will be making 2 RPC calls from the client and b) it won't be called at
all when api.env.in_server is True.

This is done by design. The vault_add.forward() generates the salt and the keys. The vault_archive.forward() will encrypt the data. These operations have to be done on the client side to secure the transport of the data from the client through the server and finally to KRA. This mechanism prevents the server from looking at the unencrypted data.

The add & archive combination was added for convenience, not for optimization. This way you would be able to archive data into a new vault using a single command. Without this, you'd have to execute two separate commands: add & archive, which will result in 2 RPC calls anyway.

17) Why are vaultcontainer objects automatically created in vault_add?

If you have to automatically create them, you also have to automatically
delete them when the command fails. But that's a hassle, so I would just
not create them automatically.

The vaultcontainer is created automatically to provide a private container (i.e. /users/<username>/) for the each user if they need it. Without this, the admin will have to create the container manually first before a user can create a vault, which would be an unreasonable requirement. If the vault_add fails, it's ok to leave the private container intact because it can be used again if the user tries to create a vault again later and it will not affect other users. If the user is deleted, the private container will be deleted too.

The code was fixed to create the container only if they are adding a vault/vault container into the user's private container. If they are adding into other container, the container must already exist.

18) Why are vaultcontainer objects automatically created in vault_find?

This is just plain wrong and has to be removed, now.

The code was supposed to create the user's private container like in #17, but the behavior has been changed. If the container being searched is the user's private container, it will ignore the container not found error and return zero results as if the private container already exists. For other containers the container must already exist. For this to work I had to add a handle_not_found() into LDAPSearch so the plugins can customize the proper search response for the missing private container.

19) What is the reason behind all the json stuff in vault_transport_cert?

vault_transport_cert.__json__() is exactly the same as
Command.__json__() and hence redundant.

Removed. It should've been cleaned up.

20) Are vault_transport_cert, vault_archive and vault_retrieve supposed
to be runnable by users? If not, add "NO_CLI = True" to the class
definition.

Yes. These commands are available if the users want to retrieve the transport cert for other purposes, or archive/retrieve data to/from the vault as a whole instead of individual secrets.

21) vault_archive is not a retrieve operation, it should be based on
LDAPUpdate instead of LDAPRetrieve. Or Command actually, since it does
not do anything with LDAP. The same applies to vault_retrieve.

The vault_archive does not actually modify the LDAP entry because it stores the data in KRA. It is actually an LDAPRetrieve operation because it needs to get the vault info before it can perform the archival operation. Same thing with vault_retrieve.

22) vault_archive will break with binary data that is not UTF-8 encoded
text.

This is where it occurs:

+        vault_data[u'data'] = unicode(data)

Generally, don't use unicode() on str values and str() on unicode values
directly, always use .decode() and .encode().

It needs to be a Unicode because json.dumps() doesn't work with binary data. Fixed by adding base-64 encoding.

23) Since vault containers are nested, the vaultcontainer object should
be child of itself.

There is no support for nested objects in the framework, but it
shouldn't be too hard to do anyway.

See #4.

24) Instead of:

+        while len(dn) > len(self.base_dn):
+
+            rdn = dn[0]
...
+            dn = DN(*dn[1:])

you can do:

+        for rdn in dn[:-len(self.base_dn)]:
...

Fixed.

25) Why are parent vaultcontainer objects automatically created in
vaultcontainer_add?

See #17.

26) Instead of the delete_entry refactoring in baseldap and
vaultcontainer_add, you can put this in vaultcontainer_add's pre_callback:

     try:
         ldap.get_entries(dn, scope=ldap.SCOPE_ONELEVEL, attrs_list=[])
     except errors.NotFound:
         pass
     else:
         if not options.get('force', False):
             raise errors.NotAllowedOnNonLeaf()

I suppose you meant vaultcontainer_del. Fixed, but this will generate an additional search for each delete.

I'm leaving the changes baseldap because it may be useful later and it doesn't change the behavior of the current code.

27) Why are parent vaultcontainer objects automatically created in
vaultcontainer_find?

See #18.

28) The vault and vaultcontainer plugins seem to be pretty similar, I
think it would make sense to put common stuff in a base class and
inherit vault and vaultcontainer from that.

I plan to refactor the common code later. Right now the focus is to get the functionality working correctly first.

More later.

Honza



--
Endi S. Dewata
>From 6c3d2ea06089a9e91741d57c412f6efbaee94a83 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edew...@redhat.com>
Date: Mon, 9 Mar 2015 12:09:20 -0400
Subject: [PATCH] Vault improvements.

The vault plugins have been modified to clean up the code, to fix
some issues, and to improve error handling. The LDAPCreate and
LDAPSearch classes have been refactored to allow subclasses to
provide custom error handling. The test scripts have been updated
accordingly.

https://fedorahosted.org/freeipa/ticket/3872
---
 API.txt                                            |  50 ++--
 ipalib/plugins/baseldap.py                         |  35 +--
 ipalib/plugins/user.py                             |   6 +-
 ipalib/plugins/vault.py                            | 269 ++++++++++-----------
 ipalib/plugins/vaultcontainer.py                   | 224 +++++++++--------
 ipalib/plugins/vaultsecret.py                      | 108 ++++-----
 ipatests/test_xmlrpc/test_vault_plugin.py          |   2 +-
 ipatests/test_xmlrpc/test_vaultcontainer_plugin.py |  24 +-
 ipatests/test_xmlrpc/test_vaultsecret_plugin.py    |   2 +-
 9 files changed, 365 insertions(+), 355 deletions(-)

diff --git a/API.txt b/API.txt
index 
ffbffa78cde372d5c7027b758be58bf07caebbc6..3a741755ab3e15e0175599a16a090b04d46d6be8
 100644
--- a/API.txt
+++ b/API.txt
@@ -4518,7 +4518,7 @@ args: 1,20,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container', attribute=False, cli_name='container', 
multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
 option: Str('escrow_public_key_file?', cli_name='escrow_public_key_file')
@@ -4543,7 +4543,7 @@ command: vault_add_member
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
@@ -4556,7 +4556,7 @@ command: vault_add_owner
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
@@ -4569,7 +4569,7 @@ command: vault_archive
 args: 1,15,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Bytes('encryption_key?', cli_name='encryption_key')
 option: Str('in?', cli_name='in')
@@ -4589,7 +4589,7 @@ output: PrimaryKey('value', None, None)
 command: vault_del
 args: 1,3,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=True, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Str('version?', exclude='webui')
 output: Output('result', <type 'dict'>, None)
@@ -4600,7 +4600,7 @@ args: 1,15,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='vault_name', 
maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, 
query=True, required=False)
-option: Str('container', attribute=False, autofill=False, 
cli_name='container', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', 
query=True, required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', 
multivalue=False, query=True, required=False)
 option: Bytes('ipaescrowpublickey', attribute=True, autofill=False, 
cli_name='escrow_public_key', multivalue=False, query=True, required=False)
 option: Bytes('ipapublickey', attribute=True, autofill=False, 
cli_name='public_key', multivalue=False, query=True, required=False)
@@ -4622,7 +4622,7 @@ args: 1,15,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container', attribute=False, autofill=False, 
cli_name='container', multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', 
required=False)
+option: Str('container_id?', cli_name='container_id')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', 
multivalue=False, required=False)
 option: Bytes('ipaescrowpublickey', attribute=True, autofill=False, 
cli_name='escrow_public_key', multivalue=False, required=False)
@@ -4642,7 +4642,7 @@ command: vault_remove_member
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
@@ -4655,7 +4655,7 @@ command: vault_remove_owner
 args: 1,7,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
@@ -4668,7 +4668,7 @@ command: vault_retrieve
 args: 1,16,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('escrow_private_key?', cli_name='escrow_private_key')
 option: Str('escrow_private_key_file?', cli_name='escrow_private_key_file')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
@@ -4690,7 +4690,7 @@ command: vault_show
 args: 1,6,3
 arg: Str('cn', attribute=True, cli_name='vault_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Flag('rights', autofill=True, default=False)
@@ -4711,7 +4711,7 @@ option: Flag('all', autofill=True, cli_name='all', 
default=False, exclude='webui
 option: Str('container_id', attribute=False, cli_name='container_id', 
multivalue=False, required=False)
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, 
required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, cli_name='parent', multivalue=False, 
pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('version?', exclude='webui')
@@ -4724,7 +4724,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', 
maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4737,7 +4737,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', 
maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4749,7 +4749,7 @@ args: 1,4,3
 arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, 
multivalue=True, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Flag('force?', autofill=True, default=False)
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Str('version?', exclude='webui')
 output: Output('result', <type 'dict'>, None)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
@@ -4762,7 +4762,7 @@ option: Str('cn', attribute=True, autofill=False, 
cli_name='container_name', max
 option: Str('container_id', attribute=False, autofill=False, 
cli_name='container_id', multivalue=False, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', 
multivalue=False, query=True, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, autofill=False, cli_name='parent', 
multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', query=True, required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
@@ -4781,7 +4781,7 @@ option: Str('container_id', attribute=False, 
autofill=False, cli_name='container
 option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: Str('description', attribute=True, autofill=False, cli_name='desc', 
multivalue=False, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent', attribute=False, autofill=False, cli_name='parent', 
multivalue=False, pattern='^[a-zA-Z0-9_.-/]+$', required=False)
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
@@ -4795,7 +4795,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', 
maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4808,7 +4808,7 @@ arg: Str('cn', attribute=True, cli_name='container_name', 
maxlength=255, multiva
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Str('group*', alwaysask=True, cli_name='groups', csv=True)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('user*', alwaysask=True, cli_name='users', csv=True)
 option: Str('version?', exclude='webui')
@@ -4820,7 +4820,7 @@ args: 1,6,3
 arg: Str('cn', attribute=True, cli_name='container_name', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('parent?', cli_name='parent')
+option: Str('parent_id?', cli_name='parent_id')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('version?', exclude='webui')
@@ -4832,7 +4832,7 @@ args: 2,12,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, 
pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description?', cli_name='desc')
 option: Str('in?', cli_name='in')
@@ -4851,7 +4851,7 @@ args: 2,8,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, 
pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('password?', cli_name='password')
 option: Str('password_file?', cli_name='password_file')
 option: Bytes('private_key?', cli_name='private_key')
@@ -4866,7 +4866,7 @@ args: 2,12,4
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, 
pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data', attribute=True, autofill=False, cli_name='data', 
multivalue=False, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', 
multivalue=False, query=True, required=False)
 option: Str('password?', cli_name='password')
@@ -4886,7 +4886,7 @@ args: 2,12,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, 
pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Bytes('data?', cli_name='data')
 option: Str('description?', cli_name='desc')
 option: Str('in?', cli_name='in')
@@ -4905,7 +4905,7 @@ args: 2,11,3
 arg: Str('vaultcn', cli_name='vault', maxlength=255, multivalue=False, 
pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=True)
 arg: Str('secret_name', attribute=True, cli_name='secret', maxlength=255, 
multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, 
required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, 
exclude='webui')
-option: Str('container?', cli_name='container')
+option: Str('container_id?', cli_name='container_id')
 option: Str('out?', cli_name='out')
 option: Str('password?', cli_name='password')
 option: Str('password_file?', cli_name='password_file')
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 
d693709ac1ba7ddb3c559199c199039b6f8bd9ac..fceaf95f42bef5fa71cbedeb291bd68d2919bc5a
 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1152,19 +1152,7 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
         try:
             self._exc_wrapper(keys, options, ldap.add_entry)(entry_attrs)
         except errors.NotFound:
-            parent = self.obj.parent_object
-            if parent:
-                raise errors.NotFound(
-                    reason=self.obj.parent_not_found_msg % {
-                        'parent': keys[-2],
-                        'oname': self.api.Object[parent].object_name,
-                    }
-                )
-            raise errors.NotFound(
-                reason=self.obj.container_not_found_msg % {
-                    'container': self.obj.container_dn,
-                }
-            )
+            self.handle_not_found(*keys, **options)
         except errors.DuplicateEntry:
             self.obj.handle_duplicate_entry(*keys)
 
@@ -1213,6 +1201,21 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
     def exc_callback(self, keys, options, exc, call_func, *call_args, 
**call_kwargs):
         raise exc
 
+    def handle_not_found(self, *args, **options):
+        parent = self.obj.parent_object
+        if parent:
+            raise errors.NotFound(
+                reason=self.obj.parent_not_found_msg % {
+                    'parent': args[-2],
+                    'oname': self.api.Object[parent].object_name,
+                }
+            )
+        raise errors.NotFound(
+            reason=self.obj.container_not_found_msg % {
+                'container': self.obj.container_dn,
+            }
+        )
+
     def interactive_prompt_callback(self, kw):
         return
 
@@ -2000,7 +2003,8 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
         except errors.EmptyResult:
             (entries, truncated) = ([], False)
         except errors.NotFound:
-            
self.api.Object[self.obj.parent_object].handle_not_found(*args[:-1])
+            self.handle_not_found(*args, **options)
+            (entries, truncated) = ([], False)
 
         for callback in self.get_callbacks('post'):
             truncated = callback(self, ldap, entries, truncated, *args, 
**options)
@@ -2026,6 +2030,9 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
             truncated=truncated,
         )
 
+    def handle_not_found(self, *args, **options):
+        self.api.Object[self.obj.parent_object].handle_not_found(*args[:-1])
+
     def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, 
**options):
         assert isinstance(base_dn, DN)
         return (filters, base_dn, scope)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 
70b237dc102f46ab62e10aab0250aa496dad60c6..f8ee3db05b5a36cf4ebb4261f8535932b834b1bf
 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -903,10 +903,12 @@ class user_del(LDAPDelete):
 
         # Delete user's private vault container.
         vaultcontainer_id = 
self.api.Object.vaultcontainer.get_private_id(owner)
-        (vaultcontainer_name, vaultcontainer_parent_id) = 
self.api.Object.vaultcontainer.split_id(vaultcontainer_id)
+        (vaultcontainer_name, vaultcontainer_parent_id) =\
+            self.api.Object.vaultcontainer.split_id(vaultcontainer_id)
 
         try:
-            self.api.Command.vaultcontainer_del(vaultcontainer_name, 
parent=vaultcontainer_parent_id)
+            self.api.Command.vaultcontainer_del(
+                vaultcontainer_name, parent_id=vaultcontainer_parent_id)
         except errors.NotFound:
             pass
 
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 
69c12aaf1c8503a345e115fb1a660aed8f85fb17..0b12491a8fb140b889b7686486c794238c7ed48e
 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -41,8 +41,8 @@ from ipalib.frontend import Command
 from ipalib import api, errors
 from ipalib import Str, Bytes, Flag
 from ipalib.plugable import Registry
-from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete, 
LDAPSearch, LDAPUpdate, LDAPRetrieve,\
-                                    LDAPAddMember, LDAPRemoveMember
+from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete, 
LDAPSearch, LDAPUpdate,\
+                                    LDAPRetrieve, LDAPAddMember, 
LDAPRemoveMember
 from ipalib.request import context
 from ipalib.plugins.user import split_principal
 from ipalib import _, ngettext
@@ -61,7 +61,7 @@ EXAMPLES:
    ipa vault-find
 """) + _("""
  List shared vaults:
-   ipa vault-find --container /shared
+   ipa vault-find --container-id /shared
 """) + _("""
  Add a standard vault:
    ipa vault-add MyVault
@@ -135,6 +135,8 @@ class vault(LDAPObject):
     Vault object.
     """)
 
+    base_dn = DN(api.env.container_vault, api.env.basedn)
+
     object_name = _('vault')
     object_name_plural = _('vaults')
 
@@ -173,19 +175,11 @@ class vault(LDAPObject):
             pattern_errmsg='may only include letters, numbers, _, ., and -',
             maxlength=255,
         ),
-        Str('container?',
-            cli_name='container',
-            label=_('Container'),
-            doc=_('Container'),
-            flags=('virtual_attribute'),
-            pattern='^[a-zA-Z0-9_.-/]+$',
-            pattern_errmsg='may only include letters, numbers, _, ., -, and /',
-        ),
         Str('vault_id?',
             cli_name='vault_id',
             label=_('Vault ID'),
             doc=_('Vault ID'),
-            flags=('virtual_attribute'),
+            flags={'no_option', 'virtual_attribute'},
         ),
         Str('description?',
             cli_name='desc',
@@ -217,22 +211,16 @@ class vault(LDAPObject):
     )
 
     def get_dn(self, *keys, **options):
-        __doc__ = _("""
+        """
         Generates vault DN from vault ID.
-        """)
+        """
 
         # get vault ID from parameters
-        name = None
-        if keys:
-            name = keys[0]
+        name = keys[-1]
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+        vault_id = container_id + name
 
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
-
-        vault_id = container_id
-        if name:
-            vault_id = container_id + name
-
-        dn = api.Object.vaultcontainer.base_dn
+        dn = self.base_dn
 
         # for each name in the ID, prepend the base DN
         for name in vault_id.split(u'/'):
@@ -242,45 +230,30 @@ class vault(LDAPObject):
         return dn
 
     def get_id(self, dn):
-        __doc__ = _("""
+        """
         Generates vault ID from vault DN.
-        """)
+        """
 
         # make sure the DN is a vault DN
-        if not dn.endswith(api.Object.vaultcontainer.base_dn):
+        if not dn.endswith(self.base_dn):
             raise ValueError('Invalid vault DN: %s' % dn)
 
         # vault DN cannot be the container base DN
-        if len(dn) == len(api.Object.vaultcontainer.base_dn):
+        if len(dn) == len(self.base_dn):
             raise ValueError('Invalid vault DN: %s' % dn)
 
         # construct the vault ID from the bottom up
         id = u''
-        while len(dn) > len(api.Object.vaultcontainer.base_dn):
-
-            rdn = dn[0]
+        for rdn in dn[:-len(self.base_dn)]:
             name = rdn['cn']
             id =  u'/' + name + id
 
-            dn = DN(*dn[1:])
-
         return id
 
-    def split_id(self, id):
-        __doc__ = _("""
-        Splits a vault ID into (vault name, container ID) tuple.
-        """)
-
-        # split ID into container ID and vault name
-        parts = id.rsplit(u'/', 1)
-
-        # return vault name and container ID
-        return (parts[1], parts[0] + u'/')
-
     def get_kra_id(self, id):
-        __doc__ = _("""
+        """
         Generates a client key ID to store/retrieve data in KRA.
-        """)
+        """
         return 'ipa:' + id
 
     def generate_symmetric_key(self, password, salt):
@@ -354,9 +327,13 @@ class vault_add(LDAPCreate):
     __doc__ = _('Create a new vault.')
 
     takes_options = LDAPCreate.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary data to archive'),
+            doc=_('Binary data to archive'),
         ),
         Str('text?',
             cli_name='text',
@@ -389,7 +366,7 @@ class vault_add(LDAPCreate):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = options.get('ipavaulttype')
         data = options.get('data')
@@ -420,18 +397,14 @@ class vault_add(LDAPCreate):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -564,9 +537,9 @@ class vault_add(LDAPCreate):
         response = super(vault_add, self).forward(*args, **options)
 
         # archive initial data
-        api.Command.vault_archive(
+        self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=data,
             password=password,
             encryption_key=encryption_key)
@@ -576,13 +549,21 @@ class vault_add(LDAPCreate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, 
**options):
         assert isinstance(dn, DN)
 
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        # set owner
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
+        owner_dn = self.api.Object.user.get_dn(username)
         entry_attrs['owner'] = owner_dn
 
-        container_dn = DN(*dn[1:])
-        api.Object.vaultcontainer.create_entry(container_dn, owner=owner_dn)
+        # container is user's private container, create container
+        if container_id == self.api.Object.vaultcontainer.get_private_id():
+            try:
+                self.api.Object.vaultcontainer.create_entry(
+                    DN(*dn[1:]), owner=owner_dn)
+            except errors.DuplicateEntry:
+                pass
 
         return dn
 
@@ -593,31 +574,39 @@ class vault_add(LDAPCreate):
 
         return dn
 
+    def handle_not_found(self, *args, **options):
+
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        reason=self.obj.parent_not_found_msg % {
+            'parent': container_id,
+            'oname': self.api.Object.vaultcontainer.object_name,
+        }
 
 @register()
 class vault_del(LDAPDelete):
     __doc__ = _('Delete a vault.')
 
-    msg_summary = _('Deleted vault "%(value)s"')
-
     takes_options = LDAPDelete.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
+    msg_summary = _('Deleted vault "%(value)s"')
+
     def post_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
 
         vault_id = self.obj.get_id(dn)
 
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # deactivate vault record in KRA
         response = kra_client.keys.list_keys(client_key_id, 
pki.key.KeyClient.KEY_STATUS_ACTIVE)
@@ -636,6 +625,13 @@ class vault_del(LDAPDelete):
 class vault_find(LDAPSearch):
     __doc__ = _('Search for vaults.')
 
+    takes_options = LDAPSearch.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
+    )
+
     msg_summary = ngettext(
         '%(count)d vault matched', '%(count)d vaults matched', 0
     )
@@ -643,14 +639,9 @@ class vault_find(LDAPSearch):
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *keys, 
**options):
         assert isinstance(base_dn, DN)
 
-        principal = getattr(context, 'principal')
-        (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
-
-        container_id = 
self.Object.vaultcontainer.normalize_id(options.get('container'))
-        base_dn = self.Object.vaultcontainer.get_dn(parent=container_id)
-
-        api.Object.vaultcontainer.create_entry(base_dn, owner=owner_dn)
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+        (name, parent_id) = 
self.api.Object.vaultcontainer.split_id(container_id)
+        base_dn = self.api.Object.vaultcontainer.get_dn(name, 
parent_id=parent_id)
 
         return (filter, base_dn, scope)
 
@@ -662,11 +653,31 @@ class vault_find(LDAPSearch):
 
         return truncated
 
+    def handle_not_found(self, *args, **options):
+
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
+
+        # vault container is user's private container, ignore
+        if container_id == self.api.Object.vaultcontainer.get_private_id():
+            return
+
+        # otherwise, raise an error
+        reason=self.obj.parent_not_found_msg % {
+            'parent': container_id,
+            'oname': self.api.Object.vaultcontainer.object_name,
+        }
 
 @register()
 class vault_mod(LDAPUpdate):
     __doc__ = _('Modify a vault.')
 
+    takes_options = LDAPUpdate.takes_options + (
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
+        ),
+    )
+
     msg_summary = _('Modified vault "%(value)s"')
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -682,9 +693,9 @@ class vault_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -700,11 +711,6 @@ class vault_show(LDAPRetrieve):
 class vault_transport_cert(Command):
     __doc__ = _('Retrieve vault transport certificate.')
 
-    # list of attributes we want exported to JSON
-    json_friendly_attributes = (
-        'takes_args',
-    )
-
     takes_options = (
         Str('out?',
             cli_name='out',
@@ -718,13 +724,6 @@ class vault_transport_cert(Command):
         ),
     )
 
-    def __json__(self):
-        json_dict = dict(
-            (a, getattr(self, a)) for a in self.json_friendly_attributes
-        )
-        json_dict['takes_options'] = list(self.get_json_options())
-        return json_dict
-
     def forward(self, *args, **options):
 
         file = options.get('out')
@@ -743,7 +742,7 @@ class vault_transport_cert(Command):
 
     def execute(self, *args, **options):
 
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
         transport_cert = kra_client.system_certs.get_transport_cert()
         return {
             'result': {
@@ -757,13 +756,13 @@ class vault_archive(LDAPRetrieve):
     __doc__ = _('Archive data into a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary data to archive'),
+            doc=_('Binary data to archive'),
         ),
         Str('text?',
             cli_name='text',
@@ -804,7 +803,7 @@ class vault_archive(LDAPRetrieve):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
@@ -812,7 +811,7 @@ class vault_archive(LDAPRetrieve):
         escrow_public_key = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -850,18 +849,14 @@ class vault_archive(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -902,9 +897,9 @@ class vault_archive(LDAPRetrieve):
                     password = unicode(getpass.getpass('Password: '))
 
                 try:
-                    api.Command.vault_retrieve(
+                    self.api.Command.vault_retrieve(
                         vault_name,
-                        container=container_id,
+                        container_id=container_id,
                         password=password)
 
                 except errors.NotFound:
@@ -959,7 +954,7 @@ class vault_archive(LDAPRetrieve):
         (file, filename) = tempfile.mkstemp()
         os.close(file)
         try:
-            api.Command.vault_transport_cert(out=unicode(filename))
+            self.api.Command.vault_transport_cert(out=unicode(filename))
             transport_cert_der = nss.read_der_from_file(filename, True)
             nss_transport_cert = nss.Certificate(transport_cert_der)
 
@@ -981,7 +976,7 @@ class vault_archive(LDAPRetrieve):
         options['nonce'] = unicode(base64.b64encode(nonce))
 
         vault_data = {}
-        vault_data[u'data'] = unicode(data)
+        vault_data[u'data'] = unicode(base64.b64encode(data))
 
         if encrypted_key:
             vault_data[u'encrypted_key'] = 
unicode(base64.b64encode(encrypted_key))
@@ -1012,7 +1007,7 @@ class vault_archive(LDAPRetrieve):
 
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        user_dn = self.api.Object['user'].get_dn(username)
+        user_dn = self.api.Object.user.get_dn(username)
 
         if user_dn not in owners and user_dn not in members:
             raise errors.ACIError(info=_("Insufficient access to vault '%s'.") 
% vault_id)
@@ -1020,12 +1015,12 @@ class vault_archive(LDAPRetrieve):
         entry_attrs['vault_id'] = vault_id
 
         # connect to KRA
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # deactivate existing vault record in KRA
         response = kra_client.keys.list_keys(
@@ -1062,9 +1057,9 @@ class vault_retrieve(LDAPRetrieve):
     __doc__ = _('Retrieve a data from a vault.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
         Flag('show_text?',
             doc=_('Show text data'),
@@ -1122,13 +1117,13 @@ class vault_retrieve(LDAPRetrieve):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -1179,7 +1174,7 @@ class vault_retrieve(LDAPRetrieve):
         (file, filename) = tempfile.mkstemp()
         os.close(file)
         try:
-            api.Command.vault_transport_cert(out=unicode(filename))
+            self.api.Command.vault_transport_cert(out=unicode(filename))
             transport_cert_der = nss.read_der_from_file(filename, True)
             nss_transport_cert = nss.Certificate(transport_cert_der)
 
@@ -1209,7 +1204,7 @@ class vault_retrieve(LDAPRetrieve):
             nonce_iv=nonce)
 
         vault_data = json.loads(json_vault_data)
-        data = str(vault_data[u'data'])
+        data = base64.b64decode(str(vault_data[u'data']))
 
         encrypted_key = None
 
@@ -1343,7 +1338,7 @@ class vault_retrieve(LDAPRetrieve):
 
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        user_dn = self.api.Object['user'].get_dn(username)
+        user_dn = self.api.Object.user.get_dn(username)
 
         if user_dn not in owners and user_dn not in members:
             raise errors.ACIError(info=_("Insufficient access to vault '%s'.") 
% vault_id)
@@ -1353,12 +1348,12 @@ class vault_retrieve(LDAPRetrieve):
         wrapped_session_key = base64.b64decode(options['session_key'])
 
         # connect to KRA
-        kra_client = api.Backend.kra.get_client()
+        kra_client = self.api.Backend.kra.get_client()
 
         kra_account = pki.account.AccountClient(kra_client.connection)
         kra_account.login()
 
-        client_key_id = self.api.Object.vault.get_kra_id(vault_id)
+        client_key_id = self.obj.get_kra_id(vault_id)
 
         # find vault record in KRA
         response = kra_client.keys.list_keys(
@@ -1388,9 +1383,9 @@ class vault_add_owner(LDAPAddMember):
     __doc__ = _('Add owners to a vault.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1410,9 +1405,9 @@ class vault_remove_owner(LDAPRemoveMember):
     __doc__ = _('Remove owners from a vault.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1432,9 +1427,9 @@ class vault_add_member(LDAPAddMember):
     __doc__ = _('Add members to a vault.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
@@ -1451,9 +1446,9 @@ class vault_remove_member(LDAPRemoveMember):
     __doc__ = _('Remove members from a vault.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('container?',
-            cli_name='container',
-            doc=_('Container'),
+        Str('container_id?',
+            cli_name='container_id',
+            doc=_('Container ID'),
         ),
     )
 
diff --git a/ipalib/plugins/vaultcontainer.py b/ipalib/plugins/vaultcontainer.py
index 
881bbea7886e06356489cd49cc32f2a6a6460a5e..e0dc728362247f7cde252e21a54fa5c23761eb49
 100644
--- a/ipalib/plugins/vaultcontainer.py
+++ b/ipalib/plugins/vaultcontainer.py
@@ -40,7 +40,7 @@ EXAMPLES:
    ipa vaultcontainer-find
 """) + _("""
  List top-level vault containers:
-   ipa vaultcontainer-find --parent /
+   ipa vaultcontainer-find --parent-id /
 """) + _("""
  Add a vault container:
    ipa vaultcontainer-add MyContainer
@@ -75,7 +75,6 @@ class vaultcontainer(LDAPObject):
     Vault container object.
     """)
 
-    base_dn = DN(api.env.container_vault, api.env.basedn)
     object_name = _('vault container')
     object_name_plural = _('vault containers')
 
@@ -109,19 +108,11 @@ class vaultcontainer(LDAPObject):
             pattern_errmsg='may only include letters, numbers, _, ., and -',
             maxlength=255,
         ),
-        Str('parent?',
-            cli_name='parent',
-            label=_('Parent'),
-            doc=_('Parent container'),
-            flags=('virtual_attribute'),
-            pattern='^[a-zA-Z0-9_.-/]+$',
-            pattern_errmsg='may only include letters, numbers, _, ., -, and /',
-        ),
         Str('container_id?',
             cli_name='container_id',
             label=_('Container ID'),
             doc=_('Container ID'),
-            flags=('virtual_attribute'),
+            flags={'no_option', 'virtual_attribute'},
         ),
         Str('description?',
             cli_name='desc',
@@ -131,22 +122,19 @@ class vaultcontainer(LDAPObject):
     )
 
     def get_dn(self, *keys, **options):
-        __doc__ = _("""
+        """
         Generates vault container DN from container ID.
-        """)
+        """
 
         # get container ID from parameters
-        name = None
-        if keys:
-            name = keys[0]
-
-        parent_id = 
api.Object.vaultcontainer.normalize_id(options.get('parent'))
+        name = keys[-1]
+        parent_id = self.normalize_id(options.get('parent_id'))
 
         container_id = parent_id
         if name:
             container_id = parent_id + name + u'/'
 
-        dn = self.base_dn
+        dn = self.api.Object.vault.base_dn
 
         # for each name in the ID, prepend the base DN
         for name in container_id.split(u'/'):
@@ -156,30 +144,26 @@ class vaultcontainer(LDAPObject):
         return dn
 
     def get_id(self, dn):
-        __doc__ = _("""
+        """
         Generates container ID from container DN.
-        """)
+        """
 
         # make sure the DN is a container DN
-        if not dn.endswith(self.base_dn):
+        if not dn.endswith(self.api.Object.vault.base_dn):
             raise ValueError('Invalid container DN: %s' % dn)
 
         # construct container ID from the bottom up
         id = u'/'
-        while len(dn) > len(self.base_dn):
-
-            rdn = dn[0]
+        for rdn in dn[:-len(self.api.Object.vault.base_dn)]:
             name = rdn['cn']
             id =  u'/' + name + id
 
-            dn = DN(*dn[1:])
-
         return id
 
     def get_private_id(self, username=None):
-        __doc__ = _("""
+        """
         Returns user's private container ID (i.e. /users/<username>/).
-        """)
+        """
 
         if not username:
             principal = getattr(context, 'principal')
@@ -188,9 +172,9 @@ class vaultcontainer(LDAPObject):
         return u'/users/' + username + u'/'
 
     def normalize_id(self, id):
-        __doc__ = _("""
+        """
         Normalizes container ID.
-        """)
+        """
 
         # if ID is empty, return user's private container ID
         if not id:
@@ -208,13 +192,13 @@ class vaultcontainer(LDAPObject):
         return self.get_private_id() + id
 
     def split_id(self, id):
-        __doc__ = _("""
+        """
         Splits a normalized container ID into (container name, parent ID) 
tuple.
-        """)
+        """
 
         # handle root ID
         if id == u'/':
-            return (None, None)
+            return (None, u'/')
 
         # split ID into parent ID, container name, and empty string
         parts = id.rsplit(u'/', 2)
@@ -223,23 +207,10 @@ class vaultcontainer(LDAPObject):
         return (parts[1], parts[0] + u'/')
 
     def create_entry(self, dn, owner=None):
-        __doc__ = _("""
+        """
         Creates a container entry and its parents.
-        """)
+        """
 
-        # if entry already exists, return
-        try:
-            self.backend.get_entry(dn)
-            return
-
-        except errors.NotFound:
-            pass
-
-        # otherwise, create parent entry first
-        parent_dn = DN(*dn[1:])
-        self.create_entry(parent_dn, owner=owner)
-
-        # then create the entry itself
         rdn = dn[0]
         entry = self.backend.make_entry(
             dn,
@@ -248,6 +219,20 @@ class vaultcontainer(LDAPObject):
                 'cn': rdn['cn'],
                 'owner': owner
             })
+
+        # if entry can be added return
+        try:
+            self.backend.add_entry(entry)
+            return
+
+        except errors.NotFound:
+            pass
+
+        # otherwise, create parent entry first
+        parent_dn = DN(*dn[1:])
+        self.create_entry(parent_dn, owner=owner)
+
+        # then create the entry again
         self.backend.add_entry(entry)
 
 
@@ -255,18 +240,33 @@ class vaultcontainer(LDAPObject):
 class vaultcontainer_add(LDAPCreate):
     __doc__ = _('Create a new vault container.')
 
+    takes_options = LDAPCreate.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = _('Added vault container "%(value)s"')
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, 
**options):
         assert isinstance(dn, DN)
 
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        # set owner
         principal = getattr(context, 'principal')
         (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
+        owner_dn = self.api.Object.user.get_dn(username)
         entry_attrs['owner'] = owner_dn
 
-        parent_dn = DN(*dn[1:])
-        self.obj.create_entry(parent_dn, owner=owner_dn)
+        # parent is user's private container, create parent
+        if parent_id == self.obj.get_private_id():
+            try:
+                self.obj.create_entry(
+                    DN(*dn[1:]), owner=owner_dn)
+            except errors.DuplicateEntry:
+                pass
 
         return dn
 
@@ -277,17 +277,26 @@ class vaultcontainer_add(LDAPCreate):
 
         return dn
 
+    def handle_not_found(self, *args, **options):
+
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        raise errors.NotFound(
+            reason=self.obj.parent_not_found_msg % {
+                'parent': parent_id,
+                'oname': self.obj.object_name,
+            }
+        )
+
 
 @register()
 class vaultcontainer_del(LDAPDelete):
     __doc__ = _('Delete a vault container.')
 
-    msg_summary = _('Deleted vault container "%(value)s"')
-
     takes_options = LDAPDelete.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
         Flag('force?',
             doc=_('Force deletion'),
@@ -295,42 +304,33 @@ class vaultcontainer_del(LDAPDelete):
         ),
     )
 
-    def delete_entry(self, pkey, *keys, **options):
-        __doc__ = _("""
-        Overwrites the base method to control deleting subtree with force 
option.
-        """)
+    msg_summary = _('Deleted vault container "%(value)s"')
 
-        ldap = self.obj.backend
-        nkeys = keys[:-1] + (pkey, )
-        dn = self.obj.get_dn(*nkeys, **options)
+    def pre_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
 
-        for callback in self.get_callbacks('pre'):
-            dn = callback(self, ldap, dn, *nkeys, **options)
-            assert isinstance(dn, DN)
-
         try:
-            self._exc_wrapper(nkeys, options, ldap.delete_entry)(dn)
+            ldap.get_entries(dn, scope=ldap.SCOPE_ONELEVEL, attrs_list=[])
         except errors.NotFound:
-            self.obj.handle_not_found(*nkeys)
-        except errors.NotAllowedOnNonLeaf:
-            # this entry is not a leaf entry
-            # if forced, delete all child nodes
-            if options.get('force'):
-                self.delete_subtree(dn, *nkeys, **options)
-            else:
-                raise
+            pass
+        else:
+            if not options.get('force', False):
+                raise errors.NotAllowedOnNonLeaf()
 
-        for callback in self.get_callbacks('post'):
-            result = callback(self, ldap, dn, *nkeys, **options)
-
-        return result
+        return dn
 
 
 @register()
 class vaultcontainer_find(LDAPSearch):
     __doc__ = _('Search for vault containers.')
 
+    takes_options = LDAPSearch.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = ngettext(
         '%(count)d vault container matched', '%(count)d vault containers 
matched', 0
     )
@@ -338,14 +338,9 @@ class vaultcontainer_find(LDAPSearch):
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *keys, 
**options):
         assert isinstance(base_dn, DN)
 
-        principal = getattr(context, 'principal')
-        (username, realm) = split_principal(principal)
-        owner_dn = self.api.Object['user'].get_dn(username)
-
-        parent_id = self.obj.normalize_id(options.get('parent'))
-        base_dn = self.obj.get_dn(parent=parent_id)
-
-        self.obj.create_entry(base_dn, owner=owner_dn)
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+        (name, grandparent_id) = self.obj.split_id(parent_id)
+        base_dn = self.obj.get_dn(name, parent_id=grandparent_id)
 
         return (filter, base_dn, scope)
 
@@ -356,11 +351,32 @@ class vaultcontainer_find(LDAPSearch):
 
         return truncated
 
+    def handle_not_found(self, *args, **options):
+
+        parent_id = self.obj.normalize_id(options.get('parent_id'))
+
+        # parent is user's private container, ignore
+        if parent_id == self.obj.get_private_id():
+            return
+
+        # otherwise, raise an error
+        reason=self.obj.parent_not_found_msg % {
+            'parent': parent_id,
+            'oname': self.obj.object_name,
+        }
+
 
 @register()
 class vaultcontainer_mod(LDAPUpdate):
     __doc__ = _('Modify a vault container.')
 
+    takes_options = LDAPUpdate.takes_options + (
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
+        ),
+    )
+
     msg_summary = _('Modified vault container "%(value)s"')
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -376,9 +392,9 @@ class vaultcontainer_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault container.')
 
     takes_options = LDAPRetrieve.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -395,9 +411,9 @@ class vaultcontainer_add_owner(LDAPAddMember):
     __doc__ = _('Add owners to a vault container.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -417,9 +433,9 @@ class vaultcontainer_remove_owner(LDAPRemoveMember):
     __doc__ = _('Remove owners from a vault container.')
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -439,9 +455,9 @@ class vaultcontainer_add_member(LDAPAddMember):
     __doc__ = _('Add members to a vault container.')
 
     takes_options = LDAPAddMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
@@ -459,9 +475,9 @@ class vaultcontainer_remove_member(LDAPRemoveMember):
 
 
     takes_options = LDAPRemoveMember.takes_options + (
-        Str('parent?',
-            cli_name='parent',
-            doc=_('Parent container'),
+        Str('parent_id?',
+            cli_name='parent_id',
+            doc=_('Parent ID'),
         ),
     )
 
diff --git a/ipalib/plugins/vaultsecret.py b/ipalib/plugins/vaultsecret.py
index 
b0896155a5af13013f13f2b3ae586da2a8832c30..7f44f0816b571dac718fa02edfd187fe8666565e
 100644
--- a/ipalib/plugins/vaultsecret.py
+++ b/ipalib/plugins/vaultsecret.py
@@ -93,7 +93,7 @@ class vaultsecret(LDAPObject):
         Bytes('data?',
             cli_name='data',
             label=_('Data'),
-            doc=_('Base-64 encoded binary secret data'),
+            doc=_('Binary secret data'),
         ),
     )
 
@@ -103,8 +103,8 @@ class vaultsecret_add(LDAPRetrieve):
     __doc__ = _('Add a new vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('description?',
@@ -113,7 +113,7 @@ class vaultsecret_add(LDAPRetrieve):
         ),
         Bytes('data?',
             cli_name='data',
-            doc=_('Base-64 encoded binary secret data'),
+            doc=_('Binary secret data'),
         ),
         Str('text?',
             cli_name='text',
@@ -147,13 +147,13 @@ class vaultsecret_add(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -256,9 +256,9 @@ class vaultsecret_add(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -276,18 +276,14 @@ class vaultsecret_add(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -315,9 +311,9 @@ class vaultsecret_add(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -338,8 +334,8 @@ class vaultsecret_del(LDAPRetrieve):
     __doc__ = _('Delete a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('password?',
@@ -366,13 +362,13 @@ class vaultsecret_del(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -463,9 +459,9 @@ class vaultsecret_del(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -496,9 +492,9 @@ class vaultsecret_del(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -515,13 +511,11 @@ class vaultsecret_del(LDAPRetrieve):
 
 @register()
 class vaultsecret_find(LDAPSearch):
-    __doc__ = _("""
-    Search for vault secrets.
-    """)
+    __doc__ = _('Search for vault secrets.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('password?',
@@ -545,13 +539,13 @@ class vaultsecret_find(LDAPSearch):
     def forward(self, *args, **options):
 
         vault_name = args[0]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -641,9 +635,9 @@ class vaultsecret_find(LDAPSearch):
             raise errors.ValidationError(name='vault_type',
                 error=_('Invalid vault type'))
 
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -678,8 +672,8 @@ class vaultsecret_mod(LDAPRetrieve):
     __doc__ = _('Modify a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Str('description?',
@@ -722,13 +716,13 @@ class vaultsecret_mod(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -830,18 +824,14 @@ class vaultsecret_mod(LDAPRetrieve):
 
         # get data
         if data:
-            if text:
-                raise errors.ValidationError(name='text',
-                    error=_('Input data already specified'))
-
-            if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+            if text or input_file:
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
         elif text:
             if input_file:
-                raise errors.ValidationError(name='input_file',
-                    error=_('Input data already specified'))
+                raise errors.MutuallyExclusiveError(
+                    reason=_('Input data specified multiple times'))
 
             data = text.encode('utf-8')
 
@@ -853,9 +843,9 @@ class vaultsecret_mod(LDAPRetrieve):
             pass
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
@@ -889,9 +879,9 @@ class vaultsecret_mod(LDAPRetrieve):
 
         # rearchive secrets
         vault_data = json.dumps(json_data)
-        response = api.Command.vault_archive(
+        response = self.api.Command.vault_archive(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             data=vault_data,
             password=password)
 
@@ -912,8 +902,8 @@ class vaultsecret_show(LDAPRetrieve):
     __doc__ = _('Display information about a vault secret.')
 
     takes_options = (
-        Str('container?',
-            cli_name='container',
+        Str('container_id?',
+            cli_name='container_id',
             doc=_('Container ID'),
         ),
         Flag('show_text?',
@@ -956,13 +946,13 @@ class vaultsecret_show(LDAPRetrieve):
 
         vault_name = args[0]
         secret_name = args[1]
-        container_id = 
api.Object.vaultcontainer.normalize_id(options.get('container'))
+        container_id = 
self.api.Object.vaultcontainer.normalize_id(options.get('container_id'))
 
         vault_type = 'standard'
         salt = None
 
         # retrieve vault info
-        response = api.Command.vault_show(vault_name, container=container_id)
+        response = self.api.Command.vault_show(vault_name, 
container_id=container_id)
         result = response['result']
 
         if result.has_key('ipavaulttype'):
@@ -1062,9 +1052,9 @@ class vaultsecret_show(LDAPRetrieve):
                 error=_('Invalid vault type'))
 
         # retrieve secrets
-        response = api.Command.vault_retrieve(
+        response = self.api.Command.vault_retrieve(
             vault_name,
-            container=container_id,
+            container_id=container_id,
             password=password,
             private_key=private_key)
 
diff --git a/ipatests/test_xmlrpc/test_vault_plugin.py 
b/ipatests/test_xmlrpc/test_vault_plugin.py
index 
04f56ecafd3a141b39a0e32f1725258582b6b141..f3a280b40d5b6972e8755f63d46013cadaa68334
 100644
--- a/ipatests/test_xmlrpc/test_vault_plugin.py
+++ b/ipatests/test_xmlrpc/test_vault_plugin.py
@@ -150,7 +150,7 @@ MszdQuc/FTSJ2DYsIwx7qq5c8mtargOjWRgZU22IgY9PKeIcitQjqw==
 -----END RSA PRIVATE KEY-----
 """
 
-class test_vault(Declarative):
+class test_vault_plugin(Declarative):
 
     cleanup_commands = [
         ('vault_del', [test_vault], {'continue': True}),
diff --git a/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py 
b/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
index 
b99f9f68b8a480908602503d726205eeec36c2d2..22e13769df19b40cd39a144df662bae8bbf53d9e
 100644
--- a/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
+++ b/ipatests/test_xmlrpc/test_vaultcontainer_plugin.py
@@ -32,12 +32,12 @@ base_container = u'base_container'
 child_container = u'child_container'
 grandchild_container = u'grandchild_container'
 
-class test_vault(Declarative):
+class test_vaultcontainer_plugin(Declarative):
 
     cleanup_commands = [
         ('vaultcontainer_del', [private_container], {'continue': True}),
-        ('vaultcontainer_del', [shared_container], {'parent': u'/shared/', 
'continue': True}),
-        ('vaultcontainer_del', [service_container], {'parent': u'/services/', 
'continue': True}),
+        ('vaultcontainer_del', [shared_container], {'parent_id': u'/shared/', 
'continue': True}),
+        ('vaultcontainer_del', [service_container], {'parent_id': 
u'/services/', 'continue': True}),
         ('vaultcontainer_del', [base_container], {'force': True, 'continue': 
True}),
     ]
 
@@ -49,7 +49,7 @@ class test_vault(Declarative):
                 'vaultcontainer_find',
                 [],
                 {
-                    'parent': u'/',
+                    'parent_id': u'/',
                 },
             ),
             'expected': {
@@ -202,7 +202,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -224,7 +224,7 @@ class test_vault(Declarative):
                 'vaultcontainer_find',
                 [],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -247,7 +247,7 @@ class test_vault(Declarative):
                 'vaultcontainer_show',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -268,7 +268,7 @@ class test_vault(Declarative):
                 'vaultcontainer_mod',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                     'description': u'shared container',
                 },
             ),
@@ -290,7 +290,7 @@ class test_vault(Declarative):
                 'vaultcontainer_del',
                 [shared_container],
                 {
-                    'parent': u'/shared/',
+                    'parent_id': u'/shared/',
                 },
             ),
             'expected': {
@@ -308,7 +308,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [service_container],
                 {
-                    'parent': u'/services/',
+                    'parent_id': u'/services/',
                 },
             ),
             'expected': {
@@ -349,7 +349,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [child_container],
                 {
-                    'parent': base_container,
+                    'parent_id': base_container,
                 },
             ),
             'expected': {
@@ -371,7 +371,7 @@ class test_vault(Declarative):
                 'vaultcontainer_add',
                 [grandchild_container],
                 {
-                    'parent': base_container + u'/' + child_container,
+                    'parent_id': base_container + u'/' + child_container,
                 },
             ),
             'expected': {
diff --git a/ipatests/test_xmlrpc/test_vaultsecret_plugin.py 
b/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
index 
68f0fb0d7085be512939e397ea49abbcf3ca3c7b..cbfd231633e7c3c000e57d52d85b83f44f71df3c
 100644
--- a/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
+++ b/ipatests/test_xmlrpc/test_vaultsecret_plugin.py
@@ -29,7 +29,7 @@ test_vaultsecret = u'test_vaultsecret'
 binary_data = '\x01\x02\x03\x04'
 text_data = u'secret'
 
-class test_vault(Declarative):
+class test_vaultsecret_plugin(Declarative):
 
     cleanup_commands = [
         ('vault_del', [test_vault], {'continue': True}),
-- 
1.9.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to