[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6832: Allow root admin to deploy in VPCs in child domains

[GitBox <gi...@apache.org>]

weizhouapache commented on code in PR #6832:
URL: https://github.com/apache/cloudstack/pull/6832#discussion_r1046866576


##########
server/src/main/java/com/cloud/network/NetworkModelImpl.java:
##########
@@ -1665,39 +1666,49 @@ public void checkCapabilityForProvider(Set<Provider> providers, Service service,
     }
 
     @Override
-    public void checkNetworkPermissions(Account caller, Network network) {
-        // dahn 20140310: I was thinking of making this an assert but
-        //                as we hardly ever test with asserts I think
-        //                we better make sure at runtime.
-        if (network == null) {
-            throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
-        }
-        // Perform account permission check
-        if (network.getGuestType() != GuestType.Shared || network.getAclType() == ACLType.Account) {
-            AccountVO networkOwner = _accountDao.findById(network.getAccountId());
-            if (networkOwner == null)
-                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
-                    ", network does not have an owner");
-            if (!Account.Type.PROJECT.equals(caller.getType()) && Account.Type.PROJECT.equals(networkOwner.getType())) {
-                checkProjectNetworkPermissions(caller, networkOwner, network);
+    public final void checkNetworkPermissions(Account caller, Network network) {
+        if (_accountMgr.isRootAdmin(caller.getAccountId()) && Boolean.TRUE.equals(AdminIsAllowedToDeployAnywhere.value())) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("root admin is permitted to do stuff on every network");
+            }
+        } else {
+            if (network == null) {
+                throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
+            }
+            s_logger.info(String.format("Checking permission for account %s (%s) on network %s (%s)", caller.getAccountName(), caller.getUuid(), network.getName(), network.getUuid()));
+            if (network.getGuestType() != GuestType.Shared || network.getAclType() == ACLType.Account) {
+                checkSharedNetworkPermissions(caller, network);

Review Comment:
   @DaanHoogland 
   the name of the methods seem misunderstanding.
   `checkSharedNetworkPermissions` includes the check on L2/Isolated networks and Account-level share networks. in summary, the owner of the network is an account.
   `checkIsolatedNetworkPermissions` includes the check on Domain-level share networks.
   



##########
server/src/main/java/com/cloud/network/NetworkModelImpl.java:
##########
@@ -1665,39 +1666,49 @@ public void checkCapabilityForProvider(Set<Provider> providers, Service service,
     }
 
     @Override
-    public void checkNetworkPermissions(Account caller, Network network) {
-        // dahn 20140310: I was thinking of making this an assert but
-        //                as we hardly ever test with asserts I think
-        //                we better make sure at runtime.
-        if (network == null) {
-            throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
-        }
-        // Perform account permission check
-        if (network.getGuestType() != GuestType.Shared || network.getAclType() == ACLType.Account) {
-            AccountVO networkOwner = _accountDao.findById(network.getAccountId());
-            if (networkOwner == null)
-                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
-                    ", network does not have an owner");
-            if (!Account.Type.PROJECT.equals(caller.getType()) && Account.Type.PROJECT.equals(networkOwner.getType())) {
-                checkProjectNetworkPermissions(caller, networkOwner, network);
+    public final void checkNetworkPermissions(Account caller, Network network) {
+        if (_accountMgr.isRootAdmin(caller.getAccountId())) {
+            if (s_logger.isDebugEnabled()) {

Review Comment:
   ok @DaanHoogland 



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org