Skip to content

[WIP] FWaaSv2.0 Policy CRUD#1789

Closed
Elethiomel wants to merge 4 commits intogophercloud:masterfrom
Elethiomel:fwaasv2_policy
Closed

[WIP] FWaaSv2.0 Policy CRUD#1789
Elethiomel wants to merge 4 commits intogophercloud:masterfrom
Elethiomel:fwaasv2_policy

Conversation

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 3, 2019

Coverage Status

Coverage increased (+0.2%) to 77.162% when pulling 5f3e0f7 on Elethiomel:fwaasv2_policy into 6dad56f on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Dec 3, 2019

Build succeeded.

@Elethiomel
Copy link
Copy Markdown
Contributor Author

@jtopjian Here's the entire Policy CRUD one for you :)

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Elethiomel Nice work. Overall this is good.

I've left a few comments to review. In addition, we also need a doc.go file with examples on how to use the functions.

Please let me know if you have any questions.

Description *string `json:"description,omitempty"`
Shared *bool `json:"shared,omitempty"`
Audited *bool `json:"audited,omitempty"`
Rules []string `json:"firewall_rules,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to remove all rules from a policy? If so, this might need to be changed to *[]string{}. If this is possible to do, we should include an acceptance test and check that the returned policy's rules length is 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed with mercedes-benz@7efe16b, but the acceptance test is probably missing. will check this on #2057

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Dec 4, 2019

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Feb 27, 2020

Build failed.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Feb 28, 2020

Build succeeded.

@chrischdi
Copy link
Copy Markdown
Contributor

Hi @Elethiomel are you still into this?

@Elethiomel
Copy link
Copy Markdown
Contributor Author

Yes I hope to progress on it soon. I've had some other major projects in the way unfortunately

chrischdi pushed a commit to mercedes-benz/gophercloud that referenced this pull request Sep 25, 2020
chrischdi pushed a commit to mercedes-benz/gophercloud that referenced this pull request Sep 25, 2020
chrischdi pushed a commit to mercedes-benz/gophercloud that referenced this pull request Sep 25, 2020
schreibergeorg pushed a commit to mercedes-benz/gophercloud that referenced this pull request Nov 13, 2020
schreibergeorg pushed a commit to mercedes-benz/gophercloud that referenced this pull request Nov 13, 2020
schreibergeorg pushed a commit to mercedes-benz/gophercloud that referenced this pull request Nov 13, 2020
schreibergeorg pushed a commit to mercedes-benz/gophercloud that referenced this pull request Nov 13, 2020
@schreibergeorg schreibergeorg mentioned this pull request Nov 13, 2020
1 task
schreibergeorg added a commit to mercedes-benz/gophercloud that referenced this pull request Nov 16, 2020
jtopjian pushed a commit that referenced this pull request Dec 9, 2020
* FWaaSv2.0 Policy

* s/Router/Policy/g as per #1789 (comment)

* Acceptance test for AddRule as per #1789 (comment)

* Acceptance test for RemoveRule as per #1789 (comment)

* fwaas_v2: review fix (#1789 (comment)) from #1789

* openstack/networking/v2/extensions/fwaas_v2/rules/results.go fix firewall_policy_id

According https://github.com/openstack/neutron-fwaas/blob/stable/ussuri/neutron_fwaas/db/firewall/v2/firewall_db_v2.py#L609 openstack does return an array here.
The docs tell it is a string.

* fix goimports

* fwaas_v2: testing to remove all rules from firewall policy (#1789 (comment))

* fwaas_v2: extract result & test for policies.AddRule

* fwaas_v2: test for RemoveRule

* apply suggestions from #2057

Co-authored-by: Colin Fowler <elethiomel@gmail.com>
Co-authored-by: Colin Fowler <colin.fowler@yoti.com>
Co-authored-by: Schlotter, Christian <christian.schlotter@daimler.com>
@schreibergeorg
Copy link
Copy Markdown
Contributor

@Elethiomel #2057 is merged, I think you can close this one. Thank you :-)

@jtopjian jtopjian closed this Dec 11, 2020
@jtopjian
Copy link
Copy Markdown
Contributor

Thank you both for your work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants