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/21 18:52:59 UTC
(tooling-trusted-releases) branch sbp updated (5545dc1a -> 75c7612d)
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
from 5545dc1a Parse attestable checks files in JSON mode
add a95c7ab8 Remove links to ATR's own downloads area
add 5def9227 #1417 - Reject on revoked
add 8c36af5a #1427 - revert token change
add b424c793 #1420 - auto-archive on CAP approval and handle failure to do so
new 75c7612d Remove all writes to the downloads directory
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/api/__init__.py | 1 -
atr/config.py | 4 +-
atr/construct.py | 9 +-
atr/docs/storage-interface.md | 13 +-
atr/get/__init__.py | 2 -
atr/get/announce.py | 3 +-
atr/get/file.py | 39 +++--
atr/get/published.py | 126 ---------------
atr/get/voting.py | 24 ---
atr/paths.py | 14 +-
atr/pgp.py | 26 +++-
atr/post/draft.py | 1 -
atr/post/keys.py | 4 +
atr/post/projects.py | 40 ++---
atr/server.py | 8 +-
atr/shared/catalog.py | 30 ++--
atr/storage/__init__.py | 2 +
atr/storage/datatypes.py | 4 +-
atr/storage/writers/announce.py | 72 +--------
atr/storage/writers/keys.py | 127 ++++++++++-----
atr/storage/writers/project.py | 28 ++++
atr/storage/writers/release.py | 231 ++++++++--------------------
atr/tasks/cap.py | 61 +++++++-
atr/tasks/checks/__init__.py | 18 ++-
atr/tasks/checks/signature.py | 35 ++++-
atr/util.py | 54 ++-----
tests/e2e/announce/test_get.py | 7 +-
tests/unit/pgp_fixtures.py | 105 +++++++++++++
tests/unit/test_announce_deletion_order.py | 2 -
tests/unit/test_catalog.py | 65 +++++---
tests/unit/test_checks_signature.py | 16 ++
tests/unit/test_construct_announce.py | 35 +----
tests/unit/test_keys_delete.py | 38 ++++-
tests/unit/test_keys_writer.py | 60 +++-----
tests/unit/test_paths.py | 28 ++--
tests/unit/test_pgp.py | 54 +++++++
tests/unit/test_release_archive_approval.py | 33 ++--
tests/unit/test_release_writer_archive.py | 61 ++------
tests/unit/test_svn_publish_helpers.py | 64 ++++----
39 files changed, 769 insertions(+), 775 deletions(-)
delete mode 100644 atr/get/published.py
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org
(tooling-trusted-releases) 01/01: Remove all writes to the downloads directory
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 75c7612d41e78e205b51d0f42814dfb24b97ec4b
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Tue Jul 21 19:52:24 2026 +0100
Remove all writes to the downloads directory
---
atr/api/__init__.py | 1 -
atr/docs/storage-interface.md | 13 +--
atr/get/__init__.py | 2 -
atr/get/published.py | 126 ------------------------
atr/get/voting.py | 24 -----
atr/post/draft.py | 1 -
atr/server.py | 8 +-
atr/storage/datatypes.py | 2 +-
atr/storage/writers/announce.py | 70 -------------
atr/storage/writers/keys.py | 59 ++++-------
atr/storage/writers/release.py | 146 ++++------------------------
tests/unit/test_announce_deletion_order.py | 2 -
tests/unit/test_keys_writer.py | 60 ++++--------
tests/unit/test_release_archive_approval.py | 3 +-
tests/unit/test_release_writer_archive.py | 61 ++----------
15 files changed, 87 insertions(+), 491 deletions(-)
diff --git a/atr/api/__init__.py b/atr/api/__init__.py
index b7409621..4eac6e99 100644
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -1370,7 +1370,6 @@ async def release_draft_delete(
data.project,
data.version,
phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
- include_downloads=False,
)
except storage.AccessError as e:
raise _http_exception_from_storage_access_error(e) from e
diff --git a/atr/docs/storage-interface.md b/atr/docs/storage-interface.md
index c0a528d5..8b219cca 100644
--- a/atr/docs/storage-interface.md
+++ b/atr/docs/storage-interface.md
@@ -173,7 +173,6 @@ The storage interface writes to the [database](database) and the filesystem. The
Only a small number of subdirectories of the state directory are written to by the storage interface, and many of these locations are also configurable. These directories, and their configuration variables, are:
* `attestable`, configured by `ATTESTABLE_STORAGE_DIR`
-* `downloads`, configured by `DOWNLOADS_STORAGE_DIR`
* `finished`, configured by `FINISHED_STORAGE_DIR`
* `subversion`, configured by `SVN_STORAGE_DIR`
* `temporary`, which is unconfigurable
@@ -182,8 +181,7 @@ Only a small number of subdirectories of the state directory are written to by t
And the purposes of these directories is as follows. Note that "immutable" here means that existing files cannot be modified, but does not preclude new files from being added.
* `attestable` [**immutable**] holds JSON files of data that ATR has automatically verified and which must now be held immutably. (We could store this data in the database, but the aim is to eventually write attestation files here, so this prepares for that approach.)
-* `downloads` [**mutable**] are hard links to released artifacts in the `finished` directory. The `finished` directory contains the files exactly as they were arranged by the release managers upon announcing the release, separated strictly into one directory per release. The `downloads` folder, on the other hand, has no restrictions on its organisation and can be rearranged.
-* `finished` [**immutable**, except for moving to external archive] contains, as mentioned above, all of the files of a release as they were when announced. This therefore constitutes an historical record and allows us to rewrite the hard links in the `downloads` directory without having to consider not accidentally deleting files by removing all references, etc.
+* `finished` [**immutable**, except for moving to external archive] contains all of the files of a release as they were when announced. The files are arranged exactly as the release managers arranged them upon announcing the release, separated strictly into one directory per release. This therefore constitutes an historical record.
* `subversion` [**mutable**] is designed to mirror two subdirectories, `dev` and `release`, of `https://dist.apache.org/repos/dist`. This is currently unused.
* `temporary` [**mutable**] holds temporary files during operations where the data cannot be modified in place. One important example is when creating a staging directory of a new revision. A subdirectory with a random name is made in this directory, and then the files in the prior version are hard linked into it. The modifications take place in this staging area before the directory is finalised and moved to `unfinished`.
* `unfinished` [**immutable**, except for moving to `finished`] contains all of the files in a release before it is announced. In other words, when the release managers compose a release, when the committee votes on the release, and when the release has been voted on but not yet announced, the files for that release are in this directory.
@@ -192,16 +190,15 @@ This list does not include any configuration files, logs, or log directories.
## How should the filesystem be backed up?
-Only the `attestable`, `downloads`, `finished`, and `unfinished` directories need to be backed up. The `subversion` directory is unused, and the `temporary` directory is for temporary staging.
+Only the `attestable`, `finished`, and `unfinished` directories need to be backed up. The `subversion` directory is unused, and the `temporary` directory is for temporary staging.
The structure of the directories that need backing up is as follows. An ellipsis, `...`, means any number of further files or subdirectories containing subdirectories or files recursively.
* `attestable/PROJECT/VERSION/REVISION.json`
-* `downloads/COMMITTEE/PATH/...`
* `finished/PROJECT/VERSION/...`
* `unfinished/PROJECT/VERSION/REVISION/`
-Because of the versioning scheme used for the `attestable`, `finished`, and `unfinished` directories, these can be incrementally updated by simple copying without deletion. The downloads directory, however, must be snapshotted as its organization is random.
+Because of the versioning scheme used for the `attestable`, `finished`, and `unfinished` directories, these can be incrementally updated by simple copying without deletion.
This list does not include any configuration files, logs, or log directories. All configuration files and the audit logs, at a minimum, should also be backed up.
@@ -239,7 +236,7 @@ This could just as easily be a Pydantic class or whatever is appropriate in the
@dataclasses.dataclass
class LinkedCommittee:
name: str
- autogenerated_keys_file: Outcome[str]
+ autogenerated_keys_file: Outcome[int]
```
-In this case, if the autogenerated keys file call succeeded without an error, the `Outcome` will be an `OutcomeResult[str]` where the `str` represents the full path to the autogenerated file.
+In this case, if the autogenerated keys file call succeeded without an error, the `Outcome` will be an `OutcomeResult[int]` where the `int` is the number of keys in the generated file.
diff --git a/atr/get/__init__.py b/atr/get/__init__.py
index 6518a7e1..5d4a0718 100644
--- a/atr/get/__init__.py
+++ b/atr/get/__init__.py
@@ -36,7 +36,6 @@ import atr.get.keys as keys
import atr.get.manual as manual
import atr.get.preview as preview
import atr.get.projects as projects
-import atr.get.published as published
import atr.get.ref as ref
import atr.get.release as release
import atr.get.report as report
@@ -73,7 +72,6 @@ __all__ = [
"manual",
"preview",
"projects",
- "published",
"ref",
"release",
"report",
diff --git a/atr/get/published.py b/atr/get/published.py
deleted file mode 100644
index 4108cc7e..00000000
--- a/atr/get/published.py
+++ /dev/null
@@ -1,126 +0,0 @@
-# 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 pathlib
-import stat
-from datetime import datetime
-from typing import Literal
-
-import aiofiles.os
-import quart
-
-import atr.blueprints.get as get
-import atr.config as config
-import atr.htm as htm
-import atr.models.safe as safe
-import atr.paths as paths
-import atr.util as util
-import atr.web as web
-
-
-@get.typed
-async def path(session: web.Committer, _published: Literal["published"], file_path: safe.RelPath) -> web.QuartResponse:
- """
- URL: /published/<path:file_path>
- View the content of a specific file in the downloads directory.
- """
- # This route is for debugging
- # When developing locally, there is no proxy to view the downloads directory
- # Therefore this path acts as a way to check the contents of that directory
- if not config.is_test_mode():
- return quart.abort(404)
- return await _path(session, str(file_path))
-
-
-@get.typed
-async def root(session: web.Committer, _published: Literal["published/"]) -> web.QuartResponse:
- """
- URL: /published/
- """
- if not config.is_test_mode():
- return quart.abort(404)
- return await _path(session, "")
-
-
-async def _directory_listing(full_path: safe.StatePath, current_path: str) -> web.ElementResponse:
- html = htm.Block(htm.html)
- html.title[f"Index of /{current_path}"]
- html.style["body { margin: 1rem; }"]
- with html.block(htm.body) as body:
- htm.h1[f"Index of /{current_path}"]
- with body.block(htm.pre) as pre:
- await _directory_listing_pre(full_path, current_path, pre)
- return web.ElementResponse(html.collect())
-
-
-async def _directory_listing_pre(full_path: safe.StatePath, current_path: str, pre: htm.Block) -> None:
- if current_path:
- parent_path = pathlib.Path(current_path).parent
- parent_url_path = str(parent_path) if (str(parent_path) != ".") else ""
- if parent_url_path:
- pre.a(href=util.as_url(path, file_path=parent_url_path))["../"]
- else:
- pre.a(href=util.as_url(root))["../"]
- pre.text("\n\n")
-
- entries = []
- dir_contents = await aiofiles.os.listdir(full_path)
- for name in dir_contents:
- try:
- stat_result = await aiofiles.os.stat(full_path / name)
- entries.append({"name": name, "stat": stat_result})
- except OSError:
- continue
- entries.sort(key=lambda e: (not stat.S_ISDIR(e["stat"].st_mode), e["name"].lower()))
-
- if entries:
- max_nlink_len = max(len(str(e["stat"].st_nlink)) for e in entries)
- max_size_len = max(len(str(e["stat"].st_size)) for e in entries)
-
- for entry in entries:
- stat_info = entry["stat"]
- is_dir = stat.S_ISDIR(stat_info.st_mode)
- mode = stat.filemode(stat_info.st_mode)
- nlink = str(stat_info.st_nlink).rjust(max_nlink_len)
- size = str(stat_info.st_size).rjust(max_size_len)
- mtime = datetime.fromtimestamp(stat_info.st_mtime).strftime("%Y-%m-%d %H:%M")
- entry_path = str(pathlib.Path(current_path) / entry["name"])
- display_name = f"{entry['name']}/" if is_dir else entry["name"]
- pre.text(f"{mode} {nlink} {size} {mtime} ")
- pre.a(href=util.as_url(path, file_path=entry_path))[display_name]
- pre.text("\n")
-
-
-async def _file_content(full_path: safe.StatePath) -> web.QuartResponse:
- return await quart.send_file(
- full_path,
- as_attachment=True,
- attachment_filename=full_path.name,
- mimetype="application/octet-stream",
- )
-
-
-async def _path(session: web.Committer, path: str) -> web.QuartResponse:
- downloads_path = paths.get_downloads_dir()
- full_path = downloads_path / path
- if await aiofiles.os.path.isdir(full_path):
- return await _directory_listing(full_path, path)
-
- if await aiofiles.os.path.isfile(full_path):
- return await _file_content(full_path)
-
- return quart.abort(404)
diff --git a/atr/get/voting.py b/atr/get/voting.py
index 70f8fa41..a6696a8e 100644
--- a/atr/get/voting.py
+++ b/atr/get/voting.py
@@ -18,7 +18,6 @@
from typing import Literal
-import aiofiles.os
import htpy
import quart
@@ -29,12 +28,10 @@ import atr.db as db
import atr.db.interaction as interaction
import atr.form as form
import atr.get.compose as compose
-import atr.get.keys as keys
import atr.get.projects as projects
import atr.htm as htm
import atr.models.safe as safe
import atr.models.sql as sql
-import atr.paths as paths
import atr.post as post
import atr.render as render
import atr.sessions as sessions
@@ -110,8 +107,6 @@ async def selected(
default_subject_template, default_body_template, options
)
- keys_file_missing = await _committee_keys_file_missing(committee)
-
async with storage.read(session) as read:
concern_groups = await shared.voting.concern_groups_for_release(read.as_general_public(), release)
flash_data = await sessions.form_error_pop(quart.request.path)
@@ -130,7 +125,6 @@ async def selected(
default_body=default_body,
min_hours=min_hours,
vote_mode=vote_mode,
- keys_file_missing=keys_file_missing,
asf_uid=session.uid,
concern_groups=concern_groups,
submitted_concerns=submitted_concerns,
@@ -175,11 +169,6 @@ def _add_automatic_publish_fields(
]
-async def _committee_keys_file_missing(committee: sql.Committee) -> bool:
- keys_file_path = paths.committee_downloads_dir(committee) / "KEYS"
- return not await aiofiles.os.path.isfile(keys_file_path)
-
-
def _default_download_path_suffix(release: sql.Release, committee: sql.Committee) -> safe.RelPath | None:
return construct.resolve_download_path_suffix(
template=release.project.policy_download_path_suffix,
@@ -234,7 +223,6 @@ async def _render_page( # noqa: C901
default_body: str,
min_hours: int,
vote_mode: sql.VoteMode,
- keys_file_missing: bool,
asf_uid: str,
concern_groups: list[util.ConcernGroup],
submitted_concerns: list[str],
@@ -283,18 +271,6 @@ async def _render_page( # noqa: C901
". There is no minimum duration, and a PMC member must resolve the vote manually.",
]
- if keys_file_missing:
- keys_url = util.as_url(keys.keys) + f"#committee-{release.committee.key}"
- page.div(".p-3.mb-4.bg-warning-subtle.border.border-warning.rounded")[
- htm.strong["Warning: "],
- "The committee ",
- htm.code["KEYS"],
- " file is missing, so the vote email cannot link to a public KEYS file. Add or associate a key, "
- "or regenerate the committee KEYS file on the ",
- htm.a(href=keys_url)["KEYS page"],
- ".",
- ]
-
cancel_url = util.as_url(
compose.selected,
project_key=release.project.key,
diff --git a/atr/post/draft.py b/atr/post/draft.py
index 95487d90..7a0c0903 100644
--- a/atr/post/draft.py
+++ b/atr/post/draft.py
@@ -103,7 +103,6 @@ async def delete(
project_key,
version_key,
phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
- include_downloads=False,
)
# Ensure that deletion errors are reported to the user
if error is not None:
diff --git a/atr/server.py b/atr/server.py
index 54738fe3..fd5a984d 100644
--- a/atr/server.py
+++ b/atr/server.py
@@ -192,7 +192,6 @@ def _app_dirs_setup(state_dir_str: str, hot_reload: bool) -> None:
pathlib.Path(state_dir_str) / "secrets" / "curated",
pathlib.Path(state_dir_str) / "secrets" / "generated",
pathlib.Path(paths.get_archives_dir()),
- pathlib.Path(paths.get_downloads_dir()),
pathlib.Path(paths.get_finished_dir()),
pathlib.Path(paths.get_quarantined_dir()),
pathlib.Path(paths.get_tmp_dir()),
@@ -1308,6 +1307,13 @@ def _validate_config(app_config: type[config.AppConfig], hot_reload: bool) -> No
print("!!!", file=sys.stderr)
# sys.exit(1)
+ if pathlib.Path(paths.get_downloads_dir()).is_dir() and (hot_reload is False):
+ print("!!!", file=sys.stderr)
+ print("WARNING: The downloads directory is no longer supported", file=sys.stderr)
+ print("Please remove downloads from the state directory", file=sys.stderr)
+ print("Announced release files are now published to SVN", file=sys.stderr)
+ print("!!!", file=sys.stderr)
+
def _validate_secrets_permissions(state_dir: pathlib.Path) -> None:
secrets_dirs = [
diff --git a/atr/storage/datatypes.py b/atr/storage/datatypes.py
index fc39a5d9..332eced7 100644
--- a/atr/storage/datatypes.py
+++ b/atr/storage/datatypes.py
@@ -83,7 +83,7 @@ class UnknownApacheUidError(ValueError):
@dataclasses.dataclass
class LinkedCommittee:
name: str
- autogenerated_keys_file: outcome.Outcome[str]
+ autogenerated_keys_file: outcome.Outcome[int]
publication: outcome.Outcome[KeysPublish]
diff --git a/atr/storage/writers/announce.py b/atr/storage/writers/announce.py
index 3f33f74c..dafd1adc 100644
--- a/atr/storage/writers/announce.py
+++ b/atr/storage/writers/announce.py
@@ -245,9 +245,6 @@ class ReleaseManager(CommitteeParticipant):
log.warning(f"SVN publication target is not configured for {project_key!s} {version_key!s}: {exc}")
else:
await self.__warn_publication_artifacts(unfinished_path, target, public_url)
- preserve = release.project.policy_preserve_download_files
- if preserve is True:
- await self.__hard_link_downloads(committee, unfinished_path, effective_download_path_suffix, dry_run=True)
# Ensure that the permissions of every directory are 755
await asyncio.to_thread(util.chmod_directories, unfinished_path)
@@ -271,16 +268,6 @@ class ReleaseManager(CommitteeParticipant):
except Exception as e:
raise storage.AccessError(f"Error moving files: {e!s}", status=500)
- # TODO: Add an audit log entry here
- # TODO: We should consider copying the files instead of hard linking
- # That way, we can write protect the pristine ATR files
- await self.__hard_link_downloads(
- committee,
- finished_path,
- effective_download_path_suffix,
- preserve=preserve,
- )
-
try:
task = sql.Task(
status=sql.TaskStatus.QUEUED,
@@ -415,8 +402,6 @@ class ReleaseManager(CommitteeParticipant):
if getattr(update_result, "rowcount", 0) != 1:
return f"Release {project_key!s} {version_key!s} is already archived"
- await self.__remove_from_downloads(release_row)
-
self.__data.add(
sql.LifecycleEvent(
project_key=release_row.project_key,
@@ -436,38 +421,6 @@ class ReleaseManager(CommitteeParticipant):
)
return None
- async def __remove_from_downloads(self, release_row: sql.Release) -> None:
- try:
- await self.__release_download_links_delete(release_row)
- except Exception:
- log.exception(
- f"Error removing download hard links for release {release_row.key!r}; continuing with archive"
- )
-
- async def __release_download_links_delete(self, release_row: sql.Release) -> None:
- finished_dir = paths.release_directory(release_row)
- if not await aiofiles.os.path.isdir(finished_dir):
- return
- release_inodes: set[int] = set()
- async for file_path in util.paths_recursive_all(finished_dir):
- try:
- stat_result = await aiofiles.os.stat(finished_dir / file_path)
- except OSError:
- continue
- release_inodes.add(stat_result.st_ino)
- if not release_inodes:
- return
- downloads_dir = paths.get_downloads_dir()
- async for link_path in util.paths_recursive_all(downloads_dir):
- full_link_path = downloads_dir / link_path
- try:
- link_stat = await aiofiles.os.stat(full_link_path)
- except OSError:
- continue
- if link_stat.st_ino in release_inodes:
- await aiofiles.os.remove(full_link_path)
- log.info(f"Deleted download hard link: {full_link_path}")
-
async def __warn_publication_artifacts(
self,
unfinished_path: safe.StatePath,
@@ -501,29 +454,6 @@ class ReleaseManager(CommitteeParticipant):
return safe.RelPath(candidate)
return None
- async def __hard_link_downloads(
- self,
- committee: sql.Committee,
- unfinished_path: safe.StatePath,
- download_path_suffix: safe.RelPath | None,
- dry_run: bool = False,
- preserve: bool = False,
- ) -> None:
- """Hard link the release files to the downloads directory."""
- downloads_path = paths.committee_downloads_dir(committee)
- if download_path_suffix is not None:
- downloads_path = downloads_path / download_path_suffix.as_path()
- # The "exist_ok" parameter means to overwrite files if True
- # We only overwrite if we're not preserving, so we supply "not preserve"
- # TODO: Add a test for this
- await util.create_hard_link_clone(
- unfinished_path,
- downloads_path,
- do_not_create_dest_dir=dry_run,
- exist_ok=not preserve,
- dry_run=dry_run,
- )
-
def __publish_revision_from_task(self, task: sql.Task) -> int | None:
result = task.result
if isinstance(result, results.SvnPublish):
diff --git a/atr/storage/writers/keys.py b/atr/storage/writers/keys.py
index 4a244427..2032833a 100644
--- a/atr/storage/writers/keys.py
+++ b/atr/storage/writers/keys.py
@@ -23,6 +23,8 @@ from __future__ import annotations
import asyncio
import datetime
import pathlib
+import shutil
+import tempfile
import textwrap
from typing import Final, NoReturn
@@ -256,9 +258,6 @@ class FoundationCommitter(GeneralPublic):
key_blocks_str=key_blocks_str,
)
- def _committee_keys_path(self, committee: sql.Committee) -> safe.StatePath:
- return paths.committee_downloads_dir(committee) / "KEYS"
-
async def _recheck_committee_drafts(self, *committee_keys: str) -> None:
# A KEYS change only invalidates signature checks, so we limit the re-queue to .asc files
affected = {committee_key for committee_key in committee_keys if committee_key}
@@ -283,58 +282,42 @@ class FoundationCommitter(GeneralPublic):
)
async def _publish_keys_to_svn(
- self, committee: sql.Committee, source: pathlib.Path
+ self, committee: sql.Committee, content: str | None
) -> outcome.Outcome[datatypes.KeysPublish]:
if not committee.automated_keys_file:
return outcome.Result(datatypes.KeysPublish.AUTOMATION_DISABLED)
if not config.get().SVN_PUBLISH_URL:
return outcome.Result(datatypes.KeysPublish.SVN_NOT_CONFIGURED)
target_url = util.svn_publish_internal_url(committee, None) + "/KEYS"
+ temp_dir: str | None = None
+ source = pathlib.Path("/dev/null")
try:
+ if content is not None:
+ temp_dir = await asyncio.to_thread(tempfile.mkdtemp, dir=paths.get_tmp_dir())
+ source = pathlib.Path(temp_dir) / "KEYS"
+ await asyncio.to_thread(source.write_text, content, encoding="utf-8")
await svn.publish_file(source, target_url, self.__asf_uid, f"Publish KEYS for {committee.key} via ATR")
except Exception as e:
log.warning(f"Failed to publish KEYS to SVN for committee {committee.key}: {e}")
return outcome.Error(e)
+ finally:
+ if temp_dir is not None:
+ await asyncio.to_thread(shutil.rmtree, temp_dir, ignore_errors=True)
return outcome.Result(datatypes.KeysPublish.PUBLISHED)
- async def _sync_committee_keys_file(
- self, committee_key: str
- ) -> tuple[str | None, outcome.Outcome[datatypes.KeysPublish]]:
+ async def _sync_committee_keys_file(self, committee_key: str) -> tuple[int, outcome.Outcome[datatypes.KeysPublish]]:
committee = await self.__data.committee(key=committee_key, _public_signing_keys=True).demand(
storage.AccessError(f"Committee not found: {committee_key}", status=404)
)
- committee_keys_path = self._committee_keys_path(committee)
- committee_keys_dir = committee_keys_path.parent
if not committee.public_signing_keys:
- try:
- if await aiofiles.os.path.exists(committee_keys_path):
- await aiofiles.os.remove(committee_keys_path)
- except OSError as e:
- raise storage.AccessError(
- f"Failed to remove KEYS file for committee {committee_key}: {e}", status=500
- ) from e
# We use an empty file for no KEYS in SVN
- svn_outcome = await self._publish_keys_to_svn(committee, pathlib.Path("/dev/null"))
- return None, svn_outcome
+ svn_outcome = await self._publish_keys_to_svn(committee, None)
+ return 0, svn_outcome
full_keys_file_content = await self._keys_file_text(committee)
- try:
- await aiofiles.os.makedirs(committee_keys_dir, exist_ok=True)
- await asyncio.to_thread(util.chmod_directories, committee_keys_dir, permissions=0o755)
- await asyncio.to_thread(committee_keys_path.path.write_text, full_keys_file_content, encoding="utf-8")
- except OSError as e:
- raise storage.AccessError(
- f"Failed to write KEYS file for committee {committee_key}: {e}", status=500
- ) from e
- except Exception as e:
- log.exception(f"An unexpected error occurred writing KEYS for committee {committee_key}: {e}")
- raise storage.AccessError(
- f"An unexpected error occurred writing KEYS for committee {committee_key}: {e}",
- status=500,
- ) from e
- svn_outcome = await self._publish_keys_to_svn(committee, committee_keys_path.path)
- return str(committee_keys_path), svn_outcome
+ svn_outcome = await self._publish_keys_to_svn(committee, full_keys_file_content)
+ return len(committee.public_signing_keys), svn_outcome
async def test_user_delete_all(self, test_uid: str) -> outcome.Outcome[int]:
"""Delete all OpenPGP keys and their links for a test user."""
@@ -774,7 +757,7 @@ class CommitteeParticipant(FoundationCommitter):
async def autogenerate_keys_file(
self,
- ) -> tuple[outcome.Outcome[str], outcome.Outcome[datatypes.KeysPublish]]:
+ ) -> tuple[outcome.Outcome[int], outcome.Outcome[datatypes.KeysPublish]]:
no_keys = storage.AccessError(
f"No keys found for committee {self.__committee_key} to generate KEYS file.", status=404
)
@@ -782,12 +765,12 @@ class CommitteeParticipant(FoundationCommitter):
committee = await self.committee()
if not committee.public_signing_keys:
return outcome.Error(no_keys), outcome.Error(no_keys)
- synced_path, svn_outcome = await self._sync_committee_keys_file(self.__committee_key)
+ key_count, svn_outcome = await self._sync_committee_keys_file(self.__committee_key)
except Exception as e:
return outcome.Error(e), outcome.Error(e)
- if synced_path is None:
+ if key_count == 0:
return outcome.Error(no_keys), svn_outcome
- return outcome.Result(synced_path), svn_outcome
+ return outcome.Result(key_count), svn_outcome
async def committee(self) -> sql.Committee:
return await self.__data.committee(key=self.__committee_key, _public_signing_keys=True).demand(
diff --git a/atr/storage/writers/release.py b/atr/storage/writers/release.py
index 5774bd75..8e19a426 100644
--- a/atr/storage/writers/release.py
+++ b/atr/storage/writers/release.py
@@ -117,7 +117,6 @@ async def _archive_release(
if getattr(update_result, "rowcount", 0) != 1:
return f"Release {project_key!s} {version_key!s} is already archived"
- await _remove_from_downloads(release)
# TODO: SVN move to archive.apache.org goes here once SVN is wired up.
data.add(
@@ -158,6 +157,25 @@ def _assert_can_start(asf_uid: str, project: sql.Project) -> None:
)
+async def _assert_no_existing_release(
+ data: db.Session, project: sql.Project, project_key: safe.ProjectKey, version: safe.VersionKey
+) -> None:
+ # TODO: Consider using Release.revision instead of ./latest
+ release = await data.release(project_key=project.key, version=str(version)).get()
+ if release is None:
+ return
+ match release.phase:
+ case sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT:
+ phase_desc = "A draft release (being composed)"
+ case sql.ReleasePhase.RELEASE_CANDIDATE:
+ phase_desc = "A release candidate (being voted on)"
+ case sql.ReleasePhase.RELEASE_PREVIEW:
+ phase_desc = "A release preview (being finished)"
+ case sql.ReleasePhase.RELEASE:
+ phase_desc = "A finished release"
+ raise storage.AccessError(f"{phase_desc} for {project_key!s} {version} already exists.", status=409)
+
+
async def _claim_release_archive_approval(
data: db.Session,
approval_request_id: int,
@@ -179,25 +197,6 @@ async def _claim_release_archive_approval(
approval.status = sql.ApprovalStatus.COMPLETED
-async def _assert_no_existing_release(
- data: db.Session, project: sql.Project, project_key: safe.ProjectKey, version: safe.VersionKey
-) -> None:
- # TODO: Consider using Release.revision instead of ./latest
- release = await data.release(project_key=project.key, version=str(version)).get()
- if release is None:
- return
- match release.phase:
- case sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT:
- phase_desc = "A draft release (being composed)"
- case sql.ReleasePhase.RELEASE_CANDIDATE:
- phase_desc = "A release candidate (being voted on)"
- case sql.ReleasePhase.RELEASE_PREVIEW:
- phase_desc = "A release preview (being finished)"
- case sql.ReleasePhase.RELEASE:
- phase_desc = "A finished release"
- raise storage.AccessError(f"{phase_desc} for {project_key!s} {version} already exists.", status=409)
-
-
async def _ensure_project_cycle(data: db.Session, project: sql.Project, version: safe.VersionKey) -> str:
try:
cycle_name = cycles.cycle_name_for_version(project, str(version))
@@ -227,41 +226,6 @@ def _normalise_signature_field(value: object) -> str | None:
return str(value)
-async def _release_download_links_delete(release: sql.Release) -> None:
- # Hard-linked downloads share an inode with the finished files, so we
- # find them by inode rather than path - that way an unknown
- # download_path_suffix isn't a blocker.
- finished_dir = paths.release_directory(release)
- if not await aiofiles.os.path.isdir(finished_dir):
- return
- release_inodes: set[int] = set()
- async for file_path in util.paths_recursive_all(finished_dir):
- try:
- stat_result = await aiofiles.os.stat(finished_dir / file_path)
- except OSError:
- continue
- release_inodes.add(stat_result.st_ino)
- if not release_inodes:
- return
- downloads_dir = paths.get_downloads_dir()
- async for link_path in util.paths_recursive_all(downloads_dir):
- full_link_path = downloads_dir / link_path
- try:
- link_stat = await aiofiles.os.stat(full_link_path)
- except OSError:
- continue
- if link_stat.st_ino in release_inodes:
- await aiofiles.os.remove(full_link_path)
- log.info(f"Deleted download hard link: {full_link_path}")
-
-
-async def _remove_from_downloads(release: sql.Release) -> None:
- try:
- await _release_download_links_delete(release)
- except Exception:
- log.exception(f"Error removing download hard links for release {release.key!r}; continuing with archive")
-
-
async def _signature_provenance_metadata_for(
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
@@ -464,7 +428,6 @@ class CommitteeParticipant(FoundationCommitter):
project_key: safe.ProjectKey,
version: safe.VersionKey,
phase: db.Opt[sql.ReleasePhase] = db.NOT_SET,
- include_downloads: bool = True,
) -> str | None:
"""Handle the deletion of database records and filesystem data for a release."""
await self.__data.begin_immediate()
@@ -482,14 +445,13 @@ class CommitteeParticipant(FoundationCommitter):
if release.phase == sql.ReleasePhase.RELEASE:
await self.__data.rollback()
return f"Release '{project_key!s} {version!s}' has been announced; it can only be archived, not deleted."
- return await self.__delete_body(release, project_key, version, include_downloads)
+ return await self.__delete_body(release, project_key, version)
async def __delete_body(
self,
release: sql.Release,
project_key: safe.ProjectKey,
version: safe.VersionKey,
- include_downloads: bool,
) -> str | None:
release_dirs = [
paths.release_directory_base(release),
@@ -543,8 +505,6 @@ class CommitteeParticipant(FoundationCommitter):
# Filesystem deletions are more likely to have permission errors than database deletions
# Therefore we do filesystem deletions first
- if include_downloads:
- await self.__delete_release_data_downloads(release)
error = await self.__delete_release_data_filesystem(release_dirs, project_key, version)
await self.__data.commit()
@@ -557,36 +517,6 @@ class CommitteeParticipant(FoundationCommitter):
)
return error
- async def __delete_release_data_downloads(self, release: sql.Release) -> None:
- try:
- await self.__release_download_links_delete(release)
- except Exception:
- log.exception(f"Error removing download hard links for release {release.key!r}; continuing with deletion")
-
- async def __release_download_links_delete(self, release: sql.Release) -> None:
- finished_dir = paths.release_directory(release)
- if not await aiofiles.os.path.isdir(finished_dir):
- return
- release_inodes: set[int] = set()
- async for file_path in util.paths_recursive_all(finished_dir):
- try:
- stat_result = await aiofiles.os.stat(finished_dir / file_path)
- except OSError:
- continue
- release_inodes.add(stat_result.st_ino)
- if not release_inodes:
- return
- downloads_dir = paths.get_downloads_dir()
- async for link_path in util.paths_recursive_all(downloads_dir):
- full_link_path = downloads_dir / link_path
- try:
- link_stat = await aiofiles.os.stat(full_link_path)
- except OSError:
- continue
- if link_stat.st_ino in release_inodes:
- await aiofiles.os.remove(full_link_path)
- log.info(f"Deleted hard link: {full_link_path}")
-
async def __delete_release_data_filesystem(
self, release_dirs: Sequence[safe.StatePath], project_key: safe.ProjectKey, version: safe.VersionKey
) -> str | None:
@@ -1590,13 +1520,12 @@ class FoundationAdmin(FoundationCommitter):
self,
project_key: safe.ProjectKey,
version: safe.VersionKey,
- include_downloads: bool = True,
) -> str | None:
release = await self.__data.release(project_key=str(project_key), version=str(version), _committee=True).demand(
storage.AccessError(f"Release '{project_key!s} {version!s}' not found.", status=404)
)
storage.ensure_project_active(release.project)
- return await self.__delete_body(release, project_key, version, include_downloads)
+ return await self.__delete_body(release, project_key, version)
async def delete_inactive(
self,
@@ -1653,7 +1582,6 @@ class FoundationAdmin(FoundationCommitter):
release: sql.Release,
project_key: safe.ProjectKey,
version: safe.VersionKey,
- include_downloads: bool,
) -> str | None:
release_dirs = [
paths.release_directory_base(release),
@@ -1707,8 +1635,6 @@ class FoundationAdmin(FoundationCommitter):
# Filesystem deletions are more likely to have permission errors than database deletions
# Therefore we do filesystem deletions first
- if include_downloads:
- await self.__delete_release_data_downloads(release)
error = await self.__delete_release_data_filesystem(release_dirs, project_key, version)
await self.__data.commit()
@@ -1721,36 +1647,6 @@ class FoundationAdmin(FoundationCommitter):
)
return error
- async def __delete_release_data_downloads(self, release: sql.Release) -> None:
- try:
- await self.__release_download_links_delete(release)
- except Exception:
- log.exception(f"Error removing download hard links for release {release.key!r}; continuing with deletion")
-
- async def __release_download_links_delete(self, release: sql.Release) -> None:
- finished_dir = paths.release_directory(release)
- if not await aiofiles.os.path.isdir(finished_dir):
- return
- release_inodes: set[int] = set()
- async for file_path in util.paths_recursive_all(finished_dir):
- try:
- stat_result = await aiofiles.os.stat(finished_dir / file_path)
- except OSError:
- continue
- release_inodes.add(stat_result.st_ino)
- if not release_inodes:
- return
- downloads_dir = paths.get_downloads_dir()
- async for link_path in util.paths_recursive_all(downloads_dir):
- full_link_path = downloads_dir / link_path
- try:
- link_stat = await aiofiles.os.stat(full_link_path)
- except OSError:
- continue
- if link_stat.st_ino in release_inodes:
- await aiofiles.os.remove(full_link_path)
- log.info(f"Deleted hard link: {full_link_path}")
-
async def __delete_release_data_filesystem(
self, release_dirs: Sequence[safe.StatePath], project_key: safe.ProjectKey, version: safe.VersionKey
) -> str | None:
diff --git a/tests/unit/test_announce_deletion_order.py b/tests/unit/test_announce_deletion_order.py
index 0c28d06a..7c863517 100644
--- a/tests/unit/test_announce_deletion_order.py
+++ b/tests/unit/test_announce_deletion_order.py
@@ -61,7 +61,6 @@ def announcing_writer(
release_manager._ReleaseManager__write_as = mock.MagicMock()
release_manager._ReleaseManager__asf_uid = "alice"
release_manager._ReleaseManager__committee_key = "alpha"
- release_manager._ReleaseManager__hard_link_downloads = mock.AsyncMock()
release_manager._ReleaseManager__promote_in_database = mock.AsyncMock()
release_manager._ReleaseManager__write_artifact_rows = mock.AsyncMock()
return release_manager
@@ -85,7 +84,6 @@ def release_row() -> SimpleNamespace:
committee=SimpleNamespace(key="alpha"),
is_active=True,
key="example",
- policy_preserve_download_files=False,
release_policy=None,
)
return SimpleNamespace(
diff --git a/tests/unit/test_keys_writer.py b/tests/unit/test_keys_writer.py
index be34d7cf..5bc26e8f 100644
--- a/tests/unit/test_keys_writer.py
+++ b/tests/unit/test_keys_writer.py
@@ -26,9 +26,9 @@ import sqlalchemy.ext.asyncio
import sqlmodel
import atr.db as db
-import atr.models.safe as safe
import atr.pgp as pgp
import atr.storage as storage
+import atr.storage.datatypes as datatypes
import atr.storage.outcome as outcome
import atr.storage.writers.keys as keys_writer
@@ -184,7 +184,7 @@ async def test_delete_committee_keys_removes_links_and_orphaned_keys(sqlite_data
writer, write_as = _make_foundation_admin(sqlite_data, "alpha")
with mock.patch.object(writer, "_sync_committee_keys_file", new_callable=mock.AsyncMock) as mock_sync:
- mock_sync.return_value = (None, outcome.Result(keys_writer.datatypes.KeysPublish.SVN_NOT_CONFIGURED))
+ mock_sync.return_value = (0, outcome.Result(keys_writer.datatypes.KeysPublish.SVN_NOT_CONFIGURED))
num_unlinked, num_deleted, _ = await writer.delete_committee_keys()
assert num_unlinked == 2
@@ -220,7 +220,7 @@ async def test_delete_committee_keys_returns_zero_when_no_keys(sqlite_data):
@pytest.mark.asyncio
-async def test_delete_key_removal_deletes_empty_keys_file(tmp_path):
+async def test_delete_key_removal_publishes_empty_keys_file():
owned_key = SimpleNamespace(
fingerprint="fp1",
committees=[SimpleNamespace(key="alpha")],
@@ -232,18 +232,13 @@ async def test_delete_key_removal_deletes_empty_keys_file(tmp_path):
data.execute.return_value = SimpleNamespace(rowcount=1)
writer, _write, write_as = _make_foundation_committer_with_audit(data)
- keys_path = tmp_path / "alpha" / "KEYS"
- keys_path.parent.mkdir(parents=True)
- keys_path.write_text("stale content", encoding="utf-8")
-
- with (
- mock.patch.object(keys_writer.paths, "get_downloads_dir", return_value=tmp_path),
- mock.patch.object(keys_writer.util, "chmod_directories"),
- ):
+ publish = mock.AsyncMock(return_value=outcome.Result(datatypes.KeysPublish.PUBLISHED))
+ with mock.patch.object(writer, "_publish_keys_to_svn", publish):
result = await writer.delete_key("fp1")
assert isinstance(result, outcome.Result)
- assert not keys_path.exists()
+ publish.assert_awaited_once()
+ assert publish.await_args.args[1] is None
data.delete.assert_not_awaited()
data.commit.assert_awaited_once()
write_as.append_to_audit_log.assert_called_once()
@@ -477,7 +472,7 @@ async def test_publish_keys_to_svn_distinguishes_publication_states() -> None:
committee = _committee("alpha", [])
with mock.patch.object(keys_writer.config, "get", return_value=SimpleNamespace(SVN_PUBLISH_URL=None)):
- unconfigured = await writer._publish_keys_to_svn(committee, pathlib.Path("/dev/null"))
+ unconfigured = await writer._publish_keys_to_svn(committee, None)
with (
mock.patch.object(
keys_writer.config, "get", return_value=SimpleNamespace(SVN_PUBLISH_URL="https://svn.example/dist/dev/atr")
@@ -487,7 +482,7 @@ async def test_publish_keys_to_svn_distinguishes_publication_states() -> None:
),
mock.patch.object(keys_writer.svn, "publish_file", new_callable=mock.AsyncMock),
):
- published = await writer._publish_keys_to_svn(committee, pathlib.Path("/dev/null"))
+ published = await writer._publish_keys_to_svn(committee, None)
assert unconfigured.result_or_raise() is keys_writer.datatypes.KeysPublish.SVN_NOT_CONFIGURED
assert published.result_or_raise() is keys_writer.datatypes.KeysPublish.PUBLISHED
@@ -506,7 +501,7 @@ async def test_publish_keys_to_svn_puts_keys_url() -> None:
),
mock.patch.object(keys_writer.svn, "publish_file", new_callable=mock.AsyncMock) as publish_file,
):
- result = await writer._publish_keys_to_svn(committee, pathlib.Path("/dev/null"))
+ result = await writer._publish_keys_to_svn(committee, None)
publish_file.assert_awaited_once()
assert publish_file.call_args.args[1] == "https://svn.example/dist/dev/atr/alpha/KEYS"
@@ -523,7 +518,7 @@ async def test_publish_keys_to_svn_skips_when_automation_disabled() -> None:
),
mock.patch.object(keys_writer.svn, "publish_file", new_callable=mock.AsyncMock) as publish_file,
):
- result = await writer._publish_keys_to_svn(committee, pathlib.Path("/dev/null"))
+ result = await writer._publish_keys_to_svn(committee, None)
publish_file.assert_not_awaited()
assert result.result_or_raise() is keys_writer.datatypes.KeysPublish.AUTOMATION_DISABLED
@@ -581,7 +576,7 @@ async def test_sync_committees_for_keys_returns_publications():
@pytest.mark.asyncio
-async def test_update_committee_associations_removal_deletes_empty_keys_file(tmp_path):
+async def test_update_committee_associations_removal_publishes_empty_keys_file():
owned_key = SimpleNamespace(fingerprint="fp1", committees=[SimpleNamespace(key="alpha")])
data = MockData(
owned_key,
@@ -589,19 +584,14 @@ async def test_update_committee_associations_removal_deletes_empty_keys_file(tmp
)
writer, write, write_as = _make_foundation_committer_with_audit(data)
- keys_path = tmp_path / "alpha" / "KEYS"
- keys_path.parent.mkdir(parents=True)
- keys_path.write_text("stale content", encoding="utf-8")
-
- with (
- mock.patch.object(keys_writer.paths, "get_downloads_dir", return_value=tmp_path),
- mock.patch.object(keys_writer.util, "chmod_directories"),
- ):
+ publish = mock.AsyncMock(return_value=outcome.Result(datatypes.KeysPublish.PUBLISHED))
+ with mock.patch.object(writer, "_publish_keys_to_svn", publish):
update = await writer.update_committee_associations("fp1", [])
assert update.added == set()
assert update.removed == {"alpha"}
- assert not keys_path.exists()
+ publish.assert_awaited_once()
+ assert publish.await_args.args[1] is None
assert write.as_committee_participant.call_count == 0
data.begin_immediate.assert_awaited_once()
data.commit.assert_awaited_once()
@@ -614,8 +604,7 @@ async def test_update_committee_associations_removal_deletes_empty_keys_file(tmp
@pytest.mark.asyncio
-async def test_update_committee_associations_removal_rewrites_keys_file_with_remaining_keys(tmp_path):
- temp_dir = safe.StatePath(tmp_path)
+async def test_update_committee_associations_removal_republishes_remaining_keys():
owned_key = SimpleNamespace(fingerprint="fp1", committees=[SimpleNamespace(key="alpha")])
remaining_key = _public_key("bbbbccccdddd1111")
data = MockData(
@@ -624,21 +613,14 @@ async def test_update_committee_associations_removal_rewrites_keys_file_with_rem
)
writer, write = _make_foundation_committer(data)
- keys_path = temp_dir / "alpha" / "KEYS"
- keys_path.parent.path.mkdir(parents=True)
- keys_path.path.write_text("stale content", encoding="utf-8")
-
- with (
- mock.patch.object(keys_writer.paths, "get_downloads_dir", return_value=temp_dir),
- mock.patch.object(keys_writer.util, "chmod_directories"),
- ):
+ publish = mock.AsyncMock(return_value=outcome.Result(datatypes.KeysPublish.PUBLISHED))
+ with mock.patch.object(writer, "_publish_keys_to_svn", publish):
update = await writer.update_committee_associations("fp1", [])
assert update.added == set()
assert update.removed == {"alpha"}
- assert keys_path.path.exists()
- content = keys_path.path.read_text(encoding="utf-8")
- assert "stale content" not in content
+ publish.assert_awaited_once()
+ content = publish.await_args.args[1]
assert remaining_key.fingerprint.upper() in content
assert "Signing keys for the alpha committee" in content
assert write.as_committee_participant.call_count == 0
diff --git a/tests/unit/test_release_archive_approval.py b/tests/unit/test_release_archive_approval.py
index 5bd767fb..5f8bc436 100644
--- a/tests/unit/test_release_archive_approval.py
+++ b/tests/unit/test_release_archive_approval.py
@@ -64,8 +64,7 @@ async def test_complete_archive_rejects_a_project_scoped_approval() -> None:
@pytest.mark.asyncio
-async def test_complete_archive_claims_and_archives_in_one_transaction(monkeypatch: pytest.MonkeyPatch) -> None:
- monkeypatch.setattr(release_writer, "_remove_from_downloads", mock.AsyncMock())
+async def test_complete_archive_claims_and_archives_in_one_transaction() -> None:
approval = _approval()
data = _data(approval=approval)
writer = _writer(data)
diff --git a/tests/unit/test_release_writer_archive.py b/tests/unit/test_release_writer_archive.py
index 075ce72c..3e090137 100644
--- a/tests/unit/test_release_writer_archive.py
+++ b/tests/unit/test_release_writer_archive.py
@@ -16,7 +16,6 @@
# under the License.
import datetime
-import os
import unittest.mock as mock
from types import SimpleNamespace
@@ -82,15 +81,6 @@ async def test_archive_rejects_a_release_not_in_the_release_phase():
await member.archive(safe.ProjectKey("example"), safe.VersionKey("1.0.0"))
-@pytest.mark.asyncio
-async def test_archive_rejects_the_latest_release_in_the_cycle():
- # The latest in a cycle may only be archived through a CAP approval vote
- target = _archive_candidate(version="2.0.0")
- member = _make_member(release_result=target, siblings=[_archive_candidate(version="1.0.0")])
- with pytest.raises(storage.AccessError, match="requires a CAP approval vote"):
- await member.archive(safe.ProjectKey("example"), safe.VersionKey("2.0.0"))
-
-
@pytest.mark.asyncio
async def test_archive_rejects_a_release_with_an_archival_vote_in_progress():
target = _archive_candidate(version="1.0.0")
@@ -104,14 +94,22 @@ async def test_archive_rejects_a_release_with_an_archival_vote_in_progress():
@pytest.mark.asyncio
-async def test_archive_succeeds_and_writes_lifecycle_event(monkeypatch):
+async def test_archive_rejects_the_latest_release_in_the_cycle():
+ # The latest in a cycle may only be archived through a CAP approval vote
+ target = _archive_candidate(version="2.0.0")
+ member = _make_member(release_result=target, siblings=[_archive_candidate(version="1.0.0")])
+ with pytest.raises(storage.AccessError, match="requires a CAP approval vote"):
+ await member.archive(safe.ProjectKey("example"), safe.VersionKey("2.0.0"))
+
+
+@pytest.mark.asyncio
+async def test_archive_succeeds_and_writes_lifecycle_event():
target = _archive_candidate(version="1.0.0")
member = _make_member(release_result=target, siblings=[_archive_candidate(version="2.0.0")])
mock_data = member._CommitteeMember__data # type: ignore[attr-defined]
update_result = mock.MagicMock()
update_result.rowcount = 1
mock_data.execute_query = mock.AsyncMock(return_value=update_result)
- monkeypatch.setattr(release, "_remove_from_downloads", mock.AsyncMock())
await member.archive(safe.ProjectKey("example"), safe.VersionKey("1.0.0"))
@@ -131,45 +129,6 @@ async def test_archive_succeeds_and_writes_lifecycle_event(monkeypatch):
assert mock_data.commit.await_count == 1
-@pytest.mark.asyncio
-async def test_download_links_delete_tolerates_unsafe_filenames(tmp_path, monkeypatch):
- finished_dir = tmp_path / "finished"
- finished_dir.mkdir()
- bad_file = finished_dir / "ELC Admin Guide.pdf"
- bad_file.write_text("data")
-
- downloads_dir = tmp_path / "downloads"
- downloads_dir.mkdir()
- link_path = downloads_dir / "ELC Admin Guide.pdf"
- os.link(bad_file, link_path)
-
- monkeypatch.setattr(release.paths, "release_directory", lambda _release: finished_dir)
- monkeypatch.setattr(release.paths, "get_downloads_dir", lambda: downloads_dir)
-
- await release._release_download_links_delete(_fake_release())
-
- assert not link_path.exists()
- assert bad_file.exists()
-
-
-@pytest.mark.asyncio
-async def test_remove_from_downloads_swallows_errors(monkeypatch):
- boom = mock.AsyncMock(side_effect=RuntimeError("boom"))
- monkeypatch.setattr(release, "_release_download_links_delete", boom)
- await release._remove_from_downloads(_fake_release())
- boom.assert_awaited_once()
-
-
-def _fake_release() -> sql.Release:
- return sql.Release(
- key="example-1.0.0",
- phase=sql.ReleasePhase.RELEASE,
- created=datetime.datetime(2026, 1, 1, tzinfo=datetime.UTC),
- project_key="example",
- version="1.0.0",
- )
-
-
def _archive_candidate(
version: str = "1.0.0",
phase: sql.ReleasePhase = sql.ReleasePhase.RELEASE,
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org