‼️ NOTICE: (aws-eks): Regression in installing Helm charts from assets #19179
Comments
oberoth
added
bug
github-actions
bot
added
the
@aws-cdk/aws-eks
otaviomacedo
changed the title
otaviomacedo
added
management/tracking
When the helm chart is specified by an asset instead of a respository URL, the `repository` variable in the custom resource is `None`. Adding a check before using the variable. This bug wasn't caught by integration tests because the existing integration test that exercises EKS clusters creates a cluster with many features, including two different helm charts: one from a URL and another one from an asset. This change includes a new integration test that exclusively deploys a cluster with a helm chart from an asset. Fixes #19179. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
One of our integration tests started failing after a [notice](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0308-cli-advisories.md) was added because `cdk version` displayed information besides the actual CLI version: ``` ● Two ways of shoing the version expect(received).toEqual(expected) // deep equality - Expected - 0 + Received + 14 2.14.0 (build 762d71b) + + NOTICES + + 19179 (aws-eks): Regression in installing Helm charts from assets + + Overview: Helm charts fail to install when provided as an asset. This + issue does not affect charts installed from a repository. + + Affected versions: framework: 2.14.0, framework: 1.146.0 + + More information at: #19179 + + + If you don’t want to see a notice anymore, use "cdk acknowledge <id>". For example, "cdk acknowledge 19179". at cli.integtest.ts:39:20 at ../helpers/cdk.ts:130:7 at ResourcePool.using (../helpers/resource-pool.ts:44:14) at ../helpers/test-helpers.ts:30:14 ``` This fixes that bug. Another option we could consider here is to keep the current behavior (where `cdk version` and `cdk --version` display different output) and just update the test case, but I think keeping them the same is probably simpler for customers. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
|
Please add your +1 👍 to let us know you have encountered this
Status: IN-PROGRESS
Overview:
Helm charts fail to install because the kubectl provider Lambda assumes 'repository' is non-null. This is not the case when a Helm chart is installed from an asset. Here's the newly introduced line that creates the regression: https://github.com/aws/aws-cdk/pull/18547/files#diff-8b2da7b95e6a38620688d9fd0f02cafb4374713386c3a50f9f9ee79369a77877R83 <- Error: 'NoneType' object has no attribute 'startswith'
Solution:
PR for the fix: #19180
Reproduction Steps
Install a Helm chart from an asset.
What did you expect to happen?
Helm chart to be installed successfully.
What actually happened?
Lambda threw an exception:
[ERROR] AttributeError: 'NoneType' object has no attribute 'startswith'
Traceback (most recent call last):
File "/var/task/index.py", line 17, in handler
return helm_handler(event, context)
File "/var/task/helm/init.py", line 83, in helm_handler
if repository.startswith('oci://'):
CDK CLI Version
2.12.0 (build c9786db)
Framework Version
No response
Node.js Version
v16.13.0
OS
Linux
Language
Typescript
Language Version
No response
Other information
No response
The text was updated successfully, but these errors were encountered: