starred/certbot-certbot-1
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-08-02 03:22:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Remove deprecated functions (#9315)

Browse Source 
* Remove deprecated functions

* rm unused imports

* actually remove execute_command!

* revert changelog

Co-authored-by: Alex Zorin <alex@zorin.id.au>
This commit is contained in:
Will Greenberg
2022-09-07 13:31:21 +10:00
committed by GitHub
co-authored by Alex Zorin
parent a4a2315537
commit 529a0e2272
4 changed files with 9 additions and 181 deletions
Download Patch File Download Diff File Expand all files Collapse all files
certbot/certbot/compat/misc.py
+2 -36
View File
@@ -10,7 +10,6 @@ import subprocess
import sys
from typing import Optional
from typing import Tuple
import warnings
from certbot import errors
from certbot.compat import os
@@ -144,8 +143,8 @@ def execute_command_status(cmd_name: str, shell_cmd: str,
subprocess.run(shell=True)
- on Windows command will be run in a Powershell shell
This differs from execute_command: it returns the exit code, and does not log the result
and output of the command.
This function returns the exit code, and does not log the result and output
of the command.
:param str cmd_name: the user facing name of the hook being run
:param str shell_cmd: shell command to execute
@@ -168,36 +167,3 @@ def execute_command_status(cmd_name: str, shell_cmd: str,
# bytes in Python 3
out, err = proc.stdout, proc.stderr
return proc.returncode, err, out
def execute_command(cmd_name: str, shell_cmd: str, env: Optional[dict] = None) -> Tuple[str, str]:
"""
Run a command:
- on Linux command will be run by the standard shell selected with
subprocess.run(shell=True)
- on Windows command will be run in a Powershell shell
This differs from execute_command: it returns the exit code, and does not log the result
and output of the command.
:param str cmd_name: the user facing name of the hook being run
:param str shell_cmd: shell command to execute
:param dict env: environ to pass into subprocess.run
:returns: `tuple` (`str` stderr, `str` stdout)
"""
# Deprecation per https://github.com/certbot/certbot/issues/8854
warnings.warn(
"execute_command will be deprecated in the future, use execute_command_status instead",
PendingDeprecationWarning
)
returncode, err, out = execute_command_status(cmd_name, shell_cmd, env)
base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
if out:
logger.info('Output from %s command %s:\n%s', cmd_name, base_cmd, out)
if returncode != 0:
logger.error('%s command "%s" returned error code %d',
cmd_name, shell_cmd, returncode)
if err:
logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
return err, out
certbot/certbot/crypto_util.py
-64
View File
@@ -15,7 +15,6 @@ from typing import Set
from typing import Tuple
from typing import TYPE_CHECKING
from typing import Union
import warnings
from cryptography import x509
from cryptography.exceptions import InvalidSignature
@@ -35,7 +34,6 @@ import josepy
from OpenSSL import crypto
from OpenSSL import SSL
import pyrfc3339
import zope.component
from acme import crypto_util as acme_crypto_util
from certbot import errors
@@ -100,41 +98,6 @@ def generate_key(key_size: int, key_dir: str, key_type: str = "rsa",
return util.Key(key_path, key_pem)
# TODO: Remove this call once zope dependencies are removed from Certbot.
def init_save_key(key_size: int, key_dir: str, key_type: str = "rsa",
elliptic_curve: str = "secp256r1",
keyname: str = "key-certbot.pem") -> util.Key:
"""Initializes and saves a privkey.
Inits key and saves it in PEM format on the filesystem.
.. note:: keyname is the attempted filename, it may be different if a file
already exists at the path.
.. deprecated:: 1.16.0
Use :func:`generate_key` instead.
:param int key_size: key size in bits if key size is rsa.
:param str key_dir: Key save directory.
:param str key_type: Key Type [rsa, ecdsa]
:param str elliptic_curve: Name of the elliptic curve if key type is ecdsa.
:param str keyname: Filename of key
:returns: Key
:rtype: :class:`certbot.util.Key`
:raises ValueError: If unable to generate the key given key_size.
"""
warnings.warn("certbot.crypto_util.init_save_key is deprecated, please use "
"certbot.crypto_util.generate_key instead.", DeprecationWarning)
config = zope.component.getUtility(interfaces.IConfig)
return generate_key(key_size, key_dir, key_type=key_type, elliptic_curve=elliptic_curve,
keyname=keyname, strict_permissions=config.strict_permissions)
def generate_csr(privkey: util.Key, names: Union[List[str], Set[str]], path: str,
must_staple: bool = False, strict_permissions: bool = True) -> util.CSR:
"""Initialize a CSR with the given private key.
@@ -165,33 +128,6 @@ def generate_csr(privkey: util.Key, names: Union[List[str], Set[str]], path: str
return util.CSR(csr_filename, csr_pem, "pem")
# TODO: Remove this call once zope dependencies are removed from Certbot.
def init_save_csr(privkey: util.Key, names: Set[str], path: str) -> util.CSR:
"""Initialize a CSR with the given private key.
.. deprecated:: 1.16.0
Use :func:`generate_csr` instead.
:param privkey: Key to include in the CSR
:type privkey: :class:`certbot.util.Key`
:param set names: `str` names to include in the CSR
:param str path: Certificate save directory.
:returns: CSR
:rtype: :class:`certbot.util.CSR`
"""
warnings.warn("certbot.crypto_util.init_save_csr is deprecated, please use "
"certbot.crypto_util.generate_csr instead.", DeprecationWarning)
config = zope.component.getUtility(interfaces.IConfig)
return generate_csr(privkey, names, path, must_staple=config.must_staple,
strict_permissions=config.strict_permissions)
# WARNING: the csr and private key file are possible attack vectors for TOCTOU
# We should either...
# A. Do more checks to verify that the CSR is trusted/valid
certbot/tests/compat/misc_test.py
+7 -46
View File
@@ -9,52 +9,7 @@ import warnings
from certbot.compat import os
class ExecuteTest(unittest.TestCase):
"""Tests for certbot.compat.misc.execute_command."""
@classmethod
def _call(cls, *args, **kwargs):
from certbot.compat.misc import execute_command
# execute_command is superseded by execute_command_status
with warnings.catch_warnings():
warnings.simplefilter('ignore', category=PendingDeprecationWarning)
return execute_command(*args, **kwargs)
def test_it(self):
for returncode in range(0, 2):
for stdout in ("", "Hello World!",):
for stderr in ("", "Goodbye Cruel World!"):
self._test_common(returncode, stdout, stderr)
def _test_common(self, returncode, stdout, stderr):
given_command = "foo"
given_name = "foo-hook"
with mock.patch("certbot.compat.misc.subprocess.run") as mock_run:
mock_run.return_value.stdout = stdout
mock_run.return_value.stderr = stderr
mock_run.return_value.returncode = returncode
with mock.patch("certbot.compat.misc.logger") as mock_logger:
self.assertEqual(self._call(given_name, given_command), (stderr, stdout))
executed_command = mock_run.call_args[1].get(
"args", mock_run.call_args[0][0])
if os.name == 'nt':
expected_command = ['powershell.exe', '-Command', given_command]
else:
expected_command = given_command
self.assertEqual(executed_command, expected_command)
mock_logger.info.assert_any_call("Running %s command: %s",
given_name, given_command)
if stdout:
mock_logger.info.assert_any_call(mock.ANY, mock.ANY,
mock.ANY, stdout)
if stderr or returncode:
self.assertIs(mock_logger.error.called, True)
class ExecuteStatusTest(ExecuteTest):
class ExecuteStatusTest(unittest.TestCase):
"""Tests for certbot.compat.misc.execute_command_status."""
@classmethod
@@ -84,6 +39,12 @@ class ExecuteStatusTest(ExecuteTest):
mock_logger.info.assert_any_call("Running %s command: %s",
given_name, given_command)
def test_it(self):
for returncode in range(0, 2):
for stdout in ("", "Hello World!",):
for stderr in ("", "Goodbye Cruel World!"):
self._test_common(returncode, stdout, stderr)
if __name__ == "__main__":
unittest.main() # pragma: no cover
certbot/tests/crypto_util_test.py
-35
View File
@@ -67,23 +67,6 @@ class GenerateKeyTest(test_util.TempDirTestCase):
self.assertRaises(ValueError, self._call, 431, self.workdir)
class InitSaveKey(unittest.TestCase):
"""Test for certbot.crypto_util.init_save_key."""
@mock.patch("certbot.crypto_util.generate_key")
@mock.patch("certbot.crypto_util.zope.component")
def test_it(self, mock_zope, mock_generate):
from certbot.crypto_util import init_save_key
mock_zope.getUtility.return_value = mock.MagicMock(strict_permissions=True)
with self.assertWarns(DeprecationWarning):
init_save_key(4096, "/some/path")
mock_generate.assert_called_with(4096, "/some/path", elliptic_curve="secp256r1",
key_type="rsa", keyname="key-certbot.pem",
strict_permissions=True)
class GenerateCSRTest(test_util.TempDirTestCase):
"""Tests for certbot.crypto_util.generate_csr."""
@mock.patch('acme.crypto_util.make_csr')
@@ -100,24 +83,6 @@ class GenerateCSRTest(test_util.TempDirTestCase):
self.assertIn('csr-certbot.pem', csr.file)
class InitSaveCsr(unittest.TestCase):
"""Tests for certbot.crypto_util.init_save_csr."""
@mock.patch("certbot.crypto_util.generate_csr")
@mock.patch("certbot.crypto_util.zope.component")
def test_it(self, mock_zope, mock_generate):
from certbot.crypto_util import init_save_csr
mock_zope.getUtility.return_value = mock.MagicMock(must_staple=True,
strict_permissions=True)
key = certbot.util.Key(file=None, pem=None)
with self.assertWarns(DeprecationWarning):
init_save_csr(key, {"dummy"}, "/some/path")
mock_generate.assert_called_with(key, {"dummy"}, "/some/path",
must_staple=True, strict_permissions=True)
class ValidCSRTest(unittest.TestCase):
"""Tests for certbot.crypto_util.valid_csr."""