feat(ec2): add nitro enclave and hibernation settings to the Instance Construct #30228

mazyu36 · 2024-05-16T04:30:33Z

Issue # (if applicable)

N/A

Reason for this change

MIssing property in the L2 Construct

Description of changes

Add nitroEnclaveEnabled and hibernationConfigured property.

Description of how you validated changes

Added unit tests and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

go-to-k

Thanks for the PR. I made some comments!

packages/aws-cdk-lib/aws-ec2/README.md

packages/aws-cdk-lib/aws-ec2/lib/instance.ts

go-to-k · 2024-05-23T11:21:13Z

packages/aws-cdk-lib/aws-ec2/lib/instance.ts

+   *
+   * @default false
+   */
+  readonly hibernationConfigured?: boolean;


Can we change to hibernationEnabled to match enclaveEnabled? (HibernationOptionsProperty in L1 has the configured, but ...)

I changed the property name.

packages/aws-cdk-lib/aws-ec2/lib/instance.ts

go-to-k · 2024-05-23T11:36:59Z

packages/aws-cdk-lib/aws-ec2/test/instance.test.ts

+  [true, true],
+  [false, false],
+])('given nitroEnclaveEnabled %p', (given: boolean, expected: boolean) => {


it needs to change the sentence If we change the parameter name.

Updated to match the parameter name.

go-to-k · 2024-05-23T11:37:42Z

packages/aws-cdk-lib/aws-ec2/test/instance.test.ts

+  [true, true],
+  [false, false],
+])('given hibernationConfigured %p', (given: boolean, expected: boolean) => {


Same as above.

Updated to match the parameter name.

go-to-k · 2024-05-23T11:38:23Z

packages/aws-cdk-lib/aws-ec2/test/instance.test.ts

+});
+
+test('throw if AWS Nitro Enclaves and hibernation are enabled', () => {


It would be good to use parameter names in the sentence.

I changed the sentence to match parameter names.

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

mazyu36 · 2024-05-23T12:40:12Z

@go-to-k
Thank you for your review!
I've addressed all your comments.

go-to-k

Please check the last comment (That is very very minor)!

packages/aws-cdk-lib/aws-ec2/README.md

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

go-to-k

Thanks!

packages/aws-cdk-lib/aws-ec2/lib/instance.ts

shikha372 · 2024-05-25T00:12:20Z

...-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-nitoro-envlaves-hibernation.ts

nit: rename file to integ.instance-nitro-enclaves-hibernation.ts

Sorry typo. I fixed it.

Thanks @mazyu36 , i see some of the files updated but not all in your last commit, you'll first need to rename the integ test file name to integ.instance-nitoro-enclaves-hibernation.ts, build and then run it to update file and its snapshot.

File name is still integ.instance-nitoro-envlaves-hibernation.ts
...-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-nitoro-envlaves-hibernation.ts

@shikha372
Thanks.
I think the link you provided seems to be an old commit.
I renamed a integ test and reran it at this commit.

And in last commit, all files appear to be renamed.

I would appreciate it if you could point out any misunderstandings on my part.

Thank mazyu, followed your last commit link, main integ test file name is still incorrect here

Oops... I misspelled "enclaves" too, sorry about that.

shikha372 · 2024-05-25T00:13:31Z

Thanks @mazyu36 for your contribution, looking at the documentation for this feature, shall we add some instance type based checks to ensure that this property is enabled only for specific type of instance ?
https://docs.aws.amazon.com/enclaves/latest/user/nitro-enclave.html#nitro-enclave-reqs

mazyu36 · 2024-05-25T03:32:15Z

@shikha372
Thank you for the review.
I had also considered adding validation, but due to the large number of instance types that would be excluded based on the requirements conditions, making it complex, and the fact that LaunchTemplate already has a similar setting implemented but no specific validation, I decided to leave it out this time as well.

However, I thought it would be inconsiderate to have no information at all, so I added an explanation and a link to the docs for enclaveEnabled. What do you think?

Since I couldn't come up with many other good ideas, I would appreciate your feedback if you have any better suggestions.

shikha372 · 2024-05-31T02:40:21Z

f instance types that would be excluded based on the requirements conditions, making it complex, and the fact that LaunchTemplate already has a similar setting implemented but no specific validation, I decided to leave it out this time as w

I see, thank you @mazyu36 for your contribution. Clear documentation will help in this case.

mazyu36 · 2024-05-31T03:40:18Z

@shikha372
Thank you.
I have added a note to the README with a link regarding the requirements for instances.

If you have any better suggestions, I would appreciate your comments.

shikha372 · 2024-05-31T19:05:51Z

...s-cdk-testing/framework-integ/test/aws-ec2/test/integ.instance-nitro-envlaves-hibernation.ts

nit: filename

I fixed it.

Pull request has been modified.

aws-cdk-automation · 2024-06-01T03:41:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 2a5b073
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mazyu36 · 2024-06-01T03:49:51Z

@shikha372
Thank you for the review. Sorry for making you go back and forth so many times.
I have revised the integ test again.

AWS CDK Team and others added 4 commits May 16, 2024 13:18

chore(release): 2.142.0

31896af

feat: add nitroEnclave and hibenartion

b0b71aa

Merge branch 'main' into ec2-enclave-hibernation

50ef96d

fix update README

73b1f15

aws-cdk-automation requested a review from a team May 16, 2024 04:30

github-actions bot added p2 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK labels May 16, 2024

fix: README

d978fc8

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 16, 2024

github-actions bot mentioned this pull request May 20, 2024

Weekly PR metrics report #30274

Closed

go-to-k suggested changes May 23, 2024

View reviewed changes

mazyu36 and others added 11 commits May 23, 2024 20:45

Merge branch 'main' into ec2-enclave-hibernation

ab58bf6

Update packages/aws-cdk-lib/aws-ec2/README.md

e6e209e

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/lib/instance.ts

ef219e8

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/lib/instance.ts

21be6d4

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/lib/instance.ts

f7c0284

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/lib/instance.ts

3d1533b

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/README.md

5ac126f

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/README.md

2cacc1f

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/README.md

b6821b6

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

Update packages/aws-cdk-lib/aws-ec2/lib/instance.ts

90792c4

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

fix: Incorporate review comments

3595648

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 23, 2024

go-to-k suggested changes May 23, 2024

View reviewed changes

packages/aws-cdk-lib/aws-ec2/README.md Show resolved Hide resolved

Update packages/aws-cdk-lib/aws-ec2/README.md

3073387

Co-authored-by: k.goto (Kenta Goto) <24818752+go-to-k@users.noreply.github.com>

go-to-k approved these changes May 23, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 23, 2024

shikha372 self-assigned this May 24, 2024

shikha372 reviewed May 25, 2024

View reviewed changes

packages/aws-cdk-lib/aws-ec2/lib/instance.ts Show resolved Hide resolved

shikha372 reviewed May 25, 2024

View reviewed changes

fix: Incorporate review comments

981b1c3

mazyu36 added 3 commits May 25, 2024 13:49

fix: update integ-test

0d71e3c

Merge branch 'main' into ec2-enclave-hibernation

c976d0e

Update README.md

6460648

docs: update README

f134309

shikha372 previously requested changes May 31, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 31, 2024

github-actions bot mentioned this pull request Jun 1, 2024

Monthly PR metrics report #30414

Open

fix: update integ test

2a5b073

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jun 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ec2): add nitro enclave and hibernation settings to the Instance Construct #30228

feat(ec2): add nitro enclave and hibernation settings to the Instance Construct #30228

mazyu36 commented May 16, 2024

go-to-k left a comment

go-to-k May 23, 2024

mazyu36 May 23, 2024

go-to-k May 23, 2024

mazyu36 May 23, 2024

go-to-k May 23, 2024

mazyu36 May 23, 2024

go-to-k May 23, 2024

mazyu36 May 23, 2024

mazyu36 commented May 23, 2024

go-to-k left a comment •

edited

go-to-k left a comment

shikha372 May 25, 2024

mazyu36 May 25, 2024

shikha372 May 31, 2024

mazyu36 May 31, 2024

shikha372 May 31, 2024

mazyu36 Jun 1, 2024 •

edited

shikha372 commented May 25, 2024

mazyu36 commented May 25, 2024

shikha372 commented May 31, 2024

mazyu36 commented May 31, 2024

shikha372 May 31, 2024

mazyu36 Jun 1, 2024

aws-cdk-automation commented Jun 1, 2024

mazyu36 commented Jun 1, 2024

		});

		test('throw if AWS Nitro Enclaves and hibernation are enabled', () => {

feat(ec2): add nitro enclave and hibernation settings to the Instance Construct #30228

Are you sure you want to change the base?

feat(ec2): add nitro enclave and hibernation settings to the Instance Construct #30228

Conversation

mazyu36 commented May 16, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

go-to-k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazyu36 commented May 23, 2024

go-to-k left a comment • edited

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazyu36 Jun 1, 2024 • edited

Choose a reason for hiding this comment

shikha372 commented May 25, 2024

mazyu36 commented May 25, 2024

shikha372 commented May 31, 2024

mazyu36 commented May 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 1, 2024

AWS CodeBuild CI Report

mazyu36 commented Jun 1, 2024

go-to-k left a comment •

edited

mazyu36 Jun 1, 2024 •

edited