From 4eb0b560c59cea45a88dc1af5e3a5952e85fc8d0 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Fri, 23 Oct 2020 06:12:54 +1100 Subject: [PATCH 01/12] manual: deprecate --manual-public-ip-logging-ok (#8381) * manual: deprecate --manual-public-ip-logging-ok * remove unused cli.report_config_interaction code Co-authored-by: ohemorange --- .../utils/certbot_call.py | 1 - certbot/CHANGELOG.md | 3 ++ certbot/certbot/_internal/cli/__init__.py | 1 - .../cli/report_config_interaction.py | 27 -------------- certbot/certbot/_internal/plugins/manual.py | 19 +--------- certbot/certbot/plugins/common.py | 4 -- certbot/tests/cli_test.py | 37 ------------------- certbot/tests/plugins/manual_test.py | 17 +-------- 8 files changed, 5 insertions(+), 104 deletions(-) delete mode 100644 certbot/certbot/_internal/cli/report_config_interaction.py diff --git a/certbot-ci/certbot_integration_tests/utils/certbot_call.py b/certbot-ci/certbot_integration_tests/utils/certbot_call.py index 2ddaa41c8..a71c610e5 100755 --- a/certbot-ci/certbot_integration_tests/utils/certbot_call.py +++ b/certbot-ci/certbot_integration_tests/utils/certbot_call.py @@ -92,7 +92,6 @@ def _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_por '--no-verify-ssl', '--http-01-port', str(http_01_port), '--https-port', str(tls_alpn_01_port), - '--manual-public-ip-logging-ok', '--config-dir', config_dir, '--work-dir', os.path.join(workspace, 'work'), '--logs-dir', os.path.join(workspace, 'logs'), diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index aff3a7cef..fc83d3b46 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -13,6 +13,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed * certbot-auto was deprecated on Debian based systems. +* CLI flag `--manual-public-ip-logging-ok` is now a no-op, generates a + deprecation warning, and will be removed in a future release. +* ### Fixed diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py index e4909840e..f6f648aa9 100644 --- a/certbot/certbot/_internal/cli/__init__.py +++ b/certbot/certbot/_internal/cli/__init__.py @@ -51,7 +51,6 @@ from certbot._internal.cli.cli_utils import ( ) # These imports depend on cli_constants and cli_utils. -from certbot._internal.cli.report_config_interaction import report_config_interaction from certbot._internal.cli.verb_help import VERB_HELP, VERB_HELP_MAP from certbot._internal.cli.group_adder import _add_all_groups from certbot._internal.cli.subparsers import _create_subparsers diff --git a/certbot/certbot/_internal/cli/report_config_interaction.py b/certbot/certbot/_internal/cli/report_config_interaction.py deleted file mode 100644 index 39266e776..000000000 --- a/certbot/certbot/_internal/cli/report_config_interaction.py +++ /dev/null @@ -1,27 +0,0 @@ -"""This is a module that reports config option interaction that should be -checked by set_by_cli""" -import six - -from certbot._internal.cli import VAR_MODIFIERS - - -def report_config_interaction(modified, modifiers): - """Registers config option interaction to be checked by set_by_cli. - - This function can be called by during the __init__ or - add_parser_arguments methods of plugins to register interactions - between config options. - - :param modified: config options that can be modified by modifiers - :type modified: iterable or str (string_types) - :param modifiers: config options that modify modified - :type modifiers: iterable or str (string_types) - - """ - if isinstance(modified, six.string_types): - modified = (modified,) - if isinstance(modifiers, six.string_types): - modifiers = (modifiers,) - - for var in modified: - VAR_MODIFIERS.setdefault(var, set()).update(modifiers) diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py index cc471230c..a2e4bb28e 100644 --- a/certbot/certbot/_internal/plugins/manual.py +++ b/certbot/certbot/_internal/plugins/manual.py @@ -84,8 +84,7 @@ permitted by DNS standards.) help='Path or command to execute for the authentication script') add('cleanup-hook', help='Path or command to execute for the cleanup script') - add('public-ip-logging-ok', action='store_true', - help='Automatically allows public IP logging (default: Ask)') + util.add_deprecated_argument(add, 'public-ip-logging-ok', 0) def prepare(self): # pylint: disable=missing-function-docstring if self.config.noninteractive_mode and not self.conf('auth-hook'): @@ -114,8 +113,6 @@ permitted by DNS standards.) return [challenges.HTTP01, challenges.DNS01] def perform(self, achalls): # pylint: disable=missing-function-docstring - self._verify_ip_logging_ok() - responses = [] for achall in achalls: if self.conf('auth-hook'): @@ -125,20 +122,6 @@ permitted by DNS standards.) responses.append(achall.response(achall.account_key)) return responses - def _verify_ip_logging_ok(self): - if not self.conf('public-ip-logging-ok'): - cli_flag = '--{0}'.format(self.option_name('public-ip-logging-ok')) - msg = ('NOTE: The IP of this machine will be publicly logged as ' - "having requested this certificate. If you're running " - 'certbot in manual mode on a machine that is not your ' - "server, please ensure you're okay with that.\n\n" - 'Are you OK with your IP being logged?') - display = zope.component.getUtility(interfaces.IDisplay) - if display.yesno(msg, cli_flag=cli_flag, force_interactive=True): - setattr(self.config, self.dest('public-ip-logging-ok'), True) - else: - raise errors.PluginError('Must agree to IP logging to proceed') - def _perform_achall_with_script(self, achall, achalls): env = dict(CERTBOT_DOMAIN=achall.domain, CERTBOT_VALIDATION=achall.validation(achall.account_key), diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py index 7d21e6d0a..5db75922b 100644 --- a/certbot/certbot/plugins/common.py +++ b/certbot/certbot/plugins/common.py @@ -55,10 +55,6 @@ class Plugin(object): def add_parser_arguments(cls, add): """Add plugin arguments to the CLI argument parser. - NOTE: If some of your flags interact with others, you can - use cli.report_config_interaction to register this to ensure - values are correctly saved/overridable during renewal. - :param callable add: Function that proxies calls to `argparse.ArgumentParser.add_argument` prepending options with unique plugin name prefix. diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 592c40be7..7475e99ea 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -505,43 +505,6 @@ class SetByCliTest(unittest.TestCase): verb = 'renew' self.assertTrue(_call_set_by_cli('webroot_map', args, verb)) - def test_report_config_interaction_str(self): - cli.report_config_interaction('manual_public_ip_logging_ok', - 'manual_auth_hook') - cli.report_config_interaction('manual_auth_hook', 'manual') - - self._test_report_config_interaction_common() - - def test_report_config_interaction_iterable(self): - cli.report_config_interaction(('manual_public_ip_logging_ok',), - ('manual_auth_hook',)) - cli.report_config_interaction(('manual_auth_hook',), ('manual',)) - - self._test_report_config_interaction_common() - - def _test_report_config_interaction_common(self): - """Tests implied interaction between manual flags. - - --manual implies --manual-auth-hook which implies - --manual-public-ip-logging-ok. These interactions don't actually - exist in the client, but are used here for testing purposes. - - """ - - args = ['--manual'] - verb = 'renew' - for v in ('manual', 'manual_auth_hook', 'manual_public_ip_logging_ok'): - self.assertTrue(_call_set_by_cli(v, args, verb)) - - # https://github.com/python/mypy/issues/2087 - cli.set_by_cli.detector = None # type: ignore - - args = ['--manual-auth-hook', 'command'] - for v in ('manual_auth_hook', 'manual_public_ip_logging_ok'): - self.assertTrue(_call_set_by_cli(v, args, verb)) - - self.assertFalse(_call_set_by_cli('manual', args, verb)) - def _call_set_by_cli(var, args, verb): with mock.patch('certbot._internal.cli.helpful_parser') as mock_parser: diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py index 933c759d6..7318783fd 100644 --- a/certbot/tests/plugins/manual_test.py +++ b/certbot/tests/plugins/manual_test.py @@ -32,8 +32,7 @@ class AuthenticatorTest(test_util.TempDirTestCase): # initialization. self.config = mock.MagicMock( http01_port=0, manual_auth_hook=None, manual_cleanup_hook=None, - manual_public_ip_logging_ok=False, noninteractive_mode=False, - validate_hooks=False, + noninteractive_mode=False, validate_hooks=False, config_dir=os.path.join(self.tempdir, "config_dir"), work_dir=os.path.join(self.tempdir, "work_dir"), backup_dir=os.path.join(self.tempdir, "backup_dir"), @@ -60,19 +59,7 @@ class AuthenticatorTest(test_util.TempDirTestCase): self.assertEqual(self.auth.get_chall_pref('example.org'), [challenges.HTTP01, challenges.DNS01]) - @test_util.patch_get_utility() - def test_ip_logging_not_ok(self, mock_get_utility): - mock_get_utility().yesno.return_value = False - self.assertRaises(errors.PluginError, self.auth.perform, []) - - @test_util.patch_get_utility() - def test_ip_logging_ok(self, mock_get_utility): - mock_get_utility().yesno.return_value = True - self.auth.perform([]) - self.assertTrue(self.config.manual_public_ip_logging_ok) - def test_script_perform(self): - self.config.manual_public_ip_logging_ok = True self.config.manual_auth_hook = ( '{0} -c "from __future__ import print_function;' 'from certbot.compat import os;' @@ -105,7 +92,6 @@ class AuthenticatorTest(test_util.TempDirTestCase): @test_util.patch_get_utility() def test_manual_perform(self, mock_get_utility): - self.config.manual_public_ip_logging_ok = True self.assertEqual( self.auth.perform(self.achalls), [achall.response(achall.account_key) for achall in self.achalls]) @@ -116,7 +102,6 @@ class AuthenticatorTest(test_util.TempDirTestCase): self.assertFalse(kwargs['wrap']) def test_cleanup(self): - self.config.manual_public_ip_logging_ok = True self.config.manual_auth_hook = ('{0} -c "import sys; sys.stdout.write(\'foo\')"' .format(sys.executable)) self.config.manual_cleanup_hook = '# cleanup' From c250957ab0c456a90be37db1a22b3d1512182b09 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 22 Oct 2020 14:01:30 -0700 Subject: [PATCH 02/12] Add .envrc. (#8382) --- .envrc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .envrc diff --git a/.envrc b/.envrc new file mode 100644 index 000000000..4d2077ebb --- /dev/null +++ b/.envrc @@ -0,0 +1,12 @@ +# This file is just a nicety for developers who use direnv. When you cd under +# the Certbot repo, Certbot's virtual environment will be automatically +# activated and then deactivated when you cd elsewhere. Developers have to have +# direnv set up and run `direnv allow` to allow this file to execute on their +# system. You can find more information at https://direnv.net/. +. venv3/bin/activate +# direnv doesn't support modifying PS1 so we unset it to squelch the error +# it'll otherwise print about this being done in the activate script. See +# https://github.com/direnv/direnv/wiki/PS1. If you would like your shell +# prompt to change like it normally does, see +# https://github.com/direnv/direnv/wiki/Python#restoring-the-ps1. +unset PS1 From b6e3a3ad020059265c780a6b0854c81927c43f9a Mon Sep 17 00:00:00 2001 From: alexzorin Date: Fri, 23 Oct 2020 11:33:45 +1100 Subject: [PATCH 03/12] register: remove report_new_account, use logger (#8393) --- certbot/certbot/_internal/account.py | 16 --------- certbot/certbot/_internal/client.py | 3 +- certbot/certbot/_internal/main.py | 7 ++-- certbot/tests/account_test.py | 18 ---------- certbot/tests/client_test.py | 53 ++++++++++++---------------- certbot/tests/main_test.py | 4 ++- 6 files changed, 30 insertions(+), 71 deletions(-) diff --git a/certbot/certbot/_internal/account.py b/certbot/certbot/_internal/account.py index c36559032..8cfe5ea11 100644 --- a/certbot/certbot/_internal/account.py +++ b/certbot/certbot/_internal/account.py @@ -11,7 +11,6 @@ import josepy as jose import pyrfc3339 import pytz import six -import zope.component from acme import fields as acme_fields from acme import messages @@ -94,21 +93,6 @@ class Account(object): self.meta == other.meta) -def report_new_account(config): - """Informs the user about their new ACME account.""" - reporter = zope.component.queryUtility(interfaces.IReporter) - if reporter is None: - return - reporter.add_message( - "Your account credentials have been saved in your Certbot " - "configuration directory at {0}. You should make a secure backup " - "of this folder now. This configuration directory will also " - "contain certificates and private keys obtained by Certbot " - "so making regular backups of this folder is ideal.".format( - config.config_dir), - reporter.MEDIUM_PRIORITY) - - class AccountMemoryStorage(interfaces.AccountStorage): """In-memory account storage.""" diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index ae57a6d91..b198a8c27 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -158,7 +158,7 @@ def register(config, account_storage, tos_cb=None): logger.warning(msg) raise errors.Error(msg) if not config.dry_run: - logger.info("Registering without email!") + logger.debug("Registering without email!") # If --dry-run is used, and there is no staging account, create one with no email. if config.dry_run: @@ -175,7 +175,6 @@ def register(config, account_storage, tos_cb=None): regr = perform_registration(acme, config, tos_cb) acc = account.Account(regr, key) - account.report_new_account(config) account_storage.save(acc, acme) eff.prepare_subscription(config, acc) diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 25e808571..07d36cd6c 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -490,11 +490,9 @@ def _determine_account(config): return True msg = ("Please read the Terms of Service at {0}. You " "must agree in order to register with the ACME " - "server at {1}".format( - terms_of_service, config.server)) + "server. Do you agree?".format(terms_of_service)) obj = zope.component.getUtility(interfaces.IDisplay) - result = obj.yesno(msg, "Agree", "Cancel", - cli_flag="--agree-tos", force_interactive=True) + result = obj.yesno(msg, cli_flag="--agree-tos", force_interactive=True) if not result: raise errors.Error( "Registration cannot proceed without accepting " @@ -518,6 +516,7 @@ def _determine_account(config): try: acc, acme = client.register( config, account_storage, tos_cb=_tos_cb) + logger.info("Account registered.") except errors.MissingCommandlineFlag: raise except errors.Error: diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index 6d215eed1..7158827dc 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -83,24 +83,6 @@ class MetaTest(unittest.TestCase): self.assertIsNotNone(meta.creation_host) self.assertIsNotNone(meta.register_to_eff) -class ReportNewAccountTest(test_util.ConfigTestCase): - """Tests for certbot._internal.account.report_new_account.""" - - def _call(self): - from certbot._internal.account import report_new_account - report_new_account(self.config) - - @mock.patch("certbot._internal.account.zope.component.queryUtility") - def test_no_reporter(self, mock_zope): - mock_zope.return_value = None - self._call() - - @mock.patch("certbot._internal.account.zope.component.queryUtility") - def test_it(self, mock_zope): - self._call() - call_list = mock_zope().add_message.call_args_list - self.assertTrue(self.config.config_dir in call_list[0][0][0]) - class AccountMemoryStorageTest(unittest.TestCase): """Tests for certbot._internal.account.AccountMemoryStorage.""" diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index bc4bced70..1b2a7413e 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -93,25 +93,22 @@ class RegisterTest(test_util.ConfigTestCase): mock_client.new_account_and_tos().terms_of_service = "http://tos" mock_client().external_account_required.side_effect = self._false_mock with mock.patch("certbot._internal.eff.prepare_subscription") as mock_prepare: - with mock.patch("certbot._internal.account.report_new_account"): - mock_client().new_account_and_tos.side_effect = errors.Error - self.assertRaises(errors.Error, self._call) - self.assertFalse(mock_prepare.called) + mock_client().new_account_and_tos.side_effect = errors.Error + self.assertRaises(errors.Error, self._call) + self.assertFalse(mock_prepare.called) - mock_client().new_account_and_tos.side_effect = None - self._call() - self.assertTrue(mock_prepare.called) + mock_client().new_account_and_tos.side_effect = None + self._call() + self.assertTrue(mock_prepare.called) def test_it(self): with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: mock_client().external_account_required.side_effect = self._false_mock - with mock.patch("certbot._internal.account.report_new_account"): - with mock.patch("certbot._internal.eff.handle_subscription"): - self._call() + with mock.patch("certbot._internal.eff.handle_subscription"): + self._call() - @mock.patch("certbot._internal.account.report_new_account") @mock.patch("certbot._internal.client.display_ops.get_email") - def test_email_retry(self, _rep, mock_get_email): + def test_email_retry(self, mock_get_email): from acme import messages self.config.noninteractive_mode = False msg = "DNS problem: NXDOMAIN looking up MX for example.com" @@ -124,8 +121,7 @@ class RegisterTest(test_util.ConfigTestCase): self.assertEqual(mock_get_email.call_count, 1) self.assertTrue(mock_prepare.called) - @mock.patch("certbot._internal.account.report_new_account") - def test_email_invalid_noninteractive(self, _rep): + def test_email_invalid_noninteractive(self): from acme import messages self.config.noninteractive_mode = True msg = "DNS problem: NXDOMAIN looking up MX for example.com" @@ -145,28 +141,25 @@ class RegisterTest(test_util.ConfigTestCase): with mock.patch("certbot._internal.eff.prepare_subscription") as mock_prepare: with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_clnt: mock_clnt().external_account_required.side_effect = self._false_mock - with mock.patch("certbot._internal.account.report_new_account"): - self.config.email = None - self.config.register_unsafely_without_email = True - self.config.dry_run = False - self._call() - mock_logger.info.assert_called_once_with(mock.ANY) - self.assertTrue(mock_prepare.called) + self.config.email = None + self.config.register_unsafely_without_email = True + self.config.dry_run = False + self._call() + mock_logger.debug.assert_called_once_with(mock.ANY) + self.assertTrue(mock_prepare.called) - @mock.patch("certbot._internal.account.report_new_account") @mock.patch("certbot._internal.client.display_ops.get_email") - def test_dry_run_no_staging_account(self, _rep, mock_get_email): + def test_dry_run_no_staging_account(self, mock_get_email): """Tests dry-run for no staging account, expect account created with no email""" with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: mock_client().external_account_required.side_effect = self._false_mock with mock.patch("certbot._internal.eff.handle_subscription"): - with mock.patch("certbot._internal.account.report_new_account"): - self.config.dry_run = True - self._call() - # check Certbot did not ask the user to provide an email - self.assertFalse(mock_get_email.called) - # check Certbot created an account with no email. Contact should return empty - self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact) + self.config.dry_run = True + self._call() + # check Certbot did not ask the user to provide an email + self.assertFalse(mock_get_email.called) + # check Certbot created an account with no email. Contact should return empty + self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact) def test_with_eab_arguments(self): with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client: diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index f8ed565bc..9727c95cc 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -481,7 +481,8 @@ class DetermineAccountTest(test_util.ConfigTestCase): self.assertTrue(self.config.email is None) @mock.patch('certbot._internal.client.display_ops.get_email') - def test_no_accounts_no_email(self, mock_get_email): + @mock.patch('certbot._internal.main.logger') + def test_no_accounts_no_email(self, mock_logger, mock_get_email): mock_get_email.return_value = 'foo@bar.baz' with mock.patch('certbot._internal.main.client') as client: @@ -493,6 +494,7 @@ class DetermineAccountTest(test_util.ConfigTestCase): self.assertEqual(self.accs[0].id, self.config.account) self.assertEqual('foo@bar.baz', self.config.email) + mock_logger.info.assert_called_once_with('Account registered.') def test_no_accounts_email(self): self.config.email = 'other email' From 00ed56afd6bd09d86e1ca3ea9fe589a2464bcc08 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 23 Oct 2020 20:02:35 +0200 Subject: [PATCH 04/12] Execute basic integration tests against Certbot dockers during CI (#8396) Fixes #8202 This PR adds an Azure Pipeline job to execute certbot plugins --prepare for each Docker image created during the CI on amd64. * Prepare basic integration tests for certbot dockers * Add a displayName for the integration tests task --- .../templates/jobs/packaging-jobs.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.azure-pipelines/templates/jobs/packaging-jobs.yml b/.azure-pipelines/templates/jobs/packaging-jobs.yml index 8da30b1f5..f4039987c 100644 --- a/.azure-pipelines/templates/jobs/packaging-jobs.yml +++ b/.azure-pipelines/templates/jobs/packaging-jobs.yml @@ -31,6 +31,25 @@ jobs: path: $(Build.ArtifactStagingDirectory) artifact: docker_$(DOCKER_ARCH) displayName: Store Docker artifact + - job: docker_run + dependsOn: docker_build + pool: + vmImage: ubuntu-18.04 + steps: + - task: DownloadPipelineArtifact@2 + inputs: + artifact: docker_amd64 + path: $(Build.SourcesDirectory) + displayName: Retrieve Docker images + - bash: set -e && docker load --input $(Build.SourcesDirectory)/images.tar + displayName: Load Docker images + - bash: | + set -ex + DOCKER_IMAGES=$(docker images --filter reference='*/certbot' --filter reference='*/dns-*' --format '{{.Repository}}:{{.Tag}}') + for DOCKER_IMAGE in ${DOCKER_IMAGES} + do docker run --rm "${DOCKER_IMAGE}" plugins --prepare + done + displayName: Run integration tests for Docker images - job: installer_build pool: vmImage: vs2017-win2016 From cfd0a6ff1fd9db9ab359a423d4529b76184c7b98 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 26 Oct 2020 23:20:27 +0100 Subject: [PATCH 05/12] Remove usage of buildkit (#8408) Fixes #8355 During the troubleshooting of #8355, I came to the conclusion that using buildkit was creating the problem. Without it all docker images are built correctly. Initially buildkit was enabled to avoid a building problem in Azure Pipeline, but I also found in my recent tests that this problem was not there anymore. You can find more details about the troubleshooting and reasoning in #8355. As a consequence, I disable the usage of buildkit in this PR which will solve the issue. --- tools/docker/build.sh | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tools/docker/build.sh b/tools/docker/build.sh index 9fcc4473c..e3ff5707a 100755 --- a/tools/docker/build.sh +++ b/tools/docker/build.sh @@ -12,19 +12,6 @@ IFS=$'\n\t' # given value is only the base of the tag because the things like the CPU # architecture are also added to the full tag. -# As of writing this, runs of this script consistently fail in Azure -# Pipelines, but they are fixed by using Docker BuildKit. A log of the failures -# that were occurring can be seen at -# https://gist.github.com/2227a05622299ce17bff9b0da714a1ff. Since using -# BuildKit is supposed to offer benefits anyway (see -# https://docs.docker.com/develop/develop-images/build_enhancements/ for more -# information), let's use it. -# -# This variable is set inside the script itself rather than in something like -# the CI config to have a consistent experience when this script is run -# locally. -export DOCKER_BUILDKIT=1 - WORK_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )" REPO_ROOT="$(dirname "$(dirname "${WORK_DIR}")")" source "$WORK_DIR/lib/common" From 4fa1df307573abf79324c1ece3b228b6dba31d74 Mon Sep 17 00:00:00 2001 From: Mark Dumay <61946753+markdumay@users.noreply.github.com> Date: Tue, 27 Oct 2020 01:22:00 +0100 Subject: [PATCH 06/12] Added links for gehirn and sakuracloud DNS plugins (#8406) --- certbot/docs/using.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 7389958aa..290ac817a 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -191,6 +191,7 @@ Once installed, you can find documentation on how to use each plugin at: * `certbot-dns-digitalocean `_ * `certbot-dns-dnsimple `_ * `certbot-dns-dnsmadeeasy `_ +* `certbot-dns-gehirn `_ * `certbot-dns-google `_ * `certbot-dns-linode `_ * `certbot-dns-luadns `_ @@ -198,6 +199,7 @@ Once installed, you can find documentation on how to use each plugin at: * `certbot-dns-ovh `_ * `certbot-dns-rfc2136 `_ * `certbot-dns-route53 `_ +* `certbot-dns-sakuracloud `_ Manual ------ From fc864543a732e0299964c21bbf89fe000a00bbbe Mon Sep 17 00:00:00 2001 From: ohemorange Date: Tue, 27 Oct 2020 10:22:40 -0700 Subject: [PATCH 07/12] Simplify/document snap creation (#8404) This PR adds the following documentation improvements to fix https://github.com/certbot/certbot/issues/7958: - Simplify building external plugins - Separate out certbot snap instructions from plugin instructions - Mention that dnsimple is just an example for the plugin instructions - Mention remote build for other architectures - Mention snap doc exists elsewhere in developer guide (`contributing.rst`) * Set up generate_dnsplugins_all.sh for all files and parametrize snapcraft and postrefreshhook files * Create constraints file in the generate_dnsplugins_all script * Separate out plugin and certbot snaps and update instructions * Add remote build instructions * Add pointers to the README to contributing.rst --- certbot/docs/contributing.rst | 12 +++ tools/snap/README.md | 97 +++++++++++++++---- tools/snap/generate_dnsplugins_all.sh | 15 +++ .../generate_dnsplugins_postrefreshhook.sh | 15 ++- tools/snap/generate_dnsplugins_snapcraft.sh | 19 ++-- 5 files changed, 119 insertions(+), 39 deletions(-) create mode 100755 tools/snap/generate_dnsplugins_all.sh diff --git a/certbot/docs/contributing.rst b/certbot/docs/contributing.rst index 95424630a..32a1ee72b 100644 --- a/certbot/docs/contributing.rst +++ b/certbot/docs/contributing.rst @@ -375,6 +375,9 @@ The script used to generate the snapcraft.yaml files for our own externally snapped plugins can be found at https://github.com/certbot/certbot/blob/master/tools/snap/generate_dnsplugins_snapcraft.sh. +For more information on building externally snapped plugins, see the section on +:ref:`Building snaps`. + Once you have created your own snap, if you have the snap file locally, it can be installed for use with Certbot by running: @@ -534,6 +537,15 @@ Use of EFFOSCCP is subject to the `EFF Code of Conduct `_. When investigating an alleged Code of Conduct violation, EFF may review discussion channels or direct messages. +.. _Building snaps: + +Building the Certbot and DNS plugin snaps +========================================= + +Instructions for how to manually build and run the Certbot snap and the externally +snapped DNS plugins that the Certbot project supplies are located in the README +file at https://github.com/certbot/certbot/tree/master/tools/snap. + Updating certbot-auto and letsencrypt-auto ========================================== diff --git a/tools/snap/README.md b/tools/snap/README.md index 71babf1a7..308c16753 100644 --- a/tools/snap/README.md +++ b/tools/snap/README.md @@ -1,7 +1,10 @@ -# Certbot Snaps +# Building Certbot Snaps ## Local Testing and Development +These instructions are recommended when testing anything about the snap setup for ease of debugging. +The architecture of the built snap is limited to the architecture of the system it is built on. + ### Initial VM Set Up These steps need to be done once to set up your VM and do not need to be run again to rebuild the snap. @@ -15,31 +18,83 @@ These steps need to be done once to set up your VM and do not need to be run aga 6. Install snapcraft with `sudo snap install --classic snapcraft`. 7. `cd ~` (or any other directory where you want our source files to be) 8. Run `git clone git://github.com/certbot/certbot` - 9. `cd certbot` + 9. `cd certbot` (All further instructions are relative to this directory.) -### Build the Snaps +### Certbot Snap -These are the steps to build and install the snaps. If you have run these steps before, you may want to run the commands in the section below to clean things up before building the snap again. +#### Reset the Environment + +If the snap has been built before, the instructions below clean up the build environment so it can reliably be used again. + + 1. `snapcraft clean --use-lxd` + 2. [Optional] `mv certbot_*_amd64.snap certbot_amd64.snap.bak` + +#### Build the Certbot Snap + +These are the steps to build and install the Certbot snap. If you have run these steps before, you may want to run the commands in the section above to clean things up or save a previous build before building the snap again (running `snapcraft` again will overwrite the previous snap). 1. Run `snapcraft --use-lxd`. 2. Install the generated snap with `sudo snap install --dangerous --classic certbot_*_amd64.snap`. You can transfer the snap to a different machine to run it there instead if you prefer. - 3. Run `tools/merge_requirements.py tools/dev_constraints.txt <(tools/strip_hashes.py letsencrypt-auto-source/pieces/dependency-requirements.txt) > certbot-dns-dnsimple/snap-constraints.txt` (this is a workaround for https://github.com/certbot/certbot/issues/8100). - 4. `cd certbot-dns-dnsimple` - 5. `snapcraft --use-lxd` - 6. Run `sudo snap set certbot trust-plugin-with-root=ok`. - 7. Install the generated snap with `sudo snap install --dangerous certbot-dns-dnsimple_*_amd64.snap`. Again, you can transfer the snap to a different machine to run it there instead if you prefer. - 8. Connect the plugin with `sudo snap connect certbot:plugin certbot-dns-dnsimple`. - 9. Connect the plugin metadata with `sudo snap connect certbot-dns-dnsimple:certbot-metadata certbot:certbot-metadata`. Install the plugin again to test refresh; logs are at `/var/snap/certbot-dns-dnsimple/current/debuglog`. - 10. Now you can run Certbot as normal. For example, `certbot plugins` should display the DNSimple plugin as installed. -### Reset the Environment +#### Run -The instructions below clean up the build environment so it can reliably be used again. +Run Certbot as normal. For example, `certbot plugins` should display the Apache and Nginx plugins. -1. `cd ~/certbot` (or to an alternate path where you put our source files) -2. `snapcraft clean --use-lxd` -3. `rm certbot_*_amd64.snap` -4. `cd certbot-dns-dnsimple` -5. `rm certbot-dns-dnsimple_*_amd64.snap` -6. `snapcraft clean --use-lxd` -7. `cd ..` +### Certbot Plugin Snaps + +These instructions use the `certbot-dns-dnsimple` plugin as an example, but all of Certbot's other plugin snaps can be built in the same way. + +#### Reset the Environment + +If the plugin snap has been built before, the instructions below clean up the build environment so it can reliably be used again. + + 1. `cd certbot-dns-dnsimple` + 2. `snapcraft clean --use-lxd` + 3. [Optional] `mv certbot-dns-dnsimple_*_amd64.snap certbot-dns-simple_amd64.snap.bak` + 4. `cd ..` + +#### Build a Certbot Plugin Snap + +These are the steps to build and install the Certbot DNSimple plugin snap. If you have run these steps before, you may want to run the commands in the section above to clean things up or save a previous build before building the snap again (running `snapcraft` again will overwrite the previous snap). + + 1. Run `tools/snap/generate_dnsplugins_all.sh` to generate all necessary files for all plugin snaps. + 2. `cd certbot-dns-dnsimple` + 3. `snapcraft --use-lxd` + 4. Run `sudo snap set certbot trust-plugin-with-root=ok`. + 5. Install the generated snap with `sudo snap install --dangerous certbot-dns-dnsimple_*_amd64.snap`. Again, you can transfer the snap to a different machine to run it there instead if you prefer. + 6. Connect the plugin with `sudo snap connect certbot:plugin certbot-dns-dnsimple`. + 7. Connect the plugin metadata with `sudo snap connect certbot-dns-dnsimple:certbot-metadata certbot:certbot-metadata`. Install the plugin again to test refresh; logs are at `/var/snap/certbot-dns-dnsimple/current/debuglog`. + +#### Run + +Run Certbot as normal. For example, `certbot plugins` should display the DNSimple plugin as installed. + +## Building for Other Architectures + +To build for an unavailable architecture or for multiple architectures simultaneously, we recommend using snapcraft's remote build feature. +It is easiest to run this from a local machine. + +### Initial Local Setup + + 1. Create or log into an Ubuntu One account [here](https://login.launchpad.net/). + 2. Install git and python with `sudo apt update && sudo apt install -y git python`. + 3. Install snapcraft with `sudo snap install --classic snapcraft`. + 4. `cd ~` (or any other directory where you want our source files to be) + 5. Run `git clone git://github.com/certbot/certbot` + 6. `cd certbot` (All further instructions are relative to this directory.) + 7. To trigger `snapcraft` to request access to your Launchpad account, run + `snapcraft remote-build --launchpad-accept-public-upload --status`. A URL where you need + to grant this access will be printed to your terminal and automatically open in your browser + if one is available. + +### Build Snaps Remotely + +Certbot provides a wrapper around snapcraft's remote build to make building all of our plugins easier. To see all available +options, run `python3 tools/snap/build_remote.py --help`. + +For example, to build all available snaps for all architectures, run `python3 tools/snap/build_remote.py ALL --archs amd64 arm64 armhf`. + +To build only the certbot snap on only amd64, run `python3 tools/snap/build_remote.py certbot --archs armhf`. + +The command will upload the entire contents of the working directory, so if the remote build +appears to hang, try using a clean clone of the `certbot` repository. diff --git a/tools/snap/generate_dnsplugins_all.sh b/tools/snap/generate_dnsplugins_all.sh new file mode 100755 index 000000000..6c41a19cd --- /dev/null +++ b/tools/snap/generate_dnsplugins_all.sh @@ -0,0 +1,15 @@ +#!/bin/bash +# Generate all necessary files for building snaps for all DNS plugins +set -eu + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +CERTBOT_DIR="$(dirname "$(dirname "${DIR}")")" + +for PLUGIN_PATH in "${CERTBOT_DIR}"/certbot-dns-*; do + bash "${CERTBOT_DIR}"/tools/snap/generate_dnsplugins_snapcraft.sh $PLUGIN_PATH + bash "${CERTBOT_DIR}"/tools/snap/generate_dnsplugins_postrefreshhook.sh $PLUGIN_PATH + # Create constraints file + "${CERTBOT_DIR}"/tools/merge_requirements.py tools/dev_constraints.txt \ + <("${CERTBOT_DIR}"/tools/strip_hashes.py letsencrypt-auto-source/pieces/dependency-requirements.txt) \ + > "${PLUGIN_PATH}"/snap-constraints.txt +done diff --git a/tools/snap/generate_dnsplugins_postrefreshhook.sh b/tools/snap/generate_dnsplugins_postrefreshhook.sh index c73a4afa9..c7d58e66e 100755 --- a/tools/snap/generate_dnsplugins_postrefreshhook.sh +++ b/tools/snap/generate_dnsplugins_postrefreshhook.sh @@ -1,13 +1,13 @@ #!/bin/bash -# Generate the hooks/post-refresh file for all DNS plugins +# Generate the hooks/post-refresh file for a DNS plugin +# Usage: bash generate_dnsplugins_postrefreshhook.sh path/to/dns/plugin +# For example, from the certbot home directory: +# tools/snap/generate_dnsplugins_postrefreshhook.sh certbot-dns-dnsimple set -eu -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -CERTBOT_DIR="$(dirname "$(dirname "${DIR}")")" - -for PLUGIN_PATH in "${CERTBOT_DIR}"/certbot-dns-*; do - mkdir -p "${PLUGIN_PATH}/snap/hooks" - cat < "${PLUGIN_PATH}/snap/hooks/post-refresh" +PLUGIN_PATH=$1 +mkdir -p "${PLUGIN_PATH}/snap/hooks" +cat < "${PLUGIN_PATH}/snap/hooks/post-refresh" #!/bin/sh -e # This file is generated by tools/generate_dnsplugins_postrefreshhook.sh and should not be edited manually. @@ -31,4 +31,3 @@ if [ "\$exit_code" -eq 1 ]; then exit 1 fi EOF -done diff --git a/tools/snap/generate_dnsplugins_snapcraft.sh b/tools/snap/generate_dnsplugins_snapcraft.sh index 7e8c256f0..c77613e33 100755 --- a/tools/snap/generate_dnsplugins_snapcraft.sh +++ b/tools/snap/generate_dnsplugins_snapcraft.sh @@ -1,15 +1,15 @@ #!/bin/bash -# Generate the snapcraft.yaml file for all DNS plugins +# Generate the snapcraft.yaml file for a DNS plugins +# Usage: bash generate_dnsplugins_snapcraft.sh path/to/dns/plugin +# For example, from the certbot home directory: +# tools/snap/generate_dnsplugins_snapcraft.sh certbot-dns-dnsimple set -e -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -CERTBOT_DIR="$(dirname "$(dirname "${DIR}")")" - -for PLUGIN_PATH in "${CERTBOT_DIR}"/certbot-dns-*; do - PLUGIN=$(basename "${PLUGIN_PATH}") - DESCRIPTION=$(grep description "${PLUGIN_PATH}/setup.py" | sed -E 's|\s+description="(.*)",|\1|g') - mkdir -p "${PLUGIN_PATH}/snap" - cat < "${PLUGIN_PATH}/snap/snapcraft.yaml" +PLUGIN_PATH=$1 +PLUGIN=$(basename "${PLUGIN_PATH}") +DESCRIPTION=$(grep description "${PLUGIN_PATH}/setup.py" | sed -E 's|\s+description="(.*)",|\1|g') +mkdir -p "${PLUGIN_PATH}/snap" +cat < "${PLUGIN_PATH}/snap/snapcraft.yaml" # This file is generated by tools/generate_dnsplugins_snapcraft.sh and should not be edited manually. name: ${PLUGIN} summary: ${DESCRIPTION} @@ -52,4 +52,3 @@ plugs: content: metadata-1 target: \$SNAP/certbot-shared EOF -done From bf07ec20b0096912fb02759f25923b9091643b63 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Wed, 28 Oct 2020 06:48:07 +1100 Subject: [PATCH 08/12] run: dont report new certs when only re-installing (#8392) --- certbot/certbot/_internal/main.py | 4 +++- certbot/tests/main_test.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 07d36cd6c..6c1482b65 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -1109,7 +1109,9 @@ def run(config, plugins): cert_path = new_lineage.cert_path if new_lineage else None fullchain_path = new_lineage.fullchain_path if new_lineage else None key_path = new_lineage.key_path if new_lineage else None - _report_new_cert(config, cert_path, fullchain_path, key_path) + + if should_get_cert: + _report_new_cert(config, cert_path, fullchain_path, key_path) _install_cert(config, le_client, domains, new_lineage) diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 9727c95cc..691ced439 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1281,13 +1281,16 @@ class MainTest(test_util.ConfigTestCase): @test_util.patch_get_utility() @mock.patch('certbot._internal.main._find_lineage_for_domains_and_certname') @mock.patch('certbot._internal.main._init_le_client') - def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility): + @mock.patch('certbot._internal.main._report_new_cert') + def test_certonly_reinstall(self, mock_report_new_cert, mock_init, + mock_renewal, mock_get_utility): mock_renewal.return_value = ('reinstall', mock.MagicMock()) mock_init.return_value = mock_client = mock.MagicMock() self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) self.assertFalse(mock_client.obtain_certificate.called) self.assertFalse(mock_client.obtain_and_enroll_certificate.called) self.assertEqual(mock_get_utility().add_message.call_count, 0) + mock_report_new_cert.assert_not_called() #self.assertTrue('donate' not in mock_get_utility().add_message.call_args[0][0]) def _test_certonly_csr_common(self, extra_args=None): From 4c347f5576f264ddfaec9cdaea1b831841fa9d24 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 28 Oct 2020 14:12:32 -0700 Subject: [PATCH 09/12] Switch to using python directly (#8413) Windows installer tests failed last night because they suddenly switched to Python 3.9. This is happening despite https://github.com/certbot/certbot/blob/bf07ec20b0096912fb02759f25923b9091643b63/.azure-pipelines/templates/jobs/packaging-jobs.yml#L92-L95 just a few lines earlier than what I modified in the PR here. I think what's going on is `py -3` is finding and preferring the newer version of Python 3, Python 3.9, which was [just recently added to the image](https://github.com/actions/virtual-environments/issues/1740#issuecomment-717849233). The [documentation for UsePythonVersion](https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/tool/use-python-version?view=azure-devops) says that: > After running this task with "Add to PATH," the `python` command in subsequent scripts will be for the highest available version of the interpreter matching the version spec and architecture. So let's just use `python` instead of `py`. --- .azure-pipelines/templates/jobs/packaging-jobs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.azure-pipelines/templates/jobs/packaging-jobs.yml b/.azure-pipelines/templates/jobs/packaging-jobs.yml index f4039987c..7862116e8 100644 --- a/.azure-pipelines/templates/jobs/packaging-jobs.yml +++ b/.azure-pipelines/templates/jobs/packaging-jobs.yml @@ -103,7 +103,7 @@ jobs: # a recent version of pip, but we also to disable the isolated feature as described in # https://github.com/certbot/certbot/issues/8256 - script: | - py -3 -m venv venv + python -m venv venv venv\Scripts\python -m pip install pip==20.2.3 setuptools==50.3.0 wheel==0.35.1 venv\Scripts\python tools\pip_install.py -e certbot-ci env: From bb45c9aa418ad7a656f9ab3df60b1d14a4389712 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 28 Oct 2020 15:08:16 -0700 Subject: [PATCH 10/12] Add Ubuntu 20.10 test farm tests (#8414) Fixes https://github.com/certbot/certbot/issues/8400. I had to switch the package installed in `apacheconftest` to `libapache2-mod-wsgi-py3` because Ubuntu 20.10 removed the Python 2 version of this module. I didn't add this AMI to `tests/letstest/auto_targets.yaml` because like Ubuntu 20.04, `certbot-auto` has never worked on the OS. * Add Ubuntu 20.20 test farm tests * Try Python 3 WSGI --- certbot-apache/tests/apache-conf-files/apache-conf-test | 2 +- tests/letstest/apache2_targets.yaml | 5 +++++ tests/letstest/targets.yaml | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/certbot-apache/tests/apache-conf-files/apache-conf-test b/certbot-apache/tests/apache-conf-files/apache-conf-test index 4838a6eee..66955819e 100755 --- a/certbot-apache/tests/apache-conf-files/apache-conf-test +++ b/certbot-apache/tests/apache-conf-files/apache-conf-test @@ -52,7 +52,7 @@ function Cleanup() { # if our environment asks us to enable modules, do our best! if [ "$1" = --debian-modules ] ; then sudo apt-get install -y apache2 - sudo apt-get install -y libapache2-mod-wsgi + sudo apt-get install -y libapache2-mod-wsgi-py3 sudo apt-get install -y libapache2-mod-macro for mod in ssl rewrite macro wsgi deflate userdir version mime setenvif ; do diff --git a/tests/letstest/apache2_targets.yaml b/tests/letstest/apache2_targets.yaml index b91fcf0b9..b45cec2c5 100644 --- a/tests/letstest/apache2_targets.yaml +++ b/tests/letstest/apache2_targets.yaml @@ -3,6 +3,11 @@ targets: #----------------------------------------------------------------------------- #Ubuntu + - ami: ami-0f2e2c076f4c2f941 + name: ubuntu20.10 + type: ubuntu + virt: hvm + user: ubuntu - ami: ami-0758470213bdd23b1 name: ubuntu20.04 type: ubuntu diff --git a/tests/letstest/targets.yaml b/tests/letstest/targets.yaml index 522cab558..a8f760ac7 100644 --- a/tests/letstest/targets.yaml +++ b/tests/letstest/targets.yaml @@ -3,6 +3,11 @@ targets: #----------------------------------------------------------------------------- #Ubuntu + - ami: ami-0f2e2c076f4c2f941 + name: ubuntu20.10 + type: ubuntu + virt: hvm + user: ubuntu - ami: ami-0758470213bdd23b1 name: ubuntu20.04 type: ubuntu From 3673ca77a533826ad7f2d61f78d4ae628bfeff9e Mon Sep 17 00:00:00 2001 From: ohemorange Date: Wed, 28 Oct 2020 15:51:16 -0700 Subject: [PATCH 11/12] Fix LXD setup in snap README (#8416) Fixes #8409. Change the line in the README to allow `sudo /snap/bin/lxd.migrate -yes` to fail (for example, if there's nothing to migrate), but the whole command to succeed. I tested this on a clean Focal install and confirmed it works. --- tools/snap/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/snap/README.md b/tools/snap/README.md index 308c16753..6d64a5163 100644 --- a/tools/snap/README.md +++ b/tools/snap/README.md @@ -12,7 +12,7 @@ These steps need to be done once to set up your VM and do not need to be run aga 1. Start with a Focal VM. You need a full virtual machine using something like DigitalOcean, EC2, or VirtualBox. Docker won't work. Another version of Ubuntu can probably be used, but Focal was used when writing these instructions. 2. Set up a user other than root with sudo privileges for use with snapcraft and run all of the following commands with it. A command to do this for a user named certbot looks like `adduser certbot && usermod -aG sudo certbot && su - certbot`. 3. Install git and python with `sudo apt update && sudo apt install -y git python`. - 4. Set up lxd for use with snapcraft by running `sudo snap install lxd && sudo /snap/bin/lxd.migrate -yes && sudo /snap/bin/lxd waitready && sudo /snap/bin/lxd init --auto` (errors here are ok; it may already + 4. Set up lxd for use with snapcraft by running `sudo snap install lxd && sudo /snap/bin/lxd.migrate -yes; sudo /snap/bin/lxd waitready && sudo /snap/bin/lxd init --auto` (errors here are ok; it may already have been installed on your system). 5. Add your current user to the lxd group and update your shell to have the new assignment by running `sudo usermod -a -G lxd ${USER} && newgrp lxd`. 6. Install snapcraft with `sudo snap install --classic snapcraft`. From 6c7b99f7e0faa60b595b06933f292d476b85799c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 28 Oct 2020 15:52:20 -0700 Subject: [PATCH 12/12] Remove fedora test farm tests (#8415) While working on https://github.com/certbot/certbot/issues/8400, I noticed our Fedora AMIs are quite out of date. I considered updating them and what we could do to avoid the AMIs becoming so out-of-date in the future, but I think we don't actually need these tests. I pulled a new count of Certbot users by OS and we have less than 7,000 Fedora users meaning only ~0.26% of Certbot users run Fedora. (I think Fedora is a popular desktop OS, but not as popular of a server OS which is where Certbot normally runs.) Also, Certbot is regularly updated on Fedora including Fedora Rawhide or the rolling release version of Fedora which is similar to Debian sid/unstable. Rawhide changes far too frequently for it to make sense for us to run tests there in my opinon, but that also means that many problems such as Certbot's unit tests failing to run because of Fedora changes will be caught there by our Fedora maintainers before we'd even see it. This is how https://github.com/certbot/certbot/issues/7106 became an issue and how I learned [Certbot worked on Python 3.9 before we could run tests on it](https://github.com/certbot/certbot/issues/8134#issuecomment-655106169). Because of all this, I think we should just simplify things and remove these tests. If a problem arises in the future, we can always add them back. --- tests/letstest/apache2_targets.yaml | 12 ------------ tests/letstest/auto_targets.yaml | 10 ---------- tests/letstest/targets.yaml | 10 ---------- 3 files changed, 32 deletions(-) diff --git a/tests/letstest/apache2_targets.yaml b/tests/letstest/apache2_targets.yaml index b45cec2c5..8e8e23116 100644 --- a/tests/letstest/apache2_targets.yaml +++ b/tests/letstest/apache2_targets.yaml @@ -42,18 +42,6 @@ targets: virt: hvm user: admin #----------------------------------------------------------------------------- - # Fedora - - ami: ami-0fcbe88944a53b4c8 - name: fedora31 - type: centos - virt: hvm - user: fedora - - ami: ami-00bbc6858140f19ed - name: fedora30 - type: centos - virt: hvm - user: fedora - #----------------------------------------------------------------------------- # CentOS - ami: ami-9887c6e7 name: centos7 diff --git a/tests/letstest/auto_targets.yaml b/tests/letstest/auto_targets.yaml index ce2a2ecbb..76b3a3dc5 100644 --- a/tests/letstest/auto_targets.yaml +++ b/tests/letstest/auto_targets.yaml @@ -47,16 +47,6 @@ targets: type: centos virt: hvm user: ec2-user - - ami: ami-0fcbe88944a53b4c8 - name: fedora31 - type: centos - virt: hvm - user: fedora - - ami: ami-00bbc6858140f19ed - name: fedora30 - type: centos - virt: hvm - user: fedora #----------------------------------------------------------------------------- # CentOS # These Marketplace AMIs must, irritatingly, have their terms manually diff --git a/tests/letstest/targets.yaml b/tests/letstest/targets.yaml index a8f760ac7..29edd1552 100644 --- a/tests/letstest/targets.yaml +++ b/tests/letstest/targets.yaml @@ -43,16 +43,6 @@ targets: type: centos virt: hvm user: ec2-user - - ami: ami-0fcbe88944a53b4c8 - name: fedora31 - type: centos - virt: hvm - user: fedora - - ami: ami-00bbc6858140f19ed - name: fedora30 - type: centos - virt: hvm - user: fedora #----------------------------------------------------------------------------- # CentOS # These Marketplace AMIs must, irritatingly, have their terms manually