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

provider/aws: Add aws_alb_listener resource #8268

Merged
merged 2 commits into from
Aug 18, 2016

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented Aug 17, 2016

This commit adds the aws_alb_listener resource and associated acceptance tests and documentation.

This is part of a series of pull requests in order to implement #8137.

}

resource "aws_alb_listener" "front_end" {
load_balancer_arn = "${aws_alb.front_end.id}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize that in our case, the ID and the ARN of the ALB are the same, but what do you think about exposing the ARN on the ALB resource, and then referencing that here?

I think that, while it's kind of a minor detail for us, it makes more sense to a user use a config like this:

load_balancer_arn = "${aws_alb.front_end.arn}"

over this

load_balancer_arn = "${aws_alb.front_end.id}"

Copy link
Contributor

Choose a reason for hiding this comment

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

The same would be true for TargetGroup below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems fairly reasonable, I'll add it shortly.

@catsby
Copy link
Contributor

catsby commented Aug 17, 2016

I think for a UX perspective we should add and export the ARNs of this and aws_alb. Other than that and minor nits, this looks great!

This commit adds the `aws_alb_listener` resource and associated
acceptance tests and documentation.
This commit adds an `arn` field to `aws_alb` and `aws_alb_target_group`
resources, in order to present a more coherant user experience to people
using resource variables in fields suffixed "arn".
@jen20 jen20 force-pushed the f-aws-application-lb-listener branch from 00e4ef6 to e38d41b Compare August 18, 2016 17:56
@jen20 jen20 merged commit 56907d9 into master Aug 18, 2016
@jen20 jen20 deleted the f-aws-application-lb-listener branch August 18, 2016 20:18
@jen20 jen20 restored the f-aws-application-lb-listener branch August 19, 2016 12:07
@stack72 stack72 deleted the f-aws-application-lb-listener branch November 25, 2016 16:27
@ghost
Copy link

ghost commented Apr 19, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants