Skip to content

Conversation

@matoom-nomu
Copy link
Contributor

@matoom-nomu matoom-nomu commented Dec 22, 2025

Issue # (if applicable)

Closes #36460

Reason for this change

AWS EKS supports deletion protection to prevent accidental cluster deletion, as documented in the CloudFormation reference.

Description of changes

  • Add deletionProtection optional property to ClusterProps interface
  • Update README with usage example

Describe any new or updated permissions being added

None

Description of how you validated changes

Added unit/integ tests.

Checklist


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

@github-actions github-actions bot added feature-request A feature should be added or improved. p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK labels Dec 22, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team December 22, 2025 08:16
@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.


TestsPassed ☑️SkippedFailed ❌️
Security Guardian Results100 ran99 passed1 failed
TestResult
Security Guardian Results
packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.eks-cluster-deletion-protection.js.snapshot/aws-cdk-eks-cluster-deletion-protection.template.json
iam-no-world-accessible-trust-policy.guard❌ failure

@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.


TestsPassed ☑️SkippedFailed ❌️
Security Guardian Results with resolved templates100 ran97 passed3 failed
TestResult
Security Guardian Results with resolved templates
packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.eks-cluster-deletion-protection.js.snapshot/aws-cdk-eks-cluster-deletion-protection.template.json
codepipeline-cross-account-role-trust-scope.guard❌ failure
guardhooks-no-root-principals-except-kms-secrets.guard❌ failure
iam-role-no-broad-principals.guard❌ failure

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 22, 2025
Copy link
Contributor

@badmintoncryer badmintoncryer left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!! I've added some comments.

Comment on lines 3967 to 3987
test.each([
[true, true],
[false, false],
])('deletionProtection(%s) should work', (input, expected) => {
// GIVEN
const { stack } = testFixture();

// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
deletionProtection: input,
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
deletionProtection: expected,
},
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

You can describe test conditions more simply.

Suggested change
test.each([
[true, true],
[false, false],
])('deletionProtection(%s) should work', (input, expected) => {
// GIVEN
const { stack } = testFixture();
// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
deletionProtection: input,
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
});
// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
deletionProtection: expected,
},
});
});
test.each([
true, false
])('deletionProtection(%s) should work', (deletionProtection) => {
// GIVEN
const { stack } = testFixture();
// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
deletionProtection,
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
});
// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
deletionProtection,
},
});
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've simplified the test descriptions as you suggested.

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: Match.not(Match.objectLike({
deletionProtection: Match.anyValue(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to use Match.absent()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right, updated it to use Match.absent().


new integ.IntegTest(app, 'aws-cdk-eks-cluster-deletion-protection-integ', {
testCases: [stack],
diffAssets: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Because false is the default setting, it may be better to remove this line.

Suggested change
diffAssets: false,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The integ-test file I referenced was using diffAssets: false,
but as you mentioned it's the default setting, so I removed it.
I'll be more careful about checking default values next time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you forgotten to push the modification?

Comment on lines 43 to 49
cdkCommandOptions: {
deploy: {
args: {
rollback: true,
},
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this condition needed? I think rollback: true is default setting for cdk deploy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

},
});

app.synth();
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is not needed.

Suggested change
app.synth();

* When true, deletion protection is enabled and the cluster cannot be deleted until protection is disabled.
* When false, the cluster can be deleted normally. This setting only applies to clusters in an active state.
*
* @default - none
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please describe the default value or behavior that happens if this property is not supplied?

Suggested change
* @default - none
// default value
* @default false
// default behaviour
* @default - deletion protection is disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed comment to use
@default - deletion protection is disabled instead of @default - none

* The current deletion protection setting for the cluster.
* When true, deletion protection is enabled and the cluster cannot be deleted until protection is disabled.
* When false, the cluster can be deleted normally. This setting only applies to clusters in an active state.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add @see section?

Suggested change
*
*
* @see https://docs.aws.amazon.com/eks/latest/userguide/deletion-protection.html
*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add the link to the documentation?

https://docs.aws.amazon.com/eks/latest/userguide/deletion-protection.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 30, 2025
@badmintoncryer
Copy link
Contributor

Could you please resolve build error?

aws-cdk-lib: /codebuild/output/src3139386751/src/actions-runner/_work/aws-cdk/aws-cdk/packages/aws-cdk-lib/aws-eks/lib/cluster.ts
aws-cdk-lib:   740:5  error  Trailing spaces not allowed  no-trailing-spaces
aws-cdk-lib:   742:5  error  Trailing spaces not allowed  no-trailing-spaces
aws-cdk-lib: /codebuild/output/src3139386751/src/actions-runner/_work/aws-cdk/aws-cdk/packages/aws-cdk-lib/aws-eks/test/cluster.test.ts
aws-cdk-lib:   3968:16  error  Missing trailing comma  @stylistic/comma-dangle

@matoom-nomu
Copy link
Contributor Author

@badmintoncryer

Could you please resolve build error?

aws-cdk-lib: /codebuild/output/src3139386751/src/actions-runner/_work/aws-cdk/aws-cdk/packages/aws-cdk-lib/aws-eks/lib/cluster.ts
aws-cdk-lib:   740:5  error  Trailing spaces not allowed  no-trailing-spaces
aws-cdk-lib:   742:5  error  Trailing spaces not allowed  no-trailing-spaces
aws-cdk-lib: /codebuild/output/src3139386751/src/actions-runner/_work/aws-cdk/aws-cdk/packages/aws-cdk-lib/aws-eks/test/cluster.test.ts
aws-cdk-lib:   3968:16  error  Missing trailing comma  @stylistic/comma-dangle

Sorry for being late, I resoleved those linting errors.
Should i resolve security gurdian checks as well ?

@badmintoncryer
Copy link
Contributor

@matoom-nomu Thanks!

Should i resolve security gurdian checks as well ?

It may be not essential. Security guardian CI is introduced experimentally and I think it has no effect to review process.

Could you please address this comment? I'll approve this PR after that.

#36474 (comment)

Have a happy new year!!

Copy link
Contributor

@badmintoncryer badmintoncryer left a comment

Choose a reason for hiding this comment

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

Thanks!

@matoom-nomu
Copy link
Contributor Author

@matoom-nomu Thanks!
Could you please address this comment? I'll approve this PR after that.

#36474 (comment)

Have a happy new year!!

Thanks for the review, have a happy new year too!

And it seems there is a build error due to the year change....

@aws-cdk/mixins-preview: - [license/notice-file] NOTICE does NOT begin with AWS Cloud Development Kit (AWS CDK),Copyright 2018-2026 Amazon.com, Inc. or its affiliates. All Rights Reserved.,'

Copyright 2018-2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.

Shall i make the Issue ticket about this ?

@badmintoncryer
Copy link
Contributor

@matoom-nomu This is annual problem. How about submit PR like this?

#32705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-request A feature should be added or improved. p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aws-eks: support deletionProtection property

3 participants