From 1b9aec291c63a1e3909f7130e6536c73ffcaba34 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 10:25:33 +0300 Subject: [PATCH 1/9] Metadata API: Use TYPE_CHECKING for cyclic import The import is useful for mypy so it can check the types. Add a pylint disable just like json.py does in the same situation. Signed-off-by: Jussi Kukkonen --- tuf/api/serialization/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tuf/api/serialization/__init__.py b/tuf/api/serialization/__init__.py index ed3a4843c0..4ec0a4aef1 100644 --- a/tuf/api/serialization/__init__.py +++ b/tuf/api/serialization/__init__.py @@ -15,6 +15,11 @@ """ import abc +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + # pylint: disable=cyclic-import + from tuf.api.metadata import Metadata, Signed # TODO: Should these be in tuf.exceptions or inherit from tuf.exceptions.Error? From f4d008cd14771f70472eb95f528a02e0c83e7a9b Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 10:26:03 +0300 Subject: [PATCH 2/9] Metadata API: Improve type hint for _signed_type Use ClassVar for extra protection, set default value to a string so type checking is ok with it Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 034eeceec9..0b8c7e5372 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -17,7 +17,7 @@ """ import tempfile from datetime import datetime, timedelta -from typing import Any, Dict, List, Mapping, Optional +from typing import Any, ClassVar, Dict, List, Mapping, Optional from securesystemslib.keys import verify_signature from securesystemslib.signer import Signature, Signer @@ -321,7 +321,7 @@ class Signed: """ # Signed implementations are expected to override this - _signed_type = None + _signed_type: ClassVar[str] = "signed" # _type and type are identical: 1st replicates file format, 2nd passes lint @property From fe23a95cb2dd669df493452add0473adb25e9829 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 10:54:04 +0300 Subject: [PATCH 3/9] Metadata API: Mark Signed as Abstract Base Class Also define from_dict()/to_dict() as abstract: this helps mypy keep track of things. Rename derived argument *_dict in the derived classes to keep the linter happy. Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 54 +++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 0b8c7e5372..2ad3de6da0 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -15,6 +15,7 @@ available in the class model. """ +import abc import tempfile from datetime import datetime, timedelta from typing import Any, ClassVar, Dict, List, Mapping, Optional @@ -304,7 +305,7 @@ def verify( ) -class Signed: +class Signed(metaclass=abc.ABCMeta): """A base class for the signed part of TUF metadata. Objects with base class Signed are usually included in a Metadata object @@ -351,6 +352,17 @@ def __init__( self.version = version self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {} + @abc.abstractmethod + def to_dict(self) -> Dict[str, Any]: + """Serialization helper that returns dict representation of self""" + raise NotImplementedError + + @classmethod + @abc.abstractmethod + def from_dict(cls, signed_dict: Dict[str, Any]) -> "Signed": + """Deserialization helper, creates object from dict representation""" + raise NotImplementedError + @classmethod def _common_fields_from_dict(cls, signed_dict: Dict[str, Any]) -> List[Any]: """Returns common fields of 'Signed' instances from the passed dict @@ -550,20 +562,20 @@ def __init__( self.roles = roles @classmethod - def from_dict(cls, root_dict: Dict[str, Any]) -> "Root": + def from_dict(cls, signed_dict: Dict[str, Any]) -> "Root": """Creates Root object from its dict representation.""" - common_args = cls._common_fields_from_dict(root_dict) - consistent_snapshot = root_dict.pop("consistent_snapshot", None) - keys = root_dict.pop("keys") - roles = root_dict.pop("roles") + common_args = cls._common_fields_from_dict(signed_dict) + consistent_snapshot = signed_dict.pop("consistent_snapshot", None) + keys = signed_dict.pop("keys") + roles = signed_dict.pop("roles") for keyid, key_dict in keys.items(): keys[keyid] = Key.from_dict(key_dict) for role_name, role_dict in roles.items(): roles[role_name] = Role.from_dict(role_dict) - # All fields left in the root_dict are unrecognized. - return cls(*common_args, keys, roles, consistent_snapshot, root_dict) + # All fields left in the signed_dict are unrecognized. + return cls(*common_args, keys, roles, consistent_snapshot, signed_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" @@ -683,13 +695,13 @@ def __init__( self.meta = meta @classmethod - def from_dict(cls, timestamp_dict: Dict[str, Any]) -> "Timestamp": + def from_dict(cls, signed_dict: Dict[str, Any]) -> "Timestamp": """Creates Timestamp object from its dict representation.""" - common_args = cls._common_fields_from_dict(timestamp_dict) - meta_dict = timestamp_dict.pop("meta") + common_args = cls._common_fields_from_dict(signed_dict) + meta_dict = signed_dict.pop("meta") meta = {"snapshot.json": MetaFile.from_dict(meta_dict["snapshot.json"])} # All fields left in the timestamp_dict are unrecognized. - return cls(*common_args, meta, timestamp_dict) + return cls(*common_args, meta, signed_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" @@ -733,15 +745,15 @@ def __init__( self.meta = meta @classmethod - def from_dict(cls, snapshot_dict: Dict[str, Any]) -> "Snapshot": + def from_dict(cls, signed_dict: Dict[str, Any]) -> "Snapshot": """Creates Snapshot object from its dict representation.""" - common_args = cls._common_fields_from_dict(snapshot_dict) - meta_dicts = snapshot_dict.pop("meta") + common_args = cls._common_fields_from_dict(signed_dict) + meta_dicts = signed_dict.pop("meta") meta = {} for meta_path, meta_dict in meta_dicts.items(): meta[meta_path] = MetaFile.from_dict(meta_dict) # All fields left in the snapshot_dict are unrecognized. - return cls(*common_args, meta, snapshot_dict) + return cls(*common_args, meta, signed_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" @@ -971,12 +983,12 @@ def __init__( self.delegations = delegations @classmethod - def from_dict(cls, targets_dict: Dict[str, Any]) -> "Targets": + def from_dict(cls, signed_dict: Dict[str, Any]) -> "Targets": """Creates Targets object from its dict representation.""" - common_args = cls._common_fields_from_dict(targets_dict) - targets = targets_dict.pop("targets") + common_args = cls._common_fields_from_dict(signed_dict) + targets = signed_dict.pop("targets") try: - delegations_dict = targets_dict.pop("delegations") + delegations_dict = signed_dict.pop("delegations") except KeyError: delegations = None else: @@ -985,7 +997,7 @@ def from_dict(cls, targets_dict: Dict[str, Any]) -> "Targets": for target_path, target_info in targets.items(): res_targets[target_path] = TargetFile.from_dict(target_info) # All fields left in the targets_dict are unrecognized. - return cls(*common_args, res_targets, delegations, targets_dict) + return cls(*common_args, res_targets, delegations, signed_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" From dfe6a6619b645ab95f4113b84a45f45548c65cb0 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 11:12:05 +0300 Subject: [PATCH 4/9] Metadata API: Type hint signed in Metadata.from_dict() Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 2ad3de6da0..edff949fd8 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -18,7 +18,7 @@ import abc import tempfile from datetime import datetime, timedelta -from typing import Any, ClassVar, Dict, List, Mapping, Optional +from typing import Any, ClassVar, Dict, List, Mapping, Optional, Type from securesystemslib.keys import verify_signature from securesystemslib.signer import Signature, Signer @@ -79,7 +79,7 @@ def from_dict(cls, metadata: Dict[str, Any]) -> "Metadata": _type = metadata["signed"]["_type"] if _type == "targets": - inner_cls = Targets + inner_cls: Type[Signed] = Targets elif _type == "snapshot": inner_cls = Snapshot elif _type == "timestamp": From 43f13a05651436f94303a3459585430511416ef2 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 11:29:42 +0300 Subject: [PATCH 5/9] Metadata: _common_fields_from_dict() should return Tuple This allows mypy to track the argument types through the constructor calls. Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index edff949fd8..cecbc9a13f 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -18,7 +18,7 @@ import abc import tempfile from datetime import datetime, timedelta -from typing import Any, ClassVar, Dict, List, Mapping, Optional, Type +from typing import Any, ClassVar, Dict, List, Mapping, Optional, Tuple, Type from securesystemslib.keys import verify_signature from securesystemslib.signer import Signature, Signer @@ -364,7 +364,9 @@ def from_dict(cls, signed_dict: Dict[str, Any]) -> "Signed": raise NotImplementedError @classmethod - def _common_fields_from_dict(cls, signed_dict: Dict[str, Any]) -> List[Any]: + def _common_fields_from_dict( + cls, signed_dict: Dict[str, Any] + ) -> Tuple[int, str, datetime]: """Returns common fields of 'Signed' instances from the passed dict representation, and returns an ordered list to be passed as leading positional arguments to a subclass constructor. @@ -383,7 +385,7 @@ def _common_fields_from_dict(cls, signed_dict: Dict[str, Any]) -> List[Any]: # what the constructor expects and what we store. The inverse operation # is implemented in '_common_fields_to_dict'. expires = formats.expiry_string_to_datetime(expires_str) - return [version, spec_version, expires] + return version, spec_version, expires def _common_fields_to_dict(self) -> Dict[str, Any]: """Returns dict representation of common fields of 'Signed' instances. From ca5f2ddd9c22c380fd644fa0d439f77aac764d7c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 13:19:45 +0300 Subject: [PATCH 6/9] Add initial mypy configuration This is an initial setup: By default check only tuf/api/, and ignore securesystemslib imports. Change lint working directory to source root: This saves repeating a lot of {toxinidir} in the command lines. Signed-off-by: Jussi Kukkonen --- requirements-test.txt | 1 + setup.cfg | 7 +++++++ tox.ini | 13 ++++++++----- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/requirements-test.txt b/requirements-test.txt index 65646ca222..692d53953e 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -11,4 +11,5 @@ coverage black isort pylint +mypy bandit diff --git a/setup.cfg b/setup.cfg index 1ffc5648ee..0b65c06a34 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,3 +6,10 @@ ignore = requirements-dev.txt .travis.yml .coveragerc + +[mypy] +warn_unused_configs = True +files = tuf/api/ + +[mypy-securesystemslib.*] +ignore_missing_imports = True diff --git a/tox.ini b/tox.ini index d3ea4f947d..84d1c87bfe 100644 --- a/tox.ini +++ b/tox.ini @@ -39,15 +39,18 @@ commands = python -m coverage report -m [testenv:lint] +changedir = {toxinidir} commands = # Use different configs for new (tuf/api/*) and legacy code # TODO: configure black and isort args in pyproject.toml (see #1161) - black --check --diff --line-length 80 {toxinidir}/tuf/api - isort --check --diff --line-length 80 --profile black -p tuf {toxinidir}/tuf/api - pylint {toxinidir}/tuf/api --rcfile={toxinidir}/tuf/api/pylintrc + black --check --diff --line-length 80 tuf/api + isort --check --diff --line-length 80 --profile black -p tuf tuf/api + pylint tuf/api --rcfile=tuf/api/pylintrc # NOTE: Contrary to what the pylint docs suggest, ignoring full paths does # work, unfortunately each subdirectory has to be ignored explicitly. - pylint {toxinidir}/tuf --ignore={toxinidir}/tuf/api,{toxinidir}/tuf/api/serialization + pylint tuf --ignore=tuf/api,tuf/api/serialization - bandit -r {toxinidir}/tuf + mypy + + bandit -r tuf From 510f224e3f76eba1f0cd3b45e3d5f739b536e89b Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 10 May 2021 14:52:59 +0300 Subject: [PATCH 7/9] tox: Run pylint in parallel pylint on the legacy code is by far the slowest part of linting (to the extent that parallelizing the tox env itself doesn't really help): pylint can fortunately parallelize itself. Signed-off-by: Jussi Kukkonen --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 84d1c87bfe..94d0d0683f 100644 --- a/tox.ini +++ b/tox.ini @@ -45,11 +45,11 @@ commands = # TODO: configure black and isort args in pyproject.toml (see #1161) black --check --diff --line-length 80 tuf/api isort --check --diff --line-length 80 --profile black -p tuf tuf/api - pylint tuf/api --rcfile=tuf/api/pylintrc + pylint -j 0 tuf/api --rcfile=tuf/api/pylintrc # NOTE: Contrary to what the pylint docs suggest, ignoring full paths does # work, unfortunately each subdirectory has to be ignored explicitly. - pylint tuf --ignore=tuf/api,tuf/api/serialization + pylint -j 0 tuf --ignore=tuf/api,tuf/api/serialization mypy From 2e3eb40cf9ccfb5d5a43786acd291b94f9c0c4f7 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 17 May 2021 19:02:23 +0300 Subject: [PATCH 8/9] Metadata API: Fix DelegatedRole.from_dict() return type Also mark the argument as Dict as we will pop() it. Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index cecbc9a13f..7e054ba057 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -815,7 +815,7 @@ def __init__( self.path_hash_prefixes = path_hash_prefixes @classmethod - def from_dict(cls, role_dict: Mapping[str, Any]) -> "Role": + def from_dict(cls, role_dict: Dict[str, Any]) -> "DelegatedRole": """Creates DelegatedRole object from its dict representation.""" name = role_dict.pop("name") keyids = role_dict.pop("keyids") From b643e5bec7390428de98fc8227ff422a3d1b1584 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 19 May 2021 14:42:18 +0300 Subject: [PATCH 9/9] Metadata API: Add type annotation to help mypy Without this mypy figures the dict is Dict[str, str] and then promptly fails when int value is inserted Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 7e054ba057..c3faafbdd5 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -660,7 +660,10 @@ def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile": def to_dict(self) -> Dict[str, Any]: """Returns the dictionary representation of self.""" - res_dict = {"version": self.version, **self.unrecognized_fields} + res_dict: Dict[str, Any] = { + "version": self.version, + **self.unrecognized_fields, + } if self.length is not None: res_dict["length"] = self.length