Re: [Freeipa-devel] [PATCH] 0004 User life cycle: support of MODRDN to a new superior

[thierry bordaz]

On 04/08/2015 03:33 PM, Jan Cholasta wrote:

Dne 8.4.2015 v 15:00 thierry bordaz napsal(a):

On 04/08/2015 08:34 AM, Jan Cholasta wrote:

Hi,

Dne 1.4.2015 v 17:40 thierry bordaz napsal(a):

Hello,

In user life cycle, Active entries are moved to Delete 
container and

Delete entries can be moved back to Staging container.
This requires a LDAP modrdn with new superior that is not 
supported

in ldap2.


Since update_entry_rdn() is used only in one spot in baseldap, I think
we can merge it and move_entry_newsuperior() into a single method
move_entry():

def move_entry(self, dn, new_dn, del_old=True):

We can easily detect whether the superior needs to be updated by
comparing dn[1:] and new_dn[1:].


Hello Jan,

Yes that is a good idea to merge those two methods. They both rely on
modrdn and a single method is enough.


Well, I had something like this in mind:

def move_entry(self, dn, new_dn, del_old=True):
assert isinstance(dn, DN)
assert isinstance(new_dn, DN)

if new_dn == dn:
raise errors.EmptyModlist()

new_rdn = new_dn[0]
if new_rdn == dn[0]:
new_rdn = None

new_superior = new_dn[1:]
if new_superior == dn[1:]:
new_superior = None

with self.error_handler():
self.conn.rename_s(dn, new_rdn, new_superior, int(del_old))
time.sleep(.3)  # Give memberOf plugin a chance to work

so that you don't have to care if you should change the RDN or the 
superior and it just does the right thing.






Maybe we can also get rid of del_old, if it's always gonna be True in
our code?


I think it is better to get this interface as close as possible as the
MODRDN call, so that del_old option will be already available for future
usage.
I agree that currently del_old is always true in case of IPA but it
could be the default value.


OK, it's not a big piece of code, so I guess we can leave it.


Thank for the reviews and the help. Here is a new patch.

thierry
From d66d9c9af23975034b2ef91ddbda99567d3d176f Mon Sep 17 00:00:00 2001
From: Thierry bordaz (tbordaz) tbor...@redhat.com
Date: Wed, 1 Apr 2015 16:42:43 +0200
Subject: [PATCH 07/12] User life cycle: allows MODRDN from ldap2

enhance update_entry_rdn so that is allows
to move an entry a new superior

Reviewed By: Jan Cholasta

https://fedorahosted.org/freeipa/ticket/3813
---
 ipalib/plugins/baseldap.py |  6 +++---
 ipapython/ipaldap.py   | 29 +++--
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 4b1c701924d57919538e0c428ea181c2e898505e..127c875b16e3cadd2a73c154e56d0385e1b342e8 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1417,10 +1417,10 @@ class LDAPUpdate(LDAPQuery, crud.Update):
 if self.obj.rdn_is_primary_key and self.obj.primary_key.name in entry_attrs:
 try:
 # RDN change
-self._exc_wrapper(keys, options, ldap.update_entry_rdn)(
+new_dn = DN((self.obj.primary_key.name, entry_attrs[self.obj.primary_key.name]), *entry_attrs.dn[1:])
+self._exc_wrapper(keys, options, ldap.move_entry)(
 entry_attrs.dn,
-RDN((self.obj.primary_key.name,
- entry_attrs[self.obj.primary_key.name])))
+new_dn)
 
 rdnkeys = keys[:-1] + (entry_attrs[self.obj.primary_key.name], )
 entry_attrs.dn = self.obj.get_dn(*rdnkeys)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index ce07006eb790c80fd42bd6eb611732ce9000db13..16cc4871cc2ed76f380cd75525ac39b5adad5bc1 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -581,6 +581,9 @@ class IPASimpleLDAPObject(object):
 dn = str(dn)
 assert isinstance(newrdn, (DN, RDN))
 newrdn = str(newrdn)
+if newsuperior:
+assert isinstance(newsuperior, DN)
+newsuperior = str(newsuperior)
 return self.conn.rename_s(dn, newrdn, newsuperior, delold)
 
 def result(self, msgid=ldap.RES_ANY, all=1, timeout=None):
@@ -1593,21 +1596,35 @@ class LDAPClient(object):
 
 entry.reset_modlist()
 
-def update_entry_rdn(self, dn, new_rdn, del_old=True):
+def move_entry(self, dn, new_dn, del_old=True):
 
-Update entry's relative distinguished name.
+Move an entry (either to a new superior or/and changing relative distinguished name)
 
 Keyword arguments:
+dn: DN of the source entry
+new_dn: DN of the target entry
 del_old -- delete old RDN value (default True)
+
+:raises:
+errors.NotFound if source entry or target superior entry doesn't exist
+errors.EmptyModlist if source and target are identical
 
-
 assert isinstance(dn, DN)
-assert 

Re: [Freeipa-devel] Designing better API compatibility

[Martin Kosek]

On 04/09/2015 09:16 AM, Jan Cholasta wrote:
 Dne 8.4.2015 v 16:44 Martin Kosek napsal(a):
 On 03/20/2015 05:00 PM, Petr Vobornik wrote:
 On 03/20/2015 04:16 PM, Petr Spacek wrote:
 On 20.3.2015 15:51, Nathaniel McCallum wrote:
 On Fri, 2015-03-20 at 09:58 -0400, Simo Sorce wrote:
 On Fri, 2015-03-20 at 14:38 +0100, Martin Kosek wrote:

 Correct. I see 2 approaches here:

 a) Thin client, which simply downloads metadata from the (old)
 server and won't
 use unsupported commands/parameters
 b) Not-so-thin client that knows the minimal API versions of
 commands/parameters (can be annotated in the code), that would
 ping the server
 first to identify it's version, validate that the chosen set of
 commands/parameters is supported on that server and then send the
 commands with
 that version.

 If we have a recognizable error the client can take an optimistic
 approach, send the command normally, if it gets an error that the
 server does not understand it, it checks the version in the reply
 and falls back to an older baseline version of the command (if
 possible) or bails out with an error.

 My understanding was that:

 1. We already publish all the information necessary to implement a
 thin client, and have for some time.
 We certainly have *some* data but real thin client will most likely require
 some changes. Some information like return types and so on are missing.

 2. Thus, the thin client would work on both new and old versions since
 it just simply translates from user input into JSON/XML.

 3. Only plugins with specific client behavior would need to be ported
 to the thin client. A prime example of this is otptoken-add-yubikey.

 My preference is solidly for implementing the thin client first. Once
 we have decoupled the client from the current plugin framework, server-
 side changes can be made in isolation. This decoupling is the move
 that is essentially necessary to provide proper API versioning. And if
 this can't land for 4.2, land it in the next release. I'd rather do
 API-stability correctly and a release later than rushed with
 compromises. We have to live with this forever.
 + all votes I have :-)


 +1

 Ok. So to sum up this thread (and do the actual changes in Trac), in FreeIPA
 4.2, we would:

 1) Prepare the API UI browser or generated API documentation so that people
 could finally see the existing API without having to read the code or inspect
 jquery sent by the Web UI.

 https://fedorahosted.org/freeipa/ticket/3129
 
 This is not related to API compatibility, it just uses the same metadata.

It is not related to API compatibility per se, but very related to better API
consumption and a low hanging fruit we could start with, since we have the
metadata already

 2) Have option for the ipa tool to send version-less command to the server
 which should thus behave as if it is the same version. Bonus points if 
 defaults
 are not filled in this case to prevent unrecoverable Unkown Option errors.

 https://fedorahosted.org/freeipa/ticket/4768
 
 Not sending version and not computing defaults are very different things and
 their implemetantion will be very different too. I would not mix them 
 together.

We are now getting more in the design, but the idea was that sending the
defaults may force server to refuse serving the command even if the caller did
not explicitly requested that option. Even if the caller did not care about the
new default option in 4.x, he would not be able to call the command as it would
be always sent to the old server.

 Rest would be left for later releases. Please holler if there is disagreement
 with this plan.
 
 I agree with Nathaniel that we should do thin client ASAP.

I agree too, but given it is not realistic for 4.2, we need to do at least
something in 4.2 for projects which need to use the CLI against older versions.

Skipping version and client defaults seemed as the low hanging fruit that could
help them. If there is a better idea about what else can be done in 4.2, I am
open to it.

Martin

-- 
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

[Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Rob Crittenden]

Right now when a new master is installed it is not configured with a CA
unless one passes in --setup-ca (or afterward runs ipa-ca-install).

Over and over we've seen people who have multiple masters and a single
CA, in some cases that CA machine is gone, leaving the realm with no CA
at all.

