sbp commented on code in PR #1561:
URL:
https://github.com/apache/tooling-trusted-releases/pull/1561#discussion_r3915460490
##########
atr/storage/writers/release.py:
##########
@@ -1598,8 +1639,14 @@ async def complete_archive(
# blocking the release
await self.__data.commit()
return None
- return await _archive_release(
- self.__data, self.__write_as, self.__asf_uid, project_key,
version_key, release
+ return await archive_release_core(
+ self.__data,
+ self.__write_as,
+ self.__asf_uid,
Review Comment:
During CAP resolve, this is `system`. Don't we always have to use a real ASF
UID when committing to SVN?
##########
atr/storage/writers/release.py:
##########
@@ -1773,6 +1820,119 @@ async def finalise_published_release(self, task_args:
args.ReleaseFinalise) -> r
message=message,
)
+ async def unpublish_from_svn_execute(self, task_args: args.SvnUnpublish)
-> results.SvnUnpublish:
+ # A None suffix means the release published to the committee dist
root, not
+ # that there's nothing to do - the files to remove come from the
artifact
+ # records, and the suffix only fixes the fallback directory.
+ suffix = task_args.download_path_suffix
+ publish_url = config.get().SVN_PUBLISH_URL
+ if not publish_url:
+ # The publish target went away between queuing and running, so fail
+ # cleanly rather than let the URL builder raise a bare ValueError.
+ raise datatypes.FailedError("SVN_PUBLISH_URL is not configured")
+ release = await self.__data.release(
+ project_key=str(task_args.project_key),
+ version=str(task_args.version_key),
+ _committee=True,
+ ).demand(datatypes.FailedError("Archived release not found for
removal"))
+ committee = release.committee
+ if committee is None:
+ raise datatypes.FailedError("Release has no committee - Invalid
state")
+
+ # The removal undoes the publish: take out only the files ATR recorded
for this
+ # release - the artifact, its signature, checksum and SBOM - grouped
under the
+ # directory each lives in. A directory the release owns outright is
taken whole,
+ # which clears its files, any subdirectories and the emptied directory
in one
+ # action; the committee's shared dist root only ever gives up our own
files.
+ artifacts = await self.__data.artifact(
+ project_key=str(task_args.project_key),
version=str(task_args.version_key)
+ ).all()
+ release_dir = str(paths.committee_dist_relpath(committee, suffix))
+ committee_root = str(paths.committee_dist_relpath(committee))
+ wanted: dict[str, set[str]] = {}
+ for artifact in artifacts:
+ directory = artifact.download_path_suffix or release_dir
+ for name in (artifact.artifact_path, artifact.signature_path,
artifact.checksum_path, artifact.sbom_path):
+ if name:
+ wanted.setdefault(directory, set()).add(name)
+ if not wanted:
+ return results.SvnUnpublish(
+ kind="svn_unpublish", svn_revision=None, message=f"No
published files recorded for {release.key}"
+ )
+
+ root_url = publish_url.rstrip("/")
+ rel_paths = await self.__plan_removals(root_url, committee_root,
wanted)
+ if not rel_paths:
+ return results.SvnUnpublish(
+ kind="svn_unpublish", svn_revision=None,
message=f"{release.key} was already absent from dist"
+ )
+
+ log_message = (
+ f"Unpublish
{task_args.project_key!s}-{task_args.version_key!s}\n\n"
+ f"Committee: {committee.key}\n"
+ "Tool: ATR\n"
+ f"Archived by {task_args.asf_uid} via ATR"
+ )
+ try:
+ output = await svn.remove_files(root_url, rel_paths,
task_args.asf_uid, log_message)
+ except svn.CommandExecutionError as exc:
+ log.error(f"SVN unpublish failed: {svn.error_message(exc)}")
+ raise datatypes.FailedError(svn.error_message(exc)) from None
+ revision = svn.parse_svnmucc_revision(output)
+ self.__write_as.append_to_audit_log(
+ asf_uid=task_args.asf_uid,
+ project_key=str(task_args.project_key),
+ version=str(task_args.version_key),
+ svn_revision=revision,
+ )
+ landed = f" as r{revision}" if revision is not None else ""
+ return results.SvnUnpublish(
+ kind="svn_unpublish",
+ svn_revision=revision,
+ message=f"Removed {len(rel_paths)} file(s) for {release.key} from
SVN{landed}",
+ )
+
+ async def __plan_removals(self, root_url: str, committee_root: str,
wanted: dict[str, set[str]]) -> list[str]:
+ # Keep only what's still in the dist area, so a re-run or a partial
earlier
+ # removal is idempotent and svnmucc - which is atomic - never fails
the whole
+ # commit on a path that has already gone. A genuine not-found means
the path is
+ # already gone; any other error (a connection or auth failure) is no
such proof,
+ # so it stays a failure.
+ rel_paths: list[str] = []
+ for directory, names in sorted(wanted.items()):
+ dir_url = f"{root_url}/{directory}"
+ if directory == committee_root:
+ # The shared dist root holds KEYS and other releases, so never
take the
+ # directory itself, only our own files still in it.
Non-recursive: a flat
+ # root may hold thousands of releases there's no reason to
walk.
+ present = await self.__list_present(dir_url, recursive=False)
Review Comment:
If a committee publishes a release straight into their committee directory,
with no prefix, and there are subdirectories, `recursive=False` means that the
listing call here won't find those files. A recursive list in SVN can still be
done with a single `svn list` command, but it's true that the output might be
huge.
##########
atr/storage/writers/release.py:
##########
@@ -1773,6 +1820,119 @@ async def finalise_published_release(self, task_args:
args.ReleaseFinalise) -> r
message=message,
)
+ async def unpublish_from_svn_execute(self, task_args: args.SvnUnpublish)
-> results.SvnUnpublish:
+ # A None suffix means the release published to the committee dist
root, not
+ # that there's nothing to do - the files to remove come from the
artifact
+ # records, and the suffix only fixes the fallback directory.
+ suffix = task_args.download_path_suffix
+ publish_url = config.get().SVN_PUBLISH_URL
+ if not publish_url:
+ # The publish target went away between queuing and running, so fail
+ # cleanly rather than let the URL builder raise a bare ValueError.
+ raise datatypes.FailedError("SVN_PUBLISH_URL is not configured")
+ release = await self.__data.release(
+ project_key=str(task_args.project_key),
+ version=str(task_args.version_key),
+ _committee=True,
+ ).demand(datatypes.FailedError("Archived release not found for
removal"))
+ committee = release.committee
+ if committee is None:
+ raise datatypes.FailedError("Release has no committee - Invalid
state")
+
+ # The removal undoes the publish: take out only the files ATR recorded
for this
+ # release - the artifact, its signature, checksum and SBOM - grouped
under the
+ # directory each lives in. A directory the release owns outright is
taken whole,
+ # which clears its files, any subdirectories and the emptied directory
in one
+ # action; the committee's shared dist root only ever gives up our own
files.
Review Comment:
This can fail if there are other files in there. Here's an example that I
had an agent actually run with this PR code:
```
PUBLISHED to project/1.0.0 by svn import:
CHANGES.txt
README.md
apache-project-1.0.0-src.tar.gz
apache-project-1.0.0-src.tar.gz.asc
apache-project-1.0.0-src.tar.gz.cdx.json
apache-project-1.0.0-src.tar.gz.cdx.json.asc
apache-project-1.0.0-src.tar.gz.cdx.json.sha512
apache-project-1.0.0-src.tar.gz.sha256
apache-project-1.0.0-src.tar.gz.sha512
RECORDED by announce in 1 Artifact row(s),
download_path_suffix='project/1.0.0':
apache-project-1.0.0-src.tar.gz
apache-project-1.0.0-src.tar.gz.asc
apache-project-1.0.0-src.tar.gz.cdx.json
apache-project-1.0.0-src.tar.gz.sha512
TASK RESULT: Removed 4 file(s) for project-1.0.0 from SVN as r2
LEFT IN SVN under project/ after unpublish:
1.0.0/
1.0.0/CHANGES.txt
1.0.0/README.md
1.0.0/apache-project-1.0.0-src.tar.gz.cdx.json.asc
1.0.0/apache-project-1.0.0-src.tar.gz.cdx.json.sha512
1.0.0/apache-project-1.0.0-src.tar.gz.sha256
```
There's an existing test for this that should prevent it, but it happens to
use exactly the subset of files that _doesn't_ cause this problem. Just adding
something like a `README.md` file or a checksum for the SBOM JSON would break
it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
