Solve last CORS issues about duplicated headers

[nesitor]

Fix: Solve last CORS errors raised cause by duplication of headers returned.

[codecov]

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 44.20%. Comparing base (4ac9099) to head (ed12122).

Files	Patch %	Lines
src/aleph/vm/orchestrator/views/__init__.py	0.00%	5 Missing ⚠️
src/aleph/vm/orchestrator/resources.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #604      +/-   ##
==========================================
+ Coverage   44.17%   44.20%   +0.02%     
==========================================
  Files          55       55              
  Lines        5023     5022       -1     
  Branches      595      595              
==========================================
+ Hits         2219     2220       +1     
+ Misses       2678     2676       -2     
  Partials      126      126              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Failed to retrieve llama text: POST 504:

504 Gateway Time-out

The server didn't respond in time.

[Antonyjin]

I've looked at the changes and don't see any particular problems. Is there a way to check it manually?

[olethanh]

I'm not sure what the use of this decorator, not just here but in general, since a global cors config was added in supervisor.py?

[nesitor]

It's a custom decorator that handles the CORS using this library: https://github.com/aio-libs/aiohttp-cors

[olethanh]

Yeah I have read the decorator code, but you realise it add the same cors config that you also added in supervisor.py in your previous PR? Doesn't that seems a bit strange to you?

[nesitor]

No, it is a public endpoint used by the frontend, our frontend or other ones, so the endpoint should accept the same configuration as the other endpoints. As the decorator name says, allow all origins on CORS.

[hoh]

This is the sixth PR about CORS issues. I still don't see any automated test on these endpoints.

Can we please get some tests with pytest that ensure that all of these enpoints respond and have the CORS headers present in the responses ?

[olethanh]

as discussed on the team chat, it's missing a lot of information to test the whole CORS information properly but it seems to improve the situation.

Before calling http://localhost:4020/status/check/fastapi from another site was crashing with this error

  File "/usr/lib/python3/dist-packages/aiosignal/__init__.py", line 36, in send                                                                         
    await receiver(*args, **kwargs)  # type: ignore                                                                                                     
  File "/home/ubuntu/aleph-venv/lib/python3.10/site-packages/aiohttp_cors/cors_config.py", line 171, in _on_response_prepare                            
    assert hdrs.ACCESS_CONTROL_ALLOW_ORIGIN not in response.headers                                                                                     
AssertionError    

Here is an export of the request I had as fetch

await fetch("http://localhost:4020/status/check/fastapi", {
    "credentials": "omit",
    "headers": {
        "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:124.0) Gecko/20100101 Firefox/124.0",
        "Accept": "*/*",
        "Accept-Language": "en-US,en;q=0.5",
        "Content-Type": "application/json",
        "Sec-Fetch-Dest": "empty",
        "Sec-Fetch-Mode": "cors",
        "Sec-Fetch-Site": "same-site"
    },
    "referrer": "http://localhost:5174/",
    "method": "GET",
    "mode": "cors"
});

Now it is not.

[nesitor]

This is the sixth PR about CORS issues. I still don't see any automated test on these endpoints.

Can we please get some tests with pytest that ensure that all of these enpoints respond and have the CORS headers present in the responses ?

Yes, it's true, we need some tests to avoid have future CORS issues, you also can create that unit tests.

[nesitor]

as discussed on the team chat, it's missing a lot of information to test the whole CORS information properly but it seems to improve the situation.

Before calling http://localhost:4020/status/check/fastapi from another site was crashing with this error

  File "/usr/lib/python3/dist-packages/aiosignal/__init__.py", line 36, in send                                                                         
    await receiver(*args, **kwargs)  # type: ignore                                                                                                     
  File "/home/ubuntu/aleph-venv/lib/python3.10/site-packages/aiohttp_cors/cors_config.py", line 171, in _on_response_prepare                            
    assert hdrs.ACCESS_CONTROL_ALLOW_ORIGIN not in response.headers                                                                                     
AssertionError    

Here is an export of the request I had as fetch

await fetch("http://localhost:4020/status/check/fastapi", {
    "credentials": "omit",
    "headers": {
        "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:124.0) Gecko/20100101 Firefox/124.0",
        "Accept": "*/*",
        "Accept-Language": "en-US,en;q=0.5",
        "Content-Type": "application/json",
        "Sec-Fetch-Dest": "empty",
        "Sec-Fetch-Mode": "cors",
        "Sec-Fetch-Site": "same-site"
    },
    "referrer": "http://localhost:5174/",
    "method": "GET",
    "mode": "cors"
});

Now it is not.

If you check the code files changed on that PR, I have removed that hardcoded headers related to CORS on some endpoint that created the issue: headers={"Access-Control-Allow-Origin": "*"}
Also, checking the team's chat, on the virtualization topic, I explain the reason about what is failing:

Seems that I found the cause about this issues. Seems that depending on some endpoints, also they have the CORS middleware, but if the response force to return the Access-Control-Allow-Origin header, the middleware crashes. It happens only on some endpoints, and the /status/check/ipv6 is one of that endpoints, because the /status/check/host or /status/check/fastapi doesn't fail. Anyway I will create now a PR solving that things with CORS to ensure that never fails again, and also I'm finishing the other PR with the bug that I found.

[olethanh]

I understood your change and have validated that this change works, I meant checking for other potential problems.

[nesitor]

I understood your change and have validated that this change works, I meant checking for other potential problems.

As I have mentioned on the team's chat today:

If you check my PR is just the fix that I'm doing, remove that duplicity handling the CORS headers. I have searched on the code that header to avoid it and use the cors middleware instead

So I have prevented to have that issue on other parts of the code and ensuring that we have the middleware set on all these endpoints.