[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#43106] [PATCH v3 2/2] services: childhurd: Support installing secre
From: |
Jan Nieuwenhuizen |
Subject: |
[bug#43106] [PATCH v3 2/2] services: childhurd: Support installing secrets from the host. |
Date: |
Tue, 01 Sep 2020 15:40:38 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Ludovic Courtès writes:
> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis:
>
>> * gnu/system/examples/bare-hurd.tmpl (%hurd-os)[services]: Add
>> secret-service.
>> * gnu/services/virtualization.scm (%hurd-vm-operating-system): Likewise.
>> (hurd-vm-shepherd-service): Use it to install secrets.
>> * doc/guix.texi (The Hurd in a Virtual Machine): Document it.
>
> Yay, minor issues, but overall LGTM!
\o/
>> (services (cons*
>> + ;; Receive secret keys on port 1004, TCP.
>> + (service secret-service-type 1004)
>
>
> [...]
>
>> + (start
>> + (with-imported-modules
>> + (source-module-closure '((gnu build secret-service)
>> + (guix build utils)))
>> + #~(let ((spawn (make-forkexec-constructor #$vm-command)))
>> + (lambda _
>> + (let ((pid (spawn))
>> + (port #$(hurd-vm-port config %hurd-vm-secrets-port))
>> + (root #$(hurd-vm-configuration-secret-root config)))
>> + (and root (directory-exists? root)
>> + (catch #t
>> + (lambda _
>> + (secret-service-send-secrets port root))
> In any case, we should assume that the VM is always running the secret
> service server, and thus call ‘secret-service-send-secrets’
> unconditionally (‘secret-service-send-secrets’ does (find-files root),
> which returns the empty list when ROOT doesn’t exist,
Yeah I was struggling a bit with this; the hurd-vm-service and the
childhurd must agree on the usage of secret-service. That's why I came
up with this root-dir #f switch...but it's certainly simpler if we say
that it must always be there. Let's see if we can get away with that!
So, I removed the root-dir checks and we always call
'secret-service-send-secrets', and changed the default from #f to
(secret-root hurd-vm-configuration-secret-root ;string
(default "/etc/childhurd")))
where "/etc/childhurd" does not need to exist.
> Perhaps ‘hurd-vm-service-type’ should unconditionally extend (via
> ‘service-extension’) ‘secret-service-type’, just to ensure that Hurd VMs
> always include the secret service.
Eh, hurd-vm-service lives in the host, the secret-services lives in the
client; am I missing something? ;-)
We could add a check for secret-service, possibly here
(define (hurd-vm-disk-image config)
"Return a disk-image for the Hurd according to CONFIG."
(let ((os (hurd-vm-configuration-os config))
(disk-size (hurd-vm-configuration-disk-size config)))
(system-image
(image
(inherit hurd-disk-image)
(size disk-size)
(operating-system os)))))
and/or insert if it it's missing...seems a bit over the top to me?
> I think.)
Yes, it does, but then the default cannot be #f, it must be a string.
I'm picking "/etc/childurd" as a default that need not exist.
>> + (lambda (keys . args)
>
> Should be “key” (singular).
Oops :-)
>> + (format (current-error-port)
>> + "failed to send secrets: ~a ~s\n" key args)
>> + (kill pid)
>
> (kill (- pid)) to kill the whole process group (just in case).
>
> I’d remove the ‘format’ call and just re-throw the exception: shepherd
> should report it correctly.
Done! Changed to unconditionally run
(catch #t
(lambda _
(secret-service-send-secrets port root))
(lambda (key . args)
(kill (- pid) SIGTERM)
(apply throw key args)))
pid)))))
>> + (service (@@ (gnu services virtualization)
>> + secret-service-type) 5999))
>
> This is useful for testing but I wouldn’t commit it (in particular
> because the example would no longer work for people who’re just spawning
> the VM and not trying to feed it secrets over TCP).
Right, removed.
> That’s it, thanks a lot!
You too!
Janneke
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com