Skip to content

Commit

Permalink
revert(aws-eks): "fix(aws-eks): Support for http proxy in EKS onEvent…
Browse files Browse the repository at this point in the history
… 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*
  • Loading branch information
ryparker authored Sep 24, 2021
1 parent 3ff1537 commit 376c837
Show file tree
Hide file tree
Showing 14 changed files with 264 additions and 401 deletions.
5 changes: 1 addition & 4 deletions packages/@aws-cdk/aws-eks/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,4 @@ tsconfig.json
junit.xml
test/
!*.lit.ts
jest.config.js

# Don't include lambda node_modules. These are installed at build time.
lib/cluster-resource-handler/node_modules
jest.config.js
Empty file.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable no-console */

// eslint-disable-next-line import/no-extraneous-dependencies
import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';
Expand Down
24 changes: 0 additions & 24 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';

// eslint-disable-next-line import/no-extraneous-dependencies
Expand Down Expand Up @@ -38,16 +37,6 @@ export abstract class ResourceHandler {
RoleArn: roleToAssume,
RoleSessionName: `AWSCDK.EKSCluster.${this.requestType}.${this.requestId}`,
});

const proxyAddress = this.httpProxyFromEnvironment();
if (proxyAddress) {
this.log(`Using proxy server: ${proxyAddress}`);
// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies
const ProxyAgent: any = require('proxy-agent');
aws.config.update({
httpOptions: { agent: new ProxyAgent(proxyAddress) },
});
}
}

public onEvent() {
Expand Down Expand Up @@ -75,19 +64,6 @@ export abstract class ResourceHandler {
console.log(JSON.stringify(x, undefined, 2));
}

/**
* Find and return the configured HTTP proxy address
*/
private httpProxyFromEnvironment(): string | undefined {
if (process.env.http_proxy) {
return process.env.http_proxy;
}
if (process.env.HTTP_PROXY) {
return process.env.HTTP_PROXY;
}
return undefined;
}

protected abstract async onCreate(): Promise<OnEventResponse>;
protected abstract async onDelete(): Promise<OnEventResponse | void>;
protected abstract async onUpdate(): Promise<(OnEventResponse & EksUpdateId) | void>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
/* eslint-disable no-console */

// eslint-disable-next-line import/no-extraneous-dependencies
import { IsCompleteResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';

import { ClusterResourceHandler } from './cluster';
import { EksClient } from './common';
import * as consts from './consts';
Expand Down

This file was deleted.

10 changes: 4 additions & 6 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as path from 'path';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { NodejsFunction } from '@aws-cdk/aws-lambda-nodejs';
import { Duration, NestedStack, Stack } from '@aws-cdk/core';
import * as cr from '@aws-cdk/custom-resources';
import { Construct } from 'constructs';
Expand Down Expand Up @@ -59,13 +58,12 @@ export class ClusterResourceProvider extends NestedStack {
private constructor(scope: Construct, id: string, props: ClusterResourceProviderProps) {
super(scope as CoreConstruct, id);

// Using NodejsFunction so that NPM dependencies (proxy-agent) are installed at synth time.
const onEvent = new NodejsFunction(this, 'OnEventHandler', {
entry: path.join(HANDLER_DIR, 'index.ts'),
const onEvent = new lambda.Function(this, 'OnEventHandler', {
code: lambda.Code.fromAsset(HANDLER_DIR),
description: 'onEvent handler for EKS cluster resource provider',
runtime: HANDLER_RUNTIME,
environment: props.environment,
handler: 'onEvent',
handler: 'index.onEvent',
timeout: Duration.minutes(1),
vpc: props.subnets ? props.vpc : undefined,
vpcSubnets: props.subnets ? { subnets: props.subnets } : undefined,
Expand Down Expand Up @@ -98,4 +96,4 @@ export class ClusterResourceProvider extends NestedStack {
* The custom resource service token for this provider.
*/
public get serviceToken() { return this.provider.serviceToken; }
}
}
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-eks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-lambda-nodejs": "0.0.0",
"@aws-cdk/aws-ssm": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/custom-resources": "0.0.0",
Expand All @@ -117,8 +116,7 @@
"@aws-cdk/custom-resources": "0.0.0",
"constructs": "^3.3.69",
"@aws-cdk/lambda-layer-awscli": "0.0.0",
"@aws-cdk/lambda-layer-kubectl": "0.0.0",
"@aws-cdk/aws-lambda-nodejs": "0.0.0"
"@aws-cdk/lambda-layer-kubectl": "0.0.0"
},
"engines": {
"node": ">= 10.13.0 <13 || >=13.7.0"
Expand Down
21 changes: 9 additions & 12 deletions packages/@aws-cdk/aws-eks/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,9 @@ describe('cluster', () => {
const nested = stack.node.tryFindChild('@aws-cdk/aws-eks.ClusterResourceProvider') as cdk.NestedStack;

const template = SynthUtils.toCloudFormation(nested);
expect(template.Resources.OnEventHandler42BEBAE0.Properties.Environment).toEqual({
Variables: {
AWS_NODEJS_CONNECTION_REUSE_ENABLED: '1',
foo: 'bar',
},
});
expect(template.Resources.OnEventHandler42BEBAE0.Properties.Environment).toEqual({ Variables: { foo: 'bar' } });


});

test('throws when trying to place cluster handlers in a vpc with no private subnets', () => {
Expand Down Expand Up @@ -654,7 +651,7 @@ describe('cluster', () => {
const { stack } = testFixtureNoVpc();

// WHEN
new eks.Cluster(stack, 'cluster', { version: CLUSTER_VERSION, prune: false });
new eks.Cluster(stack, 'cluster', { version: CLUSTER_VERSION, prune: false }) ;

// THEN
expect(stack).toHaveResource('AWS::EC2::VPC');
Expand Down Expand Up @@ -2472,7 +2469,7 @@ describe('cluster', () => {
version: CLUSTER_VERSION,
prune: false,
endpointAccess:
eks.EndpointAccess.PRIVATE,
eks.EndpointAccess.PRIVATE,
vpcSubnets: [{
subnets: [ec2.PrivateSubnet.fromSubnetAttributes(stack, 'Private1', {
subnetId: 'subnet1',
Expand Down Expand Up @@ -2571,14 +2568,14 @@ describe('cluster', () => {
const subnetConfiguration: ec2.SubnetConfiguration[] = [];

for (let i = 0; i < 20; i++) {
subnetConfiguration.push({
subnetConfiguration.push( {
subnetType: ec2.SubnetType.PRIVATE,
name: `Private${i}`,
},
);
}

subnetConfiguration.push({
subnetConfiguration.push( {
subnetType: ec2.SubnetType.PUBLIC,
name: 'Public1',
});
Expand Down Expand Up @@ -2622,14 +2619,14 @@ describe('cluster', () => {
const subnetConfiguration: ec2.SubnetConfiguration[] = [];

for (let i = 0; i < 20; i++) {
subnetConfiguration.push({
subnetConfiguration.push( {
subnetType: ec2.SubnetType.PRIVATE,
name: `Private${i}`,
},
);
}

subnetConfiguration.push({
subnetConfiguration.push( {
subnetType: ec2.SubnetType.PUBLIC,
name: 'Public1',
});
Expand Down
Loading

0 comments on commit 376c837

Please sign in to comment.