-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(eks): add support for EC2, HYBRID_LINUX, and HYPERPOD_LINUX access entry types #36350
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
Conversation
- Add `grantAccessWithType` method to EKS cluster for type-safe access entry management - Update `AccessEntry` class to support access entry type configuration - Add integration test for EKS grant access with type functionality - Update README documentation with new access entry type feature - Generate snapshot files for integration test validation - Enable fine-grained access control for EKS cluster users and roles
|
|
||||||||||||||
|
|
||||||||||||||
- Add concise README documentation for EC2, HYBRID_LINUX, and HYPERPOD_LINUX access entry types - Enhance JSDoc comments with detailed usage guidance and AWS service links - Document access policy constraints for non-STANDARD types - Update both stable and alpha packages consistently Addresses missing documentation for EKS Auto Mode, Hybrid Nodes, and SageMaker HyperPod integration.
…hanges - Update EKS grant access integration test snapshots with new asset hashes - Regenerate CloudFormation templates for cluster and kubectl providers - Update manifest and tree JSON files to reflect current state - Modify access entry implementation in both alpha and stable packages - Add test coverage for access entry type functionality - Update integration test to validate access entry type grants
- Add new asset snapshot for EKS grant access integration test - Reorganize asset directory structure with updated hash references - Update cfn-response.js, consts.js, and util.js asset files - Add new framework.js asset file to custom resource handler - Update CloudFormation template and assets manifest - Refresh tree.json and manifest.json for snapshot consistency - Remove obsolete asset files from previous snapshot version
…with accessEntryType
Add missing declare statements for 'cluster' and 'nodeRole' variables in the access entry type documentation example to fix Rosetta compilation.
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.
I think grants in general should now use Grant classes instead of using grantX() methods. See #36616
I don't this will be accepted by our current linters.
Edit: This only applies to new grants.
| * @returns {void} | ||
| */ | ||
| private addToAccessEntry(id: string, principal: string, policies: IAccessPolicy[]) { | ||
| private addToAccessEntry(id: string, principal: string, policies: IAccessPolicy[], accessEntryType?: AccessEntryType) { |
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.
Since this is a private method we can safely be able to change the API for this. We should change the arguments to take in a props argument. Something like:
| private addToAccessEntry(id: string, principal: string, policies: IAccessPolicy[], accessEntryType?: AccessEntryType) { | |
| interface AddAccessEntryOptions { | |
| id: string, | |
| principal: string, | |
| policies: IAccessPolicy[], | |
| accessEntryType?: AccessEntryType | |
| } | |
| private addToAccessEntry(props: AddAccessEntryOptions) { |
This would be helpful in case we need to add further arguments to the method; It will be easier to update in the future.
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.
Nice! Good catch! I'm on the way.
- Extract AddAccessEntryOptions interface for private addToAccessEntry method - Refactor addToAccessEntry to accept a props object instead of individual parameters - Update grantAccess to pass options object to addToAccessEntry - Apply changes to both stable and alpha EKS packages - Improves API stability for future method expansions
- Fix indentation from 3 spaces to 2 spaces for @MethodMetadata decorator - Apply fix to both stable and alpha EKS packages
- Fix closing */ comment indentation to match ESLint rules - Applied to both stable and alpha EKS packages
- Add GrantAccessOptions interface to provide extensible options - Update grantAccess method to use options object instead of direct accessEntryType parameter - Update unit tests to use new options object syntax - Apply changes to both stable and alpha EKS packages - Makes API more consistent and easier to extend in the future
- Add accessEntryType property to AccessEntry class to track entry type - Validate restricted access entry types (EC2, HYBRID_LINUX, HYPERPOD_LINUX) cannot have policies attached in addAccessPolicies() - Throw ValidationError with descriptive message when attempting to add policies to restricted types - Add comprehensive unit tests for both restricted and allowed access entry types - Apply changes to both aws-cdk-lib and aws-eks-v2-alpha packages - Prevents invalid configurations where certain access entry types should not have access policies
- Import Token from aws-cdk-lib/core in both access-entry.ts files - Add Token.isUnresolved() check before validating access policies length in constructor - Add Token.isUnresolved() check before validating access policies length in addAccessPolicies method - Prevent validation errors when access policies are defined as unresolved tokens (e.g., from Fn.importValue or other dynamic sources) - Apply changes to both @aws-cdk/aws-eks-v2-alpha and aws-cdk-lib packages for consistency
|
self-reviewed with no risks. |
| // Validate that restricted access entry types cannot have access policies | ||
| const restrictedTypes = [AccessEntryType.EC2, AccessEntryType.HYBRID_LINUX, AccessEntryType.HYPERPOD_LINUX]; | ||
| if (this.accessEntryType && restrictedTypes.includes(this.accessEntryType) && newAccessPolicies.length > 0) { | ||
| throw new ValidationError(`Access entry type '${this.accessEntryType}' cannot have access policies attached. Use AccessEntryType.STANDARD for access entries that require policies.`, this); | ||
| } |
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.
As this validation logic is repeated, it can be moved to a single re-usable private method which can be re-used here and in the constructor.
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.
Addressed.
| const restrictedTypes = [AccessEntryType.EC2, AccessEntryType.HYBRID_LINUX, AccessEntryType.HYPERPOD_LINUX]; | ||
| if (props.accessEntryType && restrictedTypes.includes(props.accessEntryType) && props.accessPolicies.length > 0) { | ||
| if (props.accessEntryType && restrictedTypes.includes(props.accessEntryType) && | ||
| !Token.isUnresolved(props.accessPolicies) && props.accessPolicies.length > 0) { |
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.
Good catch to check for tokens
- Extract duplicate validation logic into private validateAccessPoliciesForRestrictedTypes method - Remove repeated restricted types array and validation checks from constructor and addAccessPolicies - Add comprehensive JSDoc documentation for the new private method - Apply changes to both aws-eks and aws-eks-v2-alpha packages for consistency - Improves code maintainability and reduces duplication across validation points
- Add missing closing brace for validateAccessPolicies method - Fixes syntax error that prevented proper method termination - Ensures accessEntryRef getter is properly defined after validation logic
- Replace old AWS CLI layer asset with updated version - Update asset hashes and S3 object keys in CloudFormation template - Rename EC2 role and access entry resources for clarity - Remove HYBRID_LINUX and HYPERPOD access entry types from test - Update manifest and tree.json snapshots to reflect changes - Add ClusterClusterAdminRoleAccessF2BFF759 dependency to kubectl-ready output
|
Thank you for contributing! Your pull request will be automatically updated and merged (do not update manually, and be sure to allow changes to be pushed to your fork). |
- Separate type-only imports using `type` keyword in access-entry.ts files - Remove duplicate import statements in cluster.ts files - Consolidate imports from './access-entry' module with proper type separation - Apply consistent import organization pattern across aws-eks and aws-eks-v2-alpha packages - Align with established type-only import enforcement standards
|
Thank you for contributing! Your pull request will be automatically updated and merged (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status✅ The pull request has been merged at 29c3210 This pull request spent 39 minutes 43 seconds in the queue, including 28 minutes 30 seconds running CI. Required conditions to merge
|
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #34394.
Reason for this change
When provisioning an EKS cluster in Auto Mode with custom node roles, users need to grant the node role access to the cluster with the
EC2access entry type. Currently, thegrantAccess()method doesn't support specifying the access entry type, defaulting toSTANDARD, which prevents nodes from joining Auto Mode clusters.Additionally, the
AccessEntryTypeenum is missing three CloudFormation-supported types:EC2,HYBRID_LINUX, andHYPERPOD_LINUX.Description of changes
This PR adds support for specifying access entry types in the
grantAccess()method and extends theAccessEntryTypeenum with missing CloudFormation-supported values.Changes made:
AccessEntryTypeenum with three new values:EC2- For EKS Auto Mode node rolesHYBRID_LINUX- For EKS Hybrid NodesHYPERPOD_LINUX- For Amazon SageMaker HyperPodaccessEntryTypeparameter toCluster.grantAccess()methodaddToAccessEntry()method to pass type through to AccessEntry constructor@aws-cdk/aws-eks-v2-alpha(alpha) andaws-cdk-lib/aws-eks(stable) packagesAPI changes:
CloudFormation impact:
accessEntryTypeis provided:Typeproperty is set inAWS::EKS::AccessEntryaccessEntryTypeis not provided:Typeproperty remains undefined (backward compatible)No breaking changes: This is a fully backward-compatible feature addition. The new parameter is optional and placed at the end of the method signature. All existing code continues to work without modification.
Describe any new or updated permissions being added
N/A - No new IAM permissions required. This change only exposes existing CloudFormation access entry types through the CDK L2 API.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license