aptly-dev / aptly

aptly - Debian repository management tool
https://www.aptly.info/
MIT License
2.54k stars 369 forks source link

Enable SkipArchitectureCheck and IgnoreSignatures in mirror API #1300

Closed iofq closed 3 weeks ago

iofq commented 3 weeks ago

Fixes #

The command line flags -force-architectures and -ignore-signatures are not consistently implemented in the mirror API like they are in the mirror CLI. Discovered because the example repo I'm attempting to use, https://pkg.duosecurity.com/Ubuntu, is non-standard and breaks without these specified.

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

SkipArchitectureCheck and IgnoreSignatures are implemented as params for the create/POST and update/PUT handlers under /api/mirrors.

Did not implement unit tests as there doesn't appear to be a good harness for mocking downloads.

Updating functional tests would be nice, seems we can add to https://github.com/aptly-dev/aptly/blob/master/system/t12_api/mirrors.py if we potentially upload some system test files first. Would need some guidance to get started on that if desired.

Checklist

neolynx commented 3 weeks ago

awesome work, thanks !

but I wonder, if apt can handle this repo, should aptly not also be able to mirror this weird structure ?

iofq commented 3 weeks ago

but I wonder, if apt can handle this repo, should aptly not also be able to mirror this weird structure ?

I think aptly is behaving as expected here, aptly by default checks that the components and archs defined for the mirror are present in the Release file, and this repo does not list any components or archs in its Release file. The skip component and arch check are available to override that check.

This document states: "If a server is not specifying the supported architectures with this field client behavior is unspecified in this case.". It does not mention anything about Components field requirements. So it seems there are no standards for this case, and what we're seeing is aptly default behavior is actually more strict than apt itself.

Maybe aptly could be less strict about checking by default? Either move the checks from opt-out to opt-in, or perhaps check but only throw a warning, not an err.

neolynx commented 3 weeks ago

Thanks for the explanation !

Merged, and I also added a system test for SkipArchitectureCheck and IgnoreSignatures.