Skip to content

Prevent deploying IPv6 network if Zone has no IPv6 DNS configured #4177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 6 commits into from
Oct 30, 2020
Merged

Prevent deploying IPv6 network if Zone has no IPv6 DNS configured #4177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce658ac
Deploy ipv6 network if zone has IPv6 DNS
GabrielBrascher Jun 24, 2020
e060158
Add test case for networkModel.checkIp6Parameters
GabrielBrascher Jun 26, 2020
1898b2f
Merge branch 'master' into validate-ipv6-network
GabrielBrascher Sep 24, 2020
b66843e
Use CloudStack StringUtils instead of org.apache.commons.lang3.String…
GabrielBrascher Sep 27, 2020
c90e1e2
Consider an IPv6 network if ip6Cidr not null at createVlanAndPublicIp…
GabrielBrascher Oct 28, 2020
c52730d
Enhance log message on checkIp6Parameters
GabrielBrascher Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3412,10 +3412,10 @@ public Vlan createVlanAndPublicIpRange(final CreateVlanIpRangeCmd cmd) throws In
}

final boolean ipv4 = startIP != null;
final boolean ipv6 = startIPv6 != null;
final boolean ipv6 = ip6Cidr != null;

if (!ipv4 && !ipv6) {
throw new InvalidParameterValueException("StartIP or StartIPv6 is missing in the parameters!");
throw new InvalidParameterValueException("StartIP or IPv6 CIDR is missing in the parameters!");
}

if (ipv4) {
Expand Down Expand Up @@ -3781,7 +3781,7 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
ipv4 = true;
}

