sudo87 commented on code in PR #13821:
URL: https://github.com/apache/cloudstack/pull/13821#discussion_r3794634722


##########
server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java:
##########
@@ -162,14 +163,29 @@ private DnsProvider getProviderByType(DnsProviderType 
type) {
         throw new CloudRuntimeException("No plugin found for DNS provider 
type: " + type);
     }
 
+    /**
+     * Rejects a DNS provider URL that resolves to an illegal address before 
any provider client is given
+     * the chance to connect to it. See {@link UriUtils#validateUrl(String)} 
for the exact rules enforced
+     * (including the requirement that the URL declares an {@code http}/{@code 
https} scheme).
+     * Expects {@code url} to already be trimmed.
+     */
+    private void validateDnsServerUrl(String url) {
+        if (StringUtils.isBlank(url)) {
+            throw new IllegalArgumentException("URL cannot be blank.");
+        }
+        UriUtils.validateUrl(url);

Review Comment:
   Agreed with @DaanHoogland, private/site local are valid DNS server targets, 
plenty of setups will point to an internal DNS server specifically because it's 
not reachable from the internet, blocking those would make the feature of no 
use for those scenarios.
   
   Also worth noting that only root admins and domain admins can setup Public 
DNS server (usable by everyone in the domain/subdomain), while regular users 
can only setup their own private one.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to