On Wed, Sep 24, 2014 at 10:25:22AM +0200, Sumit Bose wrote:
Hi,
this patches contain the IPA provider part of
https://fedorahosted.org/sssd/ticket/2375 as described in the FreeIPA
design page
http://www.freeipa.org/page/V4/Migrating_existing_environments_to_Trust
Patches 0001, 0006 and 0007 contain new sysdb interfaces for views and
overrides. 0002 and 0003 refactor some already existing calls.
0004 adds the initial support for views and overrides and reads the view
name for the client from the FreeIPA server or sets it to 'default' when
running in ipa-server-mode. It adds some new options which currently
misses man page entries and are not added to the python config API. I
did this on purpose for the time being because I wanted to see first if
the list is complete or if some options ca be dropped.
0005 adds the request to get a specific override from the FreeIPA server
and finally in 0008 the overrides are read during a request and saves or
applied.
This patches must be applied on to of the extdom patch I send yesterday.
To add and manage views and override on the server side please have a
look at Tomas's patch set on the freeipa-devel list
(
https://www.redhat.com/archives/freeipa-devel/2014-September/msg00336.html).
bye,
Sumit
From dedbb5ae2faa13d396e84ce9911ab1e9c0246836 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Tue, 16 Sep 2014 15:18:53 +0200
Subject: [PATCH 1/8] sysdb: add sysdb_update_view_name()
ACK
A unit test for the functions would be nice in order to avoid decreasing
the code coverage of the sysdb module, but I realize we have a deadline.
We should file a ticket, though.
It would also be nice to add a comment that it's not possible to have a
view w/o a name but we are trying to be on the safe side b/c name is not
MUST as we discussed on IRC with Sumit.
From c09a8553d7371e46b84e980f38d017c7f84c2f87 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Tue, 16 Sep 2014 15:22:08 +0200
Subject: [PATCH 2/8] Add sdap_deref_search_with_filter_send()
I would prefer to also add a matching _done and _recv functions, even if
they were wrappers around existing functions. But tevent coding style is
important to follow, we already had bugs about reusing a single tevent
callbacks for multiple requests.
We can do it post-rebase, though. Just please file a ticket.
From 3097c76ba9498b90df69935d24dc9ce55f602937 Mon Sep 17 00:00:00
2001
From: Sumit Bose <sbose(a)redhat.com>
Date: Fri, 19 Sep 2014 10:26:01 +0200
Subject: [PATCH 3/8] IPA: make IPA ID context available to extdom client code
ACK
...aaand I stopped here.
That was Jakub's part of the review.
Patch 4: IPA: add view support and get view name
if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set SRV lookup plugin "
- "[%d]: %s\n", ret, strerror(ret));
- goto done;
+ if (ret == ENOENT) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Cannot find view name in the cache. " \
+ "Will do online llokup later\n");
^ typo
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_get_view_name failed.\n");
+ goto done;
+ }
+ }
ctx->ranges_search_bases =
id_ctx->ipa_options->ranges_search_bases;
+ ctx->host_search_bases = id_ctx->ipa_options->host_search_bases;
ctx->configured_explicit = configured_explicit;
This change seems to be unrelated to this patch. Shouldn't it be a
separate commit?
Patch 5: views: add ipa_get_ad_override_send()
Looks good, but can you add some more tracing debug messages there? For
example what view are we searching for and that an override was found.
Patch 6:
+ const char *obj_attrs[] = { SYSDB_OBJECTCLASS,
+ SYSDB_OVERRIDE_DN,
+ NULL};
Indentation.
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Missing anchor in overide attributes.\n");
^ typo
+ ret = EINVAL;
+ goto done;
+ }
+ if (ret != EOK) {
+ if (ret == ENOENT) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Object to override does not
exists.\n");
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed.\n");
+ }
+ goto done;
+ }
Can you use if (ret == ENOENT) else if (ret != EOK) here? I think it is
simpler.
+ if (obj_override_dn != NULL) {
+ if (strcmp(obj_override_dn, override_dn_str) != 0) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Existing [%s] and new [%s] overid DN do not match.\n",
^ typo
> + obj_override_dn, override_dn_str);
+ ret = EINVAL;
+ goto done;
+ }
> +
> + add_ref = false;
> + }
Patch 7: sysdb: sysdb_apply_default_override
Ack.
Patch 8: views: get overrides during user and group lookups
Ack.