Skip to content

Commit 80f7983

Browse files
sudo87sudo87
committed
address review comments
1 parent dc18aa7 commit 80f7983

8 files changed

Lines changed: 32 additions & 16 deletions

File tree

‎api/src/main/java/com/cloud/configuration/Resource.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ enum ResourceType { // All storage type resources are allocated_storage and not
3838
backup_storage("backup_storage", 13),
3939
bucket("bucket", 14),
4040
object_storage("object_storage", 15),
41-
gpu("gpu", 16),
42-
dns_zone("dns_zone", 17);
41+
gpu("gpu", 16);
4342

4443
private String name;
4544
private int ordinal;

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,22 @@ public class AddDnsServerCmd extends BaseCmd {
6868
@Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port number of the external DNS server")
6969
private Integer port;
7070

71-
@Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Whether the DNS server is publicly accessible by other accounts")
71+
@Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN,
72+
description = "Whether this DNS server can be used by accounts other than the owner to create and manage DNS zones")
7273
private Boolean isPublic;
7374

74-
@Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, description = "The domain suffix used for public access (e.g. public.example.com)")
75+
@Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING,
76+
description = "Domain suffix that restricts DNS zones created by non-owner accounts to subdomains of this " +
77+
"suffix (for example, sub.example.com under example.com)")
7578
private String publicDomainSuffix;
7679

7780
@Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.LIST, collectionType = CommandType.STRING,
78-
required = true, description = "Comma separated list of name servers")
81+
required = true,
82+
description = "Comma separated list of name servers; used to create NS records for the DNS Zone (for example, ns1.example.com, ns2.example.com)")
7983
private List<String> nameServers;
8084

81-
@Parameter(name = "externalserverid", type = CommandType.STRING, description = "External server id or hostname for the DNS server, e.g., 'localhost' for PowerDNS")
85+
@Parameter(name = "externalserverid", type = CommandType.STRING,
86+
description = "External server id or hostname for the DNS server, e.g., 'localhost' for PowerDNS")
8287
private String externalServerId;
8388

8489
/////////////////////////////////////////////////////

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

Lines changed: 13 additions & 6 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.List;
21+
2022
import org.apache.cloudstack.acl.RoleType;
2123
import org.apache.cloudstack.acl.SecurityChecker;
2224
import org.apache.cloudstack.api.ACL;
@@ -58,20 +60,25 @@ public class UpdateDnsServerCmd extends BaseCmd {
5860
@Parameter(name = ApiConstants.URL, type = CommandType.STRING, description = "API URL of the provider")
5961
private String url;
6062

61-
@Parameter(name = ApiConstants.DNS_API_KEY, type = CommandType.STRING, required = false, description = "API Key or Credentials for the external provider")
63+
@Parameter(name = ApiConstants.DNS_API_KEY, type = CommandType.STRING, description = "API Key or Credentials for the external provider")
6264
private String dnsApiKey;
6365

6466
@Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port number of the external DNS server")
6567
private Integer port;
6668

67-
@Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Whether the DNS server is publicly accessible by other accounts")
69+
@Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN,
70+
description = "Whether this DNS server can be used by accounts other than the owner to create and manage DNS zones")
6871
private Boolean isPublic;
6972

70-
@Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, description = "The domain suffix used for public access (e.g. public.example.com)")
73+
@Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING,
74+
description = "Domain suffix that restricts DNS zones created by non-owner accounts to subdomains of this " +
75+
"suffix (for example, sub.example.com under example.com)")
7176
private String publicDomainSuffix;
7277

73-
@Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.STRING, description = "Comma separated list of name servers")
74-
private String nameServers;
78+
@Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.LIST, collectionType = CommandType.STRING,
79+
required = true,
80+
description = "Comma separated list of name servers; used to create NS records for the DNS Zone (for example, ns1.example.com, ns2.example.com)")
81+
private List<String> nameServers;
7582

