You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by sb...@apache.org on 2026/09/23 19:16:27 UTC
(tooling-trusted-releases) branch sbp updated: Use compose form commit hashes for source comparison
This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/sbp by this push:
new d560e313 Use compose form commit hashes for source comparison
d560e313 is described below
commit d560e313431b24c023b4eb77fec98899e551b697
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Wed Sep 23 20:15:53 2026 +0100
Use compose form commit hashes for source comparison
---
atr/attestable.py | 2 +
atr/get/compose.py | 23 +++---
atr/models/attestable.py | 11 +++
atr/models/safe.py | 5 +-
atr/post/compose.py | 10 ---
atr/shared/compose.py | 3 +-
atr/source.py | 102 +++++++++++++++++++++++
atr/storage/writers/release.py | 44 ++++------
atr/storage/writers/revision.py | 27 ++++++-
atr/tasks/__init__.py | 5 +-
atr/tasks/checks/compare.py | 33 ++------
tests/unit/test_checks_compare.py | 19 ++++-
tests/unit/test_compose_polling.py | 24 ++++++
tests/unit/test_create_revision.py | 9 ++-
tests/unit/test_release_writer_commit_hash.py | 79 +++++++++++++++++-
tests/unit/test_safe_json.py | 2 +-
tests/unit/test_safe_types.py | 8 +-
tests/unit/test_source.py | 111 ++++++++++++++++++++++++++
18 files changed, 423 insertions(+), 94 deletions(-)
diff --git a/atr/attestable.py b/atr/attestable.py
index e39d10cc..8f002893 100644
--- a/atr/attestable.py
+++ b/atr/attestable.py
@@ -525,6 +525,7 @@ async def write_files_data(
effective_path_provenance: dict[str, models.ProvenanceV2] | None = None,
swhid_dirs: dict[str, str] | None = None,
sha3_hashes: dict[str, str] | None = None,
+ source: models.SourceV2 | None = None,
) -> None:
result = await _generate_files_data(
path_to_hash,
@@ -539,6 +540,7 @@ async def write_files_data(
swhid_dirs=swhid_dirs,
sha3_hashes=sha3_hashes,
)
+ result.source = source
file_path = attestable_path(project_key, version_key, revision_number)
await _atomic_write_readonly(file_path.path, result.model_dump_json(indent=2))
checks_file_path = attestable_checks_path(project_key, version_key, revision_number)
diff --git a/atr/get/compose.py b/atr/get/compose.py
index 4ed6fa8b..3b86795d 100644
--- a/atr/get/compose.py
+++ b/atr/get/compose.py
@@ -26,7 +26,6 @@ import htpy
import quart
from quart_wtf import utils
-import atr.attestable as attestable
import atr.blueprints.get as get
import atr.db as db
import atr.db.interaction as interaction
@@ -44,6 +43,7 @@ import atr.render as render
import atr.shared as shared
import atr.shared.activity as activity
import atr.shared.draft as draft
+import atr.source as source
import atr.storage as storage
import atr.strings as strings
import atr.template as template
@@ -298,21 +298,22 @@ async def _commit_hash_form_html(release: sql.Release) -> str:
action = util.as_url(
post.compose.selected, project_key=release.safe_project_key, version_key=release.safe_version_key
)
- payload = await attestable.latest_github_tp_payload(release.safe_project_key, release.safe_version_key)
- if payload is not None:
- # GitHub has attested the commit, so we show it locked rather than as an editable form
- locked = htm.div(".mb-3")[
- htpy.label(".form-label")["Commit hash"],
- htpy.input(type="text", class_="form-control", value=payload.sha, readonly=True, disabled=True),
- htm.div(".form-text")["Set by GitHub via Trusted Publishing; it cannot be changed here."],
- ]
- return str(locked)
+ selected_source = await source.current(release)
+ details = htm.Block(classes=".mb-3")
+ if selected_source.sha:
+ basis = "Explicitly supplied" if selected_source.override else "Assumed from workflow"
+ details.append(htpy.p[f"{basis}: ", htpy.code[selected_source.sha]])
+ if selected_source.repository:
+ details.append(htpy.p[f"Source repository: {selected_source.repository}"])
+ else:
+ details.append(htpy.p["Configure one GitHub repository for this project to compare its source commit."])
commit_hash_form = await form.render(
model_cls=shared.compose.SetCommitHashForm,
action=action,
submit_label="Save commit hash",
submit_classes="btn-outline-primary",
- defaults={"commit_hash": release.commit_hash or ""},
+ pre_submit=details.collect(),
+ defaults={"commit_hash": selected_source.override or ""},
)
return str(commit_hash_form)
diff --git a/atr/models/attestable.py b/atr/models/attestable.py
index bf82b214..48d74af7 100644
--- a/atr/models/attestable.py
+++ b/atr/models/attestable.py
@@ -71,11 +71,22 @@ class PathEntryV2(schema.Strict):
provenance: ProvenanceV2 | None = None
+class SourceV2(schema.Strict):
+ repository: str = ""
+ default: str | None = None
+ override: str | None = None
+
+ @property
+ def sha(self) -> str:
+ return self.override or self.default or ""
+
+
class AttestableV2(schema.Strict):
version: Literal[2] = 2
hashes: dict[str, HashEntryV2] = schema.factory(dict)
paths: dict[str, PathEntryV2] = schema.factory(dict)
policy: dict[str, Any] = schema.factory(dict)
+ source: SourceV2 | None = None
# Attestable, any version
diff --git a/atr/models/safe.py b/atr/models/safe.py
index 7b447e4f..5deff2a9 100644
--- a/atr/models/safe.py
+++ b/atr/models/safe.py
@@ -215,9 +215,8 @@ class CommitHash(SafeType):
return _HEX_LOWER
def _additional_validations(self, value: str) -> None:
- # Wide enough for a short hash through to a full SHA-256 object name
- if not (7 <= len(value) <= 64):
- raise ValueError("A commit hash must be between 7 and 64 hexadecimal characters")
+ if len(value) != 40:
+ raise ValueError("A commit hash must be exactly 40 hexadecimal characters")
class CommitteeKey(Alphanumeric):
diff --git a/atr/post/compose.py b/atr/post/compose.py
index 3603cb55..72fb678e 100644
--- a/atr/post/compose.py
+++ b/atr/post/compose.py
@@ -19,7 +19,6 @@ from typing import Literal
import quart
-import atr.attestable as attestable
import atr.blueprints.post as post
import atr.errors as errors
import atr.get as get
@@ -57,15 +56,6 @@ async def _set_commit_hash(
commit_hash: safe.CommitHash | None,
) -> web.WerkzeugResponse:
submitted = str(commit_hash) if (commit_hash is not None) else None
- # Trusted Publishing is authoritative, so a hand-entered value has to agree with its attestation
- payload = await attestable.latest_github_tp_payload(project_key, version_key)
- if (payload is not None) and (submitted != payload.sha):
- return await session.redirect(
- get.compose.selected,
- project_key=str(project_key),
- version_key=str(version_key),
- error="The commit hash was set via Trusted Publishing and cannot be edited.",
- )
try:
async with storage.write(session) as write:
wacp = await write.as_project_committee_participant(project_key)
diff --git a/atr/shared/compose.py b/atr/shared/compose.py
index c9a4ef90..e9fc5495 100644
--- a/atr/shared/compose.py
+++ b/atr/shared/compose.py
@@ -52,7 +52,8 @@ class SetCommitHashForm(form.Form):
variant: SET_COMMIT_HASH = form.value(SET_COMMIT_HASH)
commit_hash: safe.OptionalCommitHash = form.label(
"Commit hash",
- "The source commit hash for this release. Disabled when GitHub has set it via Trusted Publishing.",
+ "Enter the full 40-character source commit for the displayed repository. "
+ "Leave blank to use the workflow default, if available.",
widget=form.Widget.TEXT,
default="",
)
diff --git a/atr/source.py b/atr/source.py
new file mode 100644
index 00000000..d10c92da
--- /dev/null
+++ b/atr/source.py
@@ -0,0 +1,102 @@
+# 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.
+
+from __future__ import annotations
+
+import re
+
+import atr.attestable as attestable
+import atr.db as db
+import atr.models.attestable
+import atr.models.github as github
+import atr.models.safe as safe
+import atr.models.sql as sql
+
+
+def advance(
+ source: atr.models.attestable.SourceV2,
+ payload: github.TrustedPublisherPayload | None,
+ override: db.Opt[str | None] = db.NOT_SET,
+) -> atr.models.attestable.SourceV2:
+ source = source.model_copy()
+ if (payload is not None) and ((not source.override) or (source.repository == payload.repository)):
+ source.repository = payload.repository
+ source.default = payload.sha
+ if not isinstance(override, db.NotSet):
+ if override and (not source.repository):
+ raise ValueError("Configure a GitHub repository for this project before setting its source commit.")
+ source.override = override
+ return source
+
+
+async def current(release: sql.Release) -> atr.models.attestable.SourceV2:
+ previous = None
+ if release.latest_revision_number is not None:
+ previous = await attestable.load(
+ release.safe_project_key, release.safe_version_key, release.safe_latest_revision_number
+ )
+ return await initial(release, previous)
+
+
+async def initial(
+ release: sql.Release, previous: atr.models.attestable.Attestable | None
+) -> atr.models.attestable.SourceV2:
+ if isinstance(previous, atr.models.attestable.AttestableV2) and (previous.source is not None):
+ source = previous.source.model_copy()
+ if not source.repository:
+ source.repository = repository(release.project)
+ return source
+ source = atr.models.attestable.SourceV2(repository=repository(release.project))
+ if not release.commit_hash:
+ return source
+ payload = await attestable.latest_github_tp_payload(release.safe_project_key, release.safe_version_key)
+ if payload is not None:
+ source.repository = payload.repository
+ source.default = payload.sha
+ if release.commit_hash != source.default:
+ source.override = release.commit_hash
+ return source
+
+
+async def inputs(
+ project_key: safe.ProjectKey, version_key: safe.VersionKey, revision_number: safe.RevisionNumber
+) -> dict[str, str]:
+ source = await read(project_key, version_key, revision_number)
+ return {"source_repository": source.repository, "source_commit": source.sha}
+
+
+async def read(
+ project_key: safe.ProjectKey, version_key: safe.VersionKey, revision_number: safe.RevisionNumber
+) -> atr.models.attestable.SourceV2:
+ recorded = await attestable.load(project_key, version_key, revision_number)
+ if isinstance(recorded, atr.models.attestable.AttestableV2) and (recorded.source is not None):
+ return recorded.source
+ payload = await attestable.github_tp_payload_read(project_key, version_key, revision_number)
+ if payload is not None:
+ return atr.models.attestable.SourceV2(repository=payload.repository, default=payload.sha)
+ return atr.models.attestable.SourceV2()
+
+
+def repository(project: sql.Project) -> str:
+ if name := project.policy_github_repository_name:
+ return "apache/" + name
+ repositories = {
+ match[1].removesuffix(".git")
+ for url in project.repositories
+ if (match := re.fullmatch(r"https://github\.com/([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)/?", url))
+ }
+ return repositories.pop() if (len(repositories) == 1) else ""
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index e3fdb34a..74a10a33 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -506,35 +506,25 @@ class CommitteeParticipant(FoundationCommitter):
version_key: safe.VersionKey,
commit_hash: str | None,
) -> sql.Release:
- # Shared by the Trusted Publishing auto-fill and the manual recording form
- release = await self.__data.release(
- project_key=str(project_key), version=str(version_key), _committee=True, _project=True
- ).demand(storage.AccessError(f"Release '{project_key!s} {version_key!s}' not found.", status=404))
- storage.ensure_project_active(release.project)
- self.__write.ensure_release_writable(release)
- previous_commit_hash = release.commit_hash
- via = sql.validate_instrumented_attribute
- result = await self.__data.execute(
- sqlmodel.update(sql.Release)
- .where(
- via(sql.Release.key) == release.key,
- via(sql.Release.phase) == sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
+ try:
+ if commit_hash is not None:
+ commit_hash = str(safe.CommitHash(commit_hash))
+ await self.__write_as.revision.create_revision_with_quarantine(
+ project_key,
+ version_key,
+ self.__asf_uid,
+ allowed_phases=frozenset({sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT}),
+ description="Source commit updated through Compose",
+ source_override=commit_hash,
)
- .values(commit_hash=commit_hash)
- )
- if getattr(result, "rowcount", 0) != 1:
- await self.__data.rollback()
- raise storage.AccessError("The release state has changed, please refresh and try again", status=409)
- await self.__data.refresh(release)
- await self.__data.commit()
- self.__write_as.append_to_audit_log(
- asf_uid=self.__asf_uid,
- project_key=str(project_key),
- version=str(version_key),
- previous_commit_hash=previous_commit_hash,
- commit_hash=commit_hash,
+ except datatypes.FailedError as e:
+ raise storage.AccessError(str(e), status=409) from e
+ except ValueError as e:
+ raise storage.AccessError(str(e), status=400) from e
+ self.__data.expire_all()
+ return await self.__data.release(project_key=str(project_key), version=str(version_key)).demand(
+ storage.AccessError("Release not found", status=404)
)
- return release
async def delete(
self,
diff --git a/atr/storage/writers/revision.py b/atr/storage/writers/revision.py
index 53cba48f..0b5c0232 100644
--- a/atr/storage/writers/revision.py
+++ b/atr/storage/writers/revision.py
@@ -42,6 +42,7 @@ import atr.models.attestable
import atr.models.safe as safe
import atr.models.sql as sql
import atr.paths as paths
+import atr.source as source
import atr.storage as storage
import atr.storage.datatypes as datatypes
import atr.tasks as tasks
@@ -160,8 +161,14 @@ async def _commit_new_revision(
extracted_swhids: dict[str, str] | None = None,
sha3_hashes: dict[str, str] | None = None,
github_payload: github.TrustedPublisherPayload | None = None,
+ source_override: db.Opt[str | None] = db.NOT_SET,
) -> sql.Revision:
try:
+ source_record = source.advance(
+ await source.initial(release, previous_attestable),
+ github_payload,
+ source_override,
+ )
# This is the only place where models.Revision is constructed
# That makes models.populate_revision_sequence_and_name safe against races
# Because that event is called when data.add is called below
@@ -236,12 +243,13 @@ async def _commit_new_revision(
effective_path_provenance=effective_provenance,
swhid_dirs=swhid_dirs,
sha3_hashes=sha3_hashes,
+ source=source_record,
)
previous_commit_hash = release.commit_hash
if github_payload is not None:
await attestable.github_tp_payload_write(project_key, version_key, new_revision.safe_number, github_payload)
- release.commit_hash = github_payload.sha
+ release.commit_hash = source_record.sha or None
if attestable.can_write_file_state_rows(previous_attestable, new_revision.parent_key):
for row in attestable.compute_file_state_rows(
@@ -269,14 +277,14 @@ async def _commit_new_revision(
was_quarantined=was_quarantined,
)
- if github_payload is not None:
+ if (release.commit_hash != previous_commit_hash) or (not isinstance(source_override, db.NotSet)):
storage.audit(
action="atr.storage.writers.release.CommitteeParticipant.set_commit_hash",
asf_uid=asf_uid,
project_key=str(project_key),
version=str(version_key),
previous_commit_hash=previous_commit_hash,
- commit_hash=github_payload.sha,
+ commit_hash=release.commit_hash,
)
async with data.begin():
@@ -480,6 +488,7 @@ class CommitteeParticipant(FoundationCommitter):
clone_from: safe.RevisionNumber | None = None,
expected_revision: safe.RevisionNumber | None = None,
github_payload: github.TrustedPublisherPayload | None = None,
+ source_override: db.Opt[str | None] = db.NOT_SET,
) -> sql.Revision | sql.Quarantined:
"""Create a new revision, quarantining archives that require validation."""
async with db.session() as data:
@@ -605,6 +614,14 @@ class CommitteeParticipant(FoundationCommitter):
f"allowed: {', '.join(sorted(p.value for p in allowed_phases))}"
)
+ if (
+ (not isinstance(source_override, db.NotSet))
+ and (old_revision is None)
+ and (prior_revision_key is not None)
+ ):
+ await aioshutil.rmtree(temp_dir)
+ raise datatypes.RevisionMismatchError("The release has changed, please refresh and try again")
+
if expected_revision is not None:
expected_key = sql.revision_key(release_key, str(expected_revision))
if prior_revision_key != expected_key:
@@ -631,6 +648,9 @@ class CommitteeParticipant(FoundationCommitter):
archive_paths = detection.detect_archives_requiring_quarantine(path_to_hash, previous_attestable)
if archive_paths:
+ if not isinstance(source_override, db.NotSet):
+ await aioshutil.rmtree(temp_dir)
+ raise datatypes.FailedError("Source archives need validation before the source commit can be set.")
deduped = detection.deduplicate_quarantine_archives(archive_paths, path_to_hash)
if deduped:
return await self._quarantine_archives(
@@ -663,6 +683,7 @@ class CommitteeParticipant(FoundationCommitter):
path_provenance=path_provenance,
sha3_hashes=sha3_hashes,
github_payload=github_payload,
+ source_override=source_override,
)
async def set_tag(
diff --git a/atr/tasks/__init__.py b/atr/tasks/__init__.py
index f68bbbf0..cc3a90b0 100644
--- a/atr/tasks/__init__.py
+++ b/atr/tasks/__init__.py
@@ -36,6 +36,7 @@ import atr.models.results as results
import atr.models.safe as safe
import atr.models.sql as sql
import atr.paths as file_paths
+import atr.source as source
import atr.tasks.cap as cap
import atr.tasks.catalog_site as catalog_site
import atr.tasks.checks as checks
@@ -592,7 +593,7 @@ async def tar_gz_checks(
compare.INPUT_POLICY_KEYS,
release,
revision,
- await checks.resolve_extra_args(compare.INPUT_EXTRA_ARGS, release),
+ await source.inputs(release.safe_project_key, release.safe_version_key, revision),
file=path,
)
license_h_ck = await checks.resolve_cache_key(
@@ -693,7 +694,7 @@ async def zip_checks(
compare.INPUT_POLICY_KEYS,
release,
revision,
- await checks.resolve_extra_args(compare.INPUT_EXTRA_ARGS, release),
+ await source.inputs(release.safe_project_key, release.safe_version_key, revision),
file=path,
)
license_h_ck = await checks.resolve_cache_key(
diff --git a/atr/tasks/checks/compare.py b/atr/tasks/checks/compare.py
index bae499a8..6146a9e1 100644
--- a/atr/tasks/checks/compare.py
+++ b/atr/tasks/checks/compare.py
@@ -24,7 +24,7 @@ import shutil
import subprocess
import time
from collections.abc import Mapping
-from typing import Any, Final
+from typing import Final
import aiofiles
import aiofiles.os
@@ -35,14 +35,14 @@ import dulwich.objectspec
import dulwich.porcelain
import dulwich.refs
-import atr.attestable as attestable
import atr.config as config
import atr.log as log
-import atr.models.github as github_models
+import atr.models.attestable
import atr.models.results as results
import atr.models.safe as safe
import atr.paths as paths
import atr.sandbox as sandbox
+import atr.source as source
import atr.tasks.checks as checks
import atr.tasks.task as task
import atr.util as util
@@ -96,10 +96,10 @@ async def source_trees(args: checks.FunctionArguments) -> results.Results | None
)
return None
- payload = await attestable.github_tp_payload_read(args.project_key, args.version_key, args.revision_number)
+ payload = await source.read(args.project_key, args.version_key, args.revision_number)
checkout_dir: str | None = None
archive_dir: str | None = None
- if payload is not None:
+ if payload.repository and payload.sha:
try:
outcome = await _compare_checkout_and_archive(args, recorder, payload)
except OSError as e:
@@ -110,23 +110,20 @@ async def source_trees(args: checks.FunctionArguments) -> results.Results | None
if outcome is None:
return None
checkout_dir, archive_dir = outcome
- payload_summary = _payload_summary(payload)
log.info(
"Ran compare.source_trees successfully",
project=args.project_key,
version=args.version_key,
revision=args.revision_number,
path=args.primary_rel_path,
- github_payload=payload_summary,
+ source_commit=payload.model_dump(),
github_checkout=checkout_dir,
archive_checkout=archive_dir,
)
return None
-async def _checkout_github_source(
- payload: github_models.TrustedPublisherPayload, checkout_dir: safe.StatePath
-) -> str | None:
+async def _checkout_github_source(payload: atr.models.attestable.SourceV2, checkout_dir: safe.StatePath) -> str | None:
repo_url = f"https://github.com/{payload.repository}.git"
started_ns = time.perf_counter_ns()
try:
@@ -207,7 +204,7 @@ def _clone_repo(repo_url: str, sha: str, checkout_dir: safe.StatePath) -> None:
async def _compare_checkout_and_archive( # noqa: C901
args: checks.FunctionArguments,
recorder: checks.Recorder,
- payload: github_models.TrustedPublisherPayload,
+ payload: atr.models.attestable.SourceV2,
) -> tuple[str, str] | None:
if not (primary_abs_path := await recorder.abs_path()):
return None
@@ -376,17 +373,3 @@ async def _find_archive_root(archive_path: safe.StatePath, extract_dir: safe.Sta
extra_entries=extra_entries,
)
return ArchiveRootResult(root=found_root, extra_entries=extra_entries)
-
-
-def _payload_summary(payload: github_models.TrustedPublisherPayload | None) -> dict[str, Any]:
- if payload is None:
- return {"present": False}
- return {
- "present": True,
- "repository": payload.repository,
- "ref": payload.ref,
- "sha": payload.sha,
- "workflow_ref": payload.workflow_ref,
- "actor": payload.actor,
- "actor_id": payload.actor_id,
- }
diff --git a/tests/unit/test_checks_compare.py b/tests/unit/test_checks_compare.py
index 0b7e05dd..219eb056 100644
--- a/tests/unit/test_checks_compare.py
+++ b/tests/unit/test_checks_compare.py
@@ -19,6 +19,7 @@ import datetime
import pathlib
import shutil
import subprocess
+import unittest.mock as mock
from collections.abc import Callable, Iterable, Mapping
import aiofiles.os
@@ -28,6 +29,7 @@ import dulwich.refs
import pytest
import atr.attestable
+import atr.models.attestable
import atr.models.github
import atr.models.safe
import atr.models.safe as safe
@@ -40,14 +42,16 @@ import atr.tasks.task as task
class CheckoutRecorder:
def __init__(self, return_value: str | None = None) -> None:
self.checkout_dir: safe.StatePath | None = None
+ self.source: atr.models.attestable.SourceV2 | None = None
self.return_value = return_value
async def __call__(
self,
- payload: atr.models.github.TrustedPublisherPayload,
+ payload: atr.models.attestable.SourceV2,
checkout_dir: safe.StatePath,
) -> str | None:
self.checkout_dir = checkout_dir
+ self.source = payload
assert await aiofiles.os.path.exists(checkout_dir)
if self.return_value is not None:
return self.return_value
@@ -660,8 +664,9 @@ async def test_find_archive_root_returns_none_when_no_directories(tmp_path: path
@pytest.mark.asyncio
+@pytest.mark.parametrize("override", [None, "c" * 40])
async def test_source_trees_creates_temp_workspace_and_cleans_up(
- monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+ monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path, override: str | None
) -> None:
temp_dir = safe.StatePath(tmp_path)
recorder = RecorderStub(True)
@@ -675,6 +680,14 @@ async def test_source_trees_creates_temp_workspace_and_cleans_up(
tmp_root = temp_dir / "temporary-root"
monkeypatch.setattr(atr.attestable, "github_tp_payload_read", PayloadLoader(payload))
+ recorded = (
+ atr.models.attestable.AttestableV2(
+ source=atr.models.attestable.SourceV2(repository=payload.repository, default=payload.sha, override=override)
+ )
+ if override
+ else None
+ )
+ monkeypatch.setattr(atr.attestable, "load", mock.AsyncMock(return_value=recorded))
monkeypatch.setattr(atr.tasks.checks.compare, "_checkout_github_source", checkout)
monkeypatch.setattr(atr.tasks.checks, "resolve_archive_dir", ArchiveDirResolver(cache_dir))
monkeypatch.setattr(atr.tasks.checks.compare, "_find_archive_root", find_root)
@@ -684,6 +697,8 @@ async def test_source_trees_creates_temp_workspace_and_cleans_up(
await atr.tasks.checks.compare.source_trees(args)
assert checkout.checkout_dir is not None
+ assert checkout.source is not None
+ assert checkout.source.sha == (override or payload.sha)
checkout_dir = checkout.checkout_dir
assert checkout_dir.path.name == "github"
assert checkout_dir.parent.path.parent == tmp_root.path
diff --git a/tests/unit/test_compose_polling.py b/tests/unit/test_compose_polling.py
index abc1756b..5278c2f7 100644
--- a/tests/unit/test_compose_polling.py
+++ b/tests/unit/test_compose_polling.py
@@ -16,8 +16,14 @@
# under the License.
import pathlib
+import unittest.mock as mock
+
+import htpy
+import pytest
import atr.get.compose as compose
+import atr.models.attestable as attestable
+import atr.models.sql as sql
def test_banner_html_ongoing_only():
@@ -32,6 +38,24 @@ def test_banner_html_pending_quarantine_only():
assert 'id="ongoing-tasks-count"' in html
+@pytest.mark.parametrize("override", [None, "b" * 40])
+async def test_commit_hash_form_shows_default_and_submits_only_override(
+ monkeypatch: pytest.MonkeyPatch, override: str | None
+):
+ release = sql.Release(project_key="test", version="1.0")
+ selected = attestable.SourceV2(repository="apache/test", default="a" * 40, override=override)
+ monkeypatch.setattr(compose.source, "current", mock.AsyncMock(return_value=selected))
+ monkeypatch.setattr(compose.util, "as_url", mock.Mock(return_value="/compose/test/1.0"))
+ render = mock.AsyncMock(return_value=htpy.div["form"])
+ monkeypatch.setattr(compose.form, "render", render)
+ await compose._commit_hash_form_html(release)
+ options = render.await_args.kwargs
+ submitted = options["model_cls"](csrf_token="test", **options["defaults"])
+ assert str(submitted.commit_hash or "") == (override or "")
+ details = str(options["pre_submit"])
+ assert ("Explicitly supplied" if override else "Assumed from workflow") in details
+
+
def test_compose_polling_active_neither():
assert compose._compose_polling_active(0, 0) is False
diff --git a/tests/unit/test_create_revision.py b/tests/unit/test_create_revision.py
index 98df9368..2d2009b1 100644
--- a/tests/unit/test_create_revision.py
+++ b/tests/unit/test_create_revision.py
@@ -22,6 +22,7 @@ import unittest.mock as mock
import pytest
+import atr.models.attestable
import atr.models.github as github
import atr.models.safe as safe
import atr.models.sql as sql
@@ -218,6 +219,9 @@ async def test_commit_new_revision_writes_metadata_before_checks(
release = mock.MagicMock()
release.phase = sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT
release.commit_hash = "a" * 40
+ previous = atr.models.attestable.AttestableV2(
+ source=atr.models.attestable.SourceV2(repository="apache/test", default=release.commit_hash)
+ )
release.release_policy = None
release.project.release_policy = None
release.activity_at = datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC)
@@ -254,7 +258,7 @@ async def test_commit_new_revision_writes_metadata_before_checks(
merge_base_revision_key=None,
path_to_hash=path_hashes,
path_to_size=path_sizes,
- previous_attestable=None,
+ previous_attestable=previous,
project_key=safe.ProjectKey("proj"),
release=release,
release_key=release_key,
@@ -265,6 +269,7 @@ async def test_commit_new_revision_writes_metadata_before_checks(
)
assert write_files_data.await_args.kwargs["sha3_hashes"] == sha3_hashes
+ assert write_files_data.await_args.kwargs["source"].sha == (github_payload.sha if has_github_payload else "a" * 40)
expected_payload_calls = (
[("payload", (safe.ProjectKey("proj"), safe.VersionKey("1.0"), safe.RevisionNumber("00006"), payload))]
if has_github_payload
@@ -1147,6 +1152,8 @@ def _make_release_participant() -> release.CommitteeParticipant:
def _mock_db_session(release: mock.MagicMock, selected_revision: mock.MagicMock | None = None) -> mock.MagicMock:
release.project.committee_key = "test"
+ release.project.policy_github_repository_name = "test"
+ release.commit_hash = None
mock_query = mock.MagicMock()
mock_query.demand = mock.AsyncMock(return_value=release)
mock_selected_query = mock.MagicMock()
diff --git a/tests/unit/test_release_writer_commit_hash.py b/tests/unit/test_release_writer_commit_hash.py
index ccaab841..2ca31a37 100644
--- a/tests/unit/test_release_writer_commit_hash.py
+++ b/tests/unit/test_release_writer_commit_hash.py
@@ -16,22 +16,42 @@
# under the License.
import datetime
+import pathlib
import unittest.mock as mock
import pytest
import sqlalchemy.ext.asyncio
import sqlmodel
+import atr.attestable as attestable
+import atr.config as config
import atr.db as db
import atr.models.safe as safe
import atr.models.sql as sql
+import atr.source as source
import atr.storage as storage
import atr.storage.writers.release as release
+import atr.storage.writers.revision as revision
@pytest.fixture
-async def data():
- engine = sqlalchemy.ext.asyncio.create_async_engine("sqlite+aiosqlite:///:memory:")
+async def data(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path):
+ engine = sqlalchemy.ext.asyncio.create_async_engine(f"sqlite+aiosqlite:///{tmp_path}/test.db")
+ monkeypatch.setattr(config.get(), "STATE_DIR", str(tmp_path))
+ for attribute, directory in [
+ ("UNFINISHED_STORAGE_DIR", "unfinished"),
+ ("ATTESTABLE_STORAGE_DIR", "attestable"),
+ ("ARCHIVES_STORAGE_DIR", "archives"),
+ ]:
+ monkeypatch.setattr(config.get(), attribute, str(tmp_path / directory))
+ (tmp_path / "temporary").mkdir()
+ monkeypatch.setattr(
+ db,
+ "_global_atr_sessionmaker",
+ sqlalchemy.ext.asyncio.async_sessionmaker(engine, class_=db.Session, expire_on_commit=False),
+ )
+ monkeypatch.setattr(revision.tasks, "draft_checks", mock.AsyncMock())
+ monkeypatch.setattr(storage, "audit", mock.Mock())
try:
async with engine.begin() as connection:
await connection.run_sync(sqlmodel.SQLModel.metadata.create_all)
@@ -45,7 +65,9 @@ async def data():
@pytest.mark.parametrize("phase", list(sql.ReleasePhase))
async def test_set_commit_hash_only_updates_drafts(data: db.Session, phase: sql.ReleasePhase, commit_hash: str | None):
committee = sql.Committee(key="project", name="Project")
- project = sql.Project(key="project", name="Apache Project", committee=committee)
+ project = sql.Project(
+ key="project", name="Apache Project", committee=committee, repositories=["https://github.com/apache/project"]
+ )
release_row = sql.Release(
key="project-1.0.0",
project=project,
@@ -63,13 +85,16 @@ async def test_set_commit_hash_only_updates_drafts(data: db.Session, phase: sql.
write = mock.MagicMock()
write.authorisation.asf_uid = "tester"
write_as = mock.MagicMock()
+ write_as.revision = revision.CommitteeParticipant(write, write_as, data, "project")
writer = release.CommitteeParticipant(write, write_as, data, "project")
expected_commit_hash = "a" * 40
if phase == sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT:
result = await writer.set_commit_hash(safe.ProjectKey("project"), safe.VersionKey("1.0.0"), commit_hash)
assert result is release_row
expected_commit_hash = commit_hash
- write_as.append_to_audit_log.assert_called_once()
+ recorded = await source.read(safe.ProjectKey("project"), safe.VersionKey("1.0.0"), safe.RevisionNumber("00001"))
+ assert recorded.override == commit_hash
+ assert recorded.repository == "apache/project"
else:
with pytest.raises(storage.AccessError) as exc:
await writer.set_commit_hash(safe.ProjectKey("project"), safe.VersionKey("1.0.0"), commit_hash)
@@ -79,3 +104,49 @@ async def test_set_commit_hash_only_updates_drafts(data: db.Session, phase: sql.
await data.refresh(release_row)
assert release_row.phase == phase
assert release_row.commit_hash == expected_commit_hash
+
+
+async def test_set_commit_hash_rechecks_new_revision(data: db.Session):
+ project = sql.Project(
+ key="project",
+ name="Apache Project",
+ committee=sql.Committee(key="project", name="Project"),
+ repositories=["https://github.com/apache/project"],
+ )
+ release_row = sql.Release(
+ key=sql.release_key("project", "1.0.0"),
+ project=project,
+ version="1.0.0",
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
+ created=datetime.datetime.now(datetime.UTC),
+ )
+ data.add(release_row)
+ await data.commit()
+ write = mock.MagicMock()
+ write.authorisation.asf_uid = "tester"
+ write_as = mock.MagicMock()
+ write_as.revision = revision.CommitteeParticipant(write, write_as, data, "project")
+ writer = release.CommitteeParticipant(write, write_as, data, "project")
+ pk, vk = safe.ProjectKey("project"), safe.VersionKey("1.0.0")
+
+ await writer.set_commit_hash(pk, vk, "a" * 40)
+ first = await attestable.load(pk, vk, safe.RevisionNumber("00001"))
+ first_revision = await data.revision(release_key=release_row.key, number="00001").demand(RuntimeError())
+ with mock.patch.object(
+ revision.interaction, "latest_revision", new=mock.AsyncMock(side_effect=[None, first_revision])
+ ):
+ with pytest.raises(storage.AccessError, match="release has changed"):
+ await writer.set_commit_hash(pk, vk, "c" * 40)
+ await writer.set_commit_hash(pk, vk, "b" * 40)
+ with mock.patch.object(
+ revision.detection, "detect_archives_requiring_quarantine", return_value=[safe.RelPath("source.tar.gz")]
+ ):
+ with pytest.raises(storage.AccessError, match="need validation"):
+ await writer.set_commit_hash(pk, vk, "c" * 40)
+
+ assert await attestable.load(pk, vk, safe.RevisionNumber("00001")) == first
+ assert (await source.read(pk, vk, safe.RevisionNumber("00002"))).sha == "b" * 40
+ assert await source.inputs(pk, vk, safe.RevisionNumber("00001")) != await source.inputs(
+ pk, vk, safe.RevisionNumber("00002")
+ )
+ assert revision.tasks.draft_checks.await_count == 2
diff --git a/tests/unit/test_safe_json.py b/tests/unit/test_safe_json.py
index 0cd404c0..74291a18 100644
--- a/tests/unit/test_safe_json.py
+++ b/tests/unit/test_safe_json.py
@@ -33,7 +33,7 @@ _SAFE_TYPE_EXAMPLES: dict[type[safe.SafeType], str] = {
safe.Alphanumeric: "abc-123",
safe.OwnerNamespace: "com.example",
safe.AsfUid: "user1",
- safe.CommitHash: "1a2b3c4d",
+ safe.CommitHash: "1a2b3c4d" * 5,
safe.CommitteeKey: "tooling",
safe.Numeric: "12345",
safe.ProjectKey: "tooling",
diff --git a/tests/unit/test_safe_types.py b/tests/unit/test_safe_types.py
index 52e95887..361816da 100644
--- a/tests/unit/test_safe_types.py
+++ b/tests/unit/test_safe_types.py
@@ -74,14 +74,14 @@ def test_safe_version_types_reject_too_long():
safe.VersionKey("1" * (safe.MAX_VERSION_LENGTH + 1))
-@pytest.mark.parametrize("value", ["abc1234", "0123456789abcdef", "a" * 7, "f" * 64])
+@pytest.mark.parametrize("value", ["0123456789abcdef" + "a" * 24, "f" * 40])
def test_commit_hash_accepts_valid_hex(value: str):
assert str(safe.CommitHash(value)) == value
-@pytest.mark.parametrize("bad", ["abc", "abcdef", "a" * 65])
+@pytest.mark.parametrize("bad", ["abc1234", "a" * 39, "a" * 41, "a" * 64])
def test_commit_hash_rejects_bad_length(bad: str):
- with pytest.raises(ValueError, match="7 and 64"):
+ with pytest.raises(ValueError, match="exactly 40"):
safe.CommitHash(bad)
@@ -91,7 +91,7 @@ def test_commit_hash_rejects_non_hex(bad: str):
safe.CommitHash(bad)
-@pytest.mark.parametrize(("value", "expected"), [("ABC1234", "abc1234"), ("DeadBeef12", "deadbeef12")])
+@pytest.mark.parametrize(("value", "expected"), [("A" * 40, "a" * 40), ("DeadBeef12" * 4, "deadbeef12" * 4)])
def test_optional_commit_hash_lowercases(value: str, expected: str):
result = pydantic.TypeAdapter(safe.OptionalCommitHash).validate_python(value)
assert str(result) == expected
diff --git a/tests/unit/test_source.py b/tests/unit/test_source.py
new file mode 100644
index 00000000..5165b6f2
--- /dev/null
+++ b/tests/unit/test_source.py
@@ -0,0 +1,111 @@
+# 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 unittest.mock as mock
+
+import pytest
+
+import atr.attestable as attestable
+import atr.models.attestable
+import atr.models.github as github
+import atr.models.safe as safe
+import atr.models.sql as sql
+import atr.source as source
+
+
+@pytest.mark.parametrize("override", [None, "c" * 40])
+def test_advance_keeps_source_choice(override: str | None, github_payload: github.TrustedPublisherPayload):
+ selected = atr.models.attestable.SourceV2(repository="apache/test", default="a" * 40, override=override)
+ result = source.advance(selected, github_payload)
+ assert result.default == github_payload.sha
+ assert result.override == override
+ assert result.sha == (override or github_payload.sha)
+ assert selected.default == "a" * 40
+
+
+async def test_initial_accepts_repository_configured_after_first_revision():
+ release = sql.Release(
+ project=sql.Project(key="test", name="Test", repositories=["https://github.com/apache/test"]), version="1.0"
+ )
+ previous = atr.models.attestable.AttestableV2(source=atr.models.attestable.SourceV2())
+ selected = await source.initial(release, previous)
+ assert selected.repository == "apache/test"
+ assert previous.source.repository == ""
+
+
+@pytest.mark.parametrize("recorded_hash", ["b" * 40, "c" * 40])
+async def test_initial_preserves_legacy_choice(
+ monkeypatch: pytest.MonkeyPatch, github_payload: github.TrustedPublisherPayload, recorded_hash: str
+):
+ release = sql.Release(
+ project=sql.Project(key="test", name="Test"), project_key="test", version="1.0", commit_hash=recorded_hash
+ )
+ monkeypatch.setattr(attestable, "latest_github_tp_payload", mock.AsyncMock(return_value=github_payload))
+ selected = await source.initial(release, None)
+ assert selected.repository == github_payload.repository
+ assert selected.default == github_payload.sha
+ assert selected.sha == recorded_hash
+ assert selected.override == (recorded_hash if (recorded_hash != github_payload.sha) else None)
+
+
+def test_override_can_be_set_or_cleared():
+ selected = atr.models.attestable.SourceV2(repository="apache/test", default="a" * 40, override="b" * 40)
+ confirmed = source.advance(selected, None, "b" * 40)
+ cleared = source.advance(selected, None, None)
+ assert confirmed.sha == "b" * 40
+ assert cleared.sha == "a" * 40
+ assert cleared.override is None
+ with pytest.raises(ValueError, match="Configure a GitHub repository"):
+ source.advance(atr.models.attestable.SourceV2(), None, "b" * 40)
+
+
+async def test_read_and_cache_inputs_use_requested_snapshot(
+ monkeypatch: pytest.MonkeyPatch, github_payload: github.TrustedPublisherPayload
+):
+ pk, vk, rev = safe.ProjectKey("test"), safe.VersionKey("1.0"), safe.RevisionNumber("00001")
+ selected = atr.models.attestable.SourceV2(repository="apache/test", default=github_payload.sha, override="c" * 40)
+ monkeypatch.setattr(
+ attestable, "load", mock.AsyncMock(return_value=atr.models.attestable.AttestableV2(source=selected))
+ )
+ original = mock.AsyncMock(return_value=github_payload)
+ monkeypatch.setattr(attestable, "github_tp_payload_read", original)
+ assert (await source.read(pk, vk, rev)).sha == "c" * 40
+ assert await source.inputs(pk, vk, rev) == {"source_repository": "apache/test", "source_commit": "c" * 40}
+ attestable.load.assert_awaited_with(pk, vk, rev)
+ original.assert_not_awaited()
+ monkeypatch.setattr(attestable, "load", mock.AsyncMock(return_value=None))
+ assert (await source.read(pk, vk, rev)).sha == github_payload.sha
+ original.assert_awaited_once_with(pk, vk, rev)
+
+
+@pytest.mark.parametrize(
+ ("repositories", "policy_name", "expected"),
+ [
+ (["https://github.com/apache/test.git"], "", "apache/test"),
+ (["https://github.com/apache/first", "https://github.com/apache/second"], "", ""),
+ (["https://github.com/apache/first"], "configured", "apache/configured"),
+ ([], "", ""),
+ ],
+)
+def test_repository_requires_one_choice(repositories: list[str], policy_name: str, expected: str):
+ project = sql.Project(
+ key="test",
+ name="Test",
+ repositories=repositories,
+ release_policy=sql.ReleasePolicy(github_repository_name=policy_name),
+ )
+ assert source.repository(project) == expected
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org