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/07/28 19:17:22 UTC
(tooling-trusted-releases) branch sbp updated (1015fd27 -> 2d9ebc44)
This is an automated email from the ASF dual-hosted git repository.
sbp pushed a change to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
discard 1015fd27 Reclassify some check exceptions to allow retrying
new 2d9ebc44 Reclassify some check exceptions to allow retrying
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (1015fd27)
\
N -- N -- N refs/heads/sbp (2d9ebc44)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
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:
atr/tasks/checks/compare.py | 2 ++
atr/tasks/checks/signature.py | 19 +++++++++++++++++++
tests/unit/test_checks_compare.py | 15 +++++++++++++++
tests/unit/test_checks_signature.py | 31 +++++++++++++++++++++++++++++++
4 files changed, 67 insertions(+)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org
(tooling-trusted-releases) 01/01: Reclassify some check exceptions to allow retrying
Posted by sb...@apache.org.
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
commit 2d9ebc44fde1ae62d5e102cfaf079da2ed2469d0
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Tue Jul 28 20:16:33 2026 +0100
Reclassify some check exceptions to allow retrying
---
atr/models/checkdata.py | 1 +
atr/tasks/checks/compare.py | 171 ++++++++++++++++++-------------
atr/tasks/checks/hashing.py | 20 ++--
atr/tasks/checks/license.py | 40 +++-----
atr/tasks/checks/parity.py | 5 +-
atr/tasks/checks/paths.py | 5 +-
atr/tasks/checks/rat.py | 42 ++++----
atr/tasks/checks/sbom.py | 10 +-
atr/tasks/checks/signature.py | 42 ++++++--
atr/tasks/checks/targz.py | 21 ++--
atr/tasks/checks/zipformat.py | 20 ++--
tests/unit/test_archive_root_variants.py | 19 ++--
tests/unit/test_checks_compare.py | 36 ++++---
tests/unit/test_checks_license.py | 38 +++++--
tests/unit/test_checks_rat.py | 31 +++++-
tests/unit/test_checks_signature.py | 31 ++++++
16 files changed, 326 insertions(+), 206 deletions(-)
diff --git a/atr/models/checkdata.py b/atr/models/checkdata.py
index 0058225a..5b356505 100644
--- a/atr/models/checkdata.py
+++ b/atr/models/checkdata.py
@@ -35,6 +35,7 @@ class Rat(schema.Lax):
unapproved_licenses: int = schema.default(0)
unknown_licenses: int = schema.default(0)
errors: list[str] = schema.factory(list)
+ structural: bool = schema.default(False)
excludes_source: str = schema.default("unknown")
extended_std_applied: bool = schema.default(False)
warning: str | None = schema.default(None)
diff --git a/atr/tasks/checks/compare.py b/atr/tasks/checks/compare.py
index dc6bbdfb..7d64e2e2 100644
--- a/atr/tasks/checks/compare.py
+++ b/atr/tasks/checks/compare.py
@@ -29,6 +29,7 @@ from typing import Any, Final
import aiofiles
import aiofiles.os
import dulwich.client
+import dulwich.errors
import dulwich.objects
import dulwich.objectspec
import dulwich.porcelain
@@ -42,6 +43,7 @@ import atr.models.results as results
import atr.models.safe as safe
import atr.paths as paths
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
_CONFIG: Final = config.get()
@@ -53,7 +55,7 @@ _PERMITTED_ADDED_PATHS: Final[dict[str, list[str]]] = {
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = ["github_tp_sha"]
-CHECK_VERSION: Final[str] = "3"
+CHECK_VERSION: Final[str] = "4"
@dataclasses.dataclass
@@ -80,7 +82,7 @@ class TreeComparisonResult:
repo_only: set[str]
-async def source_trees(args: checks.FunctionArguments) -> results.Results | None: # noqa: C901
+async def source_trees(args: checks.FunctionArguments) -> results.Results | None:
recorder = await args.recorder(CHECK_VERSION)
is_source = await recorder.primary_path_is_source()
if not is_source:
@@ -97,77 +99,16 @@ async def source_trees(args: checks.FunctionArguments) -> results.Results | None
checkout_dir: str | None = None
archive_dir: str | None = None
if payload is not None:
- if not (primary_abs_path := await recorder.abs_path()):
+ try:
+ outcome = await _compare_checkout_and_archive(args, recorder, payload)
+ except OSError as e:
+ raise task.CheckRetryableError(
+ "Error reading the extracted archive tree",
+ {"error": str(e)},
+ ) from e
+ if outcome is None:
return None
- extracted_dir = await checks.resolve_archive_dir(args)
- if extracted_dir is None:
- await recorder.exception(
- "Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
- )
- return None
- tmp_dir = paths.get_tmp_dir()
- await aiofiles.os.makedirs(tmp_dir, exist_ok=True)
- async with util.async_temporary_directory(prefix="trees-", dir=tmp_dir) as temp_dir:
- github_dir = safe.StatePath(temp_dir) / "github"
- await aiofiles.os.makedirs(github_dir, exist_ok=True)
- checkout_dir = await _checkout_github_source(payload, github_dir)
- if checkout_dir is None:
- repo_url = f"https://github.com/{payload.repository}.git"
- await recorder.exception(
- "Failed to clone GitHub repository for comparison",
- {"repo_url": repo_url, "sha": payload.sha},
- )
- raise RuntimeError(f"Failed to clone {repo_url} at {payload.sha} for source tree comparison")
- archive_root_result = await _find_archive_root(primary_abs_path, extracted_dir)
- if archive_root_result.root is None:
- await recorder.concern(
- "Could not determine archive root directory for comparison",
- {"archive_path": str(primary_abs_path), "extract_dir": str(extracted_dir)},
- )
- return None
- if archive_root_result.extra_entries:
- await recorder.concern(
- "Archive contains entries outside the root directory",
- {
- "archive_path": str(primary_abs_path),
- "root": archive_root_result.root,
- "extra_entries": sorted(archive_root_result.extra_entries),
- },
- )
- return None
- archive_content_dir = extracted_dir / archive_root_result.root
- archive_dir = str(archive_content_dir)
- try:
- comparison = await _compare_trees(github_dir, archive_content_dir)
- except RuntimeError as exc:
- await recorder.exception(
- "Failed to compare source tree against GitHub checkout",
- {"error": str(exc)},
- )
- return None
- invalid_filtered: set[str] = set()
- for path in comparison.invalid:
- required = _PERMITTED_ADDED_PATHS.get(path)
- if required is None:
- invalid_filtered.add(path)
- elif not all((archive_content_dir / r).path.is_file() for r in required):
- invalid_filtered.add(path)
- if invalid_filtered:
- invalid_list = sorted(invalid_filtered)
- await recorder.concern(
- "Source archive contains files not in GitHub checkout or with different content",
- {"invalid_count": len(invalid_list), "invalid_paths": invalid_list},
- )
- return None
- repo_only_list = sorted(comparison.repo_only)
- await recorder.note(
- "Source archive is a valid subset of GitHub checkout",
- {
- "repo_only_count": len(repo_only_list),
- "repo_only_paths_sample": repo_only_list[:5],
- },
- )
+ checkout_dir, archive_dir = outcome
payload_summary = _payload_summary(payload)
log.info(
"Ran compare.source_trees successfully",
@@ -206,7 +147,15 @@ async def _checkout_github_source(
email=os.environ.get("EMAIL"),
)
return None
- except Exception:
+ except (
+ OSError,
+ RuntimeError,
+ dulwich.client.HTTPProxyUnauthorized,
+ dulwich.client.HTTPUnauthorized,
+ dulwich.errors.ChecksumMismatch,
+ dulwich.errors.GitProtocolError,
+ dulwich.errors.NotGitRepository,
+ ):
elapsed_ms = (time.perf_counter_ns() - started_ns) / 1_000_000.0
log.exception(
"Failed to clone GitHub repo for compare.source_trees",
@@ -254,6 +203,82 @@ def _clone_repo(repo_url: str, sha: str, checkout_dir: safe.StatePath) -> None:
shutil.rmtree(git_dir)
+async def _compare_checkout_and_archive( # noqa: C901
+ args: checks.FunctionArguments,
+ recorder: checks.Recorder,
+ payload: github_models.TrustedPublisherPayload,
+) -> tuple[str, str] | None:
+ if not (primary_abs_path := await recorder.abs_path()):
+ return None
+ extracted_dir = await checks.resolve_archive_dir(args)
+ if extracted_dir is None:
+ raise task.CheckRetryableError(
+ "Extracted archive tree is not available",
+ {"rel_path": str(args.primary_rel_path)},
+ )
+ tmp_dir = paths.get_tmp_dir()
+ await aiofiles.os.makedirs(tmp_dir, exist_ok=True)
+ async with util.async_temporary_directory(prefix="trees-", dir=tmp_dir) as temp_dir:
+ github_dir = safe.StatePath(temp_dir) / "github"
+ await aiofiles.os.makedirs(github_dir, exist_ok=True)
+ checkout_dir = await _checkout_github_source(payload, github_dir)
+ if checkout_dir is None:
+ repo_url = f"https://github.com/{payload.repository}.git"
+ raise task.CheckRetryableError(
+ "Failed to clone GitHub repository for comparison",
+ {"repo_url": repo_url, "sha": payload.sha},
+ )
+ archive_root_result = await _find_archive_root(primary_abs_path, extracted_dir)
+ if archive_root_result.root is None:
+ await recorder.concern(
+ "Could not determine archive root directory for comparison",
+ {"archive_path": str(primary_abs_path), "extract_dir": str(extracted_dir)},
+ )
+ return None
+ if archive_root_result.extra_entries:
+ await recorder.concern(
+ "Archive contains entries outside the root directory",
+ {
+ "archive_path": str(primary_abs_path),
+ "root": archive_root_result.root,
+ "extra_entries": sorted(archive_root_result.extra_entries),
+ },
+ )
+ return None
+ archive_content_dir = extracted_dir / archive_root_result.root
+ archive_dir = str(archive_content_dir)
+ try:
+ comparison = await _compare_trees(github_dir, archive_content_dir)
+ except RuntimeError as exc:
+ raise task.CheckRetryableError(
+ "Failed to compare source tree against GitHub checkout",
+ {"error": str(exc)},
+ ) from exc
+ invalid_filtered: set[str] = set()
+ for path in comparison.invalid:
+ required = _PERMITTED_ADDED_PATHS.get(path)
+ if required is None:
+ invalid_filtered.add(path)
+ elif not all((archive_content_dir / r).path.is_file() for r in required):
+ invalid_filtered.add(path)
+ if invalid_filtered:
+ invalid_list = sorted(invalid_filtered)
+ await recorder.concern(
+ "Source archive contains files not in GitHub checkout or with different content",
+ {"invalid_count": len(invalid_list), "invalid_paths": invalid_list},
+ )
+ return None
+ repo_only_list = sorted(comparison.repo_only)
+ await recorder.note(
+ "Source archive is a valid subset of GitHub checkout",
+ {
+ "repo_only_count": len(repo_only_list),
+ "repo_only_paths_sample": repo_only_list[:5],
+ },
+ )
+ return checkout_dir, archive_dir
+
+
async def _compare_trees(repo_dir: safe.StatePath, archive_dir: safe.StatePath) -> TreeComparisonResult:
return await asyncio.to_thread(_compare_trees_rsync, repo_dir, archive_dir)
diff --git a/atr/tasks/checks/hashing.py b/atr/tasks/checks/hashing.py
index 9ae7b9ac..1b8872f9 100644
--- a/atr/tasks/checks/hashing.py
+++ b/atr/tasks/checks/hashing.py
@@ -24,11 +24,12 @@ import aiofiles
import atr.log as log
import atr.models.results as results
import atr.tasks.checks as checks
+import atr.tasks.task as task
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = ["unsuffixed_file_hash"]
-CHECK_VERSION: Final[str] = "3"
+CHECK_VERSION: Final[str] = "4"
async def check(args: checks.FunctionArguments) -> results.Results | None:
@@ -39,8 +40,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
algorithm = hash_abs_path.path.suffix.lstrip(".")
if algorithm not in {"sha256", "sha512"}:
- await recorder.exception("Unsupported hash algorithm", {"algorithm": algorithm})
- return None
+ raise ValueError(f"Unsupported hash algorithm: {algorithm}")
# Remove the hash file suffix to get the artifact path
# This replaces the last suffix, which is what we want
@@ -63,17 +63,21 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
except FileNotFoundError as e:
await recorder.blocker("Referenced artifact not found", {"error": str(e)})
return None
- except Exception as e:
- await recorder.exception("Unable to verify hash", {"error": str(e)})
- return None
+ except OSError as e:
+ raise task.CheckRetryableError("Unable to read the artifact file", {"error": str(e)}) from e
computed_hash = hash_obj.hexdigest()
try:
async with aiofiles.open(hash_abs_path) as f:
expected_hash = await f.read()
- except Exception as e:
- await recorder.exception("Unable to verify hash", {"error": str(e)})
+ except UnicodeDecodeError as e:
+ await recorder.blocker(
+ "Malformed checksum file",
+ {"error": str(e), "hash_file": str(hash_abs_path.path)},
+ )
return None
+ except OSError as e:
+ raise task.CheckRetryableError("Unable to read the checksum file", {"error": str(e)}) from e
if (expected_value := _expected_hash_extract(expected_hash, artifact_abs_path.name)) is None:
await recorder.blocker(
diff --git a/atr/tasks/checks/license.py b/atr/tasks/checks/license.py
index 87313ba0..7bb2a875 100644
--- a/atr/tasks/checks/license.py
+++ b/atr/tasks/checks/license.py
@@ -31,6 +31,7 @@ import atr.models.safe as safe
import atr.models.schema as schema
import atr.models.sql as sql
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
# Constant that must be present in the Apache License header
@@ -82,8 +83,8 @@ INCLUDED_PATTERNS: Final[list[str]] = [
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = ["license_check_mode", "source_excludes_lightweight"]
INPUT_EXTRA_ARGS: Final[list[str]] = ["is_podling"]
-CHECK_VERSION_FILES: Final[str] = "6"
-CHECK_VERSION_HEADERS: Final[str] = "5"
+CHECK_VERSION_FILES: Final[str] = "7"
+CHECK_VERSION_HEADERS: Final[str] = "6"
_BINARY_LICENSE_FILENAMES: Final[frozenset[str]] = frozenset({"LICENSE", "LICENSE.txt"})
_BINARY_NOTICE_FILENAMES: Final[frozenset[str]] = frozenset({"NOTICE", "NOTICE.txt"})
@@ -152,11 +153,10 @@ async def files(args: checks.FunctionArguments) -> results.Results | None:
archive_dir = await checks.resolve_archive_dir(args)
if archive_dir is None:
- await recorder.exception(
+ raise task.CheckRetryableError(
"Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
+ {"rel_path": str(args.primary_rel_path)},
)
- return None
log.info(f"Checking license files for {artifact_abs_path} (rel: {args.primary_rel_path})")
@@ -170,9 +170,8 @@ async def files(args: checks.FunctionArguments) -> results.Results | None:
case MemberSkippedResult():
pass
- except Exception as e:
- log.exception("Error during license file check execution:")
- await recorder.exception("Error during license file check execution", {"error": str(e)})
+ except OSError as e:
+ raise task.CheckRetryableError("Error reading the extracted archive tree", {"error": str(e)}) from e
return None
@@ -195,11 +194,10 @@ async def headers(args: checks.FunctionArguments) -> results.Results | None:
archive_dir = await checks.resolve_archive_dir(args)
if archive_dir is None:
- await recorder.exception(
+ raise task.CheckRetryableError(
"Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
+ {"rel_path": str(args.primary_rel_path)},
)
- return None
log.info(f"Checking license headers for {artifact_abs_path} (rel: {args.primary_rel_path})")
@@ -315,12 +313,7 @@ def _files_check_core_logic(archive_dir: safe.StatePath, is_podling: bool, is_bi
if not archive_dir.path.is_dir():
# Already protected by the caller
# We add it here again to make unit testing cleaner
- yield ArtifactResult(
- status=sql.CheckResultStatus.EXCEPTION,
- message="Cache directory is not available",
- data=None,
- )
- return
+ raise task.CheckRetryableError("Cache directory is not available", {"archive_dir": str(archive_dir)})
# Check for license files in the root directory
top_entries = sorted(e for e in os.listdir(archive_dir) if not e.startswith("._"))
@@ -465,12 +458,7 @@ def _headers_check_core_logic( # noqa: C901
# )
if not archive_dir.path.is_dir():
- yield ArtifactResult(
- status=sql.CheckResultStatus.EXCEPTION,
- message="Cache directory is not available",
- data=None,
- )
- return
+ raise task.CheckRetryableError("Cache directory is not available", {"archive_dir": str(archive_dir)})
# log.info(f"Ignore lines: {ignore_lines}")
@@ -563,7 +551,7 @@ def _headers_check_core_logic_process_file(
message=f"Invalid license header: {error}",
data=None,
)
- except Exception as e:
+ except OSError as e:
return MemberResult(
status=sql.CheckResultStatus.EXCEPTION,
path=rel_path,
@@ -614,8 +602,8 @@ async def _headers_core(
None,
)
- except Exception as e:
- await recorder.exception("Error during license header check execution", {"error": str(e)})
+ except OSError as e:
+ raise task.CheckRetryableError("Error reading the extracted archive tree", {"error": str(e)}) from e
return None
diff --git a/atr/tasks/checks/parity.py b/atr/tasks/checks/parity.py
index 4b67ba15..e6201a48 100644
--- a/atr/tasks/checks/parity.py
+++ b/atr/tasks/checks/parity.py
@@ -24,7 +24,7 @@ import atr.tasks.checks as checks
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = ["cross_format_sibling_swhids"]
-CHECK_VERSION: Final[str] = "1"
+CHECK_VERSION: Final[str] = "2"
async def across_formats(args: checks.FunctionArguments) -> results.Results | None:
@@ -34,8 +34,7 @@ async def across_formats(args: checks.FunctionArguments) -> results.Results | No
primary = str(args.primary_rel_path)
attestable_data = await attestable.load(args.project_key, args.version_key, args.revision_number)
if attestable_data is None:
- await recorder.exception("Attestable data is not available", {"rel_path": primary})
- return None
+ raise RuntimeError("Attestable data is not available")
own_swhid = attestable.path_swhid_dir(attestable_data, primary)
# Do not compare archives if the primary has no SWHID
# This is a deliberate design choice
diff --git a/atr/tasks/checks/paths.py b/atr/tasks/checks/paths.py
index 318c41df..49275b5d 100644
--- a/atr/tasks/checks/paths.py
+++ b/atr/tasks/checks/paths.py
@@ -44,7 +44,7 @@ _DOC_TREE_MAX_FILES: Final = 512
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = ["binary_artifact_paths", "source_artifact_paths", "download_path_suffix"]
INPUT_EXTRA_ARGS: Final[list[str]] = ["is_podling", "all_files"]
-CHECK_VERSION: Final[str] = "7"
+CHECK_VERSION: Final[str] = "8"
async def check(args: checks.FunctionArguments) -> results.Results | None:
@@ -104,8 +104,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
return
if not await aiofiles.os.path.isdir(base_path):
- log.error(f"Base release directory does not exist or is not a directory: {base_path}")
- return
+ raise RuntimeError(f"Base release directory does not exist or is not a directory: {base_path}")
is_podling = args.extra_args.get("is_podling", False)
relative_paths = [p async for p in util.paths_recursive(base_path)]
diff --git a/atr/tasks/checks/rat.py b/atr/tasks/checks/rat.py
index 187e5ade..a7aad897 100644
--- a/atr/tasks/checks/rat.py
+++ b/atr/tasks/checks/rat.py
@@ -22,6 +22,7 @@ import subprocess
import tempfile
from typing import Final
+import defusedxml
import defusedxml.ElementTree as ElementTree
import atr.config as config
@@ -32,6 +33,7 @@ import atr.models.results as results
import atr.models.safe as safe
import atr.models.sql as sql
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
_CONFIG: Final = config.get()
@@ -68,7 +70,7 @@ _STD_EXCLUSIONS_EXTENDED: Final[list[str]] = [
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = ["license_check_mode", "source_excludes_rat"]
INPUT_EXTRA_ARGS: Final[list[str]] = []
-CHECK_VERSION: Final[str] = "4"
+CHECK_VERSION: Final[str] = "5"
class RatError(RuntimeError):
@@ -91,11 +93,10 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
archive_dir = await checks.resolve_archive_dir(args)
if archive_dir is None:
- await recorder.exception(
+ raise task.CheckRetryableError(
"Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
+ {"rel_path": str(args.primary_rel_path)},
)
- return None
log.info(f"Checking RAT licenses for {artifact_abs_path} (rel: {args.primary_rel_path})")
@@ -103,9 +104,8 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
try:
await _check_core(args, recorder, archive_dir, policy_excludes)
- except Exception as e:
- # TODO: Or bubble for task failure?
- await recorder.exception("Error running Apache RAT check", {"error": str(e)})
+ except OSError as e:
+ raise task.CheckRetryableError("Error reading the extracted archive tree", {"error": str(e)}) from e
return None
@@ -180,8 +180,10 @@ async def _check_core(
result_data = result.model_dump(exclude={"unapproved_files", "unknown_license_files"})
members_recorded = bool(result.unknown_license_files) or bool(result.unapproved_files)
- if result.errors:
- await recorder.exception(result.message, result_data)
+ if result.errors and result.structural:
+ await recorder.concern(result.message, result_data)
+ elif result.errors:
+ raise task.CheckRetryableError(result.message, result_data)
elif (not result.valid) and (not members_recorded):
await recorder.concern(result.message, result_data)
else:
@@ -236,7 +238,7 @@ def _check_core_logic_execute_rat(
message="Apache RAT process timed out",
errors=[f"Timeout: {e}"],
), None
- except Exception as e:
+ except (OSError, subprocess.SubprocessError) as e:
# Change back to the original directory before raising
os.chdir(current_dir)
log.error(f"Exception running Apache RAT: {e}")
@@ -364,18 +366,9 @@ def _synchronous(
if jar_error:
return jar_error
- try:
- with tempfile.TemporaryDirectory(prefix="rat_scratch_") as scratch_dir:
- log.info(f"Created scratch directory: {scratch_dir}")
- return _synchronous_core(archive_dir, scratch_dir, policy_excludes, rat_jar_path)
- except Exception as e:
- import traceback
-
- log.exception("Error running Apache RAT")
- return checkdata.Rat(
- message=f"Failed to run Apache RAT: {e!s}",
- errors=[str(e), traceback.format_exc()],
- )
+ with tempfile.TemporaryDirectory(prefix="rat_scratch_") as scratch_dir:
+ log.info(f"Created scratch directory: {scratch_dir}")
+ return _synchronous_core(archive_dir, scratch_dir, policy_excludes, rat_jar_path)
def _synchronous_check_jar_exists(rat_jar_path: str) -> tuple[str, checkdata.Rat | None]:
@@ -477,6 +470,7 @@ def _synchronous_core( # noqa: C901
return checkdata.Rat(
message=f"Multiple {_RAT_EXCLUDES_FILENAME} files not allowed (found {len(exclude_file_paths)})",
errors=[f"Found {len(exclude_file_paths)} {_RAT_EXCLUDES_FILENAME} files"],
+ structural=True,
)
# Narrow to single path after validation
@@ -493,6 +487,7 @@ def _synchronous_core( # noqa: C901
message=f"Failed to determine scan root: {e}",
errors=[str(e)],
excludes_source=excludes_source,
+ structural=True,
)
# Execute RAT and get results or error
@@ -506,6 +501,7 @@ def _synchronous_core( # noqa: C901
return checkdata.Rat(
message=f"Failed to build RAT command: {e}",
errors=[str(e)],
+ structural=True,
)
error_result, xml_output_path = _check_core_logic_execute_rat(command, scan_root, scratch_dir, xml_output_path)
if error_result is not None:
@@ -565,7 +561,7 @@ def _synchronous_core_parse_output(xml_file: str, base_dir: str) -> checkdata.Ra
"""Parse the XML output from Apache RAT safely."""
try:
return _synchronous_core_parse_output_core(xml_file, base_dir)
- except Exception as e:
+ except (OSError, ElementTree.ParseError, defusedxml.DefusedXmlException) as e:
log.error(f"Error parsing RAT output: {e}")
return checkdata.Rat(
message=f"Failed to parse Apache RAT output: {e!s}",
diff --git a/atr/tasks/checks/sbom.py b/atr/tasks/checks/sbom.py
index c1d9ddf7..307dc92b 100644
--- a/atr/tasks/checks/sbom.py
+++ b/atr/tasks/checks/sbom.py
@@ -23,11 +23,12 @@ import atr.analysis as analysis
import atr.log as log
import atr.models.results as results
import atr.tasks.checks as checks
+import atr.tasks.task as task
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = ["suffixed_file_existence"]
-CHECK_VERSION: Final[str] = "1"
+CHECK_VERSION: Final[str] = "2"
async def check(args: checks.FunctionArguments) -> results.Results | None:
@@ -40,9 +41,8 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
try:
result_data = await _check_core_logic(artifact_path=str(primary_abs_path))
- except Exception as e:
- await recorder.exception("Error during SBOM check execution", {"error": str(e)})
- return None
+ except OSError as e:
+ raise task.CheckRetryableError("Error during SBOM check execution", {"error": str(e)}) from e
match result_data:
case {"error": error} if error:
@@ -50,7 +50,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
case _ if result_data.get("found"):
await recorder.note("SBOM located successfully", result_data)
case _:
- await recorder.exception("SBOM location failed for unknown reasons", result_data)
+ raise RuntimeError("SBOM location failed for unknown reasons")
return None
diff --git a/atr/tasks/checks/signature.py b/atr/tasks/checks/signature.py
index 422136c2..08313ef4 100644
--- a/atr/tasks/checks/signature.py
+++ b/atr/tasks/checks/signature.py
@@ -21,6 +21,7 @@ import time
from typing import Any, Final
import openpgp
+import sqlalchemy.exc
import sqlmodel
import atr.db as db
@@ -29,6 +30,7 @@ import atr.models.results as results
import atr.models.sql as sql
import atr.pgp as pgp
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
# Release policy fields which this check relies on - used for result caching
@@ -39,7 +41,7 @@ INPUT_EXTRA_ARGS: Final[list[str]] = [
"unsuffixed_file_hash",
"unsuffixed_file_uploaders",
]
-CHECK_VERSION: Final[str] = "8"
+CHECK_VERSION: Final[str] = "9"
async def check(args: checks.FunctionArguments) -> results.Results | None:
@@ -49,8 +51,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
return None
if not (primary_rel_path := args.primary_rel_path):
- await recorder.exception("Primary relative path is required", {"primary_rel_path": primary_rel_path})
- return None
+ raise ValueError("Primary relative path is required")
artifact_rel_path = str(primary_rel_path).removesuffix(".asc")
if not (artifact_abs_path := await recorder.abs_path(artifact_rel_path)):
@@ -58,8 +59,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
committee_key = args.extra_args.get("committee_key")
if not isinstance(committee_key, str):
- await recorder.exception("Committee name is required", {"committee_key": committee_key})
- return None
+ raise ValueError("Committee name is required")
log.info(
f"Checking signature {primary_abs_path} for {artifact_abs_path}"
@@ -74,9 +74,8 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
signature_path=str(primary_abs_path),
uploader_uids=uploader_uids,
)
- except Exception as e:
- await recorder.exception("Error during signature check execution", {"error": str(e)})
- return None
+ except (OSError, sqlalchemy.exc.SQLAlchemyError) as e:
+ raise task.CheckRetryableError("Error during signature check execution", {"error": str(e)}) from e
match result_data:
case {"error": error} if error:
@@ -86,7 +85,7 @@ async def check(args: checks.FunctionArguments) -> results.Results | None:
if not result_data["signer_is_uploader"]:
await _record_uploader_concern(args, recorder, uploader_uids, result_data)
case _:
- await recorder.exception("Signature verification failed for unknown reasons", result_data)
+ raise RuntimeError("Signature verification failed for unknown reasons")
return None
@@ -168,7 +167,11 @@ def _check_core_logic_verify_signature(
try:
with open(signature_path, "rb") as sig_file:
- signature, _ = openpgp.DetachedSignature.from_armor(sig_file.read().decode("utf-8"))
+ armored = sig_file.read()
+ except OSError as e:
+ raise task.CheckRetryableError("Unable to read the signature file", {"error": str(e)}) from e
+ try:
+ signature, _ = openpgp.DetachedSignature.from_armor(armored.decode("utf-8"))
except Exception as e:
return {
"verified": False,
@@ -184,6 +187,25 @@ def _check_core_logic_verify_signature(
),
}
signature_info = signature.signature_info()
+ try:
+ with open(artifact_path, "rb"):
+ pass
+ except FileNotFoundError:
+ return {
+ "verified": False,
+ "error": "Referenced artifact not found",
+ "error_kind": "missing_artifact",
+ "debug_info": _debug_info(
+ key=None,
+ signature_info=signature_info,
+ status="Referenced artifact not found",
+ valid=False,
+ num_committee_keys=len(ascii_armored_keys),
+ key_has_apache_uid=False,
+ ),
+ }
+ except OSError as e:
+ raise task.CheckRetryableError("Unable to read the artifact file", {"error": str(e)}) from e
issuer_fingerprints = {fingerprint.lower() for fingerprint in signature_info.issuer_fingerprints}
issuer_key_ids = {key_id.lower() for key_id in signature_info.issuer_key_ids}
candidate_keys = [key for key in public_keys if _key_matches_signature(key, issuer_fingerprints, issuer_key_ids)]
diff --git a/atr/tasks/checks/targz.py b/atr/tasks/checks/targz.py
index fbf50d27..c90bbe1e 100644
--- a/atr/tasks/checks/targz.py
+++ b/atr/tasks/checks/targz.py
@@ -25,12 +25,13 @@ import atr.log as log
import atr.models.results as results
import atr.models.safe as safe
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = []
-CHECK_VERSION_STRUCTURE: Final[str] = "5"
+CHECK_VERSION_STRUCTURE: Final[str] = "6"
class RootDirectoryError(Exception):
@@ -51,10 +52,7 @@ def root_directory(archive_dir: safe.StatePath) -> tuple[str, bytes | None]:
root = entries[0]
root_path = archive_dir / root
- try:
- root_stat = root_path.path.lstat()
- except OSError as e:
- raise RootDirectoryError(f"Unable to inspect root entry '{root}': {e}") from e
+ root_stat = root_path.path.lstat()
if not stat.S_ISDIR(root_stat.st_mode):
raise RootDirectoryError(f"Root entry is not a directory: {root}")
@@ -62,7 +60,7 @@ def root_directory(archive_dir: safe.StatePath) -> tuple[str, bytes | None]:
if root == "package":
package_json_path = archive_dir / "package" / "package.json"
- with contextlib.suppress(FileNotFoundError, OSError):
+ with contextlib.suppress(FileNotFoundError):
package_json_stat = package_json_path.path.lstat()
# We do this to avoid allowing package.json to be a symlink
if stat.S_ISREG(package_json_stat.st_mode):
@@ -83,11 +81,10 @@ async def structure(args: checks.FunctionArguments) -> results.Results | None:
archive_dir = await checks.resolve_archive_dir(args)
if archive_dir is None:
- await recorder.exception(
+ raise task.CheckRetryableError(
"Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
+ {"rel_path": str(args.primary_rel_path)},
)
- return None
filename = artifact_abs_path.path.name
basename_from_filename: Final[str] = (
@@ -139,7 +136,7 @@ async def structure(args: checks.FunctionArguments) -> results.Results | None:
f"Root directory '{root}' does not match expected names '{expected_roots_display}'", data
)
except RootDirectoryError as e:
- await recorder.exception("Could not get the root directory of the archive", {"error": str(e)})
- except Exception as e:
- await recorder.exception("Unable to verify archive structure", {"error": str(e)})
+ await recorder.suggestion("Could not get the root directory of the archive", {"error": str(e)})
+ except OSError as e:
+ raise task.CheckRetryableError("Error reading the extracted archive tree", {"error": str(e)}) from e
return None
diff --git a/atr/tasks/checks/zipformat.py b/atr/tasks/checks/zipformat.py
index bfcccb35..49cce8d8 100644
--- a/atr/tasks/checks/zipformat.py
+++ b/atr/tasks/checks/zipformat.py
@@ -24,12 +24,13 @@ import atr.log as log
import atr.models.results as results
import atr.models.safe as safe
import atr.tasks.checks as checks
+import atr.tasks.task as task
import atr.util as util
# Release policy fields which this check relies on - used for result caching
INPUT_POLICY_KEYS: Final[list[str]] = []
INPUT_EXTRA_ARGS: Final[list[str]] = []
-CHECK_VERSION_STRUCTURE: Final[str] = "4"
+CHECK_VERSION_STRUCTURE: Final[str] = "5"
async def structure(args: checks.FunctionArguments) -> results.Results | None:
@@ -46,11 +47,10 @@ async def structure(args: checks.FunctionArguments) -> results.Results | None:
archive_dir = await checks.resolve_archive_dir(args)
if archive_dir is None:
- await recorder.exception(
+ raise task.CheckRetryableError(
"Extracted archive tree is not available",
- {"rel_path": args.primary_rel_path},
+ {"rel_path": str(args.primary_rel_path)},
)
- return None
log.info(f"Checking zip structure for {artifact_abs_path} (rel: {args.primary_rel_path})")
@@ -61,8 +61,8 @@ async def structure(args: checks.FunctionArguments) -> results.Results | None:
await recorder.suggestion(result_data["error"], result_data)
else:
await recorder.note(f"Zip structure OK (root: {result_data['root_dir']})", result_data)
- except Exception as e:
- await recorder.exception("Error checking zip structure", {"error": str(e)})
+ except OSError as e:
+ raise task.CheckRetryableError("Error reading the extracted archive tree", {"error": str(e)}) from e
return None
@@ -82,11 +82,7 @@ def _structure_check_core_logic(archive_dir: safe.StatePath, artifact_path: str)
non_rooted_entries: list[str] = []
for entry in entries:
entry_path = (archive_dir / entry).path
- try:
- entry_stat = entry_path.lstat()
- except OSError as e:
- return {"error": f"Unable to inspect archive root entry '{entry}': {e}", "expected_roots": expected_roots}
-
+ entry_stat = entry_path.lstat()
if stat.S_ISDIR(entry_stat.st_mode):
root_dirs.append(entry)
else:
@@ -122,7 +118,7 @@ def _structure_npm_result(
package_json_path = (archive_dir / "package" / "package.json").path
try:
package_json_stat = package_json_path.lstat()
- except (FileNotFoundError, OSError):
+ except FileNotFoundError:
return None
if not stat.S_ISREG(package_json_stat.st_mode):
diff --git a/tests/unit/test_archive_root_variants.py b/tests/unit/test_archive_root_variants.py
index 72d9a998..4bfd4360 100644
--- a/tests/unit/test_archive_root_variants.py
+++ b/tests/unit/test_archive_root_variants.py
@@ -26,6 +26,7 @@ import atr.models.sql as sql
import atr.tasks.checks as checks
import atr.tasks.checks.targz as targz
import atr.tasks.checks.zipformat as zipformat
+import atr.tasks.task as task
import tests.unit.recorders as recorders
@@ -120,11 +121,10 @@ async def test_targz_structure_fails_when_cache_unavailable(tmp_path: pathlib.Pa
recorder, args = await _targz_structure_args(tmp_path, "apache-example-1.2.3.tar.gz")
with mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=None)):
- await targz.structure(args)
+ with pytest.raises(task.CheckRetryableError, match="Extracted archive tree is not available"):
+ await targz.structure(args)
- assert any(status == sql.CheckResultStatus.EXCEPTION.value for status, _, _ in recorder.messages)
- assert not any(status == sql.CheckResultStatus.CONCERN.value for status, _, _ in recorder.messages)
- assert any("extracted archive tree is not available" in message.lower() for _, message, _ in recorder.messages)
+ assert recorder.messages == []
@pytest.mark.asyncio
@@ -221,7 +221,7 @@ async def test_targz_structure_rejects_symlink_root(tmp_path: pathlib.Path) -> N
with mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=safe.StatePath(cache_dir))):
await targz.structure(args)
- assert any(status == sql.CheckResultStatus.EXCEPTION.value for status, _, _ in recorder.messages)
+ assert any(status == sql.CheckResultStatus.SUGGESTION.value for status, _, _ in recorder.messages)
assert not any(status == sql.CheckResultStatus.CONCERN.value for status, _, _ in recorder.messages)
@@ -240,7 +240,7 @@ async def test_targz_structure_rejects_symlinked_package_json(tmp_path: pathlib.
with mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=safe.StatePath(cache_dir))):
await targz.structure(args)
- assert any(status == sql.CheckResultStatus.EXCEPTION.value for status, _, _ in recorder.messages)
+ assert any(status == sql.CheckResultStatus.SUGGESTION.value for status, _, _ in recorder.messages)
assert not any(status == sql.CheckResultStatus.CONCERN.value for status, _, _ in recorder.messages)
@@ -291,11 +291,10 @@ async def test_zipformat_structure_fails_when_cache_unavailable(tmp_path: pathli
recorder, args = await _zipformat_structure_args(tmp_path, "apache-example-1.2.3.zip")
with mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=None)):
- await zipformat.structure(args)
+ with pytest.raises(task.CheckRetryableError, match="Extracted archive tree is not available"):
+ await zipformat.structure(args)
- assert any(status == sql.CheckResultStatus.EXCEPTION.value for status, _, _ in recorder.messages)
- assert not any(status == sql.CheckResultStatus.CONCERN.value for status, _, _ in recorder.messages)
- assert any("extracted archive tree is not available" in message.lower() for _, message, _ in recorder.messages)
+ assert recorder.messages == []
def test_zipformat_structure_rejects_dated_src_suffix(tmp_path: pathlib.Path) -> None:
diff --git a/tests/unit/test_checks_compare.py b/tests/unit/test_checks_compare.py
index e4072b26..0b7e05dd 100644
--- a/tests/unit/test_checks_compare.py
+++ b/tests/unit/test_checks_compare.py
@@ -22,6 +22,7 @@ import subprocess
from collections.abc import Callable, Iterable, Mapping
import aiofiles.os
+import dulwich.client
import dulwich.objects
import dulwich.refs
import pytest
@@ -33,6 +34,7 @@ import atr.models.safe as safe
import atr.models.sql
import atr.tasks.checks
import atr.tasks.checks.compare
+import atr.tasks.task as task
class CheckoutRecorder:
@@ -743,6 +745,20 @@ async def test_source_trees_permits_pkg_info_when_pyproject_toml_exists(
@pytest.mark.asyncio
+async def test_checkout_returns_none_for_http_auth_failures(
+ monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path
+) -> None:
+ def raise_unauthorized(repo_url: str, sha: str, checkout_dir: safe.StatePath) -> None:
+ raise dulwich.client.HTTPUnauthorized("Basic", repo_url)
+
+ monkeypatch.setattr(atr.tasks.checks.compare, "_clone_repo", raise_unauthorized)
+ payload = _make_payload()
+
+ checkout_dir = await atr.tasks.checks.compare._checkout_github_source(payload, safe.StatePath(tmp_path))
+
+ assert checkout_dir is None
+
+
async def test_source_trees_raises_when_checkout_fails(monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path) -> None:
temp_dir = safe.StatePath(tmp_path)
recorder = RecorderStub(True)
@@ -757,14 +773,12 @@ async def test_source_trees_raises_when_checkout_fails(monkeypatch: pytest.Monke
monkeypatch.setattr(atr.tasks.checks, "resolve_archive_dir", ArchiveDirResolver(cache_dir))
monkeypatch.setattr(atr.tasks.checks.compare.paths, "get_tmp_dir", ReturnValue(tmp_root))
- with pytest.raises(RuntimeError, match=r"Failed to clone .* for source tree comparison"):
+ with pytest.raises(task.CheckRetryableError, match="Failed to clone GitHub repository for comparison") as excinfo:
await atr.tasks.checks.compare.source_trees(args)
- assert len(recorder.exception_calls) == 1
- message, data = recorder.exception_calls[0]
- assert message == "Failed to clone GitHub repository for comparison"
- assert isinstance(data, dict)
- assert data["repo_url"] == "https://github.com/apache/test.git"
+ assert recorder.exception_calls == []
+ assert isinstance(excinfo.value.data, dict)
+ assert excinfo.value.data["repo_url"] == "https://github.com/apache/test.git"
@pytest.mark.asyncio
@@ -829,7 +843,7 @@ async def test_source_trees_records_failure_when_archive_root_not_found(
@pytest.mark.asyncio
-async def test_source_trees_records_failure_when_cache_dir_unavailable(
+async def test_source_trees_raises_when_cache_dir_unavailable(
monkeypatch: pytest.MonkeyPatch,
) -> None:
recorder = RecorderStub(True)
@@ -845,12 +859,10 @@ async def test_source_trees_records_failure_when_cache_dir_unavailable(
)
monkeypatch.setattr(atr.tasks.checks.compare.paths, "get_tmp_dir", RaiseSync("get_tmp_dir should not be called"))
- await atr.tasks.checks.compare.source_trees(args)
+ with pytest.raises(task.CheckRetryableError, match="Extracted archive tree is not available"):
+ await atr.tasks.checks.compare.source_trees(args)
- assert len(recorder.exception_calls) == 1
- message, data = recorder.exception_calls[0]
- assert message == "Extracted archive tree is not available"
- assert isinstance(data, dict)
+ assert recorder.exception_calls == []
@pytest.mark.asyncio
diff --git a/tests/unit/test_checks_license.py b/tests/unit/test_checks_license.py
index 332c6fd2..f4b89afa 100644
--- a/tests/unit/test_checks_license.py
+++ b/tests/unit/test_checks_license.py
@@ -18,10 +18,13 @@
import pathlib
import tarfile
+import pytest
+
import atr.constants as constants
import atr.models.safe as safe
import atr.models.sql as sql
import atr.tasks.checks.license as license
+import atr.tasks.task as task
TEST_ARCHIVE = pathlib.Path(__file__).parent.parent / "e2e" / "test_files" / "apache-test-0.2.tar.gz"
TEST_ARCHIVE_BASENAME: str = TEST_ARCHIVE.name
@@ -109,12 +112,35 @@ def test_files_license_accepts_https_urls(tmp_path):
def test_files_missing_cache_dir():
- results = list(
- license._files_check_core_logic(safe.StatePath(pathlib.Path("/nonexistent")), is_podling=False, is_binary=False)
- )
- assert len(results) == 1
- assert results[0].status == sql.CheckResultStatus.EXCEPTION
- assert "not available" in results[0].message.lower()
+ with pytest.raises(task.CheckRetryableError, match="Cache directory is not available"):
+ list(
+ license._files_check_core_logic(
+ safe.StatePath(pathlib.Path("/nonexistent")), is_podling=False, is_binary=False
+ )
+ )
+
+
+def test_headers_member_error_escapes_for_non_io_failures(tmp_path, monkeypatch):
+ source_path = tmp_path / "example.py"
+ source_path.write_text("print()\n")
+
+ def broken_validate(content: bytes, filename: str) -> tuple[bool, str | None]:
+ raise ValueError("broken checker")
+
+ monkeypatch.setattr(license, "headers_validate", broken_validate)
+ with pytest.raises(ValueError, match="broken checker"):
+ license._headers_check_core_logic_process_file(source_path, "example.py")
+
+
+def test_headers_member_error_is_recorded_for_io_failures(tmp_path):
+ source_path = tmp_path / "example.py"
+ source_path.write_text("print()\n")
+ source_path.chmod(0)
+
+ result = license._headers_check_core_logic_process_file(source_path, "example.py")
+
+ assert isinstance(result, license.MemberResult)
+ assert result.status == sql.CheckResultStatus.EXCEPTION
def test_files_multiple_root_dirs(tmp_path):
diff --git a/tests/unit/test_checks_rat.py b/tests/unit/test_checks_rat.py
index 01b900e7..cdd709b7 100644
--- a/tests/unit/test_checks_rat.py
+++ b/tests/unit/test_checks_rat.py
@@ -28,6 +28,7 @@ import atr.models.safe as safe
import atr.models.sql as sql
import atr.tasks.checks as checks
import atr.tasks.checks.rat as rat
+import atr.tasks.task as task
import tests.unit.recorders as recorders
# Archive WITHOUT .rat-excludes
@@ -77,7 +78,7 @@ def test_check_includes_excludes_source_policy(rat_available: tuple[bool, bool],
@pytest.mark.asyncio
-async def test_check_routes_errors_to_exception(tmp_path: pathlib.Path) -> None:
+async def test_check_raises_retryable_for_tooling_errors(tmp_path: pathlib.Path) -> None:
recorder, args = await _rat_check_args(tmp_path, "apache-example-1.2.3-source.tar.gz")
project = types.SimpleNamespace(
policy_license_check_mode=sql.LicenseCheckMode.BOTH,
@@ -89,6 +90,30 @@ async def test_check_routes_errors_to_exception(tmp_path: pathlib.Path) -> None:
errors=["simulated tooling failure"],
)
+ with (
+ mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=safe.StatePath(tmp_path))),
+ mock.patch.object(recorder, "project", new=mock.AsyncMock(return_value=project)),
+ mock.patch.object(rat, "_synchronous", new=mock.Mock(return_value=result)),
+ ):
+ with pytest.raises(task.CheckRetryableError, match="Apache RAT process failed"):
+ await rat.check(args)
+
+ assert recorder.messages == []
+
+
+async def test_check_records_concern_for_structural_errors(tmp_path: pathlib.Path) -> None:
+ recorder, args = await _rat_check_args(tmp_path, "apache-example-1.2.3-source.tar.gz")
+ project = types.SimpleNamespace(
+ policy_license_check_mode=sql.LicenseCheckMode.BOTH,
+ policy_source_excludes_rat=[],
+ )
+ result = checkdata.Rat(
+ valid=False,
+ message="Multiple .rat-excludes files not allowed (found 2)",
+ errors=["Found 2 .rat-excludes files"],
+ structural=True,
+ )
+
with (
mock.patch.object(checks, "resolve_archive_dir", new=mock.AsyncMock(return_value=safe.StatePath(tmp_path))),
mock.patch.object(recorder, "project", new=mock.AsyncMock(return_value=project)),
@@ -98,8 +123,8 @@ async def test_check_routes_errors_to_exception(tmp_path: pathlib.Path) -> None:
statuses = [status for status, _, _ in recorder.messages]
- assert statuses == [sql.CheckResultStatus.EXCEPTION.value]
- assert any("Apache RAT process failed" in message for _, message, _ in recorder.messages)
+ assert statuses == [sql.CheckResultStatus.CONCERN.value]
+ assert any("Multiple .rat-excludes files" in message for _, message, _ in recorder.messages)
@pytest.mark.asyncio
diff --git a/tests/unit/test_checks_signature.py b/tests/unit/test_checks_signature.py
index 57096be8..783803fb 100644
--- a/tests/unit/test_checks_signature.py
+++ b/tests/unit/test_checks_signature.py
@@ -25,6 +25,7 @@ import atr.models.safe as safe
import atr.models.sql as sql
import atr.tasks.checks as checks
import atr.tasks.checks.signature as signature_check
+import atr.tasks.task as task
import tests.unit.pgp_fixtures as pgp_fixtures
import tests.unit.recorders as recorders
@@ -291,6 +292,36 @@ def test_check_core_logic_rejects_signature_from_a_revoked_subkey(tmp_path: path
assert result["error_kind"] == "revoked_key"
+def test_check_core_logic_missing_artifact_is_a_blocker(tmp_path: pathlib.Path) -> None:
+ signature_path, _artifact_path = pgp_fixtures.write_expired_subkey_fixture(tmp_path)
+
+ result = signature_check._check_core_logic_verify_signature(
+ signature_path=signature_path,
+ artifact_path=str(tmp_path / "nonexistent.tar.gz"),
+ ascii_armored_keys=[pgp_fixtures.EXPIRED_SUBKEY_PUBLIC_KEY_ASC],
+ apache_uid_map={},
+ uploader_fingerprints=set(),
+ )
+
+ assert result["verified"] is False
+ assert result["error"] == "Referenced artifact not found"
+ assert result["error_kind"] == "missing_artifact"
+
+
+def test_check_core_logic_unreadable_artifact_is_retryable(tmp_path: pathlib.Path) -> None:
+ signature_path, artifact_path = pgp_fixtures.write_expired_subkey_fixture(tmp_path)
+ pathlib.Path(artifact_path).chmod(0)
+
+ with pytest.raises(task.CheckRetryableError, match="Unable to read the artifact file"):
+ signature_check._check_core_logic_verify_signature(
+ signature_path=signature_path,
+ artifact_path=artifact_path,
+ ascii_armored_keys=[pgp_fixtures.EXPIRED_SUBKEY_PUBLIC_KEY_ASC],
+ apache_uid_map={},
+ uploader_fingerprints=set(),
+ )
+
+
def test_check_core_logic_rejects_signature_from_an_expired_subkey(tmp_path: pathlib.Path) -> None:
signature_path, artifact_path = pgp_fixtures.write_expired_subkey_fixture(tmp_path)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org