[SSSD] [PATCH] USe sysdb_serch_service() in sss_cacahe

Jakub Hrozek jhrozek at redhat.com
Sun Jan 13 13:30:46 UTC 2013


On Thu, Jan 10, 2013 at 02:10:46PM -0500, Simo Sorce wrote:
> On Thu, 2013-01-10 at 19:44 +0100, Michal Židek wrote:
> > On 01/10/2013 06:04 PM, Simo Sorce wrote:
> > > On Thu, 2013-01-10 at 17:28 +0100, Jakub Hrozek wrote:
> > >> On Thu, Jan 10, 2013 at 11:13:05AM -0500, Simo Sorce wrote:
> > >>> On Thu, 2013-01-10 at 11:06 -0500, Simo Sorce wrote:
> > >>>> On Thu, 2013-01-10 at 11:22 +0100, Pavel Březina wrote:
> > >>>>> On 01/08/2013 05:10 AM, Simo Sorce wrote:
> > >>>>>> In the patches that fix the sdap_reinit functions I introduced a new
> > >>>>>> service search helper sysdb_search_services()
> > >>>>>>
> > >>>>>> Use this helper in sss_cache instead of a home grown one with poor-man
> > >>>>>> filter parser.
> > >>>>>>
> > >>>>>> Simo.
> > >>>>>
> > >>>>> Hi,
> > >>>>> I don't think these functions are identical.
> > >>>>>
> > >>>>> -    if (strcmp(sub_filter, "*") == 0) {
> > >>>>> -        /* All services */
> > >>>>> -        ret = sysdb_enumservent(mem_ctx, sysdb, &res);
> > >>>>> -    } else {
> > >>>>> -        /* Get service by name */
> > >>>>> -        ret = sysdb_getservbyname(mem_ctx, sysdb, sub_filter,
> > >>>>> -                                  NULL, &res);
> > >>>>> -    }
> > >>>>>
> > >>>>> The sub_filter is not a valid ldb filter, but '*' or name.
> > >>>>
> > >>>> Ahh true, I'll change this to be a minimal wrapper then.
> > >>>>
> > >>>> Thanks for catching it.
> > >>>
> > >>> Actually looking at is it would be more consistent to change the filter
> > >>> in update_all_filters, however I noticed that all these filters only to
> > >>> SYSDB_NAME=<value> and none take in account aliases, while the
> > >>> getsrvbyname() function also takes in account aliases. Should the
> > >>> update_all_filters also take in account aliases for other filters like
> > >>> update_user_filter ?
> > >>> If so I can simply change update_filter for all uniformly.
> > >>>
> > >>
> > >> I think it would be a good idea.
> > >>
> > >>> Simo.
> > >>>
> > >>>> Btw why sss_cache is not tested in make check ?
> > >>>>
> > >>
> > >> None of the tools is. It's just noone wrote the tests upstream, we're
> > >> relying on Red Hat QE here.
> > >
> > > Ok I opened tickets both for alias support and tests, and another bug I
> > > found.
> > > I didn't do alias support because the additional bug may mean additional
> > > changes are needed to that part of the code anyway, so I'll leave it to
> > > someone to revisit exactly what the inputs/outputs and behavior need to
> > > be before diving in the code.
> > >
> > > Attached find respun patch that instead makes the code generate a proper
> > > filter for services too so that using sysdb_serach_service() is ok.
> > > The only thing it really changes is that it drops support for searching
> > > service aliases, but I think that is fine as alias support is
> > > inconsistent throughout the tool and a decision will be made in the
> > > ticket I opened, so by dropping alias support for services we are now
> > > more consistent even if we later decide we need to readd alias support
> > > to all calls that support aliases.
> > >
> > > Simo.
> > >
> > >
> > 
> > As you are already touching the update filters part, could you change 
> > the fmt parameter to update_filter in (in autofs part) from 
> > "(&(objectclass=%s)(%s=%s))"
> > 
> >   to
> > 
> > "(&(objectclass="SYSDB_AUTOFS_MAP_OC")(%s=%s))"
> > 
> > That should also fix https://fedorahosted.org/sssd/ticket/1754.
> 
> Ok.
> 
> rebased to add this fix.
> 
> Simo.

Nack, invalidating all services (sss_cache -S) is broken after this
patch. Please squash in the attached patch and I'll ack.
-------------- next part --------------
>From 4f8f9c9fffe6b9be8f5eb18409faee53784d5bce Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 13 Jan 2013 14:30:14 +0100
Subject: [PATCH] sss_cache: fix invalidating all services

---
 src/tools/sss_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index bcc27971f18f8e86af3a1b24dcd29dcfeeaf38f1..c6ba04e408d9f4421ebc10286c82f1fc3f8304b6 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -568,7 +568,7 @@ errno_t init_context(int argc, const char *argv[], struct cache_tool_ctx **tctx)
     }
 
     if (idb & INVALIDATE_SERVICES) {
-        ctx->service_filter = talloc_strdup(ctx, "*");
+        ctx->service_filter = talloc_asprintf(ctx, "(%s=*)", SYSDB_NAME);
         ctx->update_service_filter = false;
     } else if (service) {
         ctx->service_name = talloc_strdup(ctx, service);
-- 
1.8.0.2



More information about the sssd-devel mailing list