I think this is due to the fact that CA replicas are not created by
default and the users are not aware of the implications of a single
point-of-failure since things otherwise seem to be working.

So perhaps the default should be to install a CA unless the user
requests one not be installed. A related task may be to create an
uninstaller for just the CA.

rob

-- 
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

Re: [Freeipa-devel] Designing better API compatibility

[Petr Vobornik]

On 04/09/2015 09:35 AM, Martin Kosek wrote:

On 04/09/2015 09:16 AM, Jan Cholasta wrote:

Dne 8.4.2015 v 16:44 Martin Kosek napsal(a):

On 03/20/2015 05:00 PM, Petr Vobornik wrote:

On 03/20/2015 04:16 PM, Petr Spacek wrote:

On 20.3.2015 15:51, Nathaniel McCallum wrote:

On Fri, 2015-03-20 at 09:58 -0400, Simo Sorce wrote:

On Fri, 2015-03-20 at 14:38 +0100, Martin Kosek wrote:


Correct. I see 2 approaches here:

a) Thin client, which simply downloads metadata from the (old)
server and won't
use unsupported commands/parameters
b) Not-so-thin client that knows the minimal API versions of
commands/parameters (can be annotated in the code), that would
ping the server
first to identify it's version, validate that the chosen set of
commands/parameters is supported on that server and then send the
commands with
that version.


If we have a recognizable error the client can take an optimistic
approach, send the command normally, if it gets an error that the
server does not understand it, it checks the version in the reply
and falls back to an older baseline version of the command (if
possible) or bails out with an error.


My understanding was that:

1. We already publish all the information necessary to implement a
thin client, and have for some time.

We certainly have *some* data but real thin client will most likely require
some changes. Some information like return types and so on are missing.


2. Thus, the thin client would work on both new and old versions since
it just simply translates from user input into JSON/XML.

3. Only plugins with specific client behavior would need to be ported
to the thin client. A prime example of this is otptoken-add-yubikey.

My preference is solidly for implementing the thin client first. Once
we have decoupled the client from the current plugin framework, server-
side changes can be made in isolation. This decoupling is the move
that is essentially necessary to provide proper API versioning. And if
this can't land for 4.2, land it in the next release. I'd rather do
API-stability correctly and a release later than rushed with
compromises. We have to live with this forever.

+ all votes I have :-)



+1


Ok. So to sum up this thread (and do the actual changes in Trac), in FreeIPA
4.2, we would:

1) Prepare the API UI browser or generated API documentation so that people
could finally see the existing API without having to read the code or inspect
jquery sent by the Web UI.

https://fedorahosted.org/freeipa/ticket/3129


This is not related to API compatibility, it just uses the same metadata.


It is not related to API compatibility per se, but very related to better API
consumption and a low hanging fruit we could start with, since we have the
metadata already


+1




2) Have option for the ipa tool to send version-less command to the server
which should thus behave as if it is the same version. Bonus points if defaults
are not filled in this case to prevent unrecoverable Unkown Option errors.

https://fedorahosted.org/freeipa/ticket/4768


Not sending version and not computing defaults are very different things and
their implemetantion will be very different too. I would not mix them together.


We are now getting more in the design, but the idea was that sending the
defaults may force server to refuse serving the command even if the caller did
not explicitly requested that option. Even if the caller did not care about the
new default option in 4.x, he would not be able to call the command as it would
be always sent to the old server.


+1 that not sending defaults is essential for this case. IMHO we should 
not send them at all.





Rest would be left for later releases. Please holler if there is disagreement
with this plan.


I agree with Nathaniel that we should do thin client ASAP.


I agree too, but given it is not realistic for 4.2, we need to do at least
something in 4.2 for projects which need to use the CLI against older versions.

Skipping version and client defaults seemed as the low hanging fruit that could
help them. If there is a better idea about what else can be done in 4.2, I am
open to it.

Martin




--
Petr Vobornik

--
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

Re: [Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Simo Sorce]

On Thu, 2015-04-09 at 16:52 -0400, Rob Crittenden wrote:
 Simo Sorce wrote:
  On Thu, 2015-04-09 at 15:42 -0400, Rob Crittenden wrote:
  Petr Vobornik wrote:
  On 04/09/2015 04:05 PM, Rob Crittenden wrote:
  Right now when a new master is installed it is not configured with a CA
  unless one passes in --setup-ca (or afterward runs ipa-ca-install).
 
  Over and over we've seen people who have multiple masters and a single
  CA, in some cases that CA machine is gone, leaving the realm with no CA
  at all.
 
  I think this is due to the fact that CA replicas are not created by
  default and the users are not aware of the implications of a single
  point-of-failure since things otherwise seem to be working.
 
  So perhaps the default should be to install a CA unless the user
  requests one not be installed. A related task may be to create an
  uninstaller for just the CA.
 
  rob
 
 
  From a general perspective:
 
  When I hear replica it evokes a clone, something equal/identical.
 
  Based on this, the expected behavior for me would be that:
 
  - if master has DNS and CA, then the new replica would also have DNS and
  CA (without any configuration option needed).
  - if an optional service is missing then replica wouldn't have it as
  well by default
 
  This would required reverse options like: --no-dns.
 
  Pretty much exactly what I was thinking.
 
  For the option I think we should go with a more generic --ca, --dns,
  with the default value matching what the remote master has configured.
 
  But that's bike shedding.
 
  The real question is, what do others think? Is this worth filing a
  ticket for? It would be a subtle but significant change. This might tie
  in nicely with planned topology management too.
  
  I think I would like to see questions in interactive mode, but not force
  CA and DNS to be installed just because the other replica has them.
  
  The replica originating machines has more to do with topology (what
  master you want to replicate off) then features.
  
  So if you are doing an interactive install and the remote replica has CA
  and DNS features, it may be nice to ask: do you want to setup CA too ?
  Do you want to setup DNS too ?
  But not do it by default w/o positive confirmation.
  Esp for DNS it makes little sense as you need a change in DHCP/other
  infra for it to be of any use and all data is in LDAP anyway
  The CA case is a little bit more critical as you noted, but I think
  nagging in interactive is probably good enough.
 
 That's why I suggested this be tied to the topology plugin, so the user
 has a chance to massage things afterward in an easy manner.
 
 A less obtrusive suggestion would be to be to try to count the number of
 CAs and spit out a scary warning if it is just one.

Maybe force CA on if there is only one CA ? (Ie first 2 servers get to
be CAs) then a new CA is force installed only if one of the 2 is
killed ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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

Re: [Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Simo Sorce]

On Thu, 2015-04-09 at 15:42 -0400, Rob Crittenden wrote:
 Petr Vobornik wrote:
  On 04/09/2015 04:05 PM, Rob Crittenden wrote:
  Right now when a new master is installed it is not configured with a CA
  unless one passes in --setup-ca (or afterward runs ipa-ca-install).
 
  Over and over we've seen people who have multiple masters and a single
  CA, in some cases that CA machine is gone, leaving the realm with no CA
  at all.
 
  I think this is due to the fact that CA replicas are not created by
  default and the users are not aware of the implications of a single
  point-of-failure since things otherwise seem to be working.
 
  So perhaps the default should be to install a CA unless the user
  requests one not be installed. A related task may be to create an
  uninstaller for just the CA.
 
  rob
 
  
  From a general perspective:
  
  When I hear replica it evokes a clone, something equal/identical.
  
  Based on this, the expected behavior for me would be that:
  
  - if master has DNS and CA, then the new replica would also have DNS and
  CA (without any configuration option needed).
  - if an optional service is missing then replica wouldn't have it as
  well by default
  
  This would required reverse options like: --no-dns.
 
 Pretty much exactly what I was thinking.
 
 For the option I think we should go with a more generic --ca, --dns,
 with the default value matching what the remote master has configured.
 
 But that's bike shedding.
 
 The real question is, what do others think? Is this worth filing a
 ticket for? It would be a subtle but significant change. This might tie
 in nicely with planned topology management too.

I think I would like to see questions in interactive mode, but not force
CA and DNS to be installed just because the other replica has them.

The replica originating machines has more to do with topology (what
master you want to replicate off) then features.

So if you are doing an interactive install and the remote replica has CA
and DNS features, it may be nice to ask: do you want to setup CA too ?
Do you want to setup DNS too ?
But not do it by default w/o positive confirmation.
Esp for DNS it makes little sense as you need a change in DHCP/other
infra for it to be of any use and all data is in LDAP anyway
The CA case is a little bit more critical as you noted, but I think
nagging in interactive is probably good enough.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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

Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so

[Luc de Louw]

On 04/09/2015 02:28 PM, Jan Cholasta wrote:

