publishRenameConcurrency (publish_rename_concurrency in YAML) sets how
many index files RenameFiles renames at once. It defaults to 4, the
value that was hard-coded, and 1 renames them one at a time. Values
below 1 are treated as 1, since a semaphore with no capacity would
never admit a rename.
The setting is global rather than per publishing endpoint because the
renames run against every storage backend, not only S3.
RenameFiles walks renameMap one entry at a time. Every rename is a round trip,
and on an S3 backend RenameFile is a CopyObject followed by a DeleteObject, so
a publish that rewrites a dozen indexes spends most of its wall clock waiting
on them in sequence.
Bounded at four in flight, matching the default used for concurrent package
uploads. The first error is returned rather than the last, and the remaining
renames still run, so a failure does not leave part of the set staged behind a
half-finished switch.
Measured against MinIO behind a proxy adding 50ms each way, publishing a
repository whose Contents is dominated by one package carrying 19,500 files:
publish update drops from 2703ms to 1624ms median, n=5 either side, a 40% cut.
The published object set and its sizes are identical before and after.
RenameFiles had no test coverage. It has four cases now: every entry renamed,
the concurrency limit respected, a failure reported without abandoning the
rest, and an empty rename map.
Acquire-By-Hash entries are written for MD5Sum, SHA1, SHA256 and SHA512,
hardcoded in two places in deb/index_files.go. Every algorithm is a full
set of index copies on each publish, and on S3-like storage each of those
is a network round trip rather than a link.
Measured on an R2-backed archive of 25 packages across three suites, one
`aptly publish update` per suite:
Acquire-By-Hash off 38 S3 requests
Acquire-By-Hash on 222 S3 requests (88 HeadObject, 83 CopyObject,
35 DeleteObject, 13 PutObject)
Modern apt verifies with SHA256; MD5Sum and SHA1 are deprecated and
Debian has been phasing them out of Release files for years. Publishers
who do not need them are paying three quarters of that for nothing, with
no way to say so.
acquireByHashAlgorithms in aptly.conf now selects the set. An empty or
absent value keeps all four, so existing configurations are unaffected.
The order entries are written in comes from the code rather than the
config, so the directories created in one loop and the entries written in
another cannot disagree. An unrecognised name is an error rather than
something to skip: skipping a typo would produce "no by-hash entries for
that algorithm", which looks exactly like having configured it away.
Set per publish invocation from the config, the way SkipBz2 already is,
rather than persisted on the publish point.
Pre-existing test failures on this tree are unchanged by this commit:
deb goes 173->177 passed with the same 11 failures, utils keeps its one
(TestCompress), s3 stays green.
This patch adds validation to the version pasing logic (parseVersion) to
not allow packages to be registered which apt wouldn't accept anyway.
It then propogates validation errors through CompareVersions, the way
in which parseVersion gets called, so that appropriate decisions can be
made.
New packages with invalid versions are not accepted.
For any pre-exisiting packages with invalid versions, as much care is
taken as possible to log warnings, and not use those packages.
This PR is loosely bnased on one by by Aaron Foster
<afoster@cloudflare.com> though it has largely been written at this
point.
Signed off with internal and external emails for clarity, but commit
is under personal email where we typically do open source contributions
from.
Signed-off-by: Phil Dibowitz <pdibowitz@cloudflare.com>
Signed-off-by: Phil Dibowitz <pdibowitz@ipom.com>
Regression test for the duplicated skel payload: publishing two
components used to append each skeleton file once per component. Fails
against the previous code with doubled file contents, mirroring the
PublishRepo40Test system test at unit level.
Publish moved the skel and AppStream emission out of the per-component
loop, and codecov flags the relocated error branches as uncovered. Cover
them with deterministic failures:
- a regular file in the middle of the skel path makes the walk fail
with ENOTDIR, which is not swallowed as a not-exist error
- nested skel/AppStream names each within NAME_MAX flatten into a
temporary index file name that exceeds it, failing index creation
- an unreadable skel file (mode 0) fails the open (non-root only)
- a ~1MB filesystem as TMPDIR runs out of space mid-write, reusing the
/smallfs convention from DiskFullSuite
Also add a unit-level happy path test publishing nested skel files and
checking they land in the published tree and the Release checksums.
SkelIndex was the only index constructor not propagating acquireByHash,
so with -acquire-by-hash the Release file advertises Acquire-By-Hash: yes
while no by-hash/ entries exist for these files and apt gets a 404.
The skeleton and AppStream loops were nested inside the per-component
loop while iterating over all components themselves, so every file was
written once per component. Publishing main + contrib appended each
payload twice. Release checksums are computed after the fact, so the
duplicated content is signed and apt accepts it silently.
If two publish jobs run concurrently (which they arguably shouldn’t),
they’d step on each other toes and remove Packages.tmp file the other
job’s just created.
The API has locking for concurrent publish/drop operations on the same
prefix, but command-line operations bypass that lock. Also, not having
a predictable temporary filename is generally good from a security
standpoint, given that cryptographic signing is involved.
Work around the issue by creating a per-job temporary file.
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
* perform collection.LoadComplete inside maybeRunTaskInBackground
* have tasks use a fresh copy of taskCollectionFactory, taskCollection
* fix locking for snapshots of snapshots by locking SourceSnapshots
* use uuids, since names can be renamed
* remove useless resource lock
Resource locks need to be before the background task. creating same publish endpoint at the same time is unlikely...
* load data inside background tasks
This fixes a flaw in async apis, which loaded the published repo from the DB and mutated it outside the task closure, before the task lock was acquired.
Perform collection.LoadComplete inside maybeRunTaskInBackground and have tasks use a fresh copy of taskCollectionFactory, taskCollection
* lock source repos/snapshots for publish operations
Concurrent tasks were not properly locking their resources, leading to inconsistent published indexes:
SourceLocalRepo: iterate published.Sources (component -> source UUID), look up each local repo via localRepoCollection.ByUUID and append string(repo.Key()) to resources
SourceSnapshot: iterate b.Snapshots,look up each snapshot via snapshotCollection.ByName and append string(snapshot.ResourceKey()) to resources.
* lock pool on non MultiDist publish
* revert mutex on LinkFromPool
* use uuids, since names can be renamed
* add test for MultiDist change
Implement support for the SOURCE_DATE_EPOCH environment variable as
specified by reproducible-builds.org. When set, this variable overrides
the current timestamp in the Release file's Date and Valid-Until fields,
enabling reproducible filesystem publishes.
- Read SOURCE_DATE_EPOCH environment variable in Publish()
- Use the epoch timestamp for both Date and Valid-Until fields
- Gracefully fallback to current time if unset or invalid
- Add comprehensive tests for valid and invalid SOURCE_DATE_EPOCH values
https://wiki.debian.org/DebianRepository/Format#Signed-By says:
> **Signed-By**
> An optional field containing a comma separated list of
> OpenPGP key fingerprints to be used for validating
> the next Release file. The fingerprints must consist
> only of hex digits and may not contain spaces.
> The fingerprint specifies either the key the Release file
> must be signed with or the key the signature key must be
> a subkey of. The later match can be disabled by appending
> an exclamation mark to the fingerprint.
>
> If the field is present, a client should only accept future updates
> to the repository that are signed with keys listed in the field.
> The field should be ignored if the Valid-Until field is not present
> or if it is expired.
For both the CLI tools and JSON, the field is taken as a string verbatim.
When specified, we must also provide `Valid-Until` field,
and i'm not sure there is an 'infinity' value for it,
so 100 years will have to do?
Fixes https://github.com/aptly-dev/aptly/issues/1497
Initially found by automated repository health checks used by Termux
in https://github.com/termux/termux-packages/issues/27472
The root problem was 4.3.5a comparing less than 4.3.5-rc1-1 by aptly
According to debian "4.3.5a" > "4.3.5-rc1-1"
This is because dpkg splits hyphen for revision at the first hyphen,
whereas aptly was splitting at the last hyphen which is different from
dpkg's behaviour.
dpkg behaviour: https://git.dpkg.org/cgit/dpkg/dpkg.git/tree/lib/dpkg/parsehelp.c#n242
Perhaps this wasn't detected as there was broken tests in the repository
since the initial commit of aptly. This also fixes those tests
When aptly crashes it is possible to get a corrupt database with a dangling key reference.
This results in an error with 'key not found', eg:
ERROR: unable to load package Pall example-package 1.2.3 778cf6f877bf6e2d: key not found
This change makes `db recover` fix this situation by removing the dangling references.
The fix of the -with-filter flag causes the following previously
missing source files to be downloaded, so I updated the test file.
```
rkward_0.7.5-1~bullseyecran.0.debian.tar.xz
rkward_0.7.5-1~bullseyecran.0.dsc
rkward_0.7.5.orig.tar.gz
rpy2_3.5.12-1~bullseyecran.0.debian.tar.xz
rpy2_3.5.12-1~bullseyecran.0.dsc
rpy2_3.5.12.orig.tar.gz
```
Hansjoerg Maurer
André Roth
Rascalov
Martin Simon
Twaik Yont
Phil Dibowitz
KereMath
Andrej Shadura
Catalin Muresan
Philip Cramer
Zhang Xiao
Tim Foerster
Roman Lebedev
Juan Calderon-Perez
Yaksh Bariya
JupiterRider
Silke Hofstra
Mikel Olasagasti Uranga
Gordian Schoenherr