Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable SkipArchitectureCheck and IgnoreSignatures in mirror API #1300

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

iofq
Copy link
Contributor

@iofq iofq commented Jun 13, 2024

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

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated - See Merge Request
  • author name in AUTHORS

@neolynx
Copy link
Member

neolynx commented Jun 13, 2024

awesome work, thanks !

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

@iofq
Copy link
Contributor Author

iofq commented Jun 13, 2024

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 neolynx merged commit 7407439 into aptly-dev:master Jun 14, 2024
7 checks passed
@neolynx
Copy link
Member

neolynx commented Jun 14, 2024

Thanks for the explanation !

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants