Adding VPN options for IKE version and IKE split connections. #4904

DaanHoogland · 2021-04-09T12:41:09Z

IKE version allows selecting ike (autoselect), ikev1, or ikev2.
Split connections gives an option of separating the first right subnet from the rest, and kicking out individual statements for each right subnet for better cross-compatibility.

update per PR suggestion

Fixes #3138

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

IKE version allows selecting ike (autoselect), ikev1, or ikev2. Split connections gives an option of separating the first right subnet from the rest, and kicking out individual statements for each right subnet for better cross-compatibility. update per PR suggestion

blueorangutan · 2021-04-09T14:03:45Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 375

blueorangutan · 2021-04-10T00:26:49Z

Trillian test result (tid-397)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35329 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4904-t397-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_migrate_VM_and_root_volume	`Error`	67.15	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	53.09	test_vm_life_cycle.py

rohityadavcloud · 2021-04-12T02:40:30Z

@blueorangutan package

blueorangutan · 2021-04-12T02:41:03Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-04-12T03:34:59Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 385

rohityadavcloud · 2021-04-12T03:36:58Z

@blueorangutan test matrix

blueorangutan · 2021-04-12T03:38:03Z

@rhtyd a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2021-04-12T14:13:52Z

Trillian test result (tid-407)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35811 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4904-t407-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_scale_vm	`Failure`	13.31	test_scale_vm.py

blueorangutan · 2021-04-12T14:34:37Z

Trillian test result (tid-408)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36876 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4904-t408-kvm-centos7.zip
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

rohityadavcloud · 2021-04-12T18:31:18Z

cc @ggoodrich-ipp

rohityadavcloud · 2021-04-12T18:31:40Z

@blueorangutan ui

blueorangutan · 2021-04-12T18:32:02Z

@rhtyd a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

blueorangutan · 2021-04-12T18:36:09Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4904 (SL-JID-95)

rohityadavcloud · 2021-04-15T07:25:17Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-04-15T07:26:03Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

nvazquez · 2021-04-15T16:21:46Z

@DaanHoogland can you please resolve the conflicts?

blueorangutan · 2021-04-15T17:53:36Z

Trillian test result (tid-447)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36336 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4904-t447-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestVAppsVM>:setup	`Error`	79.58	test_vm_life_cycle.py

rohityadavcloud · 2021-04-16T08:09:56Z

@blueorangutan package

blueorangutan · 2021-04-16T08:11:02Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-04-16T08:50:02Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 426

blueorangutan · 2021-04-19T17:29:34Z

Trillian test result (tid-480)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32794 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4904-t480-kvm-centos7.zip
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

rohityadavcloud

LGTM - didn't test - this needs manual testing of ikev1+2 based s2s vpn cc @Pearl1594

Pearl1594 · 2021-04-22T04:40:55Z

api/src/main/java/org/apache/cloudstack/api/command/user/vpn/UpdateVpnCustomerGatewayCmd.java


    public Boolean getEncap() { return encap; }

+    public boolean getSplitConnections() {


Update of VPN gateway results in an NPE if splitconnection isn't passed, as the return type is boolean (primitive). We can either change this to Boolean or do the null check here.

2021-04-22 04:23:54,483 DEBUG [c.c.a.ApiServlet] (qtp1233705144-17:ctx-f384e62b ctx-1b68af67) (logid:5c3e9c53) ===END=== 172.16.250.2 -- GET id=10c9ef13-e7ab-4c40-8c8c-becd750f990a&name=vt2-right-gw1&gateway=10.0.52.84&cidrlist=10.2.0.0%2F24&ipsecpsk=password&ikepolicy=aes128-sha1%3Bmodp1536&ikelifetime=86400&ikeversion=ike&esppolicy=aes128-sha1&esplifetime=3600&command=updateVpnCustomerGateway&response=json 2021-04-22 04:23:54,492 ERROR [c.c.a.ApiAsyncJobDispatcher] (API-Job-Executor-20:ctx-39e2f033 job-78) (logid:f3cdea9e) Unexpected exception while executing org.apache.cloudstack.api.command.user.vpn.UpdateVpnCustomerGatewayCmd java.lang.NullPointerException at org.apache.cloudstack.api.command.user.vpn.UpdateVpnCustomerGatewayCmd.getSplitConnections(UpdateVpnCustomerGatewayCmd.java:150) at com.cloud.network.vpn.Site2SiteVpnManagerImpl.updateCustomerGateway(Site2SiteVpnManagerImpl.java:482) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566)

I am changing this, I noticed one other occasion, for which the field is a basic type , so it won't give the NPE, I'd rather keep the interface the original author decided on so I'll do the null check in the method.

kricud · 2021-04-22T13:38:54Z

Did testing and found following things.

Problems:

Add VPN Customer Gateway > IKE Version > set:ikev2 > press:ok
GW is created and IKE Version is set to: ike
Workaround: edit VPN Customer Gateway > IKE Version > type:ikev2

Add VPN Customer Gateway > IKE Version > set:ikev1 > press:ok
GW is created and IKE Version is set to: ike
Workaround: edit VPN Customer Gateway > IKE Version > type:ikev1

Add VPN Customer Gateway > Edit VPN Customer Gateway > CIDR list > Command failed due to Internal Server Error
Workaround: Delete VPN Customer Gateway and create again with correct settings.

