On Thu, Sep 26, 2013 at 09:40:12PM +0200, Sumit Bose wrote: > On Thu, Sep 26, 2013 at 07:39:32PM +0200, Jakub Hrozek wrote: > > On Thu, Sep 26, 2013 at 06:26:54PM +0200, Sumit Bose wrote: > > > On Wed, Sep 25, 2013 at 12:57:01PM +0200, Jakub Hrozek wrote: > > > > On Wed, Sep 25, 2013 at 11:53:36AM +0200, Sumit Bose wrote: > > > > > On Wed, Sep 25, 2013 at 11:12:02AM +0200, Jakub Hrozek wrote: > > > > > > Hi, > > > > > > > > > > > > the attached patch fixes > > > > > > https://fedorahosted.org/sssd/ticket/2079 > > > > > > > > > > > > > > > > > > I also opened https://fedorahosted.org/freeipa/ticket/3947 to stop > > > > > > setting the dns_discovery_domain parameter from the IPA installer > > > > > > completely. > > > > > > > > > > > From e714f3ef180934fa92ca805384ddbf1a510051c8 Mon Sep 17 00:00:00 > > > > > > 2001 > > > > > > From: Jakub Hrozek <jhro...@redhat.com> > > > > > > Date: Wed, 25 Sep 2013 11:03:16 +0200 > > > > > > Subject: [PATCH] IPA: Ignore dns_discovery_domain in server mode > > > > > > > > > > > > https://fedorahosted.org/sssd/ticket/2079 > > > > > > --- > > > > > > src/providers/ipa/ipa_init.c | 12 ++++++++++++ > > > > > > 1 file changed, 12 insertions(+) > > > > > > > > > > > > diff --git a/src/providers/ipa/ipa_init.c > > > > > > b/src/providers/ipa/ipa_init.c > > > > > > index > > > > > > a5ab881821c3d169285cb1dca7605a221b4cf604..f5e482344f405abd453bf54ed24110cebc65fb65 > > > > > > 100644 > > > > > > --- a/src/providers/ipa/ipa_init.c > > > > > > +++ b/src/providers/ipa/ipa_init.c > > > > > > @@ -256,6 +256,18 @@ int sssm_ipa_id_init(struct be_ctx *bectx, > > > > > > be_fo_set_srv_lookup_plugin(bectx, ipa_srv_plugin_send, > > > > > > ipa_srv_plugin_recv, srv_ctx, > > > > > > "IPA"); > > > > > > } else if (server_mode == true) { > > > > > > + /* In server mode we need to ignore the > > > > > > dns_discovery_domain if set > > > > > > + * and only discover servers based on AD domains > > > > > > + */ > > > > > > + ret = dp_opt_set_string(bectx->be_res->opts, > > > > > > DP_RES_OPT_DNS_DOMAIN, > > > > > > + NULL); > > > > > > > > > > In general I agree with the patch mainly because it looks there is no > > > > > easy way to only let the AD SRV plugin in ipa_server_mode know to > > > > > ignore > > > > > the option. > > > > > > > > > > But I would suggest to add some extra checks. > > > > > > > > > > The dns_discovery_domain option is only set by ipa-client-install if > > > > > the > > > > > host where ipa-client-install is running is not from the IPA domain. > > > > > Because in this case it might not be possible to find IPA server with > > > > > DNS SRV lookups where the domain name of the host is used. But since > > > > > we > > > > > are running on a server (server_mode == true) ipa-client-install will > > > > > by > > > > > default not configure any SRV lookups for the IPA server, i.e. there > > > > > will be no _srv_ in the ipa_server option. > > > > > > > > > > So with the default everything would work well with the patch. But if > > > > > someone modifies sssd.conf manually it might break. I would suggest to > > > > > check if the DNS domain of the host is the same as the IPA domain. If > > > > > not and ipa_server only contains _srv_ I would not set > > > > > DP_RES_OPT_DNS_DOMAIN to NULL but send a log message even to syslog > > > > > telling the admin that AD lookups will not work in this setup and > > > > > ipa_server should only contain the local hostname. > > > > > > > > OK, you're right we should add more checks, see the attached patch. > > > > > > > > But I think we should warn anytime the dns_discovery_domain is used. > > > > Because > > > > the current failover code would blindly use it for AD discovery, the > > > > setup > > > > could only ever work if dns_discovery_domain was set to the AD domain. > > > > Then > > > > the SSSD could fail over from SRV to local hostname and AD would use the > > > > DNS domain. > > > > > > yes, I agree. Patch looks good and is working as expected. I only have > > > one minor request. Since we override a config value a > > > SSSDBG_CONF_SETTINGS debug message should tell the admin about it. > > > > > > bye, > > > Sumit > > > > Thank you for the review, a new patch that prints the DEBUG message when > > the code succeeds in resetting the domain is attached. > > ACK > > bye, > Sumit
Pushed to master and sssd-1-11 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel