Skip to content

Commit

Permalink
Merge branch 'master' into add-iam-authorizer
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Dec 17, 2021
2 parents 90ce8cf + b7be71c commit fb51b90
Show file tree
Hide file tree
Showing 13 changed files with 686 additions and 83 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/issue-label-assign.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ jobs:
{"area":"package/tools","keywords":["cli","command line","init","synth","diff","bootstrap"],"labels":["package/tools"],"assignees":["rix0rrr"]},
{"area":"@aws-cdk/alexa-ask","keywords":["alexa-ask","alexa", "cfnskill"],"labels":["@aws-cdk/alexa-ask"],"assignees":["madeline-k"]},
{"area":"@aws-cdk/app-delivery","keywords":["app-delivery","PipelineDeployStackAction"],"labels":["@aws-cdk/app-delivery"],"assignees":["skinny85"]},
{"area":"@aws-cdk/assert","keywords":["assert"],"labels":["@aws-cdk/assert"],"assignees":["kaizen3031593"]},
{"area":"@aws-cdk/assertions","keywords":["assertions"],"labels":["@aws-cdk/assertions"],"assignees":["kaizen3031593"]},
{"area":"@aws-cdk/assert","keywords":["assert", "@aws-cdk/assert"],"labels":["@aws-cdk/assert"],"assignees":["kaizen3031593"]},
{"area":"@aws-cdk/assertions","keywords":["assertions", "@aws-cdk/assertions"],"labels":["@aws-cdk/assertions"],"assignees":["kaizen3031593"]},
{"area":"@aws-cdk/assets","keywords":["assets","staging"],"labels":["@aws-cdk/assets"],"assignees":["eladb"]},
{"area":"@aws-cdk/aws-accessanalyzer","keywords":["aws-accessanalyzer","accessanalyzer","cfnanalyzer"],"labels":["@aws-cdk/aws-accessanalyzer"],"assignees":["skinny85"]},
{"area":"@aws-cdk/aws-acmpca","keywords":["aws-acmpca","acmpca","certificateauthority"],"labels":["@aws-cdk/aws-acmpca"],"assignees":["skinny85"]},
Expand Down
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-eks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,21 @@ are being passed down (such as `repo`, `values`, `version`, `namespace`, `wait`,
This means that if the chart is added to CDK with the same release name, it will try to update
the chart in the cluster.

Additionally, the `chartAsset` property can be an `aws-s3-assets.Asset`. This allows the use of local, private helm charts.

```ts
import * as s3Assets from '@aws-cdk/aws-s3-assets';

declare const cluster: eks.Cluster;
const chartAsset = new s3Assets.Asset(this, 'ChartAsset', {
path: '/path/to/asset'
});

cluster.addHelmChart('test-chart', {
chartAsset: chartAsset,
});
```

Helm charts are implemented as CloudFormation resources in CDK.
This means that if the chart is deleted from your code (or the stack is
deleted), the next `cdk deploy` will issue a `helm uninstall` command and the
Expand Down
27 changes: 26 additions & 1 deletion packages/@aws-cdk/aws-eks/lib/helm-chart.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Asset } from '@aws-cdk/aws-s3-assets';
import { CustomResource, Duration, Names, Stack } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { ICluster } from './cluster';
Expand All @@ -14,8 +15,11 @@ import { Construct as CoreConstruct } from '@aws-cdk/core';
export interface HelmChartOptions {
/**
* The name of the chart.
* Either this or `chartAsset` must be specified.
*
* @default - No chart name. Implies `chartAsset` is used.
*/
readonly chart: string;
readonly chart?: string;

/**
* The name of the release.
Expand All @@ -35,6 +39,14 @@ export interface HelmChartOptions {
*/
readonly repository?: string;

/**
* The chart in the form of an asset.
* Either this or `chart` must be specified.
*
* @default - No chart asset. Implies `chart` is used.
*/
readonly chartAsset?: Asset;

/**
* The Kubernetes namespace scope of the requests.
* @default default
Expand Down Expand Up @@ -102,11 +114,23 @@ export class HelmChart extends CoreConstruct {
throw new Error('Helm chart timeout cannot be higher than 15 minutes.');
}

if (!props.chart && !props.chartAsset) {
throw new Error("Either 'chart' or 'chartAsset' must be specified to install a helm chart");
}

if (props.chartAsset && (props.repository || props.version)) {
throw new Error(
"Neither 'repository' nor 'version' can be used when configuring 'chartAsset'",
);
}

// default not to wait
const wait = props.wait ?? false;
// default to create new namespace
const createNamespace = props.createNamespace ?? true;

props.chartAsset?.grantRead(provider.handlerRole);

new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
resourceType: HelmChart.RESOURCE_TYPE,
Expand All @@ -115,6 +139,7 @@ export class HelmChart extends CoreConstruct {
RoleArn: provider.roleArn, // TODO: bake into the provider's environment
Release: props.release ?? Names.uniqueId(this).slice(-53).toLowerCase(), // Helm has a 53 character limit for the name
Chart: props.chart,
ChartAssetURL: props.chartAsset?.s3ObjectUrl,
Version: props.version,
Wait: wait || undefined, // props are stringified so we encode “false” as undefined
Timeout: timeout ? `${timeout.toString()}s` : undefined, // Helm v3 expects duration instead of integer
Expand Down
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-eks/lib/k8s-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export class KubernetesManifest extends CoreConstruct {
this.injectIngressAlbAnnotations(props.manifest, props.ingressAlbScheme ?? AlbScheme.INTERNAL);
}

new CustomResource(this, 'Resource', {
const customResource = new CustomResource(this, 'Resource', {
serviceToken: provider.serviceToken,
resourceType: KubernetesManifest.RESOURCE_TYPE,
properties: {
Expand All @@ -154,6 +154,8 @@ export class KubernetesManifest extends CoreConstruct {
SkipValidation: props.skipValidation,
},
});

this.node.defaultChild = customResource.node.defaultChild;
}

/**
Expand Down
47 changes: 37 additions & 10 deletions packages/@aws-cdk/aws-eks/lib/kubectl-handler/helm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
import logging
import os
import subprocess
import shutil
import zipfile
from urllib.parse import urlparse, unquote

logger = logging.getLogger()
logger.setLevel(logging.INFO)
Expand All @@ -12,6 +15,16 @@
outdir = os.environ.get('TEST_OUTDIR', '/tmp')
kubeconfig = os.path.join(outdir, 'kubeconfig')

def get_chart_asset_from_url(chart_asset_url):
chart_zip = os.path.join(outdir, 'chart.zip')
shutil.rmtree(chart_zip, ignore_errors=True)
subprocess.check_call(['aws', 's3', 'cp', chart_asset_url, chart_zip])
chart_dir = os.path.join(outdir, 'chart')
shutil.rmtree(chart_dir, ignore_errors=True)
os.mkdir(chart_dir)
with zipfile.ZipFile(chart_zip, 'r') as zip_ref:
zip_ref.extractall(chart_dir)
return chart_dir

def helm_handler(event, context):
logger.info(json.dumps(event))
Expand All @@ -20,17 +33,18 @@ def helm_handler(event, context):
props = event['ResourceProperties']

# resource properties
cluster_name = props['ClusterName']
role_arn = props['RoleArn']
release = props['Release']
chart = props['Chart']
version = props.get('Version', None)
wait = props.get('Wait', False)
timeout = props.get('Timeout', None)
namespace = props.get('Namespace', None)
cluster_name = props['ClusterName']
role_arn = props['RoleArn']
release = props['Release']
chart = props.get('Chart', None)
chart_asset_url = props.get('ChartAssetURL', None)
version = props.get('Version', None)
wait = props.get('Wait', False)
timeout = props.get('Timeout', None)
namespace = props.get('Namespace', None)
create_namespace = props.get('CreateNamespace', None)
repository = props.get('Repository', None)
values_text = props.get('Values', None)
repository = props.get('Repository', None)
values_text = props.get('Values', None)

# "log in" to the cluster
subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',
Expand All @@ -51,6 +65,19 @@ def helm_handler(event, context):
f.write(json.dumps(values, indent=2))

if request_type == 'Create' or request_type == 'Update':
# Ensure chart or chart_asset_url are set
if chart == None and chart_asset_url == None:
raise RuntimeError(f'chart or chartAsset must be specified')

if chart_asset_url != None:
assert(chart==None)
assert(repository==None)
assert(version==None)
if not chart_asset_url.startswith('s3://'):
raise RuntimeError(f'ChartAssetURL must point to as s3 location but is {chart_asset_url}')
# future work: support versions from s3 assets
chart = get_chart_asset_from_url(chart_asset_url)

helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace)
elif request_type == "Delete":
try:
Expand Down
105 changes: 74 additions & 31 deletions packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { InstanceType, ISecurityGroup, SubnetSelection } from '@aws-cdk/aws-ec2';
import { InstanceType, ISecurityGroup, SubnetSelection, InstanceArchitecture } from '@aws-cdk/aws-ec2';
import { IRole, ManagedPolicy, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
import { IResource, Resource, Annotations, withResolved } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { Cluster, ICluster } from './cluster';
import { CfnNodegroup } from './eks.generated';
import { INSTANCE_TYPES } from './instance-types';

/**
* NodeGroup interface
Expand Down Expand Up @@ -157,9 +156,10 @@ export interface NodegroupOptions {
*/
readonly subnets?: SubnetSelection;
/**
* The AMI type for your node group.
* The AMI type for your node group. If you explicitly specify the launchTemplate with custom AMI, do not specify this property, or
* the node group deployment will fail. In other cases, you will need to specify correct amiType for the nodegroup.
*
* @default - auto-determined from the instanceTypes property.
* @default - auto-determined from the instanceTypes property when launchTemplateSpec property is not specified
*/
readonly amiType?: NodegroupAmiType;
/**
Expand Down Expand Up @@ -357,15 +357,21 @@ export class Nodegroup extends Resource implements INodegroup {
Annotations.of(this).addWarning('"instanceType" is deprecated and will be removed in the next major version. please use "instanceTypes" instead');
}
const instanceTypes = props.instanceTypes ?? (props.instanceType ? [props.instanceType] : undefined);
let expectedAmiType = undefined;
let possibleAmiTypes: NodegroupAmiType[] = [];

if (instanceTypes && instanceTypes.length > 0) {
// if the user explicitly configured instance types, we can calculate the expected ami type.
expectedAmiType = getAmiType(instanceTypes);

// if the user explicitly configured an ami type, make sure its the same as the expected one.
if (props.amiType && props.amiType !== expectedAmiType) {
throw new Error(`The specified AMI does not match the instance types architecture, either specify ${expectedAmiType} or dont specify any`);
/**
* if the user explicitly configured instance types, we can't caculate the expected ami type as we support
* Amazon Linux 2 and Bottlerocket now. However we can check:
*
* 1. instance types of different CPU architectures are not mixed(e.g. X86 with ARM).
* 2. user-specified amiType should be included in `possibleAmiTypes`.
*/
possibleAmiTypes = getPossibleAmiTypes(instanceTypes);

// if the user explicitly configured an ami type, make sure it's included in the possibleAmiTypes
if (props.amiType && !possibleAmiTypes.includes(props.amiType)) {
throw new Error(`The specified AMI does not match the instance types architecture, either specify one of ${possibleAmiTypes} or don't specify any`);
}
}

Expand All @@ -387,11 +393,17 @@ export class Nodegroup extends Resource implements INodegroup {
nodegroupName: props.nodegroupName,
nodeRole: this.role.roleArn,
subnets: this.cluster.vpc.selectSubnets(props.subnets).subnetIds,

// if a launch template is configured, we cannot apply a default since it
// might exist in the launch template as well, causing a deployment failure.
amiType: props.launchTemplateSpec !== undefined ? props.amiType : (props.amiType ?? expectedAmiType),

/**
* Case 1: If launchTemplate is explicitly specified with custom AMI, we cannot specify amiType, or the node group deployment will fail.
* As we don't know if the custom AMI is specified in the lauchTemplate, we just use props.amiType.
*
* Case 2: If launchTemplate is not specified, we try to determine amiType from the instanceTypes and it could be either AL2 or Bottlerocket.
* To avoid breaking changes, we use possibleAmiTypes[0] if amiType is undefined and make sure AL2 is always the first element in possibleAmiTypes
* as AL2 is previously the `expectedAmi` and this avoids breaking changes.
*
* That being said, users now either have to explicitly specify correct amiType or just leave it undefined.
*/
amiType: props.launchTemplateSpec ? props.amiType : (props.amiType ?? possibleAmiTypes[0]),
capacityType: props.capacityType ? props.capacityType.valueOf() : undefined,
diskSize: props.diskSize,
forceUpdateEnabled: props.forceUpdate ?? true,
Expand Down Expand Up @@ -443,24 +455,55 @@ export class Nodegroup extends Resource implements INodegroup {
}
}