Let's say you now introduce --no-cr flag. What if we decide to change
the default to False? How would you then change the option/API?


You would have to add --cr flag.


That was the point - some clients would send ct flag, some no_cr
and there
would have to be special handling.


It is more flexible IMO to just use something like

--cr=TRUE|FALSE with TRUE being the default


I would say --append-cr=TRUE|FALSE with no default, meaning do not
add the flag
to the config at all.


I though the idea was to append the CR by default, i.e.
--append-cr=TRUE|FALSE
with TRUE being the default.



If you want to hardcode the default into the plugin, there is no benefit
in using Bool over Flag, because Flag is actually a Bool with hardcoded
default value.



I actually started with a bool, default=True. I had the problem that the 
Default value was ignored, the value was None.


Changing the default behavior is IMHO bad anyway does not matter if Bool 
or Flag.


Please advise what is you wish to be implemented :-)

Thanks,

Luc

--
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

Re: [Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Rob Crittenden]

Simo Sorce wrote:
 On Thu, 2015-04-09 at 15:42 -0400, Rob Crittenden wrote:
 Petr Vobornik wrote:
 On 04/09/2015 04:05 PM, Rob Crittenden wrote:
 Right now when a new master is installed it is not configured with a CA
 unless one passes in --setup-ca (or afterward runs ipa-ca-install).

 Over and over we've seen people who have multiple masters and a single
 CA, in some cases that CA machine is gone, leaving the realm with no CA
 at all.

 I think this is due to the fact that CA replicas are not created by
 default and the users are not aware of the implications of a single
 point-of-failure since things otherwise seem to be working.

 So perhaps the default should be to install a CA unless the user
 requests one not be installed. A related task may be to create an
 uninstaller for just the CA.

 rob


 From a general perspective:

 When I hear replica it evokes a clone, something equal/identical.

 Based on this, the expected behavior for me would be that:

 - if master has DNS and CA, then the new replica would also have DNS and
 CA (without any configuration option needed).
 - if an optional service is missing then replica wouldn't have it as
 well by default

 This would required reverse options like: --no-dns.

 Pretty much exactly what I was thinking.

 For the option I think we should go with a more generic --ca, --dns,
 with the default value matching what the remote master has configured.

 But that's bike shedding.

 The real question is, what do others think? Is this worth filing a
 ticket for? It would be a subtle but significant change. This might tie
 in nicely with planned topology management too.
 
 I think I would like to see questions in interactive mode, but not force
 CA and DNS to be installed just because the other replica has them.
 
 The replica originating machines has more to do with topology (what
 master you want to replicate off) then features.
 
 So if you are doing an interactive install and the remote replica has CA
 and DNS features, it may be nice to ask: do you want to setup CA too ?
 Do you want to setup DNS too ?
 But not do it by default w/o positive confirmation.
 Esp for DNS it makes little sense as you need a change in DHCP/other
 infra for it to be of any use and all data is in LDAP anyway
 The CA case is a little bit more critical as you noted, but I think
 nagging in interactive is probably good enough.

That's why I suggested this be tied to the topology plugin, so the user
has a chance to massage things afterward in an easy manner.

A less obtrusive suggestion would be to be to try to count the number of
CAs and spit out a scary warning if it is just one.

rob

-- 
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

Re: [Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Rob Crittenden]

Petr Vobornik wrote:
 On 04/09/2015 04:05 PM, Rob Crittenden wrote:
 Right now when a new master is installed it is not configured with a CA
 unless one passes in --setup-ca (or afterward runs ipa-ca-install).

 Over and over we've seen people who have multiple masters and a single
 CA, in some cases that CA machine is gone, leaving the realm with no CA
 at all.

 I think this is due to the fact that CA replicas are not created by
 default and the users are not aware of the implications of a single
 point-of-failure since things otherwise seem to be working.

 So perhaps the default should be to install a CA unless the user
 requests one not be installed. A related task may be to create an
 uninstaller for just the CA.

 rob

 
 From a general perspective:
 
 When I hear replica it evokes a clone, something equal/identical.
 
 Based on this, the expected behavior for me would be that:
 
 - if master has DNS and CA, then the new replica would also have DNS and
 CA (without any configuration option needed).
 - if an optional service is missing then replica wouldn't have it as
 well by default
 
 This would required reverse options like: --no-dns.

Pretty much exactly what I was thinking.

For the option I think we should go with a more generic --ca, --dns,
with the default value matching what the remote master has configured.

But that's bike shedding.

The real question is, what do others think? Is this worth filing a
ticket for? It would be a subtle but significant change. This might tie
in nicely with planned topology management too.

rob

-- 
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

Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so

[Jan Cholasta]

Dne 9.4.2015 v 12:42 Martin Kosek napsal(a):

On 04/09/2015 12:30 PM, Jan Cholasta wrote:

Dne 8.4.2015 v 22:52 Martin Kosek napsal(a):

On 04/08/2015 06:03 PM, Nathaniel McCallum wrote:

On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote:

On 08/04/15 17:46, Luc de Louw wrote:

On 04/08/2015 05:14 PM, Martin Basti wrote:

On 08/04/15 17:12, Luc de Louw wrote:


On 04/08/2015 05:05 PM, Martin Basti wrote:

On 08/04/15 16:55, Nathaniel McCallum wrote:

On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote:

Hi there,

At the moment ipa otptoken-add-yubikey does not add the
parameter
APPEND_CR. This prevents submit the password+OTP.
APPEND_CR is
usually
very handy, most people use this functionality.

The patch changes the behavior to set APPEND_CR by
default and let
the
user override this by using the the --do-not-append-cr
option.

This patch is very helpful and I would like to see it
merged. Thanks
Luc!

1. This patch needs to be formatted according to the
FreeIPA
formatting. See:
https://www.freeipa.org/page/Contribute/Patch_Format

2. The flag should be named no_cr instead of
do_not_append_cr.

3. The comment is not necessary since what the code does
is obvious.

Nathaniel


Hello,

4) this patch changes API, so please run ./makeapi to
regenerate
API.txt
file and add changes into patch + please bum API minor
version in
VERSION file

thanks.




Hi,

When running makeaip, I get the following error:
File /home/luc/freeipa/ipalib/constants.py, line 25, in
module
  from ipaplatform.paths import paths
ImportError: No module named paths

Any hints?

The other changes are ready to submit.

Thanks,

Luc

You may need to run 'make version-upgrade' or 'make' to prepare
the
module.

If it will not work, you can send incomplete patch, I will add
API
changes there, just bump VERSION please



Martin,

Thanks for your hints, seems to work, please have a look at it...

Thanks,

Luc



Thanks,

please change the comment too

-IPA_API_VERSION_MINOR=116
+IPA_API_VERSION_MINOR=117
# Last change: tbordaz - Add stageuser_add command

Otherwise patch looks good, but Nathaniel is the OTP guru, he should
say
final ack.


I'm also a tough reviewer. :)

1. Remove the unnecessary code comment.

2. There appears to be inconsistent indentation in the flag parameter
specification. It is probably a mix of tabs and spaces.

3. The git commit comment should contain one short summary line
without terminating punctuation followed by any necessary explanatory
paragraphs. You can change this via the --amend option to git
commit. Try the following:

Enable YubiKey carriage return emission via otptoken-add-yubikey

Before this patch, YubiKeys programmed by IPA would not emit the
carriage return character at the end of the OTP value. This requires
the user to press his YubiKey and then (unnecessarily) the Enter or
Return key. After this patch, the user only needs to press the YubiKey.

Should a user desire to omit the carriage return character, the --no-
cr option can be specified.

Nathaniel



One more note to the API. By my experience, using a Flag for a boolean
data input has often proved to be a bad call.

Let's say you now introduce --no-cr flag. What if we decide to change
the default to False? How would you then change the option/API?


You would have to add --cr flag.


That was the point - some clients would send ct flag, some no_cr and there
would have to be special handling.


It is more flexible IMO to just use something like

--cr=TRUE|FALSE with TRUE being the default


I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag
to the config at all.


I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE
with TRUE being the default.



If you want to hardcode the default into the plugin, there is no benefit 
in using Bool over Flag, because Flag is actually a Bool with hardcoded 
default value.


--
Jan Cholasta

--
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

Re: [Freeipa-devel] [PATCH] 811 performance: faster DN implementation

[Petr Vobornik]

On 04/02/2015 11:54 AM, Petr Viktorin wrote:

On 03/31/2015 12:11 PM, Petr Vobornik wrote:

The only different thing is a lack of utf-8 encoded str support(as
input). I don't know how much important the support is.


I don't think that support is too important (assuming IPA doesn't use
it!). However, the behavior with this patch is dangerous.
It allows unicode and ASCII strings, but fails on non-ASCII strings.
That means things will usually work, but as soon as a non-ASCII
component is introduced at the wrong place, you get an error.

