archive: add gitea generic package registry backend - #705
Draft
mahaase wants to merge 12 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #705 +/- ##
==========================================
+ Coverage 89.23% 89.33% +0.10%
==========================================
Files 50 50
Lines 16450 16586 +136
==========================================
+ Hits 14679 14817 +138
+ Misses 1771 1769 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
i'm not sure, but the root cause of the failed windows build could be: |
mahaase
force-pushed
the
feature/gitea-artifact-backend
branch
from
July 31, 2026 12:44
f72e3d7 to
fe1828f
Compare
Member
|
Thanks. I'll have a look after my vacation... |
jkloetzke
reviewed
Aug 18, 2026
jkloetzke
left a comment
jkloetzke
left a comment
Member
There was a problem hiding this comment.
Can you split the change into multiple commits? At least one for the implementation, one for the documentation and one for the tests? Also, if you need to extend the Webdav class, that should be a separate commit as well.
mahaase
force-pushed
the
feature/gitea-artifact-backend
branch
from
August 19, 2026 06:41
fe1828f to
d54543d
Compare
Contributor
Author
|
@jkloetzke Thanks for your review and feedback! Hopefully everything should be addressed. The change is now split into four commits:
|
mahaase
force-pushed
the
feature/gitea-artifact-backend
branch
from
August 19, 2026 07:16
183fe76 to
49a591d
Compare
Contributor
Author
|
disk full issue again. |
mahaase
force-pushed
the
feature/gitea-artifact-backend
branch
from
August 19, 2026 09:22
49a591d to
abade2f
Compare
jkloetzke
reviewed
Aug 23, 2026
jkloetzke
left a comment
jkloetzke
left a comment
Member
There was a problem hiding this comment.
This seems to have the same commits from #706. Maybe we should get that one merged first.
Please make the "webdav: fix pickling and support conflict replies and deletion by path" three commits. It contains three different things.
Also, "archive: strip URL credentials in the gitea backend too" should be squashed into the commit that creates the gitea backend.
Member
Please, don't let me talk to the AI. This actually feels kind of rude. In fact, it would be way faster if I'd do that myself directly. But the point of such a review is not just to get the code into shape. The more important part is to let the contributors know how things should be done. You should actually learn during the review so that future contributions need much less review comments. If I'm just talking to the Claude directly, this knowledge is probably not generated. At least this is what I have to assume. My fear is that is not a sustainable approach in the long run. Claude can generate changes like this PR quickly. But I need you as a contributor to make the first review. At least that is my hope for the future to keep the maintenance manageable. |
The HTTP basic authentication credentials are part of the archive URL. All
three places that turn that URL back into a string for display used the raw
netloc, which still carries the "user:password@" part:
* getArchiveName() feeds _namedErrorString(), so the credentials were
printed on *every* error message -- including the perfectly ordinary
"artifact not found" that occurs for each package on a cache miss. No
verbosity flag needed.
* _remoteName() is the "details" of the DOWNLOAD/UPLOAD/MAP-SRC/CACHE-BID/
CACHE-FPR/MAP-FPRNT status lines, shown with -v.
* getArchiveUri() is printed by "bob archive".
These messages routinely end up in build logs and CI consoles.
WebDav._getURL() already stripped the credentials before putting the URL on
the wire, so this only ever affected the display strings. Factor that logic
out into getNetLoc() and use it in the three spots above as well.
The optional 'name' archive setting was no workaround: _remoteName() and
getArchiveUri() do not consult it.
The user info is separated from the host by the *last* '@' of the network
location. That is what urlparse() does (it uses rpartition('@')) and what
RFC 3986 mandates, because '@' is allowed unencoded in the user info.
getNetLoc() cut at the first one instead. For a password containing an
unencoded '@' that produced a bogus host: the request URL got the remainder
of the password as its authority, so the request failed -- and the leftover
password fragment was shown in the resulting message.
Use rsplit('@', 1) so that the host matches the one urlparse() reports.
testNoCredentialsInMessages checks the three user visible strings derived from the archive URL: the password and the '@' delimiter must be gone while the host must survive. Reverting archive.py alone makes all three subtests fail. TestGetNetLoc covers the helper itself: URL without credentials, plain removal, a password containing an unencoded '@' and an IPv6 literal host.
The SSL context was created in the constructor. That rendered the object unpicklable, though, and the archive backends are sent to the up-/download executor processes. Using the http backend with "sslVerify: False" therefore aborted the build with a "cannot pickle 'SSLContext' object" error. Create the context on demand instead.
mahaase
marked this pull request as draft
August 26, 2026 11:40
urllib adds "Content-type: application/x-www-form-urlencoded" to every request that carries data unless the header is set explicitly. Binary artifacts were therefore uploaded as if they were an HTML form. Send "application/octet-stream" instead, which is what the artifacts actually are.
A WebDAV server that honours the "If-None-Match: *" header of a non-overwriting upload answers with a 412 if the file is already there. Some package registries that are not real WebDAV servers ignore the header and refuse to overwrite the file with a 409 instead. Map that to WebdavAlreadyExistsError as well so that the caller sees the usual "lost the race" condition instead of a hard error.
delete() takes a file name relative to the base path of the URL. Add deletePath() that takes the absolute path like upload() and download() do. delete() keeps its relative file name and just delegates. Needed by backends whose paths are not derived from the URL path.
All requests so far took the query string of the base URL. Let download() override it so that callers can pass request parameters. Needed for the paginated package API of a package registry.
Add a `gitea` archive backend that stores binary artifacts in a Gitea
generic package registry. The backend is configured with the server
`url`, the registry `owner` and the generic `package` name. Artifacts
are put below `{url}/api/packages/{owner}/generic/{package}/` with one
package version per artifact.
As for the http backend the HTTP basic authentication credentials are
part of the URL. Gitea accepts a personal access token in place of the
password. The registry only speaks plain HTTP (HEAD, GET, PUT and
DELETE), so the WebDav class does the transport. Like the http backend
the credentials are stripped from getArchiveName(), _remoteName() and
getArchiveUri() because these end up in the status lines and in every
"artifact not found" message.
The managed operations (scan/clean) are not implemented yet.
While at it, move the retry loop of the http backend into a small
function that both backends share.
The generic package registry can only put, get and delete single files. It
cannot enumerate its content, which is why the backend did not support the
archive command so far.
Listing is available through the package API of the server, though. It is not
part of the registry but knows about all package types:
* "GET /api/v1/packages/{owner}/generic/{package}" lists the package
versions, that is all artifacts of the archive. The reply is paginated
with at most 50 entries per request.
* ".../{version}/files" lists the files of one version together with their
size and hashes.
That is everything the ArchiveScanner needs. It walks the artifacts as if
they were stored in the "<xx>/<yy>/<rest>.tgz" layout of the file and http
backends, so the first two levels are answered from the version names alone
and only the last one has to look at the files of a version.
Doing so is not an optimization but a necessity: uploaded live-build-ids and
fingerprints create package versions that hold no artifact at all. Deriving
the file names from the version names would make the scanner stat files that
do not exist.
The file list also carries the sha256 of every file. That is used as the
change indicator of _stat(), which is a good deal more precise than the
modification time the other backends have to fall back to.
Deleting is native to the registry. The server drops the package version
along with its last file, so no extra cleanup is needed.
While at it, share the partial download loop of _getAudit() with the http
backend.
Describe the backend in the list of supported archive backends and add an example. Note how the credentials and the token scopes work, and that the managed operations rely on the package API of the server because the registry itself cannot enumerate its content.
Add a mock of the Gitea generic package registry that translates the package API layout to the on-disk layout of the shared archive tests, so that all the common upload/download assertions apply unchanged. On top of that, cover authentication via the URL and the individual error replies of the registry. The mock serves the package API of the server as well. That is what the managed operations are tested against: the three levels of the directory walk the archive command does, the pagination of the version list, a version that holds no artifact, the hash based stat, the audit trail and deleting. testNoCredentialsInMessages asserts that neither the token nor the user name of the URL show up in the three user visible strings, while the host survives.
mahaase
force-pushed
the
feature/gitea-artifact-backend
branch
from
August 26, 2026 11:53
abade2f to
f8c139e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a
giteaarchive backend that stores binary artifacts in a Gitea generic package registry. The backend is configured with the serverurl, the registryownerand the genericpackagename. Credentials are given explicitly as atoken(personal access token) or auser/passwordpair for HTTP basic authentication, so uploads work even inside the build sandbox. OptionalsslVerifyandretrieskeys tune the transport.Uploads perform a preflight check and report clearer connection and authentication errors.
depends on #706 (archive: do not leak URL credentials in user visible messages)