function getAmiTypeForInstanceType(instanceType: InstanceType) {
return INSTANCE_TYPES.graviton2.includes(instanceType.toString().substring(0, 3)) ? NodegroupAmiType.AL2_ARM_64 :
INSTANCE_TYPES.graviton.includes(instanceType.toString().substring(0, 2)) ? NodegroupAmiType.AL2_ARM_64 :
INSTANCE_TYPES.gpu.includes(instanceType.toString().substring(0, 2)) ? NodegroupAmiType.AL2_X86_64_GPU :
INSTANCE_TYPES.inferentia.includes(instanceType.toString().substring(0, 4)) ? NodegroupAmiType.AL2_X86_64_GPU :
NodegroupAmiType.AL2_X86_64;
/**
* AMI types of different architectures. Make sure AL2 is always the first element, which will be the default
* AmiType if amiType and launchTemplateSpec are both undefined.
*/
const arm64AmiTypes: NodegroupAmiType[] = [NodegroupAmiType.AL2_ARM_64, NodegroupAmiType.BOTTLEROCKET_ARM_64];
const x8664AmiTypes: NodegroupAmiType[] = [NodegroupAmiType.AL2_X86_64, NodegroupAmiType.BOTTLEROCKET_X86_64];
const gpuAmiTypes: NodegroupAmiType[] = [NodegroupAmiType.AL2_X86_64_GPU];


/**
* This function check if the instanceType is GPU instance.
* @param instanceType The EC2 instance type
*/
function isGpuInstanceType(instanceType: InstanceType): boolean {
// capture the family, generation, capabilities, and size portions of the instance type id
const instanceTypeComponents = instanceType.toString().match(/^([a-z]+)(\d{1,2})([a-z]*)\.([a-z0-9]+)$/);
if (instanceTypeComponents == null) {
throw new Error('Malformed instance type identifier');
}
const family = instanceTypeComponents[1];
return ['p', 'g', 'inf'].includes(family);
}

// this function examines the CPU architecture of every instance type and determines
// what ami type is compatible for all of them. it either throws or produces a single value because
// instance types of different CPU architectures are not supported.
function getAmiType(instanceTypes: InstanceType[]) {
const amiTypes = new Set(instanceTypes.map(i => getAmiTypeForInstanceType(i)));
if (amiTypes.size == 0) { // protective code, the current implementation will never result in this.
type AmiArchitecture = InstanceArchitecture | 'GPU';
/**
* This function examines the CPU architecture of every instance type and determines
* what AMI types are compatible for all of them. it either throws or produces an array of possible AMI types because
* instance types of different CPU architectures are not supported.
* @param instanceTypes The instance types
* @returns NodegroupAmiType[]
*/
function getPossibleAmiTypes(instanceTypes: InstanceType[]): NodegroupAmiType[] {
function typeToArch(instanceType: InstanceType): AmiArchitecture {
return isGpuInstanceType(instanceType) ? 'GPU' : instanceType.architecture;
}
const archAmiMap = new Map<AmiArchitecture, NodegroupAmiType[]>([
[InstanceArchitecture.ARM_64, arm64AmiTypes],
[InstanceArchitecture.X86_64, x8664AmiTypes],
['GPU', gpuAmiTypes],
]);
const architectures: Set<AmiArchitecture> = new Set(instanceTypes.map(typeToArch));

if (architectures.size === 0) { // protective code, the current implementation will never result in this.
throw new Error(`Cannot determine any ami type comptaible with instance types: ${instanceTypes.map(i => i.toString).join(',')}`);
}
if (amiTypes.size > 1) {
throw new Error('instanceTypes of different CPU architectures is not allowed');

if (architectures.size > 1) {
throw new Error('instanceTypes of different architectures is not allowed');
}
return amiTypes.values().next().value;

return archAmiMap.get(Array.from(architectures)[0])!;
}
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-eks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"dependencies": {
"@aws-cdk/aws-autoscaling": "0.0.0",
"@aws-cdk/aws-ec2": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
Expand All @@ -116,6 +117,7 @@
"peerDependencies": {
"@aws-cdk/aws-autoscaling": "0.0.0",
"@aws-cdk/aws-ec2": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
Expand Down
Loading

0 comments on commit fb51b90

Please sign in to comment.