On 6/25/2015 12:35 AM, Jan Cholasta wrote:
I think it would be better to use a new attribute type which inherits
from ipaPublicKey (ipaVaultPublicKey?) rather than ipaPublicKey
for assymetric vault public keys, so that assymetric public key and
escrow public key are on the same level and you can still use
ipaPublicKey to refer to either one:


OK. To be consistent the parameters need to be renamed too:
--vault-public-key and --vault-public-key-file.

It doesn't need to, there is no requirement for CLI names to always
match attribute names. (Also I don't insist on the name
"ipaVaultPublicKey", feel free to change it if you want.)

It's unchanged for now. In a previous discussion it was advised to reuse
the existing attribute type whenever possible.

Well, in this discussion, it is not. Escrow public key should also reuse
ipaPublicKey, but it can't if you use it for vault public key. By using
ipaPublicKey subtypes you can distinguish between the two uses and still
use ipaPublicKey to refer to either of them.

So what's changed? This is what you said when I posted the same patch six months ago:

In this patch I'm adding ipaVaultSalt and ipaVaultPublicKey attribute
types to store salt and public key for vault. Are there existing
attribute types that I can use instead? I see there's an ipaPublicKey,
should I use that and maybe add ipaSalt/ipaEncSalt? Thanks.

yes, please re-use existing attributes where possible.


Could you show me how to use ipaPublicKey to refer to ipaVaultPublicKey and ipaEscrowPublicKey? Under what situation would that be useful?

  a) When the non-split vault_{archive,retrieve} was called from a
server API with client-only options, it crashed. This is the broken API
I was talking about.

This is because in the current framework any API called on the server
side will be a server API, so you are not supposed to call it with
client options in the first place. Because of that limitation, the only
way to use client options is to use a separate API on the client side to
call the original API on the server side. The point is, client options
belong to client API, and server options belong to server API. In
vault_add the public key file name belongs to client API because it's
used to load a file on the client side. You should not add public key
file name option to the server API just because it can safely be ignored.

I don't disagree, but file name options do not belong to the general
client API either, as they are strictly CLI-specific.

To my understanding the current framework doesn't have a separate CLI class, so you don't have a choice but to put CLI-specific options in the client API class too. However, you do have a choice not to combine client API class and server API class because otherwise that will put CLI-specific options in the server API class too.

2. Since the vault_archive_internal inherits from Update, it accepts
all non
primary-key attributes automatically. This is incorrect since we
don't want to
update these parameters during archival. Can this behavior be

Inherit from PKQuery instead (don't forget to add "has_output =

Previously you didn't want to use LDAPQuery because of semantics
reasons. Is PKQuery fine semantically?

It's not. Currently there is a set of commands which operate on the LDAP
part of vault and another set of commands which operate on the KRA part
of vault and we don't want the commands in one set to see attributes
related to the other part of vault. If you insist on keeping both parts
in a single object, you have to resort to hackery like using PKQuery,
hence my suggestion to split the data part off to a separate object to
avoid this.

This because the framework was based on simplistic assumptions which
create unnecessary restrictions, for example:
* client API is just a proxy to server API (i.e. client and server
cannot do different things)

They can do different things the same way vault_archive/vault_retrieve
does that, the commands just can't be called the same (which is not
necessarily a bad thing).

Of course different APIs can do different things, like vault_add calling vault_archive, or vault_archive calling vault_archive_internal. The point is right now the client portion of an API (i.e. the forward() method) cannot do anything other than forwarding the request to the server, so the API has to be split into different APIs:

 * vault_archive
 * vault_archive_internal

It would be nice to have formal separation between client and server APIs so it's clear they are different but still related without resorting to ugly names:

 * client.vault_archive
 * server.vault_archive

* CLI options will be identical to client and server API options (i.e.
no CLI-only, client-only, or server-only options)

Actually, you can create CLI-only options (add include='cli' to the
param's kwargs).

I need to look at this more closely. If I understand correctly in user_del there are two 'preserve' options, the Bool preserve is for client and server API, and the Flag preserve is for CLI. Wouldn't it be better if they are stored in separate lists (or maybe separate classes)? And it looks like you still need to delete the CLI options explicitly anyway.

Does the API.txt actually show the CLI options, the client API options, or the server API options? I only see the Flag preserve, not the Bool preserve.

* a plugin will only access one type of data (i.e. LDAP plugin can only
access LDAP data)

This is not assumed anywhere in the framework, you can access whatever
you want, but you can't expect baseldap to do everything for you.

Nobody is expecting baseldap to do KRA operations.

As the
name implies, it is LDAP specific, if you want something else, you have
to implement it yourself.

In the previous patch vault_retrieve inherits from LDAPRetrieve so it can rely on baseldap to retrieve the vault entry, then on top of that it implements an additional KRA operations (without baseldap obviously). If that is not allowed, aren't you basically saying LDAP plugin can only access LDAP data?

* a command name will match the object name (i.e. must use vaultdata_mod
instead of a more intuitive vault_archive)

I don't see how consistency is a bad thing, or how this could limit
anyone doing things cleanly. I do agree that vaultdata_mod is ugly, but
it's not the only way to achieve the same goal.

Look at it from user's perspective. If you create a vault using vault-add <vault name>, then archive data using vaultdata-mod <vault name>, how is this consistent?

We know that some use cases do not fit these assumptions. Rather than
compromising the use case, or looking at workarounds as hacks, I'd
suggest finding ideas to improve the framework itself to be more

I would personally love to improve the framework (it's just retarded
sometimes as you may have noticed), but it does not have high priority
right now (not my decision).

We don't have to modify the current framework right now, but we can align new codes that don't fit the current framework to match the future framework. Although the future framework is not defined yet, some things are already clear, for example there should be separate client and server APIs. So if a command like vault_add has differing client and server options, regardless how insignificant it is, there's no reason to force it to be combined. The current framework doesn't prevent separation anyway.

Keep in mind that workarounds which screw with the object model will
always be considered hacks, even after the framework is made more

Don't get this wrong. The framework will only be considered accommodating if it allows people to implement features without 'hacking'. 'Hacking' itself is never a goal, it's the last resort to work around the framework's current limitations, just like how you ended up using PKQuery for vault_archive_internal.

Endi S. Dewata

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

Reply via email to