fix: simplify props for GuClassicLoadBalancer
#362
Conversation
Removes the `propertiesToRemove` prop. It looks like we were using this to set the scheme on the load balancer, however this is already being driven by the `internetFacing` prop provided by AWS. For a scheme of "internal", set `internetFacing` to `false`. For a scheme of "internet-facing", set `internetFacing` to `true`. See: - https://github.com/aws/aws-cdk/blob/fb6512314db1b11fc608cd62753090684ad0d3c4/packages/%40aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts#L249 - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-elb.html#cfn-ec2-elb-scheme
There was a problem hiding this comment.
Seems sensible to me 👍
If @aws/cdk adds this property by default, perhaps we should mention it here (I think it's optional in CFN, so many stacks may not have this value explicitly set).
This change is part of my bigger plans to simplify the load balancing constructs. That is, 👍🏽 to documentation, however I'd be tempted to delay it once the other load balancing changes have shipped too. The theme of the wider changes is to create a set of custom props, rather than extending AWS's. This should make it easier to set defaults and also to understand what those defaults are. |
|
🎉 This PR is included in version 6.2.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What does this change?
Removes the
propertiesToRemoveprop. It looks like we were using this to set the scheme on the load balancer, however this is already being driven by theinternetFacingprop provided by AWS.For a scheme of "internal", set
internetFacingtofalse. For a scheme of "internet-facing", setinternetFacingtotrue.See:
Does this change require changes to existing projects or CDK CLI?
Yes - the props to
GuClassicLoadBalanceris changing, withpropertiesToRemovedisappearing, so stacks would need to migrate. The migration is just setting a boolean however, so not too bad.How to test
n/a
How can we measure success?
A simpler API to the construct.
Have we considered potential risks?
n/a