harikrishna-patnala commented on code in PR #6442:
URL: https://github.com/apache/cloudstack/pull/6442#discussion_r1166400625
##########
api/src/main/java/com/cloud/network/vpc/VpcService.java:
##########
@@ -34,10 +35,10 @@
import com.cloud.network.IpAddress;
import com.cloud.user.User;
import com.cloud.utils.Pair;
+import org.apache.cloudstack.api.command.user.vpc.UpdateVPCCmd;
Review Comment:
Can we move this above in the order of
org.apache.cloudstack.api.command.user.vpc ?
##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java:
##########
@@ -266,6 +280,14 @@ public String getTungstenVirtualRouterUuid() {
return tungstenVirtualRouterUuid;
}
+ public String getSourceNatIP() {
+ return sourceNatIP;
+ }
+
+ public long getSourceNatIpId() {
Review Comment:
I think we need **L**ong here ! since this is not a required param and can
be null.
##########
server/src/test/java/com/cloud/network/NetworkServiceImplTest.java:
##########
@@ -770,4 +773,144 @@ public void
validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouterTestMu
networkServiceImplMock.validateIfServiceOfferingIsActiveAndSystemVmTypeIsDomainRouter(1l);
}
+
+ @Test
+ public void validateNotSharedNetworkRouterIPv4() {
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.L2);
+ service.validateSharedNetworkRouterIPs(null, null, null, null, null,
null, null, null, null, ntwkOff);
+ }
+
+ @Test
+ public void validateSharedNetworkRouterIPs() {
+ String startIP = "10.0.16.2";
+ String endIP = "10.0.16.100";
+ String routerIPv4 = "10.0.16.100";
+ String routerPv6 = "fd17:ac56:1234:2000::fb";
+ String startIPv6 = "fd17:ac56:1234:2000::1";
+ String endIPv6 = "fd17:ac56:1234:2000::fc";
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, startIP, endIP,
IP4_NETMASK, routerIPv4, routerPv6, startIPv6, endIPv6, IP6_CIDR, ntwkOff);
+ }
+
+ @Test
+ public void validateSharedNetworkWrongRouterIPv4() {
+ String startIP = "10.0.16.2";
+ String endIP = "10.0.16.100";
+ String routerIPv4 = "10.0.16.101";
+ String routerPv6 = "fd17:ac56:1234:2000::fb";
+ String startIPv6 = "fd17:ac56:1234:2000::1";
+ String endIPv6 = "fd17:ac56:1234:2000::fc";
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ boolean passing = false;
+ try {
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, startIP,
endIP, IP4_NETMASK, routerIPv4, routerPv6, startIPv6, endIPv6, IP6_CIDR,
ntwkOff);
+ } catch (CloudRuntimeException e) {
+ Assert.assertTrue(e.getMessage().contains("Router IPv4 IP provided
is not within the specified range: "));
+ passing = true;
+ }
+ Assert.assertTrue(passing);
+ }
+
+ @Test
+ public void validateSharedNetworkNoEndOfIPv6Range() {
+ String startIP = null;
+ String endIP = null;
+ String routerIPv4 = null;
+ String routerPv6 = "fd17:ac56:1234:2000::1";
+ String startIPv6 = "fd17:ac56:1234:2000::1";
+ String endIPv6 = null;
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, startIP, endIP,
IP4_NETMASK, routerIPv4, routerPv6, startIPv6, endIPv6, IP6_CIDR, ntwkOff);
+ }
+
+ @Test
+ public void validateSharedNetworkIPv6RouterNotInRange() {
+ String routerIPv4 = null;
+ String routerIPv6 = "fd17:ac56:1234:2001::1";
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ boolean passing = false;
+ try {
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, null, null,
IP4_NETMASK, routerIPv4, routerIPv6, null, null, IP6_CIDR, ntwkOff);
+ } catch (CloudRuntimeException e) {
+ Assert.assertTrue(e.getMessage().contains("Router IPv6 address
provided is not with the network range"));
+ passing = true;
+ }
+ Assert.assertTrue(passing);
+ }
+
+ @Test
+ public void invalidateSharedNetworkIPv6RouterAddress() {
+ String routerIPv6 = "fd17:ac56:1234:2000::fg";
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ boolean passing = false;
+ try {
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, null, null,
IP4_NETMASK, null, routerIPv6, null, null, IP6_CIDR, ntwkOff);
+ } catch (CloudRuntimeException e) {
+ Assert.assertTrue(e.getMessage().contains("Router IPv6 address
provided is of incorrect format"));
+ passing = true;
+ }
+ Assert.assertTrue(passing);
+ }
+
+ @Test
+ public void invalidateSharedNetworkIPv4RouterAddress() {
+ String routerIPv4 = "10.100.1000.1";
+ NetworkOffering ntwkOff = Mockito.mock(NetworkOffering.class);
+ when(ntwkOff.getGuestType()).thenReturn(Network.GuestType.Shared);
+ boolean passing = false;
+ try {
+ service.validateSharedNetworkRouterIPs(IP4_GATEWAY, null, null,
IP4_NETMASK, routerIPv4, null, null, null, IP6_CIDR, ntwkOff);
+ } catch (CloudRuntimeException e) {
+ Assert.assertTrue(e.getMessage().contains("Router IPv4 IP provided
is of incorrect format"));
+ passing = true;
+ }
+ Assert.assertTrue(passing);
+ }
+
+ @Test
+ public void checkAndDontSetSourceNatIp() {
+ CreateNetworkCmd cmd = new CreateNetworkCmd();
+ String srcNatIp = "10.100.1000.1";
Review Comment:
There is no usage for this ! can we delete this
##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java:
##########
@@ -104,6 +104,12 @@ public class UpdateNetworkCmd extends BaseAsyncCustomIdCmd
implements UserCmd {
@Parameter(name = ApiConstants.IP6_DNS2, type = CommandType.STRING,
description = "the second IPv6 DNS for the network. Empty string will update
the second IPv6 DNS with the value from the zone", since = "4.18.0")
private String ip6Dns2;
+ @Parameter(name = ApiConstants.SOURCE_NAT_IP, type = CommandType.STRING,
description = "IPV4 address to be assigned to the public interface of the
network router. This address must already be acquired for this network", since
= "4.19")
+ private String sourceNatIP;
+
+ @Parameter(name = ApiConstants.SOURCE_NAT_IP_ID, type = CommandType.UUID,
description = "IPV4 address to be assigned to the piublic interface of the
network router.", since = "4.19")
+ private Long sourceNatIpUuid;
Review Comment:
same here, can you check if this is an used param and delete it. I think
same is there in other commands as well
##########
server/src/main/java/com/cloud/api/ApiResponseHelper.java:
##########
@@ -37,6 +37,7 @@
import javax.inject.Inject;
+import com.cloud.network.dao.FirewallRulesDao;
Review Comment:
can we move this down in the order of "com.cloud.network"
##########
api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java:
##########
@@ -183,6 +183,20 @@ public class CreateNetworkCmd extends BaseCmd implements
UserCmd {
@Parameter(name = ApiConstants.IP6_DNS2, type = CommandType.STRING,
description = "the second IPv6 DNS for the network", since = "4.18.0")
private String ip6Dns2;
+ @Parameter(name = ApiConstants.SOURCE_NAT_IP,
+ type = CommandType.STRING,
+ description = "IPV4 address to be assigned to the public interface
of the network router. This is a hint." +
+ " If an address is given and it cannot be acquired, an
error will be returned and the network won´t be implemented," +
+ " even if so requested.",
+ since = "4.19")
+ private String sourceNatIP;
+
+ @Parameter(name = ApiConstants.SOURCE_NAT_IP_ID,
+ type = CommandType.UUID,
+ description = "IPV4 address to be assigned to the public interface
of the network router.",
+ since = "4.19")
+ private Long sourceNatIpUuid;
Review Comment:
is this parameter in use ? I don't see any usage of this.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
