From d3806a926cc740497b84c8cd2f6f3cdfe0975b6d Mon Sep 17 00:00:00 2001 From: Jeff Hodges Date: Fri, 6 Nov 2015 16:58:40 -0800 Subject: [PATCH 01/14] use boulder's integration-test.py This prevents the integration tests from getting run before the boulder processes have finished booting in most cases. There's still some small races with debug ports going up before RPC ports, but this flushes the big ones (specifically, the WFE ports), and the boulder devs going to fix the rest in integration-test.py over time. This also makes boulder-start.sh a blocking operation. Now the TravisCI integration tests no longer requires boulder-start.sh, we can let the other priority of being easier for users to control (that is, basically, make it easy to Ctrl-C) take over. That plus the idea that self-daemonizing code is tricky to get right, especially over multiple platforms led me to not trying to get start.py to make itself asynchronous. Most of this change is code movement in order to allow developers to run boulder-start.sh once and boulder-integration.sh many times while also not duplicating that code in order to run the tests in TravisCI. I'm not a huge fan of both the letsencrypt's shell scripts and boulder's integration-test.py having hard-coded file dependencies in the other's repo. This, however, seemed like the smallest path to code that would spuriously break less. All the designs I was able to come up that were maybe smaller changes either had the "starts tests before the servers are up" problem or with a "each repo uses another repo's test code file" problem. Those problem on top of the "it's a bigger change" problem led me here. --- .travis.yml | 3 +-- tests/boulder-fetch.sh | 39 ++++++++++++++++++++++++++++++++++++ tests/boulder-start.sh | 40 +++---------------------------------- tests/travis-integration.sh | 19 ++++++++++++++++++ 4 files changed, 62 insertions(+), 39 deletions(-) create mode 100755 tests/boulder-fetch.sh create mode 100755 tests/travis-integration.sh diff --git a/.travis.yml b/.travis.yml index 96e28b1b0..8dde06ceb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -60,8 +60,7 @@ addons: - rsyslog install: "travis_retry pip install tox coveralls" -before_script: '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/boulder-start.sh' -script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || (source .tox/$TOXENV/bin/activate && ./tests/boulder-integration.sh))' +script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' after_success: '[ "$TOXENV" == "cover" ] && coveralls' diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh new file mode 100755 index 000000000..a2c31b1d9 --- /dev/null +++ b/tests/boulder-fetch.sh @@ -0,0 +1,39 @@ +#!/bin/bash +# Download and run Boulder instance for integration testing + +# ugh, go version output is like: +# go version go1.4.2 linux/amd64 +GOVER=`go version | cut -d" " -f3 | cut -do -f2` + +# version comparison +function verlte { + #OS X doesn't support version sorting; emulate with sed + if [ `uname` == 'Darwin' ]; then + [ "$1" = "`echo -e \"$1\n$2\" | sed 's/\b\([0-9]\)\b/0\1/g' \ + | sort | sed 's/\b0\([0-9]\)/\1/g' | head -n1`" ] + else + [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ] + fi +} + +if ! verlte 1.5 "$GOVER" ; then + echo "We require go version 1.5 or later; you have... $GOVER" + exit 1 +fi + +set -xe + +# `/...` avoids `no buildable Go source files` errors, for more info +# see `go help packages` +go get -d github.com/letsencrypt/boulder/... +cd $GOPATH/src/github.com/letsencrypt/boulder +# goose is needed for ./test/create_db.sh +wget https://github.com/jsha/boulder-tools/raw/master/goose.gz && \ + mkdir $GOPATH/bin && \ + zcat goose.gz > $GOPATH/bin/goose && \ + chmod +x $GOPATH/bin/goose +./test/create_db.sh +# listenbuddy is needed for ./start.py +go get github.com/jsha/listenbuddy +cd - + diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index 47c1b6278..acf8f0bbf 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -1,43 +1,9 @@ #!/bin/bash -# Download and run Boulder instance for integration testing - - -# ugh, go version output is like: -# go version go1.4.2 linux/amd64 -GOVER=`go version | cut -d" " -f3 | cut -do -f2` - -# version comparison -function verlte { - #OS X doesn't support version sorting; emulate with sed - if [ `uname` == 'Darwin' ]; then - [ "$1" = "`echo -e \"$1\n$2\" | sed 's/\b\([0-9]\)\b/0\1/g' \ - | sort | sed 's/\b0\([0-9]\)/\1/g' | head -n1`" ] - else - [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ] - fi -} - -if ! verlte 1.5 "$GOVER" ; then - echo "We require go version 1.5 or later; you have... $GOVER" - exit 1 -fi - -set -xe export GOPATH="${GOPATH:-/tmp/go}" export PATH="$GOPATH/bin:$PATH" -# `/...` avoids `no buildable Go source files` errors, for more info -# see `go help packages` -go get -d github.com/letsencrypt/boulder/... +./tests/boulder-fetch.sh + cd $GOPATH/src/github.com/letsencrypt/boulder -# goose is needed for ./test/create_db.sh -wget https://github.com/jsha/boulder-tools/raw/master/goose.gz && \ - mkdir $GOPATH/bin && \ - zcat goose.gz > $GOPATH/bin/goose && \ - chmod +x $GOPATH/bin/goose -./test/create_db.sh -# listenbuddy is needed for ./start.py -go get github.com/jsha/listenbuddy -./start.py & -# Hopefully start.py bootstraps before integration test is started... +./start.py diff --git a/tests/travis-integration.sh b/tests/travis-integration.sh new file mode 100755 index 000000000..3b507bb86 --- /dev/null +++ b/tests/travis-integration.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +set -o errexit + +./tests/boulder-fetch.sh + +source .tox/$TOXENV/bin/activate + +export LETSENCRYPT_PATH=`pwd` + +cd $GOPATH/src/github.com/letsencrypt/boulder/ + +# boulder's integration-test.py has code that knows to start and wait for the +# boulder processes to start reliably and then will run the letsencrypt +# boulder-interation.sh on its own. The --letsencrypt flag says to run only the +# letsencrypt tests (instead of any other client tests it might run). We're +# going to want to define a more robust interaction point between the boulder +# and letsencrypt tests, but that will be better built off of this. +python test/integration-test.py --letsencrypt From 1014cf5d9e698eacfbc91907b0f11a11ce94620f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20L=C3=A9one?= Date: Wed, 11 Nov 2015 13:23:28 +0100 Subject: [PATCH 02/14] Dict can be litteral --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 ++------ letsencrypt-nginx/letsencrypt_nginx/parser.py | 7 +++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f10f0c241..c811501a9 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -187,12 +187,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # cert_key... can all be parsed appropriately self.prepare_server_https("443") - path = {} - - path["cert_path"] = self.parser.find_dir( - "SSLCertificateFile", None, vhost.path) - path["cert_key"] = self.parser.find_dir( - "SSLCertificateKeyFile", None, vhost.path) + path = {"cert_path": self.parser.find_dir("SSLCertificateFile", None, vhost.path), + "cert_key": self.parser.find_dir("SSLCertificateKeyFile", None, vhost.path)} # Only include if a certificate chain is specified if chain_path is not None: diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index fb79703dc..de7ab772c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -448,10 +448,9 @@ def _parse_server(server): :rtype: dict """ - parsed_server = {} - parsed_server['addrs'] = set() - parsed_server['ssl'] = False - parsed_server['names'] = set() + parsed_server = {'addrs': set(), + 'ssl': False, + 'names': set()} for directive in server: if directive[0] == 'listen': From b20acaa3e1ad9957abf7eae2f2557956a23c3e86 Mon Sep 17 00:00:00 2001 From: Matthew Ames Date: Tue, 17 Nov 2015 09:27:15 +0000 Subject: [PATCH 03/14] Update configuration.py --- letsencrypt/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 4955655f3..b370c741f 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -157,6 +157,6 @@ def _check_config_domain_sanity(domains): # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ # Characters used, domain parts < 63 chars, tld > 1 < 7 chars # first and last char is not "-" - fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Tue, 17 Nov 2015 15:45:52 +0000 Subject: [PATCH 04/14] Update configuration.py --- letsencrypt/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index b370c741f..7274a1aea 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -155,7 +155,7 @@ def _check_config_domain_sanity(domains): "Punycode domains are not supported") # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ - # Characters used, domain parts < 63 chars, tld > 1 < 7 chars + # Characters used, domain parts < 63 chars, tld > 1 < 13 chars # first and last char is not "-" fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Tue, 17 Nov 2015 23:41:26 +0000 Subject: [PATCH 05/14] Changed tld length to be anything over 1 character --- letsencrypt/configuration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 7274a1aea..f2d232c22 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -155,8 +155,8 @@ def _check_config_domain_sanity(domains): "Punycode domains are not supported") # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ - # Characters used, domain parts < 63 chars, tld > 1 < 13 chars + # Characters used, domain parts < 63 chars, tld > 1 char # first and last char is not "-" - fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Wed, 18 Nov 2015 07:18:20 +0000 Subject: [PATCH 06/14] TLDs cannot be longer than 63 characters --- letsencrypt/configuration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index f2d232c22..0ea539b5c 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -155,8 +155,8 @@ def _check_config_domain_sanity(domains): "Punycode domains are not supported") # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ - # Characters used, domain parts < 63 chars, tld > 1 char + # Characters used, domain parts < 63 chars, tld > 1 < 64 chars # first and last char is not "-" - fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Wed, 18 Nov 2015 14:26:01 -0800 Subject: [PATCH 07/14] Always use the specified GPG for signing everything. --- tools/dev-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/dev-release.sh b/tools/dev-release.sh index ffaf7881c..bd86bff44 100755 --- a/tools/dev-release.sh +++ b/tools/dev-release.sh @@ -49,7 +49,7 @@ done sed -i "s/^__version.*/__version__ = '$version'/" letsencrypt/__init__.py git add -p # interactive user input -git -c commit.gpgsign=true commit -m "Release $version" +git commit --gpg-sign="$RELEASE_GPG_KEY" -m "Release $version" git tag --local-user "$RELEASE_GPG_KEY" \ --sign --message "Release $version" "$tag" From 02562c75a3688c1cebdc0567bbc381440cc7f204 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 14:57:31 -0800 Subject: [PATCH 08/14] Remove references to --manual and --webroot --- docs/using.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index f6fb82f52..3f04fc5fa 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -184,7 +184,7 @@ Webroot If you're running a webserver that you don't want to stop to use standalone, you can use the webroot plugin to obtain a cert by -including ``certonly`` and ``--webroot`` on the command line. In +including ``certonly`` and ``-a webroot`` on the command line. In addition, you'll need to specify ``--webroot-path`` with the root directory of the files served by your webserver. For example, ``--webroot-path /var/www/html`` or @@ -200,7 +200,7 @@ If you'd like to obtain a cert running ``letsencrypt`` on a machine other than your target webserver or perform the steps for domain validation yourself, you can use the manual plugin. While hidden from the UI, you can use the plugin to obtain a cert by specifying -``certonly`` and ``--manual`` on the command line. This requires you +``certonly`` and ``-a manual`` on the command line. This requires you to copy and paste commands into another terminal session. Nginx From 2e06939fecfc8adca85b1ad28c27dd390e7e33b0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 21:15:54 -0800 Subject: [PATCH 09/14] Disable selection of misconfigured plugins --- letsencrypt/display/ops.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 37ce66b62..a240bf847 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -34,7 +34,14 @@ def choose_plugin(prepared, question): question, opts, help_label="More Info") if code == display_util.OK: - return prepared[index] + if plugin_ep.misconfigured: + util(interfaces.IDisplay).notification( + "The selected plugin encountered an error while parsing " + "your server configuration and cannot be used. The error " + "was: {0}".format(prepared[index].prepare()) + height=display_util.HEIGHT) + else: + return prepared[index] elif code == display_util.HELP: if prepared[index].misconfigured: msg = "Reported Error: %s" % prepared[index].prepare() From 279c0d9ddf49233d14f976852998cc07d8d562f4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 21:16:44 -0800 Subject: [PATCH 10/14] Comma --- letsencrypt/display/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index a240bf847..663db9307 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -38,7 +38,7 @@ def choose_plugin(prepared, question): util(interfaces.IDisplay).notification( "The selected plugin encountered an error while parsing " "your server configuration and cannot be used. The error " - "was: {0}".format(prepared[index].prepare()) + "was: {0}".format(prepared[index].prepare()), height=display_util.HEIGHT) else: return prepared[index] From 2bdc60dfef139dd9bd4e319bb61aea758ead7c0a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 21:21:42 -0800 Subject: [PATCH 11/14] Scoping rules are frustrating --- letsencrypt/display/ops.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 663db9307..5724cc542 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -34,14 +34,15 @@ def choose_plugin(prepared, question): question, opts, help_label="More Info") if code == display_util.OK: + plugin_ep = prepared[index] if plugin_ep.misconfigured: util(interfaces.IDisplay).notification( "The selected plugin encountered an error while parsing " "your server configuration and cannot be used. The error " - "was: {0}".format(prepared[index].prepare()), + "was: {0}".format(plugin_ep.prepare()), height=display_util.HEIGHT) else: - return prepared[index] + return plugin_ep elif code == display_util.HELP: if prepared[index].misconfigured: msg = "Reported Error: %s" % prepared[index].prepare() From 489e79d77763c07ac8ec0e0f19852f876e896594 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 22:13:04 -0800 Subject: [PATCH 12/14] spacing --- letsencrypt/display/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 5724cc542..224a701fb 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -39,7 +39,7 @@ def choose_plugin(prepared, question): util(interfaces.IDisplay).notification( "The selected plugin encountered an error while parsing " "your server configuration and cannot be used. The error " - "was: {0}".format(plugin_ep.prepare()), + "was:\n\n{0}".format(plugin_ep.prepare()), height=display_util.HEIGHT) else: return plugin_ep From 52361cc7305147ec624d3d225ace944c62ae981d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 22:18:48 -0800 Subject: [PATCH 13/14] Added tests --- letsencrypt/tests/display/ops_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 9d4a3a933..7427a1dc0 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -41,9 +41,11 @@ class ChoosePluginTest(unittest.TestCase): return choose_plugin(self.plugins, "Question?") @mock.patch("letsencrypt.display.ops.util") - def test_successful_choice(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 0) - self.assertEqual(self.mock_apache, self._call()) + def test_selection(self, mock_util): + mock_util().menu.side_effect = [(display_util.OK, 0), + (display_util.OK, 1)] + self.assertEqual(self.mock_stand, self._call()) + self.assertEqual(mock_util().notification.call_count, 1) @mock.patch("letsencrypt.display.ops.util") def test_more_info(self, mock_util): From 350a07086f653158fc9592de7df1b625bffc7ec8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2015 22:35:28 -0800 Subject: [PATCH 14/14] Remove confirmation in text display --- letsencrypt/display/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 224a701fb..dc5904cda 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -40,7 +40,7 @@ def choose_plugin(prepared, question): "The selected plugin encountered an error while parsing " "your server configuration and cannot be used. The error " "was:\n\n{0}".format(plugin_ep.prepare()), - height=display_util.HEIGHT) + height=display_util.HEIGHT, pause=False) else: return plugin_ep elif code == display_util.HELP: