Skip to content

Fix VM and volume metrics listing regressions #12284

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

Open
winterhazel wants to merge 4 commits into
base: 4.20
Choose a base branch
from
+37 −14
Open

Fix VM and volume metrics listing regressions #12284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c7989c0
Fix VM and volume metrics listing regression
winterhazel Dec 18, 2025
c56594b
Extract creation of search parameters & fix project resources being i…
winterhazel Dec 18, 2025
fbf7551
Remove unused import
winterhazel Dec 18, 2025
67c5241
Remove another unused import
winterhazel Dec 18, 2025
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
3 changes: 1 addition & 2 deletions plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.SystemVmResponse;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.apache.cloudstack.response.VolumeMetricsStatsResponse;

Expand All @@ -37,7 +36,7 @@ public class ListVolumesUsageHistoryCmd extends BaseResourceUsageHistoryCmd {
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, description = "the ID of the volume.")
private Long id;

@Parameter(name=ApiConstants.IDS, type=CommandType.LIST, collectionType=CommandType.UUID, entityType= SystemVmResponse.class, description="the IDs of the volumes, mutually exclusive with id.")
@Parameter(name = ApiConstants.IDS, type = CommandType.LIST, collectionType = CommandType.UUID, entityType = VolumeResponse.class, description = "the IDs of the volumes, mutually exclusive with id.")
private List<Long> ids;

@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "name of the volume (a substring match is made against the parameter value returning the data for all matching Volumes).")
Expand Down
48 changes: 36 additions & 12 deletions plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,25 +244,49 @@ public ListResponse<VolumeMetricsStatsResponse> searchForVolumeMetricsStats(List
return createVolumeMetricsStatsResponse(volumeList, volumeStatsList);
}

/**
* Outputs the parameters that should be used for access control in the query of a resource to
* {@code permittedAccounts} and {@code domainIdRecursiveListProject}.
* @param isIdProvided indicates whether any ID was provided to the command
*/
private void buildBaseACLSearchParametersForMetrics(boolean isIdProvided, List<Long> permittedAccounts, Ternary<Long, Boolean,
Project.ListProjectResourcesCriteria> domainIdRecursiveListProject) {
Account caller = CallContext.current().getCallingAccount();
Account.Type callerType = caller.getType();

boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(callerType);
domainIdRecursiveListProject.second(recursive);

// If no ID was provided, then the listing will skip project resources (null); otherwise, project resources should
// be listed as well (any long allows this)
Long id = isIdProvided ? 1L : null;
Copy link
Member

@weizhouapache weizhouapache Dec 19, 2025

Choose a reason for hiding this comment

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

is there side-effect when id is set to 1L ?

Copy link
Member Author

@winterhazel winterhazel Dec 19, 2025

Choose a reason for hiding this comment

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

The value itself is not used, com.cloud.user.AccountManagerImpl#buildACLSearchParameters just checks whether the ID is null in order to decide whether project resources should be skipped or not, so no.

cloudstack/server/src/main/java/com/cloud/user/AccountManagerImpl.java

Lines 3376 to 3378 in eb93f01

if (id == null) {
domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.SkipProjectResources);
}


// Allow users to also list metrics of resources owned by projects they belong to (-1L), and admins to list all
// metrics belonging to their domains recursively (null)
Long projectId = isIdProvided && callerType == Account.Type.NORMAL ? -1L : null;

accountMgr.buildACLSearchParameters(caller, id, null, projectId, permittedAccounts, domainIdRecursiveListProject, true, false);
}

/**
* Searches VMs based on {@code ListVMsUsageHistoryCmd} parameters.
*
* @param cmd the {@link ListVMsUsageHistoryCmd} specifying the parameters.
* @return the list of VMs.
*/
protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) {
final Long id = cmd.getId();
Account caller = CallContext.current().getCallingAccount();
List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());

boolean isIdProvided = CollectionUtils.isNotEmpty(ids);
List<Long> permittedAccounts = new ArrayList<>();
boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType());
Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false);
Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, null, null);
buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject);

Long domainId = domainIdRecursiveListProject.first();
Boolean isRecursive = domainIdRecursiveListProject.second();
Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();

Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
String name = cmd.getName();
String keyword = cmd.getKeyword();

Expand Down Expand Up @@ -357,18 +381,18 @@ protected Map<Long,List<VmStatsVO>> searchForVmMetricsStatsInternal(Date startDa
* @return the list of VMs.
*/
protected Pair<List<VolumeVO>, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) {
final Long id = cmd.getId();
Account caller = CallContext.current().getCallingAccount();
List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());

boolean isIdProvided = CollectionUtils.isNotEmpty(ids);
List<Long> permittedAccounts = new ArrayList<>();
boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType());
Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, recursive, null);
accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false);
Ternary<Long, Boolean, Project.ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(null, null, null);
buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject);

Long domainId = domainIdRecursiveListProject.first();
Boolean isRecursive = domainIdRecursiveListProject.second();
Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();

Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
String name = cmd.getName();
String keyword = cmd.getKeyword();

Expand Down
Loading