Restoring support for utf-8 encoded str looks easy to do; here's a patch
you can squash in. Or did I miss something?


I also had to fix creation of AVAs to support utf-8 encoded str as input 
for attr and value (separately).





maybe it could be attached to ticket
https://fedorahosted.org/freeipa/ticket/4947
-
DN code was optimized to be faster if DNs are created from string. This
is the major use case, since most DNs come from LDAP.

With this patch, DN creation is almost 8-10x faster (with 30K-100K DNs).

Second mojor use case - deepcopy in LDAPEntry is about 20x faster - done
by custom __deepcopy__ function.

The major change is that DN is no longer internally composed  of RDNs
and AVAs but it rather keeps the data in open ldap format - the same as
output of str2dn function. Therefore, for immutable DNs, no other
transformations are required on instantiation.

The format is:

DN: [RDN, RDN,...]
RDN: [AVA, AVA,...]
AVA: ['utf-8 encoded str - attr', 'utf-8 encode str -value', FLAG]
FLAG: int

Further indexing of DN object constructs an RDN which is just an
encapsulation of the RDN part of open ldap representation. Indexing of
RDN constructs AVA in the same fashion.

Obtained EditableAVA, EditableRDN from EditableDN shares the respected
lists of the open ldap repr. so that the change of value or attr is
reflected in parent object.



Looks good. A couple of comments:

RDN.to_openldap: _avas always has 3 components, right? I'd prefer
`list(a)` over `[a[0], a[1], a[2]]`.  Similarly for tuple in in __add__
and RDN._avas_from_sequence.


Fixed



DN._rdns_from_value: the error message at the end is wrong, RDN is also
accepted. (And, `type(value)` would be more informative than
`value.__class__.__name__`.)


Fixed



You can optimize __deepcopy__ for immutable DNs even further: just
return self!


Fixed, but kept part for EditableDN



In DN.find  rfind, RDNs are not accepted but the error message says
they are.


messages fixed



You removed the newline at end of file.



line readded
--
Petr Vobornik
From 6289202ca5c5d24c1b07754d19a292e66cfd5df2 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 25 Mar 2015 13:39:43 +0100
Subject: [PATCH] performance: faster DN implementation

DN code was optimized to be faster if DNs are created from string. This is
the major use case, since most DNs come from LDAP.

With this patch, DN creation is almost 8-10x faster (with 30K-100K DNs).

Second mojor use case - deepcopy in LDAPEntry is about 20x faster - done by
custom __deepcopy__ function.

The major change is that DN is no longer internally composed  of RDNs and
AVAs but it rather keeps the data in open ldap format - the same as output
of str2dn function. Therefore, for immutable DNs, no other transformations
are required on instantiation.

The format is:

DN: [RDN, RDN,...]
RDN: [AVA, AVA,...]
AVA: ['utf-8 encoded str - attr', 'utf-8 encode str -value', FLAG]
FLAG: int

Further indexing of DN object constructs an RDN which is just an encapsulation
of the RDN part of open ldap representation. Indexing of RDN constructs AVA in
the same fashion.

Obtained EditableAVA, EditableRDN from EditableDN shares the respected lists
of the open ldap repr. so that the change of value or attr is reflected in
parent object.
---
 ipapython/dn.py| 595 ++---
 ipatests/test_ipapython/test_dn.py |  17 +-
 2 files changed, 306 insertions(+), 306 deletions(-)

diff --git a/ipapython/dn.py b/ipapython/dn.py
index 834291fbe8696622162efa5193622d74f11f25ca..5b6570770d587937c87380f7ea19e999c3d8867d 100644
--- a/ipapython/dn.py
+++ b/ipapython/dn.py
@@ -497,6 +497,97 @@ def _adjust_indices(start, end, length):
 
 return start, end
 
+
+def _normalize_ava_input(val):
+if not isinstance(val, basestring):
+val = unicode(val).encode('utf-8')
+elif isinstance(val, unicode):
+val = val.encode('utf-8')
+return val
+
+
+def str2rdn(value):
+try:
+rdns = str2dn(value.encode('utf-8'))
+except DECODING_ERROR:
+raise ValueError(malformed AVA string = \%s\ % value)
+if len(rdns) != 1:
+raise ValueError(multiple RDN's specified by \%s\ % (value))
+return rdns[0]
+
+
+def get_ava(*args, **kwds):
+
+Get AVA from args in open ldap format(raw). Optimized for construction
+from openldap format.
+
+Allowed formats of argument list:
+1) three args - open ldap 

Re: [Freeipa-devel] [PATCH] 809 speed up convert_attribute_members

[Petr Vobornik]

On 04/02/2015 09:47 AM, Jan Cholasta wrote:

Hi,

Dne 31.3.2015 v 12:11 Petr Vobornik napsal(a):

A workaround to avoid usage of slow LDAPEntry._sync_attr #4946.

I originally wanted to avoid DN processing as well but we can't do that
because of DNs which are encoded - e.g. contains '+' or ','. Therefore
patch 811 - faster DN implementation is very useful. Also patch 809 is
useful to avoid high load of 389.

https://fedorahosted.org/freeipa/ticket/4965



1)

+dn = container_dns.get(ldap_obj_name, None)
+if not dn:
+ldap_obj = self.api.Object[ldap_obj_name]
+dn = DN(ldap_obj.container_dn, api.env.basedn)
+container_dns[ldap_obj_name] = dn
+return dn

 a) The second argument of .get() is None by default

 b) not dn matches None as well as empty DNs, use dn is not None
(it's not that there could be empty DNs here, but let's not give a
potential reader the wrong idea)

 c) It would be better to catch KeyError rather than call .get() and
check the result:

 try:
 dn = container_dns[ldap_obj_name]
 except KeyError:
 dn = ...
 container_dns[ldap_obj_name] = dn


Changed




2) Does get_new_attr() actually provide any speed up? Unless I'm missing
something, it just mirrors the virtual member attributes already readily
available from entry_attrs in new_attrs.


Yes, a bit. With 30K members and my vm get_new_attr takes ~ 0.114s. 
setdefault takes ~ 0.686s which is about 7-10% of the entire 
convert_attribute_members. Pure dict is faster.





3) get_container_dn() and get_new_attr() do not need to be functions,
since each is called just from a single spot.


Changed




4) memberdn = DN(member) could be one for loop up.



Changed



Here's what I ended up with trying to fix the above (untested):

 for attr in self.attribute_members:
 try:
 value = entry_attrs.raw[attr]
 except KeyError:
 continue
 del entry_attrs[attr]

 ldap_objs = {}
 for ldap_obj_name in self.attribute_members[attr]:
 ldap_obj = self.api.Object[ldap_obj_name]
 container_dn = DN(ldap_obj.container_dn, api.env.basedn)
 ldap_objs[container_dn] = ldap_obj

 for member in value:
 memberdn = DN(member)
 try:
 ldap_obj = ldap_objs[DN(*memberdn[1:])]
 except KeyError:
 continue

 new_attr = '%s_%s' % (attr, ldap_obj.name)
 new_value = ldap_obj.get_primary_key_from_dn(memberdn)
 entry_attrs.setdefault(new_attr, []).append(new_value)


Without any modifications the code is ~ 2.3x slower than mine. In patch 
811 DN's slice, __hash__ and __eq__ functions are optimized.




Honza


--
Petr Vobornik
From e615fde310ce7b8fc61f2d338b7e8f0f635fa175 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 18 Mar 2015 18:48:54 +0100
Subject: [PATCH] speed up convert_attribute_members

A workaround to avoid usage of slow LDAPEntry._sync_attr #4946

https://fedorahosted.org/freeipa/ticket/4965
---
 ipalib/plugins/baseldap.py | 38 +-
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 4b1c701924d57919538e0c428ea181c2e898505e..c9f98ed12d3a584597af9b0be08361bceca620e7 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -631,18 +631,38 @@ class LDAPObject(Object):
 def convert_attribute_members(self, entry_attrs, *keys, **options):
 if options.get('raw', False):
 return
+
+container_dns = {}
+new_attrs = {}
+
 for attr in self.attribute_members:
-for member in entry_attrs.setdefault(attr, []):
-for ldap_obj_name in self.attribute_members[attr]:
-ldap_obj = self.api.Object[ldap_obj_name]
-container_dn = DN(ldap_obj.container_dn, api.env.basedn)
-if member.endswith(container_dn):
-new_attr = '%s_%s' % (attr, ldap_obj.name)
-entry_attrs.setdefault(new_attr, []).append(
-ldap_obj.get_primary_key_from_dn(member)
-)
+try:
+value = entry_attrs.raw[attr]
+except KeyError:
+continue
 del entry_attrs[attr]
 
