https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
ACK
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
And it should be also documented in manual pages.
LS
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
LS
On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
Fine, OK.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
Yes, but that's an invalid configuration with default_domain_suffix enabled. I don't feel too strongly, though, I just wonder if documenting every corner case in man pages can turn into noise...if you think it's important for users to have this case documented, let's add it..
On (26/02/15 11:17), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/2569
Simple patch is attached. I do not think that man page update is necessary.
Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
Fine, OK.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
Yes, but that's an invalid configuration with default_domain_suffix enabled. I don't feel too strongly, though, I just wonder if documenting every corner case in man pages can turn into noise...if you think it's important for users to have this case documented, let's add it..
If you don't want to document all corner cases then we should not create hacks (ignore/override user specified options).
IMHO, We should just give an advice (message to syslog?) that user should enable the option use_fully_qualified_names if the option default_domain_suffix is configured.
It does not mean I'm against current patch with updating documentation.
LS
On Thu, Feb 26, 2015 at 11:26:13AM +0100, Lukas Slebodnik wrote:
On (26/02/15 11:17), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote:
On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek wrote: > https://fedorahosted.org/sssd/ticket/2569 > > Simple patch is attached. > I do not think that man page update is necessary. > > Michal
I'm sorry the review took so long. Seems to work fine: [jhrozek@client] sssd $ [(review)] getent passwd administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] getent passwd admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash [jhrozek@client] sssd $ [(review)] su - admin@ipa.example.com Password: [admin@ipa.example.com@client ~]$ logout [jhrozek@client] sssd $ [(review)] getent passwd admin [jhrozek@client] sssd $ [(review)] su - administrator Password: -sh-4.3$ logout
The code looks almost good to me, I will just put a space between "if" and "(".
CI link: http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
Fine, OK.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
Yes, but that's an invalid configuration with default_domain_suffix enabled. I don't feel too strongly, though, I just wonder if documenting every corner case in man pages can turn into noise...if you think it's important for users to have this case documented, let's add it..
If you don't want to document all corner cases then we should not create hacks (ignore/override user specified options).
IMHO, We should just give an advice (message to syslog?) that user should enable the option use_fully_qualified_names if the option default_domain_suffix is configured.
It does not mean I'm against current patch with updating documentation.
OK, let's update the documentation, then.
On Thu, 2015-02-26 at 14:01 +0100, Jakub Hrozek wrote:
On Thu, Feb 26, 2015 at 11:26:13AM +0100, Lukas Slebodnik wrote:
On (26/02/15 11:17), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote:
On (25/02/15 20:57), Jakub Hrozek wrote: > On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek > wrote: > > https://fedorahosted.org/sssd/ticket/2569 > > > > Simple patch is attached. > > I do not think that man page update is necessary. > > > > Michal > > I'm sorry the review took so long. Seems to work fine: > [jhrozek@client] sssd $ [(review)] getent passwd > administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: > > [jhrozek@client] sssd $ [(review)] getent passwd admin > [jhrozek@client] sssd $ [(review)] getent passwd > admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash > > [jhrozek@client] sssd $ [(review)] su - > admin@ipa.example.com > Password: > [admin@ipa.example.com@client ~]$ logout > [jhrozek@client] sssd $ [(review)] getent passwd admin > [jhrozek@client] sssd $ [(review)] su - administrator > Password: > -sh-4.3$ logout > > The code looks almost good to me, I will just put a > space between "if" and "(". > > CI link: > http://sssd-ci.duckdns.org/logs/job/8/31/summary.html I would prefer to add debug message(SSSDBG_CONF_SETTINGS) that we use different default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
Fine, OK.
And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
Yes, but that's an invalid configuration with default_domain_suffix enabled. I don't feel too strongly, though, I just wonder if documenting every corner case in man pages can turn into noise...if you think it's important for users to have this case documented, let's add it..
If you don't want to document all corner cases then we should not create hacks (ignore/override user specified options).
IMHO, We should just give an advice (message to syslog?) that user should enable the option use_fully_qualified_names if the option default_domain_suffix is configured.
It does not mean I'm against current patch with updating documentation.
OK, let's update the documentation, then.
I'd also recommend that if default_domain_suffix is specified, it should be a configuration error to set use_fully_qualified_names=False and that SSSD should fail to start.
On 02/26/2015 06:55 PM, Stephen Gallagher wrote:
On Thu, 2015-02-26 at 14:01 +0100, Jakub Hrozek wrote:
On Thu, Feb 26, 2015 at 11:26:13AM +0100, Lukas Slebodnik wrote:
On (26/02/15 11:17), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
On (25/02/15 23:34), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik wrote: > On (25/02/15 20:57), Jakub Hrozek wrote: >> On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek >> wrote: >>> https://fedorahosted.org/sssd/ticket/2569 >>> >>> Simple patch is attached. >>> I do not think that man page update is necessary. >>> >>> Michal >> >> I'm sorry the review took so long. Seems to work fine: >> [jhrozek@client] sssd $ [(review)] getent passwd >> administrator administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator: >> >> [jhrozek@client] sssd $ [(review)] getent passwd admin >> [jhrozek@client] sssd $ [(review)] getent passwd >> admin@ipa.example.com admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash >> >> [jhrozek@client] sssd $ [(review)] su - >> admin@ipa.example.com >> Password: >> [admin@ipa.example.com@client ~]$ logout >> [jhrozek@client] sssd $ [(review)] getent passwd admin >> [jhrozek@client] sssd $ [(review)] su - administrator >> Password: >> -sh-4.3$ logout >> >> The code looks almost good to me, I will just put a >> space between "if" and "(". >> >> CI link: >> http://sssd-ci.duckdns.org/logs/job/8/31/summary.html > I would prefer to add debug message(SSSDBG_CONF_SETTINGS) > that we use different > default value for use_fully_qualified name.
It's just internal issue, see below.
I do not see any reason why it cannot be logged.
Fine, OK.
> > And it should be also documented in manual pages.
default_domain_suffix documentation already says: "Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name, to log in"
The fact that we say fqdn=true internally is just our internal issue.
It's not just an internal change.
The result of this patch is that configuration line "use_fully_qualified_names = false" will be ignored if the option default_domain_suffix is configured
And it is not documented.
Yes, but that's an invalid configuration with default_domain_suffix enabled. I don't feel too strongly, though, I just wonder if documenting every corner case in man pages can turn into noise...if you think it's important for users to have this case documented, let's add it..
If you don't want to document all corner cases then we should not create hacks (ignore/override user specified options).
IMHO, We should just give an advice (message to syslog?) that user should enable the option use_fully_qualified_names if the option default_domain_suffix is configured.
It does not mean I'm against current patch with updating documentation.
OK, let's update the documentation, then.
I'd also recommend that if default_domain_suffix is specified, it should be a configuration error to set use_fully_qualified_names=False and that SSSD should fail to start.
New patches are attached.
SSSD loudly complains to logs and fails to start if default_domain_suffix is used and use_fully_qualified_names is set to False (as Stephen suggested). Also the change of defaults value for use_fully_quallified_names is logged (as Lukas suggested). The man page was also updated (in seperate patch).
Michal
On (03/03/15 22:45), Michal Židek wrote:
New patches are attached.
SSSD loudly complains to logs and fails to start if default_domain_suffix is used and use_fully_qualified_names is set to False (as Stephen suggested). Also the change of defaults value for use_fully_quallified_names is logged (as Lukas suggested). The man page was also updated (in seperate patch).
Michal
From 359f48a25aba03e036dfd7317dd0b34c8e80cd1e Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 +
Patch cannot be applied to master due to conflict with newly added error code ERR_TIMESPEC_NOT_SUPPORTED
LS
On 03/06/2015 05:47 PM, Lukas Slebodnik wrote:
On (03/03/15 22:45), Michal Židek wrote:
New patches are attached.
SSSD loudly complains to logs and fails to start if default_domain_suffix is used and use_fully_qualified_names is set to False (as Stephen suggested). Also the change of defaults value for use_fully_quallified_names is logged (as Lukas suggested). The man page was also updated (in seperate patch).
Michal
From 359f48a25aba03e036dfd7317dd0b34c8e80cd1e Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 +
Patch cannot be applied to master due to conflict with newly added error code ERR_TIMESPEC_NOT_SUPPORTED
LS
Thank you Lukas.
New patch is attached.
Michal
On (06/03/15 19:28), Michal Židek wrote:
On 03/06/2015 05:47 PM, Lukas Slebodnik wrote:
On (03/03/15 22:45), Michal Židek wrote:
New patches are attached.
SSSD loudly complains to logs and fails to start if default_domain_suffix is used and use_fully_qualified_names is set to False (as Stephen suggested). Also the change of defaults value for use_fully_quallified_names is logged (as Lukas suggested). The man page was also updated (in seperate patch).
Michal
From 359f48a25aba03e036dfd7317dd0b34c8e80cd1e Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 +
Patch cannot be applied to master due to conflict with newly added error code ERR_TIMESPEC_NOT_SUPPORTED
LS
Thank you Lukas.
New patch is attached.
Michal
From e16ab528c95a365c9156063ae4a761a45d2c3e3f Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 3 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index f94de30..b1146b9 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -768,6 +768,8 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, const char *tmp; int ret, val; uint32_t entry_cache_timeout;
char *default_domain;
bool fqnames_default = false;
tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) return ENOMEM;
@@ -903,11 +905,38 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, DEBUG(SSSDBG_TRACE_FUNC, "No enumeration for [%s]!\n", domain->name); }
- ret = confdb_get_string(cdb, tmp_ctx, CONFDB_MONITOR_CONF_ENTRY,
CONFDB_MONITOR_DEFAULT_DOMAIN, NULL,
&default_domain);
- if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannnot get the default domain [%d]: %s\n",
ret, strerror(ret));
goto done;
- }
- /* Determine if user/group names will be Fully Qualified
- in NSS interfaces */
- ret = get_entry_as_bool(res->msgs[0], &domain->fqnames, CONFDB_DOMAIN_FQ, 0);
- if(ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE, "Invalid value for %s\n", CONFDB_DOMAIN_FQ);
- if (default_domain != NULL) {
DEBUG(SSSDBG_CONF_SETTINGS,
"Default domain suffix set. Changing default for "
"use_fully_qualified_names to True.\n");
fqnames_default = true;
- }
- ret = get_entry_as_bool(res->msgs[0], &domain->fqnames,
CONFDB_DOMAIN_FQ, fqnames_default);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE, "Invalid value for %s\n",
CONFDB_DOMAIN_FQ);
goto done;
- }
- if (default_domain != NULL && domain->fqnames == false) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Invalid configuration detected (default_domain_suffix is used "
"while use_fully_qualified_names was set to false).\n");
ret = ERR_INVALID_CONFIG;
The function confdb_get_domain_internal returns int. It is not big problem because errnot_t is typdef to int, but strerror is used to transform return code(errno) to string. It means your new error code would be be used.
LS
On 03/10/2015 11:17 AM, Lukas Slebodnik wrote:
On (06/03/15 19:28), Michal Židek wrote:
On 03/06/2015 05:47 PM, Lukas Slebodnik wrote:
On (03/03/15 22:45), Michal Židek wrote:
New patches are attached.
SSSD loudly complains to logs and fails to start if default_domain_suffix is used and use_fully_qualified_names is set to False (as Stephen suggested). Also the change of defaults value for use_fully_quallified_names is logged (as Lukas suggested). The man page was also updated (in seperate patch).
Michal
From 359f48a25aba03e036dfd7317dd0b34c8e80cd1e Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 +
Patch cannot be applied to master due to conflict with newly added error code ERR_TIMESPEC_NOT_SUPPORTED
LS
Thank you Lukas.
New patch is attached.
Michal
From e16ab528c95a365c9156063ae4a761a45d2c3e3f Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 35 ++++++++++++++++++++++++++++++++--- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 3 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index f94de30..b1146b9 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -768,6 +768,8 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, const char *tmp; int ret, val; uint32_t entry_cache_timeout;
char *default_domain;
bool fqnames_default = false;
tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) return ENOMEM;
@@ -903,11 +905,38 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, DEBUG(SSSDBG_TRACE_FUNC, "No enumeration for [%s]!\n", domain->name); }
- ret = confdb_get_string(cdb, tmp_ctx, CONFDB_MONITOR_CONF_ENTRY,
CONFDB_MONITOR_DEFAULT_DOMAIN, NULL,
&default_domain);
- if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannnot get the default domain [%d]: %s\n",
ret, strerror(ret));
goto done;
- }
- /* Determine if user/group names will be Fully Qualified * in NSS interfaces */
- ret = get_entry_as_bool(res->msgs[0], &domain->fqnames, CONFDB_DOMAIN_FQ, 0);
- if(ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE, "Invalid value for %s\n", CONFDB_DOMAIN_FQ);
- if (default_domain != NULL) {
DEBUG(SSSDBG_CONF_SETTINGS,
"Default domain suffix set. Changing default for "
"use_fully_qualified_names to True.\n");
fqnames_default = true;
- }
- ret = get_entry_as_bool(res->msgs[0], &domain->fqnames,
CONFDB_DOMAIN_FQ, fqnames_default);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE, "Invalid value for %s\n",
CONFDB_DOMAIN_FQ);
goto done;
- }
- if (default_domain != NULL && domain->fqnames == false) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Invalid configuration detected (default_domain_suffix is used "
"while use_fully_qualified_names was set to false).\n");
ret = ERR_INVALID_CONFIG;
The function confdb_get_domain_internal returns int. It is not big problem because errnot_t is typdef to int, but strerror is used to transform return code(errno) to string. It means your new error code would be be used.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thank you Lukas.
I will not change the 'int' to 'errno_t'. We have many places in code where we do not distinguish between the two and the purpose of this patch is different. But I updated the caller to use sss_strerror so that the error message is picked up correctly.
Updated patch is attached.
Michal
On (10/03/15 16:15), Michal Židek wrote:
Thank you Lukas.
I will not change the 'int' to 'errno_t'. We have many places in code where we do not distinguish between the two and the purpose of this patch is different. But I updated the caller to use sss_strerror so that the error message is picked up correctly.
Updated patch is attached.
Michal
From 1a593350815e62483b55c9b4d801382c7fbf0ad4 Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Fri, 30 Jan 2015 15:48:04 +0100 Subject: [PATCH 1/2] Use FQDN if default domain was set
https://fedorahosted.org/sssd/ticket/2569
src/confdb/confdb.c | 37 +++++++++++++++++++++++++++++++++---- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 3 files changed, 35 insertions(+), 4 deletions(-)
ACK http://sssd-ci.duckdns.org/logs/job/9/31/summary.html
From d7af22356db18481e0bc8a6ad503221ca154e8c7 Mon Sep 17 00:00:00 2001 From: Michal Zidek mzidek@redhat.com Date: Tue, 3 Mar 2015 22:21:29 +0100 Subject: [PATCH 2/2] MAN: default_domain_suffix with use_fully_qualified_names.
https://fedorahosted.org/sssd/ticket/2569
src/man/sssd.conf.5.xml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index ca4e602..92c64b9 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -330,7 +330,10 @@ Please note that if this option is set all users from the primary domain have to use their fully qualified name, e.g. user@domain.name,
to log in.
to log in. Setting this option changes default
of use_fully_qualified_names to True. It is not
allowed to use this option together with
use_fully_qualified_names set to False. </para> <para> Default: not set
@@ -1494,7 +1497,8 @@ pam_account_expired_message = Account expired, please call help desk. name is requested. </para> <para>
Default: FALSE
Default: FALSE (TRUE if default_domain_suffix is
used) </para> </listitem> </varlistentry>
LGTM
I would like to have "review++" for man page chage from someone else.
LS
On Tue, Mar 10, 2015 at 06:52:10PM +0100, Lukas Slebodnik wrote:
LGTM
I would like to have "review++" for man page chage from someone else.
LS
Man page changes look good to me, pushed to master: * 804df4040eb142f82a44c019c7a55b5ce524583c * 9619e0ae811958821d96466e419bf7f9928086bd
sssd-devel@lists.fedorahosted.org