Re: [SSSD] [PATCH] skip rule on error instead of failing completely

[Pavel Březina]

On 07/26/2013 01:31 PM, Jakub Hrozek wrote:
>  On Fri, Jul 26, 2013 at 12:34:47PM +0200, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/2031
>
> >  From fecd2799c21dc78fcb098e786b1c7e879e943c5b Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com&gt;
> > Date: Fri, 26 Jul 2013 12:25:01 +0200
> > Subject: [PATCH 1/2] sudo: skip rule on error instead of failing completely
> >
> > https://fedorahosted.org/sssd/ticket/2031
> > ---
> >   src/providers/ldap/sdap_sudo_cache.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/providers/ldap/sdap_sudo_cache.c
> b/src/providers/ldap/sdap_sudo_cache.c
> > index
> 3c438b9307c310cd4eec030ad628ce28a66ba726..9a2d326e0e11cf90279d8327afaf6a0ab76cac85 100644
> > --- a/src/providers/ldap/sdap_sudo_cache.c
> > +++ b/src/providers/ldap/sdap_sudo_cache.c
> > @@ -135,7 +135,9 @@ sdap_save_native_sudorule_list(TALLOC_CTX *mem_ctx,
> >                                           domain, map, replies[i],
> >                                           cache_timeout, now, &usn_value);
> >           if (ret != EOK) {
> > -            goto fail;
> > +            DEBUG(SSSDBG_OP_FAILURE, ("Failed to save sudo rule, "
> > +                                      "will continue with next...\n"));
> > +            continue;
> >           }
> >
> >           /* find highest usn */
> > --
> > 1.7.11.7
> >
>
> >  From 09546d945e2b4932550c9d267c2146ac4c901e59 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com&gt;
> > Date: Fri, 26 Jul 2013 12:29:37 +0200
> > Subject: [PATCH 2/2] sudo: print better debug message when a rule has
> >   multiple cn values
> >
> > ---
> >   src/providers/ldap/sdap_sudo_cache.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/providers/ldap/sdap_sudo_cache.c
> b/src/providers/ldap/sdap_sudo_cache.c
> > index
> 9a2d326e0e11cf90279d8327afaf6a0ab76cac85..aaaa1e2aa545864c01acd160c211ecfffce874e2 100644
> > --- a/src/providers/ldap/sdap_sudo_cache.c
> > +++ b/src/providers/ldap/sdap_sudo_cache.c
> > @@ -69,7 +69,11 @@ sdap_save_native_sudorule(TALLOC_CTX *mem_ctx,
> >
> >       ret = sysdb_attrs_get_string(attrs, map[SDAP_AT_SUDO_NAME].sys_name,
> >                                    &rule_name);
> > -    if (ret != EOK) {
> > +    if (ret == ERANGE) {
> > +        DEBUG(SSSDBG_OP_FAILURE, ("Warning: found rule that contains none
> "
> > +              "or multiple CN values. It will be skipped.\n"));
> > +        return ret;
> > +    } else if (ret != EOK) {
> >           DEBUG(SSSDBG_OP_FAILURE, ("Could not get rule name [%d]: %s\n",
> >                 ret, strerror(ret)));
> >           return ret;
> > --
> > 1.7.11.7
> >
>
>  Can you make either of those DEBUG messages more verbose? I think we
>  should make it clear that we have not processed all the rules. 
New patches are attached.

>  Also I wonder if we should support multiple names by looking at RDN
>  value and picking the one that matches the RDN? That's what we do for
>  users and groups anyway. 
Yes, ctrianta will file a bug/rfe and we'll do that as separate patch.

>  What is the sudo rule name used for (except for cn=defaults) ?
Only administrative purpose.