You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/12/03 08:42:31 UTC

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5730: Enhancement: create Shared networks and VPC private gateways by users

DaanHoogland commented on a change in pull request #5730:
URL: https://github.com/apache/cloudstack/pull/5730#discussion_r761730336



##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -1703,6 +1690,109 @@ public void checkNetworkPermissions(Account owner, Network network) {
         }
     }
 
+    private void checkProjectNetworkPermissions(Account owner, Account networkOwner, Network network){
+        User user = CallContext.current().getCallingUser();
+        Project project = projectDao.findByProjectAccountId(networkOwner.getId());
+        if (project == null) {
+            throw new CloudRuntimeException("Unable to find project to which the network belongs to");
+        }
+        ProjectAccount projectAccountUser = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId());
+        if (projectAccountUser != null) {
+            if (!_projectAccountDao.canUserAccessProjectAccount(user.getAccountId(), user.getId(), networkOwner.getId())) {
+                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
+                        ", permission denied");
+            }
+        } else {
+            if (!_projectAccountDao.canAccessProjectAccount(owner.getAccountId(), networkOwner.getId())) {
+                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO) network).getUuid() +
+                        ", permission denied");
+            }
+        }
+    }
+
+    @Override
+    public void checkNetworkOperatePermissions(Account owner, Network network) {
+        if (network == null) {
+            throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
+        }
+        if (network.getGuestType() == GuestType.Shared) {
+            checkSharedNetworkOperatePermissions(owner, network);
+        } else {
+            checkNonSharedNetworkOperatePermissions(owner, network);
+        }
+    }
+
+    private void checkNonSharedNetworkOperatePermissions(Account owner, Network network) {
+        // check on isolated/L2 networks
+        if (owner.getType() == Account.ACCOUNT_TYPE_ADMIN) {
+            return;
+        }
+        Account networkOwner = _accountDao.findByIdIncludingRemoved(network.getAccountId());
+        if (owner.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
+            if (!_domainDao.isChildDomain(owner.getDomainId(), networkOwner.getDomainId())) {
+                throw new PermissionDeniedException(String.format("network %s cannot be operated by domain admin %s", network, owner));
+            }
+        } else if (owner.getType() == Account.ACCOUNT_TYPE_NORMAL) {
+            if (owner.getType() != Account.ACCOUNT_TYPE_PROJECT && networkOwner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                checkProjectNetworkPermissions(owner, networkOwner, network);
+            } else if (networkOwner.getAccountId() != owner.getAccountId()) {
+                throw new PermissionDeniedException(String.format("network %s cannot be operated by normal user %s", network, owner));
+            }
+        } else {
+            throw new PermissionDeniedException(String.format("network %s cannot be operated by this account %s", network, owner));
+        }
+    }
+
+    private void checkSharedNetworkOperatePermissions(Account owner, Network network) {
+        NetworkOffering networkOffering = _networkOfferingDao.findById(network.getNetworkOfferingId());
+        if (networkOffering.isSpecifyVlan() && owner.getType() != Account.ACCOUNT_TYPE_ADMIN) {
+            throw new PermissionDeniedException(String.format("Shared network %s with specifyvlan=true can only be operated by root admin", network));
+        }
+        if (owner.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
+            if (network.getAclType() == ACLType.Domain) {
+                // Allow domain admins to operate shared network for their domain.
+                NetworkDomainVO networkDomainMap = _networkDomainDao.getDomainNetworkMapByNetworkId(network.getId());
+                if (networkDomainMap == null) {
+                    throw new CloudRuntimeException(String.format("Cannot find domain info for Shared network %s with aclType=Domain", network));
+                }
+                if (!_domainDao.isChildDomain(owner.getDomainId(), networkDomainMap.getDomainId())) {
+                    throw new PermissionDeniedException(String.format("Shared network %s cannot be operated by domain admin %s", network, owner));
+                }
+            } else if (network.getAclType() == ACLType.Account) {
+                // Allow domain admins to operate shared network for an account in their domain.
+                NetworkAccountVO networkAccountMap = _networkAccountDao.getAccountNetworkMapByNetworkId(network.getId());
+                if (networkAccountMap == null) {
+                    throw new CloudRuntimeException(String.format("Cannot find account info for Shared network %s with aclType=Account", network));
+                }
+                if (!_domainDao.isChildDomain(owner.getDomainId(), _accountDao.findByIdIncludingRemoved(networkAccountMap.getAccountId()).getDomainId())) {
+                    throw new PermissionDeniedException(String.format("Shared network %s cannot be operated by domain admin %s", network, owner));

Review comment:
       this has the exact same message text as line https://github.com/apache/cloudstack/pull/5730/files#diff-05d77af980dadebef650b49cb8ab34f4d0040fa18ec7643cd2c945de5227933dR1759
   maybe externalise or include the reason for throwing in the text so it can be identified more easily?

##########
File path: test/integration/smoke/test_user_shared_network.py
##########
@@ -0,0 +1,634 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests of user-shared networks
+"""
+
+import logging
+import time
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources, random_gen
+
+from marvin.lib.base import (Account,
+                             Domain,
+                             Project,
+                             Configurations,
+                             ServiceOffering,
+                             Zone,
+                             Network,
+                             NetworkOffering,
+                             VPC,
+                             VpcOffering)
+
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template)
+
+NETWORK_STATE_ALLOCATED = "Allocated"
+NETWORK_STATE_IMPLEMENTED = "Implemented"
+NETWORK_STATE_SETUP = "Setup"
+NETWORK_STATE_REMOVED = "Removed"
+
+class TestUserSharedNetworks(cloudstackTestCase):
+    """
+    Test user-shared networks
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestUserSharedNetworks,
+            cls).getClsTestClient()
+        cls.admin_apiclient = cls.testClient.getApiClient()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+
+        zone = get_zone(cls.admin_apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.admin_apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestUserSharedNetworks")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        cls.domain = get_domain(cls.admin_apiclient)
+
+        # Create sub-domain
+        cls.sub_domain = Domain.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["domain1"]
+        )
+
+        # Create domain admin and normal user
+        cls.domain_admin = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1A"],
+            admin=True,
+            domainid=cls.sub_domain.id
+        )
+        cls.normal_user = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1B"],
+            domainid=cls.sub_domain.id
+        )
+        # Create project
+        cls.project = Project.create(
+          cls.admin_apiclient,
+          cls.services["project"],
+          account=cls.domain_admin.name,
+          domainid=cls.domain_admin.domainid
+        )
+        cls._cleanup.append(cls.project)
+        cls._cleanup.append(cls.domain_admin)
+        cls._cleanup.append(cls.normal_user)
+        cls._cleanup.append(cls.sub_domain)
+
+        # Create small service offering
+        cls.service_offering = ServiceOffering.create(
+            cls.admin_apiclient,
+            cls.services["service_offerings"]["small"]
+        )
+        cls._cleanup.append(cls.service_offering)
+
+        # Create network offering for user-shared networks (specifyVlan=true)
+        cls.network_offering_withvlan = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering_shared"]
+        )
+        cls.network_offering_withvlan.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_withvlan)
+
+        # Create network offering for user-shared networks (specifyVlan=false)
+        cls.services["network_offering_shared"]["specifyVlan"] = "False"
+        cls.network_offering_novlan = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering_shared"]
+        )
+        cls.network_offering_novlan.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_novlan)
+
+        # Create network offering for isolated networks
+        cls.network_offering_isolated = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering"]
+        )
+        cls.network_offering_isolated.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_isolated)
+
+        # Create vpc offering
+        cls.vpc_offering = VpcOffering.create(
+            cls.admin_apiclient,
+            cls.services["vpc_offering_multi_lb"])
+        cls.vpc_offering.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.vpc_offering)
+
+        # Create network offering for vpc tiers
+        cls.network_offering_vpc = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["nw_offering_isolated_vpc"],
+            conservemode=False
+        )
+        cls.network_offering_vpc.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_vpc)
+
+        # Create api clients for domain admin and normal user
+        cls.domainadmin_user = cls.domain_admin.user[0]
+        cls.domainadmin_apiclient = cls.testClient.getUserApiClient(
+            cls.domainadmin_user.username, cls.sub_domain.name
+        )
+        cls.normaluser_user = cls.normal_user.user[0]
+        cls.normaluser_apiclient = cls.testClient.getUserApiClient(
+            cls.normaluser_user.username, cls.sub_domain.name
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cleanup_resources(cls.admin_apiclient, cls._cleanup)
+        except Exception as ex:
+            raise Exception(f"Warning: Exception during cleanup : {ex}") from ex

Review comment:
       super call

##########
File path: test/integration/smoke/test_user_private_gateway.py
##########
@@ -0,0 +1,428 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests of user-private gateway
+"""
+
+import logging
+import time
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources, random_gen
+
+from marvin.lib.base import (Account,
+                             Domain,
+                             Project,
+                             Configurations,
+                             ServiceOffering,
+                             Zone,
+                             Network,
+                             NetworkOffering,
+                             VPC,
+                             VpcOffering,
+                             PrivateGateway)
+
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template)
+
+NETWORK_STATE_ALLOCATED = "Allocated"
+NETWORK_STATE_IMPLEMENTED = "Implemented"
+NETWORK_STATE_SETUP = "Setup"
+NETWORK_STATE_REMOVED = "Removed"
+
+class TestUserPrivateGateways(cloudstackTestCase):
+    """
+    Test user-shared networks
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestUserPrivateGateways,
+            cls).getClsTestClient()
+        cls.admin_apiclient = cls.testClient.getApiClient()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+
+        zone = get_zone(cls.admin_apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.admin_apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestUserPrivateGateways")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        cls.domain = get_domain(cls.admin_apiclient)
+
+        # Create sub-domain
+        cls.sub_domain = Domain.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["domain1"]
+        )
+
+        # Create domain admin and normal user
+        cls.domain_admin = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1A"],
+            admin=True,
+            domainid=cls.sub_domain.id
+        )
+        cls.normal_user = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1B"],
+            domainid=cls.sub_domain.id
+        )
+        # Create project
+        cls.project = Project.create(
+          cls.admin_apiclient,
+          cls.services["project"],
+          account=cls.domain_admin.name,
+          domainid=cls.domain_admin.domainid
+        )
+        cls._cleanup.append(cls.project)
+        cls._cleanup.append(cls.domain_admin)
+        cls._cleanup.append(cls.normal_user)
+        cls._cleanup.append(cls.sub_domain)
+

Review comment:
       please add in order not in reverse order. there is a super call that handles reversing the item order for cleanup

##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -1703,6 +1690,109 @@ public void checkNetworkPermissions(Account owner, Network network) {
         }
     }
 
+    private void checkProjectNetworkPermissions(Account owner, Account networkOwner, Network network){
+        User user = CallContext.current().getCallingUser();
+        Project project = projectDao.findByProjectAccountId(networkOwner.getId());
+        if (project == null) {
+            throw new CloudRuntimeException("Unable to find project to which the network belongs to");
+        }
+        ProjectAccount projectAccountUser = _projectAccountDao.findByProjectIdUserId(project.getId(), user.getAccountId(), user.getId());
+        if (projectAccountUser != null) {
+            if (!_projectAccountDao.canUserAccessProjectAccount(user.getAccountId(), user.getId(), networkOwner.getId())) {
+                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() +
+                        ", permission denied");
+            }
+        } else {
+            if (!_projectAccountDao.canAccessProjectAccount(owner.getAccountId(), networkOwner.getId())) {
+                throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO) network).getUuid() +
+                        ", permission denied");
+            }
+        }
+    }
+
+    @Override
+    public void checkNetworkOperatePermissions(Account owner, Network network) {
+        if (network == null) {
+            throw new CloudRuntimeException("cannot check permissions on (Network) <null>");
+        }
+        if (network.getGuestType() == GuestType.Shared) {
+            checkSharedNetworkOperatePermissions(owner, network);
+        } else {
+            checkNonSharedNetworkOperatePermissions(owner, network);
+        }
+    }
+
+    private void checkNonSharedNetworkOperatePermissions(Account owner, Network network) {
+        // check on isolated/L2 networks
+        if (owner.getType() == Account.ACCOUNT_TYPE_ADMIN) {
+            return;
+        }
+        Account networkOwner = _accountDao.findByIdIncludingRemoved(network.getAccountId());
+        if (owner.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
+            if (!_domainDao.isChildDomain(owner.getDomainId(), networkOwner.getDomainId())) {
+                throw new PermissionDeniedException(String.format("network %s cannot be operated by domain admin %s", network, owner));
+            }
+        } else if (owner.getType() == Account.ACCOUNT_TYPE_NORMAL) {
+            if (owner.getType() != Account.ACCOUNT_TYPE_PROJECT && networkOwner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                checkProjectNetworkPermissions(owner, networkOwner, network);
+            } else if (networkOwner.getAccountId() != owner.getAccountId()) {
+                throw new PermissionDeniedException(String.format("network %s cannot be operated by normal user %s", network, owner));
+            }
+        } else {
+            throw new PermissionDeniedException(String.format("network %s cannot be operated by this account %s", network, owner));
+        }
+    }
+
+    private void checkSharedNetworkOperatePermissions(Account owner, Network network) {
+        NetworkOffering networkOffering = _networkOfferingDao.findById(network.getNetworkOfferingId());
+        if (networkOffering.isSpecifyVlan() && owner.getType() != Account.ACCOUNT_TYPE_ADMIN) {
+            throw new PermissionDeniedException(String.format("Shared network %s with specifyvlan=true can only be operated by root admin", network));
+        }
+        if (owner.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
+            if (network.getAclType() == ACLType.Domain) {
+                // Allow domain admins to operate shared network for their domain.
+                NetworkDomainVO networkDomainMap = _networkDomainDao.getDomainNetworkMapByNetworkId(network.getId());
+                if (networkDomainMap == null) {
+                    throw new CloudRuntimeException(String.format("Cannot find domain info for Shared network %s with aclType=Domain", network));
+                }
+                if (!_domainDao.isChildDomain(owner.getDomainId(), networkDomainMap.getDomainId())) {
+                    throw new PermissionDeniedException(String.format("Shared network %s cannot be operated by domain admin %s", network, owner));
+                }
+            } else if (network.getAclType() == ACLType.Account) {
+                // Allow domain admins to operate shared network for an account in their domain.
+                NetworkAccountVO networkAccountMap = _networkAccountDao.getAccountNetworkMapByNetworkId(network.getId());
+                if (networkAccountMap == null) {
+                    throw new CloudRuntimeException(String.format("Cannot find account info for Shared network %s with aclType=Account", network));
+                }
+                if (!_domainDao.isChildDomain(owner.getDomainId(), _accountDao.findByIdIncludingRemoved(networkAccountMap.getAccountId()).getDomainId())) {
+                    throw new PermissionDeniedException(String.format("Shared network %s cannot be operated by domain admin %s", network, owner));
+                }
+            }
+        } else if (owner.getType() == Account.ACCOUNT_TYPE_NORMAL) {
+            // Allow normal users to operate shared network for themselves.
+            if (network.getAclType() == ACLType.Account) {
+                // Allow domain admin to operate shared network for an account in its domain.
+                NetworkAccountVO networkAccountMap = _networkAccountDao.getAccountNetworkMapByNetworkId(network.getId());
+                if (networkAccountMap == null) {
+                    throw new CloudRuntimeException(String.format("Cannot find account info for Shared network %s with aclType=Account", network));

Review comment:
       this is the same message as https://github.com/apache/cloudstack/pull/5730/files#diff-05d77af980dadebef650b49cb8ab34f4d0040fa18ec7643cd2c945de5227933dR1765

##########
File path: test/integration/smoke/test_user_shared_network.py
##########
@@ -0,0 +1,634 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests of user-shared networks
+"""
+
+import logging
+import time
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources, random_gen
+
+from marvin.lib.base import (Account,
+                             Domain,
+                             Project,
+                             Configurations,
+                             ServiceOffering,
+                             Zone,
+                             Network,
+                             NetworkOffering,
+                             VPC,
+                             VpcOffering)
+
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template)
+
+NETWORK_STATE_ALLOCATED = "Allocated"
+NETWORK_STATE_IMPLEMENTED = "Implemented"
+NETWORK_STATE_SETUP = "Setup"
+NETWORK_STATE_REMOVED = "Removed"
+
+class TestUserSharedNetworks(cloudstackTestCase):
+    """
+    Test user-shared networks
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestUserSharedNetworks,
+            cls).getClsTestClient()
+        cls.admin_apiclient = cls.testClient.getApiClient()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+
+        zone = get_zone(cls.admin_apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.admin_apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestUserSharedNetworks")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        cls.domain = get_domain(cls.admin_apiclient)
+
+        # Create sub-domain
+        cls.sub_domain = Domain.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["domain1"]
+        )
+
+        # Create domain admin and normal user
+        cls.domain_admin = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1A"],
+            admin=True,
+            domainid=cls.sub_domain.id
+        )
+        cls.normal_user = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1B"],
+            domainid=cls.sub_domain.id
+        )
+        # Create project
+        cls.project = Project.create(
+          cls.admin_apiclient,
+          cls.services["project"],
+          account=cls.domain_admin.name,
+          domainid=cls.domain_admin.domainid
+        )
+        cls._cleanup.append(cls.project)
+        cls._cleanup.append(cls.domain_admin)
+        cls._cleanup.append(cls.normal_user)
+        cls._cleanup.append(cls.sub_domain)
+
+        # Create small service offering
+        cls.service_offering = ServiceOffering.create(
+            cls.admin_apiclient,
+            cls.services["service_offerings"]["small"]
+        )
+        cls._cleanup.append(cls.service_offering)
+
+        # Create network offering for user-shared networks (specifyVlan=true)
+        cls.network_offering_withvlan = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering_shared"]
+        )
+        cls.network_offering_withvlan.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_withvlan)
+
+        # Create network offering for user-shared networks (specifyVlan=false)
+        cls.services["network_offering_shared"]["specifyVlan"] = "False"
+        cls.network_offering_novlan = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering_shared"]
+        )
+        cls.network_offering_novlan.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_novlan)
+
+        # Create network offering for isolated networks
+        cls.network_offering_isolated = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering"]
+        )
+        cls.network_offering_isolated.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_isolated)
+
+        # Create vpc offering
+        cls.vpc_offering = VpcOffering.create(
+            cls.admin_apiclient,
+            cls.services["vpc_offering_multi_lb"])
+        cls.vpc_offering.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.vpc_offering)
+
+        # Create network offering for vpc tiers
+        cls.network_offering_vpc = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["nw_offering_isolated_vpc"],
+            conservemode=False
+        )
+        cls.network_offering_vpc.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_vpc)
+
+        # Create api clients for domain admin and normal user
+        cls.domainadmin_user = cls.domain_admin.user[0]
+        cls.domainadmin_apiclient = cls.testClient.getUserApiClient(
+            cls.domainadmin_user.username, cls.sub_domain.name
+        )
+        cls.normaluser_user = cls.normal_user.user[0]
+        cls.normaluser_apiclient = cls.testClient.getUserApiClient(
+            cls.normaluser_user.username, cls.sub_domain.name
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cleanup_resources(cls.admin_apiclient, cls._cleanup)
+        except Exception as ex:
+            raise Exception(f"Warning: Exception during cleanup : {ex}") from ex
+
+    def setUp(self):
+        self.cleanup = []
+
+    def tearDown(self):
+        try:
+            cleanup_resources(self.admin_apiclient, self.cleanup)
+        except Exception as ex:
+            raise Exception(f"Warning: Exception during cleanup : {ex}") from ex

Review comment:
       supercall

##########
File path: test/integration/smoke/test_user_private_gateway.py
##########
@@ -0,0 +1,428 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests of user-private gateway
+"""
+
+import logging
+import time
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources, random_gen
+
+from marvin.lib.base import (Account,
+                             Domain,
+                             Project,
+                             Configurations,
+                             ServiceOffering,
+                             Zone,
+                             Network,
+                             NetworkOffering,
+                             VPC,
+                             VpcOffering,
+                             PrivateGateway)
+
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template)
+
+NETWORK_STATE_ALLOCATED = "Allocated"
+NETWORK_STATE_IMPLEMENTED = "Implemented"
+NETWORK_STATE_SETUP = "Setup"
+NETWORK_STATE_REMOVED = "Removed"
+
+class TestUserPrivateGateways(cloudstackTestCase):
+    """
+    Test user-shared networks
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestUserPrivateGateways,
+            cls).getClsTestClient()
+        cls.admin_apiclient = cls.testClient.getApiClient()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+
+        zone = get_zone(cls.admin_apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.admin_apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestUserPrivateGateways")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        cls.domain = get_domain(cls.admin_apiclient)
+
+        # Create sub-domain
+        cls.sub_domain = Domain.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["domain1"]
+        )
+
+        # Create domain admin and normal user
+        cls.domain_admin = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1A"],
+            admin=True,
+            domainid=cls.sub_domain.id
+        )
+        cls.normal_user = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1B"],
+            domainid=cls.sub_domain.id
+        )
+        # Create project
+        cls.project = Project.create(
+          cls.admin_apiclient,
+          cls.services["project"],
+          account=cls.domain_admin.name,
+          domainid=cls.domain_admin.domainid
+        )
+        cls._cleanup.append(cls.project)
+        cls._cleanup.append(cls.domain_admin)
+        cls._cleanup.append(cls.normal_user)
+        cls._cleanup.append(cls.sub_domain)
+
+        # Create small service offering
+        cls.service_offering = ServiceOffering.create(
+            cls.admin_apiclient,
+            cls.services["service_offerings"]["small"]
+        )
+        cls._cleanup.append(cls.service_offering)
+
+        # Create network offering for isolated networks
+        cls.network_offering_isolated = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["network_offering"]
+        )
+        cls.network_offering_isolated.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_isolated)
+
+        # Create vpc offering
+        cls.vpc_offering = VpcOffering.create(
+            cls.admin_apiclient,
+            cls.services["vpc_offering_multi_lb"])
+        cls.vpc_offering.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.vpc_offering)
+
+        # Create network offering for vpc tiers
+        cls.network_offering_vpc = NetworkOffering.create(
+            cls.admin_apiclient,
+            cls.services["nw_offering_isolated_vpc"],
+            conservemode=False
+        )
+        cls.network_offering_vpc.update(cls.admin_apiclient, state='Enabled')
+        cls._cleanup.append(cls.network_offering_vpc)
+
+        # Create api clients for domain admin and normal user
+        cls.domainadmin_user = cls.domain_admin.user[0]
+        cls.domainadmin_apiclient = cls.testClient.getUserApiClient(
+            cls.domainadmin_user.username, cls.sub_domain.name
+        )
+        cls.normaluser_user = cls.normal_user.user[0]
+        cls.normaluser_apiclient = cls.testClient.getUserApiClient(
+            cls.normaluser_user.username, cls.sub_domain.name
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cleanup_resources(cls.admin_apiclient, cls._cleanup)
+        except Exception as ex:
+            raise Exception(f"Warning: Exception during cleanup : {ex}") from ex
+
+    def setUp(self):
+        self.cleanup = []
+
+    def tearDown(self):
+        try:
+            cleanup_resources(self.admin_apiclient, self.cleanup)
+        except Exception as ex:
+            raise Exception(f"Warning: Exception during cleanup : {ex}") from ex

Review comment:
       please use a super call

##########
File path: test/integration/smoke/test_user_shared_network.py
##########
@@ -0,0 +1,634 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Tests of user-shared networks
+"""
+
+import logging
+import time
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.utils import cleanup_resources, random_gen
+
+from marvin.lib.base import (Account,
+                             Domain,
+                             Project,
+                             Configurations,
+                             ServiceOffering,
+                             Zone,
+                             Network,
+                             NetworkOffering,
+                             VPC,
+                             VpcOffering)
+
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template)
+
+NETWORK_STATE_ALLOCATED = "Allocated"
+NETWORK_STATE_IMPLEMENTED = "Implemented"
+NETWORK_STATE_SETUP = "Setup"
+NETWORK_STATE_REMOVED = "Removed"
+
+class TestUserSharedNetworks(cloudstackTestCase):
+    """
+    Test user-shared networks
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(
+            TestUserSharedNetworks,
+            cls).getClsTestClient()
+        cls.admin_apiclient = cls.testClient.getApiClient()
+        cls.services = cls.testClient.getParsedTestDataConfig()
+
+        zone = get_zone(cls.admin_apiclient, cls.testClient.getZoneForTests())
+        cls.zone = Zone(zone.__dict__)
+        cls.template = get_template(cls.admin_apiclient, cls.zone.id)
+        cls._cleanup = []
+
+        cls.logger = logging.getLogger("TestUserSharedNetworks")
+        cls.stream_handler = logging.StreamHandler()
+        cls.logger.setLevel(logging.DEBUG)
+        cls.logger.addHandler(cls.stream_handler)
+
+        cls.domain = get_domain(cls.admin_apiclient)
+
+        # Create sub-domain
+        cls.sub_domain = Domain.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["domain1"]
+        )
+
+        # Create domain admin and normal user
+        cls.domain_admin = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1A"],
+            admin=True,
+            domainid=cls.sub_domain.id
+        )
+        cls.normal_user = Account.create(
+            cls.admin_apiclient,
+            cls.services["acl"]["accountD1B"],
+            domainid=cls.sub_domain.id
+        )
+        # Create project
+        cls.project = Project.create(
+          cls.admin_apiclient,
+          cls.services["project"],
+          account=cls.domain_admin.name,
+          domainid=cls.domain_admin.domainid
+        )
+        cls._cleanup.append(cls.project)
+        cls._cleanup.append(cls.domain_admin)
+        cls._cleanup.append(cls.normal_user)
+        cls._cleanup.append(cls.sub_domain)

Review comment:
       please add in order and call super(TestUserSharedNetworks, cls).tearDownClass() to handle the reversing




-- 
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