You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "gpordeus (via GitHub)" <gi...@apache.org> on 2024/04/04 20:55:39 UTC
[PR] fix assignvm template permission check [cloudstack]
gpordeus opened a new pull request, #8886:
URL: https://github.com/apache/cloudstack/pull/8886
### 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)
- [X] 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
- [X] 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"}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2042251934
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2168509038
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "hsato03 (via GitHub)" <gi...@apache.org>.
hsato03 commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2168478541
LGTM.
Reproduced the tests and verified that `share.public.templates.with.other.domains` now works when assigning public template instances between different domains and that the error message has changed when the assignment fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2038268099
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8886?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 20.37%. Comparing base [(`2746225`)](https://app.codecov.io/gh/apache/cloudstack/commit/2746225b999612f156e421199e34ef8de98a3664?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`af34bc3`)](https://app.codecov.io/gh/apache/cloudstack/pull/8886?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 118 commits behind head on 4.19.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.19 #8886 +/- ##
=============================================
- Coverage 30.82% 20.37% -10.46%
+ Complexity 34017 19217 -14800
=============================================
Files 5341 4862 -479
Lines 375033 325912 -49121
Branches 54554 45811 -8743
=============================================
- Hits 115597 66397 -49200
- Misses 244159 249850 +5691
+ Partials 15277 9665 -5612
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8886/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8886/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `20.37% <100.00%> (-4.34%)` | :arrow_down: |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8886/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8886/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8886?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2168511845
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2168601591
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9964
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2044112915
<b>[SF] Trillian test result (tid-9752)</b>
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
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2039759591
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2039918941
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9173
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2039756208
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2042514324
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix assignVirtualMachine template checkAccess [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8886:
URL: https://github.com/apache/cloudstack/pull/8886#issuecomment-2042515327
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org