From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56391) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJFe2-0006P7-PK for guix-patches@gnu.org; Mon, 27 Nov 2017 04:24:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJFdy-0006sp-Po for guix-patches@gnu.org; Mon, 27 Nov 2017 04:24:06 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:51390) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eJFdy-0006sb-Lo for guix-patches@gnu.org; Mon, 27 Nov 2017 04:24:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eJFdx-0002Hc-QW for guix-patches@gnu.org; Mon, 27 Nov 2017 04:24:02 -0500 Subject: [bug#29467] [PATCH] web: Don't error about missing ssl related files. Resent-Message-ID: Received: from eggs.gnu.org ([2001:4830:134:3::10]:55873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJFcu-0006LL-CS for guix-patches@gnu.org; Mon, 27 Nov 2017 04:22:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJFcq-0006Br-6S for guix-patches@gnu.org; Mon, 27 Nov 2017 04:22:56 -0500 Received: from lepiller.eu ([2a00:5884:8208::1]:37424) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJFcp-0006AD-Sm for guix-patches@gnu.org; Mon, 27 Nov 2017 04:22:52 -0500 Received: from webmail.lepiller.eu (static-176-182-42-79.ncc.abo.bbox.fr [176.182.42.79]) by lepiller.eu (OpenSMTPD) with ESMTPSA id 05178843 (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128:NO) for ; Mon, 27 Nov 2017 09:25:16 +0000 (UTC) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Mon, 27 Nov 2017 10:22:48 +0100 From: julien lepiller In-Reply-To: <20171127082620.19237-1-mail@cbaines.net> References: <20171127082620.19237-1-mail@cbaines.net> Message-ID: <873b92b926e5037c904e1d0599ca6b63@lepiller.eu> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: 29467@debbugs.gnu.org Le 2017-11-27 09:26, Christopher Baines a écrit : > Erroring here prevents doing things like building a system using nginx > on a > different machine from where it's intended to be deployed, or creating > containers and VMs that use the ssl-certificate parts of the nginx > configuration, without also getting these files to exist. > > * gnu/services/web.scm (emit-nginx-server-config): Don't error on > missing ssl > related files. > --- > gnu/services/web.scm | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/gnu/services/web.scm b/gnu/services/web.scm > index 9d713003c..1af32278c 100644 > --- a/gnu/services/web.scm > +++ b/gnu/services/web.scm > @@ -191,16 +191,6 @@ of index files." > (syntax-parameterize ((<> (identifier-syntax x*))) > (list tail ...)) > '()))) > - (for-each > - (match-lambda > - ((record-key . file) > - (if (and file (not (file-exists? file))) > - (error > - (simple-format > - #f > - "~A in the nginx configuration for the server with name > \"~A\" does not exist" record-key server-name))))) > - `(("ssl-certificate" . ,ssl-certificate) > - ("ssl-certificate-key" . ,ssl-certificate-key))) > (list > " server {\n" > (and/l http-port " listen " (number->string <>) ";\n") Hi, when configuring nginx for the first time, users will probably forget to configure ssl properly. The default is to enable ssl and find certificates in /etc/nginx. When these files don't exist, nginx will fail to start and at least one user complained it was hard to debug. This code was introduced to prevent such a mistake. Maybe we should set the default to #f (but then users would have to configure more fields to enable https). Maybe we should add a configuration option like warn-only? (default to #f) to only warn about missing files. Or maybe there's a way to show nginx that another service is providing that file? I agree there is an issue, but your patch feels like a regression to me for the documented use-cases. WDYT?