Copilot commented on code in PR #11390:
URL: https://github.com/apache/cloudstack/pull/11390#discussion_r2571199029
##########
api/src/main/java/org/apache/cloudstack/context/CallContext.java:
##########
@@ -134,6 +138,21 @@ public Account getCallingAccount() {
return account;
}
+ public boolean isCallingAccountRootAdmin() {
+ if (isAccountRootAdmin == null) {
+ AccountService accountService;
+ try {
+ accountService =
ComponentContext.getDelegateComponentOfType(AccountService.class);
+ } catch (NoSuchBeanDefinitionException e) {
+ LOGGER.warn("Falling back to account type check for
isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId,
e.getMessage());
+ Account caller = getCallingAccount();
+ return caller != null && caller.getType() ==
Account.Type.ADMIN;
+ }
+ isAccountRootAdmin =
accountService.isRootAdmin(getCallingAccount());
+ }
+ return Boolean.TRUE.equals(isAccountRootAdmin);
+ }
Review Comment:
The fallback logic at line 149 returns the result directly without caching
it in `isAccountRootAdmin`. This means if AccountService is unavailable,
subsequent calls will repeat the check instead of using the cached value.
Either cache the result before returning, or document this as intentional
behavior.
##########
api/src/main/java/org/apache/cloudstack/context/CallContext.java:
##########
@@ -134,6 +138,21 @@ public Account getCallingAccount() {
return account;
}
+ public boolean isCallingAccountRootAdmin() {
+ if (isAccountRootAdmin == null) {
+ AccountService accountService;
+ try {
+ accountService =
ComponentContext.getDelegateComponentOfType(AccountService.class);
+ } catch (NoSuchBeanDefinitionException e) {
+ LOGGER.warn("Falling back to account type check for
isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId,
e.getMessage());
+ Account caller = getCallingAccount();
+ return caller != null && caller.getType() ==
Account.Type.ADMIN;
Review Comment:
The fallback check `caller.getType() == Account.Type.ADMIN` is inconsistent
with the actual root admin check performed by AccountService, which uses
security checkers. This could lead to different behavior when AccountService is
unavailable. Consider removing the fallback entirely or throwing an exception
to fail fast when AccountService is not available.
```suggestion
throw new CloudRuntimeException("AccountService bean not
found, cannot determine if calling account is root admin for account ID: " +
accountId, e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
