Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
List<String> RootAdminOnlyVmSettings = Arrays.asList(VmDetailConstants.GUEST_CPU_MODE, VmDetailConstants.GUEST_CPU_MODEL);

// Config keys
ConfigKey<Boolean> AllowUserViewDestroyedVM = new ConfigKey<>("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false",

Check failure on line 114 in api/src/main/java/org/apache/cloudstack/query/QueryService.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "false" 3 times.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ-FNt7fxCulrLz4jPM9&open=AZ-FNt7fxCulrLz4jPM9&pullRequest=13566
"Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account);

ConfigKey<String> UserVMDeniedDetails = new ConfigKey<>(String.class,
Expand Down Expand Up @@ -140,6 +140,12 @@
ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "true",
"Determines whether VM stats should be returned when details are not explicitly specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global);

ConfigKey<Boolean> BypassTemplateView = new ConfigKey<>("Advanced", Boolean.class, "template.list.bypass.view",
"false",
"If true, uses an optimized query path for listing templates and ISOs, which can improve performance on " +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: the concatenated description string has inconsistent leading/trailing space placement across the lines, worth double-checking the rendered text doesn't end up with a missing or doubled space.

"deployments with large numbers of templates. Automatically falls back to the standard query when" +
" filtering by tags, using the shared, featured, or community template filters, or when including" +
" removed templates.", true, ConfigKey.Scope.Global);

ListResponse<UserResponse> searchForUsers(ResponseObject.ResponseView responseView, ListUsersCmd cmd) throws PermissionDeniedException;

Expand Down
149 changes: 139 additions & 10 deletions server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@
import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao;
import com.cloud.cluster.ManagementServerHostPeerJoinVO;

import com.cloud.template.VirtualMachineTemplate;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: these new imports break alphabetical order here, and the same classes (AccountVO, SSHKeyPairVO, SSHKeyPairDao, InstanceGroupVMMapVO, NicVO, InstanceGroupVMMapDao, NicDao) get removed from their previously-correct alphabetized spots further down in this same diff.

import com.cloud.user.AccountVO;
import com.cloud.user.SSHKeyPairVO;
import com.cloud.user.dao.SSHKeyPairDao;
import com.cloud.vm.InstanceGroupVMMapVO;
import com.cloud.vm.NicVO;
import com.cloud.vm.dao.InstanceGroupVMMapDao;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.UserVmManager;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
Expand Down Expand Up @@ -207,6 +215,7 @@
import com.cloud.api.query.dao.SnapshotJoinDao;
import com.cloud.api.query.dao.StoragePoolJoinDao;
import com.cloud.api.query.dao.TemplateJoinDao;
import com.cloud.api.query.dao.TemplateListFilter;
import com.cloud.api.query.dao.UserAccountJoinDao;
import com.cloud.api.query.dao.UserVmJoinDao;
import com.cloud.api.query.dao.VolumeJoinDao;
Expand Down Expand Up @@ -332,12 +341,9 @@
import com.cloud.template.VirtualMachineTemplate.TemplateFilter;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.AccountVO;
import com.cloud.user.DomainManager;
import com.cloud.user.SSHKeyPairVO;
import com.cloud.user.User;
import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.SSHKeyPairDao;
import com.cloud.user.dao.UserDao;
import com.cloud.utils.DateUtil;
import com.cloud.utils.NumbersUtil;
Expand All @@ -353,17 +359,13 @@
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.DomainRouterVO;
import com.cloud.vm.InstanceGroupVMMapVO;
import com.cloud.vm.NicVO;
import com.cloud.vm.UserVmVO;
import com.cloud.vm.VMInstanceDetailVO;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.DomainRouterDao;
import com.cloud.vm.dao.InstanceGroupVMMapDao;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
import com.cloud.vm.dao.VMInstanceDetailsDao;
Expand Down Expand Up @@ -5089,7 +5091,11 @@
applyPublicTemplateSharingRestrictions(sc, caller);

return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc);
showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc,
buildTemplateListFilter(templateId, ids, name, keyword, templateFilter, isIso, bootable,
pageSize, startIndex, zoneId, hyperType, hypers, showDomr, onlyReady,
permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl,
parentTemplateId, showUnique));
}

/**
Expand Down Expand Up @@ -5144,7 +5150,8 @@
private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword,
HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller,
boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique, String templateType, Boolean isVnf, Boolean forCks,
Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) {
Filter searchFilter, SearchCriteria<TemplateJoinVO> sc,
TemplateListFilter listFilter) {
if (!isIso) {
// add hypervisor criteria for template case
if (hypers != null && !hypers.isEmpty()) {
Expand Down Expand Up @@ -5251,6 +5258,14 @@

// search unique templates and find details by Ids
Pair<List<TemplateJoinVO>, Integer> uniqueTmplPair;

// Only attempted when the bypass DAO supports the filter set; falls
// back to the SearchBuilder/template_view path below otherwise.
if (!showRemovedTmpl && BypassTemplateView.value() && listFilter != null && listFilter.canBypass()) {

Check warning on line 5264 in server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a primitive boolean expression here.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ-FNuETxCulrLz4jPNK&open=AZ-FNuETxCulrLz4jPNK&pullRequest=13566
uniqueTmplPair = _templateJoinDao.findDistinctTempZonePairs(listFilter);
return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique != null && showUnique, caller);
}

if (showRemovedTmpl) {
uniqueTmplPair = _templateJoinDao.searchIncludingRemovedAndCount(sc, searchFilter);
} else {
Expand All @@ -5272,6 +5287,119 @@
// about what the code was doing.
}

/**
* Build the immutable filter the bypass-the-view DAO consumes. Mirrors the
* SearchCriteria construction in {@link #searchForTemplatesInternal}; the
* SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds,
* domainPathLike, domainIdsForFeaturedCommunity, tags) drive
* {@link TemplateListFilter#canBypass()} — when any are populated,
* {@code templateChecks} routes to the legacy view-based path instead.
*/
private TemplateListFilter buildTemplateListFilter(Long templateId, List<Long> ids, String name, String keyword,

Check failure on line 5298 in server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 42 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ-FNuETxCulrLz4jPNM&open=AZ-FNuETxCulrLz4jPNM&pullRequest=13566

Check warning on line 5298 in server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Method has 21 parameters, which is greater than 7 authorized.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ-FNuETxCulrLz4jPNL&open=AZ-FNuETxCulrLz4jPNL&pullRequest=13566
TemplateFilter templateFilter, boolean isIso, Boolean bootable,
Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType,
List<HypervisorType> hypers, boolean showDomr, boolean onlyReady,
List<Account> permittedAccounts, Account caller,
ListProjectResourcesCriteria listProjectResourcesCriteria,
Map<String, String> tags, boolean showRemovedTmpl, Long parentTemplateId,
Boolean showUnique) {
TemplateListFilter.Builder b = TemplateListFilter.builder()
.templateId(templateId)
.ids(ids == null ? null : new ArrayList<>(ids))
.name(name)
.keyword(keyword)
.hypervisorType(hyperType != null && !HypervisorType.None.equals(hyperType) ? hyperType : null)
.availableHypervisors(hypers)
.format(ImageFormat.ISO) // bypass DAO uses isIso flag to flip between EQ and NEQ
.isIso(isIso)
.bootable(bootable)
.parentTemplateId(parentTemplateId)
.zoneId(zoneId)
.onlyReady(onlyReady)
.excludeSystemTemplates(!showDomr)
.showUnique(showUnique != null && showUnique)
.startIndex(startIndex)
.pageSize(pageSize)
.sortAscending(SortKeyAscending.value())
.showRemoved(showRemovedTmpl)
.tags(tags);

if (!showRemovedTmpl) {
b.templateStates(Arrays.asList(VirtualMachineTemplate.State.Active,
VirtualMachineTemplate.State.UploadAbandoned,
VirtualMachineTemplate.State.UploadError,
VirtualMachineTemplate.State.NotUploaded,
VirtualMachineTemplate.State.UploadInProgress));
}

// accountType filter (Project vs non-Project), applied only when no specific templateId.
if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
b.accountTypeNeq(Account.Type.PROJECT);
} else if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) {
b.accountTypeEq(Account.Type.PROJECT);
}

