This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-release.git
The following commit(s) were added to refs/heads/main by this push:
new 6c2d26b Add path checks
6c2d26b is described below
commit 6c2d26b8d19d1e03e21d06d0a198fb122915e38d
Author: Sean B. Palmer <[email protected]>
AuthorDate: Tue Apr 1 16:38:29 2025 +0100
Add path checks
---
atr/db/models.py | 2 +-
atr/tasks/__init__.py | 16 +---
atr/tasks/checks/__init__.py | 17 ++--
atr/tasks/checks/paths.py | 177 +++++++++++++++++++++++++++++++++++
atr/tasks/rsync.py | 29 ++----
atr/templates/draft-review-path.html | 75 ++++++++-------
atr/templates/draft-review.html | 4 +-
7 files changed, 247 insertions(+), 73 deletions(-)
diff --git a/atr/db/models.py b/atr/db/models.py
index 2279502..386be9d 100644
--- a/atr/db/models.py
+++ b/atr/db/models.py
@@ -290,7 +290,7 @@ class TaskType(str, enum.Enum):
HASHING_CHECK = "hashing_check"
LICENSE_FILES = "license_files"
LICENSE_HEADERS = "license_headers"
- # PATHS_CHECK = "paths_check"
+ PATHS_CHECK = "paths_check"
RAT_CHECK = "rat_check"
RSYNC_ANALYSE = "rsync_analyse"
SIGNATURE_CHECK = "signature_check"
diff --git a/atr/tasks/__init__.py b/atr/tasks/__init__.py
index e642cd7..2c200b6 100644
--- a/atr/tasks/__init__.py
+++ b/atr/tasks/__init__.py
@@ -23,6 +23,7 @@ import atr.db.models as models
import atr.tasks.checks.archive as archive
import atr.tasks.checks.hashing as hashing
import atr.tasks.checks.license as license
+import atr.tasks.checks.paths as paths
import atr.tasks.checks.rat as rat
import atr.tasks.checks.signature as signature
import atr.tasks.rsync as rsync
@@ -75,8 +76,8 @@ def resolve(task_type: models.TaskType) -> Callable[...,
Awaitable[str | None]]:
return license.files
case models.TaskType.LICENSE_HEADERS:
return license.headers
- # case models.TaskType.PATHS_CHECK:
- # return paths.check
+ case models.TaskType.PATHS_CHECK:
+ return paths.check
case models.TaskType.RAT_CHECK:
return rat.check
case models.TaskType.RSYNC_ANALYSE:
@@ -173,17 +174,6 @@ async def tar_gz_checks(release: models.Release, path:
str) -> list[models.Task]
path=path,
modified=modified,
),
- # models.Task(
- # status=models.TaskStatus.QUEUED,
- # task_type=models.TaskType.SBOM_GENERATE_CYCLONEDX,
- # task_args=tasks.SbomGenerateCyclonedx(
- # artifact_path=str(full_path),
- # output_path=str(full_sbom_path),
- # ).model_dump(),
- # release_name=release.name,
- # path=path,
- # modified=modified,
- # ),
]
return tasks
diff --git a/atr/tasks/checks/__init__.py b/atr/tasks/checks/__init__.py
index dcc34b5..921d1c4 100644
--- a/atr/tasks/checks/__init__.py
+++ b/atr/tasks/checks/__init__.py
@@ -20,6 +20,7 @@ from __future__ import annotations
import datetime
import pathlib
from functools import wraps
+from types import FunctionType
from typing import TYPE_CHECKING, Any, TypeVar
import pydantic
@@ -35,9 +36,13 @@ import atr.db.models as models
class Check:
def __init__(
- self, checker: Callable[..., Any], release_name: str, path: str | None
= None, afresh: bool = True
+ self, checker: str | Callable[..., Any], release_name: str, path: str
| None = None, afresh: bool = True
) -> None:
- self.checker = function_key(checker)
+ if isinstance(checker, FunctionType):
+ checker = function_key(checker)
+ if not isinstance(checker, str):
+ raise ValueError("Checker must be a string or a callable")
+ self.checker = checker
self.release_name = release_name
self.path = path
self.afresh = afresh
@@ -45,12 +50,12 @@ class Check:
@classmethod
async def create(
- cls, checker: Callable[..., Any], release_name: str, path: str | None
= None, afresh: bool = True
+ cls, checker: str | Callable[..., Any], release_name: str, path: str |
None = None, afresh: bool = True
) -> Check:
check = cls(checker, release_name, path, afresh)
if afresh is True:
# Clear outer path whether it's specified or not
- await check._clear(path)
+ await check.clear(path)
check._constructed = True
return check
@@ -64,7 +69,7 @@ class Check:
raise ValueError("Cannot specify path twice")
if self.afresh is True:
# Clear inner path only if it's specified
- await self._clear(path)
+ await self.clear(path)
result = models.CheckResult(
release_name=self.release_name,
@@ -84,7 +89,7 @@ class Check:
await session.commit()
return result
- async def _clear(self, path: str | None = None) -> None:
+ async def clear(self, path: str | None = None) -> None:
async with db.session() as data:
stmt = sqlmodel.delete(models.CheckResult).where(
db.validate_instrumented_attribute(models.CheckResult.release_name) ==
self.release_name,
diff --git a/atr/tasks/checks/paths.py b/atr/tasks/checks/paths.py
new file mode 100644
index 0000000..64451f5
--- /dev/null
+++ b/atr/tasks/checks/paths.py
@@ -0,0 +1,177 @@
+# 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 logging
+import pathlib
+import re
+from typing import Final
+
+import aiofiles.os
+import pydantic
+
+import atr.analysis as analysis
+import atr.tasks.checks as checks
+import atr.util as util
+
+_LOGGER: Final = logging.getLogger(__name__)
+
+
+class Check(pydantic.BaseModel):
+ """Arguments for the path structure and naming convention check."""
+
+ release_name: str = pydantic.Field(..., description="Name of the release
being checked")
+ base_release_dir: str = pydantic.Field(..., description="Absolute path to
the base directory of the release draft")
+
+
+async def _check_artifact_rules(base_path: pathlib.Path, relative_path:
pathlib.Path, errors: list[str]) -> None:
+ """Check rules specific to artifact files."""
+ full_path = base_path / relative_path
+
+ # RDP says that .asc is required
+ asc_path = full_path.with_suffix(full_path.suffix + ".asc")
+ if not await aiofiles.os.path.exists(asc_path):
+ errors.append(f"Missing corresponding signature file
({relative_path}.asc)")
+
+ # RDP requires one of .sha256 or .sha512
+ sha256_path = full_path.with_suffix(full_path.suffix + ".sha256")
+ sha512_path = full_path.with_suffix(full_path.suffix + ".sha512")
+ has_sha256 = await aiofiles.os.path.exists(sha256_path)
+ has_sha512 = await aiofiles.os.path.exists(sha512_path)
+ if not (has_sha256 or has_sha512):
+ errors.append(f"Missing corresponding checksum file
({relative_path}.sha256 or .sha512)")
+
+
+async def _check_metadata_rules(
+ base_path: pathlib.Path, relative_path: pathlib.Path, ext_metadata: str,
errors: list[str], warnings: list[str]
+) -> None:
+ """Check rules specific to metadata files (.asc, .sha*, etc.)."""
+ suffixes = set(relative_path.suffixes)
+
+ if ".md5" in suffixes:
+ # Forbidden by RCP, deprecated by RDP
+ errors.append("The use of .md5 is forbidden, please use .sha512")
+ if ".sha1" in suffixes:
+ # Deprecated by RDP
+ warnings.append("The use of .sha1 is deprecated, please use .sha512")
+ if ".sha" in suffixes:
+ # Discouraged by RDP
+ warnings.append("The use of .sha is discouraged, please use .sha512")
+ if ".sig" in suffixes:
+ # Forbidden by RCP, forbidden by RDP
+ errors.append("Binary signature files (.sig) are forbidden, please use
.asc")
+
+ # "Signature and checksum files for verifying distributed artifacts should
+ # not be provided, unless named as indicated above." (RDP)
+ # Also .mds is allowed, but we'll ignore that for now
+ # TODO: Is .mds supported in analysis.METADATA_SUFFIXES?
+ if ext_metadata not in {".asc", ".sha256", ".sha512", ".md5", ".sha",
".sha1"}:
+ warnings.append("The use of this metadata file is discouraged")
+
+ # Check whether the corresponding artifact exists
+ artifact_path_base = str(relative_path).removesuffix(ext_metadata)
+ full_artifact_path = base_path / artifact_path_base
+ if not await aiofiles.os.path.exists(full_artifact_path):
+ errors.append(f"Metadata file exists but corresponding artifact
'{artifact_path_base}' is missing")
+
+
+async def _check_path_process_single(
+ base_path: pathlib.Path,
+ relative_path: pathlib.Path,
+ check_errors: checks.Check,
+ check_warnings: checks.Check,
+ check_success: checks.Check,
+) -> None:
+ """Process and check a single path within the release directory."""
+ full_path = base_path / relative_path
+ relative_path_str = str(relative_path)
+
+ errors: list[str] = []
+ warnings: list[str] = []
+
+ # The Release Distribution Policy specifically allows README and CHANGES,
etc.
+ # We assume that LICENSE and NOTICE are permitted also
+ if relative_path.name == "KEYS":
+ errors.append("The KEYS file should be uploaded via the 'Keys'
section, not included in the artifact bundle")
+ if any(part.startswith(".") for part in relative_path.parts):
+ # TODO: There is not a a policy for this
+ # We should enquire as to whether such a policy should be instituted
+ # We're forbidding dotfiles to catch accidental uploads of e.g. .git
or .htaccess
+ # Such cases are likely to be in error, and could carry security risks
+ errors.append("Dotfiles are forbidden")
+
+ search = re.search(analysis.extension_pattern(), relative_path_str)
+ ext_artifact = search.group("artifact") if search else None
+ ext_metadata = search.group("metadata") if search else None
+
+ if ext_artifact:
+ _LOGGER.info("Checking artifact rules for %s", full_path)
+ await _check_artifact_rules(base_path, relative_path, errors)
+ elif ext_metadata:
+ _LOGGER.info("Checking metadata rules for %s", full_path)
+ await _check_metadata_rules(base_path, relative_path, ext_metadata,
errors, warnings)
+ else:
+ _LOGGER.info("Checking general rules for %s", full_path)
+ allowed_top_level = {"LICENSE", "NOTICE", "README", "CHANGES"}
+ if (relative_path.parent == pathlib.Path(".")) and (relative_path.name
not in allowed_top_level):
+ warnings.append(f"Unknown top level file: {relative_path.name}")
+
+ # Must aggregate errors and aggregate warnings otherwise they will be
removed by afresh=True
+ # Alternatively we could call Check.clear() manually
+ if errors:
+ await check_errors.failure("; ".join(errors), {"errors": errors},
path=relative_path_str)
+ if warnings:
+ await check_warnings.warning("; ".join(warnings), {"warnings":
warnings}, path=relative_path_str)
+ if not (errors or warnings):
+ await check_success.success(
+ "Path structure and naming conventions conform to policy", {},
path=relative_path_str
+ )
+
+
[email protected]_model(Check)
+async def check(args: Check) -> None:
+ """Check file path structure and naming conventions against ASF release
policy for all files in a release."""
+ # We refer to the following authoritative policies:
+ # - Release Creation Process (RCP)
+ # - Release Distribution Policy (RDP)
+ base_path = pathlib.Path(args.base_release_dir)
+
+ if not await aiofiles.os.path.isdir(base_path):
+ _LOGGER.error("Base release directory does not exist or is not a
directory: %s", base_path)
+ return
+
+ check_errors = await checks.Check.create(
+ checker=checks.function_key(check) + "_errors",
release_name=args.release_name, path=None, afresh=True
+ )
+ check_warnings = await checks.Check.create(
+ checker=checks.function_key(check) + "_warnings",
release_name=args.release_name, path=None, afresh=True
+ )
+ check_success = await checks.Check.create(
+ checker=checks.function_key(check) + "_success",
release_name=args.release_name, path=None, afresh=True
+ )
+ relative_paths = await util.paths_recursive(base_path)
+
+ for relative_path in relative_paths:
+ # Delegate processing of each path to the helper function
+ await _check_path_process_single(
+ base_path,
+ relative_path,
+ check_errors,
+ check_warnings,
+ check_success,
+ )
+
+ return None
diff --git a/atr/tasks/rsync.py b/atr/tasks/rsync.py
index 8aa9cca..8a66aed 100644
--- a/atr/tasks/rsync.py
+++ b/atr/tasks/rsync.py
@@ -25,8 +25,7 @@ import atr.db as db
import atr.db.models as models
import atr.tasks as tasks
import atr.tasks.checks as checks
-
-# import atr.tasks.checks.paths as paths
+import atr.tasks.checks.paths as paths
import atr.util as util
if TYPE_CHECKING:
@@ -93,23 +92,15 @@ async def _analyse_core(project_name: str, release_version:
str) -> dict[str, An
if task.task_type not in cached_tasks:
data.add(task)
- # # Add the generic path check task for every file
- # if path_check_task_key not in cached_tasks:
- # path_check_task_args = paths.Check(
- # release_name=release_name,
- # base_release_dir=str(base_path),
- # path=str(path),
- # ).model_dump()
-
- # path_check_task = models.Task(
- # status=models.TaskStatus.QUEUED,
- # task_type=tasks.Type.PATHS_CHECK,
- # task_args=paths.Check(
- # release_name=release_name,
- # base_release_dir=str(base_path),
- # path=str(path),
- # ).model_dump(),
- # )
+ path_check_task = models.Task(
+ status=models.TaskStatus.QUEUED,
+ task_type=models.TaskType.PATHS_CHECK,
+ task_args=paths.Check(
+ release_name=release_name,
+ base_release_dir=str(base_path),
+ ).model_dump(),
+ )
+ data.add(path_check_task)
await data.commit()
return {"paths": [str(path) for path in paths_recursive]}
diff --git a/atr/templates/draft-review-path.html
b/atr/templates/draft-review-path.html
index 1b05b55..87cc96b 100644
--- a/atr/templates/draft-review-path.html
+++ b/atr/templates/draft-review-path.html
@@ -32,38 +32,37 @@
<h2>Verification tasks</h2>
{% if check_results %}
+ {# Define status_counts here so it's available globally in this block #}
+ {% set status_counts = {
+ "success": check_results|selectattr("status.value", "equalto",
"success")|list|length,
+ "failure": check_results|selectattr("status.value", "equalto",
"failure")|list|length,
+ "warning": check_results|selectattr("status.value", "equalto",
"warning")|list|length,
+ "exception": check_results|selectattr("status.value", "equalto",
"exception")|list|length
+ } %}
+
<div class="d-flex align-items-center p-3 mb-3 bg-light border rounded">
<span class="fw-bold me-3">Status summary:</span>
<div class="d-flex flex-wrap gap-3">
- {% with %}
- {% set status_counts = {
- "success": check_results|selectattr("status.value",
"equalto", "success")|list|length,
- "failure": check_results|selectattr("status.value",
"equalto", "failure")|list|length,
- "warning": check_results|selectattr("status.value",
"equalto", "warning")|list|length,
- "exception": check_results|selectattr("status.value",
"equalto", "exception")|list|length
- } %}
-
- {% for status, count in status_counts.items() %}
- {% if count > 0 %}
- <div class="d-flex align-items-center gap-2 px-3 py-2 rounded
fw-medium {% if status == "success" %}bg-success-subtle border
border-success-subtle {% elif status == "failure" %}bg-danger-subtle border
border-danger-subtle {% elif status == "warning" %}bg-warning-subtle border
border-warning-subtle {% elif status == "exception" %}bg-danger-subtle border
border-danger-subtle {% endif %}">
- <span class="fs-5">{{ count }}</span>
- <span>
- {%- if status == "success" -%}
- Passed
- {%- elif status == "failure" -%}
- Issues
- {%- elif status == "warning" -%}
- Warning
- {%- elif status == "exception" -%}
- Error
- {%- else -%}
- {{ status|title }}
- {%- endif -%}
- </span>
- </div>
- {% endif %}
- {% endfor %}
- {% endwith %}
+ {% for status, count in status_counts.items() %}
+ {% if count > 0 %}
+ <div class="d-flex align-items-center gap-2 px-3 py-2 rounded
fw-medium {% if status == "success" %}bg-success-subtle border
border-success-subtle {% elif status == "failure" %}bg-danger-subtle border
border-danger-subtle {% elif status == "warning" %}bg-warning-subtle border
border-warning-subtle {% elif status == "exception" %}bg-danger-subtle border
border-danger-subtle {% endif %}">
+ <span class="fs-5">{{ count }}</span>
+ <span>
+ {%- if status == "success" -%}
+ Passed
+ {%- elif status == "failure" -%}
+ Issues
+ {%- elif status == "warning" -%}
+ Warning
+ {%- elif status == "exception" -%}
+ Error
+ {%- else -%}
+ {{ status|title }}
+ {%- endif -%}
+ </span>
+ </div>
+ {% endif %}
+ {% endfor %}
</div>
</div>
{% endif %}
@@ -83,11 +82,23 @@
{%- if check_result.status.value == "success" -%}
Passed
{%- elif check_result.status.value == "failure" -%}
- Issue
+ {% if status_counts['failure'] == 1 %}
+ Issue
+ {% else %}
+ Issues
+ {% endif %}
{%- elif check_result.status.value == "warning" -%}
- Warning
+ {% if status_counts['warning'] == 1 %}
+ Warning
+ {% else %}
+ Warnings
+ {% endif %}
{%- elif check_result.status.value == "exception" -%}
- Error
+ {% if status_counts['exception'] == 1 %}
+ Exception
+ {% else %}
+ Exceptions
+ {% endif %}
{%- else -%}
{{ check_result.status.value|title }}
{%- endif -%}
diff --git a/atr/templates/draft-review.html b/atr/templates/draft-review.html
index 23f7bee..3c83bb0 100644
--- a/atr/templates/draft-review.html
+++ b/atr/templates/draft-review.html
@@ -83,7 +83,7 @@
<span class="badge bg-success">{{
successes[path]|length }} Passed</span>
{% endif %}
{% if warnings[path]|length > 0 %}
- <span class="badge bg-warning">{{
warnings[path]|length }} {{ "Issue" if warnings[path]|length == 1 else "Issues"
}}</span>
+ <span class="badge bg-warning">{{
warnings[path]|length }} {{ "Warning" if warnings[path]|length == 1 else
"Warnings" }}</span>
{% endif %}
{% if errors[path]|length > 0 %}
<span class="badge bg-danger">{{ errors[path]|length
}} {{ "Issue" if errors[path]|length == 1 else "Issues" }}</span>
@@ -98,7 +98,7 @@
class="btn btn-sm btn-outline-primary fs-6 small
ms-2">Tools</a>
<a href="{{ as_url(routes.draft.review_path,
project_name=project_name, version_name=version_name, file_path=path) }}"
class="btn btn-sm btn-outline-primary fs-6 small
ms-2">Review file</a>
- <button class="btn btn-sm btn-outline-danger"
+ <button class="btn btn-sm btn-outline-danger ms-2"
data-bs-toggle="modal"
data-bs-target="#delete-{{ path|slugify }}">Delete
file</button>
</td>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
