Skip to content
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 for http proxy in EKS onEvent lambda #16652

Closed
wants to merge 14 commits into from

Conversation

ryparker
Copy link
Contributor

Summary

Currently when a user wants to route all of the EKS lambda's SDK requests through a proxy then they are instructed to configure an env var named HTTP_PROXY or http_proxy.

e.g.

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.

This PR:

  • Adds a package.json with the dependency '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

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

@gitpod-io
Copy link

gitpod-io bot commented Sep 24, 2021

@ryparker ryparker added contribution/core This is a PR that came from AWS. @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service labels Sep 24, 2021
@ryparker ryparker self-assigned this Sep 24, 2021
@ryparker ryparker added the pr/do-not-merge This PR should not be merged at this time. label Sep 24, 2021
@ryparker
Copy link
Contributor Author

ryparker commented Sep 24, 2021

Blocking merge with pr/do-not-merge label. Once the revert is merged this can be merged after.

jogold and others added 4 commits September 24, 2021 15:06
…6597)

Add `vault.addToAccessPolicy()` and `vault.blockRecoveryPointDeletion()`.

A vault is automatically created when creating a plan:

```ts
const plan = new backup.BackupPlan(this, 'BackupPlan');
```

Theses methods allow, among other things, to customize the access policy of
the automatically created vault:

```ts
plan.backupVault.addToAccessPolicy(...);
```


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
… lambda" (#16651)

A bug was introduced in [this commit](cf22280) that broke the `onEvent` EKS cluster handler lambda. ESBuild was inlining the node_module `proxy-agent` which was trying to read a file that did not exist (because all dependencies were bundled into a single file).

e.g.
```ts
var contextify = fs.readFileSync('/var/task/contextify.js');
```

Error:
```log
ENOENT: no such file or directory, open '/var/task/contextify.js' Logs: /aws/lambda/test-fixed-nobundle-eks-wit-OnEventHandler42BEBAE0-s2cZwaWDW0xt at Object.openSync (fs.js:462:3) at Object.readFileSync (fs.js:364:35) at loadAndCompileScript (/var/task/index.js:29479:23) at ../aws-cdk/node_modules/vm2/lib/main.js (/var/task/index.js:29490:25) at __require (/var/task/index.js:26:44) at ../aws-cdk/node_modules/vm2/index.js (/var/task/index.js:30079:23) at __require (/var/task/index.js:26:44) at ../aws-cdk/node_modules/degenerator/dist/src/index.js (/var/task/index.js:30091:17) at __require (/var/task/index.js:26:44) at ../aws-cdk/node_modules/pac-resolver/dist/index.js (/var/task/index.js:30857:25) (RequestId: c44d1357-fbce-4f96-8c23-b865c2c3aaff)
```

This reverts commit cf22280.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
# Conflicts:
#	packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
#	packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/package.json
#	packages/@aws-cdk/aws-eks/lib/cluster-resource-provider.ts
@ryparker ryparker removed the pr/do-not-merge This PR should not be merged at this time. label Sep 24, 2021
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 961ffd1
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

ryparker added a commit that referenced this pull request Sep 25, 2021
@ryparker
Copy link
Contributor Author

The commits for this branch got messy after rebasing on the revert. Just to be safe I recreated this PR on a fresh branch off master: #16652

@ryparker ryparker closed this Sep 25, 2021
@rix0rrr rix0rrr deleted the fix-node-lambda-http-proxy branch July 4, 2022 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants