Skip to content

allow filtering of listDiskOffering and listServiceOffering APIs by account or project#7082

Merged
DaanHoogland merged 8 commits into
apache:4.18from
RodrigoDLopez:allow-filtering-of-listDiskOffering-and-listServiceOffering-APIs-by-account-or-project
Nov 28, 2023
Merged

allow filtering of listDiskOffering and listServiceOffering APIs by account or project#7082
DaanHoogland merged 8 commits into
apache:4.18from
RodrigoDLopez:allow-filtering-of-listDiskOffering-and-listServiceOffering-APIs-by-account-or-project

Conversation

@RodrigoDLopez

Copy link
Copy Markdown
Contributor

Description

ACS does not allow disk and service offerings to be filtered by account or project. An extension was created that allows operators to inform a project or an account and domain when executing the listDiskOffering and listServiceOffering APIs. In this way, the APIs will only return offers whose account/project has access permission.

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)

Feature/Enhancement Scale

  • Major
  • Minor

How Has This Been Tested?

Tests were executed via CMK by calling listDiskOffering and listServiceOffering informing the parameters account and domainId or projectId. To verify the consistency of the data returned by the APIs, listDiskOffering and listServiceOffering calls were executed with the target account from the previous step, without informing the filtering parameters.

@codecov

codecov Bot commented Jan 11, 2023

Copy link
Copy Markdown

Codecov Report

Merging #7082 (0d04398) into 4.18 (e8b4911) will increase coverage by 0.04%.
Report is 55 commits behind head on 4.18.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               4.18    #7082      +/-   ##
============================================
+ Coverage     13.02%   13.07%   +0.04%     
- Complexity     9040     9107      +67     
============================================
  Files          2720     2720              
  Lines        257094   257518     +424     
  Branches      40092    40151      +59     
============================================
+ Hits          33491    33658     +167     
- Misses       219398   219629     +231     
- Partials       4205     4231      +26     
Files Coverage Δ
...ain/java/com/cloud/api/query/QueryManagerImpl.java 3.02% <0.00%> (-0.01%) ⬇️

... and 50 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a 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.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cltgm

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5245

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Failure 571.91 test_kubernetes_clusters.py

Comment thread server/src/main/java/com/cloud/api/query/QueryManagerImpl.java Outdated
@sonarqubecloud

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@BryanMLima BryanMLima left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CLGTM, did not manually tested.

@stephankruggg stephankruggg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CLGTM, did not manually test it.

@kiranchavala

Copy link
Copy Markdown
Member

@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File

@kiranchavala

Copy link
Copy Markdown
Member

Manually tested

Able to see the account id and project id in the api calls when execting the api calls

Tested by creating service offerings and was able to filter them based on account id

(4.17.2) 🐱 > list serviceofferings 
cpunumber=         cpuspeed=          domainid=          encryptroot=       
filter=            id=                isrecursive=       issystem=          
keyword=           listall=           memory=            name=              
page=              pagesize=          systemvmtype=      virtualmachineid=  
zoneid=            

(pr-7082) 🐱 > list serviceofferings 
account=           cpunumber=         cpuspeed=          domainid=          
encryptroot=       filter=            id=                isrecursive=       
issystem=          keyword=           listall=           memory=            
name=              page=              pagesize=          projectid=         
systemvmtype=      virtualmachineid=  zoneid= 

------------------------------------

(4.17.2) 🐱 > list diskofferings 
domainid=    encrypt=     filter=      id=          isrecursive= keyword=     listall=     name=        page=        pagesize=    storageid=   volumeid=    
zoneid=      


(pr-7082) 🐱 > list diskofferings 
account=     domainid=    encrypt=     filter=      id=          isrecursive= keyword=     listall=     name=        page=        pagesize=    projectid=   
storageid=   volumeid=    zoneid= 

@DaanHoogland DaanHoogland added this to the 4.18.1.0 milestone Feb 28, 2023
@soreana soreana self-assigned this Aug 7, 2023
@soreana

soreana commented Aug 7, 2023

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@soreana a [SF] 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

Copy link
Copy Markdown

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

@DaanHoogland DaanHoogland changed the base branch from main to 4.18 August 7, 2023 18:45
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SF] 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.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SF] 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

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@shwstppr shwstppr requested a review from weizhouapache October 6, 2023 12:34
@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File

@DaanHoogland

Copy link
Copy Markdown
Contributor

@weizhouapache can you check if all "owner" vs "caller" references are now correct (afayct of course)

@soreana

soreana commented Oct 9, 2023

Copy link
Copy Markdown
Member

@DaanHoogland, @soreana I just pushed new changes to when to use the caller and when to use the owner for some checks. I'm a bit removed from the community due to other activities that are consuming a lot of my time. But I'll try to be more active here from now on. If anyone can test this latest version, I would greatly appreciate it, and if further changes are necessary, please give me feedback here.

@RodrigoDLopez I think that was the last remaining comment @soreana can you re-test?

@DaanHoogland I caught up with lots of work, I can't test it now but I can test it later this week.

@DaanHoogland

DaanHoogland commented Oct 20, 2023

Copy link
Copy Markdown
Contributor

@soreana busy weekend ? ;)

@weizhouapache can you approve now?

@shwstppr

shwstppr commented Nov 8, 2023

Copy link
Copy Markdown
Contributor

Hi @soreana, will you be able to test it this week?

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan LLtest

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

[LL] Trillian Build Failed (tid-6898)

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Failure 675.71 test_kubernetes_clusters.py
test_08_migrate_vm Error 0.05 test_vm_life_cycle.py

@DaanHoogland

Copy link
Copy Markdown
Contributor

@weizhouapache can you approve of the current state of this PR?

@soreana

soreana commented Nov 20, 2023

Copy link
Copy Markdown
Member

@DaanHoogland @RodrigoDLopez I'm checking this one.

@soreana

soreana commented Nov 27, 2023

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@soreana 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

Copy link
Copy Markdown

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

@soreana

soreana commented Nov 28, 2023

Copy link
Copy Markdown
Member

@soreana I would say the use is for the user to see which offerings are available to an account (??) cc @RodrigoDLopez

@DaanHoogland So in that case I would say it is working :D

I've created the following setup, and tested the cloudmonkey output.

Root
  |----- Domain
  |       |== Account
  |       |== DomainOffering
  |----- Domain2
          |== Account2
          |== Domain2Offering

No domainid passed -> all the service offerings are listed

(localcloud)  > list serviceofferings filter=name,id
{
  "count": 4,
  "serviceoffering": [
    {
      "id": "95d95db2-2b2d-4660-853d-7871104505b5",
      "name": "Small Instance"
    },
    {
      "id": "cfdc2dc3-48c2-41db-bacf-6ef261e01b13",
      "name": "Medium Instance"
    },
    {
      "id": "c041a46b-3d52-4835-84f8-889be36b3057",
      "name": "DomainOffering"
    },
    {
      "id": "89bbe884-2aa2-4d76-9c10-e74796a0c27d",
      "name": "Domain2Offering"
    }
  ]
}

Only the domainid passed -> service offerings in that domain are listed

(localcloud)  > list serviceofferings domainid=33f0fc41-5637-48d5-8db6-f0ce866e1753 filter=name,id
{
  "count": 1,
  "serviceoffering": [
    {
      "id": "89bbe884-2aa2-4d76-9c10-e74796a0c27d",
      "name": "Domain2Offering"
    }
  ]
}

the domainid and account are passed -> all the service offerings that account has access to are listed

(localcloud)  > list serviceofferings domainid=33f0fc41-5637-48d5-8db6-f0ce866e1753 account=account2 filter=name,id
{
  "count": 3,
  "serviceoffering": [
    {
      "id": "95d95db2-2b2d-4660-853d-7871104505b5",
      "name": "Small Instance"
    },
    {
      "id": "cfdc2dc3-48c2-41db-bacf-6ef261e01b13",
      "name": "Medium Instance"
    },
    {
      "id": "89bbe884-2aa2-4d76-9c10-e74796a0c27d",
      "name": "Domain2Offering"
    }
  ]
}

@DaanHoogland @RodrigoDLopez The PR LGTM.

I tested it manually, get the same result as my previous tests..

@DaanHoogland DaanHoogland merged commit 956efb2 into apache:4.18 Nov 28, 2023
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Dec 6, 2023
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.

9 participants