aptly-dev / aptly

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

Expose the copy package operation through REST API #1258

Closed an0rak-dev closed 2 months ago

an0rak-dev commented 6 months ago

Requirements

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

Description of the Change

In case we don't have the possibility to use the aptly CLI on a repository, expose the aptly copy command through the REST API.

Checklist

neolynx commented 5 months ago

good idea ! are you working on the implementation ?

an0rak-dev commented 4 months ago

good idea ! are you working on the implementation ?

@neolynx : Absolutely ! We've been a little busy last few days but we resume our work on this 😄

an0rak-dev commented 3 months ago

@neolynx I have a dilemma regarding the tests of the new /repos copy endpoint.

I'll need to create a local aptly repo with an existing .deb package in it while setting up the test. To do so, I have three possibilities :

  1. We curl a light .deb from a well known public mirror like Debian (pro : easy to implement and platform independent. Cons : unit testing requires an active internet connection and you'll end with a random .deb file on your local system).
  2. We embedded a prebuilt, mock .deb file in the git repo as a test resource and we copy it to setup our tests (pro : no internet connection required, all the tooling to run the test is already present in the repo. Cons : We add a random binary package to the git repo.)
  3. We set up a mock executable with the debian folder and all the needed files (control,copyright, rules...) and in the make test target we forge the .deb locally using those before running the test. (Pro : self sufficient and no random binary package. cons : will increase the setup time of the test and will need some tweaks to run unit testing on systems which aren't debian based).

I kind of like the choice 3, and I'll say that at worst we can still forge the .deb using a local docker container with a volume.

What do you think ?

neolynx commented 3 months ago

I think we already have option 2:

$ find -name *.deb
./deb/testdata/dbgsym-with-source-version/dbgsym-with-source-version_0.1+aptly2021.01_amd64.deb
./deb/testdata/changes/hardlink_0.2.0_i386.deb
./deb/testdata/changes/hardlink_0.2.1_amd64.deb
./system/t06_publish/PublishRepo29Test/libboost-broken-program-options-dev_1.49.0.1_i386.deb
./system/t06_publish/FSEndpointPublishSnapshot17Test/2/libboost-broken-program-options-dev_1.49.0.1_i386.deb
./system/t06_publish/FSEndpointPublishSnapshot17Test/1/libboost-broken-program-options-dev_1.49.0.1_i386.deb
./system/t06_publish/FSEndpointPublishSnapshot18Test/2/libboost-broken-program-options-dev_1.49.0.1_i386.deb
./system/t06_publish/FSEndpointPublishSnapshot18Test/1/libboost-broken-program-options-dev_1.49.0.1_i386.deb
./system/changes/hardlink_0.2.1_amd64.deb
./system/t04_mirror/test_release2/pool/main/a/amanda/amanda-client_3.3.1-3~bpo60+1_amd64.deb
./system/files/libboost-program-options-dev_1.49.0.1_i386.deb
./system/files/libboost-program-options-dev_1.62.0.1_i386.deb

and it might be easier to create a system test for the copy API, see ReposAPITestAddFile in system/t12_api/repos.py.

the system tests you should be able to prepare with make docker-build-system-tests and then run them with make docker-system-tests, optionally with TEST=t12_api (since running all tests takes ~20mins, but might depend on previous tests to be run once)

I would suggest to remove the api/repos_test.go and create a system test in python...

an0rak-dev commented 3 months ago

Oh ! My bad, I was mislead by the existing api/*_test.go files.

I'll rewrite the test using the existing E2E system.

neolynx commented 3 months ago

rebased :)

flake8 complains: https://github.com/aptly-dev/aptly/actions/runs/9769113398/job/26967965110?pr=1308#step:9:1032

neolynx commented 3 months ago

thanks for your amazing work, @an0rak-dev !

seems like the test is failing:

[GIN] 2024/07/03 - 22:08:30 | 202 |   11.153459ms |       127.0.0.1 | POST     "/api/repos/FaAZ8T9QodtFhKL/copy/LztTvFCdwDRcIlw/libboost-program-options-dev_1.49.0.1_i386.deb?_async=True"
[GIN] 2024/07/03 - 22:08:30 | 200 |    5.489289ms |       127.0.0.1 | GET      "/api/tasks/38/wait"
[GIN] 2024/07/03 - 22:08:30 | 200 |   10.977309ms |       127.0.0.1 | GET      "/api/tasks/38"
[GIN] 2024/07/03 - 22:08:30 | 200 |   10.178303ms |       127.0.0.1 | GET      "/api/repos/FaAZ8T9QodtFhKL/packages"
[GIN] 2024/07/03 - 22:13:30 | 200 |    85.05132ms |       127.0.0.1 | GET      "/api/repos/FaAZ8T9QodtFhKL/packages"
FAIL
Traceback (most recent call last):
  File "/app/system/run.py", line 126, in run
    t.test()
  File "/app/system/lib.py", line 179, in test
    self.check()
  File "/app/system/t12_api/repos.py", line 393, in check
    self.check_equal(self.get(f"/api/repos/{repo2_name}/packages").json(),
  File "/app/system/lib.py", line 421, in check_equal
    self.verify_match(a, b, match_prepare=pprint.pformat)
  File "/app/system/lib.py", line 472, in verify_match
    raise Exception("content doesn't match:\n" + diff + "\n")
Exception: content doesn't match:
--- 
+++ 
@@ -1 +1 @@
-[]
+['Pi386 libboost-program-options-dev 1.49.0.1 918d2f433384e378']

the 2nd repo returns an emtpy list... was the package not copied ?

I think there should be no ".deb" in the :file argument...