+for member in value:
+memberdn = DN(member)
+for ldap_obj_name in self.attribute_members[attr]:
+ldap_obj = self.api.Object[ldap_obj_name]
+try:
+container_dn = container_dns[ldap_obj_name]
+except KeyError:
+

Re: [Freeipa-devel] [PATCH 408-423] ldap: Remove IPASimpleLDAPObject

[Petr Viktorin]

On 04/08/2015 03:18 PM, Jan Cholasta wrote:

Hi,

the attached patches remove IPASimpleLDAPObject from ipaldap.

As a result, the one and only IPA LDAP API is the LDAPClient API.


This is definitely an improvement :)

0408: ACK  (woohoo!)
0409: ACK
0410:
I quite like the new __init__ signature, and the context manager 
functionality.
Can you add a comment for the `object.__setattr__(self, '_conn', None)` 
in _disconnect? It's a real eyesore.

0411: ACK
0412: Can _force_schema_updates be set already in __init__?
0413: ACK
0414: ACK
0415: ACK
0416: I think you should show off the `with` statement support here.
0417: ... and here
0418: ACK
0419: ACK
0420: ACK
0421: ACK
0422: ACK, and good riddance


--
Petr Viktorin

--
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

Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so

[Jan Cholasta]

Dne 8.4.2015 v 22:52 Martin Kosek napsal(a):

On 04/08/2015 06:03 PM, Nathaniel McCallum wrote:

On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote:

On 08/04/15 17:46, Luc de Louw wrote:

On 04/08/2015 05:14 PM, Martin Basti wrote:

On 08/04/15 17:12, Luc de Louw wrote:


On 04/08/2015 05:05 PM, Martin Basti wrote:

On 08/04/15 16:55, Nathaniel McCallum wrote:

On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote:

Hi there,

At the moment ipa otptoken-add-yubikey does not add the
parameter
APPEND_CR. This prevents submit the password+OTP.
APPEND_CR is
usually
very handy, most people use this functionality.

The patch changes the behavior to set APPEND_CR by
default and let
the
user override this by using the the --do-not-append-cr
option.

This patch is very helpful and I would like to see it
merged. Thanks
Luc!

1. This patch needs to be formatted according to the
FreeIPA
formatting. See:
https://www.freeipa.org/page/Contribute/Patch_Format

2. The flag should be named no_cr instead of
do_not_append_cr.

3. The comment is not necessary since what the code does
is obvious.

Nathaniel


Hello,

4) this patch changes API, so please run ./makeapi to
regenerate
API.txt
file and add changes into patch + please bum API minor
version in
VERSION file

thanks.




Hi,

When running makeaip, I get the following error:
   File /home/luc/freeipa/ipalib/constants.py, line 25, in
module
 from ipaplatform.paths import paths
ImportError: No module named paths

Any hints?

The other changes are ready to submit.

Thanks,

Luc

You may need to run 'make version-upgrade' or 'make' to prepare
the
module.

If it will not work, you can send incomplete patch, I will add
API
changes there, just bump VERSION please



Martin,

Thanks for your hints, seems to work, please have a look at it...

Thanks,

Luc



Thanks,

please change the comment too

-IPA_API_VERSION_MINOR=116
+IPA_API_VERSION_MINOR=117
   # Last change: tbordaz - Add stageuser_add command

Otherwise patch looks good, but Nathaniel is the OTP guru, he should
say
final ack.


I'm also a tough reviewer. :)

1. Remove the unnecessary code comment.

2. There appears to be inconsistent indentation in the flag parameter
specification. It is probably a mix of tabs and spaces.

3. The git commit comment should contain one short summary line
without terminating punctuation followed by any necessary explanatory
paragraphs. You can change this via the --amend option to git
commit. Try the following:

Enable YubiKey carriage return emission via otptoken-add-yubikey

Before this patch, YubiKeys programmed by IPA would not emit the
carriage return character at the end of the OTP value. This requires
the user to press his YubiKey and then (unnecessarily) the Enter or
Return key. After this patch, the user only needs to press the YubiKey.

Should a user desire to omit the carriage return character, the --no-
cr option can be specified.

Nathaniel



One more note to the API. By my experience, using a Flag for a boolean
data input has often proved to be a bad call.

Let's say you now introduce --no-cr flag. What if we decide to change
the default to False? How would you then change the option/API?


You would have to add --cr flag.



It is more flexible IMO to just use something like

--cr=TRUE|FALSE with TRUE being the default


I would say --append-cr=TRUE|FALSE with no default, meaning do not add 
the flag to the config at all.




Martin




--
Jan Cholasta

--
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

Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so

[Martin Kosek]

On 04/09/2015 12:30 PM, Jan Cholasta wrote:
 Dne 8.4.2015 v 22:52 Martin Kosek napsal(a):
 On 04/08/2015 06:03 PM, Nathaniel McCallum wrote:
 On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote:
 On 08/04/15 17:46, Luc de Louw wrote:
 On 04/08/2015 05:14 PM, Martin Basti wrote:
 On 08/04/15 17:12, Luc de Louw wrote:

 On 04/08/2015 05:05 PM, Martin Basti wrote:
 On 08/04/15 16:55, Nathaniel McCallum wrote:
 On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote:
 Hi there,

 At the moment ipa otptoken-add-yubikey does not add the
 parameter
 APPEND_CR. This prevents submit the password+OTP.
 APPEND_CR is
 usually
 very handy, most people use this functionality.

 The patch changes the behavior to set APPEND_CR by
 default and let
 the
 user override this by using the the --do-not-append-cr
 option.
 This patch is very helpful and I would like to see it
 merged. Thanks
 Luc!

 1. This patch needs to be formatted according to the
 FreeIPA
 formatting. See:
 https://www.freeipa.org/page/Contribute/Patch_Format

 2. The flag should be named no_cr instead of
 do_not_append_cr.

 3. The comment is not necessary since what the code does
 is obvious.

 Nathaniel

 Hello,

 4) this patch changes API, so please run ./makeapi to
 regenerate
 API.txt
 file and add changes into patch + please bum API minor
 version in
 VERSION file

 thanks.



 Hi,

 When running makeaip, I get the following error:
File /home/luc/freeipa/ipalib/constants.py, line 25, in
 module
  from ipaplatform.paths import paths
 ImportError: No module named paths

 Any hints?

 The other changes are ready to submit.

 Thanks,

 Luc
 You may need to run 'make version-upgrade' or 'make' to prepare
 the
 module.

 If it will not work, you can send incomplete patch, I will add
 API
 changes there, just bump VERSION please


 Martin,

 Thanks for your hints, seems to work, please have a look at it...

 Thanks,

 Luc


 Thanks,

 please change the comment too

 -IPA_API_VERSION_MINOR=116
 +IPA_API_VERSION_MINOR=117
# Last change: tbordaz - Add stageuser_add command

 Otherwise patch looks good, but Nathaniel is the OTP guru, he should
 say
 final ack.

 I'm also a tough reviewer. :)

 1. Remove the unnecessary code comment.

 2. There appears to be inconsistent indentation in the flag parameter
 specification. It is probably a mix of tabs and spaces.

 3. The git commit comment should contain one short summary line
 without terminating punctuation followed by any necessary explanatory
 paragraphs. You can change this via the --amend option to git
 commit. Try the following:

 Enable YubiKey carriage return emission via otptoken-add-yubikey

 Before this patch, YubiKeys programmed by IPA would not emit the
 carriage return character at the end of the OTP value. This requires
 the user to press his YubiKey and then (unnecessarily) the Enter or
 Return key. After this patch, the user only needs to press the YubiKey.

 Should a user desire to omit the carriage return character, the --no-
 cr option can be specified.

 Nathaniel


 One more note to the API. By my experience, using a Flag for a boolean
 data input has often proved to be a bad call.

 Let's say you now introduce --no-cr flag. What if we decide to change
 the default to False? How would you then change the option/API?
 
 You would have to add --cr flag.

That was the point - some clients would send ct flag, some no_cr and there
would have to be special handling.

 It is more flexible IMO to just use something like

 --cr=TRUE|FALSE with TRUE being the default
 
 I would say --append-cr=TRUE|FALSE with no default, meaning do not add the 
 flag
 to the config at all.

I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE
with TRUE being the default.

-- 
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

Re: [Freeipa-devel] Proposal: reverse stance on installing CA on new masters

[Petr Vobornik]

On 04/09/2015 04:05 PM, Rob Crittenden wrote:

Right now when a new master is installed it is not configured with a CA
unless one passes in --setup-ca (or afterward runs ipa-ca-install).

Over and over we've seen people who have multiple masters and a single
CA, in some cases that CA machine is gone, leaving the realm with no CA
at all.

I think this is due to the fact that CA replicas are not created by
default and the users are not aware of the implications of a single
point-of-failure since things otherwise seem to be working.

So perhaps the default should be to install a CA unless the user
requests one not be installed. A related task may be to create an
uninstaller for just the CA.

rob



From a general perspective:

When I hear replica it evokes a clone, something equal/identical.

Based on this, the expected behavior for me would be that:

- if master has DNS and CA, then the new replica would also have DNS and 
CA (without any configuration option needed).
- if an optional service is missing then replica wouldn't have it as 
well by default


This would required reverse options like: --no-dns.
--
Petr Vobornik

--
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

Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Simo Sorce]

On Thu, 2015-04-09 at 15:38 +0200, Jan Cholasta wrote:
 Dne 9.4.2015 v 14:41 Simo Sorce napsal(a):
  On Wed, 2015-03-25 at 11:52 +0100, Martin Babinsky wrote:
  On 03/23/2015 03:13 PM, Simo Sorce wrote:
  On Mon, 2015-03-23 at 14:22 +0100, Petr Spacek wrote:
  On 23.3.2015 14:08, Simo Sorce wrote:
  On Mon, 2015-03-23 at 12:48 +0100, Martin Babinsky wrote:
  On 03/17/2015 06:00 PM, Simo Sorce wrote:
  On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:
  On 03/16/2015 12:15 PM, Martin Kosek wrote:
  On 03/13/2015 05:37 PM, Martin Babinsky wrote:
  Attaching the next iteration of patches.
 
  I have tried my best to reword the ipa-client-install man page bit 
  about the
  new option. Any suggestions to further improve it are welcome.
 
  I have also slightly modified the 'kinit_keytab' function so that 
  in Kerberos
  errors are reported for each attempt and the text of the last 
  error is retained
  when finally raising exception.
 
  The approach looks very good. I think that my only concern with 
  this patch is
  this part:
 
  +ccache.init_creds_keytab(keytab=ktab, principal=princ)
  ...
  +except krbV.Krb5Error as e:
  +last_exc = str(e)
  +root_logger.debug(Attempt %d/%d: failed: %s
  +  % (attempt, attempts, last_exc))
  +time.sleep(1)
  +
  +root_logger.debug(Maximum number of attempts (%d) reached
  +  % attempts)
  +raise StandardError(Error initializing principal %s: %s
  +% (principal, last_exc))
 
  The problem here is that this function will raise the super-generic
  StandardError instead of the proper with all the context and 
  information about
  the error that the caller can then process.
 
  I think that
 
  except krbV.Krb5Error as e:
  if attempt == max_attempts:
  log something
  raise
 
  would be better.
 
 
  Yes that seems reasonable. I'm just thinking whether we should 
  re-raise
  Krb5Error or raise ipalib.errors.KerberosError? the latter options 
  makes
  more sense to me as we would not have to additionally import 
  Krb5Error
  everywhere and it would also make the resulting errors more 
  consistent.
 
  I am thinking about someting like this:
 
  except krbV.Krb5Error as e:
 if attempt == attempts:
 # log that we have reaches maximum number of attempts
 raise KerberosError(minor=str(e))
 
  What do you think?
 
  Are you retrying on any error ?
  Please do *not* do that, if you retry many times on an error that
  indicates the password is wrong you may end up locking an 
  administrative
  account. If you want to retry you should do it only for very specific
  timeout errors.
 
  Simo.
 
 
  I have taken a look at the logs attached to the original BZ
  (https://bugzilla.redhat.com/show_bug.cgi?id=1161722).
 
  In ipaclient-install.log the kinit error is:
 
  Cannot contact any KDC for realm 'ITW.USPTO.GOV' while getting initial
  credentials
 
  which can be translated to krbV.KRB5_KDC_UNREACH error. However,
  krb5kdc.log (http://pastebin.test.redhat.com/271394) reports errors
  which are seemingly unrelated to the root cause (kinit timing out on
  getting host TGT).
 
  Thus I'm not quite sure which errors should we chceck against in this
  case, anyone care to advise? These are potential candidates:
 
  KRB5KDC_ERR_SVC_UNAVAILABLE, A service is not available that is
  required to process the request
  KRB5KRB_ERR_RESPONSE_TOO_BIG,Response too big for UDP, retry with 
  TCP
  KRB5_REALM_UNKNOWN,  Cannot find KDC for requested realm
  KRB5_KDC_UNREACH,Cannot contact any KDC for requested realm
 
 
  The only ones that you should retry on, at first glance are
  KRB5_KDC_UNREACH, KRB5KDC_ERR_SVC_UNAVAILABLE.
 
  You should never see KRB5KRB_ERR_RESPONSE_TOO_BIG in the script as it
  should be handled automatically by the library, and if you get
  KRB5_REALM_UNKNOWN I do not think that retrying will make any
  difference.
 
  I might be wrong but I was under the impression that this feature was 
  also for
  workarounding replication delay - service is not available / key is not
  present / something like that.
 
  (This could happen if host/principal was added to one server but then the
  client connected to another server or so.)
 
  If we have that problem we should instead use a temporary krb5.conf file
  that lists explicitly only the server we are joining.
 
  Simo.
 
 
  This is already done since ipa-3-0: by default only one server/KDC is
  used during client install so there are actually no problems with
  replication delay, only with KDC timeouts.
 
  Anyway I'm sending updated patches.
 
  LGTM!
 
  Simo.
 
 
 
 Some comments:
 
 Patch 15:
 
 1) The functions should be as similar as possible:
 
  a) kinit_password() should have a 'ccache_path' argument instead of 
 passing the path in KRB5CCNAME in 

Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Jan Cholasta]

Dne 9.4.2015 v 14:41 Simo Sorce napsal(a):

On Wed, 2015-03-25 at 11:52 +0100, Martin Babinsky wrote:

On 03/23/2015 03:13 PM, Simo Sorce wrote:

On Mon, 2015-03-23 at 14:22 +0100, Petr Spacek wrote:

On 23.3.2015 14:08, Simo Sorce wrote:

On Mon, 2015-03-23 at 12:48 +0100, Martin Babinsky wrote:

On 03/17/2015 06:00 PM, Simo Sorce wrote:

On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:

On 03/16/2015 12:15 PM, Martin Kosek wrote:

On 03/13/2015 05:37 PM, Martin Babinsky wrote:

Attaching the next iteration of patches.

I have tried my best to reword the ipa-client-install man page bit about the
new option. Any suggestions to further improve it are welcome.

I have also slightly modified the 'kinit_keytab' function so that in Kerberos
errors are reported for each attempt and the text of the last error is retained
when finally raising exception.


The approach looks very good. I think that my only concern with this patch is
this part:

+ccache.init_creds_keytab(keytab=ktab, principal=princ)
...
+except krbV.Krb5Error as e:
+last_exc = str(e)
+root_logger.debug(Attempt %d/%d: failed: %s
+  % (attempt, attempts, last_exc))
+time.sleep(1)
+
+root_logger.debug(Maximum number of attempts (%d) reached
+  % attempts)
+raise StandardError(Error initializing principal %s: %s
+% (principal, last_exc))

The problem here is that this function will raise the super-generic
StandardError instead of the proper with all the context and information about
the error that the caller can then process.

I think that

except krbV.Krb5Error as e:
if attempt == max_attempts:
log something
raise

would be better.



Yes that seems reasonable. I'm just thinking whether we should re-raise
Krb5Error or raise ipalib.errors.KerberosError? the latter options makes
more sense to me as we would not have to additionally import Krb5Error
everywhere and it would also make the resulting errors more consistent.

I am thinking about someting like this:

except krbV.Krb5Error as e:
   if attempt == attempts:
   # log that we have reaches maximum number of attempts
   raise KerberosError(minor=str(e))

What do you think?


Are you retrying on any error ?
Please do *not* do that, if you retry many times on an error that
indicates the password is wrong you may end up locking an administrative
account. If you want to retry you should do it only for very specific
timeout errors.

Simo.