Suggestions:

Edit help text for Split Connections: Only for IKEv2, whether to split multiple right subnet CIDRs into multiple connection statements.
Source:https://wiki.strongswan.org/projects/strongswan/wiki/FAQ#Multiple-subnets-per-SA
Essence: If you use IKEv1, you need to be a roadwarrior and use the UNITY extension.

Add help text for IKE Version : Connections marked with ike will use IKEv2 when initiating, but accept any protocol version when responding.
Source:https://wiki.strongswan.org/projects/strongswan/wiki/connsection

rohityadavcloud · 2021-04-23T07:58:24Z

Thanks for sharing test results @kricud

Pearl1594 · 2021-04-26T05:04:57Z

ui/src/views/network/CreateVpnCustomerGateway.vue

-          esppolicy: esppolicy
+          esppolicy: esppolicy,
+          splitconnections: values.splitconnections,
+          ikeversion: values.ikeversion


Suggested change

ikeversion: values.ikeversion

ikeversion: values.ikeVersions

This causes ikeversion to be null at the time of creation of the customer gateway - as it isn't passed, hence defaults to ike

Pearl1594 · 2021-04-26T05:59:32Z

ui/src/config/section/network.js

          docHelp: 'adminguide/networking_and_traffic.html#updating-and-removing-a-vpn-customer-gateway',
          dataView: true,
-          args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'esppolicy', 'esplifetime', 'dpd', 'forceencap']
+          args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap']


Suggested change

args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap']

args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap'],

mapping: {

ikeversion: {

options: ['ike', 'ikev1', 'ikev2']

}

}

To list the ike versions during the update operation

rohityadavcloud · 2021-04-26T08:20:50Z

Closing, as the PR has moved to - #4953 (Pearl is not committer, unable to push changes on this PR).

DaanHoogland mentioned this pull request Apr 9, 2021

Adding VPN options for IKE version and IKE split connections. #4137

Closed

5 tasks

DaanHoogland requested a review from Pearl1594 April 9, 2021 12:45

DaanHoogland added this to the 4.15.1.0 milestone Apr 9, 2021

Merge branch '4.15' into split-connections

dad1269

rohityadavcloud assigned Pearl1594 Apr 16, 2021

rohityadavcloud requested review from Spaceman1984 and shwstppr April 16, 2021 10:47

rohityadavcloud approved these changes Apr 21, 2021

View reviewed changes

Pearl1594 reviewed Apr 22, 2021

View reviewed changes

basic/wrapper type guard

40c53fd

DaanHoogland mentioned this pull request Apr 23, 2021

WIP: Vault Integration #2850

Closed

12 tasks

rohityadavcloud requested a review from Pearl1594 April 24, 2021 13:26

Pearl1594 requested changes Apr 26, 2021

View reviewed changes

Pearl1594 reviewed Apr 26, 2021

View reviewed changes

rohityadavcloud closed this Apr 26, 2021

rohityadavcloud mentioned this pull request Apr 26, 2021

Adding VPN options for IKE version and IKE split connections #4953

Merged

12 tasks


		public Boolean getEncap() { return encap; }

		public boolean getSplitConnections() {

-          args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap']
+          args: ['name', 'gateway', 'cidrlist', 'ipsecpsk', 'ikepolicy', 'ikelifetime', 'ikeversion', 'esppolicy', 'esplifetime', 'dpd', 'splitconnections', 'forceencap'],
+          mapping: {
+            ikeversion: {
+              options: ['ike', 'ikev1', 'ikev2']
+            }
+          }

Adding VPN options for IKE version and IKE split connections. #4904

Adding VPN options for IKE version and IKE split connections. #4904

Uh oh!

Conversation

DaanHoogland commented Apr 9, 2021 • edited by nvazquez Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

blueorangutan commented Apr 9, 2021

Uh oh!

blueorangutan commented Apr 10, 2021

Uh oh!

rohityadavcloud commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

rohityadavcloud commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

rohityadavcloud commented Apr 12, 2021

Uh oh!

rohityadavcloud commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

blueorangutan commented Apr 12, 2021

Uh oh!

rohityadavcloud commented Apr 15, 2021

Uh oh!

blueorangutan commented Apr 15, 2021

Uh oh!

nvazquez commented Apr 15, 2021

Uh oh!

blueorangutan commented Apr 15, 2021

Uh oh!

rohityadavcloud commented Apr 16, 2021

Uh oh!

blueorangutan commented Apr 16, 2021

Uh oh!

blueorangutan commented Apr 16, 2021

Uh oh!

blueorangutan commented Apr 19, 2021

Uh oh!

rohityadavcloud left a comment

Choose a reason for hiding this comment

Uh oh!

Pearl1594 Apr 22, 2021

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kricud commented Apr 22, 2021

Uh oh!

rohityadavcloud commented Apr 23, 2021

Uh oh!

Pearl1594 Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

Pearl1594 Apr 26, 2021

Choose a reason for hiding this comment

Uh oh!

rohityadavcloud commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

DaanHoogland commented Apr 9, 2021 •

edited by nvazquez

Loading

DaanHoogland Apr 22, 2021 •

edited

Loading