Support host-specific proxies with proxy config YAML #837
Conversation
… and publicHostsFile mappings
…e urls reaching proxy exit status: return exit code 21 when recorder detects proxy failure codes from the browser
ikreymer
force-pushed
the
proxy-pac-support
branch
from
69773ea to
d688041
Compare
| @@ -641,6 +642,12 @@ class ArgParser { | |||
| type: "string", | |||
| }, | |||
|
|
|||
| proxyServerConfig: { | |||
| describe: | |||
| "if set, path to yaml/json file that configures multiple path servers per URL regex", | |||
There was a problem hiding this comment.
Same question about JSON support as above
| @@ -123,6 +123,12 @@ function initArgs() { | |||
| type: "string", | |||
| }, | |||
|
|
|||
| proxyServerConfig: { | |||
| describe: | |||
| "if set, path to yaml/json file that configures multiple path servers per URL regex", | |||
There was a problem hiding this comment.
Does this actually support JSON? From docs and code I'm only seeing YAML support
There was a problem hiding this comment.
YAML is supposed to be a superset of JSON, as of 1.2 at least! (Although there are some edge-cases, but not sure we need to worry about, according to: https://news.ycombinator.com/item?id=30052633)
We could attempt to parse as JSON if YAML parsing fails.
FWIW, the main config is also parsed via a YAML parser but is stored as JSON in Browsertrix - we have not had any issues with it.
| if (!params.proxyMap?.matchHosts || !params.proxyMap?.proxies) { | ||
| if (defaultProxyEntry) { | ||
| logger.debug("Using Single Proxy", {}, "proxy"); | ||
| } | ||
| return { proxyServer: defaultProxyEntry?.proxyUrl }; | ||
| } |
There was a problem hiding this comment.
Maybe I'm reading this wrong, but it seems like the single proxyServer will only be returned here if host-specific proxies aren't also passed? If we're giving precedence to single proxy servers, maybe the wider if clause here isn't necessary?
There was a problem hiding this comment.
Ah good catch, I think I updated this later to only override the default proxy (with no host configured), so the other configs still take precedence. I can see a use case for both, I suppose could add another option for this..
There was a problem hiding this comment.
Yeah, maybe another option would be good. We have clients who require all traffic to come from a specific IP or country, so I think we do need to have an option to make sure that's the case, even if we want to by default have the host-specific proxies override.
Co-authored-by: Tessa Walsh <[email protected]>
Fixes #836