Skip to content

<fix>[compute]: fix VM clone quota check fail #3624

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
zstack-robot-1 wants to merge 1 commit into
base: 5.5.12
Choose a base branch
from
+97 −20
Open

<fix>[compute]: fix VM clone quota check fail #3624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 16 additions & 2 deletions compute/src/main/java/org/zstack/compute/allocator/QuotaAllocatorFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import org.springframework.beans.factory.annotation.Configurable;
import org.zstack.compute.vm.VmQuotaOperator;
import org.zstack.header.allocator.AbstractHostAllocatorFlow;
import org.zstack.header.errorcode.ErrorCode;
import org.zstack.header.errorcode.OperationFailureException;
import org.zstack.header.identity.AccountConstant;
import org.zstack.identity.Account;
import org.zstack.identity.QuotaUtil;
Expand Down Expand Up @@ -51,6 +53,12 @@ public void allocate() {
}

throwExceptionIfIAmTheFirstFlow();
// skip checkquota if the operator is admin
String sessionAccountUuid = spec.getSessionAccountUuid();
if (sessionAccountUuid != null && AccountConstant.isAdminPermission(sessionAccountUuid)) {
next(candidates);
return;
}
Comment on lines +56 to +61
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 检查 AccountConstant.isAdminPermission 的所有使用场景，以及是否有其他管理员检查方式
echo "=== AccountConstant.isAdminPermission 的实现 ==="
ast-grep --pattern 'static boolean isAdminPermission($$$) {
  $$$
}'

echo ""
echo "=== QuotaUtil.isAdminAccount 的使用 ==="
rg -n "isAdminAccount" --type java -C 2

Repository: MatheMatrix/zstack

Length of output: 8656

🏁 Script executed:

# View the relevant lines in QuotaAllocatorFlow.java to confirm the usage
echo "=== QuotaAllocatorFlow.java lines 56-65 ==="
sed -n '56,65p' compute/src/main/java/org/zstack/compute/allocator/QuotaAllocatorFlow.java

echo ""
echo "=== Search for AccountConstant.isAdminPermission usage in quota-related code ==="
rg -n "isAdminPermission.*skip|skip.*isAdminPermission" --type java

echo ""
echo "=== Check other quota flow or allocator files for comparison ==="
fd ".*Allocator.*\.java$|.*Quota.*\.java$" compute/src/main/java --exec grep -l "isAdminPermission\|isAdminAccount" {} \;

Repository: MatheMatrix/zstack

Length of output: 911

🏁 Script executed:

# Check how QuotaChecker.java and VmQuotaOperator.java handle the resource owner admin check
echo "=== QuotaChecker.java quota skip logic ==="
sed -n '35,50p' identity/src/main/java/org/zstack/identity/QuotaChecker.java

echo ""
echo "=== VmQuotaOperator.java quota check for resource owner ==="
sed -n '400,415p' compute/src/main/java/org/zstack/compute/vm/VmQuotaOperator.java

echo ""
echo "=== Check if there are existing quota checks in compute module ==="
rg -A 5 -B 5 "skip.*checkquota\|checkquota.*skip" compute/ --type java

Repository: MatheMatrix/zstack

Length of output: 1623

🏁 Script executed:

# Get the complete method context to understand the full quota check flow
echo "=== Full QuotaAllocatorFlow.java method context ==="
sed -n '50,100p' compute/src/main/java/org/zstack/compute/allocator/QuotaAllocatorFlow.java

echo ""
echo "=== Check QuotaUtil to understand when isAdminPermission vs isAdminAccount should be used ==="
sed -n '155,175p' identity/src/main/java/org/zstack/identity/QuotaUtil.java

Repository: MatheMatrix/zstack

Length of output: 2828

建议将管理员权限检查改为与其他配额检查保持一致。

在 QuotaAllocatorFlow.java 中，第 58 行和第 65 行使用 AccountConstant.isAdminPermission() 来跳过配额检查，但该方法仅检查账户 UUID 是否等于 INITIAL_SYSTEM_ADMIN_UUID（初始系统管理员）。

根据代码库中其他配额检查的实现模式，应该使用 QuotaUtil.isAdminAccount() 来检查，该方法会判断账户类型是否为 SystemAdmin（覆盖所有系统管理员账户）。具体体现在：

  • QuotaChecker.java 第 41 行使用 isAdminAccount()
  • VmQuotaOperator.java 第 408 行使用 isAdminAccount()
  • QuotaUtil.java 第 166 行采用组合检查：(isAdminPermission() || isAdminAccount())

