Hi,
attached are three small but important patches related to sssd-secrets. The context is that I started to write tests and manpage for sssd-secrets and noticed some issues. I hope the patches themselves offer a nice commit message.
To test the socket activation, you can just install the RPMs built with these patches and call the socket responder, for example: curl -H "Content-Type: application/json" \ --unix-socket /var/run/secrets.socket \ -XGET http://localhost/secrets/
(You can see more examples in the WIP manpage at: https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a07...)
If these patches are accepted, I'll also amend the manpage to list the service and the socket.
btw it occured to me we might want to support service self-termination, but I would prefer to do that in another patchset and tracked with a ticket.
On Mon, Aug 15, 2016 at 04:01:13PM +0200, Jakub Hrozek wrote:
Hi,
attached are three small but important patches related to sssd-secrets. The context is that I started to write tests and manpage for sssd-secrets and noticed some issues. I hope the patches themselves offer a nice commit message.
To test the socket activation, you can just install the RPMs built with these patches and call the socket responder, for example: curl -H "Content-Type: application/json" \ --unix-socket /var/run/secrets.socket \ -XGET http://localhost/secrets/
(You can see more examples in the WIP manpage at: https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a07...)
If these patches are accepted, I'll also amend the manpage to list the service and the socket.
btw it occured to me we might want to support service self-termination, but I would prefer to do that in another patchset and tracked with a ticket.
Let's try it with the patches actually..
On Mon, Aug 15, 2016 at 4:05 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Mon, Aug 15, 2016 at 04:01:13PM +0200, Jakub Hrozek wrote:
Hi,
attached are three small but important patches related to sssd-secrets. The context is that I started to write tests and manpage for sssd-secrets and noticed some issues. I hope the patches themselves offer a nice commit message.
To test the socket activation, you can just install the RPMs built with these patches and call the socket responder, for example: curl -H "Content-Type: application/json" \ --unix-socket /var/run/secrets.socket \ -XGET http://localhost/secrets/
(You can see more examples in the WIP manpage at: https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a07...)
If these patches are accepted, I'll also amend the manpage to list the service and the socket.
btw it occured to me we might want to support service self-termination, but I would prefer to do that in another patchset and tracked with a ticket.
Let's try it with the patches actually..
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
Patchs 0001 and 0002 look good to me. About the patch 0003, it has a commit message typo (s/acticate/activate). By a simple review it looks good to me but, sincerely, I'd like to have someone else to review it as well (I'll do the tests later on ...).
Best Regards, -- Fabiano Fidêncio
On Mon, Aug 15, 2016 at 4:17 PM, Fabiano Fidêncio fidencio@redhat.com wrote:
On Mon, Aug 15, 2016 at 4:05 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Mon, Aug 15, 2016 at 04:01:13PM +0200, Jakub Hrozek wrote:
Hi,
attached are three small but important patches related to sssd-secrets. The context is that I started to write tests and manpage for sssd-secrets and noticed some issues. I hope the patches themselves offer a nice commit message.
To test the socket activation, you can just install the RPMs built with these patches and call the socket responder, for example: curl -H "Content-Type: application/json" \ --unix-socket /var/run/secrets.socket \ -XGET http://localhost/secrets/
(You can see more examples in the WIP manpage at: https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a07...)
If these patches are accepted, I'll also amend the manpage to list the service and the socket.
btw it occured to me we might want to support service self-termination, but I would prefer to do that in another patchset and tracked with a ticket.
Let's try it with the patches actually..
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
Patchs 0001 and 0002 look good to me. About the patch 0003, it has a commit message typo (s/acticate/activate). By a simple review it looks good to me but, sincerely, I'd like to have someone else to review it as well (I'll do the tests later on ...).
Okay. I've done some tests and the patches are working as expected. During the tests and with Jakub's help a non-related bug was found, basically because the "/var/libs/sss/secrets/" dir wasn't created. Jakub will provide a patch for this issue.
So, please, consider these patches ACKed and feel free to push them (considering they pass CI, which I didn't run).
Best Regards, -- Fabiano Fidêncio
On Mon, 2016-08-15 at 16:05 +0200, Jakub Hrozek wrote:
On Mon, Aug 15, 2016 at 04:01:13PM +0200, Jakub Hrozek wrote:
Hi,
attached are three small but important patches related to sssd-secrets. The context is that I started to write tests and manpage for sssd-secrets and noticed some issues. I hope the patches themselves offer a nice commit message.
To test the socket activation, you can just install the RPMs built with these patches and call the socket responder, for example: curl -H "Content-Type: application/json" \ --unix-socket /var/run/secrets.socket \ -XGET http://localhost/secrets/
(You can see more examples in the WIP manpage at: https://github.com/jhrozek/sssd/commit/508c9eec040cada06bc6a61fe200f2db20a07...)
If these patches are accepted, I'll also amend the manpage to list the service and the socket.
btw it occured to me we might want to support service self-termination, but I would prefer to do that in another patchset and tracked with a ticket.
Let's try it with the patches actually..
LGTM all three.
Simo.
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
LS
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
I'm still working on the manpage..
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
LS
On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
See the new patch:
# systemctl disable sssd-secrets.service Removed symlink /etc/systemd/system/sockets.target.wants/sssd-secrets.socket. # systemctl enable sssd-secrets.service Created symlink from /etc/systemd/system/sockets.target.wants/sssd-secrets.socket to /usr/lib/systemd/system/sssd-secrets.socket.
I think that's the Also= directive at work
The Description now shows with "systemctl show sssd-secrets.service" and .socket.
On (17/08/16 13:46), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
See the new patch:
# systemctl disable sssd-secrets.service Removed symlink /etc/systemd/system/sockets.target.wants/sssd-secrets.socket. # systemctl enable sssd-secrets.service Created symlink from /etc/systemd/system/sockets.target.wants/sssd-secrets.socket to /usr/lib/systemd/system/sssd-secrets.socket.
I think that's the Also= directive at work
OK
The Description now shows with "systemctl show sssd-secrets.service" and .socket.
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we should try to restart sssd-secrets.service and not just sssd-secrets.socket. Otherwise, old sssd-sevice will be running after upgrade.
LS
On Wed, Aug 17, 2016 at 02:30:15PM +0200, Lukas Slebodnik wrote:
On (17/08/16 13:46), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote:
From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Mon, 15 Aug 2016 14:10:23 +0200 Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets
Adds two new files: sssd-secrets.socket and sssd-secrets.service. These can be used to socket-acticate the secrets responder even without explicitly starting it in the sssd config file.
The specfile activates the socket after installation which means that the admin would just be able to use the secrets socket and the sssd_secrets responder would be started automatically by systemd.
The sssd-secrets responder is started as root, mostly because I didn't think of an easy way to pass the uid/gid to the responders without asking about the sssd user identity in the first place. But nonetheless, the sssd-secrets responder wasn't tested as non-root and at least the initialization should be performed as root for the time being.
Makefile.am | 21 +++++++++++++++++++-- contrib/sssd.spec.in | 5 +++++ src/sysv/systemd/sssd-secrets.service.in | 2 ++ src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/sysv/systemd/sssd-secrets.service.in create mode 100644 src/sysv/systemd/sssd-secrets.socket.in
diff --git a/Makefile.am b/Makefile.am index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3848,7 +3848,10 @@ systemdunit_DATA = systemdconf_DATA = if HAVE_SYSTEMD_UNIT systemdunit_DATA += \
src/sysv/systemd/sssd.service
src/sysv/systemd/sssd.service \
src/sysv/systemd/sssd-secrets.socket \
src/sysv/systemd/sssd-secrets.service \
$(NULL)
if WITH_JOURNALD systemdconf_DATA += \ src/sysv/systemd/journal.conf @@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ -e 's|@sbindir[@]|$(sbindir)|g' \ -e 's|@environment_file[@]|$(environment_file)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \
-e 's|@libexecdir[@]|$(libexecdir)|g' \ -e 's|@prefix[@]|$(prefix)|g'
replace_script = \ @@ -3897,7 +3901,10 @@ replace_script = \
EXTRA_DIST += \ src/sysv/systemd/sssd.service.in \
- src/sysv/systemd/journal.conf.in
- src/sysv/systemd/journal.conf.in \
- src/sysv/systemd/sssd-secrets.socket.in \
- src/sysv/systemd/sssd-secrets.service.in \
- $(NULL)
src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile @$(MKDIR_P) src/sysv/systemd/ @@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile @$(MKDIR_P) src/sysv/systemd/ $(replace_script)
+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile
- @$(MKDIR_P) src/sysv/systemd/
- $(replace_script)
SSSD_USER_DIRS = \ $(DESTDIR)$(dbpath) \ $(DESTDIR)$(keytabdir) \ @@ -4122,6 +4137,8 @@ endif done; rm -Rf ldb_mod_test_dir rm -f $(builddir)/src/sysv/systemd/sssd.service
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket
- rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service rm -f $(builddir)/src/sysv/systemd/journal.conf
CLEANFILES += *.X */*.X */*/*.X diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -746,6 +746,8 @@ done %{_sbindir}/sssd %if (0%{?use_systemd} == 1) %{_unitdir}/sssd.service +%{_unitdir}/sssd-secrets.socket +%{_unitdir}/sssd-secrets.service %else %{_initrddir}/%{name} %endif @@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us # systemd %post common %systemd_post sssd.service +%systemd_post sssd-secrets.socket
%preun common %systemd_preun sssd.service +%systemd_preun sssd-secrets.socket
%postun common %systemd_postun_with_restart sssd.service +%systemd_postun_with_restart sssd-secrets.socket
systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
%else # sysv diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in new file mode 100644 index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 --- /dev/null +++ b/src/sysv/systemd/sssd-secrets.service.in @@ -0,0 +1,2 @@ +[Service] +ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
See the new patch:
# systemctl disable sssd-secrets.service Removed symlink /etc/systemd/system/sockets.target.wants/sssd-secrets.socket. # systemctl enable sssd-secrets.service Created symlink from /etc/systemd/system/sockets.target.wants/sssd-secrets.socket to /usr/lib/systemd/system/sssd-secrets.socket.
I think that's the Also= directive at work
OK
The Description now shows with "systemctl show sssd-secrets.service" and .socket.
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we should try to restart sssd-secrets.service and not just sssd-secrets.socket. Otherwise, old sssd-sevice will be running after upgrade.
OK, the attached patch also adds "%systemd_postun_with_restart sssd-secrets.service"
On (17/08/16 15:03), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 02:30:15PM +0200, Lukas Slebodnik wrote:
On (17/08/16 13:46), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote:
On (15/08/16 16:05), Jakub Hrozek wrote: >From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 >From: Jakub Hrozek jhrozek@redhat.com >Date: Mon, 15 Aug 2016 14:10:23 +0200 >Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets > >Adds two new files: sssd-secrets.socket and sssd-secrets.service. These >can be used to socket-acticate the secrets responder even without >explicitly starting it in the sssd config file. > >The specfile activates the socket after installation which means that >the admin would just be able to use the secrets socket and the >sssd_secrets responder would be started automatically by systemd. > >The sssd-secrets responder is started as root, mostly because I didn't >think of an easy way to pass the uid/gid to the responders without >asking about the sssd user identity in the first place. But nonetheless, >the sssd-secrets responder wasn't tested as non-root and at least the >initialization should be performed as root for the time being. >--- > Makefile.am | 21 +++++++++++++++++++-- > contrib/sssd.spec.in | 5 +++++ > src/sysv/systemd/sssd-secrets.service.in | 2 ++ > src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ > 4 files changed, 31 insertions(+), 2 deletions(-) > create mode 100644 src/sysv/systemd/sssd-secrets.service.in > create mode 100644 src/sysv/systemd/sssd-secrets.socket.in > >diff --git a/Makefile.am b/Makefile.am >index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 >--- a/Makefile.am >+++ b/Makefile.am >@@ -3848,7 +3848,10 @@ systemdunit_DATA = > systemdconf_DATA = > if HAVE_SYSTEMD_UNIT > systemdunit_DATA += \ >- src/sysv/systemd/sssd.service >+ src/sysv/systemd/sssd.service \ >+ src/sysv/systemd/sssd-secrets.socket \ >+ src/sysv/systemd/sssd-secrets.service \ >+ $(NULL) > if WITH_JOURNALD > systemdconf_DATA += \ > src/sysv/systemd/journal.conf >@@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ > -e 's|@sbindir[@]|$(sbindir)|g' \ > -e 's|@environment_file[@]|$(environment_file)|g' \ > -e 's|@localstatedir[@]|$(localstatedir)|g' \ >+ -e 's|@libexecdir[@]|$(libexecdir)|g' \ > -e 's|@prefix[@]|$(prefix)|g' > > replace_script = \ >@@ -3897,7 +3901,10 @@ replace_script = \ > > EXTRA_DIST += \ > src/sysv/systemd/sssd.service.in \ >- src/sysv/systemd/journal.conf.in >+ src/sysv/systemd/journal.conf.in \ >+ src/sysv/systemd/sssd-secrets.socket.in \ >+ src/sysv/systemd/sssd-secrets.service.in \ >+ $(NULL) > > src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile > @$(MKDIR_P) src/sysv/systemd/ >@@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile > @$(MKDIR_P) src/sysv/systemd/ > $(replace_script) > >+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile >+ @$(MKDIR_P) src/sysv/systemd/ >+ $(replace_script) >+ >+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile >+ @$(MKDIR_P) src/sysv/systemd/ >+ $(replace_script) >+ > SSSD_USER_DIRS = \ > $(DESTDIR)$(dbpath) \ > $(DESTDIR)$(keytabdir) \ >@@ -4122,6 +4137,8 @@ endif > done; > rm -Rf ldb_mod_test_dir > rm -f $(builddir)/src/sysv/systemd/sssd.service >+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket >+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service > rm -f $(builddir)/src/sysv/systemd/journal.conf > > CLEANFILES += *.X */*.X */*/*.X >diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in >index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 >--- a/contrib/sssd.spec.in >+++ b/contrib/sssd.spec.in >@@ -746,6 +746,8 @@ done > %{_sbindir}/sssd > %if (0%{?use_systemd} == 1) > %{_unitdir}/sssd.service >+%{_unitdir}/sssd-secrets.socket >+%{_unitdir}/sssd-secrets.service > %else > %{_initrddir}/%{name} > %endif >@@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us > # systemd > %post common > %systemd_post sssd.service >+%systemd_post sssd-secrets.socket > > %preun common > %systemd_preun sssd.service >+%systemd_preun sssd-secrets.socket > > %postun common > %systemd_postun_with_restart sssd.service >+%systemd_postun_with_restart sssd-secrets.socket > systemd_post and systemd_preun should be used just with *.socket Because you do not want to directly enable/disable socket activated service
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
> %else > # sysv >diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in >new file mode 100644 >index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 >--- /dev/null >+++ b/src/sysv/systemd/sssd-secrets.service.in >@@ -0,0 +1,2 @@ >+[Service] >+ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files
I checked another socket activated service (cups, pcscd) and they have "Also=$name.socket" in Install section for service But on the other hand other socket activated services(cockpit, dbus, systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
Anyway, It would be good to add "[Unit]" section to both files + Description and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
See the new patch:
# systemctl disable sssd-secrets.service Removed symlink /etc/systemd/system/sockets.target.wants/sssd-secrets.socket. # systemctl enable sssd-secrets.service Created symlink from /etc/systemd/system/sockets.target.wants/sssd-secrets.socket to /usr/lib/systemd/system/sssd-secrets.socket.
I think that's the Also= directive at work
OK
The Description now shows with "systemctl show sssd-secrets.service" and .socket.
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we should try to restart sssd-secrets.service and not just sssd-secrets.socket. Otherwise, old sssd-sevice will be running after upgrade.
OK, the attached patch also adds "%systemd_postun_with_restart sssd-secrets.service"
ACK++
http://sssd-ci.duckdns.org/logs/job/51/87/summary.html
LS
On Wed, Aug 17, 2016 at 04:25:47PM +0200, Lukas Slebodnik wrote:
On (17/08/16 15:03), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 02:30:15PM +0200, Lukas Slebodnik wrote:
On (17/08/16 13:46), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 12:49:26PM +0200, Lukas Slebodnik wrote:
On (17/08/16 12:27), Jakub Hrozek wrote:
On Wed, Aug 17, 2016 at 10:23:51AM +0200, Lukas Slebodnik wrote: > On (15/08/16 16:05), Jakub Hrozek wrote: > >From 86a24747f45bdb3caeb8c36d63c74a08aed90421 Mon Sep 17 00:00:00 2001 > >From: Jakub Hrozek jhrozek@redhat.com > >Date: Mon, 15 Aug 2016 14:10:23 +0200 > >Subject: [PATCH 3/3] BUILD: Ship systemd service file for sssd-secrets > > > >Adds two new files: sssd-secrets.socket and sssd-secrets.service. These > >can be used to socket-acticate the secrets responder even without > >explicitly starting it in the sssd config file. > > > >The specfile activates the socket after installation which means that > >the admin would just be able to use the secrets socket and the > >sssd_secrets responder would be started automatically by systemd. > > > >The sssd-secrets responder is started as root, mostly because I didn't > >think of an easy way to pass the uid/gid to the responders without > >asking about the sssd user identity in the first place. But nonetheless, > >the sssd-secrets responder wasn't tested as non-root and at least the > >initialization should be performed as root for the time being. > >--- > > Makefile.am | 21 +++++++++++++++++++-- > > contrib/sssd.spec.in | 5 +++++ > > src/sysv/systemd/sssd-secrets.service.in | 2 ++ > > src/sysv/systemd/sssd-secrets.socket.in | 5 +++++ > > 4 files changed, 31 insertions(+), 2 deletions(-) > > create mode 100644 src/sysv/systemd/sssd-secrets.service.in > > create mode 100644 src/sysv/systemd/sssd-secrets.socket.in > > > >diff --git a/Makefile.am b/Makefile.am > >index 7c15bd7cdb336bc38f2055382121d73a3583b1e7..5d38fc96462356471f1f1506403b2d76961d485d 100644 > >--- a/Makefile.am > >+++ b/Makefile.am > >@@ -3848,7 +3848,10 @@ systemdunit_DATA = > > systemdconf_DATA = > > if HAVE_SYSTEMD_UNIT > > systemdunit_DATA += \ > >- src/sysv/systemd/sssd.service > >+ src/sysv/systemd/sssd.service \ > >+ src/sysv/systemd/sssd-secrets.socket \ > >+ src/sysv/systemd/sssd-secrets.service \ > >+ $(NULL) > > if WITH_JOURNALD > > systemdconf_DATA += \ > > src/sysv/systemd/journal.conf > >@@ -3886,6 +3889,7 @@ edit_cmd = $(SED) \ > > -e 's|@sbindir[@]|$(sbindir)|g' \ > > -e 's|@environment_file[@]|$(environment_file)|g' \ > > -e 's|@localstatedir[@]|$(localstatedir)|g' \ > >+ -e 's|@libexecdir[@]|$(libexecdir)|g' \ > > -e 's|@prefix[@]|$(prefix)|g' > > > > replace_script = \ > >@@ -3897,7 +3901,10 @@ replace_script = \ > > > > EXTRA_DIST += \ > > src/sysv/systemd/sssd.service.in \ > >- src/sysv/systemd/journal.conf.in > >+ src/sysv/systemd/journal.conf.in \ > >+ src/sysv/systemd/sssd-secrets.socket.in \ > >+ src/sysv/systemd/sssd-secrets.service.in \ > >+ $(NULL) > > > > src/sysv/systemd/sssd.service: src/sysv/systemd/sssd.service.in Makefile > > @$(MKDIR_P) src/sysv/systemd/ > >@@ -3907,6 +3914,14 @@ src/sysv/systemd/journal.conf: src/sysv/systemd/journal.conf.in Makefile > > @$(MKDIR_P) src/sysv/systemd/ > > $(replace_script) > > > >+src/sysv/systemd/sssd-secrets.socket: src/sysv/systemd/sssd-secrets.socket.in Makefile > >+ @$(MKDIR_P) src/sysv/systemd/ > >+ $(replace_script) > >+ > >+src/sysv/systemd/sssd-secrets.service: src/sysv/systemd/sssd-secrets.service.in Makefile > >+ @$(MKDIR_P) src/sysv/systemd/ > >+ $(replace_script) > >+ > > SSSD_USER_DIRS = \ > > $(DESTDIR)$(dbpath) \ > > $(DESTDIR)$(keytabdir) \ > >@@ -4122,6 +4137,8 @@ endif > > done; > > rm -Rf ldb_mod_test_dir > > rm -f $(builddir)/src/sysv/systemd/sssd.service > >+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.socket > >+ rm -f $(builddir)/src/sysv/systemd/sssd-secrets.service > > rm -f $(builddir)/src/sysv/systemd/journal.conf > > > > CLEANFILES += *.X */*.X */*/*.X > >diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in > >index a9a43560e61acf1cb4b2388d50303587252c2c19..b58eebba54a82a041f72507b430ceca708a34632 100644 > >--- a/contrib/sssd.spec.in > >+++ b/contrib/sssd.spec.in > >@@ -746,6 +746,8 @@ done > > %{_sbindir}/sssd > > %if (0%{?use_systemd} == 1) > > %{_unitdir}/sssd.service > >+%{_unitdir}/sssd-secrets.socket > >+%{_unitdir}/sssd-secrets.service > > %else > > %{_initrddir}/%{name} > > %endif > >@@ -1077,12 +1079,15 @@ getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "Us > > # systemd > > %post common > > %systemd_post sssd.service > >+%systemd_post sssd-secrets.socket > > > > %preun common > > %systemd_preun sssd.service > >+%systemd_preun sssd-secrets.socket > > > > %postun common > > %systemd_postun_with_restart sssd.service > >+%systemd_postun_with_restart sssd-secrets.socket > > > systemd_post and systemd_preun should be used just with *.socket > Because you do not want to directly enable/disable socket activated service > > systemd_postun_with_restart is an alias for "systemctl try-restart" > for package upgrade. So we shoudl try to restart sssd-secrets.service
I agree about not starting, but are you sure about not *enabling*? I can test that, but I thought that if the service wasn't enabled, then systemd wouldn't start it.
The latest patch do not enable sssd-secrets.service and that's fine. and other socket activated services are not enabled by default. So we needn't change systemd_post, systemd_preun.
[root@host ~]# systemctl status rpcbind.socket rpcbind.service ● rpcbind.socket - RPCbind Server Activation Socket Loaded: loaded (/usr/lib/systemd/system/rpcbind.socket; enabled; vendor preset: disabled) Active: active (listening) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /var/run/rpcbind.sock (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on RPCbind Server Activation Socket.
● rpcbind.service - RPC bind service Loaded: loaded (/usr/lib/systemd/system/rpcbind.service; indirect; vendor preset: disabled) Active: inactive (dead)
[root@host ~]# systemctl status cockpit.socket cockpit.service ● cockpit.socket - Cockpit Web Service Socket Loaded: loaded (/usr/lib/systemd/system/cockpit.socket; enabled; vendor preset: enabled) Active: active (listening) since Fri 2016-08-12 08:32:39 CEST; 5 days ago Docs: man:cockpit-ws(8) Listen: [::]:9090 (Stream)
Aug 12 08:32:39 host.brq.redhat.com systemd[1]: Listening on Cockpit Web Service Socket.
● cockpit.service - Cockpit Web Service Loaded: loaded (/usr/lib/systemd/system/cockpit.service; static; vendor preset: disabled) Active: inactive (dead) Docs: man:cockpit-ws(8)
[root@host ~]# systemctl status dbus.socket dbus.service ● dbus.socket - D-Bus System Message Bus Socket Loaded: loaded (/usr/lib/systemd/system/dbus.socket; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Listen: /run/dbus/system_bus_socket (Stream)
Aug 02 19:12:01 host.brq.redhat.com systemd[1]: Listening on D-Bus System Message Bus Socket.
● dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; static; vendor preset: disabled) Active: active (running) since Tue 2016-08-02 19:12:01 CEST; 2 weeks 0 days ago Docs: man:dbus-daemon(1) Main PID: 1102 (dbus-daemon) Tasks: 2 (limit: 4915) CGroup: /system.slice/dbus.service └─1102 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
> > > %else > > # sysv > >diff --git a/src/sysv/systemd/sssd-secrets.service.in b/src/sysv/systemd/sssd-secrets.service.in > >new file mode 100644 > >index 0000000000000000000000000000000000000000..4236cc2eb85c83c574da8a96ca9d760922aacbd9 > >--- /dev/null > >+++ b/src/sysv/systemd/sssd-secrets.service.in > >@@ -0,0 +1,2 @@ > >+[Service] > >+ExecStart=@libexecdir@/sssd/sssd_secrets --uid 0 --gid 0 --debug-to-files > > I checked another socket activated service (cups, pcscd) > and they have "Also=$name.socket" in Install section for service > But on the other hand other socket activated services(cockpit, dbus, > systemd-journald) does not have it. So I'm not sure whether we need it.
Hmm, Also= seems to imply that if the service is enabled, the socket should be enabled as well, which IMO makes sense. So I will add it.
> > > Anyway, It would be good to add "[Unit]" section to both files + Description > and later also Documentation=man:$name($number)
Do we need Unit? I thought that if it's not set, then systemd would use the same name as the socket for the service. But I /can/ add it if you prefer.
It might work without it but it would be good to have human readable description and later also hint for man page.
e.g. [root@host ~]# systemctl cat cockpit.socket # /usr/lib/systemd/system/cockpit.socket [Unit] Description=Cockpit Web Service Socket Documentation=man:cockpit-ws(8)
[Socket] ListenStream=9090
[Install] WantedBy=sockets.target
[root@host ~]# systemctl cat cockpit.service # /usr/lib/systemd/system/cockpit.service [Unit] Description=Cockpit Web Service Documentation=man:cockpit-ws(8) Requires=cockpit.socket
[Service] ExecStartPre=/usr/sbin/remotectl certificate --ensure --user=root --group=cockpit-ws --selinux-type=etc_t ExecStart=/usr/libexec/cockpit-ws PermissionsStartOnly=true User=cockpit-ws Group=cockpit-ws
I'm still working on the manpage..
It was just a note that you should not forget :-)
See the new patch:
# systemctl disable sssd-secrets.service Removed symlink /etc/systemd/system/sockets.target.wants/sssd-secrets.socket. # systemctl enable sssd-secrets.service Created symlink from /etc/systemd/system/sockets.target.wants/sssd-secrets.socket to /usr/lib/systemd/system/sssd-secrets.socket.
I think that's the Also= directive at work
OK
The Description now shows with "systemctl show sssd-secrets.service" and .socket.
systemd_postun_with_restart is an alias for "systemctl try-restart" for package upgrade. So we should try to restart sssd-secrets.service and not just sssd-secrets.socket. Otherwise, old sssd-sevice will be running after upgrade.
OK, the attached patch also adds "%systemd_postun_with_restart sssd-secrets.service"
ACK++
* master: b3a22ee1d91aa4ed1544475be16ec2b7cf886180 942b4ce6e60e88e4e31600655fad8980f3986f68 733100a12138a701d0ae7ef5af2b04b08e225033
sssd-devel@lists.fedorahosted.org