GNU bug report logs - #70314
[PATCH] guix: scripts: environment: add tls certs to networked containers

Previous Next

Package: guix-patches;

Reported by: Richard Sent <richard <at> freakingpenguin.com>

Date: Tue, 9 Apr 2024 19:15:01 UTC

Severity: normal

Tags: patch

Merged with 75917

Full log


Message #17 received at 70314 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>, 70314 <at> debbugs.gnu.org
Subject: Re: [bug#70314] [PATCH] guix: scripts: environment: add tls certs
 to networked containers
Date: Sun, 15 Sep 2024 23:49:27 +0200
Hi,

Richard Sent <richard <at> freakingpenguin.com> skribis:

> * guix/scripts/environment.scm: Add --no-tls flag. By default when starting a
> container with -N, add nss-certs package and set SSL_CERT_DIR and
> SSL_CERT_FILE environment variables. When --no-tls is passed, default to old
> behavior.
> * doc/guix.texi: Document it.
>
> Change-Id: I3d222522fa9785fbf589f15efd14e6d6d072bfa7

[...]

> +  #:autoload   (gnu packages certs) (nss-certs)
>    #:autoload   (gnu packages bash) (bash)
>    #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
>    #:autoload   (gnu packages package-management) (guix)
> @@ -72,6 +73,9 @@ (define-module (guix scripts environment)
>  (define %default-shell
>    (or (getenv "SHELL") "/bin/sh"))
>  
> +(define %default-tls-certs
> +  (list nss-certs))

This would force all the package modules to be loaded upfront.  Instead
you should arrange to not refer to ‘nss-certs’ until it’s needed.

This matters for startup time.  To see how it affects the command, you
can run:

  strace -c guix shell coreutils -- true

The second run should make as few system calls as possible.

> +                 (lambda (opt name arg result)
> +                   (alist-cons 'no-tls? #t result)))

Internally, I would reverse the logic to have ‘tls?’ instead (as a rule
of thumb, I always avoid negating Booleans in code).

> +                  (('network? . #t)
> +                   (if (assoc-ref opts 'no-tls?)
> +                       '()
> +                       (manifest-entries
> +                        (packages->manifest %default-tls-certs))))

Can we delay changes to the manifest until after all options have been
parsed, so we know whether ‘-C’ has been passed?

That way ‘guix shell -N --no-tls’ does not add ‘nss-certs’ to the
environments.

>  (define* (launch-environment/container #:key command bash user user-mappings
>                                         profile manifest link-profile? network?
> -                                       map-cwd? emulate-fhs? nesting?
> +                                       no-tls? map-cwd? emulate-fhs? nesting?

Same as above: ‘tls?’ rather than ‘no-tls?’.

Please make sure tests/guix-shell*.sh and tests/guix-environment*.sh
pass.

Thanks,
Ludo’.




This bug report was last modified 80 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.