Add support for default security group rules - #3978
Conversation
Add a new openstack/networking/v2/extensions/security/defaultrules package supporting the List, Create, Get and Delete operations of the Neutron /v2.0/default-security-group-rules API, provided by the security-groups-default-rules extension (Neutron 2023.2). The API does not support update nor bulk create for this resource. The package mirrors the existing security/rules package and reuses its RuleDirection, RuleProtocol and RuleEtherType constants. Unit tests and an acceptance test are included. For gophercloud#3175 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pierre-Samuel Le Stang <pierre-samuel.le-stang@ovhcloud.com>
github-actions
Bot
added
edit:networking
|
coverage: 63.638% (+0.07%) from 63.573% — pslestang:add-default-security-group-rules into gophercloud:main |
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Thanks @pslestang for working on this. Left a few comments, let me know what you think.
| UsedInDefaultSG *bool `q:"used_in_default_sg"` | ||
| UsedInNonDefaultSG *bool `q:"used_in_non_default_sg"` |
| // the API. Filtering is achieved by passing in struct field values that map to | ||
| // the default security group rule attributes you want to see returned. SortKey |
There was a problem hiding this comment.
This sentence about filtering confused me a little. Wouldn't be the idea of this something like: we pass field values for the attributes that we would like to use as filters.?
| th.AssertEquals(t, description, rule.Description) | ||
| th.AssertEquals(t, "ingress", rule.Direction) | ||
| th.AssertEquals(t, "IPv4", rule.EtherType) | ||
| th.AssertEquals(t, fromPort, rule.PortRangeMin) | ||
| th.AssertEquals(t, toPort, rule.PortRangeMax) | ||
| th.AssertEquals(t, string(rules.ProtocolTCP), rule.Protocol) | ||
| th.AssertEquals(t, "PARENT", rule.RemoteGroupID) | ||
| th.AssertEquals(t, true, rule.UsedInDefaultSG) | ||
| th.AssertEquals(t, false, rule.UsedInNonDefaultSG) |
There was a problem hiding this comment.
We usually do a GET for the created resource and then compare the returned value with the expected, instead of comparing with the result of a creation operation. Some APIs return different values for these operations and comparing with the GET result shows that in a N + 1 operation the resource state is as expected.
So IMO, you might want to move these checks after the defaultrules.Get and then compare with the getRule SG.
3 participants
Add a new openstack/networking/v2/extensions/security/defaultrules package supporting the List, Create, Get and Delete operations of the Neutron /v2.0/default-security-group-rules API, provided by the security-groups-default-rules extension (Neutron 2023.2). The API does not support update nor bulk create for this resource.
The package mirrors the existing security/rules package and reuses its RuleDirection, RuleProtocol and RuleEtherType constants. Unit tests and an acceptance test are included.
Fixes #3175
List security group default rules (GET /v2.0/default-security-group-rules):
https://docs.openstack.org/api-ref/network/v2/index.html#list-security-group-default-rules
Create security group default rule (POST /v2.0/default-security-group-rules):
https://docs.openstack.org/api-ref/network/v2/index.html#create-security-group-default-rule
Show security group default rule (GET /v2.0/default-security-group-rules/{id}):
https://docs.openstack.org/api-ref/network/v2/index.html#show-security-group-default-rule
Delete security group default rule (DELETE /v2.0/default-security-group-rules/{id}):
https://docs.openstack.org/api-ref/network/v2/index.html#delete-security-group-default-rule