[PATCH 0/2] Support service extensions on the "final" service values

  • Open
  • quality assurance status badge
Details
2 participants
  • Alex Kost
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important

Debbugs page

L
L
Ludovic Courtès wrote on 30 May 2017 14:58
(address . guix-patches@gnu.org)
20170530215850.7522-1-ludo@gnu.org
Hello!

This patch adds support for service extensions that modify the
"final" values of a service. This is meant to implement cross-cutting
concerns as well as system-wide customization as discussed with Alex
long ago:


To summarize, a "finalization extension" (for lack of a better name)
gets the final value of a service and returns a new value for that
service. This is in contrast with a "normal" extension which can only
contribute to the value of a target service, and not inspect the value
of that target service.

For example, for the /etc service, a "normal" extension can only add
entries for /etc. A "finalization" extension can instead inspect and
change all the /etc entries. IOW, it is a sort of a "sudo" for service
extensions; it's also quite inelegant compared to the "normal" extension
mechanism, but it's certainly useful.

A use case is given in the second patch: we change all the PAM services
to use pam_elogind.so or pam_limits.so. Likewise, the 'rename-etc-files'
service below shows how to rename all the files in /etc (for illustration
purposes only :-)):

(define rename-etc-files
(let ((rename (lambda (prefix entries)
(map (match-lambda
((name . rest)
(cons (string-append prefix name)
rest)))
entries))))
(service-type
(name 'rename-etc-files)
(extensions (list (service-extension etc-service-type
(const '())
rename))))))


(operating-system
;; ...
(services (cons* (service rename-etc-files "foo-")
...)))

I think this should fulfill the need that Alex had expressed, which is
to not only be able to add files to /etc, but also to have the ability
to inspect and modify what goes to /etc.

The first patch currently lacks doc. I'll work on it if there's consensus
on the approach.

Feedback welcome!

Ludo'.

Ludovic Courtès (2):
DRAFT services: Extensions can specify a "finalization" procedure.
system: pam: Remove custom API to transform PAM services.

gnu/services.scm | 52 ++++++++++++++++++++++++++++++++++++++----------
gnu/services/base.scm | 33 ++++++++++++++++--------------
gnu/services/desktop.scm | 23 +++++++++++----------
gnu/system/pam.scm | 44 ++++++++--------------------------------
tests/services.scm | 34 +++++++++++++++++++++++++++++++
5 files changed, 114 insertions(+), 72 deletions(-)

--
2.13.0
L
L
Ludovic Courtès wrote on 30 May 2017 15:05
[PATCH 1/2] DRAFT services: Extensions can specify a "finalization" procedure.
(address . 27155@debbugs.gnu.org)
20170530220509.8254-1-ludo@gnu.org
TODO: Add doc

* gnu/services.scm (<service-extension>)[finalize]: New field.
Rename 'service-extension' to '%service-extension'.
(right-identity): New procedure.
(service-extension): New macro.
(fold-services)[apply-finalization, compose*]: New procedures.
Honor finalizations.
* tests/services.scm ("fold-services with finalizations"): New test.
---
gnu/services.scm | 52 ++++++++++++++++++++++++++++++++++++++++++----------
tests/services.scm | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 10 deletions(-)

Toggle diff (127 lines)
diff --git a/gnu/services.scm b/gnu/services.scm
index 5c314748d..4ebce753b 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -119,10 +119,24 @@
;;; Code:
(define-record-type <service-extension>
- (service-extension target compute)
+ (%service-extension target compute finalize)
service-extension?
- (target service-extension-target) ;<service-type>
- (compute service-extension-compute)) ;params -> params
+ (target service-extension-target) ;<service-type>
+ (compute service-extension-compute) ;value -> extension value
+ (finalize service-extension-finalize)) ;self other -> other
+
+(define (right-identity a b) b)
+
+(define-syntax service-extension
+ (syntax-rules ()
+ "Instantiate an extension of services of type TARGET. COMPUTE takes the
+value of the source service and returns the extension value of the target.
+Optionally, FINALIZE takes the value of the source service and the final value
+of the target, and returns a new value for the target."
+ ((_ target compute)
+ (%service-extension target compute right-identity))
+ ((_ target compute finalize)
+ (%service-extension target compute finalize))))
(define &no-default-value
;; Value used to denote service types that have no associated default value.
@@ -664,6 +678,21 @@ TARGET-TYPE; return the root service adjusted accordingly."
(($ <service-extension> _ compute)
(compute (service-value service))))))
+ (define (apply-finalization target)
+ (lambda (service)
+ (match (find (matching-extension target)
+ (service-type-extensions (service-kind service)))
+ (($ <service-extension> _ _ finalize)
+ (lambda (final)
+ (finalize (service-value service) final))))))
+
+ (define (compose* procs)
+ (match procs
+ (()
+ identity)
+ (_
+ (apply compose procs))))
+
(match (filter (lambda (service)
(eq? (service-kind service) target-type))
services)
@@ -671,15 +700,18 @@ TARGET-TYPE; return the root service adjusted accordingly."
(let loop ((sink sink))
(let* ((dependents (map loop (dependents sink)))
(extensions (map (apply-extension sink) dependents))
+ ;; We distinguish COMPOSE and EXTEND because PARAMS typically
+ ;; has a different type than the elements of EXTENSIONS.
(extend (service-type-extend (service-kind sink)))
(compose (service-type-compose (service-kind sink)))
- (params (service-value sink)))
- ;; We distinguish COMPOSE and EXTEND because PARAMS typically has a
- ;; different type than the elements of EXTENSIONS.
- (if extend
- (service (service-kind sink)
- (extend params (compose extensions)))
- sink))))
+ (value (if extend
+ (extend (service-value sink)
+ (compose extensions))
+ (service-value sink)))
+ (kind (service-kind sink))
+ (finalizations (map (apply-finalization sink)
+ dependents)))
+ (service kind ((compose* finalizations) value)))))
(()
(raise
(condition (&missing-target-service-error
diff --git a/tests/services.scm b/tests/services.scm
index 8484ee982..bb42e352a 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -88,6 +88,40 @@
(and (eq? (service-kind r) t1)
(service-value r))))
+(test-equal "fold-services with finalizations"
+ '(final 600 (initial-value 5 4 3 2 1 xyz 600))
+
+ ;; Similar to the one above, but this time with "finalization" extensions
+ ;; that modify the final result of compose/extend.
+ (let* ((t1 (service-type (name 't1) (extensions '())
+ (compose concatenate)
+ (extend cons)))
+ (t2 (service-type (name 't2)
+ (extensions
+ (list (service-extension t1
+ (cut list 'xyz <>)
+ (lambda (t2 t1)
+ `(final ,t2 ,t1)))))
+ (compose (cut reduce + 0 <>))
+ (extend *)))
+ (t3 (service-type (name 't3)
+ (extensions
+ (list (service-extension t2 identity)
+ (service-extension t1 list)))))
+ (t4 (service-type (name 't4)
+ (extensions
+ (list (service-extension t2 (const 0)
+ *)))))
+ (r (fold-services (cons* (service t1 'initial-value)
+ (service t2 4)
+ (service t4 10)
+ (map (lambda (x)
+ (service t3 x))
+ (iota 5 1)))
+ #:target-type t1)))
+ (and (eq? (service-kind r) t1)
+ (service-value r))))
+
(test-assert "fold-services, ambiguity"
(let* ((t1 (service-type (name 't1) (extensions '())
(compose concatenate)
--
2.13.0
L
L
Ludovic Courtès wrote on 30 May 2017 15:05
[PATCH 2/2] system: pam: Remove custom API to transform PAM services.
(address . 27155@debbugs.gnu.org)
20170530220509.8254-2-ludo@gnu.org
This specific way to extend 'pam-root-service-type' has been subsumed by
the "finalization extensions" of services.

* gnu/system/pam.scm (<pam-configuration>): Remove.
(/etc-entry): Adjust accordingly.
(extend-configuration): Remove.
(pam-root-service-type)[extend]: Set to 'append'.
(pam-root-service): Remove #:transform parameter. Adjust 'service'
form.
* gnu/services/desktop.scm (pam-extension-procedure): Rename to...
(elogind-pam-extension): ... this. Expect the complete list of
services and map over it.
(elogind-service-type): Change PAM-ROOT-SERVICE-TYPE extension to refer
to 'elogind-pam-extension'.
* gnu/services/base.scm (limits-pam-extension): New procedure.
(pam-limits-service-type): Remove 'pam-extension' procedure. Adjust
PAM-ROOT-SERVICE-TYPE extension accordingly.
---
gnu/services/base.scm | 33 ++++++++++++++++++---------------
gnu/services/desktop.scm | 23 ++++++++++++-----------
gnu/system/pam.scm | 44 ++++++++------------------------------------
3 files changed, 38 insertions(+), 62 deletions(-)

Toggle diff (179 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 7cd9a34ca..d36f5c410 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1239,6 +1239,21 @@ information on the configuration file syntax."
(service syslog-service-type config))
+(define (limits-pam-extension limits-file pam-services)
+ "Modify some of PAM-SERVICES to use 'pam_limits.so'."
+ (map (lambda (pam)
+ (let ((pam-limits (pam-entry
+ (control "required")
+ (module "pam_limits.so")
+ (arguments '("conf=/etc/security/limits.conf")))))
+ (if (member (pam-service-name pam) '("login" "su" "slim"))
+ (pam-service
+ (inherit pam)
+ (session (cons pam-limits
+ (pam-service-session pam))))
+ pam)))
+ pam-services))
+
(define pam-limits-service-type
(let ((security-limits
;; Create /etc/security containing the provided "limits.conf" file.
@@ -1250,26 +1265,14 @@ information on the configuration file syntax."
(mkdir #$output)
(stat #$limits-file)
(symlink #$limits-file
- (string-append #$output "/limits.conf"))))))))
- (pam-extension
- (lambda (pam)
- (let ((pam-limits (pam-entry
- (control "required")
- (module "pam_limits.so")
- (arguments '("conf=/etc/security/limits.conf")))))
- (if (member (pam-service-name pam)
- '("login" "su" "slim"))
- (pam-service
- (inherit pam)
- (session (cons pam-limits
- (pam-service-session pam))))
- pam)))))
+ (string-append #$output "/limits.conf")))))))))
(service-type
(name 'limits)
(extensions
(list (service-extension etc-service-type security-limits)
(service-extension pam-root-service-type
- (lambda _ (list pam-extension))))))))
+ (const '())
+ limits-pam-extension))))))
(define* (pam-limits-service #:optional (limits '()))
"Return a service that makes selected programs respect the list of
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 36049587d..6495bc94c 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2015 Andy Wingo <wingo@igalia.com>
;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2016 Sou Bunnbu <iyzsong@gmail.com>
@@ -637,21 +637,21 @@ include the @command{udisksctl} command, part of UDisks, and GNOME Disks."
"ELOGIND_CONF_FILE"
(elogind-configuration-file config))))
-(define (pam-extension-procedure config)
- "Return an extension for PAM-ROOT-SERVICE-TYPE that ensures that all the PAM
-services use 'pam_elogind.so', a module that allows elogind to keep track of
-logged-in users (run 'loginctl' to see elogind's world view of users and
-seats.)"
+(define (elogind-pam-extension config pam-services)
+ "Change PAM-SERVICES so that each of them uses 'pam_elogind.so', a module
+that allows elogind to keep track of logged-in users (run 'loginctl' to see
+elogind's world view of users and seats), and return that."
(define pam-elogind
(pam-entry
(control "required")
(module (file-append (elogind-package config)
"/lib/security/pam_elogind.so"))))
- (list (lambda (pam)
- (pam-service
- (inherit pam)
- (session (cons pam-elogind (pam-service-session pam)))))))
+ (map (lambda (pam)
+ (pam-service
+ (inherit pam)
+ (session (cons pam-elogind (pam-service-session pam)))))
+ pam-services))
(define elogind-service-type
(service-type (name 'elogind)
@@ -669,7 +669,8 @@ seats.)"
;; Extend PAM with pam_elogind.so.
(service-extension pam-root-service-type
- pam-extension-procedure)
+ (const '())
+ elogind-pam-extension)
;; We need /run/user, /run/systemd, etc.
(service-extension file-system-service-type
diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
index eedf93394..b1bfab7ba 100644
--- a/gnu/system/pam.scm
+++ b/gnu/system/pam.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -281,50 +281,22 @@ authenticate to run COMMAND."
;;; PAM root service.
;;;
-;; Overall PAM configuration: a list of services, plus a procedure that takes
-;; one <pam-service> and returns a <pam-service>. The procedure is used to
-;; implement cross-cutting concerns such as the use of the 'elogind.so'
-;; session module that keeps track of logged-in users.
-(define-record-type* <pam-configuration>
- pam-configuration make-pam-configuration? pam-configuration?
- (services pam-configuration-services) ;list of <pam-service>
- (transform pam-configuration-transform)) ;procedure
-
-(define (/etc-entry config)
+(define (/etc-entry services)
"Return the /etc/pam.d entry corresponding to CONFIG."
- (match config
- (($ <pam-configuration> services transform)
- (let ((services (map transform services)))
- `(("pam.d" ,(pam-services->directory services)))))))
-
-(define (extend-configuration initial extensions)
- "Extend INITIAL with NEW."
- (let-values (((services procs)
- (partition pam-service? extensions)))
- (pam-configuration
- (services (append (pam-configuration-services initial)
- services))
- (transform (apply compose
- (pam-configuration-transform initial)
- procs)))))
+ `(("pam.d" ,(pam-services->directory services))))
(define pam-root-service-type
(service-type (name 'pam)
(extensions (list (service-extension etc-service-type
/etc-entry)))
- ;; Arguments include <pam-service> as well as procedures.
+ ;; Arguments are <pam-service> objects.
(compose concatenate)
- (extend extend-configuration)))
+ (extend append)))
-(define* (pam-root-service base #:key (transform identity))
+(define* (pam-root-service base)
"The \"root\" PAM service, which collects <pam-service> instance and turns
-them into a /etc/pam.d directory, including the <pam-service> listed in BASE.
-TRANSFORM is a procedure that takes a <pam-service> and returns a
-<pam-service>. It can be used to implement cross-cutting concerns that affect
-all the PAM services."
- (service pam-root-service-type
- (pam-configuration (services base)
- (transform transform))))
+them into a /etc/pam.d directory, including the <pam-service> listed in BASE."
+ (service pam-root-service-type base))
--
2.13.0
L
L
Ludovic Courtès wrote on 31 May 2017 06:36
control message for bug #27155
(address . control@debbugs.gnu.org)
87o9u9du70.fsf@gnu.org
severity 27155 important
A
A
Alex Kost wrote on 1 Jun 2017 02:57
Re: bug#27155: [PATCH 0/2] Support service extensions on the "final" service values
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 27155@debbugs.gnu.org)
8760ggrpxm.fsf@gmail.com
Ludovic Courtès (2017-05-30 23:58 +0200) wrote:

Toggle quote (22 lines)
> Hello!
>
> This patch adds support for service extensions that modify the
> "final" values of a service. This is meant to implement cross-cutting
> concerns as well as system-wide customization as discussed with Alex
> long ago:
>
> https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00623.html
> https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html
>
> To summarize, a "finalization extension" (for lack of a better name)
> gets the final value of a service and returns a new value for that
> service. This is in contrast with a "normal" extension which can only
> contribute to the value of a target service, and not inspect the value
> of that target service.
>
> For example, for the /etc service, a "normal" extension can only add
> entries for /etc. A "finalization" extension can instead inspect and
> change all the /etc entries. IOW, it is a sort of a "sudo" for service
> extensions; it's also quite inelegant compared to the "normal" extension
> mechanism, but it's certainly useful.

Definitely!

Toggle quote (28 lines)
> A use case is given in the second patch: we change all the PAM services
> to use pam_elogind.so or pam_limits.so. Likewise, the 'rename-etc-files'
> service below shows how to rename all the files in /etc (for illustration
> purposes only :-)):
>
> (define rename-etc-files
> (let ((rename (lambda (prefix entries)
> (map (match-lambda
> ((name . rest)
> (cons (string-append prefix name)
> rest)))
> entries))))
> (service-type
> (name 'rename-etc-files)
> (extensions (list (service-extension etc-service-type
> (const '())
> rename))))))
>
>
> (operating-system
> ;; ...
> (services (cons* (service rename-etc-files "foo-")
> ...)))
>
> I think this should fulfill the need that Alex had expressed, which is
> to not only be able to add files to /etc, but also to have the ability
> to inspect and modify what goes to /etc.

This is great! Just what I wanted, and thanks for this example! Based
on it, I made the following service:

(define replace-etc/profile-type
(let ((replace
(lambda (file entries)
(cons `("profile" ,file)
(map (match-lambda
((name . rest)
(cons (if (string= name "profile")
(string-append "original-profile")
name)
rest)))
entries)))))
(service-type
(name 'replace-etc/profile)
(extensions (list (service-extension etc-service-type
(const '())
replace))))))

(service replace-etc/profile-type (local-file ".../my-system-profile"))

So now I can use my own "/etc/profile", moreover I can look at the
"/etc/original-profile" anytime. I already use a system with this
service and I enjoy it, thanks a lot!

Toggle quote (3 lines)
> The first patch currently lacks doc. I'll work on it if there's consensus
> on the approach.

I agree with this approach!

--
Alex
L
L
Ludovic Courtès wrote on 1 Jun 2017 04:24
(name . Alex Kost)(address . alezost@gmail.com)(address . 27155@debbugs.gnu.org)
871sr43q89.fsf@gnu.org
Hi Alex,

Alex Kost <alezost@gmail.com> skribis:

Toggle quote (26 lines)
> This is great! Just what I wanted, and thanks for this example! Based
> on it, I made the following service:
>
> (define replace-etc/profile-type
> (let ((replace
> (lambda (file entries)
> (cons `("profile" ,file)
> (map (match-lambda
> ((name . rest)
> (cons (if (string= name "profile")
> (string-append "original-profile")
> name)
> rest)))
> entries)))))
> (service-type
> (name 'replace-etc/profile)
> (extensions (list (service-extension etc-service-type
> (const '())
> replace))))))
>
> (service replace-etc/profile-type (local-file ".../my-system-profile"))
>
> So now I can use my own "/etc/profile", moreover I can look at the
> "/etc/original-profile" anytime. I already use a system with this
> service and I enjoy it, thanks a lot!

Awesome, I’m glad you like it! It was long overdue.

Thanks for taking the time to test!

Ludo’.
L
L
Ludovic Courtès wrote on 3 Jun 2017 14:21
(address . 27155@debbugs.gnu.org)(name . Alex Kost)(address . alezost@gmail.com)
8737bgkbsy.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (12 lines)
> This patch adds support for service extensions that modify the
> "final" values of a service. This is meant to implement cross-cutting
> concerns as well as system-wide customization as discussed with Alex
> long ago:
>
> https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00623.html
> https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html
>
> To summarize, a "finalization extension" (for lack of a better name)
> gets the final value of a service and returns a new value for that
> service.

I found a better name: “customizations”.

Toggle quote (6 lines)
> For example, for the /etc service, a "normal" extension can only add
> entries for /etc. A "finalization" extension can instead inspect and
> change all the /etc entries. IOW, it is a sort of a "sudo" for service
> extensions; it's also quite inelegant compared to the "normal" extension
> mechanism, but it's certainly useful.

Not liking the “sudo” aspect of this patch, I thought it would be
natural if service types could control how customizations apply. That
way, the PAM or /etc service could still guarantee, for instance, that
customization does not add or remove entries, and so on.

In the end, this control by the service type makes it easier to reason
about what extensions do, whereas the “sudo” style means that an
extension can alter the service’s value in any possible way.

So I started modifying this patch set to add a ‘customize’ field to
<service-type>, next to ‘extend’. For the PAM and /etc services,
‘customize’ would compose and apply procedures that modify an entry, for
instance.

Then I realized that the only difference between ‘customize’ and
‘extend’ would be the meaning attached to it. IOW, both are some kind
of an extension.

So at this point, I started wondering whether we should just allow
service types to declare several extension points. So for PAM, we’d do:

Toggle snippet (21 lines)
(define pam-service-addition
;; The extension point to add PAM services.
(service-extension-point
(compose concatenate)
(extend append)))

(define pam-service-cutomization
;; The extension point to customize PAM services.
(service-extension-point
(compose compose)
(extend append)))

(define pam-root-service-type
(service-type (name 'pam)
(extensions (list (service-extension etc-service-type
/etc-entry)))

(extension-points (list pam-service-addtion
pam-service-customization))))

But then ‘service-extension’ would need to specify not only the target
service type but also the target extension point, which means more
boilerplate, etc.

So after so much thought and hacking, I feel like the ad hoc solution at
was not that bad after all.

Sorry to bother you with philosophical design questions when we already
have two ways to solve the problem at hand, but I feel like there’s a
pattern worth looking for!

Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 27155@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 27155
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch