From 95557fa9b49fa2615774ed6ef734817dcd11b819 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 9 Jan 2019 12:37:45 -0800 Subject: [PATCH] Stop using staging in apacheconftests (#6647) Fixes #6585. I wrote up three suggestions for fixing this at https://github.com/certbot/certbot/issues/6585#issuecomment-448054502. I took the middle approach of requiring the user to provide an ACME server to use. I like this better than the other approaches which were: > Resolve #5938 instead of this issue. There is value in these tests as is over the compatibility tests in that they don't use Docker and run on different OSes. > Spin up a local Python server to return the directory object. Trying to set up a dummy ACME server seemed hacky and error prone. Other notes about this PR are: * I put the Pebble setup in `tox.ini` rather than `.travis.yml` as this seems much cleaner and more natural. * I created a new `tox` environment called `apacheconftest-with-pebble` that reuses the code from `testenv:apacheconftest` so `apacheconftest` can continue to be used with servers other than Pebble like is done in our test farm tests. * I chose the environment variable `SERVER` for consistency with our integration tests. I chose to not give this environment variable a default but to fail fast when it is not set. * I ran test farm tests on this PR and they passed. --- .travis.yml | 3 ++- .../tests/apache-conf-files/apache-conf-test | 10 +++++++++- tests/letstest/scripts/test_apache2.sh | 1 + tox.ini | 14 ++++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 89885d08e..a35d43b63 100644 --- a/.travis.yml +++ b/.travis.yml @@ -58,8 +58,9 @@ matrix: before_install: addons: - python: "2.7" - env: TOXENV=apacheconftest + env: TOXENV=apacheconftest-with-pebble sudo: required + services: docker - python: "2.7" env: TOXENV=nginxroundtrip diff --git a/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test b/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test index dcbba9d3e..4838a6eee 100755 --- a/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test +++ b/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test @@ -3,6 +3,11 @@ # A hackish script to see if the client is behaving as expected # with each of the "passing" conf files. +if [ -z "$SERVER" ]; then + echo "Please set SERVER to the ACME server's directory URL." + exit 1 +fi + export EA=/etc/apache2/ TESTDIR="`dirname $0`" cd $TESTDIR/passing @@ -56,13 +61,16 @@ if [ "$1" = --debian-modules ] ; then done fi +CERTBOT_CMD="sudo $(command -v certbot) --server $SERVER -vvvv" +CERTBOT_CMD="$CERTBOT_CMD --debug --apache --register-unsafely-without-email" +CERTBOT_CMD="$CERTBOT_CMD --agree-tos certonly -t --no-verify-ssl" FAILS=0 trap CleanupExit INT for f in *.conf ; do echo -n testing "$f"... Setup - RESULT=`echo c | sudo $(command -v certbot) -vvvv --debug --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` + RESULT=`echo c | $CERTBOT_CMD 2>&1` if echo $RESULT | grep -Eq \("Which names would you like"\|"mod_macro is not yet"\) ; then echo passed else diff --git a/tests/letstest/scripts/test_apache2.sh b/tests/letstest/scripts/test_apache2.sh index 4036e6efa..d24de2458 100755 --- a/tests/letstest/scripts/test_apache2.sh +++ b/tests/letstest/scripts/test_apache2.sh @@ -54,6 +54,7 @@ if [ $? -ne 0 ] ; then fi if [ "$OS_TYPE" = "ubuntu" ] ; then + export SERVER="$BOULDER_URL" venv/bin/tox -e apacheconftest else echo Not running hackish apache tests on $OS_TYPE diff --git a/tox.ini b/tox.ini index 71491c34a..95b2f2c64 100644 --- a/tox.ini +++ b/tox.ini @@ -152,6 +152,20 @@ commands = commands = {[base]pip_install} acme . certbot-apache certbot-compatibility-test {toxinidir}/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test --debian-modules +passenv = + SERVER + +[testenv:apacheconftest-with-pebble] +commands = + {toxinidir}/tests/pebble-fetch.sh + {[testenv:apacheconftest]commands} +passenv = + HOME + GOPATH + PEBBLEPATH + PEBBLE_STRICT +setenv = + SERVER=https://localhost:14000/dir [testenv:nginxroundtrip] commands =