I have taken a look at the logs attached to the original BZ
(https://bugzilla.redhat.com/show_bug.cgi?id=1161722).

In ipaclient-install.log the kinit error is:

Cannot contact any KDC for realm 'ITW.USPTO.GOV' while getting initial
credentials

which can be translated to krbV.KRB5_KDC_UNREACH error. However,
krb5kdc.log (http://pastebin.test.redhat.com/271394) reports errors
which are seemingly unrelated to the root cause (kinit timing out on
getting host TGT).

Thus I'm not quite sure which errors should we chceck against in this
case, anyone care to advise? These are potential candidates:

KRB5KDC_ERR_SVC_UNAVAILABLE, A service is not available that is
required to process the request
KRB5KRB_ERR_RESPONSE_TOO_BIG,Response too big for UDP, retry with TCP
KRB5_REALM_UNKNOWN,  Cannot find KDC for requested realm
KRB5_KDC_UNREACH,Cannot contact any KDC for requested realm



The only ones that you should retry on, at first glance are
KRB5_KDC_UNREACH, KRB5KDC_ERR_SVC_UNAVAILABLE.

You should never see KRB5KRB_ERR_RESPONSE_TOO_BIG in the script as it
should be handled automatically by the library, and if you get
KRB5_REALM_UNKNOWN I do not think that retrying will make any
difference.


I might be wrong but I was under the impression that this feature was also for
workarounding replication delay - service is not available / key is not
present / something like that.

(This could happen if host/principal was added to one server but then the
client connected to another server or so.)


If we have that problem we should instead use a temporary krb5.conf file
that lists explicitly only the server we are joining.

Simo.



This is already done since ipa-3-0: by default only one server/KDC is
used during client install so there are actually no problems with
replication delay, only with KDC timeouts.

Anyway I'm sending updated patches.


LGTM!

Simo.




Some comments:

Patch 15:

1) The functions should be as similar as possible:

a) kinit_password() should have a 'ccache_path' argument instead of 
passing the path in KRB5CCNAME in the 'env' argument.


b) I don't think kinit_password() should have the 'env' argument at 
all. You can always call kinit with LC_ALL=C and set other variables in 
os.environ if you want.


c) The arguments should have the same ordering.

d) Either set KRB5CCNAME in both 

Re: [Freeipa-devel] Designing better API compatibility

[Jan Cholasta]

Dne 8.4.2015 v 16:44 Martin Kosek napsal(a):

On 03/20/2015 05:00 PM, Petr Vobornik wrote:

On 03/20/2015 04:16 PM, Petr Spacek wrote:

On 20.3.2015 15:51, Nathaniel McCallum wrote:

On Fri, 2015-03-20 at 09:58 -0400, Simo Sorce wrote:

On Fri, 2015-03-20 at 14:38 +0100, Martin Kosek wrote:


Correct. I see 2 approaches here:

a) Thin client, which simply downloads metadata from the (old)
server and won't
use unsupported commands/parameters
b) Not-so-thin client that knows the minimal API versions of
commands/parameters (can be annotated in the code), that would
ping the server
first to identify it's version, validate that the chosen set of
commands/parameters is supported on that server and then send the
commands with
that version.


If we have a recognizable error the client can take an optimistic
approach, send the command normally, if it gets an error that the
server does not understand it, it checks the version in the reply
and falls back to an older baseline version of the command (if
possible) or bails out with an error.


My understanding was that:

1. We already publish all the information necessary to implement a
thin client, and have for some time.

We certainly have *some* data but real thin client will most likely require
some changes. Some information like return types and so on are missing.


2. Thus, the thin client would work on both new and old versions since
it just simply translates from user input into JSON/XML.

3. Only plugins with specific client behavior would need to be ported
to the thin client. A prime example of this is otptoken-add-yubikey.

My preference is solidly for implementing the thin client first. Once
we have decoupled the client from the current plugin framework, server-
side changes can be made in isolation. This decoupling is the move
that is essentially necessary to provide proper API versioning. And if
this can't land for 4.2, land it in the next release. I'd rather do
API-stability correctly and a release later than rushed with
compromises. We have to live with this forever.

+ all votes I have :-)



+1


Ok. So to sum up this thread (and do the actual changes in Trac), in FreeIPA
4.2, we would:

1) Prepare the API UI browser or generated API documentation so that people
could finally see the existing API without having to read the code or inspect
jquery sent by the Web UI.

https://fedorahosted.org/freeipa/ticket/3129


This is not related to API compatibility, it just uses the same metadata.



2) Have option for the ipa tool to send version-less command to the server
which should thus behave as if it is the same version. Bonus points if defaults
are not filled in this case to prevent unrecoverable Unkown Option errors.

https://fedorahosted.org/freeipa/ticket/4768


Not sending version and not computing defaults are very different things 
and their implemetantion will be very different too. I would not mix 
them together.




Rest would be left for later releases. Please holler if there is disagreement
with this plan.


I agree with Nathaniel that we should do thin client ASAP.



Thanks,
Martin




--
Jan Cholasta

--
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

Re: [Freeipa-devel] Designing better API compatibility

[Jan Cholasta]

Dne 9.4.2015 v 09:45 Petr Vobornik napsal(a):

On 04/09/2015 09:35 AM, Martin Kosek wrote:

On 04/09/2015 09:16 AM, Jan Cholasta wrote:

Dne 8.4.2015 v 16:44 Martin Kosek napsal(a):

On 03/20/2015 05:00 PM, Petr Vobornik wrote:

On 03/20/2015 04:16 PM, Petr Spacek wrote:

On 20.3.2015 15:51, Nathaniel McCallum wrote:

On Fri, 2015-03-20 at 09:58 -0400, Simo Sorce wrote:

On Fri, 2015-03-20 at 14:38 +0100, Martin Kosek wrote:


Correct. I see 2 approaches here:

a) Thin client, which simply downloads metadata from the (old)
server and won't
use unsupported commands/parameters
b) Not-so-thin client that knows the minimal API versions of
commands/parameters (can be annotated in the code), that would
ping the server
first to identify it's version, validate that the chosen set of
commands/parameters is supported on that server and then send the
commands with
that version.


If we have a recognizable error the client can take an optimistic
approach, send the command normally, if it gets an error that the
server does not understand it, it checks the version in the reply
and falls back to an older baseline version of the command (if
possible) or bails out with an error.


My understanding was that:

1. We already publish all the information necessary to implement a
thin client, and have for some time.

We certainly have *some* data but real thin client will most
likely require
some changes. Some information like return types and so on are
missing.


2. Thus, the thin client would work on both new and old versions
since
it just simply translates from user input into JSON/XML.

3. Only plugins with specific client behavior would need to be
ported
to the thin client. A prime example of this is otptoken-add-yubikey.

My preference is solidly for implementing the thin client first.
Once
we have decoupled the client from the current plugin framework,
server-
side changes can be made in isolation. This decoupling is the move
that is essentially necessary to provide proper API versioning.
And if
this can't land for 4.2, land it in the next release. I'd rather do
API-stability correctly and a release later than rushed with
compromises. We have to live with this forever.

+ all votes I have :-)



+1


Ok. So to sum up this thread (and do the actual changes in Trac), in
FreeIPA
4.2, we would:

1) Prepare the API UI browser or generated API documentation so that
people
could finally see the existing API without having to read the code
or inspect
jquery sent by the Web UI.

https://fedorahosted.org/freeipa/ticket/3129


This is not related to API compatibility, it just uses the same
metadata.


It is not related to API compatibility per se, but very related to
better API
consumption and a low hanging fruit we could start with, since we have
the
metadata already


+1




2) Have option for the ipa tool to send version-less command to the
server
which should thus behave as if it is the same version. Bonus points
if defaults
are not filled in this case to prevent unrecoverable Unkown Option
errors.

https://fedorahosted.org/freeipa/ticket/4768


Not sending version and not computing defaults are very different
things and
their implemetantion will be very different too. I would not mix them
together.


We are now getting more in the design, but the idea was that sending the
defaults may force server to refuse serving the command even if the
caller did
not explicitly requested that option. Even if the caller did not care
about the
new default option in 4.x, he would not be able to call the command as
it would
be always sent to the old server.


+1 that not sending defaults is essential for this case. IMHO we should
not send them at all.


I agree with that, I'm just saying it won't be as simple as it sounds 
and certainly not as simple as not sending the version.







Rest would be left for later releases. Please holler if there is
disagreement
with this plan.


I agree with Nathaniel that we should do thin client ASAP.


I agree too, but given it is not realistic for 4.2, we need to do at
least
something in 4.2 for projects which need to use the CLI against older
versions.

Skipping version and client defaults seemed as the low hanging fruit
that could
help them. If there is a better idea about what else can be done in
4.2, I am
open to it.

Martin







--
Jan Cholasta

--
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

Re: [Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Simo Sorce]

