On Thu, 13 Mar 2014, Martin Kosek wrote:
On 03/13/2014 01:36 PM, Martin Kosek wrote:
On 03/13/2014 01:33 PM, Alexander Bokovoy wrote:
On Thu, 13 Mar 2014, Petr Spacek wrote:
On 13.3.2014 13:20, Martin Kosek wrote:
On 03/13/2014 01:10 PM, Alexander Bokovoy wrote:
On Thu, 13 Mar 2014, Martin Kosek wrote:
On 03/13/2014 01:01 PM, Alexander Bokovoy wrote:
On Thu, 13 Mar 2014, Martin Kosek wrote:
On 03/13/2014 12:45 PM, Tomas Babej wrote:
Hi,
Changes the code in the idrange_del method to not only check for
the root domains that match the SID in the IDRange, but for the
SIDs of subdomains of trusts as well.
https://fedorahosted.org/freeipa/ticket/4247
This is a very complicated validation procedure IMO. Lot of subcommands,
lot of
LDAP searches.
Why can't we do just one LDAP search with
- base api.env.container_trusts
- scope SUB
- filter
(&(objectclass=ipaNTTrustedDomain)(ipanttrusteddomainsid=range_sid))
When errors.NotFound is raised, we are OK. When it is not raised, we have a
problem.
Wouldn't it be simpler?
No. Please do not do optimization here. It is a code that is called very
rarely and expressiveness is more important here than optimizing access
to couple of entries in LDAP.
I am not optimizing - I am actually making the validation much simpler.
What is
more simple and straightforward?
A) One ldap.find_entries call
B) A loop, numerous subcommands and LDAP searches
So far I've been successful in keeping details on how trust objects are
represented in LDAP hidden from the rest of the framework code by
encapsulating it all in trust.py. The change you propose will
make it leaking to idrange.py. If we start changing the structure (which
is maintained by ipasam module, not the framework), we will have more
maintenance problems with the code spread out.
Ok, I can see your point, but I am still not sure it warrants that complicated
validation and a new dependency between commands. You cannot that easily change
the structure of the subdomains anyway as it would break all older servers
which expect the subdomains to be where they are.
In plugins, we do LDAP searches in cases like this one quite regularly IMO, it
is not something unprecendented. And there is a good reason, simple LDAP call
is much easier and less error prone to changes in our frameworks than calling
subcommands. One glitch or a change in the subcommand can break not only the
subcommand, but it's all callers.
Note that you can encapsulate the search proposed by Martin in a function
defined in trusts.py so it doesn't need to be implemented idrange.py.
It is just a matter of finding the right name for it.
I wanted to propose that as well.
We already have conditional import of ipaserver.dcerpc, we can use
ipalibs.plugins.trust import for that helper, just don't export it
through API.
This may work as well, we just need to be cautious in the idrange plugin so
that the idrange-del works even when ipa-server-trust-ad package is not
installed on the system (which would probably break current patch too, when I
am thinking about it).
Martin
I take it back - it is not so good idea. You may be running this command on a
master without ipa-server-trust-ad installed, but which may be however
installed on other IPA master and thus we do want to check for the trustdomains.
We need to enclose this check to simple LDAP call in idrange.py as I said in
the beginning.
Ok, this is an argument I agree with.
Tomas, could you please change the code correspondingly?
--
/ Alexander Bokovoy
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
