This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new e333f2705aa user-shared networks: fix few issues (#6887)
e333f2705aa is described below
commit e333f2705aa39007683fca83edac704410e72ca1
Author: Wei Zhou <[email protected]>
AuthorDate: Mon Oct 9 09:41:44 2023 +0200
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
---
.../apache/cloudstack/api/ResponseGenerator.java | 2 +-
.../vpc/ListPrivateGatewaysCmdByAdminCmd.java | 35 +++++++++++++
.../command/user/vpc/CreatePrivateGatewayCmd.java | 2 +-
.../command/user/vpc/ListPrivateGatewaysCmd.java | 7 ++-
.../main/java/com/cloud/api/ApiResponseHelper.java | 6 ++-
.../java/com/cloud/network/NetworkServiceImpl.java | 61 +++++++++++++++-------
.../com/cloud/server/ManagementServerImpl.java | 2 +
test/integration/component/test_ip_reservation.py | 6 +--
8 files changed, 93 insertions(+), 28 deletions(-)
diff --git a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java
b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java
index 88b3571385a..1a0df486298 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java
@@ -438,7 +438,7 @@ public interface ResponseGenerator {
* @param result
* @return
*/
- PrivateGatewayResponse createPrivateGatewayResponse(PrivateGateway result);
+ PrivateGatewayResponse createPrivateGatewayResponse(ResponseView view,
PrivateGateway result);
/**
* @param result
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/ListPrivateGatewaysCmdByAdminCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/ListPrivateGatewaysCmdByAdminCmd.java
new file mode 100644
index 00000000000..13a63e9cdd8
--- /dev/null
+++
b/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());
+
+}
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreatePrivateGatewayCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreatePrivateGatewayCmd.java
index 8527bd0e7fb..cf1315c9d55 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreatePrivateGatewayCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/CreatePrivateGatewayCmd.java
@@ -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 {
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListPrivateGatewaysCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListPrivateGatewaysCmd.java
index 47861c8362e..8813cccc779 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListPrivateGatewaysCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListPrivateGatewaysCmd.java
@@ -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());
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
index 1eb9dd84259..7ed31e21bc3 100644
--- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -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) {
diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
index 7b160416547..4d439ceda43 100644
--- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java
@@ -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();
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 77a652d7690..3620e52d547 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -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);
diff --git a/test/integration/component/test_ip_reservation.py
b/test/integration/component/test_ip_reservation.py
index 0ea01be4799..838aa30f7d0 100644
--- a/test/integration/component/test_ip_reservation.py
+++ b/test/integration/component/test_ip_reservation.py
@@ -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):
