wido commented on code in PR #13821:
URL: https://github.com/apache/cloudstack/pull/13821#discussion_r3802133834
##########
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:
> Yeah I am fine with this restriction, since the MS is connecting to this
URL directly. Just want to flag the tradeoff: it means a domain admin can no
longer set up a private DNS server for their own domain and make it public, and
regular users can't register a private one for themselves either, only root
admin gets to use a private/internal address at all.
>
> To not lose that use case entirely, we would probably want to let root
admin set up a DNS server on behalf of a specific domain or account, so a
domain admin/user can still end up with a private DNS server, provisioned by
root admin rather than self-serve. Could be considered for future enhancement.
>
> cc: @wido
Agree. I think setting up should be done by the root-admin (for now) and
then assigned to a specific domain/account or made public.
--
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]
