Vpnaas: Create IPSec site connection #810
Conversation
| conn, err := CreateSiteConnection(t, client, ikepolicy.ID, ipsecpolicy.ID, service.ID, peerEPGroup.ID, localEPGroup.ID) | ||
| if err != nil { | ||
| t.Fatalf("Unable to create IPSec Site Connection: %v", err) | ||
| } |
There was a problem hiding this comment.
The resources here don't get deleted because they can't get deleted without deletion of the IPSec site connection. I will change this in a future PR as soon as it's possible to delete Site connections and endpoint groups
There was a problem hiding this comment.
Yep - that's the way to go here :)
|
Build succeeded.
|
|
@jtopjian This is ready for review |
simonre
changed the title
|
|
||
| // The route mode. | ||
| // A valid value is static, which is the default. | ||
| RouteMode string `json:"route_mode,omitempty"` |
There was a problem hiding this comment.
According to https://github.com/openstack/neutron-vpnaas/blob/master/neutron_vpnaas/extensions/vpnaas.py#L273-L275, it's not possible to POST route_mode. https://github.com/openstack/neutron-vpnaas/blob/058469e1b99b647537a5228c6a384d93df5484df/neutron_vpnaas/db/vpn/vpn_db.py#L177 also shows it being hard-coded. Perhaps this a read-only (results.go) field?
|
|
||
| // The authentication mode. | ||
| // A valid value is psk, which is the default. | ||
| AuthenticationMode string `json:"auth_mode,omitempty"` |
There was a problem hiding this comment.
auth_mode looks similar to route_mode in that it's not possible to POST with it:
| DPD DPD `json:"dpd"` | ||
|
|
||
| // AuthenticationMode is the authentication mode. | ||
| AuthenticationMode string `json:"auth_mode"` |
There was a problem hiding this comment.
Kind of minor, but let's change this to AuthMode to match the JSON string.
| conn, err := CreateSiteConnection(t, client, ikepolicy.ID, ipsecpolicy.ID, service.ID, peerEPGroup.ID, localEPGroup.ID) | ||
| if err != nil { | ||
| t.Fatalf("Unable to create IPSec Site Connection: %v", err) | ||
| } |
There was a problem hiding this comment.
Yep - that's the way to go here :)
| // A valid value is response-only or bi-directional. Default is bi-directional. | ||
| Initiator Initiator `json:"initiator,omitempty"` | ||
|
|
||
| // (Deprecated) Unique list of valid peer private CIDRs in the form < net_address > / < prefix > . |
There was a problem hiding this comment.
I recommend removing the (Deprecated) part here unless you can give further information about the deprecation (ie: when will it be removed and what is the recommended alternative).
I don't see any notes in the Python code mentioning deprecation. It's always possible the API doc is out of date and maybe this field is no longer deprecated?
…'(Deprecated)' in comment
simonre
force-pushed
the
vpnaas-ipsecsiteconn-create
branch
from
26e2a5e
to
ff51bd5
Compare
|
Build succeeded.
|
|
@jtopjian This is ready for review again |
|
LGTM! Nice work on this :) |
For #723
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://github.com/openstack/neutron-vpnaas/blob/058469e1b99b647537a5228c6a384d93df5484df/neutron_vpnaas/db/vpn/vpn_db.py#L150
API:
https://developer.openstack.org/api-ref/network/v2/#create-ipsec-connection