// ACL handling — mirrors the templateFilter switch in searchForTemplatesInternal.
// For "hard" templateFilter values, set requiresViewFallback so canBypass() returns
// false and the dispatcher falls back to the legacy view-based path.
if (templateId != null) {
return b.build();
}

List<Long> permittedAccountIds = new ArrayList<>();
for (Account account : permittedAccounts) {
permittedAccountIds.add(account.getId());
}

if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) {
// Walks the domain hierarchy and ORs domainId IS NULL — not yet modeled in bypass SQL.
b.publicTemplate(Boolean.TRUE);
b.featured(templateFilter == TemplateFilter.featured ? Boolean.TRUE : Boolean.FALSE);
b.requiresViewFallback(true);
} else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) {
if (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) {
// Match the existing path's domain resolution (see searchForTemplatesInternal:4498-4502):
// scope to the queried account's domain when one was specified, else the caller's.
Long domainIdForScope = !permittedAccounts.isEmpty()
? permittedAccounts.get(0).getDomainId()
: caller.getDomainId();
DomainVO scopeDomain = _domainDao.findById(domainIdForScope);
if (scopeDomain != null) {
b.domainPathLike(scopeDomain.getPath() + "%");
}
}
if (!permittedAccountIds.isEmpty()) {
b.accountIds(permittedAccountIds);
b.accountIdRequiredForFilter(true);
}
} else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) {
// launch_permission join — not modeled in bypass SQL.
b.requiresViewFallback(true);
} else if (templateFilter == TemplateFilter.executable) {
b.publicOrAccountIdComposite(true);
if (!permittedAccountIds.isEmpty()) {
b.accountIds(permittedAccountIds);
}
} else if (templateFilter == TemplateFilter.all && caller.getType() != Account.Type.ADMIN) {
// Non-admin "all" composite splits on listProjectResourcesCriteria:
// SkipProjectResources → (public OR domainPath LIKE) — bypass-eligible
// else → (public OR account_id IN OR sharedAccountId IN) — needs launch_permission, falls back
if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
DomainVO callerDomain = _domainDao.findById(caller.getDomainId());
if (callerDomain != null) {
b.publicOrDomainPathComposite(true);
b.domainPathLike(callerDomain.getPath() + "%");
} else {
b.requiresViewFallback(true);
}
} else {
b.requiresViewFallback(true);
}
}

return b.build();
}

// findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair
private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair,
boolean showRemoved, boolean showUnique, Account caller) {
Expand Down Expand Up @@ -6285,7 +6413,8 @@
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList};
AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains,
ReturnVmStatsOnVmList, BypassTemplateView };
}

@Override
Expand Down
15 changes: 15 additions & 0 deletions server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,20 @@ public interface TemplateJoinDao extends GenericDao<TemplateJoinVO, Long> {

Pair<List<TemplateJoinVO>, Integer> searchIncludingRemovedAndCount(final SearchCriteria<TemplateJoinVO> sc, final Filter filter);

/**
* Bypass-the-view Phase 1 implementation. Issues a hand-tuned SQL query
* directly against the underlying tables (vm_template, account,
* template_store_ref, image_store, template_zone_ref, data_center) instead
* of going through {@code template_view}. Caller must check
* {@link TemplateListFilter#canBypass()} first; this method does not handle
* tags, sharedAccountIds, domainPath, or featured/community-style domainId
* filters and will throw IllegalArgumentException if those are populated.
*
* Returns TemplateJoinVO objects with only {@code id} (when
* {@code filter.showUnique}) or {@code tempZonePair} populated, plus the
* total distinct count for pagination.
*/
Pair<List<TemplateJoinVO>, Integer> findDistinctTempZonePairs(TemplateListFilter filter);

List<TemplateJoinVO> findByDistinctIds(Long... ids);
}
Loading
Loading