anydistro / bxt

Next generation repository maintenance tool (WIP)
GNU Affero General Public License v3.0
0 stars 4 forks source link

Copy package request reports 200 but no package is copied #92

Closed fhdk closed 2 days ago

fhdk commented 1 month ago

Sending a copy request returns 200 OK but the package is not copied from branch/repo/arch to the destination.

The tests/30_pkg_copy.py script from bxtctl is used

bxt_copy_pkg : 
{('to_copy',
  '[{"name": "arch-install-scripts-28-1-any.pkg.tar.zst", "from_section": '
  '{"branch": "unstable", "repository": "extra", "architecture": "x86_64"}, '
  '"to_section": {"branch": "unstable", "repository": "multilib", '
  '"architecture": "x86_64"}}]')}
copy request begin -->  2024-07-13 14:49:11
copy response recv -->  2024-07-13 14:49:11
               headers  {'Alt-Svc': 'h3=":443"; ma=2592000', 'Content-Length': '15', 'Content-Type': 'application/json; charset=utf-8', 'Date': 'Sat, 13 Jul 2024 12:49:11 GMT', 'Server': 'Caddy, drogon/1.8.2'}
               status   200
LordTermor commented 1 month ago

name is just a package name, not a full file name, so it should be "name": "arch-install-scripts" but I'll add error reporting and clear out OpenAPI docs about that.

fhdk commented 1 month ago

Thank you for the clarification. I clearly didn't think of that - got so used to being specific :smile:

I assume this is the same rule with delete then.

A quick test - changing the package name - I still get 200 - but nothing happens -

bxt_copy_pkg : 
{('to_copy',
  '[{"name": "arch-install-scripts", "from_section": {"branch": "unstable", '
  '"repository": "extra", "architecture": "x86_64"}, "to_section": {"branch": '
  '"unstable", "repository": "multilib", "architecture": "x86_64"}}]')}
copy request begin -->  2024-07-13 16:16:46
LordTermor commented 1 month ago

I assume this is the same rule with delete then.

It is, I'll clear out this in OpenAPI

A quick test - changing the package name - I still get 200 - but nothing happens -

I will check that.

fhdk commented 1 month ago

When I create move/copy/delete transaction in the webUI - and do the commit - the transaction succeed.

When I post a multipart/form-data using Python - the same action does not succeed.

Thus the reasoning is - what am I doing wrong ?

As theupload files action succeed - I am thinking - the move/copy/delete is optional but the files section is required (per documentation) I am speculating if the there is bug in endpoint which cause the transaction to fail if there is no fileupload component?

Also the log entry is empty - when done with the webUI the log entry shows the action(s)

fhdk commented 1 month ago

I am specualting if I am getting the documentation all wrong.

When looking at swagger endpoint - and the PackageSection model I read it as

{"branch": "unstable", "repository": "multilib", "architecture": "x86_64"}

but what if I am only supposed to provide the values ?

("unstable", "multilib", "x86_64")
LordTermor commented 1 month ago

When I create move/copy/delete transaction in the webUI - and do the commit - the transaction succeed.

You actually can try inspecting Network tab in browser dev tools (Control + Shift + I) to check what it sends

fhdk commented 1 month ago

I can do that - but with documentation - that shouldn't be necessary.

I feel like I am supposed to reverse engineer your work - with good documentation that shouldn't be needed.

You know exactly how you send the data - but you do not care to explain it in the documentation.

LordTermor commented 1 month ago

I feel like I am supposed to reverse engineer your work - with good documentation that shouldn't be needed.

No you don't but I don't really understand what's the problem with docs.

I've checked - copying worked with the following JSON which perfectly follows openapi's schema:

[{
    "name": "arch-install-scripts",
    "from_section": {
        "branch": "unstable",
        "repository": "extra",
        "architecture": "x86_64"
    },
    "to_section":{
        "branch": "unstable",
        "repository": "extra",
        "architecture": "aarch64"
    }
}]

Here's a Insomnia's log:


