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): proxy support and allow assigning a security group to all cluster handler functions #17200

Merged
merged 22 commits into from Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0d87a93
Added layer to both cluster handler functions, renamed `onEventLayer`…
ryparker Oct 27, 2021
dcfb784
Corrected doc comments and reordered props to match
ryparker Oct 27, 2021
cd4578e
removed `@aws-cdk/aws-eks/proxy-agent` from root package.json
ryparker Oct 28, 2021
0e0da6f
revert remove yaml from root package.json
ryparker Oct 28, 2021
cb10cce
Merge branch 'master' into eks-cluster-handler-security-group-prop
ryparker Oct 28, 2021
4ab257a
Merge branch 'eks-cluster-handler-security-group-prop' of https://git…
ryparker Oct 28, 2021
d569176
Using require to import `proxy-agent`
ryparker Oct 28, 2021
5208526
docs: removed `onEvent` documentation
ryparker Oct 28, 2021
dbfe9a6
docs: removed incorrect code comment on onEvent
ryparker Oct 28, 2021
92a6c39
docs(aws-eks/README): replaced `http_proxy` -> `https_proxy`
ryparker Nov 3, 2021
d3f71a7
Added deprecated flags next to old `onEvent` docs
ryparker Nov 3, 2021
3ff5d7e
Merge branch 'master' into eks-cluster-handler-security-group-prop
ryparker Nov 4, 2021
9a0c15e
Merge branch 'master' into eks-cluster-handler-security-group-prop
iliapolo Nov 4, 2021
d485430
Update packages/@aws-cdk/aws-eks/README.md
ryparker Nov 4, 2021
2949fbf
Update packages/@aws-cdk/aws-eks/README.md
ryparker Nov 4, 2021
68f7655
revert: removed `proxyAgentLayer` and reconnected `onEventLayer`
ryparker Nov 4, 2021
a9a502c
Merge branch 'eks-cluster-handler-security-group-prop' of https://git…
ryparker Nov 4, 2021
6593348
fix: using unique name for each `NodeProxyAgentLayer`
ryparker Nov 4, 2021
223e6fb
clean: referencing same NodeProxyAgentLayer instead of creating multi…
ryparker Nov 4, 2021
1d3a3b8
clean: removed unnecessary doc changes to `onEventLayer`
ryparker Nov 4, 2021
07ca90a
Merge branch 'master' into eks-cluster-handler-security-group-prop
ryparker Nov 4, 2021
991bb0d
Merge branch 'master' into eks-cluster-handler-security-group-prop
mergify[bot] Nov 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/@aws-cdk/aws-eks/README.md
Expand Up @@ -537,16 +537,21 @@ If the endpoint does not expose private access (via `EndpointAccess.PUBLIC`) **o

#### Cluster Handler

