From 613e698199e8c81dda07932c12efaf5a77558e0d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 Jan 2023 13:05:01 -0800 Subject: [PATCH 1/5] disable random sleep in lock_test.py (#9545) --- tests/lock_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lock_test.py b/tests/lock_test.py index a1e706c2f..09e11b25f 100644 --- a/tests/lock_test.py +++ b/tests/lock_test.py @@ -134,7 +134,7 @@ def set_up_command(config_dir: str, logs_dir: str, work_dir: str, nginx_dir: str return ( 'certbot --cert-path {0} --key-path {1} --config-dir {2} ' '--logs-dir {3} --work-dir {4} --nginx-server-root {5} --debug ' - '--force-renewal --nginx -vv '.format( + '--force-renewal --nginx -vv --no-random-sleep-on-renew '.format( test_util.vector_path('cert.pem'), test_util.vector_path('rsa512_key.pem'), config_dir, logs_dir, work_dir, nginx_dir).split()) From 81ff6fcc0d1460d669d516dc4df6d90c39507cfa Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 24 Jan 2023 14:06:53 -0800 Subject: [PATCH 2/5] acme.messages.Error: add mutability (#9546) * acme.messages.Error: add mutability As of Python 3.11, an exception caught within a `with` statement will update the __traceback__ attribute. Because acme.messages.Error was immutable, this was causing a knock-on exception, causing certbot to exit abnormally. This commit hacks in mutability for acme.messages.Error Fixes #9539 * Add CHANGELOG entry --- acme/acme/messages.py | 7 +++++++ acme/tests/messages_test.py | 17 +++++++++++++++++ certbot/CHANGELOG.md | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 0e02a054e..c1af1991a 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -123,6 +123,9 @@ class Error(jose.JSONObjectWithFields, errors.Error): https://datatracker.ietf.org/doc/html/rfc7807 + Note: Although Error inherits from JSONObjectWithFields, which is immutable, + we add mutability for Error to comply with the Python exception API. + :ivar str typ: :ivar str title: :ivar str detail: @@ -185,6 +188,10 @@ class Error(jose.JSONObjectWithFields, errors.Error): return code return None + # Hack to allow mutability on Errors (see GH #9539) + def __setattr__(self, name: str, value: Any) -> None: + return object.__setattr__(self, name, value) + def __str__(self) -> str: result = b' :: '.join( part.encode('ascii', 'backslashreplace') for part in diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index cbff65771..7d70822a8 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -1,6 +1,7 @@ """Tests for acme.messages.""" from typing import Dict import unittest +import contextlib from unittest import mock import warnings @@ -91,6 +92,22 @@ class ErrorTest(unittest.TestCase): u"Problem for {1.identifier.value}: {1.typ} :: {1.description} :: {1.detail} :: {1.title}").format( self.error_with_subproblems, self.subproblem)) + # this test is based on a minimal reproduction of a contextmanager/immutable + # exception related error: https://github.com/python/cpython/issues/99856 + def test_with_context_manager(self): + from acme.messages import Error + + @contextlib.contextmanager + def context(): + yield + + try: + with context(): + raise self.error_custom + except Error as e: + self.assertIsNotNone(self.error_custom.__traceback__) + + class ConstantTest(unittest.TestCase): """Tests for acme.messages._Constant.""" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 387373fff..763425f2d 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -17,7 +17,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Fixed a bug where Certbot would crash with `AttributeError: can't set attribute` on ACME server errors in Python 3.11. See [GH #9539](https://github.com/certbot/certbot/issues/9539). More details about these changes can be found on our GitHub repo. From 4ad71ab5ae78b0a7698d983cc588d91be11c5768 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 Jan 2023 17:00:06 -0800 Subject: [PATCH 3/5] Fix tox environments (#9547) * fix cover tox envs * make test work on all Pythons * Remove unused import Co-authored-by: alexzorin Co-authored-by: alexzorin --- .../templates/jobs/standard-tests-jobs.yml | 8 ++++---- acme/tests/messages_test.py | 17 +++++++---------- certbot/docs/contributing.rst | 2 +- tox.ini | 2 +- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml index c49e22bc1..1e6d7a352 100644 --- a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml +++ b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml @@ -7,10 +7,10 @@ jobs: macos-py37-cover: IMAGE_NAME: macOS-12 PYTHON_VERSION: 3.7 - TOXENV: py37-cover + TOXENV: cover macos-cover: IMAGE_NAME: macOS-12 - TOXENV: py3-cover + TOXENV: cover windows-py37: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.7 @@ -18,7 +18,7 @@ jobs: windows-py39-cover: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.9 - TOXENV: py39-cover-win + TOXENV: cover-win windows-integration-certbot: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.9 @@ -37,7 +37,7 @@ jobs: TOXENV: py37 linux-cover: IMAGE_NAME: ubuntu-22.04 - TOXENV: py3-cover + TOXENV: cover linux-lint: IMAGE_NAME: ubuntu-22.04 TOXENV: lint-posix diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index 7d70822a8..405e801a2 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -94,18 +94,15 @@ class ErrorTest(unittest.TestCase): # this test is based on a minimal reproduction of a contextmanager/immutable # exception related error: https://github.com/python/cpython/issues/99856 - def test_with_context_manager(self): - from acme.messages import Error - - @contextlib.contextmanager - def context(): - yield + def test_setting_traceback(self): + self.assertIsNone(self.error_custom.__traceback__) try: - with context(): - raise self.error_custom - except Error as e: - self.assertIsNotNone(self.error_custom.__traceback__) + 1/0 + except ZeroDivisionError as e: + self.error_custom.__traceback__ = e.__traceback__ + + self.assertIsNotNone(self.error_custom.__traceback__) class ConstantTest(unittest.TestCase): diff --git a/certbot/docs/contributing.rst b/certbot/docs/contributing.rst index 6d1faf061..49cc1fc81 100644 --- a/certbot/docs/contributing.rst +++ b/certbot/docs/contributing.rst @@ -130,7 +130,7 @@ For debugging, we recommend putting Once you are done with your code changes, and the tests in ``foo_test.py`` pass, run all of the unit tests for Certbot and check for coverage with ``tox --e py3-cover``. You should then check for code style with ``tox -e lint`` (all +-e cover``. You should then check for code style with ``tox -e lint`` (all files) or ``pylint --rcfile=.pylintrc path/to/file.py`` (single file at a time). diff --git a/tox.ini b/tox.ini index a3436e95a..2bdd5e25c 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ [tox] skipsdist = true -envlist = {py3-cover,lint,mypy}-{win,posix} +envlist = {cover,lint,mypy}-{win,posix} [base] # pip installs the requested packages in editable mode From c79a5d440747a75b3e10ce95e5b4e5b265c67826 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 Jan 2023 13:15:51 -0800 Subject: [PATCH 4/5] Start sending coverage data to codecov (#9544) * set up codecov * export coverage data to xml --- .azure-pipelines/main.yml | 6 +++-- .../templates/jobs/standard-tests-jobs.yml | 5 ++++ .../templates/steps/tox-steps.yml | 25 +++++++++++++++++++ .github/codecov.yml | 7 ++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 .github/codecov.yml diff --git a/.azure-pipelines/main.yml b/.azure-pipelines/main.yml index cae4f799c..d3c3a152e 100644 --- a/.azure-pipelines/main.yml +++ b/.azure-pipelines/main.yml @@ -1,8 +1,10 @@ -trigger: none +# We run the test suite on commits to master so codecov gets coverage data +# about the master branch and can use it to track coverage changes. +trigger: + - master pr: - master - '*.x' jobs: - template: templates/jobs/standard-tests-jobs.yml - diff --git a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml index 1e6d7a352..5d0044d19 100644 --- a/.azure-pipelines/templates/jobs/standard-tests-jobs.yml +++ b/.azure-pipelines/templates/jobs/standard-tests-jobs.yml @@ -11,6 +11,7 @@ jobs: macos-cover: IMAGE_NAME: macOS-12 TOXENV: cover + UPLOAD_COVERAGE: 1 windows-py37: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.7 @@ -19,10 +20,12 @@ jobs: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.9 TOXENV: cover-win + UPLOAD_COVERAGE: 1 windows-integration-certbot: IMAGE_NAME: windows-2019 PYTHON_VERSION: 3.9 TOXENV: integration-certbot + UPLOAD_COVERAGE: 1 linux-oldest-tests-1: IMAGE_NAME: ubuntu-22.04 PYTHON_VERSION: 3.7 @@ -38,6 +41,7 @@ jobs: linux-cover: IMAGE_NAME: ubuntu-22.04 TOXENV: cover + UPLOAD_COVERAGE: 1 linux-lint: IMAGE_NAME: ubuntu-22.04 TOXENV: lint-posix @@ -49,6 +53,7 @@ jobs: PYTHON_VERSION: 3.8 TOXENV: integration ACME_SERVER: pebble + UPLOAD_COVERAGE: 1 apache-compat: IMAGE_NAME: ubuntu-22.04 TOXENV: apache_compat diff --git a/.azure-pipelines/templates/steps/tox-steps.yml b/.azure-pipelines/templates/steps/tox-steps.yml index fbda960a5..a85e1519a 100644 --- a/.azure-pipelines/templates/steps/tox-steps.yml +++ b/.azure-pipelines/templates/steps/tox-steps.yml @@ -55,3 +55,28 @@ steps: AWS_SECRET_ACCESS_KEY: $(AWS_SECRET_ACCESS_KEY) AWS_EC2_PEM_FILE: $(testFarmPem.secureFilePath) displayName: Run tox + # For now, let's omit `set -e` and avoid the script exiting with a nonzero + # status code to prevent problems here from causing build failures. If + # this turns out to work well, we can change this. + - bash: | + python3 tools/pip_install.py -I coverage + case "$AGENT_OS" in + Darwin) + CODECOV_URL="https://uploader.codecov.io/latest/macos/codecov" + ;; + Linux) + CODECOV_URL="https://uploader.codecov.io/latest/linux/codecov" + ;; + Windows_NT) + CODECOV_URL="https://uploader.codecov.io/latest/windows/codecov.exe" + ;; + *) + echo "Unexpected OS" + exit 0 + esac + curl --retry 3 -o codecov "$CODECOV_URL" + chmod +x codecov + coverage xml + ./codecov || echo "Uploading coverage data failed" + condition: ne(variables['UPLOAD_COVERAGE'], '') + displayName: Upload coverage data diff --git a/.github/codecov.yml b/.github/codecov.yml new file mode 100644 index 000000000..23d2fbbe9 --- /dev/null +++ b/.github/codecov.yml @@ -0,0 +1,7 @@ +# This disables all reporting from codecov. Let's just set it up to collect +# data for now and then we can play with the settings here. +comment: false +coverage: + status: + project: off + patch: off From b0748b69e77f7d4058472feb8d61f94ffa8ff7ac Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Wed, 25 Jan 2023 15:59:22 -0800 Subject: [PATCH 5/5] Replace probot/stale app with a Github Action (#9466) * Replace probot/stale app with a Github Action This creates a Github Actions workflow which seems to be the supported way of automarking issues as stale. Adds a dry-run flag to test it out. * small fixups * cron typo * disable unnecessary permissions * use friendlier name --- .github/stale.yml | 35 ------------------------------- .github/workflows/stale.yml | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 35 deletions(-) delete mode 100644 .github/stale.yml create mode 100644 .github/workflows/stale.yml diff --git a/.github/stale.yml b/.github/stale.yml deleted file mode 100644 index 2e4106314..000000000 --- a/.github/stale.yml +++ /dev/null @@ -1,35 +0,0 @@ -# Configuration for https://github.com/marketplace/stale - -# Number of days of inactivity before an Issue or Pull Request becomes stale -daysUntilStale: 365 - -# Number of days of inactivity before an Issue or Pull Request with the stale label is closed. -# Set to false to disable. If disabled, issues still need to be closed manually, but will remain marked as stale. -# When changing this value, be sure to also update markComment below. -daysUntilClose: 30 - -# Ignore issues with an assignee (defaults to false) -exemptAssignees: true - -# Label to use when marking as stale -staleLabel: needs-update - -# Comment to post when marking as stale. Set to `false` to disable -markComment: > - We've made a lot of changes to Certbot since this issue was opened. If you - still have this issue with an up-to-date version of Certbot, can you please - add a comment letting us know? This helps us to better see what issues are - still affecting our users. If there is no activity in the next 30 days, this - issue will be automatically closed. - -# Comment to post when closing a stale Issue or Pull Request. -closeComment: > - This issue has been closed due to lack of activity, but if you think it - should be reopened, please open a new issue with a link to this one and we'll - take a look. - -# Limit the number of actions per hour, from 1-30. Default is 30 -limitPerRun: 1 - -# Don't mark pull requests as stale. -only: issues diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml new file mode 100644 index 000000000..010259922 --- /dev/null +++ b/.github/workflows/stale.yml @@ -0,0 +1,42 @@ +name: Update Stale Issues +on: + schedule: + # Run at midnight every night + - cron: '24 1 * * *' +permissions: + issues: write +jobs: + stale: + runs-on: ubuntu-latest + steps: + - uses: actions/stale@v6 + with: + # REMOVEME: dry run to see if this works + debug-only: true + + # Idle number of days before marking issues stale + days-before-issue-stale: 365 + + # Idle number of days before closing stale issues + days-before-issue-close: 30 + + # Ignore issues with an assignee + exempt-all-issue-assignees: true + + # Label to use when marking as stale + stale-issue-label: needs-update + + stale-issue-message: > + We've made a lot of changes to Certbot since this issue was opened. If you + still have this issue with an up-to-date version of Certbot, can you please + add a comment letting us know? This helps us to better see what issues are + still affecting our users. If there is no activity in the next 30 days, this + issue will be automatically closed. + + close-issue-message: > + This issue has been closed due to lack of activity, but if you think it + should be reopened, please open a new issue with a link to this one and we'll + take a look. + + # Limit the number of actions per hour, from 1-30. Default is 30 + operations-per-run: 1