> POST /api/packages/commit HTTP/2
> Host: bxt.staging.manjaro.org
> cookie: access_token=<hidden>; token=<hidden>
> content-type: multipart/form-data; boundary=X-INSOMNIA-BOUNDARY
> user-agent: Insomnia/2023.5.5
> accept: */*
> content-length: 333

* TLSv1.2 (OUT), TLS header, Supplemental data (23):

| --X-INSOMNIA-BOUNDARY
| Content-Disposition: form-data; name="to_copy"
| [{
|   "name": "arch-install-scripts",
|   "from_section": {
|       "branch": "unstable",
|       "repository": "extra",
|       "architecture": "x86_64"
|   },
|   "to_section":{
|       "branch": "unstable",
|       "repository": "extra",
|       "architecture": "aarch64"
|   }
| }]
| --X-INSOMNIA-BOUNDARY--

* We are completely uploaded and fine
* TLSv1.2 (IN), TLS header, Supplemental data (23):

< HTTP/2 200 
< alt-svc: h3=":443"; ma=2592000
< content-type: application/json; charset=utf-8
< date: Fri, 19 Jul 2024 04:16:18 GMT
< server: Caddy
< server: drogon/1.8.2
< content-length: 15

* TLSv1.2 (IN), TLS header, Supplemental data (23):
* Received 15 B chunk
* Connection #1 to host bxt.staging.manjaro.org left intact

UPD: I'm trying your code right now and it indeed doesn't work for me with the same structure, looking what's wrong.

fhdk commented 1 month ago

Looking at your request header - I see cookie auth

cookie: access_token=<hidden>; token=<hidden>

I use bearer token ?

Though that does not hint why it works with upload but not with the other form parts.

LordTermor commented 1 month ago

Ok I got it. requests adds filename parameter to the resulting multipart request body.

--c5e67c2433cb54f875ef027822f79147\r\nContent-Disposition: form-data; name="to_copy"; filename="to_copy"\r\n\r\n[{"name": "arch-install-scripts", "from_section": {"branch": "unstable", "repository": "multilib", "architecture": "x86_64"}, "to_section": {"branch": "unstable", "repository": "core", "architecture": "x86_64"}}]\r\n--c5e67c2433cb54f875ef027822f79147--\r\n

Which confuses drogon's MultiPartParser into thinking it's a file while it's not.

https://github.com/anydistro/bxt/blob/3dd55df491742749fe4b916fc7bce9583e722aaf/daemon/presentation/web-controllers/PackageController.cpp#L54-L55

And params_map ends up being empty while files_map contains the actual data. So bxt just thinks you sent an empty commit.

I think you should do something like


bxt_copy_pkg = {
    (
        "to_copy",
        ( None,  # <--- Note 'None' here
            json.dumps(
                [{"name": test_pkg, "from_section": section_b, "to_section": section_a}]
            )
        ),
    ),
}

And do that every time you send non-file data.

fhdk commented 1 month ago

Yes - I am doing that - that part of the tuple refers to the filename when a filename is sent and with None the data is added as the form content.

LordTermor commented 1 month ago

Yes - I am doing that - that part of the tuple refers to the filename when a filename is sent and with None the data is added as the form content.

but it's not in your copy test

https://github.com/fhdk/bxtctl/blob/825924ab710e9b166c7db4a2c0f3052b2b443b20/tests/30_pkg_copy.py#L69-L78

fhdk commented 1 month ago

I have tried a lot of different ways of providing the form to get the endpoint to accept it.

I have not succeeded yet.

I have just pushed the latest code

fhdk commented 1 month ago

The copy request details when script is executed - the result is 200 OK - but nothing happens on the endpoint

bxt_copy_pkg : 
req headers : {'Authorization': 'Bearer *******', 'Accept': 'application/json', 'Content-Type': 'multipart/form-data', 'Content-Length': '337'}
req url     : https://bxt.staging.manjaro.org/api/packages/commit
req data    : b'--ec1132e3511ce0f908ece9c759498ccd\r\nContent-Disposition: form-data; name="to_copy"\r\n\r\n[{"name": "arch-install-scripts", "from_section": {"branch": "unstable", "repository": "extra", "architecture": "aarch64"}, "to_section": {"branch": "unstable", "repository": "extra", "architecture": "x86_64"}}]\r\n--ec1132e3511ce0f908ece9c759498ccd--\r\n': 
copy request begin -->  2024-07-19 09:06:17
copy response recv -->  2024-07-19 09:06:17
               headers  {'Alt-Svc': 'h3=":443"; ma=2592000', 'Content-Length': '15', 'Content-Type': 'application/json; charset=utf-8', 'Date': 'Fri, 19 Jul 2024 07:06:17 GMT', 'Server': 'Caddy, drogon/1.8.2'}
               status   200
LordTermor commented 1 month ago

Your bxtctl user doesn't have access to sections other than */*/x86_64 though... I've tried with default user (which has * permissions) and it works.

image

fhdk commented 1 month ago

OHH :man_facepalming: - I did forget that I set the bxtctl to only use x86_64 - in any case you are right about the endpoint not reporting 403 - I only used the aarch64 because you copy/moved it there. Completely forgot about the limited permissions.

My previous tests were limited to x86_64

fhdk commented 1 month ago

After corrected for the aarch architecture - using x86_64 as permissions should checkout there - the request is accepted as expected but the package does not copy from unstable/multilib to testing/extra.

Is there some backend checks which rejects an incompatible copy between repos ?

The adapted script output

bxt_copy_pkg : 
req headers : {'Authorization': 'Bearer *****', 'Accept': 'application/json', 'Content-Type': 'multipart/form-data', 'Content-Length': '338'}
req url     : https://bxt.staging.manjaro.org/api/packages/commit
form data   : b'--39d47ec36e96e9c880922bddf21a41f5\r\nContent-Disposition: form-data; name="to_copy"\r\n\r\n[{"name": "arch-install-scripts", "from_section": {"branch": "unstable", "repository": "multilib", "architecture": "x86_64"}, "to_section": {"branch": "testing", "repository": "extra", "architecture": "x86_64"}}]\r\n--39d47ec36e96e9c880922bddf21a41f5--\r\n': 
copy request begin -->  2024-07-19 09:33:42
response recv -->  2024-07-19 09:33:42
      headers -->  {'Alt-Svc': 'h3=":443"; ma=2592000', 'Content-Length': '15', 'Content-Type': 'application/json; charset=utf-8', 'Date': 'Fri, 19 Jul 2024 07:33:42 GMT', 'Server': 'Caddy, drogon/1.8.2'}
       status -->  200
LordTermor commented 1 month ago

Is there some backend checks which rejects an incompatible copy between repos ?

Nope there isn't. I will try all your checks on my local instance to see what's happening on backend.

fhdk commented 1 month ago

I don't think either ( the rejection thought )

fhdk commented 2 days ago

The copy package transaction request now works.

Perhaps it is a combination of efforts - I don't know - what I do know - it is no longer an issue