apache / cloudstack

Apache CloudStack is an opensource Infrastructure as a Service (IaaS) cloud computing platform
https://cloudstack.apache.org/
Apache License 2.0
1.83k stars 1.07k forks source link

User friendly name of Downloaded Templates Volumes and ISOs #9252

Closed harikrishna-patnala closed 2 weeks ago

harikrishna-patnala commented 2 weeks ago

Description

This PR addresses the improvement #6949

Previously when a template or volume or ISO is downloaded then the file name generated for the downloaded file is a random URL, with this PR we have improved to use the name of the object to set as the file name.

During the preparation of URL, MS uses only alphanumeric characters and replaces others with Hyphen (-).

For example

image

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Downloaded multiple templates, volumes with various names.

How did you try to break this feature and the system with this change?

harikrishna-patnala commented 2 weeks ago

@blueorangutan package

blueorangutan commented 2 weeks ago

@harikrishna-patnala 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.

blueorangutan commented 2 weeks ago

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9945

sureshanaparti commented 2 weeks ago

@blueorangutan test

blueorangutan commented 2 weeks ago

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

blueorangutan commented 2 weeks ago

[SF] Trillian test result (tid-10445) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 42388 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9252-t10445-kvm-centos7.zip Smoke tests completed. 130 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_02_trigger_shutdown Failure 341.70 test_safe_shutdown.py
DaanHoogland commented 2 weeks ago

tested and works as expected. @rohityadavcloud you are right that the URL is predictable, but the file won't be generated until the user requests it. as discussed we can add a path element to make it unpredictable but still allow the file to be downloaded with the user friendly name. @harikrishna-patnala , this would also address the potential issue that two users have a file with the same name. So instead of http://w.x.u.z/userdata/<filename or uuid>.<extension>, they will download http://w.x.u.z/userdata/<uuid>/<filename>.<extension>

harikrishna-patnala commented 2 weeks ago

@blueorangutan package

blueorangutan commented 2 weeks ago

@harikrishna-patnala 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.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 43 lines in your changes missing coverage. Please review.

Project coverage is 14.95%. Comparing base (8d02e5f) to head (dad049b). Report is 17 commits behind head on 4.19.

Files Patch % Lines
...tastore/driver/CloudStackImageStoreDriverImpl.java 0.00% 21 Missing :warning:
...nt/api/storage/CreateEntityDownloadURLCommand.java 0.00% 13 Missing :warning:
...cloudstack/storage/template/UploadManagerImpl.java 0.00% 4 Missing :warning:
...oudstack/diagnostics/to/DiagnosticsDataObject.java 0.00% 3 Missing :warning:
...va/com/cloud/storage/upload/UploadMonitorImpl.java 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## 4.19 #9252 +/- ## ============================================ - Coverage 14.95% 14.95% -0.01% - Complexity 11016 11017 +1 ============================================ Files 5378 5380 +2 Lines 469930 470073 +143 Branches 59422 59721 +299 ============================================ + Hits 70301 70307 +6 - Misses 391842 391981 +139 + Partials 7787 7785 -2 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9252/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9252/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.28% <ø> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9252/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.66% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blueorangutan commented 2 weeks ago

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10012

harikrishna-patnala commented 2 weeks ago

@DaanHoogland I've updated the PR which adds the UUID in the path

image

Please review