Skip to content

fix(caddy): remove extra path in the admin caddy routes#632

Merged
darkweak merged 6 commits intomasterfrom
fix/caddy/remove-extra-path-in-the-admin-caddy-routes
May 30, 2025
Merged

fix(caddy): remove extra path in the admin caddy routes#632
darkweak merged 6 commits intomasterfrom
fix/caddy/remove-extra-path-in-the-admin-caddy-routes

Conversation

@darkweak
Copy link
Owner

Closes #631

@darkweak darkweak self-assigned this May 23, 2025
@netlify
Copy link

netlify bot commented May 23, 2025

Deploy Preview for teal-sprinkles-4c7f14 canceled.

Name Link
🔨 Latest commit 238ae6b
🔍 Latest deploy log https://app.netlify.com/projects/teal-sprinkles-4c7f14/deploys/6839e2bb6402a100087568a4

@darkweak darkweak force-pushed the fix/caddy/remove-extra-path-in-the-admin-caddy-routes branch 3 times, most recently from 2fca624 to 206f709 Compare May 30, 2025 11:40
@darkweak darkweak force-pushed the fix/caddy/remove-extra-path-in-the-admin-caddy-routes branch from 206f709 to e334d4e Compare May 30, 2025 15:05
@darkweak darkweak force-pushed the fix/caddy/remove-extra-path-in-the-admin-caddy-routes branch from c4ad836 to 238ae6b Compare May 30, 2025 16:54
@github-actions
Copy link

@darkweak darkweak merged commit 516ed8c into master May 30, 2025
21 checks passed
return []caddy.AdminRoute{
{
Pattern: basepath + "/{params...}",
Pattern: "/{params...}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely cause issues with other modules that add to the admin API. It's not namespaced, so the souin API endpoint ends up being a catch-all for all requests. Running this config:

{
	cache {
		api {
			debug
			prometheus
			souin
		}
	}
	debug
}
localhost {
	route {
		cache
		reverse_proxy localhost:8080
	}
}

http://localhost:8080 {
	respond "hi"
}

I get this response for API requests unrelated to souin:

$ curl http://localhost:2019/test/
{"error":"resource not found: /test/"}

Which is coming from here:

return caddy.APIError{
HTTPStatus: http.StatusNotFound,
Err: fmt.Errorf("resource not found: %v", request.URL.Path),
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darkweak, I think this is critical because it may be an unintended side-effect

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.

basepath customization not possible

2 participants