search

aws-quickstart / cdk-eks-blueprints

AWS Quick Start Team
Apache License 2.0
433 stars 191 forks source link

aws-cdk-lib hardcoded to a specific version #944

Open DeveloperC286 opened 4 months ago

DeveloperC286 commented 4 months ago

Describe the bug

The version of the dependency aws-cdk-lib in cdk-eks-blueprints is hardcoded to a specific version. Therefore, if there is a mismatch with the version of aws-cdk-lib of your project you encounter a TypeScript complication error.

Expected Behavior

That I am able to upgrade the version of aws-cdk-lib in my package.json and I do not get a complication error.

Current Behavior

If I change aws-cdk-lib in my package.json to a version not matching what cdk-eks-blueprints wants I get a complication error.

Reproduction Steps

Use "@aws-quickstart/eks-blueprints": "^1.13.1 with "aws-cdk-lib": "2.97.0" defined in your package.json.

Possible Solution

Additional Information/Context

No response

CDK CLI Version

N/A

EKS Blueprints Version

No response

Node.js Version

N/A

Environment details (OS name and version, etc.)

N/A

Other information

Related

DeveloperC286 commented 4 months ago

Thinking more about this, you actually do not even need to touch your package.json manually to get this complication errors. Your usage/installation commands will install @aws-quickstart/eks-blueprints with a caret(^) so it is not pinned. So if I do an npm update I could start getting complication errors.

DeveloperC286 commented 4 months ago

Not pinning the aws-cdk-lib version is something @aws-cdk/lambda-layer-kubectl-v* does e.g. https://github.com/cdklabs/awscdk-asset-kubectl/blob/kubectl-v29/main/package.json#L72

DeveloperC286 commented 4 months ago

I think an important thing to note about CDK users is that they may not come from a development background. Even with my development background, prior experience with CDK and basic Node/NPM knowledge this was still an hour of debugging for me to resolve it.

shapirov103 commented 4 months ago

@DeveloperC286 thank you for your feedback.

You can upgrade the version of CDK to any version without issues if you use the override functionality as shown here. We should document it better for sure.

However, your feedback is spot-on. Many users don't have dev background or background in node/npm and it created more issues than we expected.

The pinned version was a conscious choice, as we observed cases when upgrading CDK version broke blueprints functionality. It is probably time to reevaluate this as it was more common in the past.

DeveloperC286 commented 4 months ago

You can upgrade the version of CDK to any version without issues if you use the override functionality as shown here. We should document it better for sure.

Thank you for the link, I had found that solution from yourself in your replys to prior issues. Once I knew what the problem was it was simple to fix, but getting from a typescript error to the problem was the challenging part.

However, your feedback is spot-on. Many users don't have dev background or background in node/npm and it created more issues than we expected.

I was asked by someone without a development background for help, someone who is very proficient with AWS and CloudFormation and without my help this would have been a roadblocker for adopting cdk-eks-blueprints.

The pinned version was a conscious choice, as we observed cases when upgrading CDK version broke blueprints functionality. It is probably time to reevaluate this as it was more common in the past.

A combination to tackle both problems of making sure people do not use untested CDK versions and removing this dependency issue could be to use a version range e.g. accept 2.80 -> 2.110(or what ever versions you have tested).

shapirov103 commented 4 months ago

@DeveloperC286 1.14.0 release is out and should have the fix to address this issue. CDK depedency is moved to peerDependencies. You can read more on the behavior here.

This approach eliminates the situation when CDK version mismatch is causing a compile issue. It also makes it easier to change the version of CDK to the latest (with warning).

Please let me know if you have any feedback.

DeveloperC286 commented 3 months ago

@DeveloperC286 1.14.0 release is out and should have the fix to address this issue. CDK depedency is moved to peerDependencies. You can read more on the behavior here.

This approach eliminates the situation when CDK version mismatch is causing a compile issue. It also makes it easier to change the version of CDK to the latest (with warning).

Please let me know if you have any feedback.

Thank you, this is far easier now to know about the missing/mismatched CDK version rather than a mysterious complication error :tada:

My only comment would be it would be great if the version wasn't pinned to a specific version. Even a range of versions it has been tested against would be better.

shapirov103 commented 3 months ago

Atm, the pipeline is only testing against the version specified in the release. It seems that the peer dependency solution is a good indication of the version we tested against, but also allows setting any other CDK version if you need to. I can relax the pinned version to a range, but atm the team is only equipped to accept issues against the tested version as testing gets more and more complex with the proliferation of options and addons.