Bug 1431153

This file does not change any behaviour.
It is a template for users who want to enable logging to journald.

sh$ grep journal.conf sssd.spec 
%config(noreplace) %{_sysconfdir}/systemd/system/sssd.service.d/journal.conf

sh$ cat /etc/systemd/system/sssd.service.d/journal.conf
[Service]
# Uncomment *both* of the following lines to enable debug logging
# to go to journald instead of /var/log/sssd. You will need to
# run 'systemctl daemon-reload' and then restart the SSSD service
# for this to take effect
#ExecStart=
#ExecStart=/usr/sbin/sssd -i

Per systemd.unit(5):

       Table 1.  Load path when running in system mode (--system).
       ┌────────────────────────┬─────────────────────┐
       │Path                    │ Description         │
       ├────────────────────────┼─────────────────────┤
       │/etc/systemd/system     │ Local configuration │
       ├────────────────────────┼─────────────────────┤
       │/run/systemd/system     │ Runtime units       │
       ├────────────────────────┼─────────────────────┤
       │/usr/lib/systemd/system │ Units of installed  │
       │                        │ packages            │
       └────────────────────────┴─────────────────────┘

Files owned by installed packages should not be in /etc/systemd/system.

If it's just an example, please put it to the documentation directory.

It is not against packaging guidelines

And BTW this files is aimed for local configuration; (to change default logging from files to journald). So location is correct.

It is very bad packaging engineering.

For the user to enable something, they should make a symlink to existing file, not edit existing file, and deal with .rpmnew when the next version of SSSD changes the recommendation.

(In reply to Jan Pazdziora from comment #5)
> It is very bad packaging engineering.

But you proposal for symlink to %config file in /usr is even worse.
It violates fedora packaging guidelines.

http://fedoraproject.org/wiki/Packaging:Guidelines#Configuration_files
"Don't use %config or %config(noreplace) under /usr. /usr is deemed to not contain configuration files in Fedora."

The packaging guidelines in fact says systemd unit files must not be marked %config[1], so I assume this file also should not be marked config and be moved elsewhere.
Packages are not supposed to drop files in /etc/systemd/system

[1] http://fedoraproject.org/wiki/Packaging:Systemd
"Systemd unit .service files must not be marked as %config files."

This is something that either go in %{_unitdir} or in a doc directory.
Please move it.

It is not ".service" file it is a configuration snippet and all sssd service files are properly stored in %{_unitdir}.

sh$ rpm -ql sssd-common | grep service$
/usr/lib/systemd/system/sssd-autofs.service
/usr/lib/systemd/system/sssd-ifp.service
/usr/lib/systemd/system/sssd-nss.service
/usr/lib/systemd/system/sssd-pac.service
/usr/lib/systemd/system/sssd-pam.service
/usr/lib/systemd/system/sssd-secrets.service
/usr/lib/systemd/system/sssd-ssh.service
/usr/lib/systemd/system/sssd-sudo.service
/usr/lib/systemd/system/sssd.service

(In reply to Lukas Slebodnik from comment #6)
> (In reply to Jan Pazdziora from comment #5)
> > It is very bad packaging engineering.
> 
> But you proposal for symlink to %config file in /usr is even worse.
> It violates fedora packaging guidelines.

I did not say where that symlink should point to.

(In reply to Lukas Slebodnik from comment #8)
> It is not ".service" file it is a configuration snippet and all sssd service

It is .service file snippet.

To be honest I am not even sure why we have this file which just contains comments.
Add the comments to the main service file that is in /lib/systemd/system and let users create a unit file based on the comments if they need.

Also please stop closing this bug until the discussion is settled.
Having a package dropping files in /etc/systemd *is* an issue.

(In reply to Simo Sorce from comment #11)
> To be honest I am not even sure why we have this file which just contains
> comments.
Explained in comment#1

> Add the comments to the main service file that is in /lib/systemd/system and
> let users create a unit file based on the comments if they need.
> 
Current approach is much user-friendly.
Because users will not notice change in /usr/lib/systemd/system/sssd.service
like in commit https://github.com/SSSD/sssd/commit/d4063e9a21a4e203bee7e0a0144fa8cabb14cc46

It's much convenient to compare journald.conf and journald.conf.rpmnew

> Also please stop closing this bug until the discussion is settled.
> Having a package dropping files in /etc/systemd *is* an issue.

I cannot see a reason to have opened ticket which is not a bug.
And discussion can keep going even in closed ticket.

I started a thread on fedora-devel to sort this out, so far 2 responses both agree that rpms should drop nothing in /etc/systemd/system:

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
/message/J4KJEC25HZ7BCZPR4UIY2TMXQPV5MX2Y/

I do not consider empty file in /etc/systemd/system as a unit file.

Reassigning to sgallagh who is author of this feature.
NOTE: removing file is not a solution; it will break upgrades. from "Type=forking"  to "Type=notify"
https://pagure.io/SSSD/sssd/c/d4063e9a21a4e203bee7e0a0144fa8cabb14cc46
https://pagure.io/SSSD/sssd/c/7b4704a10958bb7d3390db9eff863875d2b643f7

This ticket is definitely a bug, for the record. I made a bad choice when I landed this the way I did and it's biting us now.

The proper approach here should probably have been to change the SSSD unit file so that it would look something like:


```
[Unit]
Description=System Security Services Daemon
# SSSD must be running before we permit user sessions
Before=systemd-user-sessions.service nss-user-lookup.target
Wants=nss-user-lookup.target

[Service]
EnvironmentFile=-/etc/sysconfig/sssd
ExecStart=/usr/sbin/sssd -i ${DEBUG_LOGGER}
Type=notify
NotifyAccess=main
CapabilityBoundingSet=CAP_IPC_LOCK CAP_CHOWN CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE CAP_KILL CAP_NET_ADMIN CAP_SYS_NICE CAP_FOWNER CAP_SETGID CAP_SETUID CAP_SYS_ADMIN CAP_SYS_RESOURCE CAP_BLOCK_SUSPEND

[Install]
WantedBy=multi-user.target

```

/etc/sysconfig/sssd

```
# SSSD debug logs go to local files
DEBUG_LOGGER=-f

```


Now, with that being said, I'm not sure how we would want to handle the migration. One idea: detect in %post if /etc/systemd/system/sssd.service.d/journal.conf hashes to one of two known results: either the original commented-out version or the version that contains just the two lines uncommented. If it's the original version, just delete it. If it's the uncommented version, `echo "DEBUG_LOGGER=" >> /etc/sysconfig/sssd` (this will override the earlier value in the file, then delete the override file. If it doesn't match to either, someone made manual edits and we should leave it alone.

This sounds like a reasonable idea, with the caveat that you should remove also any .rpmnew .rpmsave or whatever may have been created during upgrades where a changed file was present.

This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Fedora 25 changed to end-of-life (EOL) status on 2017-12-12. Fedora 25 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.