You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by ar...@apache.org on 2026/06/09 14:15:19 UTC
(tooling-trusted-releases) branch arm updated (0d862bff -> c81611e5)
This is an automated email from the ASF dual-hosted git repository.
alitheg pushed a change to branch arm
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
from 0d862bff Constrain recipient policy update from API to accepted domain/subdomains
add 19228235 Turn off release deletion to enable testing a bug fix
add 81b1e00e Avoid shadowing a standard library module, and add a lint
add b514a135 Update dependencies
add f4e41861 Test for arbitrary emails
new c81611e5 #1265 - tighten up some parameter types to maintain safe type safety
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.asf.yaml | 3 +
.pre-commit-config.yaml | 2 +-
atr/api/__init__.py | 27 +++--
atr/db/interaction.py | 4 +-
atr/get/checks.py | 8 +-
atr/get/tokens.py | 4 +-
atr/models/api.py | 12 +--
atr/models/args.py | 8 +-
atr/models/sql.py | 10 ++
atr/post/draft.py | 6 +-
atr/post/keys.py | 12 +--
atr/render.py | 10 +-
atr/shared/catalog.py | 6 +-
atr/shared/keys.py | 4 +-
atr/ssh.py | 10 +-
atr/storage/{types.py => datatypes.py} | 0
atr/storage/readers/checks.py | 6 +-
atr/storage/readers/releases.py | 24 ++---
atr/storage/readers/tokens.py | 10 +-
atr/storage/writers/distributions.py | 8 +-
atr/storage/writers/keys.py | 72 ++++++-------
atr/storage/writers/release.py | 58 ++++++-----
atr/storage/writers/revision.py | 14 +--
atr/storage/writers/tokens.py | 14 +--
atr/tasks/gha.py | 12 +--
atr/tasks/inactivity.py | 2 +-
atr/util.py | 4 +-
pip-audit.requirements | 16 +--
pyproject.toml | 4 +
scripts/keys_import.py | 6 +-
tests/unit/test_catalog.py | 12 +--
tests/unit/test_checker_stats.py | 8 +-
tests/unit/test_checks_table.py | 8 +-
tests/unit/test_concern_acknowledgements.py | 10 +-
tests/unit/test_create_revision.py | 14 +--
tests/unit/test_create_revision_quarantine.py | 4 +-
tests/unit/test_inactivity.py | 4 +-
tests/unit/test_keys_writer.py | 14 +--
tests/unit/test_release_writer_hashgen.py | 10 +-
tests/unit/test_safe_json.py | 90 ++++++++++++++++
tests/unit/test_source_artifact_check.py | 22 ++--
tests/unit/test_svn_publish_writer.py | 4 +-
tests/unit/test_tokens_safe.py | 12 +--
uv.lock | 145 +++++++++++++-------------
44 files changed, 421 insertions(+), 312 deletions(-)
rename atr/storage/{types.py => datatypes.py} (100%)
create mode 100644 tests/unit/test_safe_json.py
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org
(tooling-trusted-releases) 01/01: #1265 - tighten up some parameter types to maintain safe type safety
Posted by ar...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alitheg pushed a commit to branch arm
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit c81611e56e0f4ce44ee9dff7cb64742362125fef
Author: Alastair McFarlane <ar...@apache.org>
AuthorDate: Tue Jun 9 15:13:02 2026 +0100
#1265 - tighten up some parameter types to maintain safe type safety
---
atr/api/__init__.py | 11 ++---
atr/db/interaction.py | 4 +-
atr/models/api.py | 12 ++---
atr/models/args.py | 8 ++--
atr/models/sql.py | 10 ++++
atr/shared/catalog.py | 6 +--
atr/storage/writers/distributions.py | 8 ++--
atr/tasks/gha.py | 12 ++---
tests/unit/test_catalog.py | 12 ++---
tests/unit/test_safe_json.py | 90 ++++++++++++++++++++++++++++++++++++
10 files changed, 134 insertions(+), 39 deletions(-)
diff --git a/atr/api/__init__.py b/atr/api/__init__.py
index 4561099d..9a12fcfc 100644
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -96,7 +96,7 @@ async def checks_list(
return models.api.ChecksListResults(
endpoint="/checks/list",
checks=check_results,
- checks_revision=release.unwrap_revision_number,
+ checks_revision=release.safe_latest_revision_number,
current_phase=release.phase,
).model_dump(mode="json"), 200
@@ -139,7 +139,7 @@ async def checks_list_revision(
return models.api.ChecksListResults(
endpoint="/checks/list",
checks=check_results,
- checks_revision=str(revision),
+ checks_revision=revision,
current_phase=release_result.phase,
).model_dump(mode="json"), 200
@@ -216,9 +216,8 @@ async def catalog_project(
)
return models.api.CatalogProjectResults(
endpoint="/catalog/project",
- project=project.key,
+ project=project.safe_key,
cle_url=catalog.cle_project_url(atr_host, project.key),
- grouped=assembled.grouped,
versions=assembled.versions,
cycles=assembled.cycles,
).model_dump(mode="json"), 200
@@ -794,9 +793,9 @@ async def keys_upload(
gc.collect()
raise exceptions.BadRequest(util.PRIVATE_KEY_UPLOAD_WARNING)
asf_uid = _jwt_asf_uid()
- selected_committee_key = data.committee
+ selected_committee_key = safe.CommitteeKey(data.committee)
async with storage.write(asf_uid) as write:
- wacm = write.as_committee_member(selected_committee_key)
+ wacm = write.as_committee_member(str(selected_committee_key))
outcomes: outcome.List[datatypes.Key] = await wacm.keys.ensure_associated(filetext)
# TODO: It would be nice to serialise the actual outcomes
diff --git a/atr/db/interaction.py b/atr/db/interaction.py
index 52a74f44..c639f242 100644
--- a/atr/db/interaction.py
+++ b/atr/db/interaction.py
@@ -105,7 +105,7 @@ class TrustedBallotDetail:
is_binding: bool
is_carried: bool = False
receipt_message_id: str
- revision_number_at_cast: str
+ revision_number_at_cast: safe.RevisionNumber
status_label: str
voter_asf_uid: str
voter_fullname: str
@@ -1087,7 +1087,7 @@ async def _trusted_ballot_details_from_ballots(
is_binding=is_binding,
is_carried=is_carried,
receipt_message_id=ballot.receipt_message_id,
- revision_number_at_cast=ballot.revision_number_at_cast,
+ revision_number_at_cast=ballot.safe_revision_number_at_cast,
status_label=binding_label if is_binding else non_binding_label,
voter_asf_uid=ballot.voter_asf_uid,
voter_fullname=ballot.voter_fullname,
diff --git a/atr/models/api.py b/atr/models/api.py
index 4a8dac02..c8b62bfa 100644
--- a/atr/models/api.py
+++ b/atr/models/api.py
@@ -50,7 +50,7 @@ class CatalogArtifact(schema.Strict):
class CatalogVersion(schema.Strict):
- version: str
+ version: safe.VersionKey
status: Literal["released", "archived"]
released: datetime.datetime | None
svn_revision: int | None
@@ -76,11 +76,9 @@ class CatalogCycle(schema.Strict):
class CatalogProjectResults(schema.Strict):
endpoint: Literal["/catalog/project"] = schema.alias("endpoint")
- project: str
+ project: safe.ProjectKey
# The project-wide CLE feed covering every cycle and release.
cle_url: str | None = None
- # True when the project groups versions into cycles; then read `cycles`, else `versions`.
- grouped: bool
versions: Sequence[CatalogVersion]
cycles: Sequence[CatalogCycle]
@@ -88,7 +86,7 @@ class CatalogProjectResults(schema.Strict):
class ChecksListResults(schema.Strict):
endpoint: Literal["/checks/list"] = schema.alias("endpoint")
checks: Sequence[sql.CheckResult]
- checks_revision: str = schema.example("00005")
+ checks_revision: safe.RevisionNumber = schema.example("00005")
current_phase: sql.ReleasePhase = schema.example(sql.ReleasePhase.RELEASE_CANDIDATE)
@pydantic.field_validator("current_phase", mode="before")
@@ -331,7 +329,7 @@ class KeysUploadResults(schema.Strict):
results: Sequence[KeysUploadResult | KeysUploadException]
success_count: int = schema.example(1)
error_count: int = schema.example(0)
- submitted_committee: str = schema.example("example")
+ submitted_committee: safe.CommitteeKey = schema.example("example")
class KeysUserResults(schema.Strict):
@@ -875,7 +873,7 @@ class TrustedBallotEntry(schema.Strict):
is_binding: bool = schema.example(True)
is_carried: bool = schema.default_example(False, False)
vote_round: int | None = schema.default_example(None, 1)
- revision_number_at_cast: str = schema.example("00005")
+ revision_number_at_cast: safe.RevisionNumber = schema.example("00005")
receipt_message_id: str = schema.example("102ed8a-503db792-79bc789-b8ca87ce@apache.org")
cast_at: datetime.datetime = schema.example(datetime.datetime(2025, 5, 1, 12, 0, tzinfo=datetime.UTC))
diff --git a/atr/models/args.py b/atr/models/args.py
index 7bcec259..8865c219 100644
--- a/atr/models/args.py
+++ b/atr/models/args.py
@@ -34,11 +34,11 @@ class DistributionWorkflow(schema.Strict):
"""Arguments for the task to start a GitHub Actions distribution workflow."""
namespace: str = schema.description("Namespace to distribute to")
- package: str = schema.description("Package to distribute")
- version: str = schema.description("Version to distribute")
+ package: safe.Alphanumeric = schema.description("Package to distribute")
+ version: safe.VersionKey = schema.description("Version to distribute")
staging: bool = schema.description("Whether this is a staging distribution")
- project_key: str = schema.description("Project key in ATR")
- version_key: str = schema.description("Version key in ATR")
+ project_key: safe.ProjectKey = schema.description("Project key in ATR")
+ version_key: safe.VersionKey = schema.description("Version key in ATR")
phase: str = schema.description("Release phase in ATR")
asf_uid: str = schema.description("ASF UID of the user triggering the workflow")
committee_key: str = schema.description("Committee key in ATR")
diff --git a/atr/models/sql.py b/atr/models/sql.py
index 0124059c..37aa8fa3 100644
--- a/atr/models/sql.py
+++ b/atr/models/sql.py
@@ -1578,6 +1578,11 @@ class BallotPaper(sqlmodel.SQLModel, table=True):
if isinstance(self.created, str):
self.created = datetime.datetime.fromisoformat(self.created.rstrip("Z"))
+ @property
+ def safe_revision_number_at_cast(self) -> safe.RevisionNumber:
+ """Get the typesafe validated revision number recorded when the ballot was cast"""
+ return safe.RevisionNumber(self.revision_number_at_cast)
+
__table_args__ = (
sqlalchemy.Index(
"ix_ballotpaper_release_vote_round_voter_id",
@@ -1886,6 +1891,11 @@ class Artifact(sqlmodel.SQLModel, table=True):
# 1-M: Release -> [Artifact]
release: Release | None = sqlmodel.Relationship(back_populates="artifacts")
+ @property
+ def safe_version_key(self) -> safe.VersionKey:
+ """Get the typesafe validated version for the Artifact"""
+ return safe.VersionKey(self.version)
+
# ReleasePolicy: Project
class ReleasePolicy(sqlmodel.SQLModel, table=True):
diff --git a/atr/shared/catalog.py b/atr/shared/catalog.py
index fae37fce..68647c6f 100644
--- a/atr/shared/catalog.py
+++ b/atr/shared/catalog.py
@@ -153,9 +153,9 @@ def _versions(
committee: sql.Committee | None,
atr_host: str | None = None,
) -> list[models.api.CatalogVersion]:
- by_version: dict[str, list[sql.Artifact]] = collections.defaultdict(list)
+ by_version: dict[safe.VersionKey, list[sql.Artifact]] = collections.defaultdict(list)
for artifact in artifacts:
- by_version[artifact.version].append(artifact)
+ by_version[artifact.safe_version_key].append(artifact)
versions: list[models.api.CatalogVersion] = []
for version, rows in by_version.items():
@@ -175,7 +175,7 @@ def _versions(
released=(release.released or release.created) if (release is not None) else None,
svn_revision=max(svn_revisions) if svn_revisions else None,
cycle=cycle.cycle if (cycle is not None) else None,
- cle_url=cle_release_url(atr_host, release.project_key, version)
+ cle_url=cle_release_url(atr_host, release.project_key, str(version))
if (atr_host is not None) and cle_eligible and (release is not None)
else None,
artifacts=[_artifact(row, committee, downloadable) for row in rows],
diff --git a/atr/storage/writers/distributions.py b/atr/storage/writers/distributions.py
index cdcd5e02..853299de 100644
--- a/atr/storage/writers/distributions.py
+++ b/atr/storage/writers/distributions.py
@@ -127,10 +127,10 @@ class CommitteeMember(CommitteeParticipant):
task_args=args.DistributionWorkflow(
name=str(release_key),
namespace=str(owner_namespace) if owner_namespace else "",
- package=str(package),
- version=str(version),
- project_key=str(project_key),
- version_key=str(version_key),
+ package=package,
+ version=version,
+ project_key=project_key,
+ version_key=version_key,
phase=phase,
platform=platform.name,
staging=staging,
diff --git a/atr/tasks/gha.py b/atr/tasks/gha.py
index 94724c25..80f2a70f 100644
--- a/atr/tasks/gha.py
+++ b/atr/tasks/gha.py
@@ -27,7 +27,6 @@ import atr.db as db
import atr.log as log
import atr.models.args as args
import atr.models.results as results
-import atr.models.safe as safe
import atr.models.sql as sql
import atr.storage as storage
import atr.tasks as tasks
@@ -118,8 +117,7 @@ async def trigger_workflow(
task_args: args.DistributionWorkflow, *, task_id: int | None = None
) -> results.Results | None:
unique_id = f"atr-dist-{task_args.name}-{uuid.uuid4()}"
- project = safe.ProjectKey(task_args.project_key)
- safe.VersionKey(task_args.version_key)
+ project = task_args.project_key
try:
sql_platform = sql.DistributionPlatform[task_args.platform]
except KeyError:
@@ -131,12 +129,12 @@ async def trigger_workflow(
"atr-id": unique_id,
"asf-uid": task_args.asf_uid,
"task_id": task_id,
- "project": task_args.project_key,
+ "project": str(task_args.project_key),
"phase": task_args.phase,
- "version": task_args.version_key,
+ "version": str(task_args.version_key),
"distribution-owner-namespace": task_args.namespace,
- "distribution-package": task_args.package,
- "distribution-version": task_args.version,
+ "distribution-package": str(task_args.package),
+ "distribution-version": str(task_args.version),
# **task_args.arguments,
},
}
diff --git a/tests/unit/test_catalog.py b/tests/unit/test_catalog.py
index e89441f9..3d73717f 100644
--- a/tests/unit/test_catalog.py
+++ b/tests/unit/test_catalog.py
@@ -109,7 +109,7 @@ def test_status_reflects_release_and_archive_state() -> None:
_artifact(project, "4.1.7", "a-4.1.7.tar.gz", release=archived),
]
- versions = {v.version: v for v in catalog._versions(artifacts, {}, project.committee_key)}
+ versions = {str(v.version): v for v in catalog._versions(artifacts, {}, project.committee_key)}
assert versions["5.0.2"].status == "released"
assert versions["4.1.7"].status == "archived"
@@ -124,7 +124,7 @@ def test_only_released_versions_are_downloadable() -> None:
_artifact(project, "4.1.7", "a-4.1.7.tar.gz", release=archived, svn_revision=28114),
]
- versions = {v.version: v for v in catalog._versions(artifacts, {}, project.committee_key)}
+ versions = {str(v.version): v for v in catalog._versions(artifacts, {}, project.committee_key)}
assert versions["5.0.2"].artifacts[0].downloadable is True
assert versions["4.1.7"].artifacts[0].downloadable is False
@@ -146,7 +146,7 @@ def test_versions_are_newest_first() -> None:
versions = catalog._versions(artifacts, {}, project.committee_key)
- assert [v.version for v in versions] == ["5.0.2", "5.0.1"]
+ assert [str(v.version) for v in versions] == ["5.0.2", "5.0.1"]
def test_cycles_are_ordered_by_latest_activity() -> None:
@@ -188,7 +188,7 @@ def test_cle_url_links_versions_backed_by_a_release_when_a_host_is_supplied() ->
released = _release(project, "5.0.2", cycle_key="cassandra-default", released=_NOW)
artifacts = [_artifact(project, "5.0.2", "a-5.0.2.tar.gz", release=released)]
- versions = {v.version: v for v in catalog._versions(artifacts, {}, project.committee_key, "atr.example.org")}
+ versions = {str(v.version): v for v in catalog._versions(artifacts, {}, project.committee_key, "atr.example.org")}
assert versions["5.0.2"].cle_url == "https://atr.example.org/api/cle/release/cassandra/5.0.2"
@@ -197,7 +197,7 @@ def test_cle_url_is_omitted_for_versions_without_a_backing_release() -> None:
project = _project()
artifacts = [_artifact(project, "3.0.0", "a-3.0.0.tar.gz", svn_revision=100)]
- versions = {v.version: v for v in catalog._versions(artifacts, {}, project.committee_key, "atr.example.org")}
+ versions = {str(v.version): v for v in catalog._versions(artifacts, {}, project.committee_key, "atr.example.org")}
assert versions["3.0.0"].cle_url is None
@@ -208,6 +208,6 @@ def test_cle_url_is_omitted_when_no_host_is_supplied() -> None:
released = _release(project, "5.0.2", cycle_key="cassandra-default", released=_NOW)
artifacts = [_artifact(project, "5.0.2", "a-5.0.2.tar.gz", release=released)]
- versions = {v.version: v for v in catalog._versions(artifacts, {}, project.committee_key)}
+ versions = {str(v.version): v for v in catalog._versions(artifacts, {}, project.committee_key)}
assert versions["5.0.2"].cle_url is None
diff --git a/tests/unit/test_safe_json.py b/tests/unit/test_safe_json.py
new file mode 100644
index 00000000..dafecb42
--- /dev/null
+++ b/tests/unit/test_safe_json.py
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import json
+import pathlib
+
+import sqlalchemy.dialects.sqlite as sqlite
+
+import atr.models.safe as safe
+import atr.models.sql as sql
+
+# SafeJSON ignores the dialect, but its signature wants one, so hand it the
+# SQLite dialect the app actually runs on.
+_DIALECT = sqlite.dialect()
+
+# A valid construction example for every concrete safe type. The completeness
+# test below fails if a new safe type is added without an entry here, so the
+# round-trip coverage can't quietly fall behind the type tree.
+_SAFE_TYPE_EXAMPLES: dict[type[safe.SafeType], str] = {
+ safe.Alphanumeric: "abc-123",
+ safe.OwnerNamespace: "com.example",
+ safe.AsfUid: "user1",
+ safe.CommitteeKey: "tooling",
+ safe.Numeric: "12345",
+ safe.ProjectKey: "tooling",
+ safe.ReleaseKey: "tooling-0.0.1",
+ safe.RelPath: "dir/file.txt",
+ safe.RevisionNumber: "00001",
+ safe.VersionKey: "0.0.1",
+}
+
+# StatePath isn't a SafeType subclass and carries a managed root, so it's built
+# and checked on its own rather than from the example table.
+_STATE_ROOT = pathlib.Path("/srv/atr/state")
+_STATE_PATH = safe.StatePath(_STATE_ROOT / "releases" / "example", _STATE_ROOT)
+
+
+def _concrete_safe_types() -> set[type[safe.SafeType]]:
+ # Walk the whole SafeType tree. The base SafeType has an empty valid-char
+ # set and isn't used directly, so the leaves and intermediate concrete
+ # types (Alphanumeric, Numeric) are what we care about.
+ found: set[type[safe.SafeType]] = set()
+ stack = list(safe.SafeType.__subclasses__())
+ while stack:
+ cls = stack.pop()
+ found.add(cls)
+ stack.extend(cls.__subclasses__())
+ return found
+
+
+def test_every_safe_type_has_a_serialisation_example() -> None:
+ missing = _concrete_safe_types() - set(_SAFE_TYPE_EXAMPLES)
+ assert not missing, f"New safe type(s) without SafeJSON round-trip coverage: {sorted(c.__name__ for c in missing)}"
+
+
+def test_all_safe_types_round_trip_through_safe_json() -> None:
+ # Build a dict holding one instance of every safe type, then put it through
+ # the SafeJSON write/read cycle the task_args column uses.
+ values: dict[str, object] = {cls.__name__: cls(example) for cls, example in _SAFE_TYPE_EXAMPLES.items()}
+ values["StatePath"] = _STATE_PATH
+
+ encoded = sql.SafeJSON().process_bind_param(values, _DIALECT)
+
+ # The point of the test: nothing safe-typed leaks past the write boundary,
+ # so the encoded form is plain JSON that the stdlib encoder can handle.
+ assert json.loads(json.dumps(encoded)) == encoded
+
+ decoded = sql.SafeJSON().process_result_value(encoded, _DIALECT)
+ assert decoded is not None
+
+ # Plain safe types come back as their string form (Pydantic re-types them
+ # when a model loads the args); only StatePath rebuilds itself, root and all.
+ for cls, example in _SAFE_TYPE_EXAMPLES.items():
+ assert decoded[cls.__name__] == str(cls(example))
+ assert isinstance(decoded["StatePath"], safe.StatePath)
+ assert decoded["StatePath"] == _STATE_PATH
+ assert decoded["StatePath"].root == _STATE_PATH.root
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org