Vpnaas: Create IKE policy #785
Conversation
|
Build succeeded.
|
|
Build succeeded.
|
|
@jtopjian This is ready for review |
There was a problem hiding this comment.
@simonre The usual rebase stuff. It might make sense to maybe hold off on so many pending PRs to cut down on the amount of rebasing? Up to you, though :)
I've left some comments inline, mostly about some fields which I don't think are valid for IKE policies - but let me know if I'm wrong.
| // EncapsulationMode is the encapsulation mode. | ||
| // A valid value is tunnel or transport. | ||
| // Default is tunnel. | ||
| EncapsulationMode EncapsulationMode `json:"encapsulation_mode,omitempty"` |
There was a problem hiding this comment.
It doesn't look like this is a valid field for IKE?
| "github.com/gophercloud/gophercloud/acceptance/tools" | ||
| ) | ||
|
|
||
| func TestPolicyCRUD(t *testing.T) { |
There was a problem hiding this comment.
As mentioned in the IPSec PR, this test name clashes so we'll want to change it.
| // TransformProtocol is the transform protocol. | ||
| // A valid value is ESP, AH, or AH- ESP. | ||
| // Default is ESP. | ||
| TransformProtocol TransformProtocol `json:"transform_protocol,omitempty"` |
There was a problem hiding this comment.
It doesn't look like this is a valid field, either?
| Phase1NegotiationMode string `json:"phase1_negotiation_mode"` | ||
|
|
||
| // IkeVersion is the IKE version. | ||
| IkeVersion string `json:"ike_version"` |
There was a problem hiding this comment.
Let's rename this to IKEVersion.
simonre
force-pushed
the
vpnaas-ikepolicy-create
branch
from
ba4f0a6
to
2f36a9e
Compare
|
Sorry about the invalid fields, that was a result of some sloppy copy/pasting of the fields from the IPSecPolicy Create function |
|
Build succeeded.
|
|
@jtopjian I've rebased this and addressed your comments. I think this is ready for re-review |
| @@ -63,6 +64,30 @@ func CreateIPSecPolicy(t *testing.T, client *gophercloud.ServiceClient) (*ipsecp | |||
| } | |||
|
|
|||
| t.Logf("Successfully created IPSec policy %s", policyName) | |||
| t.Logf("Successfully created IKE policy %s", policyName) | |||
There was a problem hiding this comment.
Really minor nit, but looks like this line should be removed.
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
…nit test for ike policy creation
simonre
force-pushed
the
vpnaas-ikepolicy-create
branch
from
4ef6102
to
5476bd1
Compare
|
@jtopjian The projectID field is returned by my working environment. I added it to the result and changed the unit tests. |
|
Build succeeded.
|
|
@simonre Thank you for checking into that! This looks good. Nice work on this. Thank you again for all of your patience and work on this suite. |
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#L312
API:
https://developer.openstack.org/api-ref/network/v2/#create-ike-policy