Fix assignVirtualMachine template checkAccess

[gp-santos]

Description

This PR changes the checkAccess of when assigning a VM to check for the template itself instead of the account owner of the template. In this way, there is no need for checking if the template is public (it already does so inside checkAccess) and it gives a better message when the assigning fails because of it:

Failed to move vm due to Account [...] does not have permission to launch instances from template [...]

Instead of:

Failed to move vm due to Account [{"accountName":"[...]","id":[...],"uuid":"[...]"}] does not have permission to operate with resource

Also, current ACS lets you assign a VM with a public template of domain B to an account of domain A even if share.public.templates.with.other.domains is false. This also fixes it.

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)
• build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

How Has This Been Tested?

In a local lab with:

• In domain /A, a domain admin and a user. A private template of user A.
• In domain /A/AB, a user. A public template of user AB.
• In domain /B, a user. A public template of user B.

I then reproduced the following cases:

• Created a VM with userA and privateTemplateA.
• With ROOT Admin, assign the VM to user B.

Failed to move vm due to Account usrb does not have permission to launch instances from template macha

• With Domain Admin A, assign the VM to user B through cmk.

🙈 Error: (HTTP 530, error code 9999) Failed to move vm due to Account doma does not have permission to operate within domain id=a453e418-f295-47c1-9728-c62121ae54f3

• With Domain Admin A, assign the VM to user AB.

Failed to move vm due to Account usrab does not have permission to launch instances from template macha

---

• Created a VM with userAB and publicTemplateAB.
• With Domain Admin A, assign the VM to user A. Completed successfully.

---

• Created a VM with userA and privateTemplateA.
• With Root Admin, assign the VM to Root Admin. Completed successfully.

---

• Created a VM with userA and privateTemplateA.
• With Domain Admin A, assign the VM to Domain Admin A.

Failed to move vm due to Account doma does not have permission to launch instances from template macha

• With Root Admin, assign the VM to Domain Admin A.

Failed to move vm due to Account doma does not have permission to launch instances from template macha

---

• Created a VM with userB and publicTemplateB. (share.public.templates.with.other.domains true)
• With Root Admin, assign the VM to user A. Completed successfully.

---

• Created a VM with userB and publicTemplateB. (share.public.templates.with.other.domains false)
• With Root Admin, assign the VM to user A.

Failed to move vm due to Account [{"accountName":"usra","id":4,"uuid":"01ebae51-0368-49d6-918c-6c6f2f4fb781"}]is not allowed to access the template Template {"format":"QCOW2","id":202,"uniqueName":"202-5-bd209989-477a-357d-8d6e-983f3d6052ae"}

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.10%. Comparing base (2746225) to head (af34bc3).
Report is 251 commits behind head on 4.19.

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #8886      +/-   ##
============================================
+ Coverage     30.82%   33.10%   +2.27%     
- Complexity    34017    34315     +298     
============================================
  Files          5341     4998     -343     
  Lines        375033   348272   -26761     
  Branches      54554    49846    -4708     
============================================
- Hits         115597   115283     -314     
+ Misses       244159   217810   -26349     
+ Partials      15277    15179      -98     

Flag	Coverage Δ
simulator-marvin-tests	24.72% <100.00%> (+0.02%)	⬆️
uitests	?
unit-tests	16.58% <0.00%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti 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]

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

[DaanHoogland]

clgtm

[weizhouapache]

code lgtm

makes sense

[DaanHoogland]

@blueorangutan test

[blueorangutan]

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

[sureshanaparti]

lgtm

[blueorangutan]

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

Test	Result	Time (s)	Test File

[hsato03]

LGTM.

Reproduced the tests and verified that the share.public.templates.with.other.domains (false and true) now works when assigning public template instances between different domains and that the error message has changed when the assignment fails.

[weizhouapache]

@blueorangutan package

[blueorangutan]

@weizhouapache 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]

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

[gp-santos]

Hi, @DaanHoogland @weizhouapache

Is there anything missing in this one?

[weizhouapache]

Hi, @DaanHoogland @weizhouapache

Is there anything missing in this one?

@gpordeus
nothing from my side. I added the label needs-testing by mistake, now it is removed

[DaanHoogland]

I think @weizhouapache accidenly add the tag https://github.com/apache/cloudstack/labels/status%3Aneeds-testing when he saw the test report by @hsato03 , thinking he was removing it. It is ready I think. (@weizhouapache already approved)