Merge ~ahasenack/ubuntu/+source/python-certbot-nginx:focal-certbot-nginx-1875471 into ubuntu/+source/python-certbot-nginx:ubuntu/focal

[Andreas Hasenack]

## NOTE
This MP is against ubuntu/focal, not ubuntu/focal-devel, as focal had a python-certbot-nginx package in focal-proposed when the release was cut and groovy opened. The package in focal-proposed never made it to focal and became part of groovy instead.

The bug has details on what happened, and how it was fixed. The options we had were outlined in https://bugs.launchpad.net/ubuntu/+source/python-certbot-nginx/+bug/1875471/comments/12 and upstream suggested a fourth alternative in https://bugs.launchpad.net/ubuntu/+source/python-certbot-nginx/+bug/1875471/comments/15 which is what I adopted.

Test PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/certbot-tlssni01-1875471-d

Running all the tests properly needs a specific setup, which I suggest to leave for the SRU verification. I have done those with my test PPA already prior to submitting this.

What can easily be tested is the python error which originated this:

AttributeError: module 'acme.challenges' has no attribute 'TLSSNI01'

Just run this, on any host/container (no need to replace the fake domain):

sudo apt install python3-certbot-nginx
sudo certbot -d example.org --agree-tos --staging --register-unsafely-without-email --nginx

The fixed version won't fail with AttributeError, but will try to fetch a certificate for example.org, and that will of course fail and is fine.

The second check to make is to confirm that the build-time tests were run. Search build logs for "dh_auto_test".

[Christian Ehrhardt  (paelzer)]

Read all the bug updates - for how much was going on there the MP doesn't look so bad after all :-)

[Christian Ehrhardt  (paelzer)]

After wondering "hey this is a new version" at first this seems ok.
0.39 -> 0.40 is minimal and fixes the issues we see.

Also I agree not going the full >=1.3.x that is in groovy.

The backport of the test-fix LGTM as well.

The SRU Template is great and should resolve all confusion for anyone looking at the case.

Buildlog and changelog look good as well.
+1

review: Approve

[Andreas Hasenack (ahasenack)]

Thanks, tagging and uploading 723b05582944144e68428246b1d478afa80625cb

$ git push pkg upload/0.40.0-0ubuntu0.1
Enumerating objects: 45, done.
Counting objects: 100% (45/45), done.
Delta compression using up to 4 threads
Compressing objects: 100% (23/23), done.
Writing objects: 100% (29/29), 3.89 KiB | 442.00 KiB/s, done.
Total 29 (delta 20), reused 5 (delta 5)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/python-certbot-nginx
 * [new tag] upload/0.40.0-0ubuntu0.1 -> upload/0.40.0-0ubuntu0.1

$ dput ubuntu ../python-certbot-nginx_0.40.0-0ubuntu0.1_source.changes
Checking signature on .changes
gpg: ../python-certbot-nginx_0.40.0-0ubuntu0.1_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../python-certbot-nginx_0.40.0-0ubuntu0.1.dsc: Valid signature from AC983EB5BF6BCBA9
Package includes an .orig.tar.gz file although the debian revision suggests
that it might not be required. Multiple uploads of the .orig.tar.gz may be
rejected by the upload queue management software.
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading python-certbot-nginx_0.40.0-0ubuntu0.1.dsc: done.
  Uploading python-certbot-nginx_0.40.0.orig.tar.gz: done.
  Uploading python-certbot-nginx_0.40.0.orig.tar.gz.asc: done.
  Uploading python-certbot-nginx_0.40.0-0ubuntu0.1.debian.tar.xz: done.
  Uploading python-certbot-nginx_0.40.0-0ubuntu0.1_source.buildinfo: done.
  Uploading python-certbot-nginx_0.40.0-0ubuntu0.1_source.changes: done.
Successfully uploaded packages.

[Andreas Hasenack (ahasenack)]

This was released already.

diff --git a/PKG-INFO b/PKG-INFO
index e7aaec3..bbef3c2 100644
--- a/PKG-INFO
+++ b/PKG-INFO
@@ -1,6 +1,6 @@
 Metadata-Version: 2.1
 Name: certbot-nginx
