OpenStack Firewall as a Service #342

ggiamarchi · 2015-01-27T01:05:49Z

To-do list

Firewall rules
- CRUD implementation
- Unit tests
- Acceptance tests
Firewall policies
- CRUD implementation
- Insert and remove rules implementation
- Unit tests
- Acceptance tests
Firewalls
- CRUD implementation
- Unit tests
- Acceptance tests

Waiting for rackspace/gophercloud#342

ggiamarchi · 2015-02-09T01:38:11Z

This one is ready for review.

jrperritt · 2015-02-10T02:55:17Z

Ok, great! I'll review and test tomorrow morning.

jrperritt · 2015-02-11T01:03:22Z

openstack/networking/v2/extensions/fwaas/firewalls/requests.go

+	TenantID     string
+	Name         string
+	Description  string
+	AdminStateUp *bool


We'll probably want to create types for the *bool. For instance, here maybe variables named Up and Down:

type AdminState *bool var ( up bool = true Up AdminState = &up down bool = false Down AdminState = &down )

Then users could supply them like:
opts := firewalls.CreateOpts{ Name: "fwName", AdminStateUp: firewalls.Up}

Done in b2a5a9f

jrperritt · 2015-02-11T01:22:17Z

Hmm, I hit a snag: my devstack implementation doesn't support the FWaaS extension. Is there anyone else who can verify that the acceptance tests are passing?

Aside from that, this is looking really good; There are just some minor consistency things to fix.

jrperritt · 2015-02-12T01:32:15Z

Can you provide a link to the FWaaS documentation that you're working off of? I found this one, but it doesn't really describe the parameters (e.g. data type, required vs optional). I usually work off of this site, but it doesn't even mention FWaaS (which is largely why it isn't already in Gophercloud).

ggiamarchi · 2015-02-12T02:01:29Z

I don't use any other documentation. But the first one you mentioned, actually contains detail about attributes with types, descriptions and if it is required or not :

This documentation is globally correct but it contains some little mistakes.

ggiamarchi · 2015-02-14T01:44:22Z

@jrperritt Any update about the tests check of this PR ? If you don't have an OpenStack with FWaaS you can easily bootstrap a devstack with FWaaS up and running using this script on a fresh Ubuntu 14.04.

@julienvey @haklop Maybe you can run and validate acceptance tests ?

jrperritt · 2015-02-16T04:42:37Z

@ggiamarchi No, I can't run the tests; hopefully someone else can. Maybe @jtopjian has FWaaS available?

jrperritt · 2015-02-16T04:55:00Z

openstack/networking/v2/extensions/fwaas/rules/requests.go

+//
+// Default policy settings return only those firewall rules that are owned by the
+// tenant who submits the request, unless an admin user submits the request.
+func List(c *gophercloud.ServiceClient, opts ListOpts) pagination.Pager {


I didn't notice these the first time I reviewed this, but can we change all the Opts parameters to interfaces named OptsBuilder? Most the other packages should have examples of this (for an example, see here) .This will let other providers user theses functions with different Opts.

I actually forgot it because my first implementation was inspired by the LBaaS code that do not use OptsBuilder.

Done in 0059767.

jtopjian · 2015-02-16T05:29:25Z

Unfortunately I don't have FWaaS available. I'll check to see how difficult it would be to set it up on my Neutron test cloud later this week.

julienvey · 2015-02-16T11:57:47Z

@jtopjian @jrperritt I just ran the acceptance tests successfully on an OpenStack Cloud with FWaaS

=== RUN TestFirewall
--- PASS: TestFirewall (4.50s)
    policy_test.go:59: Created policy: &policies.CreateOpts{TenantID:"", Name:"", Description:"", Shared:(*bool)(nil), Audited:(*bool)(nil), Rules:[]string(nil)}
    firewall_test.go:56: Created firewall: &firewalls.CreateOpts{TenantID:"", Name:"gophercloud test", Description:"acceptance test", AdminStateUp:(*bool)(nil), Shared:(*bool)(nil), PolicyID:"9447e6cf-c51d-4714-afa9-53fec2669817"}
    firewall_test.go:86: Getting firewall ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:86: Getting firewall ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:69: Listing firewalls: ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:80: Updated firewall ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:86: Getting firewall ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:86: Getting firewall ID [d8e62745-c028-4898-b559-df236f264e94]
    firewall_test.go:93: Deleted firewall d8e62745-c028-4898-b559-df236f264e94
    policy_test.go:107: Deleted policy 9447e6cf-c51d-4714-afa9-53fec2669817
=== RUN TestFirewallPolicy
--- PASS: TestFirewallPolicy (1.34s)
    rule_test.go:47: Created rule: &rules.CreateOpts{Protocol:"tcp", Action:"allow", TenantID:"", Name:"", Description:"", IPVersion:0, SourceIPAddress:"", DestinationIPAddress:"", SourcePort:"", DestinationPort:"", Shared:(*bool)(nil), Enabled:(*bool)(nil)}
    policy_test.go:59: Created policy: &policies.CreateOpts{TenantID:"", Name:"gophercloud test", Description:"acceptance test", Shared:(*bool)(nil), Audited:(*bool)(nil), Rules:[]string{"1c552cce-63cb-4927-a29f-444eb84eff85"}}
    policy_test.go:72: Listing policies: ID [18d2f4e5-afdd-4c10-87ea-d35f38faf98c]
    policy_test.go:72: Listing policies: ID [31d6368b-3d8b-49a1-af5d-901283223801]
    policy_test.go:72: Listing policies: ID [4c0743e4-0361-4ed0-a6ce-2a8959d978a9]
    policy_test.go:72: Listing policies: ID [63337440-d0ad-4a3c-9e26-777a6e360119]
    policy_test.go:72: Listing policies: ID [6e3d5d6d-1356-43eb-91ef-d6f70888632d]
    policy_test.go:72: Listing policies: ID [89d7ef67-4cb8-485f-83ac-14085e616137]
    policy_test.go:72: Listing policies: ID [9971cc9f-ccbc-4b71-abfe-9fe88c4392a3]
    policy_test.go:72: Listing policies: ID [a0a3a1b1-d985-4507-b785-311f8e981fa6]
    policy_test.go:72: Listing policies: ID [afc19c5c-d136-4dbf-b614-c0b784a4f68d]
    policy_test.go:72: Listing policies: ID [e0f918c2-7a10-45d1-a5f9-9b2a2026bf3e]
    policy_test.go:72: Listing policies: ID [e2a5fb51-698c-4898-87e8-f1eee6b50919]
    policy_test.go:72: Listing policies: ID [f2b08c1e-aa81-4668-8ae1-1401bcb0576c]
    policy_test.go:83: Updated policy ID [9971cc9f-ccbc-4b71-abfe-9fe88c4392a3]
    policy_test.go:101: Getting policy ID [9971cc9f-ccbc-4b71-abfe-9fe88c4392a3]
    policy_test.go:89: Removed rule [1c552cce-63cb-4927-a29f-444eb84eff85] from policy ID [9971cc9f-ccbc-4b71-abfe-9fe88c4392a3]
    policy_test.go:95: Inserted rule [1c552cce-63cb-4927-a29f-444eb84eff85] into policy ID [9971cc9f-ccbc-4b71-abfe-9fe88c4392a3]
    policy_test.go:107: Deleted policy 9971cc9f-ccbc-4b71-abfe-9fe88c4392a3
    rule_test.go:83: Deleted rule 1c552cce-63cb-4927-a29f-444eb84eff85
=== RUN TestFirewallRules
2015/02/16 12:53:13 Making request:
"{\"firewall_rule\":{\"destination_ip_address\":\"192.168.1.0/24\",\"destination_port\":null,\"source_port\":\"1234\"}}"
--- PASS: TestFirewallRules (0.72s)
    rule_test.go:47: Created rule: &rules.CreateOpts{Protocol:"tcp", Action:"allow", TenantID:"", Name:"gophercloud_test", Description:"acceptance test", IPVersion:0, SourceIPAddress:"", DestinationIPAddress:"192.168.0.0/24", SourcePort:"", DestinationPort:"22", Shared:(*bool)(nil), Enabled:(*bool)(nil)}
    rule_test.go:60: Listing rules: ID [03d2a6ad-633f-431a-8463-4370d06a22c8]
    rule_test.go:60: Listing rules: ID [09bcd328-33cc-4167-9302-5f2468dbf9a4]
    rule_test.go:60: Listing rules: ID [0dfe4a54-25fc-4348-aefe-fbe091f4b8a0]
    rule_test.go:60: Listing rules: ID [0fc87832-96df-466e-ab1a-44d1f478eaad]
    rule_test.go:60: Listing rules: ID [2c8467aa-9585-41c1-9811-2e8b59645608]
    rule_test.go:60: Listing rules: ID [327a1477-1d88-4826-a881-ed8f56142a2e]
    rule_test.go:60: Listing rules: ID [350288f9-4aea-4e13-b434-91141e5e5960]
    rule_test.go:60: Listing rules: ID [3e5a4e53-4390-4b22-baea-75529dbeafb8]
    rule_test.go:60: Listing rules: ID [470c518f-880a-4b87-8ffb-d3abbff9d037]
    rule_test.go:60: Listing rules: ID [47dc7e49-9bec-4484-b3ef-18431414196a]
    rule_test.go:60: Listing rules: ID [53f5bf0a-05e3-4a30-800b-611151a9685d]
    rule_test.go:60: Listing rules: ID [6b777957-a236-44aa-abee-cb6402645cec]
    rule_test.go:60: Listing rules: ID [77e8adb5-3621-478e-abe6-d74566de236e]
    rule_test.go:60: Listing rules: ID [7aefe7f9-01f6-4387-900e-177684e3c59d]
    rule_test.go:60: Listing rules: ID [7b998345-1660-428f-969d-bfcdb4c0dbaf]
    rule_test.go:60: Listing rules: ID [80965cc0-2d60-48e8-8fe9-78be016f10b6]
    rule_test.go:60: Listing rules: ID [94129848-4eb8-4c8e-9164-cd8ee60faa49]
    rule_test.go:60: Listing rules: ID [a18ee08e-9d5d-4c9e-9372-c4b8f71c6c75]
    rule_test.go:60: Listing rules: ID [a6125a5a-25f2-4617-bcfd-599c604c3dd5]
    rule_test.go:60: Listing rules: ID [ae2cb604-1799-42dc-bcb1-b66d744e070f]
    rule_test.go:60: Listing rules: ID [b6017900-7811-4428-9940-a02033042c88]
    rule_test.go:60: Listing rules: ID [c854fab5-bdaf-4a86-9359-78de93e5df01]
    rule_test.go:60: Listing rules: ID [d9ea2584-5e6a-4058-930d-a7f78c31f6d5]
    rule_test.go:60: Listing rules: ID [dca10349-8125-4674-b350-15e9c5e9130b]
    rule_test.go:60: Listing rules: ID [e0649a2b-a327-45ed-a3a0-f068cb3fcf37]
    rule_test.go:60: Listing rules: ID [e07558b1-5e18-4d2d-a79b-0a95f36d1715]
    rule_test.go:60: Listing rules: ID [f03bd950-6c56-4f5e-a307-45967078f507]
    rule_test.go:71: Updated rule ID [09bcd328-33cc-4167-9302-5f2468dbf9a4]
    rule_test.go:77: Getting rule ID [09bcd328-33cc-4167-9302-5f2468dbf9a4]
    rule_test.go:83: Deleted rule 09bcd328-33cc-4167-9302-5f2468dbf9a4
PASS
ok      github.com/rackspace/gophercloud/acceptance/openstack/networking/v2/extensions/fwaas    6.576s

jtopjian · 2015-02-16T15:56:25Z

@julienvey Nice - thank you 😄

jrperritt · 2015-02-16T16:51:39Z

@julienvey Great, thank you.
@ggiamarchi Ok, tests are confirmed to be passing. I reviewed the PR again yesterday and made some additional comments.

The old way does not allow to handle updates correctly. When a nullable field is set and we want to remove the value we need to be able to set a null value in the json request body. For instance this happen in firewall rules for field source_ip_address (among others).

ggiamarchi · 2015-02-16T22:54:39Z

@jrperritt I have just pushed fixes for your comments.

jrperritt · 2015-02-16T22:57:00Z

Great, I'll have a look.

jrperritt · 2015-02-16T23:16:49Z

I think this is ready for merge. Great job, @ggiamarchi , and thank you for your patience :)