The `ClusterHandler` is a Lambda function responsible to interact with the EKS API in order to control the cluster lifecycle. To provision this function inside the VPC, set the `placeClusterHandlerInVpc` property to `true`. This will place the function inside the private subnets of the VPC based on the selection strategy specified in the [`vpcSubnets`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#vpcsubnetsspan-classapi-icon-api-icon-experimental-titlethis-api-element-is-experimental-it-may-change-without-noticespan) property.
The `ClusterHandler` is a set of Lambda functions (`onEventHandler`, `isCompleteHandler`) responsible for interacting with the EKS API in order to control the cluster lifecycle. To provision these functions inside the VPC, set the `placeClusterHandlerInVpc` property to `true`. This will place the functions inside the private subnets of the VPC based on the selection strategy specified in the [`vpcSubnets`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-eks.Cluster.html#vpcsubnetsspan-classapi-icon-api-icon-experimental-titlethis-api-element-is-experimental-it-may-change-without-noticespan) property.

You can configure the environment of this function by specifying it at cluster instantiation. For example, this can be useful in order to configure an http proxy:
You can configure the environment of the Cluster Handler functions by specifying it at cluster instantiation. For example, this can be useful in order to configure an http proxy:

```ts
const cluster = new eks.Cluster(this, 'hello-eks', {
version: eks.KubernetesVersion.V1_21,
clusterHandlerEnvironment: {
'http_proxy': 'http://proxy.myproxy.com'
}
http_proxy: 'http://proxy.myproxy.com'
},
/**
* If proxy is not open to public you may pass a security group to the
ryparker marked this conversation as resolved.
Show resolved Hide resolved
* Cluster Handler Lambdas.
ryparker marked this conversation as resolved.
Show resolved Hide resolved
*/
clusterHandlerSecurityGroup: proxyInstanceSecurityGroup
});
```

Expand Down
Expand Up @@ -41,12 +41,6 @@ export abstract class ResourceHandler {
}

public onEvent() {
// 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() },
});
ryparker marked this conversation as resolved.
Show resolved Hide resolved

switch (this.requestType) {
case 'Create': return this.onCreate();
case 'Update': return this.onUpdate();
Expand Down
Expand Up @@ -8,7 +8,13 @@ import { EksClient } from './common';
import * as consts from './consts';
import { FargateProfileResourceHandler } from './fargate';

// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies
const ProxyAgent = require('proxy-agent');
ryparker marked this conversation as resolved.
Show resolved Hide resolved

aws.config.logger = console;
aws.config.update({
httpOptions: { agent: new ProxyAgent() },
});

let eks: aws.EKS | undefined;

Expand Down
27 changes: 18 additions & 9 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-provider.ts
Expand Up @@ -40,7 +40,14 @@ export interface ClusterResourceProviderProps {
*
* If not defined, a default layer will be used.
*/
readonly onEventLayer?: lambda.ILayerVersion;
readonly proxyAgentLayer?: lambda.ILayerVersion;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the name change...?

Copy link
Contributor Author

@ryparker ryparker Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for the name change is because the proxy-agent layer is now being used in both onEventHandler and isCompleteHandler Lambdas. 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) 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. However if we'd rather not allow users to override that layer then we may still want to deprecate the onEventLayer prop since it is no longer being used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're over optimizing here. Can we just leave everything as is as far as layers go and simply add the NodeProxyAgentLayer to the lambda's that needed and thats it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. I'll remove the new prop and leave the onEventLayer prop. I'll also reconnect the onEventLayer to the onEventLambda.


/**
* The security group to associate with the functions.
*
* @default - No security group.
*/
readonly securityGroup?: ec2.ISecurityGroup;
}

/**
Expand All @@ -66,6 +73,10 @@ export class ClusterResourceProvider extends NestedStack {
private constructor(scope: Construct, id: string, props: ClusterResourceProviderProps) {
super(scope as CoreConstruct, id);

// Allow user to override the layer. Layer must contain `proxy-agent` node_module which is required to proxy AWS SDK requests.
const proxyAgentLayer = props.proxyAgentLayer ? props.proxyAgentLayer : new NodeProxyAgentLayer(this, 'NodeProxyAgentLayer');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this should be configurable. The lambda function code now must have this layer in order to work, so we should simply be adding it in addition to the externally provided layer.


// This is the only Lambda that calls AWS's EKS API.
const onEvent = new lambda.Function(this, 'OnEventHandler', {
code: lambda.Code.fromAsset(HANDLER_DIR),
description: 'onEvent handler for EKS cluster resource provider',
Expand All @@ -75,24 +86,21 @@ export class ClusterResourceProvider extends NestedStack {
timeout: Duration.minutes(1),
vpc: props.subnets ? props.vpc : undefined,
vpcSubnets: props.subnets ? { subnets: props.subnets } : undefined,
securityGroups: props.securityGroup ? [props.securityGroup] : undefined,
layers: [proxyAgentLayer],
});

// Allow user to customize the layer
if (!props.onEventLayer) {
// `NodeProxyAgentLayer` provides `proxy-agent` which is needed to configure `aws-sdk-js` with a user provided proxy.
onEvent.addLayers(new NodeProxyAgentLayer(this, 'NodeProxyAgentLayer'));
} else {
onEvent.addLayers(props.onEventLayer);
}

const isComplete = new lambda.Function(this, 'IsCompleteHandler', {
code: lambda.Code.fromAsset(HANDLER_DIR),
description: 'isComplete handler for EKS cluster resource provider',
runtime: HANDLER_RUNTIME,
environment: props.environment,
handler: 'index.isComplete',
timeout: Duration.minutes(1),
vpc: props.subnets ? props.vpc : undefined,
vpcSubnets: props.subnets ? { subnets: props.subnets } : undefined,
securityGroups: props.securityGroup ? [props.securityGroup] : undefined,
layers: [proxyAgentLayer],
});

this.provider = new cr.Provider(this, 'Provider', {
Expand All @@ -102,6 +110,7 @@ export class ClusterResourceProvider extends NestedStack {
queryInterval: Duration.minutes(1),
vpc: props.subnets ? props.vpc : undefined,
vpcSubnets: props.subnets ? { subnets: props.subnets } : undefined,
securityGroups: props.securityGroup ? [props.securityGroup] : undefined,
ryparker marked this conversation as resolved.
Show resolved Hide resolved
});

props.adminRole.grant(onEvent.role!, 'sts:AssumeRole');
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource.ts
Expand Up @@ -26,7 +26,8 @@ export interface ClusterResourceProps {
readonly environment?: { [key: string]: string };
readonly subnets?: ec2.ISubnet[];
readonly secretsEncryptionKey?: kms.IKey;
readonly onEventLayer?: lambda.ILayerVersion;
readonly proxyAgentLayer?: lambda.ILayerVersion;
readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;
}

/**
Expand Down Expand Up @@ -65,7 +66,8 @@ export class ClusterResource extends CoreConstruct {
subnets: props.subnets,
vpc: props.vpc,
environment: props.environment,
onEventLayer: props.onEventLayer,
proxyAgentLayer: props.proxyAgentLayer,
securityGroup: props.clusterHandlerSecurityGroup,
});

const resource = new CustomResource(this, 'Resource', {
Expand Down
109 changes: 81 additions & 28 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Expand Up @@ -129,12 +129,23 @@ export interface ICluster extends IResource, ec2.IConnectable {
*/
readonly kubectlMemory?: Size;

/**
* A security group to associate with the Cluster Handler's Lambdas.
* The Cluster Handler's Lambdas are responsible for calling AWS's EKS API.
*
* Requires `placeClusterHandlerInVpc` to be set to true.
*
* @default - No security group.
* @attribute
*/
readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;

/**
* An AWS Lambda layer that includes the NPM dependency `proxy-agent`.
*
* If not defined, a default layer will be used.
* @default - If not defined, a default layer will be used.
*/
readonly onEventLayer?: lambda.ILayerVersion;
readonly proxyAgentLayer?: lambda.ILayerVersion;

/**
* Indicates whether Kubernetes resources can be automatically pruned. When
Expand Down Expand Up @@ -310,16 +321,23 @@ export interface ClusterAttributes {
readonly kubectlMemory?: Size;

/**
* An AWS Lambda Layer which includes the NPM dependency `proxy-agent`. This layer
* is used by the onEvent handler to route AWS SDK requests through a proxy.
* A security group id to associate with the Cluster Handler's Lambdas.
* The Cluster Handler's Lambdas are responsible for calling AWS's EKS API.
*
* @default - No security group.
*/
readonly clusterHandlerSecurityGroupId?: string;

/**
* An AWS Lambda Layer which includes the NPM dependency `proxy-agent`. This layer
* is used by the Cluster Handler to route AWS SDK requests through a proxy.
* The handler expects the layer to include the following node_modules:
*
* proxy-agent
*
* @default - a layer bundled with this module.
*/
readonly onEventLayer?: lambda.ILayerVersion;
readonly proxyAgentLayer?: lambda.ILayerVersion;

/**
* Indicates whether Kubernetes resources added through `addManifest()` can be
Expand Down Expand Up @@ -452,13 +470,6 @@ export interface ClusterOptions extends CommonClusterOptions {
*/
readonly kubectlEnvironment?: { [key: string]: string };

/**
* Custom environment variables when interacting with the EKS endpoint to manage the cluster lifecycle.
*
* @default - No environment variables.
*/
readonly clusterHandlerEnvironment?: { [key: string]: string };

/**
* An AWS Lambda Layer which includes `kubectl`, Helm and the AWS CLI.
*
Expand Down Expand Up @@ -490,27 +501,48 @@ export interface ClusterOptions extends CommonClusterOptions {
*/
readonly kubectlMemory?: Size;

/**
* Custom environment variables when interacting with the EKS endpoint to manage the cluster lifecycle.
*
* @default - No environment variables.
*/
readonly clusterHandlerEnvironment?: { [key: string]: string };

/**
* A security group to associate with the Cluster Handler's Lambdas.
* The Cluster Handler's Lambdas are responsible for calling AWS's EKS API.
*
* Requires `placeClusterHandlerInVpc` to be set to true.
*
* @default - No security group.
*/
readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;

/**
* An AWS Lambda Layer which includes the NPM dependency `proxy-agent`.
*
* By default, the provider will use the layer included in the
* "aws-lambda-layer-node-proxy-agent" SAR application which is available in all
* commercial regions.
*
* To deploy the layer locally, visit
* https://github.com/aws-samples/aws-lambda-layer-node-proxy-agent/blob/master/cdk/README.md
* for instructions on how to prepare the .zip file and then define it in your
* app as follows:
* To deploy the layer locally define it in your app as follows:
*
* ```ts
* const layer = new lambda.LayerVersion(this, 'node-proxy-agent-layer', {
* const layer = new lambda.LayerVersion(this, 'proxy-agent-layer', {
* code: lambda.Code.fromAsset(`${__dirname}/layer.zip`)),
* compatibleRuntimes: [lambda.Runtime.NODEJS_14_X]
* compatibleRuntimes: [lambda.Runtime.NODEJS_12_X]
* })
* ```
*
* @default - the layer provided by the `aws-lambda-layer-node-proxy-agent` SAR app.
* @see https://github.com/aws-samples/aws-lambda-layer-node-proxy-agent
* @default - a layer bundled with this module.
*/
readonly proxyAgentLayer?: lambda.ILayerVersion;

/**
* Deprecated
*
* @default - a layer bundled with this module.
* @deprecated use `proxyAgentLayer` instead
*/
readonly onEventLayer?: lambda.ILayerVersion;

Expand Down Expand Up @@ -749,6 +781,7 @@ abstract class ClusterBase extends Resource implements ICluster {
public abstract readonly kubectlSecurityGroup?: ec2.ISecurityGroup;
public abstract readonly kubectlPrivateSubnets?: ec2.ISubnet[];
public abstract readonly kubectlMemory?: Size;
public abstract readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;
public abstract readonly prune: boolean;
public abstract readonly openIdConnectProvider: iam.IOpenIdConnectProvider;
public abstract readonly awsAuth: AwsAuth;
Expand Down Expand Up @@ -902,7 +935,7 @@ abstract class ClusterBase extends Resource implements ICluster {
// cluster or if `mapRole` is set to false. By default this should happen.
let mapRole = options.mapRole ?? true;
if (mapRole && !(this instanceof Cluster)) {
// do the mapping...
// do the mapping...
Annotations.of(autoScalingGroup).addWarning('Auto-mapping aws-auth role for imported cluster is not supported, please map role manually');
mapRole = false;
}
Expand Down Expand Up @@ -1100,10 +1133,21 @@ export class Cluster extends ClusterBase {
public readonly kubectlMemory?: Size;

/**
* The AWS Lambda layer that contains the NPM dependency `proxy-agent`. If
* undefined, a SAR app that contains this layer will be used.
* A security group to associate with the Cluster Handler's Lambdas.
* The Cluster Handler's Lambdas are responsible for calling AWS's EKS API.
*
* Requires `placeClusterHandlerInVpc` to be set to true.
*
* @default - No security group.
*/
public readonly onEventLayer?: lambda.ILayerVersion;
public readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;

/**
* An AWS Lambda layer that includes the NPM dependency `proxy-agent`.
*
* If not defined, a default layer will be used.
*/
public readonly proxyAgentLayer?: lambda.ILayerVersion;

/**
* Determines if Kubernetes resources can be pruned automatically.
Expand Down Expand Up @@ -1188,9 +1232,11 @@ export class Cluster extends ClusterBase {
this.endpointAccess = props.endpointAccess ?? EndpointAccess.PUBLIC_AND_PRIVATE;
this.kubectlEnvironment = props.kubectlEnvironment;
this.kubectlLayer = props.kubectlLayer;
this.onEventLayer = props.onEventLayer;
this.kubectlMemory = props.kubectlMemory;

this.proxyAgentLayer = props.proxyAgentLayer;
this.clusterHandlerSecurityGroup = props.clusterHandlerSecurityGroup;

const privateSubnets = this.selectPrivateSubnets().slice(0, 16);
const publicAccessDisabled = !this.endpointAccess._config.publicAccess;
const publicAccessRestricted = !publicAccessDisabled
Expand All @@ -1215,6 +1261,10 @@ export class Cluster extends ClusterBase {
throw new Error('Cannot place cluster handler in the VPC since no private subnets could be selected');
}

if (props.clusterHandlerSecurityGroup && !placeClusterHandlerInVpc) {
throw new Error('Cannot specify clusterHandlerSecurityGroup without placeClusterHandlerInVpc set to true');
}

const resource = this._clusterResource = new ClusterResource(this, 'Resource', {
name: this.physicalName,
environment: props.clusterHandlerEnvironment,
Expand All @@ -1241,7 +1291,8 @@ export class Cluster extends ClusterBase {
secretsEncryptionKey: props.secretsEncryptionKey,
vpc: this.vpc,
subnets: placeClusterHandlerInVpc ? privateSubnets : undefined,
onEventLayer: this.onEventLayer,
clusterHandlerSecurityGroup: this.clusterHandlerSecurityGroup,
proxyAgentLayer: this.proxyAgentLayer,
});

if (this.endpointAccess._config.privateAccess && privateSubnets.length !== 0) {
Expand Down Expand Up @@ -1827,8 +1878,9 @@ class ImportedCluster extends ClusterBase {
public readonly kubectlSecurityGroup?: ec2.ISecurityGroup | undefined;
public readonly kubectlPrivateSubnets?: ec2.ISubnet[] | undefined;
public readonly kubectlLayer?: lambda.ILayerVersion;
public readonly onEventLayer?: lambda.ILayerVersion;
public readonly kubectlMemory?: Size;
public readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup | undefined;
public readonly proxyAgentLayer?: lambda.ILayerVersion;
public readonly prune: boolean;

// so that `clusterSecurityGroup` on `ICluster` can be configured without optionality, avoiding users from having
Expand All @@ -1845,8 +1897,9 @@ class ImportedCluster extends ClusterBase {
this.kubectlEnvironment = props.kubectlEnvironment;
this.kubectlPrivateSubnets = props.kubectlPrivateSubnetIds ? props.kubectlPrivateSubnetIds.map((subnetid, index) => ec2.Subnet.fromSubnetId(this, `KubectlSubnet${index}`, subnetid)) : undefined;
this.kubectlLayer = props.kubectlLayer;
this.onEventLayer = props.onEventLayer;
this.kubectlMemory = props.kubectlMemory;
this.clusterHandlerSecurityGroup = props.clusterHandlerSecurityGroupId ? ec2.SecurityGroup.fromSecurityGroupId(this, 'ClusterHandlerSecurityGroup', props.clusterHandlerSecurityGroupId) : undefined;
this.proxyAgentLayer = props.proxyAgentLayer;
this.prune = props.prune ?? true;

let i = 1;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/fargate-profile.ts
Expand Up @@ -149,7 +149,7 @@ export class FargateProfile extends CoreConstruct implements ITaggable {

const provider = ClusterResourceProvider.getOrCreate(this, {
adminRole: props.cluster.adminRole,
onEventLayer: props.cluster.onEventLayer,
proxyAgentLayer: props.cluster.proxyAgentLayer,
});

this.podExecutionRole = props.podExecutionRole ?? new iam.Role(this, 'PodExecutionRole', {
Expand Down