Repository navigation
Push npm and Composer packages into the proxy - #464
Conversation
d218443 to
f5aeacf
Compare
andrew
left a comment
andrew
left a comment
There was a problem hiding this comment.
Thanks for this, the split into commits made it easy to follow, and building the metadata entries from the stored manifest is the right call over the bare list in #134. Two things need fixing before merge, plus three smaller ones.
A missed cache read on a pushed artifact falls through to upstream and overwrites it. GetLocalArtifact (internal/handler/hosted.go:352) returns whatever checkCache returns, and checkCache returns nil, nil both when Storage.Open fails and after an integrity failure has cleared storage_path. The npm and Composer download handlers then carry on to the upstream path, GetOrFetchArtifact fetches the upstream tarball for the same version and filename, and updateCacheDB upserts the row with source = 'upstream' and manifest = NULL. A single storage error while downloading a patched request-2.88.3 serves the unpatched upstream tarball, drops the pushed record, and queues its object for deletion. When the record is source = local and the cache read misses, GetLocalArtifact should return an error (503, or 404 when there's no storage path) so the handler never falls back to upstream. A test that removes the stored object and asserts the upstream is never requested would cover it.
UpsertPackage now clears packages.source. The conflict clauses at internal/database/queries.go:74 and :91 gained source = excluded.source, but source isn't in the INSERT column list, so excluded.source is always NULL and every package upsert nulls the column. Nothing in the hosted feature uses it; those two lines should go.
Composer manifest lookup can pick a nested file. readComposerManifest (internal/handler/hosted_manifest.go:93) takes the first entry that is either composer.json or <dir>/composer.json, in zip order, so a dist zip that stores tests/composer.json before the root file is refused with a name mismatch. Prefer an exact root entry and only fall back to the single-top-directory form when there isn't one.
Denied pushed versions stay in metadata. Push and download both refuse denied versions, but the merges at npm.go:455 and composer.go:328 list every pushed version. Denying a pushed version after the fact leaves it listed, so resolvers pick it and then get a 403 on the tarball. Filtering denied versions out of local before the merge keeps metadata and downloads consistent.
CLI command names. proxy list and proxy delete sit next to serve, stats and mirror and read as cache operations, and renaming them after a release would break scripts. Please nest them as proxy hosted push|list|delete.
|
Thanks for the careful review, all five were right. Each fix is its own commit on top, so the changes since your review are easy to see:
|
andrew
left a comment
andrew
left a comment
There was a problem hiding this comment.
Thanks, all five fixes hold up. One problem with the checkCache change in f4dbbb2:
Pushed artifacts aren't served with storage.cache_artifacts: false. checkCache returns early when p.StreamArtifacts is set (if err != nil || result != nil || p.StreamArtifacts), and lookupCache returns nil, nil in that mode before looking at the record. GetLocalArtifact therefore returns nil, and the npm and Composer download handlers go to the upstream. With StreamArtifacts set after pushing request@2.88.3, GetLocalArtifact returns nil and GetOrFetchArtifactFromURL serves the upstream tarball, while the packument still lists the pushed version with the pushed sha512. npm clients then fail the integrity check (or install the unpatched upstream code where that check doesn't apply), and hosted-only scopes and Composer versions the upstream doesn't list answer 404.
Pushed archives aren't a cache, so streaming upstream artifacts shouldn't change how they're served. GetLocalArtifact should read the stored object regardless of StreamArtifacts, with the same 503/404 handling as now when it can't. A test that pushes a version, sets StreamArtifacts, downloads it and asserts the pushed bytes are served without an upstream fetch would cover it.
Artifacts get a source column: upstream for cached copies of a registry, local for archives pushed to the proxy. Existing rows become upstream through the new 012_add_artifact_source migration. Local artifacts aren't a cache, so the LRU size limit doesn't count or evict them and storage retention never selects them. They are only removed by deleting them. A local artifact has no upstream URL and stores it as empty. Refs git-pkgs#133
hosted.enabled turns on pushing packages to the proxy and is off by default, like mirror_api. A pushed version is served in place of the upstream version with the same number, so anyone who can write could swap a package for every client. Enabling it therefore requires hosted.token, which supports ${VAR_NAME} expansion and can be set with PROXY_HOSTED_TOKEN. hosted.max_upload_size caps one archive and defaults to 100MB.
requireBearerToken compares SHA-256 digests of the presented and configured tokens in constant time and answers 401 with a WWW-Authenticate challenge otherwise.
Refs git-pkgs#133
With hosted.enabled, PUT /api/hosted/{ecosystem}/{name}/{version} stores the request body as the archive for that version, marked local. It replaces any cached upstream copy and is served from the cache without contacting the upstream. GET /api/hosted/{ecosystem}/{name} lists what was pushed and DELETE removes a version again, after which clients get the upstream archive if there is one. Pushing and deleting need the bearer token; listing is open like the rest of the API.
Download handlers look archives up by the filename clients request, so a push is stored under the name its ecosystem derives from name and version: {name}-{version}.tgz for npm, without the scope, and {package}-{version}.zip for Composer. Other ecosystems are refused for now.
The Composer download handler resolves the upstream metadata before it looks at the cache and answers 404 for versions the upstream doesn't list. It now serves a pushed archive first. Only artifacts marked local take that path, so cached upstream archives still go through the metadata and commit checks.
The version's integrity is set to the pushed bytes, so the cache's integrity check verifies them rather than an upstream digest. Names and versions with path separators, dot segments or control characters are refused before anything is stored.
Refs git-pkgs#133
proxy push, proxy list and proxy delete call the /api/hosted endpoints of a running proxy. The proxy URL comes from -url or PROXY_URL and the token from PROXY_HOSTED_TOKEN, so it never ends up in shell history or process listings. Document the hosted config block, the endpoints, the supported ecosystems and how pushed archives interact with eviction. Refs git-pkgs#133
Resolvers take a version's dependencies from the registry metadata, not from the archive, so listing a pushed version needs its manifest. A push now reads package.json from the top directory of an npm tarball, or composer.json from the root or the single top directory of a Composer zip, and keeps it in the new artifacts.manifest column (migration 013_add_artifact_manifest). The manifest has to name the pushed package and version. A Composer manifest may leave the version out, and its name is compared case-insensitively. An archive that can't be read or whose manifest names something else is refused with 400 and removed from storage. The version's integrity is now the archive's sha512, which npm clients expect in dist.integrity. Refs git-pkgs#134
With hosted packages enabled, the packument of a package with pushed versions lists each of them, built from its package.json with dist pointing at the proxy and carrying the archive's sha512, and the time it was pushed. A pushed version takes the place of an upstream entry with the same version. Cooldown and the denylist still filter only upstream versions, and dist-tags stay as upstream has them. When the upstream doesn't know the package, the packument holds only the pushed versions, with latest set to the highest of them. Requests for a single pushed version are answered from its entry. Rewrites that include pushed versions bypass the rewrite cache, which is keyed by the upstream document alone, so a push or delete shows on the next request and changes the ETag. Packages without pushed versions take the cached path as before, and with hosted packages disabled there is no extra lookup. Refs git-pkgs#134
With hosted packages enabled, a package's p2 metadata lists its pushed versions, built from composer.json without the root-only keys, with a zip dist at the proxy and the time they were pushed. dist.reference is the archive's sha256 so a re-push isn't served from Composer's cache, and shasum is left empty. Dev versions go to the ~dev file and tagged versions to the other. A pushed version takes the place of an upstream entry with the same version, with or without a leading v. Pushed entries follow the upstream ones and go through the same minified writer, so fields an earlier upstream entry set and the pushed one lacks are written as __unset rather than inherited. When the upstream doesn't know the package, the document holds only the pushed versions. Like npm, documents with pushed versions bypass the rewrite cache. Refs git-pkgs#134
The packument lists pushed versions without cooldown, but the tarball download checked cooldown against the upstream's publish times before looking at the cache. A pushed override of a version the upstream published recently was listed and then withheld with 404. Pushed tarballs are now served right after the denylist check. Refs git-pkgs#134
…s alone A package the upstream doesn't know is served from its pushed versions, but the upstream is still asked about it. Once someone publishes the same name there, their versions are listed next to the pushed ones and a version range can resolve to them. hosted.npm_scopes and hosted.composer_vendors (PROXY_HOSTED_NPM_SCOPES, PROXY_HOSTED_COMPOSER_VENDORS) name namespaces whose packages are served from pushed versions alone. Their metadata is built from what was pushed, npm latest included, and anything not pushed answers 404. The upstream is never asked about them. Packages in other namespaces are proxied as before. The npm metadata handler's decisions about pushed versions move into serveLocalMetadata. Refs git-pkgs#133 Refs git-pkgs#134
The artifact source change added source = excluded.source to the packages upsert as well. source isn't in that INSERT's column list, so every package upsert set the column to NULL. The packages upsert is back to what it was, with a test that an existing source survives it. Refs git-pkgs#133
checkCache answered nil both when a stored object couldn't be opened and after an integrity failure had cleared the record. Download handlers then fetched the upstream archive for the same version and filename, and recording it turned the pushed row into an upstream one without its manifest and queued the pushed object for deletion. One storage error while downloading a patched version served the unpatched upstream archive and dropped the push. checkCache now looks at the record on a miss. For a pushed artifact it returns ErrHostedArtifactUnavailable when the object is recorded but couldn't be served, and ErrHostedArtifactMissing when no object is recorded any more. Every path that would fetch upstream goes through it, so none of them can replace the pushed artifact. Downloads answer 503 and 404 for the two. Refs git-pkgs#133
The manifest lookup took the first composer.json at the root or inside a top directory, in zip order. Dist archives keep composer.json files for tests and examples, so tests/composer.json stored before the root file refused the push with a name mismatch. The root composer.json now always wins. Without one, the manifest inside a top directory is used only when there is exactly one; more than one refuses the archive as ambiguous. Refs git-pkgs#134
Pushing and downloading refuse denied versions, but npm and Composer metadata listed every pushed version. A version denied after it was pushed stayed listed, so resolvers picked it and then got 403 on the download. localVersions now leaves denied versions out, which covers both ecosystems and the hosted-only namespaces; npm latest falls back to the highest allowed version. /api/hosted still lists them, so operators can see and delete them. Refs git-pkgs#134
proxy list and proxy delete sat next to serve, stats and mirror and read as cache operations. The commands are now proxy hosted push, proxy hosted list and proxy hosted delete, before a release makes the names hard to change. proxy hosted without a known subcommand prints the three. Refs git-pkgs#133
- GetLocalArtifact went through checkCache, which reports a miss when StreamArtifacts is set, so npm and Composer downloads fetched the upstream archive instead of the pushed one - Split readStored out of lookupCache and read pushed artifacts through it regardless of StreamArtifacts - Keep the denylist check and the 503/404 answers for unreadable or missing pushed artifacts - Test npm and Composer downloads of pushed archives with StreamArtifacts set Refs git-pkgs#133
3d03f31 to
c0ac418
Compare
|
@andrew Fixed in c0ac418: I also rebased onto main to resolve the conflicts. The only one was in |
2 participants
Operators can push a package archive into the proxy under a name and version. The proxy serves it in place of the upstream archive with the same version, or next to the upstream's versions, and lists it in the package metadata so clients resolve it like any release. This covers npm and Composer; the other ecosystems can follow one at a time on the same path.
Refs #133
Refs #134
Pushing, listing and deleting
With
hosted.enabled, three endpoints are registered:PUT /api/hosted/{ecosystem}/{name}/{version}GET /api/hosted/{ecosystem}/{name}DELETE /api/hosted/{ecosystem}/{name}/{version}proxy hosted push,proxy hosted listandproxy hosted deletewrap them, taking the URL from-url/PROXY_URLand the token fromPROXY_HOSTED_TOKEN.The proxy has no authentication of its own, and a pushed version replaces the upstream one for every client, so enabling
hostedrequireshosted.token. The check compares SHA-256 digests in constant time. The endpoints sit under/api/hostedrather than/api/packages, which already lists cached packages.Download handlers look archives up by the filename clients request, so a push is stored under the name its ecosystem derives:
{name}-{version}.tgzfor npm (without the scope) and{package}-{version}.zipfor Composer. The Composer download handler resolved upstream metadata before looking at the cache and answered 404 for versions the upstream doesn't list; it now serves a pushed archive first.A push reads
package.jsonorcomposer.jsonout of the archive and refuses it with 400 when the name or version doesn't match. The manifest is kept in the newartifacts.manifestcolumn.Storage
Artifacts get a
sourcecolumn,upstreamorlocal. The LRU size limit and storage retention skip local artifacts; only a delete removes them. A pushed version's integrity is the archive's sha512, so the cache's integrity check verifies the pushed bytes. Scanning, when enabled, checks pushed archives like fetched ones.Metadata
The issue suggested a list of
version, filename, sha256, urlentries. Resolvers take a version's dependencies from the registry metadata, though, so entries without them would install the package without its dependencies. Entries are built from the stored manifest instead.package.json,dist.tarballat the proxy anddist.integrityas the sha512. A request for a single pushed version is answered from its entry. Pushed tarballs are served before the cooldown check, since the packument lists them without cooldown.composer.json(root-only keys dropped) and a zip dist at the proxy,referencebeing the sha256 andshasumempty. Dev versions go to the~devfile. Pushed entries go through the existing minified writer, so they don't inherit fields such asversion_normalizedfrom the upstream entry before them.dist-tagsstay as the upstream has them.latestis the highest of them.hosteddisabled there is no extra lookup.Hosted-only namespaces
A private package served this way is still looked up upstream. If someone later publishes the same name there, their versions are listed next to the pushed ones, and a range can resolve to them.
hosted.npm_scopesandhosted.composer_vendorsname namespaces whose packages are served from pushed versions alone: the upstream is never asked about them, and anything not pushed answers 404.Not included
type=registrycache export) #410). Thesourcecolumn, the eviction rules and the token check are meant to carry over.npm publishsupport. Composer has no publish protocol of its own.Testing
go test -race ./...passes on SQLite and on Postgres (PROXY_DATABASE_URL), andgolangci-lint runreports no issues. Each commit builds and passes on its own. Tests cover the migrations, the token check, the API (including a running server withhostedenabled and disabled), the CLI against a stub server, manifest checks, and npm and Composer metadata and downloads with cooldown, the rewrite cache and hosted-only namespaces. Swagger docs are regenerated.