Skip to content

Enable SNI in SSVM#4588

Closed
olivierlemasle wants to merge 1 commit into
apache:4.15from
apalia:ssvm-sni
Closed

Enable SNI in SSVM#4588
olivierlemasle wants to merge 1 commit into
apache:4.15from
apalia:ssvm-sni

Conversation

@olivierlemasle

@olivierlemasle olivierlemasle commented Jan 15, 2021

Copy link
Copy Markdown
Contributor

Description

This PR enables SNI support in the SSVM. This is required to download templates from web servers with HTTPS and SNI (e.g. behind Cloudfront).

Fixes: #4587

NB: Disabling SNI in the SSVM was done originally in commit 1d45b75, back in 2014; the comment included "Java 1.7 - disable using SNI since copyTemplate doesnt work for SSL". However, registerTemplate already supported https, so I don't see why SNI was disabled.

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

N/A

How Has This Been Tested?

Manually tested (cf section "Temporary fix" in #4587)

@yadvr

yadvr commented Jan 19, 2021

Copy link
Copy Markdown
Member

I suppose we'll need to test copy template (one zone to another) with this change cc @shwstppr

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2550

@yadvr

yadvr commented Jan 21, 2021

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File

@yadvr yadvr requested review from shwstppr and yadvr January 25, 2021 07:48
@yadvr yadvr changed the base branch from master to 4.14 January 25, 2021 07:49
@yadvr yadvr changed the base branch from 4.14 to master January 25, 2021 07:49
@yadvr

yadvr commented Jan 25, 2021

Copy link
Copy Markdown
Member

@olivierlemasle should this apply for master/4.15 or do you want to change the base branch to 4.14?

@olivierlemasle

Copy link
Copy Markdown
Contributor Author

Hi @rhtyd, I'm ok to change the base branch to 4.14 and include it in 4.14.x.

@olivierlemasle olivierlemasle changed the base branch from master to 4.14 January 26, 2021 15:10
@olivierlemasle olivierlemasle changed the base branch from 4.14 to master January 26, 2021 15:15
@olivierlemasle olivierlemasle changed the base branch from master to 4.14 January 26, 2021 15:19
@shwstppr

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2597

@yadvr

yadvr commented Jan 27, 2021

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File

@shwstppr shwstppr left a comment

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.

Lgtm

@yadvr

yadvr commented Jan 28, 2021

Copy link
Copy Markdown
Member

@olivierlemasle have you tested if copy template across works with the fix?

Otherwise, it will require this manual test case - setup zone/env with SSL enable for systemvms, add another zone with cluster/hosts etc and try to use the copyTemplate API.

@yadvr

yadvr commented Feb 2, 2021

Copy link
Copy Markdown
Member

@shwstppr can you confirm manual testing? Thanks

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2635

@DaanHoogland DaanHoogland self-assigned this Feb 2, 2021
@DaanHoogland

Copy link
Copy Markdown
Contributor

@olivierlemasle have you tested if copy template across works with the fix?

Otherwise, it will require this manual test case - setup zone/env with SSL enable for systemvms, add another zone with cluster/hosts etc and try to use the copyTemplate API.

Good call, @rhtyd it fails:
image

@DaanHoogland

DaanHoogland commented Feb 2, 2021

Copy link
Copy Markdown
Contributor

cloudmonkey call:

(localcloud) SBCM5> > copy template id=a0a6f548-d14d-4f0c-bff5-4c3f3633d155 sourcezoneid=1f739580-43f3-4d93-828a-39f6c4d8847a destzoneid=e9a804d5-2555-4618-b6ed-d34683f6b15c 
{
  "template": {
    "account": "admin",
    "bits": 0,
    "checksum": "7824c3610f07a79b46ce83e5f34d49caddb3740746b74e37d26ff69ef33635b192e2511d88c902bb40981eb6ae3a211489b24c913232304be9dcd4f56454db27",
    "childtemplates": [],
    "created": "2021-02-02T13:44:17+0000",
    "crossZones": false,
    "details": {},
    "directdownload": false,
    "displaytext": "mini",
    "domain": "ROOT",
    "domainid": "27255132-6554-11eb-840f-1e0048013b28",
    "format": "OVA",
    "hypervisor": "VMware",
    "id": "a0a6f548-d14d-4f0c-bff5-4c3f3633d155",
    "isdynamicallyscalable": true,
    "isextractable": true,
    "isfeatured": true,
    "ispublic": true,
    "isready": false,
    "name": "mini",
    "ostypeid": "36f30ac6-6554-11eb-840f-1e0048013b28",
    "ostypename": "Linux 4.x Kernel (64-bit)",
    "passwordenabled": true,
    "requireshvm": true,
    "sshkeyenabled": false,
    "status": "PKIX path validation failed: java.security.cert.CertPathValidatorException: validity check failed",
    "tags": [],
    "templatetype": "USER",
    "url": "http://dl.openvm.eu/cloudstack/macchinina/x86_64/macchinina-vmware.ova",
    "zoneid": "e9a804d5-2555-4618-b6ed-d34683f6b15c",
    "zonename": "ref-trl-2282-k-M7-daan-hoogland"
  }
}

@DaanHoogland

Copy link
Copy Markdown
Contributor

@olivierlemasle please see above. I think the disabling was with good reason.

@shwstppr shwstppr left a comment

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.

Based on testing reported, this will need changes
Let's move this to 4.15.1 milestone

@DaanHoogland DaanHoogland left a comment

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.

@olivierlemasle , we'll probably need to postpone this due to the issues during testing. These need to be fixed.

@yadvr yadvr modified the milestones: 4.14.1.0, 4.15.1.0 Feb 4, 2021
@yadvr yadvr changed the base branch from 4.14 to 4.15 February 4, 2021 09:15
@yadvr

yadvr commented Feb 4, 2021

Copy link
Copy Markdown
Member

Moved to next milestone, needs several round of multi-zone testing.

@yadvr

yadvr commented Apr 7, 2021

Copy link
Copy Markdown
Member

@DaanHoogland @olivierlemasle are you working/testing this - should this be moved to 4.16?

@DaanHoogland

DaanHoogland commented Apr 7, 2021

Copy link
Copy Markdown
Contributor

No @rhtyd see my test results above. I'm 👎 on this for now, and don't know of the way forward.

@olivierlemasle

Copy link
Copy Markdown
Contributor Author

@rhtyd Sorry I did not have access to a multi-AZ CloudStack environment for the moment. @DaanHoogland's tests show that this enableSNIExtension=false was there on purpose, even if don't understand the reason and if it causes #4587...

I have to investigate further.

@yadvr yadvr modified the milestones: 4.15.1.0, 4.16.0.0 Apr 26, 2021
@yadvr

yadvr commented Jun 17, 2021

Copy link
Copy Markdown
Member

ping @olivierlemasle any updates, is this still relevant?

@olivierlemasle

Copy link
Copy Markdown
Contributor Author

The issue #4587 is still relevant.

However, my fix did cause a regression. This went out of my radar, but I'll definitely investigate and look for a proper fix.
I think we can close this PR, and I'll open a new one if/when I find a proper fix.

@DaanHoogland DaanHoogland removed their assignment Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Register template from URL does not work with HTTPS and SNI

5 participants