-Version: 0.39.0
+Version: 0.40.0
 Summary: Nginx plugin for Certbot
 Home-page: https://github.com/letsencrypt/letsencrypt
 Author: Certbot Project
diff --git a/certbot_nginx.egg-info/PKG-INFO b/certbot_nginx.egg-info/PKG-INFO
index e7aaec3..bbef3c2 100644
--- a/certbot_nginx.egg-info/PKG-INFO
+++ b/certbot_nginx.egg-info/PKG-INFO
@@ -1,6 +1,6 @@
 Metadata-Version: 2.1
 Name: certbot-nginx
-Version: 0.39.0
+Version: 0.40.0
 Summary: Nginx plugin for Certbot
 Home-page: https://github.com/letsencrypt/letsencrypt
 Author: Certbot Project
diff --git a/certbot_nginx.egg-info/SOURCES.txt b/certbot_nginx.egg-info/SOURCES.txt
index 082c212..be61861 100644
--- a/certbot_nginx.egg-info/SOURCES.txt
+++ b/certbot_nginx.egg-info/SOURCES.txt
@@ -72,5 +72,4 @@ docs/_static/.gitignore
 docs/_templates/.gitignore
 docs/api/nginxparser.rst
 docs/api/obj.rst
-docs/api/parser.rst
-docs/api/tls_sni_01.rst
\ No newline at end of file
+docs/api/parser.rst
\ No newline at end of file
diff --git a/certbot_nginx/configurator.py b/certbot_nginx/configurator.py
index 9571591..fe5c7da 100644
--- a/certbot_nginx/configurator.py
+++ b/certbot_nginx/configurator.py
@@ -1107,7 +1107,7 @@ class NginxConfigurator(common.Installer):
     ###########################################################################
     def get_chall_pref(self, unused_domain):  # pylint: disable=no-self-use
         """Return list of challenge preferences."""
-        return [challenges.HTTP01, challenges.TLSSNI01]
+        return [challenges.HTTP01]
 
     # Entry point in main.py for performing challenges
     def perform(self, achalls):
diff --git a/certbot_nginx/http_01.py b/certbot_nginx/http_01.py
index 70147a4..4dcc532 100644
--- a/certbot_nginx/http_01.py
+++ b/certbot_nginx/http_01.py
@@ -29,10 +29,10 @@ class NginxHttp01(common.ChallengePerformer):
     :param list indices: Meant to hold indices of challenges in a
         larger array. NginxHttp01 is capable of solving many challenges
         at once which causes an indexing issue within NginxConfigurator
