Skip to content

fix(eks-v2): respect securityGroup(s) in KubectlProviderOptions#37247

Open
letsgomeow wants to merge 5 commits intoaws:mainfrom
letsgomeow:feature/36653-new
Open

fix(eks-v2): respect securityGroup(s) in KubectlProviderOptions#37247
letsgomeow wants to merge 5 commits intoaws:mainfrom
letsgomeow:feature/36653-new

Conversation

@letsgomeow
Copy link

Issue # (if applicable)

Closes #36653.

Reason for this change

When specifying a securityGroup in KubectlProviderOptions, the
value was being ignored. The cluster's security group was always
applied to the kubectl handler (Lambda) instead of the user-specified
one.

The root cause was twofold:

  1. cluster.ts was not passing securityGroup through to the
    KubectlProvider constructor
  2. kubectl-provider.ts always fell back to
    props.cluster.clusterSecurityGroup regardless of whether
    props.securityGroup was set

Description of changes

kubectl-provider.ts

  • Fixed the security group resolution logic with the following
    priority order:
    1. securityGroups (new array property) — highest priority
    2. securityGroup (existing single property) — backwards compatible fallback
    3. clusterSecurityGroup — default (preserves existing behavior)
  • Added new securityGroups?: ec2.ISecurityGroup[] property to
    KubectlProviderOptions to support specifying multiple security
    groups (consistent with lambda.Function which accepts an array)
  • Emits a warning via Annotations.of(this).addWarningV2 when both
    securityGroup and securityGroups are specified simultaneously

cluster.ts

  • Pass securityGroup and securityGroups from
    _kubectlProviderOptions to the KubectlProvider constructor

Question for maintainers:
securityGroup and securityGroups use ec2.ISecurityGroup rather
than ec2.ISecurityGroupRef, which triggers an awslint warning
suppressed with [disable-awslint:prefer-ref-interface].
The reason is that the underlying lambda.Function expects
ISecurityGroup[], so converting from ISecurityGroupRef would add
unnecessary complexity. The existing securityGroup property also
uses ISecurityGroup for consistency.
Is this approach acceptable, or would you prefer a different solution?

Describe any new or updated permissions being added

No new IAM permissions are required.

Description of how you validated changes

  • Added 5 unit tests in cluster.test.ts covering:
    • securityGroups only (multiple security groups applied correctly)
    • securityGroup only (backwards compatibility)
    • Both specified (warning issued, securityGroups takes priority)
    • Neither specified (default clusterSecurityGroup used — existing behavior preserved)
    • Empty securityGroups array treated as unspecified
  • Added an integration test
    (integ.eks-kubectl-security-groups.ts) with a single stack
    containing one VPC and three EKS clusters, each demonstrating a
    different security group configuration

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 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Mar 14, 2026
@aws-cdk-automation aws-cdk-automation requested a review from a team March 14, 2026 07:10
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2026

⚠️ 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 Results48 ran48 passed
TestResult
No test annotations available

@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 2026

⚠️ 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 templates48 ran48 passed
TestResult
No test annotations available

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 14, 2026
@leonmk-aws leonmk-aws self-assigned this Mar 16, 2026
@leonmk-aws leonmk-aws added the pr/needs-integration-tests-deployment Requires the PR to deploy the integration test snapshots. label Mar 16, 2026
@leonmk-aws leonmk-aws deployed to deployment-integ-test March 16, 2026 10:42 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p1 pr/needs-integration-tests-deployment Requires the PR to deploy the integration test snapshots. pr/needs-maintainer-review This PR needs a review from a Core Team Member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(aws-eks-v2-alpha): Despite specifying SecurityGroups in KubectlProviderOptions, they are ignored and ClusterSecurityGroup is applied instead.

3 participants