[Freeipa-devel] [PATCH] 0042-0048 AD trusts support (master)

[Martin Kosek]

On Thu, 2012-04-12 at 18:08 +0300, Alexander Bokovoy wrote:
> Hi Martin!
> 
> On Thu, 12 Apr 2012, Martin Kosek wrote:
...
> >3) I would not try to import ipaserver.dcerpc every time the command is
> >executed:
> >+        try:
> >+            import ipaserver.dcerpc
> >+        except Exception, e:
> >+            raise errors.NotFound(name=_('AD Trust setup'),
> >+                  reason=_('Cannot perform join operation without Samba
> >4 python bindings installed'))
> >
> >I would rather do it once in the beginning and set a flag:
> >
> >try:
> >    import ipaserver.dcerpc
> >     _bindings_installed = True
> >except Exception:
> >    _bindings_installed = False
> >
> >...
> The idea was that this code is only executed on the server. We need to
> differentiate between:
> - running on client
> - running on server, no samba4 python bindings
> - running on server with samba4 python bindings
> 
> By making it executed all time you are affecting the client code as
> well while with current approach it only affects server side.

Across our code base, this situation is currently solved with this
condition:

if api.env.in_server and api.env.context in ['lite', 'server']:
    # try-import block

> 
> 
> >+    def execute(self, *keys, **options):
> >+        # Join domain using full credentials and with random trustdom
> >+        # secret (will be generated by the join method)
> >+        trustinstance = None
> >+        if not _bindings_installed:
> >+            raise errors.NotFound(name=_('AD Trust setup'),
> >+                  reason=_('Cannot perform join operation without Samba
> >4 python bindings installed'))
> >
> >
> >4) Another import inside a function:
> >+        def arcfour_encrypt(key, data):
> >+            from Crypto.Cipher import ARC4
> >+            c = ARC4.new(key)
> >+            return c.encrypt(data)
> Same here, it is only needed on server side.
> 
> Let us get consensus over 3) and 4) and I'll fix patches altogether (and
> push).
> 

Yeah, I would fix in the same way as 3).

Martin