Skip to content
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

feat: Expose Load Balancer Scheme for EB Recipes #838

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

ashovlin
Copy link
Member

Issue #, if available: DOTNET-7553

Description of changes:

This adds a new option setting for "Load Balancer Scheme" to Linux and Windows Elastic Beanstalk recipes.
image

This is only visible when:

  • Deploying a Load Balanced application (as opposed to single instance)
  • Deploying into a VPC (otherwise it's implicitly "public")

This corresponds to the ELBScheme option in the aws:ec2:vpc namespace: https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/command-options-general.html#command-options-general-ec2vpc

Note: I didn't add an integration test for this because it requires valid VPC selections, and I don't believe we're covering every possible option. Let me know if you think we should add one.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ashovlin ashovlin requested review from normj, 96malhar and philasmar June 24, 2024 16:07
…oad Balancer when deploying an Elastic Beanstalk application into a VPC
@ashovlin ashovlin force-pushed the shovlia/eb-internal-lbs branch from 8fb4de8 to 0964d21 Compare June 24, 2024 16:12
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.57%. Comparing base (2c676af) to head (69845e4).

Additional details and impacted files
@@             Coverage Diff             @@
##              dev     #838       +/-   ##
===========================================
+ Coverage   32.41%   61.57%   +29.16%     
===========================================
  Files         272      278        +6     
  Lines       10746    10801       +55     
  Branches     1482     1492       +10     
===========================================
+ Hits         3483     6651     +3168     
+ Misses       6974     3612     -3362     
- Partials      289      538      +249     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

"Id": "LoadBalancerScheme",
"Name": "Load Balancer Scheme",
"Category": "LoadBalancer",
"Description": "Specify \"Internal\" if your application serves requests only from connected VPCs. Public load balancers serve requests from the Internet.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add quotes around "public" and make both "Internal" and "Public" lowercase to indicate what the actual values are.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, waiting for other reviewer's feedback before rework.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added quotes around "Public" via https://github.com/aws/aws-dotnet-deploy/compare/728ba2390e178d1ed44c2a832458ddcefee621e9..69845e4684d83f12f2ba998f5abb889dbbc86897

I left them capitalized since that's what we're showing in the UI (even though they're lowercase in CDK)
image

"Id": "LoadBalancerScheme",
"Name": "Load Balancer Scheme",
"Category": "LoadBalancer",
"Description": "Specify \"Internal\" if your application serves requests only from connected VPCs. Public load balancers serve requests from the Internet.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add quotes around "public" and make both "Internal" and "Public" lowercase to indicate what the actual values are.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashovlin ashovlin force-pushed the shovlia/eb-internal-lbs branch from 728ba23 to 69845e4 Compare July 2, 2024 15:00
@ashovlin ashovlin merged commit 538ee7d into dev Jul 2, 2024
11 checks passed
@ashovlin ashovlin deleted the shovlia/eb-internal-lbs branch July 2, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants