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

provider/openstack: Add support for FWaaS Router Insertion #12488

Closed
fatmcgav opened this issue Mar 7, 2017 · 7 comments · Fixed by #12589
Closed

provider/openstack: Add support for FWaaS Router Insertion #12488

fatmcgav opened this issue Mar 7, 2017 · 7 comments · Fixed by #12589

Comments

@fatmcgav
Copy link
Contributor

fatmcgav commented Mar 7, 2017

@jtopjian I notice you've added support for the fwaasrouterinsertion FWaaS router extension into Gophercloud here.

Are you working on adding the functionality into Terraform?

If not, I'm happy to take a run at it...

@jtopjian
Copy link
Contributor

jtopjian commented Mar 7, 2017

Go for it :)

@fatmcgav
Copy link
Contributor Author

fatmcgav commented Mar 7, 2017

@jtopjian
Copy link
Contributor

jtopjian commented Mar 8, 2017

@fatmcgav Yeah, that's the first method I would try.

@fatmcgav
Copy link
Contributor Author

fatmcgav commented Mar 9, 2017

@jtopjian Damn, there was me thinking this would be nice and easy... :'(

So I'm struggling with the fact that we're nesting the firewalls.CreateOpts in FirewallCreateOpts in types.go.

My initial changes can be seen here: fatmcgav@5bccc78

The log output is here: https://gist.github.com/fatmcgav/2b4aa2fa221a128aa2aa3f02cfe3c893
As you can see on Line 10, we end up with a nested CreateOptsBuilder that I can't seem to work out how to remove :(

Thoughts?

Cheers
Gav

@fatmcgav
Copy link
Contributor Author

fatmcgav commented Mar 9, 2017

Hah, and just like that I took a second look at the code, and spotted that you've used opts.CreateOptsBuilder.ToFirewallMap() in Gophercloud.

Implemented the same in TF type, and the creation worked :D
fatmcgav@140b60c

Now to test updates...

@fatmcgav
Copy link
Contributor Author

fatmcgav commented Mar 10, 2017

@jtopjian OK, so I think I've got it all working...

In the end, I had to overload the Firewall type to add the RouterIDs field to get the reading to work.
This effectively removes the use of the routerinsertion package aswell.

Latest code is here: 865a2d8

Comments welcome.
Appreciate it needs a bit of a tidy up, remove some of the log lines and commented code, but would be good to have a quick review make sure I'm not doing anything too stupid... ;)

@ghost
Copy link

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

Successfully merging a pull request may close this issue.

2 participants