-
Notifications
You must be signed in to change notification settings - Fork 4k
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
@aws-cdk/aws-eks: cluster.addHelmChart
doesn't pass SkipCrds
#31831
Labels
@aws-cdk/aws-eks
Related to Amazon Elastic Kubernetes Service
@aws-cdk/aws-lambda
Related to AWS Lambda
bug
This issue is a bug.
effort/small
Small work item – less than a day of effort
p1
Comments
1 task
cluster.addHelmChart
doesn't pass SkipCrds
cluster.addHelmChart
doesn't pass SkipCrds
I just read your PR. Yes, looks like that was a miss. Thank you for the report and PR. |
This was referenced Oct 28, 2024
Comments on closed issues and PRs are hard for our team to see. |
1 similar comment
Comments on closed issues and PRs are hard for our team to see. |
yashkh-amzn
pushed a commit
to yashkh-amzn/aws-cdk
that referenced
this issue
Feb 21, 2025
### Issue # (if applicable) Closes aws#31831. ### Reason for this change The `kubectl` custom resource lambda was not correctly passing the `skipCrds` parameter of `aws_eks.Cluster.addHelmChart` all the way through to the invocation of `helm`. As such, unexpected behaviour was observed where, even if `skipCrds` was `true`, `helm` running within the lambda would still attempt to install any relevant CRDs as part of the chart. ### Description of changes Adds the `skip_crds` argument to the call to `helm` within the `helm_handler` function. ### Description of how you validated changes I've run the unit tests and verified no changes. I cannot run the integration tests as I cannot afford the cost of deploying a full EKS cluster into my personal AWS account and I can't link this PR to our org's GitHub as per your guidelines, so I can't deploy into an account our org owns through CI. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
@aws-cdk/aws-eks
Related to Amazon Elastic Kubernetes Service
@aws-cdk/aws-lambda
Related to AWS Lambda
bug
This issue is a bug.
effort/small
Small work item – less than a day of effort
p1
Describe the bug
The kubectl custom resource handler has a special path for handling helm charts. This path has a property for
SkipCrds
, which should do as expected:aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/kubectl-handler/helm/__init__.py
Line 55 in 318eae6
The
helm
function also accepts askip_crds
argument:aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/kubectl-handler/helm/__init__.py
Line 161 in 318eae6
But the line that links the two together does not pass that argument, so it is impossible to tell
cluster.addHelmChart
not to install CRDs:aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/kubectl-handler/helm/__init__.py
Line 94 in 318eae6
Regression Issue
Last Known Working CDK Version
No response
Expected Behavior
Passing
skipCrds
tocluster.addHelmChart
should bypass installing CRDs via helm.Current Behavior
CRDs are still installed via helm even if
skipCrds
is given as an argument tocluster.addHelmChart
.Reproduction Steps
cluster.addHelmChart
to install a new helm chart that includes CRDs.skipCrds: true
as an argument toaddHelmChart
.Possible Solution
The source of the Python
kubectl-handler
lambda needs to be updated to correctly pass this argument.Additional Information/Context
No response
CDK CLI Version
2.162.1 (build 10aa526)
Framework Version
No response
Node.js Version
20.18.0
OS
MacOS Sequoia 15.0.1
Language
TypeScript
Language Version
TypeScript (5.6.3), Python 3.11
Other information
No response
The text was updated successfully, but these errors were encountered: