Work on issues found by pylint (#896)

* Look at possibly-used-before-assignment.

* Use latest beta releases of ansible-core 2.19 for mypy and pylint.

* Look at unsupported-*.

* Look at unknown-option-value.

* Look at redefined-builtin.

* Look at superfluous-parens.

* Look at unspecified-encoding.

* Adjust to new cryptography version and to ansible-core 2.17's pylint.

* Look at super-with-arguments.

* Look at no-else-*.

* Look at try-except-raise.

* Look at inconsistent-return-statements.

* Look at redefined-outer-name.

* Look at redefined-argument-from-local.

* Look at attribute-defined-outside-init.

* Look at unused-variable.

* Look at protected-access.

* Look at raise-missing-from.

* Look at arguments-differ.

* Look at useless-suppression and use-symbolic-message-instead.

* Look at consider-using-dict-items.

* Look at consider-using-in.

* Look at consider-using-set-comprehension.

* Look at consider-using-with.

* Look at use-dict-literal.
This commit is contained in:
Felix Fontein
2025-05-18 00:57:28 +02:00
committed by GitHub
parent a3a5284f97
commit 318462fa24
96 changed files with 1748 additions and 1598 deletions

View File

@@ -119,7 +119,7 @@ class ACMEAccount:
if "location" in info:
self.client.set_account_uri(info["location"])
return True, result
elif info["status"] == 200:
if info["status"] == 200:
# Account did exist
if result.get("status") == "deactivated":
# A bug in Pebble (https://github.com/letsencrypt/pebble/issues/179) and
@@ -130,12 +130,11 @@ class ACMEAccount:
# requests authorized by that account's key."
if not allow_creation:
return False, None
else:
raise ModuleFailException("Account is deactivated")
raise ModuleFailException("Account is deactivated")
if "location" in info:
self.client.set_account_uri(info["location"])
return False, result
elif (
if (
info["status"] in (400, 404)
and result["type"] == "urn:ietf:params:acme:error:accountDoesNotExist"
and not allow_creation
@@ -144,7 +143,7 @@ class ACMEAccount:
# (According to RFC 8555, Section 7.3.1, the HTTP status code MUST be 400.
# Unfortunately Digicert does not care and sends 404 instead.)
return False, None
elif (
if (
info["status"] == 403
and result["type"] == "urn:ietf:params:acme:error:unauthorized"
and "deactivated" in (result.get("detail") or "")
@@ -154,15 +153,13 @@ class ACMEAccount:
# might need adjustment in error detection.
if not allow_creation:
return False, None
else:
raise ModuleFailException("Account is deactivated")
else:
raise ACMEProtocolException(
module=self.client.module,
msg="Registering ACME account failed",
info=info,
content_json=result,
)
raise ModuleFailException("Account is deactivated")
raise ACMEProtocolException(
module=self.client.module,
msg="Registering ACME account failed",
info=info,
content_json=result,
)
def get_account_data(self) -> dict[str, t.Any] | None:
"""

View File

@@ -244,7 +244,9 @@ class ACMEClient:
passphrase=self.account_key_passphrase,
)
except KeyParsingError as e:
raise ModuleFailException(f"Error while parsing account key: {e.msg}")
raise ModuleFailException(
f"Error while parsing account key: {e.msg}"
) from e
self.account_jwk = self.account_key_data["jwk"]
self.account_jws_header = {
"alg": self.account_key_data["alg"],
@@ -307,7 +309,7 @@ class ACMEClient:
except Exception as e:
raise ModuleFailException(
f"Failed to encode payload / headers as JSON: {e}"
)
) from e
return self.backend.sign(
payload64=payload64, protected64=protected64, key_data=key_data
@@ -456,10 +458,10 @@ class ACMEClient:
result = decoded_result
else:
result = content
except ValueError:
except ValueError as exc:
raise NetworkException(
f"Failed to parse the ACME response: {url} {content}"
)
) from exc
else:
result = content
@@ -569,10 +571,10 @@ class ACMEClient:
try:
result = self.module.from_json(content.decode("utf8"))
parsed_json_result = True
except ValueError:
except ValueError as exc:
raise NetworkException(
f"Failed to parse the ACME response: {uri} {content!r}"
)
) from exc
else:
result = content
else:
@@ -642,30 +644,32 @@ def create_default_argspec(
Provides default argument spec for the options documented in the acme doc fragment.
"""
result = ArgumentSpec(
argument_spec=dict(
acme_directory=dict(type="str", required=True),
acme_version=dict(type="int", choices=[2], default=2),
validate_certs=dict(type="bool", default=True),
select_crypto_backend=dict(
type="str", default="auto", choices=["auto", "openssl", "cryptography"]
),
request_timeout=dict(type="int", default=10),
),
argument_spec={
"acme_directory": {"type": "str", "required": True},
"acme_version": {"type": "int", "choices": [2], "default": 2},
"validate_certs": {"type": "bool", "default": True},
"select_crypto_backend": {
"type": "str",
"default": "auto",
"choices": ["auto", "openssl", "cryptography"],
},
"request_timeout": {"type": "int", "default": 10},
},
)
if with_account:
result.update_argspec(
account_key_src=dict(type="path", aliases=["account_key"]),
account_key_content=dict(type="str", no_log=True),
account_key_passphrase=dict(type="str", no_log=True),
account_uri=dict(type="str"),
account_key_src={"type": "path", "aliases": ["account_key"]},
account_key_content={"type": "str", "no_log": True},
account_key_passphrase={"type": "str", "no_log": True},
account_uri={"type": "str"},
)
if require_account_key:
result.update(required_one_of=[["account_key_src", "account_key_content"]])
result.update(mutually_exclusive=[["account_key_src", "account_key_content"]])
if with_certificate:
result.update_argspec(
csr=dict(type="path"),
csr_content=dict(type="str"),
csr={"type": "path"},
csr_content={"type": "str"},
)
result.update(
required_one_of=[["csr", "csr_content"]],

View File

@@ -211,9 +211,7 @@ class CryptographyChainMatcher(ChainMatcher):
class CryptographyBackend(CryptoBackend):
def __init__(self, *, module: AnsibleModule) -> None:
super(CryptographyBackend, self).__init__(
module=module, with_timezone=CRYPTOGRAPHY_TIMEZONE
)
super().__init__(module=module, with_timezone=CRYPTOGRAPHY_TIMEZONE)
def parse_key(
self,
@@ -242,7 +240,7 @@ class CryptographyBackend(CryptoBackend):
password=to_bytes(passphrase) if passphrase is not None else None,
)
except Exception as e:
raise KeyParsingError(f"error while loading key: {e}")
raise KeyParsingError(f"error while loading key: {e}") from e
if isinstance(key, cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey):
rsa_pk = key.public_key().public_numbers()
return {
@@ -256,7 +254,7 @@ class CryptographyBackend(CryptoBackend):
},
"hash": "sha256",
}
elif isinstance(
if isinstance(
key, cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePrivateKey
):
ec_pk = key.public_key().public_numbers()
@@ -296,8 +294,7 @@ class CryptographyBackend(CryptoBackend):
"hash": hashalg,
"point_size": point_size,
}
else:
raise KeyParsingError(f'unknown key type "{type(key)}"')
raise KeyParsingError(f'unknown key type "{type(key)}"')
def sign(
self, *, payload64: str, protected64: str, key_data: dict[str, t.Any]
@@ -332,6 +329,8 @@ class CryptographyBackend(CryptoBackend):
rr = convert_int_to_hex(r, digits=2 * key_data["point_size"])
ss = convert_int_to_hex(s, digits=2 * key_data["point_size"])
signature = binascii.unhexlify(rr) + binascii.unhexlify(ss)
else:
raise AssertionError("Can never be reached") # pragma: no cover
return {
"protected": protected64,
@@ -472,8 +471,10 @@ class CryptographyBackend(CryptoBackend):
cert = cryptography.x509.load_pem_x509_certificate(b_cert_content)
except Exception as e:
if cert_filename is None:
raise BackendException(f"Cannot parse certificate: {e}")
raise BackendException(f"Cannot parse certificate {cert_filename}: {e}")
raise BackendException(f"Cannot parse certificate: {e}") from e
raise BackendException(
f"Cannot parse certificate {cert_filename}: {e}"
) from e
if now is None:
now = self.get_now()
@@ -508,8 +509,10 @@ class CryptographyBackend(CryptoBackend):
cert = cryptography.x509.load_pem_x509_certificate(b_cert_content)
except Exception as e:
if cert_filename is None:
raise BackendException(f"Cannot parse certificate: {e}")
raise BackendException(f"Cannot parse certificate {cert_filename}: {e}")
raise BackendException(f"Cannot parse certificate: {e}") from e
raise BackendException(
f"Cannot parse certificate {cert_filename}: {e}"
) from e
ski = None
try:

View File

@@ -45,7 +45,12 @@ if t.TYPE_CHECKING:
)
_OPENSSL_ENVIRONMENT_UPDATE = dict(LANG="C", LC_ALL="C", LC_MESSAGES="C", LC_CTYPE="C")
_OPENSSL_ENVIRONMENT_UPDATE = {
"LANG": "C",
"LC_ALL": "C",
"LC_MESSAGES": "C",
"LC_CTYPE": "C",
}
def _extract_date(
@@ -66,7 +71,7 @@ def _extract_date(
except ValueError as exc:
raise BackendException(
f"Failed to parse '{name}' date{cert_filename_suffix}: {exc}"
)
) from exc
def _decode_octets(octets_text: str) -> bytes:
@@ -118,7 +123,7 @@ class OpenSSLCLIBackend(CryptoBackend):
def __init__(
self, *, module: AnsibleModule, openssl_binary: str | None = None
) -> None:
super(OpenSSLCLIBackend, self).__init__(module=module, with_timezone=True)
super().__init__(module=module, with_timezone=True)
if openssl_binary is None:
openssl_binary = module.get_bin_path("openssl", True)
self.openssl_binary = openssl_binary
@@ -156,11 +161,11 @@ class OpenSSLCLIBackend(CryptoBackend):
raise KeyParsingError(
f"failed to create temporary content file: {err}",
exception=traceback.format_exc(),
)
) from err
f.close()
# Parse key
account_key_type = None
with open(key_file, "rt") as fi:
with open(key_file, "r", encoding="utf-8") as fi:
for line in fi:
m = re.match(
r"^\s*-{5,}BEGIN\s+(EC|RSA)\s+PRIVATE\s+KEY-{5,}\s*$", line
@@ -228,7 +233,7 @@ class OpenSSLCLIBackend(CryptoBackend):
},
"hash": "sha256",
}
elif account_key_type == "ec":
if account_key_type == "ec":
pub_data = re.search(
r"pub:\s*\n\s+04:([a-f0-9\:\s]+?)\nASN1 OID: (\S+)(?:\nNIST CURVE: (\S+))?",
out_text,

View File

@@ -77,14 +77,14 @@ def _parse_acme_timestamp(
"""
# RFC 3339 (https://www.rfc-editor.org/info/rfc3339)
timestamp_str = _reduce_fractional_digits(timestamp_str)
for format in (
for time_format in (
"%Y-%m-%dT%H:%M:%SZ",
"%Y-%m-%dT%H:%M:%S.%fZ",
"%Y-%m-%dT%H:%M:%S%z",
"%Y-%m-%dT%H:%M:%S.%f%z",
):
try:
result = datetime.datetime.strptime(timestamp_str, format)
result = datetime.datetime.strptime(timestamp_str, time_format)
except ValueError:
pass
else:
@@ -117,7 +117,7 @@ class CryptoBackend(metaclass=abc.ABCMeta):
raise BackendException(f"Invalid value for {name}: {value!r}")
return result
except OpenSSLObjectError as exc:
raise BackendException(str(exc))
raise BackendException(str(exc)) from exc
def interpolate_timestamp(
self,

View File

@@ -166,11 +166,11 @@ class ACMECertificateClient:
continue
challenge_data = authz.get_challenge_data(client=self.client)
data.append(
dict(
identifier=authz.identifier,
identifier_type=authz.identifier_type,
challenges=challenge_data,
)
{
"identifier": authz.identifier,
"identifier_type": authz.identifier_type,
"challenges": challenge_data,
}
)
dns_challenge = challenge_data.get(dns_challenge_type)
if dns_challenge:
@@ -321,7 +321,7 @@ class ACMECertificateClient:
"""
if self.csr is None and self.csr_content is None:
raise ModuleFailException("No CSR has been provided")
for identifier, authz in order.authorizations.items():
for authz in order.authorizations.values():
if authz.status != "valid":
authz.raise_error(
error_msg=f'Status is {authz.status!r} and not "valid"',

View File

@@ -71,7 +71,7 @@ class CertificateChain:
process_links(
info=info,
callback=lambda link, relation: result._process_links(
callback=lambda link, relation: result._process_links( # pylint: disable=protected-access
client=client, link=link, relation=relation
),
)

View File

@@ -295,10 +295,10 @@ class Authorization:
raise ACMEProtocolException(
module=module,
msg=f"Failed to validate challenge for {self.combined_identifier}: {error_msg}. {'; '.join(error_details)}",
extras=dict(
identifier=self.combined_identifier,
authorization=self.data,
),
extras={
"identifier": self.combined_identifier,
"authorization": self.data,
},
)
def find_challenge(self, *, challenge_type: str) -> Challenge | None:
@@ -374,7 +374,7 @@ class Authorization:
"""
authz = cls(url=url)
authz_deactivate = {"status": "deactivated"}
result, info = client.send_signed_request(
result, _info = client.send_signed_request(
url, authz_deactivate, fail_on_error=True
)
authz._setup(client=client, data=result)

View File

@@ -40,12 +40,12 @@ def format_error_problem(
subproblems = problem.get("subproblems")
if subproblems is not None:
msg = f"{msg} Subproblems:"
for index, problem in enumerate(subproblems):
for index, subproblem in enumerate(subproblems):
index_str = f"{subproblem_prefix}{index}"
problem_str = format_error_problem(
problem, subproblem_prefix=f"{index_str}."
subproblem_str = format_error_problem(
subproblem, subproblem_prefix=f"{index_str}."
)
msg = f"{msg}\n({index_str}) {problem_str}"
msg = f"{msg}\n({index_str}) {subproblem_str}"
return msg
@@ -55,7 +55,7 @@ class ModuleFailException(Exception):
"""
def __init__(self, msg: str, **args: t.Any) -> None:
super(ModuleFailException, self).__init__(self, msg)
super().__init__(self, msg)
self.msg = msg
self.module_fail_args = args
@@ -100,7 +100,7 @@ class ACMEProtocolException(ModuleFailException):
except Exception:
pass
extras = extras or dict()
extras = extras or {}
error_code = None
error_type = None
@@ -152,7 +152,7 @@ class ACMEProtocolException(ModuleFailException):
elif content is not None:
add_msg = f" The raw result: {to_text(content)}"
super(ACMEProtocolException, self).__init__(f"{msg}.{add_msg}", **extras)
super().__init__(f"{msg}.{add_msg}", **extras)
self.problem: dict[str, t.Any] = {}
self.subproblems: list[dict[str, t.Any]] = []
self.error_code = error_code

View File

@@ -29,7 +29,7 @@ def read_file(fn: str | os.PathLike) -> bytes:
with open(fn, "rb") as f:
return f.read()
except Exception as e:
raise ModuleFailException(f'Error while reading file "{fn}": {e}')
raise ModuleFailException(f'Error while reading file "{fn}": {e}') from e
# This function was adapted from an earlier version of https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/uri.py
@@ -55,7 +55,7 @@ def write_file(
raise ModuleFailException(
f"failed to create temporary content file: {err}",
exception=traceback.format_exc(),
)
) from err
f.close()
checksum_src = None
checksum_dest = None
@@ -94,7 +94,7 @@ def write_file(
raise ModuleFailException(
f"failed to copy {tmpsrc} to {dest}: {err}",
exception=traceback.format_exc(),
)
) from err
os.remove(tmpsrc)
return changed

View File

@@ -60,13 +60,13 @@ def pem_to_der(
lines = pem_content.splitlines()
elif pem_filename is not None:
try:
with open(pem_filename, "rt") as f:
with open(pem_filename, "r", encoding="utf-8") as f:
lines = list(f)
except Exception as err:
raise ModuleFailException(
f"cannot load PEM file {pem_filename}: {err}",
exception=traceback.format_exc(),
)
) from err
else:
raise ModuleFailException(
"One of pem_filename and pem_content must be provided"