if (startIPv6 != null) {
if (vlanIp6Cidr != null) {
Copy link
Member Author

@GabrielBrascher GabrielBrascher Jun 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CloudStack uses SLAAC for managing IPv6 ranges, therefore it is not necessary to have a start/end IPv6 address; on the other hand, IPv6 CIDR is mandatory for IPv6 networks.

That is why I changed this and other pieces of code that had start/end ipv6 address as mandatory.

ipv6 = true;
}

Expand Down
37 changes: 22 additions & 15 deletions server/src/main/java/com/cloud/network/NetworkModelImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.utils.StringUtils;
import org.apache.cloudstack.context.CallContext;
import org.apache.commons.collections.CollectionUtils;
import org.apache.log4j.Logger;
Expand Down Expand Up @@ -115,7 +116,6 @@
import com.cloud.user.DomainManager;
import com.cloud.user.User;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.StringUtils;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.DB;
Expand Down Expand Up @@ -2207,15 +2207,9 @@ public boolean isNetworkInlineMode(Network network) {

@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
if (!NetUtils.isValidIp6(startIPv6)) {
throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
}
if (!NetUtils.isValidIp6(endIPv6)) {
throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
}

if (!(ip6Gateway != null && ip6Cidr != null)) {
throw new InvalidParameterValueException("ip6Gateway and ip6Cidr should be defined when startIPv6/endIPv6 are passed in");
if (StringUtils.isBlank(ip6Gateway) || StringUtils.isBlank(ip6Cidr)) {
throw new InvalidParameterValueException("ip6Gateway and ip6Cidr should be defined for an IPv6 network work properly");
}

if (!NetUtils.isValidIp6(ip6Gateway)) {
Expand All @@ -2224,16 +2218,29 @@ public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gatew
if (!NetUtils.isValidIp6Cidr(ip6Cidr)) {
throw new InvalidParameterValueException("Invalid ip6cidr");
}
if (!NetUtils.isIp6InNetwork(startIPv6, ip6Cidr)) {
throw new InvalidParameterValueException("startIPv6 is not in ip6cidr indicated network!");
}
if (!NetUtils.isIp6InNetwork(endIPv6, ip6Cidr)) {
throw new InvalidParameterValueException("endIPv6 is not in ip6cidr indicated network!");
}

if (!NetUtils.isIp6InNetwork(ip6Gateway, ip6Cidr)) {
throw new InvalidParameterValueException("ip6Gateway is not in ip6cidr indicated network!");
}

if (StringUtils.isNotBlank(startIPv6)) {
if (!NetUtils.isValidIp6(startIPv6)) {
throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
}
if (!NetUtils.isIp6InNetwork(startIPv6, ip6Cidr)) {
throw new InvalidParameterValueException("startIPv6 is not in ip6cidr indicated network!");
}
}

if (StringUtils.isNotBlank(endIPv6)) {
if (!NetUtils.isValidIp6(endIPv6)) {
throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
}
if (!NetUtils.isIp6InNetwork(endIPv6, ip6Cidr)) {
throw new InvalidParameterValueException("endIPv6 is not in ip6cidr indicated network!");
}
}

int cidrSize = NetUtils.getIp6CidrSize(ip6Cidr);
// we only support cidr == 64
if (cidrSize != 64) {
Expand Down
14 changes: 9 additions & 5 deletions server/src/main/java/com/cloud/network/NetworkServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,10 +726,10 @@ public NicSecondaryIp allocateSecondaryGuestIP(final long nicId, IpAddresses req
String ip6addr = null;
//Isolated network can exist in Basic zone only, so no need to verify the zone type
if (network.getGuestType() == Network.GuestType.Isolated) {
if ((ipv4Address != null || NetUtils.isIpv4(network.getGateway()) && org.apache.commons.lang3.StringUtils.isBlank(ipv6Address))) {
if ((ipv4Address != null || NetUtils.isIpv4(network.getGateway()) && isBlank(ipv6Address))) {
ipaddr = _ipAddrMgr.allocateGuestIP(network, ipv4Address);
}
if (ipv6Address != null) {
if (isNotBlank(ipv6Address)) {
ip6addr = ipv6AddrMgr.allocateGuestIpv6(network, ipv6Address);
}
} else if (network.getGuestType() == Network.GuestType.Shared) {
Expand Down Expand Up @@ -763,7 +763,7 @@ public NicSecondaryIp allocateSecondaryGuestIP(final long nicId, IpAddresses req
return null;
}

if (ipaddr != null || ip6addr != null) {
if (isNotBlank(ipaddr) || isNotBlank(ip6addr)) {
// we got the ip addr so up the nics table and secodary ip
final String ip4AddrFinal = ipaddr;
final String ip6AddrFinal = ip6addr;
Expand Down Expand Up @@ -1195,7 +1195,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac
if (startIP != null) {
ipv4 = true;
}
if (startIPv6 != null) {
if (isNotBlank(ip6Cidr) && isNotBlank(ip6Gateway)) {
Copy link
Member Author

@GabrielBrascher GabrielBrascher Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to SLAAC implementation, an IPv6 network needs CIDR and Gateway. With the refactored code a network is "marked" as ipv6 if IPv6 CIDR and Gateway are not null.

ipv6 = true;
}

Expand Down Expand Up @@ -1273,6 +1273,10 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac
if (zone.getNetworkType() != NetworkType.Advanced || ntwkOff.getGuestType() != Network.GuestType.Shared) {
throw new InvalidParameterValueException("Can only support create IPv6 network with advance shared network!");
}

if(isBlank(zone.getIp6Dns1()) && isBlank(zone.getIp6Dns2())) {
throw new InvalidParameterValueException("Can only create IPv6 network if the zone has IPv6 DNS! Please configure the zone IPv6 DNS1 and/or IPv6 DNS2.");
}
}

if (isNotBlank(isolatedPvlan) && (zone.getNetworkType() != NetworkType.Advanced || ntwkOff.getGuestType() == GuestType.Isolated)) {
Expand Down Expand Up @@ -2759,7 +2763,7 @@ private void verifyAlreadyMigratedTiers(long migratedVpcId, long vpcOfferingId,
for (Network tier : migratedTiers) {
String tierNetworkOfferingUuid = networkToOffering.get(tier.getUuid());

if (!StringUtils.isNotBlank(tierNetworkOfferingUuid)) {
if (!isNotBlank(tierNetworkOfferingUuid)) {
throwInvalidIdException("Failed to resume migrating VPC as the specified tierNetworkOfferings is not complete", String.valueOf(tier.getUuid()), "networkUuid");
}

Expand Down
69 changes: 69 additions & 0 deletions server/src/test/java/com/cloud/network/NetworkModelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkServiceProviderDao;
import com.cloud.network.dao.PhysicalNetworkServiceProviderVO;
Expand Down Expand Up @@ -90,6 +91,11 @@ public class NetworkModelTest {
private static final long PHYSICAL_NETWORK_1_ID = 1L;
private static final long PHYSICAL_NETWORK_2_ID = 2L;

private static final String IPV6_CIDR = "fd59:16ba:559b:243d::/64";
Copy link
Member Author

@GabrielBrascher GabrielBrascher Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nowadays IPv6 networking support only /64 CIDR. Added Unit tests covering this.

private static final String IPV6_GATEWAY = "fd59:16ba:559b:243d::1";
private static final String START_IPV6 = "fd59:16ba:559b:243d:0:0:0:2";
private static final String END_IPV6 = "fd59:16ba:559b:243d:ffff:ffff:ffff:ffff";

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
Expand Down Expand Up @@ -194,4 +200,67 @@ public void testAddDisabledConfigDriveEntriesOnZoneAdvancedZoneNonExistingConfig
addProviderToPhysicalNetwork(anyLong(), eq(Provider.ConfigDrive.getName()), isNull(Long.class), isNull(List.class));
}

@Test
public void checkIp6ParametersTestAllGood() {
networkModel.checkIp6Parameters(START_IPV6, END_IPV6, IPV6_GATEWAY,IPV6_CIDR);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestCidr32() {
String ipv6cidr = "fd59:16ba:559b:243d::/32";
String endipv6 = "fd59:16ba:ffff:ffff:ffff:ffff:ffff:ffff";
networkModel.checkIp6Parameters(START_IPV6, endipv6, IPV6_GATEWAY,ipv6cidr);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestCidr63() {
String ipv6cidr = "fd59:16ba:559b:243d::/63";
String endipv6 = "fd59:16ba:559b:243d:ffff:ffff:ffff:ffff";
networkModel.checkIp6Parameters(START_IPV6, endipv6, IPV6_GATEWAY,ipv6cidr);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestCidr65() {
String ipv6cidr = "fd59:16ba:559b:243d::/65";
String endipv6 = "fd59:16ba:559b:243d:7fff:ffff:ffff:ffff";
networkModel.checkIp6Parameters(START_IPV6, endipv6, IPV6_GATEWAY,ipv6cidr);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestCidr120() {
String ipv6cidr = "fd59:16ba:559b:243d::/120";
String endipv6 = "fd59:16ba:559b:243d:0:0:0:ff";
networkModel.checkIp6Parameters(START_IPV6, endipv6, IPV6_GATEWAY,ipv6cidr);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestNullGateway() {
networkModel.checkIp6Parameters(START_IPV6, END_IPV6, null,IPV6_CIDR);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestNullCidr() {
networkModel.checkIp6Parameters(START_IPV6, END_IPV6, IPV6_GATEWAY,null);
}

@Test(expected = InvalidParameterValueException.class)
public void checkIp6ParametersTestNullCidrAndNulGateway() {
networkModel.checkIp6Parameters(START_IPV6, END_IPV6, null,null);
}

@Test
public void checkIp6ParametersTestNullStartIpv6() {
networkModel.checkIp6Parameters(null, END_IPV6, IPV6_GATEWAY,IPV6_CIDR);
}

@Test
public void checkIp6ParametersTestNullEndIpv6() {
networkModel.checkIp6Parameters(START_IPV6, null, IPV6_GATEWAY,IPV6_CIDR);
}

@Test
public void checkIp6ParametersTestNullStartAndEndIpv6() {
networkModel.checkIp6Parameters(null, null, IPV6_GATEWAY,IPV6_CIDR);
}

}