Skip to content

Commit 4df11a4

Browse files
sudo87sudo87
committed
normalize dns zone and record in svc layer, always use dotless data in svc and handle dot version in client
1 parent c64cf81 commit 4df11a4

7 files changed

Lines changed: 256 additions & 35 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/user/dns/CreateDnsZoneCmd.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.cloudstack.api.command.user.dns;
1919

20+
import java.util.Arrays;
21+
2022
import javax.inject.Inject;
2123

2224
import org.apache.cloudstack.api.APICommand;
@@ -27,13 +29,14 @@
2729
import org.apache.cloudstack.api.ServerApiException;
2830
import org.apache.cloudstack.api.response.DnsServerResponse;
2931
import org.apache.cloudstack.api.response.DnsZoneResponse;
30-
import org.apache.cloudstack.api.response.NetworkResponse;
3132
import org.apache.cloudstack.context.CallContext;
3233
import org.apache.cloudstack.dns.DnsProviderManager;
3334
import org.apache.cloudstack.dns.DnsZone;
35+
import org.apache.commons.lang3.StringUtils;
3436

3537
import com.cloud.event.EventTypes;
3638
import com.cloud.exception.ResourceAllocationException;
39+
import com.cloud.utils.EnumUtils;
3740

3841
@APICommand(name = "createDnsZone", description = "Creates a new DNS Zone on a specific server",
3942
responseObject = DnsZoneResponse.class, requestHasSensitiveInfo = false,
@@ -55,10 +58,6 @@ public class CreateDnsZoneCmd extends BaseAsyncCreateCmd {
5558
required = true, description = "The ID of the DNS server to host this zone")
5659
private Long dnsServerId;
5760

58-
@Parameter(name = ApiConstants.NETWORK_ID, type = CommandType.UUID, entityType = NetworkResponse.class,
59-
description = "Optional: The Guest Network to associate with this zone for auto-registration")
60-
private Long networkId;
61-
6261
@Parameter(name = ApiConstants.TYPE, type = CommandType.STRING,
6362
description = "The type of zone (Public, Private). Defaults to Public.")
6463
private String type;
@@ -78,12 +77,15 @@ public Long getDnsServerId() {
7877
return dnsServerId;
7978
}
8079

81-
public Long getNetworkId() {
82-
return networkId;
83-
}
84-
85-
public String getType() {
86-
return type;
80+
public DnsZone.ZoneType getType() {
81+
if (StringUtils.isBlank(type)) {
82+
return DnsZone.ZoneType.Public;
83+
}
84+
DnsZone.ZoneType zoneType = EnumUtils.getEnumIgnoreCase(DnsZone.ZoneType.class, type);
85+
if (type == null) {
86+
throw new IllegalArgumentException("Invalid type value, supported values are: " + Arrays.toString(DnsZone.ZoneType.values()));
87+
}
88+
return zoneType;
8789
}
8890

8991
public String getDescription() {

‎engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ CREATE TABLE `cloud`.`dns_server` (
6262
`name` varchar(255) NOT NULL COMMENT 'display name of the dns server',
6363
`provider_type` varchar(255) NOT NULL COMMENT 'Provider type such as PowerDns',
6464
`url` varchar(1024) NOT NULL COMMENT 'dns server url',
65-
`dns_username` varchar(255) NOT NULL COMMENT 'username or email for dns server credentials',
65+
`dns_username` varchar(255) COMMENT 'username or email for dns server credentials',
6666
`api_key` varchar(255) NOT NULL COMMENT 'dns server api_key',
67-
`dns_server_name` varchar(255) COMMENT 'dns server name e.g. localhost for powerdns',
67+
`external_server_id` varchar(255) COMMENT 'dns server id e.g. localhost for powerdns',
6868
`port` int(11) DEFAULT NULL COMMENT 'optional dns server port',
6969
`name_servers` varchar(1024) DEFAULT NULL COMMENT 'Comma separated list of name servers',
7070
`is_public` tinyint(1) NOT NULL DEFAULT '0',

‎plugins/dns/powerdns/src/main/java/org/apache/cloudstack/dns/powerdns/PowerDnsClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public String discoverAuthoritativeServerId(String baseUrl, Integer port, String
139139
public String createZone(String baseUrl, Integer port, String apiKey, String externalServerId, String zoneName,
140140
String zoneKind, boolean dnsSecFlag, List<String> nameServers) throws DnsProviderException {
141141

142-
validateServerId(baseUrl, port, externalServerId, apiKey);
142+
validateServerId(baseUrl, port, apiKey, externalServerId);
143143
String normalizedZone = normalizeZone(zoneName);
144144
ObjectNode json = MAPPER.createObjectNode();
145145
json.put(ApiConstants.NAME, normalizedZone);
@@ -167,7 +167,7 @@ public String createZone(String baseUrl, Integer port, String apiKey, String ext
167167
public void updateZone(String baseUrl, Integer port, String apiKey, String externalServerId, String zoneName,
168168
String zoneKind, Boolean dnsSecFlag, List<String> nameServers) throws DnsProviderException {
169169

170-
validateServerId(baseUrl, port, externalServerId, apiKey);
170+
validateServerId(baseUrl, port, apiKey, externalServerId);
171171
String normalizedZone = normalizeZone(zoneName);
172172
String encodedZone = URLEncoder.encode(normalizedZone, StandardCharsets.UTF_8);
173173
String url = buildUrl(baseUrl, port,"/servers/" + externalServerId + "/zones/" + encodedZone);

‎plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.dns;
19+
20+
import static org.apache.cloudstack.dns.DnsUtil.appendPublicSuffixToZone;
21+
import static org.apache.cloudstack.dns.DnsUtil.normalizeDomain;
22+
import static org.junit.Assert.assertEquals;
23+
import static org.junit.Assert.fail;
24+
25+
import java.util.Arrays;
26+
import java.util.Collection;
27+
28+
import org.apache.logging.log4j.util.Strings;
29+
import org.junit.Test;
30+
import org.junit.runner.RunWith;
31+
import org.junit.runners.Parameterized;
32+
33+
@RunWith(Parameterized.class)
34+
public class DnsUtilTest {
35+
private final String userZoneName;
36+
private final String publicSuffix;
37+
private final String expectedResult;
38+
private final boolean expectException;
39+
40+
public DnsUtilTest(String userZoneName,
41+
String publicSuffix,
42+
String expectedResult,
43+
boolean expectException) {
44+
this.userZoneName = userZoneName;
45+
this.publicSuffix = publicSuffix;
46+
this.expectedResult = expectedResult;
47+
this.expectException = expectException;
48+
}
49+
50+
@Parameterized.Parameters
51+
public static Collection<Object[]> data() {
52+
return Arrays.asList(new Object[][]{
53+
{"tenant1.com", "example.com", "tenant1.example.com", false},
54+
{"dev.tenant2.com", "example.com", "dev.tenant2.example.com", false},
55+
{"tenant3.example.com", "example.com", "tenant3.example.com", false},
56+
{"Tenant1.CoM", "ExAmple.CoM", "tenant1.example.com", false},
57+
{"tenant1.com.", "example.com.", "tenant1.example.com", false},
58+
{"tenant1.com", "", "tenant1.com", false},
59+
{"tenant1.com", null, "tenant1.com", false},
60+
{"test.abc.com", "abc.com", "test.abc.com", false},
61+
{"sub.test.abc.com", "abc.com", "sub.test.abc.com", false},
62+
{"test.ai.abc.com", "abc.com", "test.ai.abc.com", false},
63+
{"deep.sub.abc.com", "abc.com", "deep.sub.abc.com", false},
64+
{"abc.com", "xyz.com", "abc.xyz.com", false},
65+
{"test.xyz.com", "xyz.com", "test.xyz.com", false},
66+
{"test.com.xyz.com", "xyz.com", "test.com.xyz.com", false},
67+
{"tenant", "example.com", null, true}, // single label
68+
{"test", "abc.com", null, true},
69+
{"example.com.", "example.com", null, true},
70+
{"example.com", "example.com", null, true}, // root level forbidden
71+
{"abc.com", "abc.com", null, true}, // root level forbidden
72+
{"tenant1.org", "example.com", null, true}, // TLD mismatch
73+
{"test.ai", "abc.com", null, true}, // TLD mismatch
74+
{null, "example.com", null, true},
75+
});
76+
}
77+
78+
@Test
79+
public void testAppendPublicSuffix() {
80+
if (expectException) {
81+
try {
82+
executeAppendSuffixTest(userZoneName, publicSuffix);
83+
fail("Expected IllegalArgumentException");
84+
} catch (IllegalArgumentException ignored) {
85+
// noop
86+
}
87+
} else {
88+
String result;
89+
if (Strings.isNotBlank(publicSuffix)) {
90+
result = executeAppendSuffixTest(userZoneName, publicSuffix);
91+
} else {
92+
result = appendPublicSuffixToZone(normalizeDomain(userZoneName), publicSuffix);
93+
}
94+
assertEquals(expectedResult, result);
95+
}
96+
}
97+
98+
String executeAppendSuffixTest(String zoneName, String domainSuffix) {
99+
return appendPublicSuffixToZone(
100+
normalizeDomain(zoneName),
101+
normalizeDomain(domainSuffix));
102+
}
103+
}

‎server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.ArrayList;
2121
import java.util.Collections;
2222
import java.util.List;
23+
import java.util.stream.Collectors;
2324

2425
import javax.inject.Inject;
2526

@@ -117,6 +118,11 @@ public DnsServer addDnsServer(AddDnsServerCmd cmd) {
117118
isDnsPublic = false;
118119
publicDomainSuffix = null;
119120
}
121+
122+
if (StringUtils.isNotBlank(publicDomainSuffix)) {
123+
publicDomainSuffix = DnsUtil.normalizeDomain(publicDomainSuffix);
124+
}
125+
120126
DnsProviderType type = DnsProviderType.fromString(cmd.getProvider());
121127
DnsServerVO server = new DnsServerVO(cmd.getName(), cmd.getUrl(), cmd.getPort(), cmd.getExternalServerId(), type,
122128
cmd.getDnsUserName(), cmd.getCredentials(), isDnsPublic, publicDomainSuffix, cmd.getNameServers(),
@@ -208,7 +214,7 @@ public DnsServer updateDnsServer(UpdateDnsServerCmd cmd) {
208214
}
209215

210216
if (cmd.getPublicDomainSuffix() != null) {
211-
dnsServer.setPublicDomainSuffix(cmd.getPublicDomainSuffix());
217+
dnsServer.setPublicDomainSuffix(DnsUtil.normalizeDomain(cmd.getPublicDomainSuffix()));
212218
}
213219

214220
if (cmd.getNameServers() != null) {
@@ -255,6 +261,7 @@ public DnsServerResponse createDnsServerResponse(DnsServer server) {
255261
response.setId(server.getUuid());
256262
response.setName(server.getName());
257263
response.setUrl(server.getUrl());
264+
response.setPort(server.getPort());
258265
response.setProvider(server.getProviderType());
259266
response.setPublic(server.isPublic());
260267
response.setNameServers(server.getNameServers());
@@ -272,7 +279,7 @@ public DnsServer getDnsServer(Long id) {
272279
public boolean deleteDnsZone(Long zoneId) {
273280
DnsZoneVO zone = dnsZoneDao.findById(zoneId);
274281
if (zone == null) {
275-
throw new InvalidParameterValueException("DNS zone with ID " + zoneId + " not found.");
282+
throw new InvalidParameterValueException("DNS zone not found for the given ID.");
276283
}
277284

278285
Account caller = CallContext.current().getCallingAccount();
@@ -347,23 +354,29 @@ public ListResponse<DnsZoneResponse> listDnsZones(ListDnsZonesCmd cmd) {
347354

348355
@Override
349356
public DnsRecordResponse createDnsRecord(CreateDnsRecordCmd cmd) {
357+
String recordName = StringUtils.trimToEmpty(cmd.getName()).toLowerCase();
358+
if (StringUtils.isBlank(recordName)) {
359+
throw new InvalidParameterValueException("Empty DNS record name is not allowed");
360+
}
350361
DnsZoneVO zone = dnsZoneDao.findById(cmd.getDnsZoneId());
351362
if (zone == null) {
352363
throw new InvalidParameterValueException("DNS zone not found.");
353364
}
354-
355365
Account caller = CallContext.current().getCallingAccount();
356366
accountMgr.checkAccess(caller, null, true, zone);
357367
DnsServerVO server = dnsServerDao.findById(zone.getDnsServerId());
358368
try {
369+
DnsRecord.RecordType type = cmd.getType();
370+
List<String> normalizedContents = cmd.getContents().stream()
371+
.map(value -> DnsUtil.normalizeDnsRecordValue(value, type)).collect(Collectors.toList());
372+
DnsRecord record = new DnsRecord(recordName, type, normalizedContents, cmd.getTtl());
359373
DnsProvider provider = getProvider(server.getProviderType());
360-
DnsRecord record = new DnsRecord(cmd.getName(), cmd.getType(), cmd.getContents(), cmd.getTtl());
361374
String normalizedRecordName = provider.addRecord(server, zone, record);
362375
record.setName(normalizedRecordName);
363376
return createDnsRecordResponse(record);
364377
} catch (Exception ex) {
365378
logger.error("Failed to add DNS record via provider", ex);
366-
throw new CloudRuntimeException(String.format("Failed to add DNS record: %s", cmd.getName()));
379+
throw new CloudRuntimeException(String.format("Failed to add DNS record: %s", recordName));
367380
}
368381
}
369382

@@ -397,7 +410,7 @@ public boolean deleteDnsRecord(DeleteDnsRecordCmd cmd) {
397410
public ListResponse<DnsRecordResponse> listDnsRecords(ListDnsRecordsCmd cmd) {
398411
DnsZoneVO zone = dnsZoneDao.findById(cmd.getDnsZoneId());
399412
if (zone == null) {
400-
throw new InvalidParameterValueException(String.format("DNS zone with ID %s not found.", cmd.getDnsZoneId()));
413+
throw new InvalidParameterValueException("DNS zone not found for the given ID.");
401414
}
402415
Account caller = CallContext.current().getCallingAccount();
403416
accountMgr.checkAccess(caller, null, true, zone);
@@ -435,28 +448,30 @@ public List<String> listProviderNames() {
435448

436449
@Override
437450
public DnsZone allocateDnsZone(CreateDnsZoneCmd cmd) {
438-
Account caller = CallContext.current().getCallingAccount();
451+
if (StringUtils.isBlank(cmd.getName())) {
452+
throw new InvalidParameterValueException("DNS zone name cannot be empty");
453+
}
454+
455+
String dnsZoneName = DnsUtil.normalizeDomain(cmd.getName());
439456
DnsServerVO server = dnsServerDao.findById(cmd.getDnsServerId());
440457
if (server == null) {
441-
throw new InvalidParameterValueException("DNS server not found");
458+
throw new InvalidParameterValueException(String.format("DNS server not found for the given ID: %s", cmd.getDnsServerId()));
442459
}
460+
461+
Account caller = CallContext.current().getCallingAccount();
443462
boolean isOwner = (server.getAccountId() == caller.getId());
444-
if (!server.isPublic() && !isOwner) {
445-
throw new PermissionDeniedException("You do not have permission to use this DNS server.");
446-
}
447-
DnsZone.ZoneType type = DnsZone.ZoneType.Public;
448-
if (cmd.getType() != null) {
449-
try {
450-
type = DnsZone.ZoneType.valueOf(cmd.getType());
451-
} catch (IllegalArgumentException e) {
452-
throw new InvalidParameterValueException("Invalid DNS zone Type");
463+
if (!isOwner) {
464+
if (!server.isPublic()) {
465+
throw new PermissionDeniedException("You do not have permission to use this DNS server.");
453466
}
467+
dnsZoneName = DnsUtil.appendPublicSuffixToZone(dnsZoneName, DnsUtil.normalizeDomain(server.getPublicDomainSuffix()));
454468
}
455-
DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(cmd.getName(), server.getId(), type);
469+
DnsZone.ZoneType type = cmd.getType();
470+
DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(dnsZoneName, server.getId(), type);
456471
if (existing != null) {
457472
throw new InvalidParameterValueException("DNS zone already exists on this server.");
458473
}
459-
DnsZoneVO dnsZoneVO = new DnsZoneVO(cmd.getName(), type, server.getId(), caller.getId(), caller.getDomainId(), cmd.getDescription());
474+
DnsZoneVO dnsZoneVO = new DnsZoneVO(dnsZoneName, type, server.getId(), caller.getId(), caller.getDomainId(), cmd.getDescription());
460475
return dnsZoneDao.persist(dnsZoneVO);
461476
}
462477

0 commit comments

Comments
 (0)