-
Notifications
You must be signed in to change notification settings - Fork 82
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
CDK v2.0 #79
Comments
Avoid CF deploy time errors when specifying multiple prefixes or suffixes in notification filters. Closes #3347 Possibly something to fix in v2.0 (#3398)
This comment has been minimized.
This comment has been minimized.
Done |
Could we define a comment tag to identify code that will become obsolete once these depreciations are removed? Something like: // TODO:v2.0.0 remove this block
legacy_method();
// TODO:v2.0.0 end remove this block EDIT: I used this kind of comment a few times in this PR: aws/aws-cdk@12d99eb |
Not sure if you want to go this route, but the Angular repo uses a tslint rule to manage this, and similarly to @nmussy's suggestion, you tag code like this:
This prevents CI from passing on a version-bump PR if the deletion-target has been met but the code block hasn't been removed |
* deprecate `EcsOptimizedAmi` for `EcsOptimizedImage` * constructor(props) replaced by strongly typed static methods * Windows AMI support * deprecate `EcsOptimizedAmi`, `EcsOptimizedAmiProps` * will require changes on v2 shipment #3398 Fixes #2574
I think this issue would need a major version bump. Moving a resource from a CR to a native CFN resource would most likely delete and recreate the resource. |
In this case it won't because the CR doesn't implement a |
Closing this in favor of #118 |
Proposal detailing a new major version of aws-cdk. Lays out the plan for rolling out the release and continuity for support of current v1 modules. **Note** Lets talk about issues marked as "More info needed" to decide if they should be included or not. I didn't quite understand what they were or why they required incurring breaking changes. Related to: #79
Proposal detailing a new major version of aws-cdk. Lays out the plan for rolling out the release and continuity for support of current v1 modules. **Note** Lets talk about issues marked as "More info needed" to decide if they should be included or not. I didn't quite understand what they were or why they required incurring breaking changes. Related to: #79
Proposal detailing a new major version of aws-cdk. Lays out the plan for rolling out the release and continuity for support of current v1 modules. **Note** Lets talk about issues marked as "More info needed" to decide if they should be included or not. I didn't quite understand what they were or why they required incurring breaking changes. Related to: #79
How can we make suggestions for things to consider for CDK v2.0? Open an issue? |
Comments here with the suggested issue are good. #79 |
One of AWS's core tenets is to be secure, and many constructs have defaults that are overly permissive. I propose changing construct defaults to be more aligned with AWS defaults, or to be secure by default. Two examples off the top of my head are: By default, VPC endpoints allow all traffic from the VPC (see the open parameter). When creating an endpoint in the console, by default no traffic is allowed. I would change this default to be "false". By default, NetworkLoadBalancedFargate service is fully open to the public internet (see the publicLoadBalancer parameter). I would change this to be false by default. |
One of the well-architected tenets is cost optimization (and there's the Amazon LP of Frugality), so can we try to avoid having CDK create unnecessary resources? Eg: By default, the L3 VPC construct creates an EIP for each AZ. The default limit is 5 EIPs. To deploy more than one VPC in the same region, you first need to request a quota increase. If you're using those VPCs for Lambda(s), then the EIPs will go unused. (And of course, EIPs are billed only if they are unused.) There seems to be no option in the L3 construct to not have EIPs for the VPC. In a multi-region, microservice architecture, this could easily end up being hundreds of unused EIPs. Suppose we have a distributed system that consists of 10 microservices, each in their own VPC. Suppose for each microservices, we deploy 1 test instance, and a production instance in 4 regions, with 3 AZs per VPC. Now, we have 150 unused EIPs which will cumulatively cost over $500 per month. It's possible to delete unnecessary resources (once you realize that they exist) using this obscure and undocumented method, but in this case, a better solution might be to have no EIPs by default or to have different L3 VPC constructs for different purposes (ie. create a LambdaVPC that encompasses best practices for running a Lambda VPC). I only have this one example, but I wouldn't have even noticed this example if I didn't run into the quota limit when I was trying to deploy multiple serverless applications to the same development account. It would be very easy for other instances of this problem to occur completely unnoticed until you dive deep into a bill at some point in the future. |
Adding aws/aws-cdk#3763 to the radar of 2.0 since it involves replacing existing deployed constructs. @SoManyHs |
Proposal detailing a new major version of aws-cdk. Lays out the plan for rolling out the release and continuity for support of current v1 modules. Related to: #79
Along the lines of what @flemjame-at-amazon said, it would be great if security-relevant properties either defaulted to a "most secure" setting, or, where the property may be availability or cost impacting, were required properties so developers could make a conscious decision about the setting. To give a concrete example:
It may not be clear to developers that this would default to unencrypted, or that |
Description
Proposal detailing how we release aws-cdk 2.0 and how we want to handle major versions in the future.
Things to consider
v2.0 Candidates
Required
Recommended
Not Recommended
addToPolicy
withaddTo{Principal,Identity}Policy
More Info Needed
Progress
The text was updated successfully, but these errors were encountered: