user-shared networks: fix few issues (#6887)

This PR fixes few issues:

    - check ip range of new network instead of network cidr, so that the two networks can use same cidr but no IP conflicts.
    - Private gateways: return vlan number only for root admins
    - when update isolated network, check new guest vm cidr and IPs of neworks/vpc gateways associated to it

Author: weizhouapache

api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java

@@ -438,7 +438,7 @@ List<TemplateResponse> createTemplateResponses(ResponseView view, VirtualMachine
      * @param result
      * @return
      */
-    PrivateGatewayResponse createPrivateGatewayResponse(PrivateGateway result);
+    PrivateGatewayResponse createPrivateGatewayResponse(ResponseView view, PrivateGateway result);
 
     /**
      * @param result

api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/ListPrivateGatewaysCmdByAdminCmd.java

@@ -0,0 +1,35 @@
+// 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.
+package org.apache.cloudstack.api.command.admin.vpc;
+
+import org.apache.log4j.Logger;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.command.admin.AdminCmd;
+import org.apache.cloudstack.api.command.user.vpc.ListPrivateGatewaysCmd;
+import org.apache.cloudstack.api.response.PrivateGatewayResponse;
+
+import com.cloud.network.vpc.VpcGateway;
+
+@APICommand(name = "listPrivateGateways", description = "List private gateways", responseObject = PrivateGatewayResponse.class, entityType = {VpcGateway.class},
+        responseView = ResponseObject.ResponseView.Full,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ListPrivateGatewaysCmdByAdminCmd extends ListPrivateGatewaysCmd implements AdminCmd {
+    public static final Logger s_logger = Logger.getLogger(ListPrivateGatewaysCmdByAdminCmd.class.getName());
+
+}

api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreatePrivateGatewayCmd.java

@@ -169,7 +169,7 @@ public void create() throws ResourceAllocationException {
     public void execute() throws InsufficientCapacityException, ConcurrentOperationException, ResourceAllocationException, ResourceUnavailableException {
         PrivateGateway result = _vpcService.applyVpcPrivateGateway(getEntityId(), true);
         if (result != null) {
-            PrivateGatewayResponse response = _responseGenerator.createPrivateGatewayResponse(result);
+            PrivateGatewayResponse response = _responseGenerator.createPrivateGatewayResponse(getResponseView(), result);
             response.setResponseName(getCommandName());
             setResponseObject(response);
         } else {

api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListPrivateGatewaysCmd.java

@@ -25,6 +25,8 @@
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd;
 import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.command.user.UserCmd;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.PrivateGatewayResponse;
 import org.apache.cloudstack.api.response.VpcResponse;
@@ -34,8 +36,9 @@
 import com.cloud.utils.Pair;
 
 @APICommand(name = "listPrivateGateways", description = "List private gateways", responseObject = PrivateGatewayResponse.class, entityType = {VpcGateway.class},
+        responseView = ResponseObject.ResponseView.Restricted,
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
-public class ListPrivateGatewaysCmd extends BaseListProjectAndAccountResourcesCmd {
+public class ListPrivateGatewaysCmd extends BaseListProjectAndAccountResourcesCmd implements UserCmd {
     public static final Logger s_logger = Logger.getLogger(ListPrivateGatewaysCmd.class.getName());
 
 
@@ -90,7 +93,7 @@ public void execute() {
         ListResponse<PrivateGatewayResponse> response = new ListResponse<PrivateGatewayResponse>();
         List<PrivateGatewayResponse> projectResponses = new ArrayList<PrivateGatewayResponse>();
         for (PrivateGateway gateway : gateways.first()) {
-            PrivateGatewayResponse gatewayResponse = _responseGenerator.createPrivateGatewayResponse(gateway);
+            PrivateGatewayResponse gatewayResponse = _responseGenerator.createPrivateGatewayResponse(getResponseView(), gateway);
             projectResponses.add(gatewayResponse);
         }
         response.setResponses(projectResponses, gateways.second());

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -3357,10 +3357,12 @@ public VpcResponse createVpcResponse(ResponseView view, Vpc vpc) {
     }
 
     @Override
-    public PrivateGatewayResponse createPrivateGatewayResponse(PrivateGateway result) {
+    public PrivateGatewayResponse createPrivateGatewayResponse(ResponseView view, PrivateGateway result) {
         PrivateGatewayResponse response = new PrivateGatewayResponse();
         response.setId(result.getUuid());
-        response.setBroadcastUri(result.getBroadcastUri());
+        if (view == ResponseView.Full) {
+            response.setBroadcastUri(result.getBroadcastUri());
+        }
         response.setGateway(result.getGateway());
         response.setNetmask(result.getNetmask());
         if (result.getVpcId() != null) {

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

@@ -178,11 +178,13 @@
 import com.cloud.network.vpc.NetworkACL;
 import com.cloud.network.vpc.PrivateIpVO;
 import com.cloud.network.vpc.Vpc;
+import com.cloud.network.vpc.VpcGatewayVO;
 import com.cloud.network.vpc.VpcManager;
 import com.cloud.network.vpc.VpcVO;
 import com.cloud.network.vpc.dao.NetworkACLDao;
 import com.cloud.network.vpc.dao.PrivateIpDao;
 import com.cloud.network.vpc.dao.VpcDao;
+import com.cloud.network.vpc.dao.VpcGatewayDao;
 import com.cloud.network.vpc.dao.VpcOfferingDao;
 import com.cloud.offering.NetworkOffering;
 import com.cloud.offerings.NetworkOfferingVO;
@@ -388,6 +390,8 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
     @Inject
     Ipv6GuestPrefixSubnetNetworkMapDao ipv6GuestPrefixSubnetNetworkMapDao;
     @Inject
+    VpcGatewayDao vpcGatewayDao;
+    @Inject
     AlertManager alertManager;
     @Inject
     DomainRouterDao routerDao;
@@ -1939,24 +1943,17 @@ private Network implementAssociatedNetwork(Long associatedNetworkId, Account cal
         if (domainId != null && associatedNetwork.getDomainId() != domainId) {
             throw new InvalidParameterValueException("The new network and associated network MUST be in same domain");
         }
-        if (cidr != null && associatedNetwork.getCidr() != null && NetUtils.isNetworksOverlap(cidr, associatedNetwork.getCidr())) {
-            throw new InvalidParameterValueException("The cidr overlaps with associated network: " + associatedNetwork.getName());
-        }
-        List<NetworkDetailVO> associatedNetworks = _networkDetailsDao.findDetails(Network.AssociatedNetworkId, String.valueOf(associatedNetworkId), null);
-        for (NetworkDetailVO networkDetailVO : associatedNetworks) {
-            NetworkVO associatedNetwork2 = _networksDao.findById(networkDetailVO.getResourceId());
-            if (associatedNetwork2 != null) {
-                List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(associatedNetwork2.getId());
-                if (vlans.isEmpty()) {
-                    continue;
-                }
-                String startIP2 = vlans.get(0).getIpRange().split("-")[0];
-                String endIP2 = vlans.get(0).getIpRange().split("-")[1];
-                if (StringUtils.isNoneBlank(startIp, startIP2) && NetUtils.ipRangesOverlap(startIp, endIp, startIP2, endIP2)) {
-                    throw new InvalidParameterValueException("The startIp/endIp overlaps with network: " + associatedNetwork2.getName());
-                }
+        if (cidr != null && associatedNetwork.getCidr() != null) {
+            String[] guestVmCidrPair = associatedNetwork.getCidr().split("\\/");
+            String[] cidrIpRange = NetUtils.getIpRangeFromCidr(guestVmCidrPair[0], Long.valueOf(guestVmCidrPair[1]));
+            if (StringUtils.isNoneBlank(startIp, endIp) && NetUtils.ipRangesOverlap(startIp, endIp, cidrIpRange[0], cidrIpRange[1])) {
+                throw new InvalidParameterValueException(String.format("The IP range (%s-%s) overlaps with cidr of associated network: %s (%s)",
+                        startIp, endIp, associatedNetwork.getName(), associatedNetwork.getCidr()));
             }
         }
+        // Check IP range overlap on shared networks and vpc private gateways associated to the same network
+        checkIpRangeOverlapWithAssociatedNetworks(associatedNetworkId, startIp, endIp);
+
         associatedNetwork = implementedNetworkInCreation(caller, zone, associatedNetwork);
         if (associatedNetwork == null || (associatedNetwork.getState() != Network.State.Implemented && associatedNetwork.getState() != Network.State.Setup)) {
             throw new InvalidParameterValueException("Unable to implement associated network " + associatedNetwork);
@@ -3065,8 +3062,9 @@ public Network updateGuestNetwork(final UpdateNetworkCmd cmd) {
             if (networkOfferingChanged) {
                 throw new InvalidParameterValueException("Cannot specify this network offering change and guestVmCidr at same time. Specify only one.");
             }
-            if (!(network.getState() == Network.State.Implemented)) {
-                throw new InvalidParameterValueException("The network must be in " + Network.State.Implemented + " state. IP Reservation cannot be applied in " + network.getState() + " state");
+            if (network.getState() != Network.State.Implemented && network.getState() != Network.State.Allocated) {
+                throw new InvalidParameterValueException(String.format("The network must be in %s or %s state. IP Reservation cannot be applied in %s state",
+                        Network.State.Implemented, Network.State.Allocated, network.getState()));
             }
             if (!NetUtils.isValidIp4Cidr(guestVmCidr)) {
                 throw new InvalidParameterValueException("Invalid format of Guest VM CIDR.");
@@ -3125,6 +3123,9 @@ public Network updateGuestNetwork(final UpdateNetworkCmd cmd) {
                 }
             }
 
+            // Check IP range overlap on shared networks and vpc private gateways associated to this network
+            checkIpRangeOverlapWithAssociatedNetworks(networkId, cidrIpRange[0], cidrIpRange[1]);
+
             // When reservation is applied for the first time, network_cidr will be null
             // Populate it with the actual network cidr
             if (network.getNetworkCidr() == null) {
@@ -5911,6 +5912,30 @@ private List<Long> convertAccountNamesToAccountIds(final Account caller, final L
         return accountIds;
     }
 
+    private void checkIpRangeOverlapWithAssociatedNetworks(Long associatedNetworkId, String startIp, String endIp) {
+        List<NetworkDetailVO> associatedNetworks = _networkDetailsDao.findDetails(Network.AssociatedNetworkId, String.valueOf(associatedNetworkId), null);
+        for (NetworkDetailVO networkDetailVO : associatedNetworks) {
+            NetworkVO associatedNetwork2 = _networksDao.findById(networkDetailVO.getResourceId());
+            if (associatedNetwork2 != null) {
+                List<VlanVO> vlans = _vlanDao.listVlansByNetworkId(associatedNetwork2.getId());
+                if (vlans.isEmpty()) {
+                    VpcGatewayVO vpcGateway = vpcGatewayDao.getVpcGatewayByNetworkId(associatedNetwork2.getId());
+                    if (vpcGateway != null && NetUtils.ipRangesOverlap(startIp, endIp, vpcGateway.getIp4Address(), vpcGateway.getIp4Address())) {
+                        throw new InvalidParameterValueException(String.format("The startIp/endIp (%s - %s) overlaps with vpc private gateway %s (%s): ",
+                                startIp, endIp, associatedNetwork2.getName(), vpcGateway.getIp4Address()));
+                    }
+                    continue;
+                }
+                String startIP2 = vlans.get(0).getIpRange().split("-")[0];
+                String endIP2 = vlans.get(0).getIpRange().split("-")[1];
+                if (StringUtils.isNoneBlank(startIp, startIP2) && NetUtils.ipRangesOverlap(startIp, endIp, startIP2, endIP2)) {
+                    throw new InvalidParameterValueException(String.format("The startIp/endIp (%s - %s) overlaps with network %s (%s - %s)",
+                            startIp, endIp, associatedNetwork2.getName(), startIP2, endIP2));
+                }
+            }
+        }
+    }
+
     @Override
     public String getConfigComponentName() {
         return NetworkService.class.getSimpleName();

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -309,6 +309,7 @@
 import org.apache.cloudstack.api.command.admin.vpc.CreateVPCOfferingCmd;
 import org.apache.cloudstack.api.command.admin.vpc.DeletePrivateGatewayCmd;
 import org.apache.cloudstack.api.command.admin.vpc.DeleteVPCOfferingCmd;
+import org.apache.cloudstack.api.command.admin.vpc.ListPrivateGatewaysCmdByAdminCmd;
 import org.apache.cloudstack.api.command.admin.vpc.ListVPCsCmdByAdmin;
 import org.apache.cloudstack.api.command.admin.vpc.UpdateVPCCmdByAdmin;
 import org.apache.cloudstack.api.command.admin.vpc.UpdateVPCOfferingCmd;
@@ -3834,6 +3835,7 @@ public List<Class<?>> getCommands() {
         cmdList.add(ListVPCsCmdByAdmin.class);
         cmdList.add(UpdateVPCCmdByAdmin.class);
         cmdList.add(CreatePrivateGatewayByAdminCmd.class);
+        cmdList.add(ListPrivateGatewaysCmdByAdminCmd.class);
         cmdList.add(UpdateLBStickinessPolicyCmd.class);
         cmdList.add(UpdateLBHealthCheckPolicyCmd.class);
         cmdList.add(GetUploadParamsForTemplateCmd.class);

test/integration/component/test_ip_reservation.py

@@ -1069,7 +1069,7 @@ def test_network_not_implemented(self):
         # 1. update guestvmcidr of isolated network (non persistent)
         #
         # validation
-        # should throw exception as network is not in implemented state as no vm is created
+        # should succeed although network is not in implemented state
 
         networkOffering = self.isolated_network_offering
         resultSet = createIsolatedNetwork(self, networkOffering.id)
@@ -1078,9 +1078,7 @@ def test_network_not_implemented(self):
         else:
             isolated_network = resultSet[1]
 
-        with self.assertRaises(Exception):
-            isolated_network.update(self.apiclient, guestvmcidr="10.1.1.0/26")
-        return
+        isolated_network.update(self.apiclient, guestvmcidr="10.1.1.0/26")
 
     @attr(tags=["advanced"], required_hardware="false")
     def test_vm_create_after_reservation(self):