-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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(eks-v2-alpha): prevent IAM role creation when node pools are empty #33780
base: main
Are you sure you want to change the base?
Conversation
…empty When node pools are disabled (by setting an empty array in nodePools), the IAM role will not be created, preventing deployment failures with the error 'When Compute Config nodeRoleArn is not null or empty, nodePool value(s) must be provided'. Fixes #33771
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request
. Additionally, if clarification is needed, add Clarification Request
to a comment.
✅ A exemption request has been requested. Please wait for a maintainer's review.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33780 +/- ##
=======================================
Coverage 82.38% 82.38%
=======================================
Files 120 120
Lines 6937 6937
Branches 1170 1170
=======================================
Hits 5715 5715
Misses 1119 1119
Partials 103 103
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Exemption Request for snapshots update |
Thanks @pahud for contributing this PR, i verified that this can be disabled through nodePool as an empty array from the doc |
Hi @shikha372 I was able to deploy this using import * as ec2 from 'aws-cdk-lib/aws-ec2';
import * as iam from 'aws-cdk-lib/aws-iam';
import { App, Stack, StackProps } from 'aws-cdk-lib';
import { KubectlV32Layer } from '@aws-cdk/lambda-layer-kubectl-v32';
import * as eks from '../lib';
import { Construct } from 'constructs';
import * as integ from '@aws-cdk/integ-tests-alpha';
export class EksAutoModeCluster extends Stack {
constructor(scope: Construct, id: string, props: StackProps) {
super(scope, id, props);
const vpc = ec2.Vpc.fromLookup(this, 'Vpc', { isDefault: true });
const mastersRole = new iam.Role(this, 'Role', {
assumedBy: new iam.AccountRootPrincipal(),
});
new eks.Cluster(this, 'hello-eks2', {
vpc,
mastersRole,
version: eks.KubernetesVersion.V1_32,
kubectlProviderOptions: {
kubectlLayer: new KubectlV32Layer(this, 'kubectl'),
},
defaultCapacityType: eks.DefaultCapacityType.AUTOMODE,
compute: {
nodePools: [],
},
});
}
}
const app = new App();
const env = {
account: process.env.CDK_DEFAULT_ACCOUNT,
region: process.env.CDK_DEFAULT_REGION,
};
const stack = new EksAutoModeCluster(app, 'eks-auto-mode-stack2', { env });
new integ.IntegTest(app, 'aws-cdk-eks-cluster-integ', {
testCases: [stack],
}); ![]() Do we still want to add a new integ test just for this as adding a new integ test for eks cluster would create additional time when we re-run all integ tests for EKS but I am pretty sure it deploys. |
When node pools are disabled (by setting an empty array in nodePools), the IAM role will not be created, preventing deployment failures with the error 'When Compute Config nodeRoleArn is not null or empty, nodePool value(s) must be provided.
Issue # (if applicable)
Fixes #33771
Reason for this change
When using EKS Auto Mode with empty node pools (by setting
nodePools: []
), the IAM role was still being created by the L2 construct, causing stack deployment failures. The AWS service returns an error stating that whennodeRoleArn
is provided, node pool values must also be provided.Description of changes
Modified the
computeConfig
section in theCfnCluster
resource to check ifnodePools
is empty before assigningnodeRoleArn
. IfnodePools
is empty,nodeRoleArn
will be set toundefined
to prevent the unnecessary creation of the IAM role.The change ensures that when users explicitly disable node pools by providing an empty array, the IAM role won't be created, allowing the cluster to be provisioned successfully.
Added a test case to verify that when node pools are empty:
Describe any new or updated permissions being added
No new or updated IAM permissions are being added. This change actually prevents the creation of an IAM role when it's not needed.
Description of how you validated changes
Added a new test case in
automode.test.ts
that verifies:nodeRoleArn
property is not included in the CloudFormation template when node pools are emptyThe test passes, confirming that our fix works as expected.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license