-
Notifications
You must be signed in to change notification settings - Fork 2
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
ALB as internal/internet facing 🥅 #12
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks straight forward gtg on that. Is there an intended terraform output or simply adding the variable? I ran terraform plan in the ecs directory and was prompted for a name of an App Mesh.
@shanice-skylight No output. If you'd like, we can pair tomorrow and we can go over the requirements to run the terraform. A tfvars file is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I left an optional consideration for your review, but that does not block this PR from going in.
@@ -31,6 +31,7 @@ | |||
|
|||
| Name | Description | Type | Default | Required | | |||
|------|-------------|------|---------|:--------:| | |||
| <a name="input_alb_internal"></a> [alb\_internal](#input\_alb\_internal) | Whether the ALB is public or private | `bool` | `true` | no | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking, for your consideration: the concepts of "internal" and "private" may be disjoint to certain end users or jurisdictions. You may want to consider buffing the description to include something like, "Whether the ALB is public (intended for external access) or private (only intended to be accessed within your AWS private cloud)."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll include these clarifications on the next PR!
@@ -56,6 +56,7 @@ No modules. | |||
|
|||
| Name | Description | Type | Default | Required | | |||
|------|-------------|------|---------|:--------:| | |||
| <a name="input_alb_internal"></a> [alb\_internal](#input\_alb\_internal) | Flag to determine if the ALB is public | `bool` | `true` | no | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment regarding the concepts of private vs internal.
@@ -1,3 +1,8 @@ | |||
variable "alb_internal" { | |||
type = bool | |||
description = "Flag to determine if the ALB is public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment regarding the concepts of private vs internal.
DEVOPS PULL REQUEST
Related Issue
Changes Proposed
Additional Information