-        who must return all responses in order.  Imagine NginxConfigurator
-        maintaining state about where all of the http-01 Challenges,
-        TLS-SNI-01 Challenges belong in the response array.  This is an
-        optional utility.
+        who must return all responses in order. Imagine
+        NginxConfigurator maintaining state about where all of the
+        challenges, possibly of different types, belong in the response
+        array. This is an optional utility.
 
     """
 
diff --git a/certbot_nginx/tests/configurator_test.py b/certbot_nginx/tests/configurator_test.py
index 19624a7..237e22d 100644
--- a/certbot_nginx/tests/configurator_test.py
+++ b/certbot_nginx/tests/configurator_test.py
@@ -97,7 +97,7 @@ class NginxConfiguratorTest(util.NginxTest):
             errors.PluginError, self.config.enhance, 'myhost', 'unknown_enhancement')
 
     def test_get_chall_pref(self):
-        self.assertEqual([challenges.HTTP01, challenges.TLSSNI01],
+        self.assertEqual([challenges.HTTP01],
                          self.config.get_chall_pref('myhost'))
 
     def test_save(self):
diff --git a/certbot_nginx/tests/http_01_test.py b/certbot_nginx/tests/http_01_test.py
index 41c4b95..c6d35b8 100644
--- a/certbot_nginx/tests/http_01_test.py
+++ b/certbot_nginx/tests/http_01_test.py
@@ -73,11 +73,11 @@ class HttpPerformTest(util.NginxTest):
             self.http01.add_chall(achall)
             acme_responses.append(achall.response(self.account_key))
 
-        sni_responses = self.http01.perform()
+        http_responses = self.http01.perform()
 
-        self.assertEqual(len(sni_responses), 4)
+        self.assertEqual(len(http_responses), 4)
         for i in six.moves.range(4):
-            self.assertEqual(sni_responses[i], acme_responses[i])
+            self.assertEqual(http_responses[i], acme_responses[i])
 
     def test_mod_config(self):
         self.http01.add_chall(self.achalls[0])
diff --git a/debian/changelog b/debian/changelog
index cc87eaa..0b4f53c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+python-certbot-nginx (0.40.0-0ubuntu0.1) focal; urgency=medium
+
+  * Cope with newer python-acme that dropped TLSSNI01 (LP: #1875471):
+    - new upstream version: 0.40.0
+    - d/rules: actually run the tests by fixing the expression that looks
+      for nocheck in DEB_BUILD_OPTIONS
+    - d/p/fix-tests-with-newer-acme.patch: fix tests with newer python-acme
+      that has no TLSSNI01. Thanks to Brad Warren <[e-mail]>
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 05 May 2020 15:39:00 -0300
+
 python-certbot-nginx (0.39.0-1) unstable; urgency=medium
 
   * New upstream version 0.39.0
diff --git a/debian/patches/fix-tests-with-newer-acme.patch b/debian/patches/fix-tests-with-newer-acme.patch
new file mode 100644
index 0000000..2cdd71d
--- /dev/null
+++ b/debian/patches/fix-tests-with-newer-acme.patch
@@ -0,0 +1,45 @@
+Description: fix tests with newer python-acme that has no TLSSNI01
+ This extracts the minimal pieces from upstream's bigger refactoring
+ necessary to cope with python-acme's removal of TLSSNI01 in the version shipped
+ in Focal.
+Author: Brad Warren <[e-mail]>
+Origin: upstream, https://gist.github.com/bmw/e4f13e17d1f4647c9d6be730c7ec3512
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/python-certbot-nginx/+bug/1875471
+Applied-Upstream: https://github.com/certbot/certbot/commit/4abd81e2186eddc67551d61a8260440bd177d18d
+Last-Update: 2020-05-05
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+diff --git a/certbot-nginx/certbot_nginx/tests/http_01_test.py b/certbot-nginx/certbot_nginx/tests/http_01_test.py
+index d05370c68..8e0450f6a 100644
+--- a/certbot_nginx/tests/http_01_test.py
++++ b/certbot_nginx/tests/http_01_test.py
+@@ -1,6 +1,7 @@
+ """Tests for certbot_nginx.http_01"""
+ import unittest
+ 
++import josepy as jose
+ import mock
+ import six
+ 
+@@ -8,17 +9,19 @@ from acme import challenges
+ 
+ from certbot import achallenges
+ 
+-from certbot.plugins import common_test
+ from certbot.tests import acme_util
++from certbot.tests import util as test_util
+ 
+ from certbot_nginx.obj import Addr
+ from certbot_nginx.tests import util
+ 
++AUTH_KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem"))
++
+ 
+ class HttpPerformTest(util.NginxTest):
+     """Test the NginxHttp01 challenge."""
+ 
+-    account_key = common_test.AUTH_KEY
++    account_key = AUTH_KEY
+     achalls = [
+         achallenges.KeyAuthorizationAnnotatedChallenge(
+             challb=acme_util.chall_to_challb(
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..86c7057
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@
+fix-tests-with-newer-acme.patch
diff --git a/debian/rules b/debian/rules
index c057a16..154080a 100755
--- a/debian/rules
+++ b/debian/rules
@@ -21,6 +21,6 @@ override_dh_installdocs:
 	dh_installdocs -p python3-certbot-nginx
 
 override_dh_auto_test:
-ifdef (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))
+ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))
 	python3 setup.py test
 endif
diff --git a/docs/api/tls_sni_01.rst b/docs/api/tls_sni_01.rst
deleted file mode 100644
index 5074f63..0000000
--- a/docs/api/tls_sni_01.rst
+++ /dev/null
@@ -1,5 +0,0 @@
-:mod:`certbot_nginx.tls_sni_01`
------------------------------------
-
-.. automodule:: certbot_nginx.tls_sni_01
-   :members:
diff --git a/setup.py b/setup.py
index 9c69a53..b53e166 100644
--- a/setup.py
+++ b/setup.py
@@ -4,7 +4,7 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.39.0'
+version = '0.40.0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.