provider/openstack: implement neutron security groups and rules #6410

cristicalin · 2016-04-29T03:13:08Z

This WIP patch adds support for neutron security groups and
security group rules. This requires a newer version of the
github.com/rackspace/gophercloud (i built and tested against
current master)

Unlike the nova security group, the neutron security group
is a separate entity from the security group rules, they
are created separately and have different identifiers.

An added benefit is that the neutron implementation allows
an admin to target a specific tenant. In this case you can
have a stack that also provisions the security side of a tenant.

cristicalin · 2016-04-29T03:17:32Z

The build fails because of an older version of github.com/rackspace/gophercloud in the vendor folder.
I haven't included the updated dependency in this initial pull request as I only submit it for initial comments.

jtopjian · 2016-04-29T03:42:17Z

builtin/providers/openstack/resource_openstack_networking_secgroup_rule_v2.go

+	d.Set("remote_group_id", security_group_rule.RemoteGroupID)
+	d.Set("remote_ip_prefix", security_group_rule.RemoteIPPrefix)
+	d.Set("tenant_id", security_group_rule.TenantID)
+	return nil


For the above attributes to be Set, they have to be marked as "computed" in the argument/schema definition. Arguments that are required don't have to be set.

jtopjian · 2016-04-29T03:46:15Z

@cristicalin This is a welcomed addition!

I've left two comments about some areas to fix up, but in general, this is off to a great start. Let me know when you open a PR to Gophercloud and I can review it and (hopefully) help push it along.

Nice work! 😄

cristicalin · 2016-04-29T04:45:14Z

I've made the suggested changes.

PR to Gophercloud for what ?
The security/groups and security/rules are already in place in master we just need to pull them into terraform.
I will open a separate pull request to terraform for the refresh, from my tests the current master doesn't appear to cause any issues but I want do some more extensive tests next week to make sure nothing gets horribly or subtly broken.

I'll also work next week on proper unit/acceptance tests and add relevant info to the docs.

jtopjian · 2016-04-29T04:51:18Z

oh! I misunderstood. I thought the tests were failing because you were also patching Gophercloud and using a local dev version. The vendoring is only pulling in packages that are in use, which is why the Neutron security group stuff isn't included yet... Sorry about that!

All sounds good!

cristicalin · 2016-04-29T16:24:45Z

I managed to find the time today to figure out how to properly use godep to satisfy the dependencies.
I won't open a separate pull request for the group and rule deps as they can easily fit into the current PR.

cristicalin · 2016-04-30T02:32:19Z

@jtopjian this should be now in a good shape to merge, please have a look when you can. I'll be happy to make any modifications you suggest.

jtopjian · 2016-05-01T16:04:38Z

builtin/providers/openstack/resource_openstack_networking_secgroup_rule_v2.go

+			},
+			"ethertype": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,


Ethertype is required in Gophercloud.

According to http://developer.openstack.org/api-ref-networking-v2-ext.html it is not.
But looking at https://github.com/rackspace/gophercloud/blob/master/openstack/networking/v2/extensions/security/rules/requests.go#L113 It looks like the gophercloud code checks for a valid ethertype which cannot be void so I'll mark it as required.

jtopjian · 2016-05-01T16:24:05Z

@cristicalin Looking really good! I left a few comments, but I'd say they're minor compared to the rest of the work you've done 😄

One other comment is to not forget to modify website/source/layouts/openstack.erb to add the resources to the sidebar.

Let me know if you have any questions or if something sounds off -- I only quickly reviewed this on a Sunday morning.

jtopjian · 2016-05-01T16:33:40Z

Also, in preparation for merging into Terraform, can you squash everything into two commits:

provider/openstack: Neutron security group resources
vendor: Updating Gophercloud dependencies

These can wait until all reviewing is done -- just wanted to give you a heads up on the final step before this gets merged.

cristicalin · 2016-05-02T19:37:08Z

I've made the modifications you suggested except for checking the protocol values as they are already checked in the gophercloud code Create function (see my comment on that).

I'll squash the commits as requested when you give a green light for the code.

jtopjian · 2016-05-03T04:02:09Z

builtin/providers/openstack/resource_openstack_networking_secgroup_rule_v2.go

@@ -83,6 +88,16 @@ func resourceNetworkingSecGroupRuleV2Create(d *schema.ResourceData, meta interfa
 		return fmt.Errorf("Error creating OpenStack networking client: %s", err)
 	}

+	portRangeMin := d.Get("port_min_range").(string)


It looks like there were a few issues with my rough sketch of this. This should be d.Get("port_range_min").(int)

jtopjian · 2016-05-03T04:13:29Z

@cristicalin Just the one change I noted and we're good to go 😄

Import networking/v2/extensions/security/groups and networking/v2/extensions/security/rules from gophercloud. These will be used by two new resouces openstack_networking_secgroup_v2 and openstack_networking_secgroup_rule_v2.

cristicalin · 2016-05-03T08:40:30Z

I made the last modifications (also updated the docs about the relevant requirements) and squashed the commits into two.

this implements two new resource types: * openstack_networking_secgroup_v2 - create a neutron security group * openstack_networking_secgroup_rule_v2 - create a newutron security group rule Unlike their nova counterparts the neutron security groups allow a user to specify the target tenant_id allowing a cloud admin to create per tenant resources.

jtopjian · 2016-05-03T14:18:31Z

Thank you for your time and hard work with this!

ghost · 2020-04-25T02:22:50Z

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.

jtopjian added provider/openstack labels Apr 29, 2016

jtopjian reviewed Apr 29, 2016
View reviewed changes

cristicalin changed the title ~~[WIP] implement neutron security groups and rules~~ provider/openstack: implement neutron security groups and rules Apr 30, 2016

jtopjian reviewed May 1, 2016
View reviewed changes

jtopjian reviewed May 3, 2016
View reviewed changes

vendor: Updating Gophercloud dependencies

00c953f

Import networking/v2/extensions/security/groups and networking/v2/extensions/security/rules from gophercloud. These will be used by two new resouces openstack_networking_secgroup_v2 and openstack_networking_secgroup_rule_v2.

cristicalin force-pushed the neutron_security_groups_v1 branch from 5cc749a to de5c025 Compare May 3, 2016 08:12

cristicalin force-pushed the neutron_security_groups_v1 branch from de5c025 to 6fe8269 Compare May 3, 2016 09:19

jtopjian merged commit 8f2b6e8 into hashicorp:master May 3, 2016

jtopjian mentioned this pull request May 3, 2016

provider/openstack: Decouple Security Group Rules from Security Groups #3601

Closed

cristicalin deleted the neutron_security_groups_v1 branch May 3, 2016 14:40

cristicalin restored the neutron_security_groups_v1 branch May 3, 2016 15:00

bobtfish mentioned this pull request May 5, 2016

Rebase from hashicorp/terraform Yelp/terraform#15

Merged

cristicalin deleted the neutron_security_groups_v1 branch May 25, 2016 11:39

ghost locked and limited conversation to collaborators Apr 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/openstack: implement neutron security groups and rules #6410

provider/openstack: implement neutron security groups and rules #6410

cristicalin commented Apr 29, 2016

cristicalin commented Apr 29, 2016

jtopjian Apr 29, 2016

jtopjian commented Apr 29, 2016

cristicalin commented Apr 29, 2016

jtopjian commented Apr 29, 2016

cristicalin commented Apr 29, 2016

cristicalin commented Apr 30, 2016

jtopjian May 1, 2016

cristicalin May 2, 2016

jtopjian commented May 1, 2016

jtopjian commented May 1, 2016

cristicalin commented May 2, 2016

jtopjian May 3, 2016

jtopjian commented May 3, 2016

cristicalin commented May 3, 2016

jtopjian commented May 3, 2016

ghost commented Apr 25, 2020

provider/openstack: implement neutron security groups and rules #6410

provider/openstack: implement neutron security groups and rules #6410

Conversation

cristicalin commented Apr 29, 2016

cristicalin commented Apr 29, 2016

jtopjian Apr 29, 2016

Choose a reason for hiding this comment

jtopjian commented Apr 29, 2016

cristicalin commented Apr 29, 2016

jtopjian commented Apr 29, 2016

cristicalin commented Apr 29, 2016

cristicalin commented Apr 30, 2016

jtopjian May 1, 2016

Choose a reason for hiding this comment

cristicalin May 2, 2016

Choose a reason for hiding this comment

jtopjian commented May 1, 2016

jtopjian commented May 1, 2016

cristicalin commented May 2, 2016

jtopjian May 3, 2016

Choose a reason for hiding this comment

jtopjian commented May 3, 2016

cristicalin commented May 3, 2016

jtopjian commented May 3, 2016

ghost commented Apr 25, 2020