Port fixes #114
Port fixes #114
Conversation
| noSecurityGroups := d.Get("no_security_groups").(bool) | ||
|
|
||
| // Check and make sure an invalid security group configuration wasn't given. | ||
| if noSecurityGroups && len(securityGroups) > 0 { |
There was a problem hiding this comment.
Is there a reason that 'ConflictsWith' won't work for this?
e.g. https://github.com/terraform-providers/terraform-provider-openstack/blob/b217557828d76c0ce6a7c207f7525be0cffc98fc/openstack/resource_openstack_fw_firewall_v1.go#L72
There was a problem hiding this comment.
Do you know if the following is possible?
resource "openstack_fw_firewall" "foo" {
no_routers = false
associated_routers = ...
}Would Terraform flag this since both are defined in the schema even though one is false?
There was a problem hiding this comment.
Looks like TF does flag it:
2 error(s) occurred:
- openstack_fw_firewall_v1.firewall_1a: "associated_routers": conflicts with no_routers (false)
- openstack_fw_firewall_v1.firewall_1a: "no_routers": conflicts with associated_routers ([]interface {}{"${openstack_networking_router_v2.tf_nc_test_rtr_1a.id}"})
Whether that's a beneficial behaviour or not, I'm not sure yet ;)
There was a problem hiding this comment.
Whether that's a beneficial behaviour or not, I'm not sure yet ;)
Yeah, that's where I'm at, too. The reasons I didn't use ConflictsWith were:
- Assumption that the above error happened (thanks for looking into that - I was just being lazy)
- Thinking of the workflow where a user would initially have
no_security_groups = true, then later addsecurity_group_idsand setno_security_groupstofalserather than remove it.
ConflictsWith continues to be the thing we want to use the most but never works out :)
There was a problem hiding this comment.
It almost feels like ConflictsWith should handle booleans' and only flag if not set to the Default value...
This commit updates how ports are handled within the networking v2 resources. This update is in response to a fixed bug in Gophercloud which changes how security groups and allowed address pairs are handled in a Networking Port. This commit introduces a new field called `no_security_groups`. If set to "true", then the port will contain no security groups. If `no_security_groups` is set to false and no `security_group_ids` are specified, then the Networking Service's default behavior will happen, which is usually to apply the "default" security group to the port. If `no_security_groups` and one or more `security_group_ids` are specified, then only the specified security groups will be applied to the port. At any point in time, a user can set `no_security_groups` to true and have all security groups removed. A computed field called `all_security_group_ids` has also been added. This field will contain all security groups that have been both explicitly and implicitly applied to the port. With Allowed Address Pairs, this commit fixes a bug where pairs were removed even if they were not supposed to be removed.
|
@fatmcgav I rebased with master, retested, and ran the entire CI test suite. All is passing. Unless you have any objections, let's plan to merge this tomorrow and we'll request a new release. We'll most likely have to hold off on 1.0.0 since this is a breaking change, but this is a fix we need to get in place. |
|
Hello, I tested this PR and I can confirm that it fixes the two issues I had:
Thks! |
|
@lhuard1A Thank you very much for testing 😄 |
@fatmcgav This fixes a bug I recently found in Gophercloud which explains some oddities we've seen with ports and allowed address pairs. Let me know if these changes check out with your cloud.
For #24