subject:"\[GitHub\] \[cloudstack\] harikrishna\-patnala commented on a diff in pull request #6442\: Possibility to choose the source NAT IP address on a isolated network or VPC"

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

2023-05-03 Thread via GitHub



harikrishna-patnala commented on code in PR #6442:
URL: https://github.com/apache/cloudstack/pull/6442#discussion_r1183283308


##
.github/workflows/ci.yml:
##
@@ -79,7 +79,8 @@ jobs:
   smoke/test_metrics_api
   smoke/test_migration
   smoke/test_multipleips_per_nic
-  smoke/test_nested_virtualization",
+  smoke/test_nested_virtualization
+  smoke/test_set_source_nat",

Review Comment:
   test_set_source_nat.py smoke test file is missing !



##
api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java:
##
@@ -159,10 +159,9 @@ public class IPAddressResponse extends 
BaseResponseWithAnnotations implements Co
 @Param(description="the name of the Network where ip belongs to")
 private String networkName;
 
-/*
-@SerializedName(ApiConstants.JOB_ID) @Param(description="shows the 
current pending asynchronous job ID. This tag is not returned if no current 
pending jobs are acting on the volume")
-private IdentityProxy jobId = new IdentityProxy("async_job");
-*/
+@SerializedName(ApiConstants.HAS_RULES)
+@Param(description="whether the ip address has 
Firewall/PortForwarding/LoadBalancing rules defined")

Review Comment:
   please help me understand where this is used or how is it helpful. Also I 
see we are currently setting this flag only based on firewall rules (not 
portforwarding/loadbalancing rules), do we need to update the comment !
   
   ipResponse.setHasRules(firewallRulesDao.countRulesByIpId(ipAddr.getId()) > 
0);



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

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

2023-04-14 Thread via GitHub



harikrishna-patnala commented on code in PR #6442:
URL: https://github.com/apache/cloudstack/pull/6442#discussion_r1166499359


##
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) {
+

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

2023-04-14 Thread via GitHub



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,

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #6442: Possibility to choose the source NAT IP address on a isolated network or VPC

3 matches

Site Navigation

Mail list logo

Footer information