-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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: Use the GroupName in EC2-Classic security group #5184
provider/aws: Use the GroupName in EC2-Classic security group #5184
Conversation
3046200
to
0c3b159
Compare
rebased |
0c3b159
to
4c23103
Compare
Hey @ephemeralsnow – sorry for the silence here, I'm just returning from a leave. I like your implementation over mine in #4983, but in testing I've hit a snag. Consider this tf config: provider "aws" {
region = "us-east-1"
}
resource "aws_security_group" "other_web" {
name = "tf_other_acc_tests"
description = "Used in the terraform acceptance tests"
tags {
Name = "tf-acc-test"
}
}
resource "aws_security_group" "web" {
name = "terraform_acceptance_test_example"
description = "Used in the terraform acceptance tests"
ingress {
protocol = "tcp"
from_port = 80
to_port = 8000
security_groups = ["${aws_security_group.other_web.name}"]
}
ingress = {
protocol = "tcp"
from_port = 80
to_port = 80
self = true
security_groups = [
"${aws_elb.bar.source_security_group}",
#"amazon-elb/amazon-elb-sg",
]
}
tags {
Name = "tf-acc-test"
}
}
# Create a new load balancer
resource "aws_elb" "bar" {
name = "foobar-terraform-elb"
availability_zones = ["us-east-1d", "us-east-1c"]
listener {
instance_port = 8000
instance_protocol = "http"
lb_port = 80
lb_protocol = "http"
}
tags {
Name = "foobar-terraform-elb"
}
}
output "elb_sg" {
value = "${aws_elb.bar.source_security_group}"
} The important part there is the In your example in 4983 you're using the combination, though we need to support both. I'm curious if you have thoughts on how best to do that? My first inclination is to special case any time we get "amazon-elb-sg" as a security group, and automatically prepend "amazon-elb" to it. Honestly though, I don't use EC2 Classic that much so I'm curious if you had thoughts on that. |
Hey @catsby I tried to test the this change. (prepend the owner id)
In EC2-Classic, I think
In VPC, I think that there is no problem because it use the Please tell me if there is a place likely to be a problem something to the other. Thanks |
Hey @ephemeralsnow thanks for the reply. I agree and think this will work out, so I'm going to take your branch and merge in some of my tests from mine and post a new PR with the two combined, then likely merge that. Thanks for your help and work here! |
Please do not forget to fix the document. :D |
Fixes an issue where security groups would fail to update after applying an initial security_group, because we were improperly saving the id of the group and not the name (EC2 Classic only). This is a PR combining #4983 and #5184 . It's majority @ephemeralsnow's work.
Consolidated into #5533 |
Fixes an issue where security groups would fail to update after applying an initial security_group, because we were improperly saving the id of the group and not the name (EC2 Classic only). This is a PR combining hashicorp/terraform#4983 and hashicorp/terraform#5184 . It's majority @ephemeralsnow's work.
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. |
It is related to #4983
This PR is a fix of the problems that are caused by repeating the
terraform apply
.The fixes is the following.
amazon-elb/amazon-elb-sg
test.tf.json
Part of the
terraform.tfstate
that have been created by running theterraform apply
on v0.6.11.Part of the
terraform.tfstate
that have been created by running theterraform apply
on this PR.