From e2618f1a9b71d2d9f90c3df1c8076109a4cedb1a Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 13 Aug 2024 14:24:45 -0400 Subject: [PATCH 1/7] impl: make Publisher a RootModel This allows us to use Pydantic's model APIs directly, instead of going through a `TypeAdapter`. This changeset also enables typechecking on our unit tests. Signed-off-by: William Woodruff --- Makefile | 2 +- pyproject.toml | 2 ++ src/pypi_attestations/_impl.py | 4 ++-- test/test_impl.py | 33 ++++++++++++++++++--------------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index 0f4cccd..a927b27 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ lint: $(VENV)/pyvenv.cfg . $(VENV_BIN)/activate && \ ruff format --check $(ALL_PY_SRCS) && \ ruff check $(ALL_PY_SRCS) && \ - mypy + mypy src/ test/ . $(VENV_BIN)/activate && \ interrogate -c pyproject.toml . diff --git a/pyproject.toml b/pyproject.toml index e0bdaee..0901cf5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,7 @@ omit = ["src/pypi_attestations/_cli.py", "src/pypi_attestations/__main__.py"] [tool.mypy] mypy_path = "src" packages = "pypi_attestations" +plugins = ["pydantic.mypy"] allow_redefinition = true check_untyped_defs = true disallow_incomplete_defs = true @@ -88,6 +89,7 @@ ignore = ["ANN101", "ANN102", "D203", "D213", "COM812", "ISC001"] "D", # no docstrings in tests "S101", # asserts are expected in tests "SLF001", # private APIs are expected in tests + "ANN401", # dynamic types are OK in tests ] [tool.interrogate] diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index d7346d3..209b986 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -14,7 +14,7 @@ from cryptography import x509 from cryptography.hazmat.primitives import serialization from packaging.utils import parse_sdist_filename, parse_wheel_filename -from pydantic import Base64Bytes, BaseModel, ConfigDict, Field, field_validator +from pydantic import Base64Bytes, BaseModel, ConfigDict, Field, RootModel, field_validator from pydantic.alias_generators import to_snake from pydantic_core import ValidationError from sigstore._utils import _sha256_streaming @@ -383,7 +383,7 @@ class GitLabPublisher(_PublisherBase): """ -Publisher = Annotated[GitHubPublisher | GitLabPublisher, Field(discriminator="kind")] +Publisher = RootModel[Annotated[GitHubPublisher | GitLabPublisher, Field(discriminator="kind")]] class AttestationBundle(BaseModel): diff --git a/test/test_impl.py b/test/test_impl.py index 538df1c..090577f 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -4,12 +4,13 @@ import os from hashlib import sha256 from pathlib import Path +from typing import Any import pretend import pypi_attestations._impl as impl import pytest import sigstore -from pydantic import TypeAdapter, ValidationError +from pydantic import ValidationError from sigstore.dsse import DigestSet, StatementBuilder, Subject from sigstore.models import Bundle from sigstore.oidc import IdentityToken @@ -75,7 +76,7 @@ def test_roundtrip(self, id_token: IdentityToken) -> None: def test_wrong_predicate_raises_exception(self, monkeypatch: pytest.MonkeyPatch) -> None: def dummy_predicate(self_: StatementBuilder, _: str) -> StatementBuilder: # wrong type here to have a validation error - self_._predicate_type = False + self_._predicate_type = False # type: ignore[assignment] return self_ monkeypatch.setattr(sigstore.dsse.StatementBuilder, "predicate_type", dummy_predicate) @@ -100,7 +101,7 @@ def in_validity_period(_: IdentityToken) -> bool: def test_multiple_signatures( self, id_token: IdentityToken, monkeypatch: pytest.MonkeyPatch ) -> None: - def get_bundle(*_) -> Bundle: # noqa: ANN002 + def get_bundle(*_: Any) -> Bundle: # noqa: ANN002 # Duplicate the signature to trigger a Conversion error bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) bundle._inner.dsse_envelope.signatures.append(bundle._inner.dsse_envelope.signatures[0]) @@ -468,19 +469,21 @@ def test_ultranormalize_dist_filename_invalid(input: str) -> None: class TestPublisher: def test_discriminator(self) -> None: gh_raw = {"kind": "GitHub", "repository": "foo/bar", "workflow": "publish.yml"} - gh = TypeAdapter(impl.Publisher).validate_python(gh_raw) + gh = impl.Publisher.model_validate(gh_raw) - assert isinstance(gh, impl.GitHubPublisher) - assert gh.repository == "foo/bar" - assert gh.workflow == "publish.yml" - assert TypeAdapter(impl.Publisher).validate_json(json.dumps(gh_raw)) == gh + assert isinstance(gh, impl.Publisher) + assert isinstance(gh.root, impl.GitHubPublisher) + assert gh.root.repository == "foo/bar" + assert gh.root.workflow == "publish.yml" + assert impl.Publisher.model_validate_json(json.dumps(gh_raw)) == gh gl_raw = {"kind": "GitLab", "repository": "foo/bar/baz", "environment": "publish"} - gl = TypeAdapter(impl.Publisher).validate_python(gl_raw) - assert isinstance(gl, impl.GitLabPublisher) - assert gl.repository == "foo/bar/baz" - assert gl.environment == "publish" - assert TypeAdapter(impl.Publisher).validate_json(json.dumps(gl_raw)) == gl + gl = impl.Publisher.model_validate(gl_raw) + assert isinstance(gl, impl.Publisher) + assert isinstance(gl.root, impl.GitLabPublisher) + assert gl.root.repository == "foo/bar/baz" + assert gl.root.environment == "publish" + assert impl.Publisher.model_validate_json(json.dumps(gl_raw)) == gl def test_wrong_kind(self) -> None: with pytest.raises(ValueError, match="Input should be 'GitHub'"): @@ -499,9 +502,9 @@ def test_claims(self) -> None: "this-too": 123, }, } - pub = TypeAdapter(impl.Publisher).validate_python(raw) + pub = impl.Publisher.model_validate(raw) - assert pub.claims == { + assert pub.root.claims == { "this": "is-preserved", "this-too": 123, } From bd1542c158d87ccd8b3c4ecf2dd5a02005a89d46 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 13 Aug 2024 14:26:40 -0400 Subject: [PATCH 2/7] CHANGELOG: record changes Signed-off-by: William Woodruff --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1373f9e..57cec9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- The type of `Publisher` has changed to `RootModel[...]` + ([#43](https://github.com/trailofbits/pypi-attestations/pull/43)) + ## [0.0.10] ### Changed From 076dfeef33033b3527734f2d4a30a406fa363ce9 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 13 Aug 2024 14:48:28 -0400 Subject: [PATCH 3/7] pyproject: fix lint extra Signed-off-by: William Woodruff --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 0901cf5..f3e89b1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,6 +34,8 @@ lint = [ "types-requests", "types-toml", "interrogate", + # linting relies on test deps, since we also typecheck our test suite + "pypi-attestations[test]", ] dev = ["pypi-attestations[doc,test,lint]", "twine", "wheel", "build"] From 9b376cdfb596029bc84075553135fc321786c415 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 13 Aug 2024 14:49:10 -0400 Subject: [PATCH 4/7] pyproject: remove old dev deps Signed-off-by: William Woodruff --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f3e89b1..84f23d4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ lint = [ # linting relies on test deps, since we also typecheck our test suite "pypi-attestations[test]", ] -dev = ["pypi-attestations[doc,test,lint]", "twine", "wheel", "build"] +dev = ["pypi-attestations[doc,test,lint]", "build"] [project.urls] From 406593a51411121837bae1c892f0831c9c37c66b Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Wed, 14 Aug 2024 11:20:36 -0400 Subject: [PATCH 5/7] Update test/test_impl.py Co-authored-by: dm --- test/test_impl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_impl.py b/test/test_impl.py index 090577f..50c7087 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -101,7 +101,7 @@ def in_validity_period(_: IdentityToken) -> bool: def test_multiple_signatures( self, id_token: IdentityToken, monkeypatch: pytest.MonkeyPatch ) -> None: - def get_bundle(*_: Any) -> Bundle: # noqa: ANN002 + def get_bundle(*_: Any) -> Bundle: # Duplicate the signature to trigger a Conversion error bundle = Bundle.from_json(gh_signed_dist_bundle_path.read_bytes()) bundle._inner.dsse_envelope.signatures.append(bundle._inner.dsse_envelope.signatures[0]) From 58f0a246b4d48185804acfb57dbccb3eb98a2a2e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Wed, 14 Aug 2024 17:08:02 -0400 Subject: [PATCH 6/7] drop RootModel Signed-off-by: William Woodruff --- src/pypi_attestations/_impl.py | 4 ++-- test/test_impl.py | 28 +++++++++++++--------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/pypi_attestations/_impl.py b/src/pypi_attestations/_impl.py index 209b986..d7346d3 100644 --- a/src/pypi_attestations/_impl.py +++ b/src/pypi_attestations/_impl.py @@ -14,7 +14,7 @@ from cryptography import x509 from cryptography.hazmat.primitives import serialization from packaging.utils import parse_sdist_filename, parse_wheel_filename -from pydantic import Base64Bytes, BaseModel, ConfigDict, Field, RootModel, field_validator +from pydantic import Base64Bytes, BaseModel, ConfigDict, Field, field_validator from pydantic.alias_generators import to_snake from pydantic_core import ValidationError from sigstore._utils import _sha256_streaming @@ -383,7 +383,7 @@ class GitLabPublisher(_PublisherBase): """ -Publisher = RootModel[Annotated[GitHubPublisher | GitLabPublisher, Field(discriminator="kind")]] +Publisher = Annotated[GitHubPublisher | GitLabPublisher, Field(discriminator="kind")] class AttestationBundle(BaseModel): diff --git a/test/test_impl.py b/test/test_impl.py index 50c7087..5afe8f5 100644 --- a/test/test_impl.py +++ b/test/test_impl.py @@ -10,7 +10,7 @@ import pypi_attestations._impl as impl import pytest import sigstore -from pydantic import ValidationError +from pydantic import TypeAdapter, ValidationError from sigstore.dsse import DigestSet, StatementBuilder, Subject from sigstore.models import Bundle from sigstore.oidc import IdentityToken @@ -469,21 +469,19 @@ def test_ultranormalize_dist_filename_invalid(input: str) -> None: class TestPublisher: def test_discriminator(self) -> None: gh_raw = {"kind": "GitHub", "repository": "foo/bar", "workflow": "publish.yml"} - gh = impl.Publisher.model_validate(gh_raw) + gh: impl.Publisher = TypeAdapter(impl.Publisher).validate_python(gh_raw) - assert isinstance(gh, impl.Publisher) - assert isinstance(gh.root, impl.GitHubPublisher) - assert gh.root.repository == "foo/bar" - assert gh.root.workflow == "publish.yml" - assert impl.Publisher.model_validate_json(json.dumps(gh_raw)) == gh + assert isinstance(gh, impl.GitHubPublisher) + assert gh.repository == "foo/bar" + assert gh.workflow == "publish.yml" + assert TypeAdapter(impl.Publisher).validate_json(json.dumps(gh_raw)) == gh gl_raw = {"kind": "GitLab", "repository": "foo/bar/baz", "environment": "publish"} - gl = impl.Publisher.model_validate(gl_raw) - assert isinstance(gl, impl.Publisher) - assert isinstance(gl.root, impl.GitLabPublisher) - assert gl.root.repository == "foo/bar/baz" - assert gl.root.environment == "publish" - assert impl.Publisher.model_validate_json(json.dumps(gl_raw)) == gl + gl: impl.Publisher = TypeAdapter(impl.Publisher).validate_python(gl_raw) + assert isinstance(gl, impl.GitLabPublisher) + assert gl.repository == "foo/bar/baz" + assert gl.environment == "publish" + assert TypeAdapter(impl.Publisher).validate_json(json.dumps(gl_raw)) == gl def test_wrong_kind(self) -> None: with pytest.raises(ValueError, match="Input should be 'GitHub'"): @@ -502,9 +500,9 @@ def test_claims(self) -> None: "this-too": 123, }, } - pub = impl.Publisher.model_validate(raw) + pub: impl.Publisher = TypeAdapter(impl.Publisher).validate_python(raw) - assert pub.root.claims == { + assert pub.claims == { "this": "is-preserved", "this-too": 123, } From 03c393ad648e95b0ed38af0a7f8d03604308d72e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Wed, 14 Aug 2024 17:08:16 -0400 Subject: [PATCH 7/7] CHANGELOG: undo Signed-off-by: William Woodruff --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57cec9f..1373f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Changed - -- The type of `Publisher` has changed to `RootModel[...]` - ([#43](https://github.com/trailofbits/pypi-attestations/pull/43)) - ## [0.0.10] ### Changed