On 08/16/2016 02:52 PM, Jakub Hrozek wrote:
On Mon, Aug 15, 2016 at 04:03:17PM +0200, Petr Cech wrote:
> On 08/12/2016 04:05 PM, Petr Cech wrote:
>> On 08/12/2016 03:36 PM, Jakub Hrozek wrote:
>>> On Fri, Aug 12, 2016 at 02:51:21PM +0200, Petr Cech wrote:
>>>> On 08/12/2016 11:27 AM, Jakub Hrozek wrote:
>>>>> On Wed, Aug 10, 2016 at 08:54:25AM +0200, Petr Cech wrote:
>>>>>> Sorry, I experienced some issue with mailing list.
>>>>>> So I send it again.
>>>>>>
>>>>>> -------- Forwarded Message --------
>>>>>> Subject: Re: [SSSD] Re: [PATCH SET] AD_PROVIDER:
ad_enabled_domains
>>>>>> Date: Tue, 9 Aug 2016 17:29:38 +0200
>>>>>> From: Petr Cech <pcech(a)redhat.com>
>>>>>> To: sssd-devel(a)lists.fedorahosted.org
>>>>>>
>>>>>> On 08/09/2016 11:07 AM, Jakub Hrozek wrote:
>>>>>>> On Mon, Jul 25, 2016 at 06:18:28PM +0200, Petr Cech wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> there is fixed patch set attached.
>>>>>>>>>
>>>>>>>>> Segmentation fault was caused by wrong pointer :-(,
sorry.
>>>>>>>>>
>>>>>>>>> This new patch set has new debug message. I am open
to dissccus the
>>>>>>>>> debug_level and content of message. Any improving
idea?
>>>>>>>>>
>>>>>>>>> I hit one issue during testing -- sometimes if I am
connected to
>>>>>>>>> subdomain
>>>>>>>>> and I enable only sibling subdomain (the master is
added
>>>>>>>>> automaticaly) and
>>>>>>>>> forest root is not enabled -- I see only master and
sibling not.
>>>>>>>>> But if I
>>>>>>>>> added sleep for cycle (for using dbg) to function
>>>>>>>>> ad_subdomains_init()
>>>>>>>>> everythink is OK.
>>>>>>>>> Any idea?
>>>>>>> Can you test that case with valgrind? This sounds like some
>>>>>>> uninitilized
>>>>>>> variable condition.
>>>>>>
>>>>>>
>>>>>> I didn't run valgrind but I have new information.
>>>>>>
>>>>>> If you clear the cache and reset sssd, first attempt to obtain
>>>>>> information
>>>>>> about user from sibling domain fails. The second and the other
>>>>>> attempts runs
>>>>>> correctly.
>>>>>>
>>>>>> I see that the sibling domain is enabled. But if I look more
>>>>>> carefully there
>>>>>> is message in log (gamma.domain.bootes is sibling domain):
>>>>>>
>>>>>> [sssd[be[beta.domain.bootes]]] [dp_req_new] (0x0020): Unknown
domain:
>>>>>> gamma.domain.bootes
>>>>>>
>>>>>> First attempt should works too but you should wait nearly exactly
6
>>>>>> seconds
>>>>>> after restart sssd.
>>>>>>
>>>>>> New patch set is attached.
>>>>>
>>>>> I can't start SSSD with these patches:
>>>>> (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
>>>>> [dp_target_run_constructor] (0x0010): Target [subdomains]
>>>>> constructor failed [22]: Invalid argument
>>>>> (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
>>>>> [dp_load_targets] (0x0020): Unable to load target [subdomains] [22]:
>>>>> Invalid argument.
>>>>> (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]] [dp_init]
>>>>> (0x0020): Unable to initialize DP targets [1432158209]: Internal
Error
>>>>> (Fri Aug 12 11:25:38 2016) [sssd[be[win.trust.test]]]
>>>>> [dp_terminate_active_requests] (0x0400): Terminating active data
>>>>> provider requests
>>>>>
>>>>> I have:
>>>>> $ git log --oneline origin/master..HEAD
>>>>> 3b2f910 TESTS: Adding tests for ad_enabled_domains option
>>>>> 7ac9517 AD_PROVIDER: ad_enabled_domains - other then master
>>>>> fdbbd30 AD_PROVIDER: ad_enabled_domains - only master
>>>>> ebaa14d AD_PROVIDER: Initializing of ad_enabled_domains
>>>>> 38989af AD_PROVIDER: Add ad_enabled_domains option
>>>>>
>>>>> $ git rev-list origin/master..HEAD
>>>>> 3b2f9106c2c5bea1681cf1f752fc5f3256a04300
>>>>> 7ac9517f78dc4dcde4c4c613ec450a3f3fc8f644
>>>>> fdbbd30adf9da7a3c2510029c2e8c3789a3083a0
>>>>> ebaa14dd1dd0e4f55a2bc4e647ce848e36970dd2
>>>>> 38989afa14bfc89712808867b80e667d34e068b3
>>>>
>>>> Hello Jakub,
>>>>
>>>> I wasn't able to reproduce your bug. Is it true that I use F23 for
>>>> testing
>>>> this patch for historical reasons. I should try it with F24 too.
>>>>
>>>> I sent whole patch set to CI,
>>>>
http://sssd-ci.duckdns.org/logs/job/51/45/summary.html
>>>> but I think it is not conclusive because out tests don't contain AD
>>>> server.
>>>>
>>>> I will look at it again. But now I would like finish tests for
>>>> netgroups.
>>>
>>> I don't think it has to do with Fedora version. Maybe my sssd.conf would
>>> help:
>>>
>>> [domain/win.trust.test]
>>> ad_domain = win.trust.test
>>> krb5_realm = WIN.TRUST.TEST
>>> realmd_tags = manages-system joined-with-adcli
>>> cache_credentials = True
>>> id_provider = ad
>>> krb5_store_password_if_offline = True
>>> default_shell = /bin/bash
>>> ldap_id_mapping = True
>>> use_fully_qualified_names = True
>>> fallback_homedir = /home/%u@%d
>>> ad_enable_gc = false
>>> debug_level = 10
>>>
>>> access_provider = simple
>>>
>>> #ad_enabled_domains = win.trust.test, siblingdom.win.trust.test
>>> #debug_level = 7
>>>
>>> dyndns_update = false
>>
>> Thanks,
>>
>> I see now where's the problem. I didn't try to comment
>> ad_enabled_domains in config for long time. If this option missing it
>> will crash.
>>
>> [dp_target_run_constructor] (0x0010): Target [subdomains] constructor
>> failed [22]: Invalid argument
>>
>> I hope it will be easy to fix it.
>
> Hello,
>
> I fixed little bug (wrong return code for missing option)
> in ad_get_enabled_domains().
>
> New patch set is attached.
>
> There is still one strange behaviour:
>
> If you clear the cache and reset sssd, first attempt to obtain
> information about user from sibling domain fails. The second and the other
> attempts runs correctly.
>
> I see that the sibling domain is enabled. But if I look more
> carefully there is message in log (gamma.domain.bootes is sibling
> domain):
>
> [sssd[be[beta.domain.bootes]]] [dp_req_new] (0x0020): Unknown domain:
> gamma.domain.bootes
>
> First attempt should works too but you should wait nearly exactly 6
> seconds after restart sssd.
I think this is an unrelated bug where the subdomains take a bit of time
to be discovered. I think the responder was waiting until the subdomains
were autodiscovered, did we break this?
Anyway, it's not related to these patches (and I can't reproduce it
locally either)
I have two last small comments:
Thanks Jakub for review and comments.
New fixed version of patch set is attached.
> From 24d32d0eb12ddc433e64ffd6411e9e13f0067b35 Mon Sep 17 00:00:00
2001
> From: Petr Cech <pcech(a)redhat.com>
> Date: Fri, 13 May 2016 05:21:07 -0400
> Subject: [PATCH 1/5] AD_PROVIDER: Add ad_enabled_domains option
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2828
Did you already have the manpage hunk checked by some native English
speaker?
No native speaker have seen it.
> --- a/src/man/sssd-ad.5.xml
> +++ b/src/man/sssd-ad.5.xml
> @@ -114,6 +114,28 @@ ldap_id_mapping = False
> </varlistentry>
>
> <varlistentry>
> + <term>ad_enabled_domains (string)</term>
> + <listitem>
> + <para>
> + The comma-separated list of the enabled Active
> + Directory domains. This is optional. If provided,
> + SSSD will not contact domains not listed in this
> + option. If not provided, all domains from AD forest
> + are enabled.
In particular, I'm not sure about the double negatives (not contact
domains not listed..)
And normally we list the default value in a separate para.
Addressed.
> + </para>
> + <para>
> + For proper operation, this option should be
> + specified as the lower-case version of the long
> + version of the Active Directory domain.
> + </para>
> + <para>
> + The short domain name (also known as the NetBIOS
> + or the flat name) is autodetected by the SSSD.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term>ad_server, ad_backup_server (string)</term>
> <listitem>
> <para>
> From 807401f380d6f23e4b4594d32ecd45223e602d10 Mon Sep 17 00:00:00 2001
> From: Petr Cech <pcech(a)redhat.com>
> Date: Mon, 27 Jun 2016 11:51:30 +0200
> Subject: [PATCH 4/5] AD_PROVIDER: ad_enabled_domains - other then master
>
> We can skip looking up other domains if
> option ad_enabled_domains doesn't contain them.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2828
> ---
[...]
> static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
> struct sss_domain_info *domain,
> + const char **enabled_domains_list,
> size_t nsd, struct sysdb_attrs **sd,
> struct sysdb_attrs *root,
> size_t *_nsd_out,
> @@ -493,9 +504,10 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
> size_t i, sdi;
> struct sysdb_attrs **sd_out;
> const char *sd_name;
> + const char *root_name;
> errno_t ret;
>
> - if (root == NULL) {
> + if (root == NULL && (enabled_domains_list == NULL)) {
The parens around enabled_domains_list are not needed, can you drop
them?
Addressed.
> /* We are connected directly to the root domain. The
'sd'
> * list is complete and we can just use it
> */
The other patches can be considered ACKed.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
--
Petr^4 Čech