Skip to content
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

caddyfile: add handle_response blocks to reverse_proxy #3707

Closed
maxatome opened this issue Sep 2, 2020 · 4 comments
Closed

caddyfile: add handle_response blocks to reverse_proxy #3707

maxatome opened this issue Sep 2, 2020 · 4 comments
Labels
feature ⚙️ New feature or request

Comments

@maxatome
Copy link
Contributor

maxatome commented Sep 2, 2020

Caddyfile support for the handle_response option of reverse_proxy would be nice.

See the JSON configuration here:

https://caddyserver.com/docs/json/apps/http/servers/errors/routes/handle/reverse_proxy/handle_response/

Maybe it could look something like this in the Caddyfile:

reverse_proxy ... {
	...
	handle_response header X-Accel-Redirect {
		... 
	}
	handle_response status 4xx {
		...
	}
	handle_response {
		...
	}
}

(Responses can be matched by headers or status code. Just showing a few examples here.)

Originally posted by @mholt in #3347 (comment)

@maxatome
Copy link
Contributor Author

maxatome commented Sep 2, 2020

@mholt do you have an idea of how to represent "routes" list in handle_response block?

handle_response [header <field>|status <status>] {
    status <status>
    routes?
}

I didn't find an other place where it is UnmarshalCaddyfiled so I prefer to ask before doing it the wrong way...

@francislavoie francislavoie added the feature ⚙️ New feature or request label Sep 2, 2020
@francislavoie
Copy link
Member

francislavoie commented Sep 2, 2020

I think it's tricky to have status <status> inside of the block, because it's not a real handler directive (but there are ways around this, which I'll explain below).

But for "routes", take a look at how the handle directive is parsed, i.e. parseHandle -> ParseSegmentAsSubroute:

// ParseSegmentAsSubroute parses the segment such that its subdirectives
// are themselves treated as directives, from which a subroute is built
// and returned.
func ParseSegmentAsSubroute(h Helper) (caddyhttp.MiddlewareHandler, error) {
allResults, err := parseSegmentAsConfig(h)
if err != nil {
return nil, err
}
return buildSubroute(allResults, h.groupCounter)
}

There are ways around what I said above about status, like reading and deleting tokens before passing them through to ParseSegmentAsSubroute. The php_fastcgi directive uses this approach to allow for its own options intermixed with reverse_proxy directive options, while consuming the php_fastcgi options, and passing through the rest to reverse_proxy. You can get an idea of how that looks here:

// read the subdirectives that we allow as overrides to
// the php_fastcgi shortcut
// NOTE: we delete the tokens as we go so that the reverse_proxy
// unmarshal doesn't see these subdirectives which it cannot handle
for dispenser.Next() {
for dispenser.NextBlock(0) {
switch dispenser.Val() {
case "root":
if !dispenser.NextArg() {
return nil, dispenser.ArgErr()
}
fcgiTransport.Root = dispenser.Val()
dispenser.Delete()
dispenser.Delete()
case "split":
extensions = dispenser.RemainingArgs()
dispenser.Delete()
for range extensions {
dispenser.Delete()
}
if len(extensions) == 0 {
return nil, dispenser.ArgErr()
}

That approach is super tedious and finnicky though, so I don't super recommend it. But it could work if handle_response must support other options than the ones that exist in other directives.

@maxatome
Copy link
Contributor Author

maxatome commented Sep 6, 2020

Thanks @francislavoie, what do you think of #3712?
I am not sure I did it the right way, so tell me...

@mholt
Copy link
Member

mholt commented May 2, 2021

Done in #4021.

@mholt mholt closed this as completed May 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants