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

0.8.6 is not backwards compatible for OpenStack security groups #12102

Closed
Kiemes opened this issue Feb 20, 2017 · 6 comments · Fixed by #12119
Closed

0.8.6 is not backwards compatible for OpenStack security groups #12102

Kiemes opened this issue Feb 20, 2017 · 6 comments · Fixed by #12119
Labels

Comments

@Kiemes
Copy link

Kiemes commented Feb 20, 2017

Version: >=0.8.6

Affected Resource(s)

openstack_networking_secgroup_v2
openstack_networking_secgroup_rule_v2

Terraform Configuration Files

resource "openstack_networking_secgroup_v2" "secgroup" {
  region      = "CustomRegion"
  name        = "terraform-test-group"
}

Before 0.8.6 Terraform the default OpenStack behaviour was implemented. The created security group did contain two egress rules allowing all outgoing IPv4 and IPv6 traffic.

Executing the same script with >=0.8.6 creates an empty security group.

That means, you need to create two additional security group rules.

resource "openstack_networking_secgroup_rule_v2" "secgroup_rule_v4" {
  direction = "egress"
  ethertype = "IPv4"
  security_group_id = "${openstack_networking_secgroup_v2.secgroup.id}"
}

resource "openstack_networking_secgroup_rule_v2" "secgroup_rule_v6" {
  direction = "egress"
  ethertype = "IPv6"
  security_group_id = "${openstack_networking_secgroup_v2.secgroup.id}"
}

Executing this with >=0.8.6 creates again the expected security group with its rules.

Executing this with <0.8.6 breaks because the rules already exist.

Having a running system where the security group was created the old way, and running an update with the new version including the explicit egress rule fails.

It would be great, if the breaking change could be reverted since it is not backwards compatible as @jtopjian suggested.
Creating the security group entity is not breaking, but you need to introduce new egress rules which are breaking.

Alternatively to removing the change completely would be to introduce a flag in the security group resource. Without the flag, the sec group should still have the default egress rules. This would be backwards compatible. But people could specify the flag in order to get a "clean/empty/secure by default" security group as I guess this is what you wanted to achieve with the change.

Suggested example:

resource "openstack_networking_secgroup_v2" "secgroup" {
  region      = "CustomRegion"
  name        = "terraform-test-group"
  empty      = "true"
}

default of empty being false

@jtopjian
Copy link
Contributor

That means, you need to create two additional security group rules.

This is correct. As discussed in #9799, this feature was requested by three users.

Executing this with >=0.8.6 creates again the expected security group with its rules.
Executing this with <0.8.6 breaks because the rules already exist.

Just to make sure I'm understanding the problem described, you are running multiple versions of Terraform across a shared state / environment?

It would be great, if the breaking change could be reverted since it is not backwards compatible as @jtopjian suggested.

"backwards compatible", in this case, meant that existing infrastructure created with Terraform will continue to still work. Security Groups created with previous versions would have still had the default rules. New groups would need the rules explicitly created.

Running older versions of Terraform on infrastructure created by newer versions of Terraform is not a scenario considered. It usually isn't.

Alternatively to removing the change completely would be to introduce a flag in the security group resource. Without the flag, the sec group should still have the default egress rules.

This is certainly possible to do and the better approach to this problem. Thank you for suggesting it.

@sklevenz
Copy link

+1 for the flag

@Kiemes
Copy link
Author

Kiemes commented Feb 20, 2017

@jtopjian Thanks for the quick response.
I totally agree that there is a need for being able to create security groups which can be closed down more on the egress side. I was just not happy with the way it was implemented :)

What failed for us is the following situation:
We had a security group with default egress rules (open everything up). So in the terraform.tf file we did not specify egress rules. We applied this tf file with an old CLI (<0.8.6) and everything worked as expected.

Now we got an additional new OpenStack where we applied the same tf file but on our images, the CLI was updated to 0.8.6. This broke our CI which is running on the resources created with Terraform as the outgoing traffic was not allowed.

That's why we added the two egress rules explicitly to the tf file. Executing Terraform on the new OpenStack worked again.

But applying this modified tf file on the old, already existing OpenStacks, failed since it tried to create the two new egress rules as well. But of course they existed still from the first run.

I hope this makes our situation which we ran into a bit more clear.

This is certainly possible to do and the better approach to this problem. Thank you for suggesting it.

Can I deduct from your statement that you are going to put this on your backlog?
Would be great to have this.

@jtopjian
Copy link
Contributor

@Kiemes Thank you for the clarification. I do sympathize with the issue you ran into and I apologize for that.

Can I deduct from your statement that you are going to put this on your backlog?
Would be great to have this.

Yes, I'm currently working on it now. Just to make sure I don't run into a third case, I'm currently reviewing Neutron specs as well as Neutron drivers to fully understand the behavior of default rules.

@jtopjian
Copy link
Contributor

@Kiemes I've just submitted #12119 which should fix this. Please let me know if you have any questions or feedback. :)

@ghost
Copy link

ghost commented Apr 16, 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 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants