On 06/02/2015 11:22 PM, Alexander Bokovoy wrote: > On Tue, 02 Jun 2015, Endi Sukma Dewata wrote: >> Please take a look at the new patch. >> >> On 6/2/2015 10:05 AM, Martin Kosek wrote: >>>>> 4) In the vault-archive forward method, you use "pki" module. However, >>>>> this module will be only available on FreeIPA PKI-powered servers and >>>>> not on FreeIPA clients - so this will not work unless freeipa-client >>>>> gets a dependency on pki-base - which is definitely not something we >>>>> want... >>>> >>>> In my opinion it should be fine to require pki-base on the client because >>>> it >>>> contains just the client library, unless you have other concerns? Any >>>> objections to having pki-nss and pki-cryptography dependencies on the >>>> client? >>>> >>>> Even if we can change the client code not to depend on "pki" module, since >>>> in >>>> this framework the client and server code are written in the same plugin, >>>> the >>>> "import pki" still cannot be removed since it's still needed by the server >>>> code, and I don't think conditional import is a good programming practice. >>> >>> I have major concerns here. Look at the different between installing >>> "freeipa-client" and "freeipa-client + pki-base" on my F21: >>> >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> $ sudo yum install freeipa-client >>> ... >>> Install 1 Package (+4 Dependent packages) >>> >>> Total download size: 2.6 M >>> Installed size: 14 M >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> $ sudo yum install freeipa-client pki-base >>> ... >>> Install 2 Packages (+288 Dependent packages) >>> >>> Total download size: 160 M >>> Installed size: 235 M >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> This is obviously a no-go for client. The conditional import is smaller >>> concern >>> that big dependency growth on the client. We do them in trust plugin for >>> example and it works fine (though I agree it is not ideal programming >>> practice). >>> >>> IMO, we should limit new freeipa-client dependencies only to >>> python-cryptography (or also python-nss in the worst case, in case >>> python-cryptography is not enough) - there should be no pki dependencies at >>> all, these should be only on the server side. >> >> OK. I opened a ticket to split the pki-base into separate Python and Java >> packages: >> https://fedorahosted.org/pki/ticket/1399 >> >> For now in this patch I added conditional imports for pki.account and pki.key >> which are needed to access KRA on the server side. I removed dependency on >> pki.crypto on the client side and replaced it with direct python-nss code. >> >> On 6/2/2015 1:40 PM, Simo Sorce wrote: >>> I have coded in python (jwcrypto) >>> support for some key wrapping not yet available in python-cryptography >>> and can lend you the code as needed. >> >> Thanks. I'll get back to you when it's time to add support for >> python-cryptography in KRA: >> https://fedorahosted.org/pki/ticket/1400 >> >> On 6/2/2015 10:16 AM, Alexander Bokovoy wrote: >>> Yes, please use conditional import here, it is perfectly valid use case >>> for the client side. >> >> On 6/2/2015 1:40 PM, Simo Sorce wrote: >>> conditional import is just fine >> >> The conditional imports that I've seen usually are used for importing >> different versions of the same module, which I think is acceptable because >> the dependency always exists. In the vault case we're selectively importing a >> module depending on where the code runs. I think that is bad because it adds >> complexity and it's easy to make mistakes. Any code that depends on that >> module would have to be (a) guarded: >> >> if pki_is_loaded: >> ... call pki ... >> >> or (b) used in a method that's only called if the module is loaded: >> >> def do_something(self): # runs only on server >> ... call pki ... >> >> The (a) is similar to #ifdef's which should be avoidable using OOD, and in >> (b) we may inadvertently call a wrong method indirectly. I think ideally the >> client and server code should be in separate files (so they can be deployed >> separately too), but the framework doesn't seem to allow that. > This exactly the case we have to use here and we are using that in > trusts case as well -- some code has to run on server only and shouldn't > cause to install Samba related packages on the client. This is because > IPA client is actually using the same IPA plugins that server uses, to > have access to the API calls metadata and client-side callbacks are > defined in the same place where server-side callbacks are. It is IPA > framework design, so we have to use what we have.
This is planned to be changed BTW, when we start with the "Thin Client" concept and have different code/plugins for FreeIPA server side and client side. > In other words, it is not necessarily an evil under conditions we are > dealing with. > -- 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