Fix local upload from browser failing due to ssvm cert not trusted#13204
Fix local upload from browser failing due to ssvm cert not trusted#13204abh1sar wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13204 +/- ##
============================================
- Coverage 16.27% 16.26% -0.01%
+ Complexity 13440 13431 -9
============================================
Files 5665 5666 +1
Lines 500555 500628 +73
Branches 60789 60797 +8
============================================
- Hits 81445 81408 -37
- Misses 410004 410112 +108
- Partials 9106 9108 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a UI-side pre-check for SSVM reachability/certificate trust before starting “upload from local” flows, so browser-blocked requests (e.g., due to an expired/untrusted SSVM TLS cert) can be surfaced with user guidance instead of failing silently.
Changes:
- Introduces an SSVM “certificate not trusted” warning state with actions to open the SSVM origin and retry.
- Adds a lightweight
fetch()probe before initiating template/ISO/volume uploads to detect browser-level connection/TLS failures. - Adds new i18n strings for the warning and retry actions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| ui/src/views/storage/UploadLocalVolume.vue | Adds SSVM probe + warning UI and refactors upload to use an uploading state. |
| ui/src/views/image/RegisterOrUploadTemplate.vue | Adds SSVM probe + warning UI for local template uploads, with retry flow. |
| ui/src/views/image/RegisterOrUploadIso.vue | Adds SSVM probe + warning UI for local ISO uploads, with retry flow. |
| ui/public/locales/en.json | Adds UI strings for the SSVM warning and retry actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| await fetch(origin, { method: 'HEAD', mode: 'no-cors' }) | ||
| return true | ||
| } catch (e) { | ||
| return false |
| async probeSsvmCert (origin) { | ||
| try { | ||
| await fetch(origin, { method: 'HEAD', mode: 'no-cors' }) | ||
| return true | ||
| } catch (e) { | ||
| return false |
vishesh92
left a comment
vishesh92
left a comment
There was a problem hiding this comment.
clgtm. didn't test.
IMO, it would be better if we can extract the common code out.
|
@abh1sar please check the copilot's comments and see if they are valid. |
|
Have extracted the code out and answered review comments |
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
ui/src/views/image/RegisterOrUploadIso.vue:530
- The file count guard shows an error when
fileList.length > 1but does not return, so the upload proceeds anyway. Return early after the notification to avoid attempting a multi-file upload.
const { fileList } = this
if (this.fileList.length > 1) {
this.$notification.error({
message: this.$t('message.upload.iso.failed'),
description: this.$t('message.error.upload.iso.description'),
duration: 0
})
}
| <span v-if="uploading"> | ||
| <loading-outlined /> | ||
| {{ $t('message.upload.file.processing') }} | ||
| <a-progress :percent="uploadPercentage" /> | ||
| </span> |
| @@ -1124,12 +1157,18 @@ export default { | |||
| duration: 0 | |||
| }) | |||
| <a :href="ssvmOrigin" target="_blank" rel="noopener noreferrer"> | ||
| <a-button>{{ $t('label.ssvm.open.cert.page') }}</a-button> | ||
| </a> |
| <a :href="ssvmOrigin" target="_blank" rel="noopener noreferrer"> | ||
| <a-button>{{ $t('label.ssvm.open.cert.page') }}</a-button> | ||
| </a> |
| <a :href="ssvmOrigin" target="_blank" rel="noopener noreferrer"> | ||
| <a-button>{{ $t('label.ssvm.open.cert.page') }}</a-button> | ||
| </a> |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17961 |
Description
This PR fixes #12490
UI Pre-check
Before initiating upload, have the UI make a test request to the SSVM and guide users to accept the certificate if needed.
Fixed Template, ISO and Volume upload.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Screen.recording.mov
How did you try to break this feature and the system with this change?