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
This commit is contained in:
Wei Zhou 2023-10-09 09:41:44 +02:00 committed by GitHub
parent 72cf9740f9
commit e333f2705a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 93 additions and 28 deletions

View File

@ -438,7 +438,7 @@ public interface ResponseGenerator {
* @param result
* @return
*/
PrivateGatewayResponse createPrivateGatewayResponse(PrivateGateway result);
PrivateGatewayResponse createPrivateGatewayResponse(ResponseView view, PrivateGateway result);
/**
* @param result

View File

@ -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());
}

View File

@ -169,7 +169,7 @@ public class CreatePrivateGatewayCmd extends BaseAsyncCreateCmd implements UserC
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 {

View File

@ -25,6 +25,8 @@ import org.apache.cloudstack.api.APICommand;
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.network.vpc.VpcGateway;
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 class ListPrivateGatewaysCmd extends BaseListProjectAndAccountResourcesCm
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());

View File

@ -3357,10 +3357,12 @@ public class ApiResponseHelper implements ResponseGenerator {
}
@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) {

View File

@ -178,11 +178,13 @@ import com.cloud.network.security.SecurityGroupService;
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 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
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 class NetworkServiceImpl extends ManagerBase implements NetworkService, C
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 class NetworkServiceImpl extends ManagerBase implements NetworkService, C
}
}
// 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 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C
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();

View File

@ -309,6 +309,7 @@ import org.apache.cloudstack.api.command.admin.vpc.CreateVPCCmdByAdmin;
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 class ManagementServerImpl extends ManagerBase implements ManagementServe
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);

View File

@ -1069,7 +1069,7 @@ class TestFailureScnarios(cloudstackTestCase):
# 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 @@ class TestFailureScnarios(cloudstackTestCase):
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):