Skip to content

Conversation

@gpordeus
Copy link
Collaborator

Description

This PR allows selecting a target account (or project) during volume creation through the UI, using #8919's OwnershipSelection.

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

Screenshots (if appropriate):

image

How Has This Been Tested?

Environment with one domain (dom), besides root, and a domain admin and an user accounts in both root and dom, as well as the Root Admin. Every account created one project each.

Table lists all options shown in UI, when logged as each of the accounts:
(User accounts did not show the selection fields Owner type, Domain and Account/Project. Neither did any of the projects.)

Logged in as Listed domains Listed accounts Listed projects
ROOT Admin (admin) - ROOT
- ROOT/dom
- admin, dadm, usr
- dom-adm, dom-usr
- admin-proj, dadm-proj, usr-proj
- dom-adm-proj, dom-usr-proj
ROOT Domain Admin (dadm) - ROOT
- ROOT/dom
- admin, dadm, usr
- dom-adm, dom-usr
- admin-proj, dadm-proj, usr-proj
- *
ROOT User (usr) --- --- ---
ROOT/dom Domain Admin (dom-adm) - ROOT/dom - dom-adm, dom-usr - dom-adm-proj, dom-usr-proj
ROOT/dom User (dom-usr) --- --- ---

Testing this, I found a bug on listProjects. A domain admin can't list a subdomain's projects, with isrecursive or not.

I'm still working on the best way to go about it and will open a PR to fix it, but it's not ready yet.

@codecov
Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 4.18%. Comparing base (cb48202) to head (726ba87).
Report is 37 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##            main   #9265      +/-   ##
========================================
- Coverage   4.19%   4.18%   -0.01%     
========================================
  Files        369     369              
  Lines      30233   30297      +64     
  Branches    5343    5365      +22     
========================================
  Hits        1269    1269              
- Misses     28820   28884      +64     
  Partials     144     144              
Flag Coverage Δ
uitests 4.18% <ø> (-0.01%) ⬇️

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.

@DaanHoogland
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/9265 (QA-JID-383)

@DaanHoogland
Copy link
Contributor

@gpordeus two concerns

  1. image somehow offerings are not found.
  2. is this meant for (domain) admins or for every user?

@rohityadavcloud rohityadavcloud requested a review from shwstppr June 18, 2024 07:07
@rohityadavcloud rohityadavcloud added this to the 4.20.0.0 milestone Jun 18, 2024
@gpordeus
Copy link
Collaborator Author

Hi, @DaanHoogland , thanks for checking this out.

somehow offerings are not found.

The offerings not found is a regression that happened on 4.19 (version of QA environment), where calling listDiskOfferings with domainid and account name short circuits in searching for domain-only disk offerings. This was fixed (I believe on #8321, so it will probably be fine on 4.19.1) and the offerings show up on the form when using 4.20, which is the target.

is this meant for (domain) admins or for every user?

Just admins, it uses the same verification as #8919.

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

Copy link
Member

@winterhazel winterhazel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually tested in a local lab running on 4.20. Everything seems to be working as intended.

My environment had the following domains, accounts and projects:

  • ROOT: admin (root admin), pr (project)
  • ROOT/d1: d1 (domain admin), u1 (user), pd1 (project)
  • ROOT/d2: d2 (domain admin), pd2 (project)

I did the following tests:

  • I logged into every admin account and verified that the ownership selection only showed accounts they had access to. I created the volume and verified they were created for the chosen account.
  • I logged into account u1 and verified that the ownership selection did not get shown. I created the volume and verified that it was created for account u1.
  • I verified that the ownership selection was not shown for projects. I created the volume and verified that it was created for the project.

@DaanHoogland DaanHoogland merged commit de683a5 into apache:main Jul 1, 2024
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants