Repository navigation
ui: list resources of project-owned instances outside project view - #14581
DaanHoogland wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #14581 +/- ##
============================================
+ Coverage 16.45% 16.47% +0.01%
- Complexity 13710 13714 +4
============================================
Files 5669 5669
Lines 501734 501737 +3
Branches 60961 60967 +6
============================================
+ Hits 82584 82655 +71
+ Misses 409895 409819 -76
- Partials 9255 9263 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland 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. |
|
@1fanwang, please review my version of your fix here. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19901 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-17490)
|
|
Unsure if it's something wrong on my setup but I cannot see templates on the "Default view" anymore without the following patch. diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue
index 72cb74399d..dba8501023 100644
--- a/ui/src/views/compute/DeployVM.vue
+++ b/ui/src/views/compute/DeployVM.vue
@@ -1007,7 +1007,7 @@ export default {
groups: false
},
owner: {
- projectid: store.getters.project?.id,
+ projectid: store.getters.project?.id || null,
domainid: store.getters.project?.id ? null : store.getters.userInfo.domainid,
account: store.getters.project?.id ? null : store.getters.userInfo.account
},The rest seems to work. I can see the resources from the default view while they are owned by a project. |
|
@bddvlpr , thanks for the finding, I'll look into that and update. |
2dacf7d to
34cb371
Compare
|
@bddvlpr, it wasn't you/your env, but a conflict with an old PR. I added a fix. hope all works now. |
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
Good morning, @DaanHoogland, Thank you for the fix. Apologies for taking so long to test; I first tried to (remotely through IPMI) install a new physical development area, but had issues compiling the code (especially failing tests such as LDAP, though unrelated to this incident). To avoid delaying the testing above, I ran it in the provided environment. With the provided QA link above, I was able to test this. I can also confirm that it fixes #14572. |
can you review? |
|
Good morning, @DaanHoogland, After reviewing your proposed code changes (apologies; I have some coding experience but am not a coder by trade), I noticed a potential issue. I have made the following observations: Observation 1
Suggestion: The above scenario does not lead to too many functional issues, and perhaps adding the tests you ran to include, e.g., However, the following is a more "drastic" potential issue that is present/introduced. Observation 2:A user-owned instance with a Volume snapshot exists. Suggestion: Consider changing the volumes and their snapshots’ ownership before changing the instance’s ownership. However, I am not sure if it would be possible without a chase condition. Also, I am not sure if this (changing all the ownerships of the VM and its related objects) is the expected behaviour. Should snapshot ownership ever be transferred between users and projects? If not, then this "bug" will persist. |
|
On a side note, the proposed fix also fixes a possibly unknown issue. Without the fix, it is not possible to make a volume snapshot of a project-owned volume in the UI. When a project-owned VM is viewed and a volume snapshot is attempted, the volume dropdown list is empty. When attempting to perform the action from Storage -> Volumes -> selecting the specific volume and then clicking on Snapshot, it produced an error: “Failed to create Snapshot due to an internal error creating Snapshot for volume”. |
The VolumesTab change and its new tests match mine apart from the API helper name that 4.20 uses. I did not review the other files in this pull request. I agree with closing the 4.22 pull request in favour of this one, so I have closed it. |
6f332e9 to
d85ab4e
Compare
|
Thanks @vanecka, Observation 1: same cause as the rest of this PR: the Instance Snapshots tab (and the Backups tab) on the instance page didn't send the instance's project. Both now do, so a project-owned instance's snapshots and backups show up outside the project view as well. Note that a snapshot taken while the instance was still yours stays owned by you, so it shows under Compute → Instance Snapshots in the Default view rather than on the (now project-owned) instance's tab. Observation 2: that's deliberate server-side behaviour: assigning an instance to another owner is refused while its volumes have snapshots ("Detach volume or remove snapshots for the volume before assigning VM to another user"). Whether snapshots should move along with the instance is a fair question, but it's a backend change outside the scope of this UI fix. Could you open a separate issue for it? On the side note: nice and bummer ;). Glad the snapshot dialog works now. The "internal error creating Snapshot" issue is worth its own issue I think. I would like not to creep the scope of this PR for it. Can you reproduce it and add some logging information to the issue, if you create it? |
|



Description
When a project-owned instance is opened outside its project (Default view, or another project selected), several lists on the instance page come up empty or show the caller's own resources instead of the project's. The list APIs skip project resources unless the request names a project, and these calls did not send one.
Each call now sends the instance's own
projectid:listVolumes)listNetworks)listSecurityGroups);SecurityGroupSelectiongets an optionalprojectIdprop, so the deploy wizards keep using the selected projectlistInstanceGroups)The instance's own project is used rather than
projectid=-1, because-1only reaches the caller's own account and the projects the caller is a member of, which would hide other accounts' resources from a domain admin. An instance outside any project sends noprojectid, so its requests are unchanged. The project view keeps working as before: the request interceptor only adds the selected project when a call has none.The Add network dialog also no longer throws when there is no network left to offer.
The VolumesTab spec comes from #14158 (without its GiB label test, which depends on a change that is not on 4.20).
Fixes #14572
Fixes #14573
Fixes #14070
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
VolumesTab.spec.jscovers root admin and domain admin callers in Default view, in the instance's project and in another project, plus instances outside any project and system VMs. Without the fix, the three cases for a project instance outside its project fail.projectid: the Volumes tab sendsprojectid=<project>; for an instance outside any project the request is unchanged.How did you try to break this feature and the system with this change?
Checked that an explicit
projectidwins over the selected project in the request interceptor and that an undefined one falls back to it, so the project view and instances outside any project send the same requests as before.SecurityGroupSelectiondropsaccount/domainidwhen a project is passed, since the API rejectsaccounttogether withprojectid.🤖 Generated with Claude Code