Skip to content

feat: add support for Caddy + Souin #112

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

Closed
wants to merge 1 commit into from

Conversation

JacquesDurand
Copy link

@JacquesDurand JacquesDurand commented Feb 10, 2023

Hi !
This is an attempt to answer #108 by adding support for the Caddy Web Server and Souin, its http-cache module.
As for now this is very much a work-in-progress. I did not manage to make it run locally yet, and any advice is definitely appreciated.
Edit: Workflow seems to run, "locally" and through docker.

To run it with a local caddy instance, you need to build a caddy server with the souin module, using xcaddy:

xcaddy build --with github.com/darkweak/souin/plugins/caddy

Then run the newly built caddy with the Caddyfile in the docker directory:

./caddy run --config docker/caddy/Caddyfile

And you can test it ( with the npm server running: npm run server )

npm run --silent cli --base="http://localhost:8006" > "results/caddy.json"

@JacquesDurand JacquesDurand marked this pull request as ready for review February 13, 2023 08:55
@mnot
Copy link
Collaborator

mnot commented Feb 16, 2023

Hello! Thanks for the contribution.

It looks like the tests are running, but Caddy is interfering; if you hover over the first blue question mark circle in the caddy column, it says: "PUT config resulted in 200 OK", which indicates that the config response status code is being changed (it should be a 201 Created).

You can dig into this by running the test on its own:

./test-docker.sh -i freshness-none caddy

... which should show you a detailed trace of the requests and responses, both from the client and server's standpoints.

I suspect what's happening is that Caddy is somehow defaulting to blocking or not properly handling PUT requests when being used as a reverse proxy.

@JacquesDurand
Copy link
Author

Hi ! Sorry, I have been a bit busy. I did see indeed that some tests were not working as intended, I'll try to look into it asap :)
@darkweak, pinging you just in case you would like to have a look at some point :)

@JacquesDurand
Copy link
Author

JacquesDurand commented Mar 2, 2023

Commenting again just to confirm that it must be an issue(?) with Caddy transforming the response status code from 201 to 200 (while keeping the response body as is). It seems to also apply for other HTTP methods. For instance, trying to call

GET http://localhost:8006/config/SOME_UUID

(with 8006 being caddy port, proxying to Node on 8000), should return a 405 (cf handle-config.mjs), but also returns a 200.

I'll try to setup a simple reproducer to catch what might be wrong (or badly setup from my part)

PS: I read about no-op requests for Caddy (https://caddy.community/t/why-caddy-emits-empty-200-ok-responses-by-default/17634), but it does not seem to be our case here

@darkweak
Copy link
Contributor

darkweak commented Mar 2, 2023

I will check that too @JacquesDurand

@darkweak
Copy link
Contributor

darkweak commented Mar 3, 2023

@mnot I don't understand why it keeps getting gray circle for some cateogries (e.g. Cache-Control freshness category) but works for others.
Capture d’écran 2023-03-03 à 12 14 05

Edit: I didn't read the icon description at the bottom.

@mnot
Copy link
Collaborator

mnot commented Jul 12, 2023

I've got it working with caddy + cache-handler; see the notes.

@mnot mnot closed this Jul 12, 2023
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.

3 participants