Skip to content
This repository was archived by the owner on Jul 16, 2024. It is now read-only.

Commit 007d9a8

Browse files
authored
fix: notebookplatform fix for type mismatch (#353)
* fix: resolve issue failing the deployment of managedendpoint * fix: adding e2e test for notebook-platform.ts * fix: add managednode groups for notebook platform without podtempalte labels * fix: add default version for emr on eks to be used if no version is provided in the managedendpoint definition * fix: amend the unit tests to pass the tests. Added the new nodegroup -notebookwithoutpodtemplate- Co-authored-by: Lotfi Mouhib <[email protected]>
1 parent b3e1064 commit 007d9a8

File tree

7 files changed

+136
-6
lines changed

7 files changed

+136
-6
lines changed

core/src/emr-eks-platform/emr-eks-cluster.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class EmrEksCluster extends TrackedConstruct {
154154

155155
return stack.node.tryFindChild(id) as EmrEksCluster || emrEksCluster!;
156156
}
157-
private static readonly EMR_VERSIONS = ['emr-6.5.0-latest', 'emr-6.4.0-latest', 'emr-6.3.0-latest', 'emr-6.2.0-latest', 'emr-5.33.0-latest', 'emr-5.32.0-latest'];
157+
private static readonly EMR_VERSIONS = ['emr-6.6.0-latest', 'emr-6.5.0-latest', 'emr-6.4.0-latest', 'emr-6.3.0-latest', 'emr-6.2.0-latest', 'emr-5.33.0-latest', 'emr-5.32.0-latest'];
158158
private static readonly DEFAULT_EMR_VERSION = 'emr-6.4.0-latest';
159159
private static readonly DEFAULT_EKS_VERSION = KubernetesVersion.V1_21;
160160
private static readonly DEFAULT_CLUSTER_NAME = 'data-platform';
@@ -408,6 +408,7 @@ export class EmrEksCluster extends TrackedConstruct {
408408
// Add a nodegroup for notebooks
409409
this.addEmrEksNodegroup('notebookDriver', EmrEksNodegroup.NOTEBOOK_DRIVER);
410410
this.addEmrEksNodegroup('notebookExecutor', EmrEksNodegroup.NOTEBOOK_EXECUTOR);
411+
this.addEmrEksNodegroup('notebookWithoutPodTemplate', EmrEksNodegroup.NOTEBOOK_WITHOUT_PODTEMPLATE);
411412
}
412413
// Create an Amazon S3 Bucket for default podTemplate assets
413414
this.assetBucket = AraBucket.getOrCreate(this, { bucketName: `${this.clusterName.toLowerCase()}-emr-eks-assets`, encryption: BucketEncryption.KMS_MANAGED });
@@ -452,7 +453,7 @@ export class EmrEksCluster extends TrackedConstruct {
452453
// Replace the pod template location for driver and executor with the correct Amazon S3 path in the notebook default config
453454
// NotebookDefaultConfig.applicationConfiguration[0].properties['spark.kubernetes.driver.podTemplateFile'] = this.assetBucket.s3UrlForObject(`${this.podTemplateLocation.objectKey}/notebook-driver.yaml`);
454455
// NotebookDefaultConfig.applicationConfiguration[0].properties['spark.kubernetes.executor.podTemplateFile'] = this.assetBucket.s3UrlForObject(`${this.podTemplateLocation.objectKey}/notebook-executor.yaml`);
455-
this.notebookDefaultConfig = JSON.stringify(NotebookDefaultConfig);
456+
this.notebookDefaultConfig = JSON.parse(JSON.stringify(NotebookDefaultConfig));
456457

457458
// Replace the pod template location for driver and executor with the correct Amazon S3 path in the critical default config
458459
CriticalDefaultConfig.applicationConfiguration[0].properties['spark.kubernetes.driver.podTemplateFile'] = this.assetBucket.s3UrlForObject(`${this.podTemplateLocation.objectKey}/critical-driver.yaml`);

core/src/notebook-platform/notebook-platform.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ export enum IdpRelayState {
165165
* ```
166166
*/
167167
export class NotebookPlatform extends TrackedConstruct {
168+
private static readonly DEFAULT_EMR_VERSION = 'emr-6.6.0-latest';
168169
private static readonly STUDIO_PRINCIPAL: string = 'elasticmapreduce.amazonaws.com';
169170
private readonly studioId: string;
170171
private readonly workSpaceSecurityGroup: SecurityGroup;
@@ -391,7 +392,7 @@ export class NotebookPlatform extends TrackedConstruct {
391392
`${user.identityName}${index}`,
392393
notebookManagedEndpoint.executionPolicy,
393394
),
394-
emrOnEksVersion: emrOnEksVersion ? emrOnEksVersion : undefined,
395+
emrOnEksVersion: emrOnEksVersion ? emrOnEksVersion : NotebookPlatform.DEFAULT_EMR_VERSION,
395396
configurationOverrides: configOverride ? configOverride : undefined,
396397
},
397398

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: MIT-0
3+
4+
/**
5+
* Tests EmrEksCluster
6+
*
7+
* @group integ/notebook-platform
8+
*/
9+
10+
import { ManagedPolicy, PolicyStatement } from '@aws-cdk/aws-iam';
11+
import * as cdk from '@aws-cdk/core';
12+
import { ArnFormat, Aws } from '@aws-cdk/core';
13+
import { SdkProvider } from 'aws-cdk/lib/api/aws-auth';
14+
import { CloudFormationDeployments } from 'aws-cdk/lib/api/cloudformation-deployments';
15+
16+
import { NotebookPlatform, StudioAuthMode } from '../../src';
17+
import { EmrEksCluster } from '../../src/emr-eks-platform';
18+
19+
jest.setTimeout(2000000);
20+
// GIVEN
21+
const integTestApp = new cdk.App();
22+
const stack = new cdk.Stack(integTestApp, 'notebookPlatformE2eTest');
23+
24+
const emrEksCluster = EmrEksCluster.getOrCreate(stack, {
25+
eksAdminRoleArn: 'arn:aws:iam::123445678912:role/gromav',
26+
});
27+
28+
const notebookPlatform = new NotebookPlatform(stack, 'platform-notebook', {
29+
emrEks: emrEksCluster,
30+
eksNamespace: 'notebookspace',
31+
studioName: 'testNotebook',
32+
studioAuthMode: StudioAuthMode.IAM,
33+
});
34+
35+
36+
const policy1 = new ManagedPolicy(stack, 'MyPolicy1', {
37+
statements: [
38+
new PolicyStatement({
39+
resources: ['*'],
40+
actions: ['s3:*'],
41+
}),
42+
new PolicyStatement({
43+
resources: [
44+
stack.formatArn({
45+
account: Aws.ACCOUNT_ID,
46+
region: Aws.REGION,
47+
service: 'logs',
48+
resource: '*',
49+
arnFormat: ArnFormat.NO_RESOURCE_NAME,
50+
}),
51+
],
52+
actions: [
53+
'logs:*',
54+
],
55+
}),
56+
],
57+
});
58+
59+
notebookPlatform.addUser([{
60+
identityName: 'janeDoe',
61+
notebookManagedEndpoints: [
62+
{
63+
emrOnEksVersion: 'emr-6.4.0-latest',
64+
executionPolicy: policy1,
65+
},
66+
],
67+
}]);
68+
69+
new cdk.CfnOutput(stack, 'EmrEksAdminRoleOutput', {
70+
value: emrEksCluster.eksCluster.adminRole.roleArn,
71+
exportName: 'emrEksAdminRole',
72+
});
73+
74+
describe('deploy succeed', () => {
75+
it('can be deploy succcessfully', async () => {
76+
// GIVEN
77+
const stackArtifact = integTestApp.synth().getStackByName(stack.stackName);
78+
79+
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
80+
profile: process.env.AWS_PROFILE,
81+
});
82+
const cloudFormation = new CloudFormationDeployments({ sdkProvider });
83+
84+
// WHEN
85+
const deployResult = await cloudFormation.deployStack({
86+
stack: stackArtifact,
87+
});
88+
89+
// THEN
90+
expect(deployResult.outputs.emrEksAdminRole).toEqual('arn:aws:iam::123445678912:role/gromav');
91+
92+
}, 9000000);
93+
});
94+
95+
afterAll(async () => {
96+
const stackArtifact = integTestApp.synth().getStackByName(stack.stackName);
97+
98+
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
99+
profile: process.env.AWS_PROFILE,
100+
});
101+
const cloudFormation = new CloudFormationDeployments({ sdkProvider });
102+
103+
await cloudFormation.destroyStack({
104+
stack: stackArtifact,
105+
});
106+
});

core/test/unit/cdk-nag/nag-emr-eks.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,18 @@ NagSuppressions.addResourceSuppressionsByPath(
403403
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
404404
);
405405

406+
NagSuppressions.addResourceSuppressionsByPath(
407+
stack,
408+
'eks-emr-studio/data-platformCluster/NodegroupnotebookWithoutPodTemplate-0/NodeGroupRole/Resource',
409+
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
410+
);
411+
412+
NagSuppressions.addResourceSuppressionsByPath(
413+
stack,
414+
'eks-emr-studio/data-platformCluster/NodegroupnotebookWithoutPodTemplate-1/NodeGroupRole/Resource',
415+
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
416+
);
417+
406418
test('No unsuppressed Warnings', () => {
407419
const warnings = Annotations.fromStack(stack).findWarning('*', Match.stringLikeRegexp('AwsSolutions-.*'));
408420
console.log(warnings);

core/test/unit/cdk-nag/nag-notebook-platform.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,18 @@ NagSuppressions.addResourceSuppressionsByPath(
178178
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
179179
);
180180

181+
NagSuppressions.addResourceSuppressionsByPath(
182+
stack,
183+
'eks-emr-studio/data-platformCluster/NodegroupnotebookWithoutPodTemplate-0/NodeGroupRole/Resource',
184+
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
185+
);
186+
187+
NagSuppressions.addResourceSuppressionsByPath(
188+
stack,
189+
'eks-emr-studio/data-platformCluster/NodegroupnotebookWithoutPodTemplate-1/NodeGroupRole/Resource',
190+
[{ id: 'AwsSolutions-IAM4', reason: 'Nodegroups are using AWS Managed Policies' }],
191+
);
192+
181193
NagSuppressions.addResourceSuppressionsByPath(
182194
stack,
183195
'eks-emr-studio/data-platformCluster/NodegroupnotebookExecutor-0/NodeGroupRole/Resource',

core/test/unit/emr-eks-platform/emr-eks-cluster.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ test('EKS should have a helm chart for deploying the Kubernetes Dashboard', () =
124124

125125
test('EKS cluster should have the default Nodegroups', () => {
126126

127-
expect(emrEksClusterStack).toCountResources('AWS::EKS::Nodegroup', 11);
127+
expect(emrEksClusterStack).toCountResources('AWS::EKS::Nodegroup', 13);
128128

129129
expect(emrEksClusterStack).toHaveResource('AWS::EKS::Nodegroup', {
130130
AmiType: 'AL2_x86_64',

core/test/unit/notebook-platform/notebook-platform.test.ts

-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77
* @group unit/notebook-data-platform
88
*/
99

10-
//TODO REDO this unit test to support the new way of deploying notebooks with nested stacks
11-
1210
import * as assertCDK from '@aws-cdk/assert';
1311
import { ManagedPolicy, PolicyDocument, PolicyStatement } from '@aws-cdk/aws-iam';
1412
import { Stack } from '@aws-cdk/core';

0 commit comments

Comments
 (0)