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(a)ipa.example.com
> >>
>admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash
> >> >[jhrozek@client] sssd $ [(review)] su - admin(a)ipa.example.com
> >> >Password:
> >> >[admin@ipa.example.com(a)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(a)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