OpenStack Firewall as a Service

ggiamarchi · 2015-02-16T23:36:57Z

🎉 ✌️ 😸

ggiamarchi added a commit to haklop/terraform that referenced this pull request Feb 2, 2015

Use ggiamarchi/gophercloud fork

055bfec

Waiting for rackspace/gophercloud#342

ggiamarchi force-pushed the fwaas branch from 99c2331 to 918a5b8 Compare February 4, 2015 01:51

julienvey mentioned this pull request Feb 4, 2015

OpenStack Provider hashicorp/terraform#924

Merged

5 tasks

ggiamarchi force-pushed the fwaas branch 3 times, most recently from b015917 to 401a745 Compare February 9, 2015 01:35

ggiamarchi changed the title ~~[wip] OpenStack Firewall as a service~~ OpenStack Firewall as a Service Feb 9, 2015

ggiamarchi force-pushed the fwaas branch from 401a745 to 10f1c58 Compare February 9, 2015 12:51

ggiamarchi mentioned this pull request Feb 9, 2015

OpenStack Firewall as a Service jrperritt/terraform#3

Merged

3 tasks

jrperritt reviewed Feb 11, 2015
View reviewed changes

smashwilson mentioned this pull request Feb 13, 2015

Use client.Request instead of perigee #367

Merged

jrperritt reviewed Feb 16, 2015
View reviewed changes

ggiamarchi added 15 commits February 16, 2015 23:02

Support FWaaS - Rules

0e69514

Support FWaaS - Policies

506d150

Support FWaaS - Firewalls

9441b81

Firewall rules bugfixes & enhancements

68c49f8

Fix bugs & warnings

914b10f

Add mapstructure tags on Policy result

4645912

Add unit tests for firewall rules

b8dca24

Add unit tests for firewall policies

3e5b293

Add unit tests for firewalls

6d3d006

Add acceptance test for firewall rules

21a7267

Add acceptance test for firewall policies

0bf7cd3

Add acceptance test for firewalls

5571b6f

Use solid types for *bool attributes

092b45f

Error handling on struct to map mappings

ed2ed80

ggiamarchi force-pushed the fwaas branch from dca32b6 to f1f9800 Compare February 16, 2015 22:04

Change type *string to string for fields name & description

f150399

ggiamarchi force-pushed the fwaas branch from f1f9800 to f150399 Compare February 16, 2015 22:05

Introduce OptsBuilder for code style consistency

0059767

jrperritt added a commit that referenced this pull request Feb 16, 2015

Merge pull request #342 from ggiamarchi/fwaas

374de6c

OpenStack Firewall as a Service

jrperritt merged commit 374de6c into rackspace:master Feb 16, 2015

ggiamarchi deleted the fwaas branch February 16, 2015 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenStack Firewall as a Service #342

OpenStack Firewall as a Service #342

ggiamarchi commented Jan 27, 2015

ggiamarchi commented Feb 9, 2015

jrperritt commented Feb 10, 2015

jrperritt Feb 11, 2015

ggiamarchi Feb 11, 2015

ggiamarchi Feb 12, 2015

jrperritt commented Feb 11, 2015

jrperritt commented Feb 12, 2015

ggiamarchi commented Feb 12, 2015

ggiamarchi commented Feb 14, 2015

jrperritt commented Feb 16, 2015

jrperritt Feb 16, 2015

ggiamarchi Feb 16, 2015

jtopjian commented Feb 16, 2015

julienvey commented Feb 16, 2015

jtopjian commented Feb 16, 2015

jrperritt commented Feb 16, 2015

ggiamarchi commented Feb 16, 2015

jrperritt commented Feb 16, 2015

jrperritt commented Feb 16, 2015

ggiamarchi commented Feb 16, 2015

OpenStack Firewall as a Service #342

OpenStack Firewall as a Service #342

Conversation

ggiamarchi commented Jan 27, 2015

ggiamarchi commented Feb 9, 2015

jrperritt commented Feb 10, 2015

jrperritt Feb 11, 2015

Choose a reason for hiding this comment

ggiamarchi Feb 11, 2015

Choose a reason for hiding this comment

ggiamarchi Feb 12, 2015

Choose a reason for hiding this comment

jrperritt commented Feb 11, 2015

jrperritt commented Feb 12, 2015

ggiamarchi commented Feb 12, 2015

ggiamarchi commented Feb 14, 2015

jrperritt commented Feb 16, 2015

jrperritt Feb 16, 2015

Choose a reason for hiding this comment

ggiamarchi Feb 16, 2015

Choose a reason for hiding this comment

jtopjian commented Feb 16, 2015

julienvey commented Feb 16, 2015

jtopjian commented Feb 16, 2015

jrperritt commented Feb 16, 2015

ggiamarchi commented Feb 16, 2015

jrperritt commented Feb 16, 2015

jrperritt commented Feb 16, 2015

ggiamarchi commented Feb 16, 2015