fix(cli): proxy support is broken #5803
Conversation
rix0rrr
requested review from
RomainMuller,
eladb,
iliapolo,
MrArnoldPalmer and
nija-at
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| httpOptions.proxy = options.proxyAddress; | ||
| if (proxyAddress && caBundlePath) { | ||
| throw new Error(`Cannot specify Proxy (${proxyAddress}) and CA Bundle (${caBundlePath}) at the same time`); | ||
| // Maybe it's possible after all, but I've been staring at |
There was a problem hiding this comment.
If I understand correctly, there is a use case for using a proxy and a CA bundle, no?
There was a problem hiding this comment.
Probably.
-
We didn't have CA certs before so not adding the feature is not a regression from that respect.
-
This PR is fixing the regression of proxy support working at all. Adding support for CAs while using the ProxyAgent can be a feature request that we deal with a later date. Have you seen the source code linked? I really don't want to add more risk by trying to figure out what is happening there that at this time.
There was a problem hiding this comment.
Fair enough. so maybe we can improve the error message to read:
At the moment, it is impossible to use a proxy with a custom CA bundle (see issue https://github...).
b56f159 to
5dfec17
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Behavior in #5791 is unchanged with this version. |
Proxy support was broken in the PR that introduced support for custom CA
bundles. Fix the support.
Fixes #5743, fixes #5791.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license