[SSSD] [PATCHES] Remove requests when AD subdomain goes away

Ondrej Kos okos at redhat.com
Wed Sep 4 09:09:52 UTC 2013


On 08/30/2013 10:47 AM, Pavel Březina wrote:
> On 08/28/2013 11:01 AM, Ondrej Kos wrote:
>> On 08/26/2013 11:39 AM, Pavel Březina wrote:
>>> On 08/08/2013 02:21 PM, Ondrej Kos wrote:
>>>> On 08/08/2013 01:08 AM, Pavel Březina wrote:
>>>>> On 08/05/2013 04:10 PM, Ondrej Kos wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Attached are three patches,
>>>>>>
>>>>>> [PATCH 1/3] Make subdomain refresh period configurable
>>>>>> * Adds the ad_subdomain_refresh_period and
>>>>>> ipa_subdomain_refresh_period
>>>>>> configuration options. This isn't needed to be pushed, but I think it
>>>>>> can be beneficial. Also, I needed to write this anyway to work with
>>>>>> the
>>>>>> refresh.
>>>>>>
>>>>>> [PATCH 2/3] DP: Store list of back-end tevent requests
>>>>>> * Adds every created request to list, and removes every terminated.
>>>>>> This
>>>>>> is to enable iteration through active requests, to fix the issue
>>>>>> addressed in https://fedorahosted.org/sssd/ticket/1968
>>>>>>
>>>>>> [PATCH 3/3] Clean list of domain requests
>>>>>> * fixes https://fedorahosted.org/sssd/ticket/1968
>>>>>> * Goes through the list of tevent requests introduced in previous
>>>>>> patch
>>>>>> an those, which match the vanished domain are terminated.
>>>>>>
>>>>>> Ondra
>>>>>
>>>>> Hi,
>>>>>
>>>>>> +void be_clean_dom_req_list(struct be_ctx *be_ctx)
>>>>>> +{
>>>>>> +    struct be_req *be_req;
>>>>>> +    struct sss_domain_info *domain;
>>>>>> +    char *domain_name;
>>>>>> +
>>>>>> +    domain_name = be_ctx->domain->name;
>>>>>> +
>>>>>> +    DEBUG(SSSDBG_TRACE_FUNC,
>>>>>> +            ("Removing requests for domain %s\n", domain_name));
>>>>>> +
>>>>>> +    DLIST_FOR_EACH(be_req, be_ctx->be_reqlist) {
>>>>>> +
>>>>>> +        domain = be_req_get_domain(be_req);
>>>>>> +
>>>>>> +        if (strcmp(domain_name, domain->name) == 0) {
>>>>>> +            be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
>>>>>> +        }
>>>>>> +    }
>>>>>> +}
>>>>>
>>>>>> +struct sss_domain_info *be_req_get_domain(struct be_req *be_req)
>>>>>> +{
>>>>>> +    struct be_ctx *be_ctx = be_req_get_be_ctx(be_req);
>>>>>> +
>>>>>> +    return be_ctx ? be_ctx->domain : NULL;
>>>>>> +}
>>>>>
>>>>>
>>>>> please, correct me if I'm wrong (I haven't test those patches yet),
>>>>> but
>>>>> it won't work as expected. This will always remove *all* be_req, since
>>>>> you're comparing be_ctx->domain with be_req->be_ctx->domain which is
>>>>> the
>>>>> same.
>>>>>
>>>>> You want to compare name of the subdomain that was removed with the
>>>>> actual domain (original domain or one of the subdomain) used in the
>>>>> request - this information is not available at the moment.
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> I don't know how this happened, I probably sent patchset from VM.
>>>> Actual
>>>> and rebased patches are attached, thanks for noticing this!
>>>>
>>>> Ondra
>>>
>>> Hi,
>>>
>>> Patch 1: Make subdomain refresh period configurable
>>>
>>> You are creating two options:
>>> - ad_subdomain_refresh_period
>>> - ipa_subdomain_refresh_period
>>>
>>> I'd merge it into one subdomain_refresh_period. Does anyone else have
>>> any opinion on this?
>>>
>>> Anyway, you put AD option into IPA manpage:
>>>> +++ b/src/man/sssd-ipa.5.xml
>>>> @@ -208,6 +208,19 @@
>>>>                  </varlistentry>
>>>>
>>>>                  <varlistentry>
>>>> +                    <term>ad_subdomain_refresh_period (integer)</term>
>>>
>>> Patch 3:
>>> You are calling be_clean_dom_req_list() only in ad subdomains, it needs
>>> to be called also in ipa.
>>>
>>> The main point of this ticket is to safely free sdap_domain structure.
>>> However, you don't call talloc_free() in these patches :-)
>>>
>>>
>>>> void be_clean_dom_req_list(struct be_ctx *be_ctx, const char
>>>> *domain_name)
>>>> {
>>>>     struct be_req *be_req;
>>>>     struct sss_domain_info *domain;
>>>>
>>>>     DEBUG(SSSDBG_TRACE_FUNC,
>>>>             ("Removing requests for domain %s\n", domain_name));
>>>>
>>>>     DLIST_FOR_EACH(be_req, be_ctx->be_reqlist) {
>>>>
>>>>         domain = be_req_get_domain(be_req);
>>>>
>>>>         if (domain == NULL) continue;
>>>>
>>>>         if (strcmp(domain_name, domain->name) == 0) {
>>>
>>> be_ctx->domain is always the domain from sssd.conf [domain] section.
>>> That means that this is a dead code. You need to associate the subdomain
>>> with be_req and then remove those request that works with this
>>> subdomain.
>>
>> I don't see what you're implying here.
>>
>> *be_ctx->domain* is not used here at all
>>
>> *be_ctx->be_reqlist* should contain all request for the provider
>> *domain_name* is name of subdomain which is getting terminated
>> *domain* is subdomain info fetched from request which is being processed
>> in current iteration through the request list
>>
>>>
>>>>             be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
>>>>         }
>>>>     }
>>>> }
>
> FYI we talked about this offline and it turned out that I was right :-)
>
> be_req_get_domain(be_req) returns be_req->be_ctx->domain, which is
> always the main domain.
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
New patchset attached.

With this, we store the domain name to the request, where available. I'd 
suggest to file a ticket and store it for every request, but for this 
purpose it's enough.

Ondra

-- 
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-subdomain-refresh-period-configurable.patch
Type: text/x-patch
Size: 9872 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130904/24177a61/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-DP-Store-list-of-back-end-tevent-requests.patch
Type: text/x-patch
Size: 2459 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130904/24177a61/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-DP-Save-request-domain-name-if-available.patch
Type: text/x-patch
Size: 6881 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130904/24177a61/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Clean-list-of-domain-requests.patch
Type: text/x-patch
Size: 3245 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130904/24177a61/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-LDAP-free-subdomain-structure.patch
Type: text/x-patch
Size: 807 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130904/24177a61/attachment-0004.bin>


More information about the sssd-devel mailing list