Allow templates & ISOs from IPv6 host address.

[GabrielBrascher]

Description

This PR removes a conditional in com.cloud.utils.UriUtils.validateUrl(java.lang.String, java.lang.String).

I don't see reasons to block IPv6 addresses when registering templates & ISOs.
Tested and the whole CloudStack template registration works fine when using IPv6 addresses (both on management node & template repository).

Please let me know if there are any know issues that justify blocking IPv6 addresses.

I've tested on a 4.16.0.0 environment, however, the PR's base branch is main.
I can rebase it for 4.16.1.0 if that sounds good.

Fixes: #3047

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

• Register a new template in the UI (from IPv6 host)
• Register a new kubernetes ISO (from IPv6 host)
• Check DB entries validating that all is correct
• Mount secondary storage and ensure that Template and ISO have been properly placed in their secondary storage folders

[GabrielBrascher]

@blueorangutan package

[blueorangutan]

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2101

[GabrielBrascher]

@blueorangutan package

[blueorangutan]

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[DaanHoogland]

looks good, are we sure we covered all bases with repect to ipv6?

[GabrielBrascher]

That code is 10 years old, in "another era" where there was not much IPv6 support.
Nowadays, most repositories serve both IPv4 and IPv6.

@DaanHoogland It is important to note that it could fail to register templates/ISOs from repositories with IPv6 addresses only, in the scenarios where SSVMs do not have an IPv6 address (might be the majority of cases, I don't know who configures or not SystemVMs with IPv6).

With that said, I still prefer to have an exception such as "could not reach the host", instead of getting CloudStack blocking right away in case of IPv6 assuming that it is not supported.

To put some context. The reported issue happened in a test-env where Java was configured to prefer IPv6 over IPv4 -Djava.net.preferIPv6Addresses=true. By default, we don't have Java choosing IPv6 over IPv4. We should add preferIPv6Addresses in the future. The repositories that I've tested always had IPv6 & IPv4, thus it worked fine for any network setup.

[blueorangutan]

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2110

[GabrielBrascher]

@blueorangutan package

[blueorangutan]

@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖️ el7 ✔️ el8 ✖️ debian ✖️ suse15. SL-JID 2113

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2116

[sureshanaparti]

@blueorangutan test

[blueorangutan]

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[sureshanaparti]

Hi @GabrielBrascher Can you rebase/test templates & ISOs registration (and VM deploy) with 4.16 if this is good in 4.16.1. Thanks.

[GabrielBrascher]

@sureshanaparti sounds good. Rebased branch to 4.16.

[sureshanaparti]

@sureshanaparti sounds good. Rebased branch to 4.16.

thanks @GabrielBrascher

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2123

[sureshanaparti]

@blueorangutan test

[blueorangutan]

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[blueorangutan]

Trillian test result (tid-2808)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39427 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5826-t2808-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[sureshanaparti]

LGTM

[shwstppr]

@GabrielBrascher will it make sense to skip IPv6 check only for direct download templates. For normal templates with IPv6 hosts it may still not work on SSVM?

[weizhouapache]

@GabrielBrascher will it make sense to skip IPv6 check only for direct download templates. For normal templates with IPv6 hosts it may still not work on SSVM?

I have same questions.

[GutoVeronezi]

LGTM

[yadvr]

Maybe run a quick test @weizhouapache@shwstppr with this PR and http://download.cloudstack.org/ which has AAAA/ipv6 record (2a00:f10:121:400:403:9cff:fe00:37f)?

[GabrielBrascher]

@GabrielBrascher will it make sense to skip IPv6 check only for direct download templates. For normal templates with IPv6 hosts it may still not work on SSVM?

@shwstppr good point.
Maybe we need to first make sure SSVMs have IPv6 addresses. And in the meantime, make it just for direct download.

[sureshanaparti]

@GabrielBrascher will it make sense to skip IPv6 check only for direct download templates. For normal templates with IPv6 hosts it may still not work on SSVM?

@shwstppr good point. Maybe we need to first make sure SSVMs have IPv6 addresses. And in the meantime, make it just for direct download.

Hi @GabrielBrascher Can you update the changes as appropriate (skip IPv6 check for direct download templates now, and re-visit the normal templates with SSVM's IPv6 address check later ?)

[sureshanaparti]

@GabrielBrascher will it make sense to skip IPv6 check only for direct download templates. For normal templates with IPv6 hosts it may still not work on SSVM?

@shwstppr good point. Maybe we need to first make sure SSVMs have IPv6 addresses. And in the meantime, make it just for direct download.

Hi @GabrielBrascher Can you update the changes as appropriate (skip IPv6 check for direct download templates now, and re-visit the normal templates with SSVM's IPv6 address check later ?)

Hi @GabrielBrascher Can you confirm if these changes are addressed in this PR. Thanks.

[shwstppr]

@GabrielBrascher doing something like following works,

diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
index 00dfee244c..1cf058d565 100644
--- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
+++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
@@ -216,7 +216,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase {
public TemplateProfile prepare(RegisterTemplateCmd cmd) throws ResourceAllocationException {
TemplateProfile profile = super.prepare(cmd);
String url = profile.getUrl();
- UriUtils.validateUrl(cmd.getFormat(), url);
+ UriUtils.validateUrl(cmd.getFormat(), url, cmd.isDirectDownload());
if (cmd.isDirectDownload()) {
DigestHelper.validateChecksumString(cmd.getChecksum());
Long templateSize = performDirectDownloadUrlValidation(cmd.getFormat(), url, cmd.getZoneIds());
diff --git a/utils/src/main/java/com/cloud/utils/UriUtils.java b/utils/src/main/java/com/cloud/utils/UriUtils.java
index 6b222289b6..eb84d4555f 100644
--- a/utils/src/main/java/com/cloud/utils/UriUtils.java
+++ b/utils/src/main/java/com/cloud/utils/UriUtils.java
@@ -262,7 +262,7 @@ public class UriUtils {
return validateUrl(null, url);
}
- public static Pair<String, Integer> validateUrl(String format, String url) throws IllegalArgumentException {
+ public static Pair<String, Integer> validateUrl(String format, String url, boolean skipIpv6Check) throws IllegalArgumentException {
try {
URI uri = new URI(url);
if ((uri.getScheme() == null) ||
@@ -283,7 +283,7 @@ public class UriUtils {
if (hostAddr.isAnyLocalAddress() || hostAddr.isLinkLocalAddress() || hostAddr.isLoopbackAddress() || hostAddr.isMulticastAddress()) {
throw new IllegalArgumentException("Illegal host specified in url");
}
- if (hostAddr instanceof Inet6Address) {
+ if (!skipIpv6Check && hostAddr instanceof Inet6Address) {
throw new IllegalArgumentException("IPV6 addresses not supported (" + hostAddr.getHostAddress() + ")");
}
} catch (UnknownHostException uhe) {
@@ -301,6 +301,10 @@ public class UriUtils {
}
}
+ public static Pair<String, Integer> validateUrl(String format, String url) throws IllegalArgumentException {
+ return validateUrl(format, url, false);
+ }
+
/**
* Add element to priority list examining node attributes: priority (for urls) and type (for checksums)
*/

Normal template:

Direct download:

[GabrielBrascher]

@sureshanaparti@shwstppr sorry for the delay here, got a bit stuck with other tasks.
Not sure if I will be able to do this one in time for 4.16.1.

@shwstppr thanks for the test, if you want to cherry-pick/create new PR addressing it would be nice as well.

[sureshanaparti]

@sureshanaparti@shwstppr sorry for the delay here, got a bit stuck with other tasks. Not sure if I will be able to do this one in time for 4.16.1.

@shwstppr thanks for the test, if you want to cherry-pick/create new PR addressing it would be nice as well.

Thanks for the update @GabrielBrascher I've created another PR #5900 extending your work and @shwstppr suggestions. Closing this one.