7683
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "Update state for the DNS server (Enabled, Disabled)")
7784
private String state;
@@ -95,7 +102,7 @@ public Boolean isPublic() {
95102
public String getPublicDomainSuffix() {
96103
return publicDomainSuffix;
97104
}
98-
public String getNameServers() { return nameServers; }
105+
public String getNameServers() { return String.join(",", nameServers); }
99106

100107
@Override
101108
public long getEntityOwnerId() {

‎api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public class NicResponse extends BaseResponse {
151151
private Boolean isEnabled;
152152

153153
@SerializedName(ApiConstants.NIC_DNS_NAME)
154-
@Param(description = "Public IP address associated with this NIC via Static NAT rule")
154+
@Param(description = "DNS name associated with this NIC's IP address")
155155
private String nicDnsName;
156156

157157
public void setVmId(String vmId) {

‎api/src/test/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmdTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import static org.mockito.Mockito.mock;
2424
import static org.mockito.Mockito.when;
2525

26+
import java.util.Arrays;
27+
2628
import org.apache.cloudstack.api.ServerApiException;
2729
import org.apache.cloudstack.api.response.DnsServerResponse;
2830
import org.apache.cloudstack.dns.DnsServer;
@@ -43,7 +45,7 @@ private UpdateDnsServerCmd createCmd() throws Exception {
4345
setField(cmd, "port", 9090);
4446
setField(cmd, "isPublic", true);
4547
setField(cmd, "publicDomainSuffix", "updated.example.com");
46-
setField(cmd, "nameServers", "ns1.updated.com,ns2.updated.com");
48+
setField(cmd, "nameServers", Arrays.asList("ns1.updated.com", "ns2.updated.com"));
4749
setField(cmd, "state", "Enabled");
4850
return cmd;
4951
}

‎engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121

2222
import org.apache.cloudstack.api.ApiConstants;
23+
import org.apache.commons.collections.CollectionUtils;
2324
import org.springframework.stereotype.Component;
2425

2526
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
@@ -48,6 +49,9 @@ public void addDetail(long resourceId, String key, String value, boolean display
4849

4950
@Override
5051
public void removeDetailsForValuesIn(String resourceName, List<String> values) {
52+
if (CollectionUtils.isEmpty(values)) {
53+
return;
54+
}
5155
SearchCriteria<NicDetailVO> sc = NameValuesSearch.create();
5256
sc.setParameters(ApiConstants.NAME, resourceName);
5357
sc.setParameters(ApiConstants.VALUE, values.toArray());

‎server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ public boolean configure(final String name, final Map<String, Object> params) th
328328
accountResourceLimitMap.put(Resource.ResourceType.backup_storage.name(), Long.parseLong(_configDao.getValue(BackupManager.DefaultMaxAccountBackupStorage.key())));
329329
accountResourceLimitMap.put(Resource.ResourceType.bucket.name(), Long.parseLong(_configDao.getValue(BucketApiService.DefaultMaxAccountBuckets.key())));
330330
accountResourceLimitMap.put(Resource.ResourceType.object_storage.name(), Long.parseLong(_configDao.getValue(BucketApiService.DefaultMaxAccountObjectStorage.key())));
331-
accountResourceLimitMap.put(ResourceType.dns_zone.name(), DefaultMaxDnsAccounts.value());
332331

333332
domainResourceLimitMap.put(Resource.ResourceType.public_ip.name(), Long.parseLong(_configDao.getValue(Config.DefaultMaxDomainPublicIPs.key())));
334333
domainResourceLimitMap.put(Resource.ResourceType.snapshot.name(), Long.parseLong(_configDao.getValue(Config.DefaultMaxDomainSnapshots.key())));

‎ui/src/config/section/network.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1527,7 +1527,7 @@ export default {
15271527
},
15281528
{
15291529
name: 'dnsserver',
1530-
title: 'label.dns.server',
1530+
title: 'label.dns.servers',
15311531
icon: 'cloud-server-outlined',
15321532
permission: ['listDnsServers'],
15331533
columns: ['name', 'url', 'provider', 'ispublic', 'port', 'nameservers', 'publicdomainsuffix'],

0 commit comments

Comments
 (0)