Skip to content

Commit f164726

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 f164726

8 files changed

Lines changed: 250 additions & 33 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: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package org.apache.cloudstack.dns;
2+
3+
import static org.apache.cloudstack.dns.DnsUtil.appendPublicSuffixToZone;
4+
import static org.apache.cloudstack.dns.DnsUtil.normalizeDomain;
5+
import static org.junit.Assert.assertEquals;
6+
import static org.junit.Assert.fail;
7+
8+
import java.util.Arrays;
9+
import java.util.Collection;
10+
11+
import org.apache.logging.log4j.util.Strings;
12+
import org.junit.Test;
13+
import org.junit.runner.RunWith;
14+
import org.junit.runners.Parameterized;
15+
16+
@RunWith(Parameterized.class)
17+
public class DnsUtilTest {
18+
private final String userZoneName;
19+
private final String publicSuffix;
20+
private final String expectedResult;
21+
private final boolean expectException;
22+
23+
public DnsUtilTest(String userZoneName,
24+
String publicSuffix,
25+
String expectedResult,
26+
boolean expectException) {
27+
this.userZoneName = userZoneName;
28+
this.publicSuffix = publicSuffix;
29+
this.expectedResult = expectedResult;
30+
this.expectException = expectException;
31+
}
32+
33+
@Parameterized.Parameters
34+
public static Collection<Object[]> data() {
35+
return Arrays.asList(new Object[][]{
36+
{"tenant1.com", "example.com", "tenant1.example.com", false},
37+
{"dev.tenant2.com", "example.com", "dev.tenant2.example.com", false},
38+
{"tenant3.example.com", "example.com", "tenant3.example.com", false},
39+
{"Tenant1.CoM", "ExAmple.CoM", "tenant1.example.com", false},
40+
{"tenant1.com.", "example.com.", "tenant1.example.com", false},
41+
{"tenant1.com", "", "tenant1.com", false},
42+
{"tenant1.com", null, "tenant1.com", false},
43+
{"test.abc.com", "abc.com", "test.abc.com", false},
44+
{"sub.test.abc.com", "abc.com", "sub.test.abc.com", false},
45+
{"test.ai.abc.com", "abc.com", "test.ai.abc.com", false},
46+
{"deep.sub.abc.com", "abc.com", "deep.sub.abc.com", false},
47+
{"abc.com", "xyz.com", "abc.xyz.com", false},
48+
{"test.xyz.com", "xyz.com", "test.xyz.com", false},
49+
{"test.com.xyz.com", "xyz.com", "test.com.xyz.com", false},
50+
{"tenant", "example.com", null, true}, // single label
51+
{"test", "abc.com", null, true},
52+
{"example.com.", "example.com", null, true},
53+
{"example.com", "example.com", null, true}, // root level forbidden
54+
{"abc.com", "abc.com", null, true}, // root level forbidden
55+
{"tenant1.org", "example.com", null, true}, // TLD mismatch
56+
{"test.ai", "abc.com", null, true}, // TLD mismatch
57+
{null, "example.com", null, true},
58+
});
59+
}
60+
61+
@Test
62+
public void testAppendPublicSuffix() {
63+
if (expectException) {
64+
try {
65+
executeAppendSuffixTest(userZoneName, publicSuffix);
66+
fail("Expected IllegalArgumentException");
67+
} catch (IllegalArgumentException ignored) {
68+
// noop
69+
}
70+
} else {
71+
String result;
72+
if (Strings.isNotBlank(publicSuffix)) {
73+
result = executeAppendSuffixTest(userZoneName, publicSuffix);
74+
} else {
75+
result = appendPublicSuffixToZone(normalizeDomain(userZoneName), publicSuffix);
76+
}
77+
assertEquals(expectedResult, result);
78+
}
79+
}
80+
81+
String executeAppendSuffixTest(String zoneName, String domainSuffix) {
82+
return appendPublicSuffixToZone(
83+
normalizeDomain(zoneName),
84+
normalizeDomain(domainSuffix));
85+
}
86+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import static org.junit.Assert.assertEquals;
2121

22+
import org.apache.commons.validator.routines.DomainValidator;
2223
import org.junit.Test;
2324
import org.mockito.InjectMocks;
2425

@@ -81,4 +82,16 @@ public void testNormalizeZoneNormalization() {
8182
assertEquals("www.example.com.", result);
8283
}
8384

85+
@Test
86+
public void testsomething() {
87+
DomainValidator validator = DomainValidator.getInstance(true);
88+
System.out.println(validator.isValid("example.com"));
89+
System.out.println(validator.isValid("dev.example.com"));
90+
System.out.println(validator.isValid("dev.example.ai"));
91+
System.out.println(validator.isValid("dev.example.ai."));
92+
System.out.println(validator.isValid("example.org."));
93+
System.out.println(validator.isValid("example.in"));
94+
System.out.println(validator.isValid("localhost"));
95+
}
96+
8497
}

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

Lines changed: 32 additions & 17 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());
@@ -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

@@ -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)