-
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(aws-eks): support http proxy in EKS onEvent lambda #16657
Conversation
@otaviomacedo Would you be able to help me get this released once the CI passes? I found the release docs state to run |
CI is failing but this passes locally. Any ideas? Will continue to investigate on my end. Failure: @aws-cdk/aws-eks: Bundling asset Stack/@aws-cdk--aws-eks.ClusterResourceProvider/OnEventHandler/Code/Stage...
@aws-cdk/aws-eks: FAIL test/service-account.test.js (15.937 s)
@aws-cdk/aws-eks: � service account › add Service Account › defaults should have default namespace and lowercase unique id
@aws-cdk/aws-eks: Failed to bundle asset Stack/@aws-cdk--aws-eks.ClusterResourceProvider/OnEventHandler/Code/Stage, bundle output is located at /tmp/cdk.outzgYgUF/bundling-temp-4d81264e075d2e80c50708b10b07860eeab9a1dfc587b7e5c4cc63407f6728bf-error: Error: bash exited with status 1
@aws-cdk/aws-eks: 396 |
@aws-cdk/aws-eks: 397 | /**
@aws-cdk/aws-eks: > 398 | * Determine the directory where we're going to write the bundling output
@aws-cdk/aws-eks: | ^
@aws-cdk/aws-eks: 399 | *
@aws-cdk/aws-eks: 400 | * This is the target directory where we're going to write the staged output
@aws-cdk/aws-eks: 401 | * files if we can (if the hash is fully known), or a temporary directory
@aws-cdk/aws-eks: at AssetStaging.bundle (../core/lib/asset-staging.ts:398:13)
@aws-cdk/aws-eks: at AssetStaging.stageByBundling (../core/lib/asset-staging.ts:246:10)
@aws-cdk/aws-eks: at stageThisAsset (../core/lib/asset-staging.ts:137:35)
@aws-cdk/aws-eks: at Cache.obtain (../core/lib/private/cache.ts:24:13)
@aws-cdk/aws-eks: at new AssetStaging (../core/lib/asset-staging.ts:162:44)
@aws-cdk/aws-eks: at new Asset (../aws-s3-assets/lib/asset.ts:68:21)
@aws-cdk/aws-eks: at AssetCode.bind (../aws-lambda/lib/code.ts:183:20)
@aws-cdk/aws-eks: at new Function (../aws-lambda/lib/function.ts:331:29)
@aws-cdk/aws-eks: at new NodejsFunction (../aws-lambda-nodejs/lib/function.ts:51:5)
@aws-cdk/aws-eks: at new ClusterResourceProvider (lib/cluster-resource-provider.ts:63:21) |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
I'm a little concerned that the CI all of a sudden passed without an explicit change. Now it looks like it failed when mergify updated the branch with master. It seems to be a bundling error. I was able to reproduce the error locally by removing I also noticed that
If we don’t support bundling |
…ved use of `NodeJsFunction` in `aws-eks` package
In this last commit I implemented a new package This layer can be reused in other modules if needed. If there are any other custom resources that implement a Node Lambda which happens to use something like the I needed this layer so that I could have the I also copied the way we allow a user to override layers for the kuberctl lambdas using the optional |
…dk` and `aws-cdk-lib`
… is available in the lambda now
I'm deploying an EKS cluster using the locally built CDK packages and manually checking if the provided proxy is in fact being used. |
@otaviomacedo, re-requested your review since I made a major refactor of my fix. |
@Mergifyio refresh |
Command
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ll cluster handler functions (#17200) ## Summary This PR is intended for CDK EKS users who require all traffic to be routed through a proxy. Currently if a user does not allow internet connections to the VPC without going through a proxy, then deploying an EKS cluster will result in a timeout error: ```sh Received response status [FAILED] from custom resource. Message returned: Error: 2021-10-20T14:20:47.028Z d86e3ef4-45ce-4130-988f-c4663f7f8c80 Task timed out after 60.06 seconds ``` Fixes: #12469, SIM D29159517 Related to but does not resolve: `https://github.com/aws/aws-cdk/issues/12171` ## ⚙️ Changes _Expand each list item for additional details._ <details> <summary><strong>Corrected "Cluster Handler" docs to clarify that 2 lambdas are created (<code>onEventHandler</code>, <code>isCompleteHandler</code>)</strong></summary> <br /> Our docs [currently describe the "Cluster Handler" as one Lambda function that interacts with the EKS API](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler). However this is not accurate. The "Cluster Handler" actually creates [two Lambdas](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-provider.ts#L69-L96) for the Custom Resource, `onEventHandler` and `isCompleteHandler`, both interact with the AWS API. </details> <details> <summary><strong>Passes the <code>clusterHandlerEnvironment</code> to both Cluster Handler Lambdas</strong></summary> <br /> The `clusterHandlerEnvironment` is the [recommended method](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler) of passing a proxy url (i.g. `http_proxy: 'http://my-proxy.com:3128'`) to the Cluster Handler. Currently the `clusterHandlerEnvironment` is only passed to the Cluster Handler's `onEventHandler` Lambda. [The `onEventHandler` was believed to be the only Cluster Handler Lambda that interacts with the AWS EKS API](#12469 (comment)), however this is not entirely true. Both the `onEventHandler` and `isCompleteHandler` call the AWS EKS API. Following the execution process of `isCompleteHandler` when creating an EKS cluster: 1. [`index.isComplete()` (this is the Lambda handler)](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/index.ts#L48) 2. [`common.isComplete()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts#L59) 3. [`cluster.isCreateComplete()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L56) 4. [`cluster.isActive()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L196) 5. [Request to EKS API](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L198) (results in timeout because proxy is not used) This change allows the user to pass proxy urls as environment variables to **both** Lambdas using `clusterHandlerEnvironment`. </details> <details> <summary><strong>Renames the prop <code>onEventLayer</code> -> <code>proxyAgentLayer</code>, and provides the layer to both Cluster Handler Lambdas</strong></summary> <br /> The proxy-agent layer is now used in both `onEventHandler` and `isCompleteHandler` lambdas in order to support proxy configurations. Because of this change, i've deprecated the original `onEventLayer` and created a new prop `proxyAgentLayer` since we will now be passing this prop into more than just the `onEventHandler` Lambda. The `onEventLayer` prop was introduced [a few weeks ago (sept 24)](#16657) so it should not impact many users (if any). The prop would only be used if the user wishes to bundle the layer themselves with a custom proxy agent. This prop follows the [same user customization we allow with the kubectl handler](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#kubectllayer). Another suitable name for this prop could have been `clusterHandlerLayer` but I chose `proxyAgentLayer` because it represents **what** the layer is used for, instead of describing **where** it's used. This also follows the convention of the pre-existing [`kubectlLayer` prop](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#kubectllayer). </details> <details> <summary><strong>Adds the EKS cluster prop <code>clusterHandlerSecurityGroup</code></strong></summary> <br /> If a proxy address is provided to the Cluster Handler Lambdas, but the proxy instance is not open to the world, then the dynamic IPs of the Cluster Handler Lambdas will be denied access. To solve this, i've implemented a new Cluster prop `clusterHandlerSecurityGroup`. This `clusterHandlerSecurityGroup` prop will allow the user to pass a Security Group to both Lambda functions and the Custom Resource provider. This is very similar to how we [already allow users to pass Security Groups to the Kubectl Handler](https://github.com/aws/aws-cdk/blob/7f194000697b85deb410ae0d7f7d4ac3c2654bcc/packages/%40aws-cdk/aws-eks/lib/kubectl-provider.ts#L83) </details> ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
## Summary Currently when a user wants to route all of the EKS lambda's `aws-sdk-js` requests through a proxy then they are [instructed to configure an env var named `HTTP_PROXY` or `http_proxy`](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler). e.g. ```ts const cluster = new eks.Cluster(this, 'hello-eks', { version: eks.KubernetesVersion.V1_21, clusterHandlerEnvironment: { 'http_proxy': 'http://proxy.myproxy.com' } }); ``` However the JS SDK [requires further configuration to enable proxy support](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/node-configuring-proxies.html). This PR: **The below changes have been refactored to avoid use of `NodeJsFunction`. See the PR comments below for [reasoning](aws/aws-cdk#16657 (comment)) and [updated changes](aws/aws-cdk#16657 (comment) - ~~Adds a `package.json` with the dependency ['http-proxy-agent'](https://github.com/TooTallNate/node-http-proxy-agent) to the `cluster-resource-handler/` lambda bundle~~ - ~~Uses `NodeJSFunction` to install lambda dependencies and bundle.~~ - Adds a condition that checks the environment for `HTTP_PROXY` or `http_proxy` values. If present then configures the aws-sdk to use that proxy (using `http-proxy-agent`). ~~Note: I placed the `http-proxy-agent` in the `devDependencies` of `package.json`. If the dependency is placed in the `dependencies` section then the CDK builder [throws an error: `NPM Package cluster-resources-handler inside jsii package '@aws-cdk/aws-eks', can only have devDependencies`](https://github.com/aws/aws-cdk/blob/7dae114b7aac46321b8d8572e6837428b4c633b2/tools/pkglint/lib/rules.ts#L1332)~~ Fixes: SIM D29159517, aws/aws-cdk#12469 Tested this using squid proxy on an ec2 instance within the same VPC as the EKS cluster. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
## Summary Currently when a user wants to route all of the EKS lambda's `aws-sdk-js` requests through a proxy then they are [instructed to configure an env var named `HTTP_PROXY` or `http_proxy`](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler). e.g. ```ts const cluster = new eks.Cluster(this, 'hello-eks', { version: eks.KubernetesVersion.V1_21, clusterHandlerEnvironment: { 'http_proxy': 'http://proxy.myproxy.com' } }); ``` However the JS SDK [requires further configuration to enable proxy support](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/node-configuring-proxies.html). This PR: **The below changes have been refactored to avoid use of `NodeJsFunction`. See the PR comments below for [reasoning](aws#16657 (comment)) and [updated changes](aws#16657 (comment) - ~~Adds a `package.json` with the dependency ['http-proxy-agent'](https://github.com/TooTallNate/node-http-proxy-agent) to the `cluster-resource-handler/` lambda bundle~~ - ~~Uses `NodeJSFunction` to install lambda dependencies and bundle.~~ - Adds a condition that checks the environment for `HTTP_PROXY` or `http_proxy` values. If present then configures the aws-sdk to use that proxy (using `http-proxy-agent`). ~~Note: I placed the `http-proxy-agent` in the `devDependencies` of `package.json`. If the dependency is placed in the `dependencies` section then the CDK builder [throws an error: `NPM Package cluster-resources-handler inside jsii package '@aws-cdk/aws-eks', can only have devDependencies`](https://github.com/aws/aws-cdk/blob/7dae114b7aac46321b8d8572e6837428b4c633b2/tools/pkglint/lib/rules.ts#L1332)~~ Fixes: SIM D29159517, aws#12469 Tested this using squid proxy on an ec2 instance within the same VPC as the EKS cluster. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ll cluster handler functions (aws#17200) ## Summary This PR is intended for CDK EKS users who require all traffic to be routed through a proxy. Currently if a user does not allow internet connections to the VPC without going through a proxy, then deploying an EKS cluster will result in a timeout error: ```sh Received response status [FAILED] from custom resource. Message returned: Error: 2021-10-20T14:20:47.028Z d86e3ef4-45ce-4130-988f-c4663f7f8c80 Task timed out after 60.06 seconds ``` Fixes: aws#12469, SIM D29159517 Related to but does not resolve: `https://github.com/aws/aws-cdk/issues/12171` ## ⚙️ Changes _Expand each list item for additional details._ <details> <summary><strong>Corrected "Cluster Handler" docs to clarify that 2 lambdas are created (<code>onEventHandler</code>, <code>isCompleteHandler</code>)</strong></summary> <br /> Our docs [currently describe the "Cluster Handler" as one Lambda function that interacts with the EKS API](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler). However this is not accurate. The "Cluster Handler" actually creates [two Lambdas](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-provider.ts#L69-L96) for the Custom Resource, `onEventHandler` and `isCompleteHandler`, both interact with the AWS API. </details> <details> <summary><strong>Passes the <code>clusterHandlerEnvironment</code> to both Cluster Handler Lambdas</strong></summary> <br /> The `clusterHandlerEnvironment` is the [recommended method](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#cluster-handler) of passing a proxy url (i.g. `http_proxy: 'http://my-proxy.com:3128'`) to the Cluster Handler. Currently the `clusterHandlerEnvironment` is only passed to the Cluster Handler's `onEventHandler` Lambda. [The `onEventHandler` was believed to be the only Cluster Handler Lambda that interacts with the AWS EKS API](aws#12469 (comment)), however this is not entirely true. Both the `onEventHandler` and `isCompleteHandler` call the AWS EKS API. Following the execution process of `isCompleteHandler` when creating an EKS cluster: 1. [`index.isComplete()` (this is the Lambda handler)](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/index.ts#L48) 2. [`common.isComplete()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts#L59) 3. [`cluster.isCreateComplete()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L56) 4. [`cluster.isActive()`](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L196) 5. [Request to EKS API](https://github.com/aws/aws-cdk/blob/0cabb9f2d2f50c03337cd6f35bf47fc54ada3a21/packages/%40aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts#L198) (results in timeout because proxy is not used) This change allows the user to pass proxy urls as environment variables to **both** Lambdas using `clusterHandlerEnvironment`. </details> <details> <summary><strong>Renames the prop <code>onEventLayer</code> -> <code>proxyAgentLayer</code>, and provides the layer to both Cluster Handler Lambdas</strong></summary> <br /> The proxy-agent layer is now used in both `onEventHandler` and `isCompleteHandler` lambdas in order to support proxy configurations. Because of this change, i've deprecated the original `onEventLayer` and created a new prop `proxyAgentLayer` since we will now be passing this prop into more than just the `onEventHandler` Lambda. The `onEventLayer` prop was introduced [a few weeks ago (sept 24)](aws#16657) so it should not impact many users (if any). The prop would only be used if the user wishes to bundle the layer themselves with a custom proxy agent. This prop follows the [same user customization we allow with the kubectl handler](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#kubectllayer). Another suitable name for this prop could have been `clusterHandlerLayer` but I chose `proxyAgentLayer` because it represents **what** the layer is used for, instead of describing **where** it's used. This also follows the convention of the pre-existing [`kubectlLayer` prop](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#kubectllayer). </details> <details> <summary><strong>Adds the EKS cluster prop <code>clusterHandlerSecurityGroup</code></strong></summary> <br /> If a proxy address is provided to the Cluster Handler Lambdas, but the proxy instance is not open to the world, then the dynamic IPs of the Cluster Handler Lambdas will be denied access. To solve this, i've implemented a new Cluster prop `clusterHandlerSecurityGroup`. This `clusterHandlerSecurityGroup` prop will allow the user to pass a Security Group to both Lambda functions and the Custom Resource provider. This is very similar to how we [already allow users to pass Security Groups to the Kubectl Handler](https://github.com/aws/aws-cdk/blob/7f194000697b85deb410ae0d7f7d4ac3c2654bcc/packages/%40aws-cdk/aws-eks/lib/kubectl-provider.ts#L83) </details> ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Summary
Currently when a user wants to route all of the EKS lambda's
aws-sdk-js
requests through a proxy then they are instructed to configure an env var namedHTTP_PROXY
orhttp_proxy
.e.g.
However the JS SDK requires further configuration to enable proxy support.
This PR:
The below changes have been refactored to avoid use of
NodeJsFunction
. See the PR comments below for reasoning and updated changes.Adds apackage.json
with the dependency 'http-proxy-agent' to thecluster-resource-handler/
lambda bundleUsesNodeJSFunction
to install lambda dependencies and bundle.HTTP_PROXY
orhttp_proxy
values. If present then configures the aws-sdk to use that proxy (usinghttp-proxy-agent
).Note: I placed thehttp-proxy-agent
in thedevDependencies
ofpackage.json
. If the dependency is placed in thedependencies
section then the CDK builder throws an error:NPM Package cluster-resources-handler inside jsii package '@aws-cdk/aws-eks', can only have devDependencies
Fixes: SIM D29159517, #12469
Tested this using squid proxy on an ec2 instance within the same VPC as the EKS cluster.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license