caddy icon indicating copy to clipboard operation
caddy copied to clipboard

feat: Multiple 'to' upstreams in reverse-proxy cmd

Open parrotmac opened this issue 3 years ago • 7 comments

From https://github.com/caddyserver/caddy/issues/4692

This is a proposal to add the ability to specify multiple upstream/to targets to the reverse-proxy command.

Example usage:

caddy reverse-proxy --from :8080 --to host1.example.com,host2.example.com

What do we think of the implementation? Any suggestions for a different interface?

parrotmac avatar Apr 10 '22 05:04 parrotmac

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 10 '22 05:04 CLAassistant

Thanks for the contribution!

I think it is more typical to repeat a flag than to have custom parsing within a flag's value, i.e. --to host1 --to host2 instead of --to host1,host2. This would probably be my preference, anyway.

That said, I'm not entirely sure how nicely our flags API handles that at the moment. What do you think?

mholt avatar Apr 11 '22 18:04 mholt

@mholt is there even a way to repeat flags with Go's built-in stuff? We probably can with cobra #4565 but I don't think we can as-is? I don't see a fl.Array() or fl.Slice() etc, cause it wouldn't be type safe :thinking:

francislavoie avatar Apr 11 '22 19:04 francislavoie

@francislavoie Yeah, I had to look this up but I knew I did it once before, years ago. We would need to define our own type: https://stackoverflow.com/questions/45487377/golang-flag-ignore-missing-flag-and-parse-multiple-duplicate-flags

Which, I think maybe that's suitable for this. It's just a few lines and very simple. I do think the CLI looks cleaner this way, personally; but maybe that's just my preference. :man_shrugging:

mholt avatar Apr 11 '22 19:04 mholt

Ah ok, that's not so bad. Yeah I suppose we should do it that way then :+1:

francislavoie avatar Apr 11 '22 19:04 francislavoie

I think it is more typical to repeat a flag than to have custom parsing within a flag's value, i.e. --to host1 --to host2 instead of --to host1,host2. This would probably be my preference, anyway.

That said, I'm not entirely sure how nicely our flags API handles that at the moment. What do you think?

~Yeah, I actually quite prefer the multiple flags form but wasn't sure if it was worth going that route. Here's an example of how that could be implemented. If that's preferred I'm happy to switch to something like that.~ Edit ha! Looks like y'all beat me to exactly this same thing! 😂

parrotmac avatar Apr 11 '22 19:04 parrotmac

@parrotmac So, did we decide to use multiple --to or to stick with comma separated values? Honestly I'm probably OK with either, I just think the multiple flag form will be clearer.

mholt avatar Jul 06 '22 20:07 mholt