建议将第 58 行和第 65 行改为使用 new QuotaUtil().isAdminAccount() 以保持配额检查逻辑的一致性。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/allocator/QuotaAllocatorFlow.java`
around lines 56 - 61, The admin-check currently in QuotaAllocatorFlow uses
AccountConstant.isAdminPermission(sessionAccountUuid) to skip quota checks;
change it to use QuotaUtil's account-type check so it matches other quota logic:
replace the AccountConstant.isAdminPermission(...) checks around
spec.getSessionAccountUuid() in QuotaAllocatorFlow with a call to new
QuotaUtil().isAdminAccount(...) (or the equivalent QuotaUtil.isAdminAccount
usage) so the flow treats all SystemAdmin accounts as admins consistent with
QuotaChecker and VmQuotaOperator.


final String vmInstanceUuid = spec.getVmInstance().getUuid();
final String accountUuid = Account.getAccountUuidOfResource(vmInstanceUuid);
Expand All @@ -60,21 +68,27 @@ public void allocate() {
}

if (!spec.isFullAllocate()) {
new VmQuotaOperator().checkVmCupAndMemoryCapacity(accountUuid,
ErrorCode error = new VmQuotaOperator().checkVmCupAndMemoryCapacityWithResult(accountUuid,
accountUuid,
spec.getCpuCapacity(),
spec.getMemoryCapacity(),
new QuotaUtil().makeQuotaPairs(accountUuid));
if (error != null) {
throw new OperationFailureException(error);
}

next(candidates);
return;
}

new VmQuotaOperator().checkVmInstanceQuota(
ErrorCode error = new VmQuotaOperator().checkVmInstanceQuotaWithResult(
accountUuid,
accountUuid,
vmInstanceUuid,
new QuotaUtil().makeQuotaPairs(accountUuid));
if (error != null) {
throw new OperationFailureException(error);
}
next(candidates);
}
}
71 changes: 56 additions & 15 deletions compute/src/main/java/org/zstack/compute/vm/VmQuotaOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.zstack.core.db.SimpleQuery;
import org.zstack.core.errorcode.ErrorFacade;
import org.zstack.header.apimediator.ApiMessageInterceptionException;
import org.zstack.header.errorcode.ErrorCode;
import org.zstack.header.identity.APIChangeResourceOwnerMsg;
import org.zstack.header.identity.AccountType;
import org.zstack.header.identity.Quota;
Expand Down Expand Up @@ -151,12 +152,27 @@ private void checkTotalVMQuota(String currentAccountUuid,
String vmInstanceUuid,
long totalVmNumQuota,
long totalVmNum) {
ErrorCode error = checkTotalVMQuotaWithResult(currentAccountUuid,
resourceTargetOwnerAccountUuid,
vmInstanceUuid,
totalVmNumQuota,
totalVmNum);
if (error != null) {
throw new ApiMessageInterceptionException(error);
}
}

private ErrorCode checkTotalVMQuotaWithResult(String currentAccountUuid,
String resourceTargetOwnerAccountUuid,
String vmInstanceUuid,
long totalVmNumQuota,
long totalVmNum) {
if (Q.New(VmInstanceVO.class)
.eq(VmInstanceVO_.uuid, vmInstanceUuid)
.notNull(VmInstanceVO_.lastHostUuid)
.isExists()) {
// Dirty hack - VM with last host UUID means existing VM.
return;
return null;
}

QuotaUtil.QuotaCompareInfo quotaCompareInfo;
Expand All @@ -167,18 +183,17 @@ private void checkTotalVMQuota(String currentAccountUuid,
quotaCompareInfo.quotaValue = totalVmNumQuota;
quotaCompareInfo.currentUsed = totalVmNum;
quotaCompareInfo.request = 1;
new QuotaUtil().CheckQuota(quotaCompareInfo);
return new QuotaUtil().checkQuotaAndReturn(quotaCompareInfo);
}

@Transactional(readOnly = true)
public void checkVmInstanceQuota(String currentAccountUuid,
String resourceTargetOwnerAccountUuid,
String vmInstanceUuid,
Map<String, Quota.QuotaPair> pairs) {
public ErrorCode checkVmInstanceQuotaWithResult(String currentAccountUuid,
String resourceTargetOwnerAccountUuid,
String vmInstanceUuid,
Map<String, Quota.QuotaPair> pairs) {
long vmNumQuota = pairs.get(VmQuotaConstant.VM_RUNNING_NUM).getValue();

VmQuotaUtil.VmQuota vmQuotaUsed = new VmQuotaUtil().getUsedVmCpuMemory(resourceTargetOwnerAccountUuid, null);
//
{
QuotaUtil.QuotaCompareInfo quotaCompareInfo;
quotaCompareInfo = new QuotaUtil.QuotaCompareInfo();
Expand All @@ -188,22 +203,38 @@ public void checkVmInstanceQuota(String currentAccountUuid,
quotaCompareInfo.quotaValue = vmNumQuota;
quotaCompareInfo.currentUsed = vmQuotaUsed.runningVmNum;
quotaCompareInfo.request = 1;
new QuotaUtil().CheckQuota(quotaCompareInfo);
ErrorCode error = new QuotaUtil().checkQuotaAndReturn(quotaCompareInfo);
if (error != null) {
return error;
}
}
//
checkTotalVMQuota(currentAccountUuid,

ErrorCode error = checkTotalVMQuotaWithResult(currentAccountUuid,
resourceTargetOwnerAccountUuid,
vmInstanceUuid,
pairs.get(VmQuotaConstant.VM_TOTAL_NUM).getValue(),
vmQuotaUsed.totalVmNum);
//
if (error != null) {
return error;
}

VmInstanceVO vm = dbf.getEntityManager().find(VmInstanceVO.class, vmInstanceUuid);

checkVmCupAndMemoryCapacity(currentAccountUuid, resourceTargetOwnerAccountUuid, vm.getCpuNum(), vm.getMemorySize(), pairs);
return checkVmCupAndMemoryCapacityWithResult(currentAccountUuid, resourceTargetOwnerAccountUuid, vm.getCpuNum(), vm.getMemorySize(), pairs);
}

public void checkVmInstanceQuota(String currentAccountUuid,
String resourceTargetOwnerAccountUuid,
String vmInstanceUuid,
Map<String, Quota.QuotaPair> pairs) {
ErrorCode error = checkVmInstanceQuotaWithResult(currentAccountUuid, resourceTargetOwnerAccountUuid, vmInstanceUuid, pairs);
if (error != null) {
throw new ApiMessageInterceptionException(error);
}
}

@Transactional(readOnly = true)
public void checkVmCupAndMemoryCapacity(String currentAccountUuid, String resourceTargetOwnerAccountUuid, long cpu, long memory, Map<String, Quota.QuotaPair> pairs) {
public ErrorCode checkVmCupAndMemoryCapacityWithResult(String currentAccountUuid, String resourceTargetOwnerAccountUuid, long cpu, long memory, Map<String, Quota.QuotaPair> pairs) {
VmQuotaUtil.VmQuota vmQuotaUsed = new VmQuotaUtil().getUsedVmCpuMemory(resourceTargetOwnerAccountUuid);
long cpuNumQuota = pairs.get(VmQuotaConstant.VM_RUNNING_CPU_NUM).getValue();
long memoryQuota = pairs.get(VmQuotaConstant.VM_RUNNING_MEMORY_SIZE).getValue();
Expand All @@ -217,7 +248,10 @@ public void checkVmCupAndMemoryCapacity(String currentAccountUuid, String resour
quotaCompareInfo.quotaValue = cpuNumQuota;
quotaCompareInfo.currentUsed = vmQuotaUsed.runningVmCpuNum;
quotaCompareInfo.request = cpu;
new QuotaUtil().CheckQuota(quotaCompareInfo);
ErrorCode error = new QuotaUtil().checkQuotaAndReturn(quotaCompareInfo);
if (error != null) {
return error;
}
}
{
QuotaUtil.QuotaCompareInfo quotaCompareInfo;
Expand All @@ -228,7 +262,14 @@ public void checkVmCupAndMemoryCapacity(String currentAccountUuid, String resour
quotaCompareInfo.quotaValue = memoryQuota;
quotaCompareInfo.currentUsed = vmQuotaUsed.runningVmMemorySize;
quotaCompareInfo.request = memory;
new QuotaUtil().CheckQuota(quotaCompareInfo);
return new QuotaUtil().checkQuotaAndReturn(quotaCompareInfo);
}
}

public void checkVmCupAndMemoryCapacity(String currentAccountUuid, String resourceTargetOwnerAccountUuid, long cpu, long memory, Map<String, Quota.QuotaPair> pairs) {
ErrorCode error = checkVmCupAndMemoryCapacityWithResult(currentAccountUuid, resourceTargetOwnerAccountUuid, cpu, memory, pairs);
if (error != null) {
throw new ApiMessageInterceptionException(error);
}
}

Expand Down
5 changes: 5 additions & 0 deletions header/src/main/java/org/zstack/header/allocator/AllocateHostMsg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class AllocateHostMsg extends NeedReplyMessage {
private long oldMemoryCapacity = 0;
private AllocationScene allocationScene;
private String architecture;
private String sessionAccountUuid;

public List<Set<String>> getOptionalPrimaryStorageUuids() {
return optionalPrimaryStorageUuids;
Expand Down Expand Up @@ -211,4 +212,8 @@ public String getArchitecture() {
public void setArchitecture(String architecture) {
this.architecture = architecture;
}

public String getSessionAccountUuid() { return sessionAccountUuid; }

public void setSessionAccountUuid(String sessionAccountUuid) { this.sessionAccountUuid = sessionAccountUuid; }
}
8 changes: 8 additions & 0 deletions header/src/main/java/org/zstack/header/allocator/HostAllocatorSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class HostAllocatorSpec {
private long oldMemoryCapacity = 0;
private AllocationScene allocationScene;
private String architecture;
private String sessionAccountUuid;

public AllocationScene getAllocationScene() {
return allocationScene;
Expand Down Expand Up @@ -161,7 +162,13 @@ public List<String> getL3NetworkUuids() {
}
return l3NetworkUuids;
}
public String getSessionAccountUuid() {
return sessionAccountUuid;
}

public void setSessionAccountUuid(String sessionAccountUuid) {
this.sessionAccountUuid = sessionAccountUuid;
}
public void setL3NetworkUuids(List<String> l3NetworkUuids) {
this.l3NetworkUuids = l3NetworkUuids;
}
Expand Down Expand Up @@ -250,6 +257,7 @@ public static HostAllocatorSpec fromAllocationMsg(AllocateHostMsg msg) {
msg.getOptionalPrimaryStorageUuids().forEach(spec::addOptionalPrimaryStorageUuids);
spec.setAllocationScene(msg.getAllocationScene());
spec.setArchitecture(msg.getArchitecture());
spec.setSessionAccountUuid(msg.getSessionAccountUuid());
if (msg.getSystemTags() != null && !msg.getSystemTags().isEmpty()){
spec.setSystemTags(new ArrayList<String>(msg.getSystemTags()));
}
Expand Down
15 changes: 12 additions & 3 deletions identity/src/main/java/org/zstack/identity/QuotaUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,31 @@ public String getResourceOwnerAccountUuid(String resourceUuid) {
}

@Transactional(readOnly = true)
public void CheckQuota(QuotaCompareInfo quotaCompareInfo) {
public ErrorCode checkQuotaAndReturn(QuotaCompareInfo quotaCompareInfo) {
logger.trace(String.format("dump quota QuotaCompareInfo: \n %s",
JSONObjectUtil.toJsonString(quotaCompareInfo)));
String accountName = Q.New(AccountVO.class)
.select(AccountVO_.name)
.eq(AccountVO_.uuid, quotaCompareInfo.resourceTargetOwnerAccountUuid)
.findValue();
if (quotaCompareInfo.currentUsed + quotaCompareInfo.request > quotaCompareInfo.quotaValue) {
throw new ApiMessageInterceptionException(err(ORG_ZSTACK_IDENTITY_10002, IdentityErrors.QUOTA_EXCEEDING,
return err(ORG_ZSTACK_IDENTITY_10002, IdentityErrors.QUOTA_EXCEEDING,
"quota exceeding." +
"The resource owner(or target resource owner) account[uuid: %s name: %s] exceeds a quota[name: %s, value: %s], " +
"Current used:%s, Request:%s. Please contact the administrator.",
quotaCompareInfo.resourceTargetOwnerAccountUuid, StringUtils.trimToEmpty(accountName),
quotaCompareInfo.quotaName, quotaCompareInfo.quotaValue,
quotaCompareInfo.currentUsed, quotaCompareInfo.request
));
);
}

return null;
}

public void CheckQuota(QuotaCompareInfo quotaCompareInfo) {
ErrorCode error = checkQuotaAndReturn(quotaCompareInfo);
if (error != null) {
throw new ApiMessageInterceptionException(error);
}
}

Expand Down