On Wed, 2015-03-25 at 11:52 +0100, Martin Babinsky wrote:
 On 03/23/2015 03:13 PM, Simo Sorce wrote:
  On Mon, 2015-03-23 at 14:22 +0100, Petr Spacek wrote:
  On 23.3.2015 14:08, Simo Sorce wrote:
  On Mon, 2015-03-23 at 12:48 +0100, Martin Babinsky wrote:
  On 03/17/2015 06:00 PM, Simo Sorce wrote:
  On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:
  On 03/16/2015 12:15 PM, Martin Kosek wrote:
  On 03/13/2015 05:37 PM, Martin Babinsky wrote:
  Attaching the next iteration of patches.
 
  I have tried my best to reword the ipa-client-install man page bit 
  about the
  new option. Any suggestions to further improve it are welcome.
 
  I have also slightly modified the 'kinit_keytab' function so that in 
  Kerberos
  errors are reported for each attempt and the text of the last error 
  is retained
  when finally raising exception.
 
  The approach looks very good. I think that my only concern with this 
  patch is
  this part:
 
  +ccache.init_creds_keytab(keytab=ktab, principal=princ)
  ...
  +except krbV.Krb5Error as e:
  +last_exc = str(e)
  +root_logger.debug(Attempt %d/%d: failed: %s
  +  % (attempt, attempts, last_exc))
  +time.sleep(1)
  +
  +root_logger.debug(Maximum number of attempts (%d) reached
  +  % attempts)
  +raise StandardError(Error initializing principal %s: %s
  +% (principal, last_exc))
 
  The problem here is that this function will raise the super-generic
  StandardError instead of the proper with all the context and 
  information about
  the error that the caller can then process.
 
  I think that
 
 except krbV.Krb5Error as e:
 if attempt == max_attempts:
 log something
 raise
 
  would be better.
 
 
  Yes that seems reasonable. I'm just thinking whether we should re-raise
  Krb5Error or raise ipalib.errors.KerberosError? the latter options 
  makes
  more sense to me as we would not have to additionally import Krb5Error
  everywhere and it would also make the resulting errors more consistent.
 
  I am thinking about someting like this:
 
 except krbV.Krb5Error as e:
if attempt == attempts:
# log that we have reaches maximum number of attempts
raise KerberosError(minor=str(e))
 
  What do you think?
 
  Are you retrying on any error ?
  Please do *not* do that, if you retry many times on an error that
  indicates the password is wrong you may end up locking an administrative
  account. If you want to retry you should do it only for very specific
  timeout errors.
 
  Simo.
 
 
  I have taken a look at the logs attached to the original BZ
  (https://bugzilla.redhat.com/show_bug.cgi?id=1161722).
 
  In ipaclient-install.log the kinit error is:
 
  Cannot contact any KDC for realm 'ITW.USPTO.GOV' while getting initial
  credentials
 
  which can be translated to krbV.KRB5_KDC_UNREACH error. However,
  krb5kdc.log (http://pastebin.test.redhat.com/271394) reports errors
  which are seemingly unrelated to the root cause (kinit timing out on
  getting host TGT).
 
  Thus I'm not quite sure which errors should we chceck against in this
  case, anyone care to advise? These are potential candidates:
 
  KRB5KDC_ERR_SVC_UNAVAILABLE, A service is not available that is
  required to process the request
  KRB5KRB_ERR_RESPONSE_TOO_BIG,Response too big for UDP, retry with 
  TCP
  KRB5_REALM_UNKNOWN,  Cannot find KDC for requested realm
  KRB5_KDC_UNREACH,Cannot contact any KDC for requested realm
 
 
  The only ones that you should retry on, at first glance are
  KRB5_KDC_UNREACH, KRB5KDC_ERR_SVC_UNAVAILABLE.
 
  You should never see KRB5KRB_ERR_RESPONSE_TOO_BIG in the script as it
  should be handled automatically by the library, and if you get
  KRB5_REALM_UNKNOWN I do not think that retrying will make any
  difference.
 
  I might be wrong but I was under the impression that this feature was also 
  for
  workarounding replication delay - service is not available / key is not
  present / something like that.
 
  (This could happen if host/principal was added to one server but then the
  client connected to another server or so.)
 
  If we have that problem we should instead use a temporary krb5.conf file
  that lists explicitly only the server we are joining.
 
  Simo.
 
 
 This is already done since ipa-3-0: by default only one server/KDC is 
 used during client install so there are actually no problems with 
 replication delay, only with KDC timeouts.
 
 Anyway I'm sending updated patches.

LGTM!

Simo.


-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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

Re: [Freeipa-devel] [PATCH] 810 speed up indirect member processing

[Petr Vobornik]

On 04/08/2015 10:21 AM, Jan Cholasta wrote:

Hi,

Dne 31.3.2015 v 12:11 Petr Vobornik napsal(a):

the old implementation tried to get all entries which are member of
group. That means also user. User can't have any members therefore this
costly processing was unnecessary.

New implementation reduces the search only to entries which can have
entries.

Also page size was removed to avoid paging by small pages(default size:
100) which is very slow for many members.

https://fedorahosted.org/freeipa/ticket/4947

Useful to test with #809


1) To search for entries with members, you should search for entries
with the member attribute set ('(member=*)'), not for entries with some
arbitrary object class.


Replaced, new presence index added




2) I don't like how the search in get_memberindirect is limited to an
arbitrary hard-coded subtree. You should go through the object's
attribute_members to figure out which subtrees to search.



The subtree search was removed.



3) Since memberindirect and memberofindirect are not real attributes,
you must define their syntax in ipaldap before you cat set them using
.raw[], otherwise they will be decoded to wrong type.


Added.



4) The processing of memberof should be done even when memberofindirect
is not requested, otherwise its value will depend on whether
memberofindirect was requested or not.


True, but it's the same behavior as before. Could be changed in other patch.




5) I would prefer if all membership processing
(.convert_attribute_members() and .get_indirect_members()) was done in a
single LDAPObject method.


Now, as before, get_indirect_members is called before post callbacks and 
convert_attribute_members after. If it should be combined, it should be 
done separately.





Honza


--
Petr Vobornik
From 27dfa6d0e2a1815d496f47b4f10b5a6307f51fda Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 31 Mar 2015 10:59:37 +0200
Subject: [PATCH] speed up indirect member processing

the old implementation tried to get all entries which are member of group.
That means also user. User can't have any members therefore this costly
processing was unnecessary.

New implementation reduces the search only to entries which have members.

Also page size was removed to avoid paging by small pages(default size: 100)
which is very slow for many members.

https://fedorahosted.org/freeipa/ticket/4947
---
 install/updates/20-indices.update |  2 +-
 ipalib/plugins/baseldap.py| 72 +++
 ipalib/plugins/host.py|  2 +-
 ipalib/plugins/role.py|  8 ++--
 ipapython/ipaldap.py  |  2 +
 ipaserver/plugins/ldap2.py| 90 ---
 6 files changed, 81 insertions(+), 95 deletions(-)

diff --git a/install/updates/20-indices.update b/install/updates/20-indices.update
index a8a432d9c28a7fb4ca74582e36d4c39fd98df2cf..a9ec9f9eb9bcc228dcbb7eba99879ce5a8251e80 100644
--- a/install/updates/20-indices.update
+++ b/install/updates/20-indices.update
@@ -27,7 +27,7 @@ default:nsSystemIndex: false
 only:nsIndexType: eq,pres,sub
 
 dn: cn=member,cn=index,cn=userRoot,cn=ldbm database,cn=plugins,cn=config
-only:nsIndexType: eq,sub
+only:nsIndexType: eq,pres,sub
 
 dn: cn=uniquemember,cn=index,cn=userRoot,cn=ldbm database,cn=plugins,cn=config
 only:nsIndexType: eq,sub
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index c9f98ed12d3a584597af9b0be08361bceca620e7..9c75bc048283546902a89825ba489d42b26f10fd 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -663,6 +663,61 @@ class LDAPObject(Object):
 new_attr.append(new_value)
 break
 
+def get_memberindirect(self, group_entry):
+
+Get indirect members
+
+
+mo_filter = self.backend.make_filter({'memberof': group_entry.dn})
+filter = self.backend.combine_filters(
+('(member=*)', mo_filter), self.backend.MATCH_ALL)
+try:
+result, truncated = self.backend.find_entries(
+base_dn=self.api.env.basedn,
+filter=filter,
+attrs_list=['member'],
+size_limit=-1, # paged search will get everything anyway
+paged_search=True)
+if truncated:
+raise errors.LimitsExceeded()
+except errors.NotFound:
+result = []
+
+indirect = set()
+for entry in result:
+indirect.update(entry.raw.get('member', []))
+indirect.difference_update(group_entry.raw.get('member', []))
+
+if indirect:
+group_entry.raw['memberindirect'] = list(indirect)
+
+def get_memberofindirect(self, entry):
+
+dn = entry.dn
+filter = self.backend.make_filter(
+{'member': dn, 'memberuser': dn, 'memberhost': dn})
+try:
+result, truncated = self.backend.find_entries(
+