Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 8 commits into from
Nov 28, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6dc851a
allow filtering of listDiskOffering and listServiceOffering APIs by a…
Jan 9, 2023
90a416d
address Wei review
Jan 18, 2023
6f47e07
Merge branch '4.18' into allow-filtering-of-listDiskOffering-and-list…
RodrigoDLopez Aug 14, 2023
f135a51
changes BaseListDomainResourcesCmd by BaseListProjectAndAccountResour…
Aug 14, 2023
b996ba3
Merge branch '4.18' into allow-filtering-of-listDiskOffering-and-list…
RodrigoDLopez Aug 16, 2023
d9e5868
Apply suggestions from code review
RodrigoDLopez Aug 16, 2023
2ce4f62
removes unused imports and address Wei reviews
Aug 16, 2023
0d04398
small adjustment when use caller or owner
Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@
// under the License.
package org.apache.cloudstack.api.command.user.offering;

import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd;
import org.apache.cloudstack.api.response.StoragePoolResponse;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
import org.apache.log4j.Logger;

import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseListDomainResourcesCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.response.DiskOfferingResponse;
import org.apache.cloudstack.api.response.ListResponse;

@APICommand(name = "listDiskOfferings", description = "Lists all available disk offerings.", responseObject = DiskOfferingResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd {
public class ListDiskOfferingsCmd extends BaseListProjectAndAccountResourcesCmd {
public static final Logger s_logger = Logger.getLogger(ListDiskOfferingsCmd.class.getName());


Expand Down
4 changes: 2 additions & 2 deletions ...rc/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,20 @@
// under the License.
package org.apache.cloudstack.api.command.user.offering;

import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd;
import org.apache.cloudstack.api.response.ZoneResponse;
import org.apache.log4j.Logger;

import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseListDomainResourcesCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.ServiceOfferingResponse;
import org.apache.cloudstack.api.response.UserVmResponse;

@APICommand(name = "listServiceOfferings", description = "Lists all available service offerings.", responseObject = ServiceOfferingResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class ListServiceOfferingsCmd extends BaseListDomainResourcesCmd {
public class ListServiceOfferingsCmd extends BaseListProjectAndAccountResourcesCmd {
public static final Logger s_logger = Logger.getLogger(ListServiceOfferingsCmd.class.getName());


Expand Down
35 changes: 20 additions & 15 deletions server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2919,6 +2919,8 @@ private Pair<List<DiskOfferingJoinVO>, Integer> searchForDiskOfferingsInternal(L
Object id = cmd.getId();
Object keyword = cmd.getKeyword();
Long domainId = cmd.getDomainId();
Long projectId = cmd.getProjectId();
String accountName = cmd.getAccountName();
Boolean isRootAdmin = _accountMgr.isRootAdmin(account.getAccountId());
Boolean isRecursive = cmd.isRecursive();
Long zoneId = cmd.getZoneId();
Expand All @@ -2928,7 +2930,7 @@ private Pair<List<DiskOfferingJoinVO>, Integer> searchForDiskOfferingsInternal(L
// Keeping this logic consistent with domain specific zones
// if a domainId is provided, we just return the disk offering
// associated with this domain
if (domainId != null) {
if (domainId != null && accountName == null) {
if (_accountMgr.isRootAdmin(account.getId()) || isPermissible(account.getDomainId(), domainId)) {
// check if the user's domain == do's domain || user's domain is
// a child of so's domain for non-root users
Expand Down Expand Up @@ -3009,9 +3011,9 @@ private Pair<List<DiskOfferingJoinVO>, Integer> searchForDiskOfferingsInternal(L

// Filter offerings that are not associated with caller's domain
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
Account caller = CallContext.current().getCallingAccount();
if (caller.getType() != Account.Type.ADMIN) {
Domain callerDomain = _domainDao.findById(caller.getDomainId());
account = _accountMgr.finalizeOwner(account, accountName, domainId, projectId);
if (!Account.Type.ADMIN.equals(account.getType())) {
Domain callerDomain = _domainDao.findById(account.getDomainId());
List<Long> domainIds = findRelatedDomainIds(callerDomain, isRecursive);

List<Long> ids = _diskOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
Expand Down Expand Up @@ -3090,6 +3092,8 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
searchFilter.addOrderBy(ServiceOfferingJoinVO.class, "id", true);

Account caller = CallContext.current().getCallingAccount();
Long projectId = cmd.getProjectId();
String accountName = cmd.getAccountName();
Object name = cmd.getServiceOfferingName();
Object id = cmd.getId();
Object keyword = cmd.getKeyword();
Expand All @@ -3105,9 +3109,10 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
Integer cpuSpeed = cmd.getCpuSpeed();
Boolean encryptRoot = cmd.getEncryptRoot();

final Account owner = _accountMgr.finalizeOwner(caller, accountName, domainId, projectId);
SearchCriteria<ServiceOfferingJoinVO> sc = _srvOfferingJoinDao.createSearchCriteria();
if (!_accountMgr.isRootAdmin(caller.getId()) && isSystem) {
throw new InvalidParameterValueException("Only ROOT admins can access system's offering");
throw new InvalidParameterValueException("Only ROOT admins can access system offerings.");
}

// Keeping this logic consistent with domain specific zones
Expand All @@ -3116,8 +3121,8 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
if (domainId != null && !_accountMgr.isRootAdmin(caller.getId())) {
// check if the user's domain == so's domain || user's domain is a
// child of so's domain
if (!isPermissible(caller.getDomainId(), domainId)) {
throw new PermissionDeniedException("The account:" + caller.getAccountName() + " does not fall in the same domain hierarchy as the service offering");
if (!isPermissible(owner.getDomainId(), domainId)) {
throw new PermissionDeniedException("The account:" + owner.getAccountName() + " does not fall in the same domain hierarchy as the service offering");
}
}

Expand All @@ -3129,7 +3134,7 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
throw ex;
}

_accountMgr.checkAccess(caller, null, true, vmInstance);
_accountMgr.checkAccess(owner, null, true, vmInstance);

currentVmOffering = _srvOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId());
if (! currentVmOffering.isDynamic()) {
Expand Down Expand Up @@ -3177,22 +3182,22 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte
}

// boolean includePublicOfferings = false;
if ((_accountMgr.isNormalUser(caller.getId()) || _accountMgr.isDomainAdmin(caller.getId())) || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) {
if ((_accountMgr.isNormalUser(owner.getId()) || _accountMgr.isDomainAdmin(owner.getId())) || owner.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) {
// For non-root users.
if (isSystem) {
throw new InvalidParameterValueException("Only root admins can access system's offering");
}
if (isRecursive) { // domain + all sub-domains
if (caller.getType() == Account.Type.NORMAL) {
if (owner.getType() == Account.Type.NORMAL) {
throw new InvalidParameterValueException("Only ROOT admins and Domain admins can list service offerings with isrecursive=true");
}
}
} else {
// for root users
if (caller.getDomainId() != 1 && isSystem) { // NON ROOT admin
throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering");
if (owner.getDomainId() != 1 && isSystem) { // NON ROOT admin
throw new InvalidParameterValueException("Non ROOT admins cannot access system's offering.");
}
if (domainId != null) {
if (domainId != null && accountName == null) {
sc.addAnd("domainId", Op.FIND_IN_SET, String.valueOf(domainId));
}
}
Expand Down Expand Up @@ -3276,8 +3281,8 @@ private Pair<List<ServiceOfferingJoinVO>, Integer> searchForServiceOfferingsInte

// Filter offerings that are not associated with caller's domain
// Fetch the offering ids from the details table since theres no smart way to filter them in the join ... yet!
if (caller.getType() != Account.Type.ADMIN) {
Domain callerDomain = _domainDao.findById(caller.getDomainId());
if (owner.getType() != Account.Type.ADMIN) {
Domain callerDomain = _domainDao.findById(owner.getDomainId());
List<Long> domainIds = findRelatedDomainIds(callerDomain, isRecursive);

List<Long> ids = _srvOfferingDetailsDao.findOfferingIdsByDomainIds(domainIds);
Expand Down