-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix: CDK does not honor NO_PROXY settings #16751
Conversation
CDK was extracting the value of `HTTPS?_PROXY` and passing this to `proxy-agent` explicitly, which resulted in not honoring the `NO_PROXY` setting. This removes that behavior and lets `proxy-agent` delegate to `proxy-from-env`, which will leverage values in `HTTPS?_PROXY` and NO_PROXY correctly. Fixes #7121
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
// request. | ||
// | ||
// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies | ||
const ProxyAgent: any = require('proxy-agent'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this change is causing EKS deployments to fail. CloudFormation now throws Internal Failure.
when trying to create.
This handler is used in the "onComplete" Lambda function which does not have the proxy-agent
available. We only pass an http_proxy
env var into the "onEvent" Lambda since that is the only Lambda that makes requests with the aws-sdk-js
.
I tested this by moving the proxy logic to the onEvent()
method and was able to successfully deploy again. I'll make a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed that new ProxyAgent()
does not automatically detect process.env.http_proxy
or process.env.HTTP_PROXY
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's what ProxyAgent looks like when you log it in the Lambda:
Code:
const agent = new ProxyAgent()
console.log(`agent: ${JSON.stringify(agent, null, 2)}`);
Logs:
2021-10-01T22:32:09.812Z 03de5aeb-e937-4b3d-91a3-aee2c524cb47 INFO agent: {}
Code:
const agent = new ProxyAgent(process.env.http_agent);
console.log(`agent: ${JSON.stringify(agent, null, 2)}`);
Logs:
2021-10-01T22:30:23.701Z 0ffc17d9-134f-498a-a8d9-dcfbe0754ae5 INFO agent: {
"proxy": {
"protocol": "http:",
"slashes": true,
"auth": "user1:user1",
"host": "184.73.16.134",
"port": null,
"hostname": "184.73.16.134",
"hash": null,
"search": null,
"query": null,
"pathname": "/",
"path": "/",
"href": "http://user1:[email protected]/"
},
"proxyUri": "http://user1:[email protected]"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested using https://github.com/ryparker/aws-cdk-sample-eks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's normal and expected that the ProxyAgent
instance does not look the same. If provided with a proxy, it will use ALWAYS and ONLY that proxy. When not specified, it will delegate to proxy-from-env
, which will look at the correct proxy environment variable for the request' protocol & honor NO_PROXY.
I'd argue that the broken Lambda function should receive an HTTPS_PROXY
and not HTTP_PROXY
, as the AWS SDK typically makes only TSL connections... So it should be expected NOT to use HTTP_PROXY
(using that value is incorrect there and should be considered a bug).
…6761) ## Summary This [commit](ceab036) broke EKS deployments. CloudFormation throws "Internal failure." when attempting to create an EKS cluster. Full details : https://github.com/aws/aws-cdk/pull/16751/files#r720549975 This reverts commit ceab036. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
* '15588' of https://github.com/xykkong/aws-cdk: (47 commits) chore: rollback `GenericSSMParameterImage` deprecation (backport aws#16798) (aws#16800) chore(deps): bump actions/setup-node from 2.4.0 to 2.4.1 (aws#16778) Update CHANGELOG.md chore(release): 1.126.0 feat(assertions): matcher support for `templateMatches()` API (aws#16789) feat(stepfunctions-tasks): add step concurrency level to EmrCreateCluster (aws#15242) docs(s3): correct heading levels Object Ownership / Bucket deletion (aws#16790) chore(individual-pkg-gen): fix bug in setting alpha package visibility (aws#16787) fix(s3): setting `autoDeleteObjects` to `false` empties the bucket (aws#16756) fix(iam): `User.fromUserArn` does not work for ARNs that include a path (aws#16269) fix(cli): progress bar overshoots count by 1 for stack updates (aws#16168) fix(config): add SourceAccount condition to Lambda permission (aws#16617) docs(events): add a note about not using `EventPattern` with `CfnRule` (aws#16715) docs(core): fix reference to nonexistant enum value (aws#16716) chore(s3-deployments): update python version on BucketDeployment handler (aws#16771) chore: set response-requested length to 2 and closing-soon to 5 (aws#16763) fix(revert): "fix: CDK does not honor NO_PROXY settings (aws#16751)" (aws#16761) docs(GitHub issue templates): Upgrade to GitHub Issues v2 (aws#16592) chore: reset jsii-rosetta worker count to default (aws#16755) fix: CDK does not honor NO_PROXY settings (aws#16751) ...
## Summary CDK was extracting the value of HTTPS?_PROXY and passing this to proxy-agent explicitly, which resulted in not honoring the NO_PROXY setting. This removes that behavior and lets proxy-agent delegate to proxy-from-env, which will leverage values in HTTPS?_PROXY and NO_PROXY correctly. Tested by deploying [this sample repo](https://github.com/ryparker/aws-cdk-sample-eks) and monitoring Squid proxy logs while triggering the "onEvent" Lambda. Fixes #7121 Related PRs: #16751, #16751 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
CDK was extracting the value of `HTTPS?_PROXY` and passing this to `proxy-agent` explicitly, which resulted in not honoring the `NO_PROXY` setting. This removes that behavior and lets `proxy-agent` delegate to `proxy-from-env`, which will leverage values in `HTTPS?_PROXY` and NO_PROXY correctly. Fixes #7121
…6761) ## Summary This [commit](ceab036) broke EKS deployments. CloudFormation throws "Internal failure." when attempting to create an EKS cluster. Full details : https://github.com/aws/aws-cdk/pull/16751/files#r720549975 This reverts commit ceab036. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
## Summary CDK was extracting the value of HTTPS?_PROXY and passing this to proxy-agent explicitly, which resulted in not honoring the NO_PROXY setting. This removes that behavior and lets proxy-agent delegate to proxy-from-env, which will leverage values in HTTPS?_PROXY and NO_PROXY correctly. Tested by deploying [this sample repo](https://github.com/ryparker/aws-cdk-sample-eks) and monitoring Squid proxy logs while triggering the "onEvent" Lambda. Fixes #7121 Related PRs: #16751, #16751 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
CDK was extracting the value of `HTTPS?_PROXY` and passing this to `proxy-agent` explicitly, which resulted in not honoring the `NO_PROXY` setting. This removes that behavior and lets `proxy-agent` delegate to `proxy-from-env`, which will leverage values in `HTTPS?_PROXY` and NO_PROXY correctly. Fixes aws#7121
…ws#16761) ## Summary This [commit](aws@ceab036) broke EKS deployments. CloudFormation throws "Internal failure." when attempting to create an EKS cluster. Full details : https://github.com/aws/aws-cdk/pull/16751/files#r720549975 This reverts commit ceab036. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
## Summary CDK was extracting the value of HTTPS?_PROXY and passing this to proxy-agent explicitly, which resulted in not honoring the NO_PROXY setting. This removes that behavior and lets proxy-agent delegate to proxy-from-env, which will leverage values in HTTPS?_PROXY and NO_PROXY correctly. Tested by deploying [this sample repo](https://github.com/ryparker/aws-cdk-sample-eks) and monitoring Squid proxy logs while triggering the "onEvent" Lambda. Fixes aws#7121 Related PRs: aws#16751, aws#16751 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
CDK was extracting the value of
HTTPS?_PROXY
and passing this toproxy-agent
explicitly, which resulted in not honoring theNO_PROXY
setting.
This removes that behavior and lets
proxy-agent
delegate toproxy-from-env
, which will leverage values inHTTPS?_PROXY
andNO_PROXY correctly.
Fixes #7121
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license