From 80f79839107c0174c7112ea20f1a5079604419d9 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Tue, 28 Apr 2026 23:30:07 +0530 Subject: [PATCH] address review comments --- .../com/cloud/configuration/Resource.java | 3 +-- .../api/command/user/dns/AddDnsServerCmd.java | 13 +++++++++---- .../command/user/dns/UpdateDnsServerCmd.java | 19 +++++++++++++------ .../cloudstack/api/response/NicResponse.java | 2 +- .../user/dns/UpdateDnsServerCmdTest.java | 4 +++- .../com/cloud/vm/dao/NicDetailsDaoImpl.java | 4 ++++ .../ResourceLimitManagerImpl.java | 1 - ui/src/config/section/network.js | 2 +- 8 files changed, 32 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/com/cloud/configuration/Resource.java b/api/src/main/java/com/cloud/configuration/Resource.java index 41abce02eb4..97be7f9d64c 100644 --- a/api/src/main/java/com/cloud/configuration/Resource.java +++ b/api/src/main/java/com/cloud/configuration/Resource.java @@ -38,8 +38,7 @@ public interface Resource { backup_storage("backup_storage", 13), bucket("bucket", 14), object_storage("object_storage", 15), - gpu("gpu", 16), - dns_zone("dns_zone", 17); + gpu("gpu", 16); private String name; private int ordinal; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/AddDnsServerCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/AddDnsServerCmd.java index 9227fb84d19..fcf38e8978a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/AddDnsServerCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/AddDnsServerCmd.java @@ -68,17 +68,22 @@ public class AddDnsServerCmd extends BaseCmd { @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port number of the external DNS server") private Integer port; - @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Whether the DNS server is publicly accessible by other accounts") + @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, + description = "Whether this DNS server can be used by accounts other than the owner to create and manage DNS zones") private Boolean isPublic; - @Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, description = "The domain suffix used for public access (e.g. public.example.com)") + @Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, + description = "Domain suffix that restricts DNS zones created by non-owner accounts to subdomains of this " + + "suffix (for example, sub.example.com under example.com)") private String publicDomainSuffix; @Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.LIST, collectionType = CommandType.STRING, - required = true, description = "Comma separated list of name servers") + required = true, + description = "Comma separated list of name servers; used to create NS records for the DNS Zone (for example, ns1.example.com, ns2.example.com)") private List nameServers; - @Parameter(name = "externalserverid", type = CommandType.STRING, description = "External server id or hostname for the DNS server, e.g., 'localhost' for PowerDNS") + @Parameter(name = "externalserverid", type = CommandType.STRING, + description = "External server id or hostname for the DNS server, e.g., 'localhost' for PowerDNS") private String externalServerId; ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java index 7b3050db1d8..96725d4a10f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java @@ -17,6 +17,8 @@ package org.apache.cloudstack.api.command.user.dns; +import java.util.List; + import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.ACL; @@ -58,20 +60,25 @@ public class UpdateDnsServerCmd extends BaseCmd { @Parameter(name = ApiConstants.URL, type = CommandType.STRING, description = "API URL of the provider") private String url; - @Parameter(name = ApiConstants.DNS_API_KEY, type = CommandType.STRING, required = false, description = "API Key or Credentials for the external provider") + @Parameter(name = ApiConstants.DNS_API_KEY, type = CommandType.STRING, description = "API Key or Credentials for the external provider") private String dnsApiKey; @Parameter(name = ApiConstants.PORT, type = CommandType.INTEGER, description = "Port number of the external DNS server") private Integer port; - @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Whether the DNS server is publicly accessible by other accounts") + @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, + description = "Whether this DNS server can be used by accounts other than the owner to create and manage DNS zones") private Boolean isPublic; - @Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, description = "The domain suffix used for public access (e.g. public.example.com)") + @Parameter(name = ApiConstants.PUBLIC_DOMAIN_SUFFIX, type = CommandType.STRING, + description = "Domain suffix that restricts DNS zones created by non-owner accounts to subdomains of this " + + "suffix (for example, sub.example.com under example.com)") private String publicDomainSuffix; - @Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.STRING, description = "Comma separated list of name servers") - private String nameServers; + @Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.LIST, collectionType = CommandType.STRING, + required = true, + description = "Comma separated list of name servers; used to create NS records for the DNS Zone (for example, ns1.example.com, ns2.example.com)") + private List nameServers; @Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "Update state for the DNS server (Enabled, Disabled)") private String state; @@ -95,7 +102,7 @@ public class UpdateDnsServerCmd extends BaseCmd { public String getPublicDomainSuffix() { return publicDomainSuffix; } - public String getNameServers() { return nameServers; } + public String getNameServers() { return String.join(",", nameServers); } @Override public long getEntityOwnerId() { diff --git a/api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java index ad09380e2a4..5ad41ad6224 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java @@ -151,7 +151,7 @@ public class NicResponse extends BaseResponse { private Boolean isEnabled; @SerializedName(ApiConstants.NIC_DNS_NAME) - @Param(description = "Public IP address associated with this NIC via Static NAT rule") + @Param(description = "DNS name associated with this NIC's IP address") private String nicDnsName; public void setVmId(String vmId) { diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmdTest.java index c3fe2c821fb..f3c0fcd4fe0 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmdTest.java @@ -23,6 +23,8 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.Arrays; + import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DnsServerResponse; import org.apache.cloudstack.dns.DnsServer; @@ -43,7 +45,7 @@ public class UpdateDnsServerCmdTest extends BaseDnsCmdTest { setField(cmd, "port", 9090); setField(cmd, "isPublic", true); setField(cmd, "publicDomainSuffix", "updated.example.com"); - setField(cmd, "nameServers", "ns1.updated.com,ns2.updated.com"); + setField(cmd, "nameServers", Arrays.asList("ns1.updated.com", "ns2.updated.com")); setField(cmd, "state", "Enabled"); return cmd; } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java index 9ee2e41b34b..3b391a50350 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/NicDetailsDaoImpl.java @@ -20,6 +20,7 @@ package com.cloud.vm.dao; import java.util.List; import org.apache.cloudstack.api.ApiConstants; +import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase; @@ -48,6 +49,9 @@ public class NicDetailsDaoImpl extends ResourceDetailsDaoBase imple @Override public void removeDetailsForValuesIn(String resourceName, List values) { + if (CollectionUtils.isEmpty(values)) { + return; + } SearchCriteria sc = NameValuesSearch.create(); sc.setParameters(ApiConstants.NAME, resourceName); sc.setParameters(ApiConstants.VALUE, values.toArray()); diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 2983d788586..729da7caa33 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -328,7 +328,6 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim accountResourceLimitMap.put(Resource.ResourceType.backup_storage.name(), Long.parseLong(_configDao.getValue(BackupManager.DefaultMaxAccountBackupStorage.key()))); accountResourceLimitMap.put(Resource.ResourceType.bucket.name(), Long.parseLong(_configDao.getValue(BucketApiService.DefaultMaxAccountBuckets.key()))); accountResourceLimitMap.put(Resource.ResourceType.object_storage.name(), Long.parseLong(_configDao.getValue(BucketApiService.DefaultMaxAccountObjectStorage.key()))); - accountResourceLimitMap.put(ResourceType.dns_zone.name(), DefaultMaxDnsAccounts.value()); domainResourceLimitMap.put(Resource.ResourceType.public_ip.name(), Long.parseLong(_configDao.getValue(Config.DefaultMaxDomainPublicIPs.key()))); domainResourceLimitMap.put(Resource.ResourceType.snapshot.name(), Long.parseLong(_configDao.getValue(Config.DefaultMaxDomainSnapshots.key()))); diff --git a/ui/src/config/section/network.js b/ui/src/config/section/network.js index ebc98c4f4fb..2c111b2b3cc 100644 --- a/ui/src/config/section/network.js +++ b/ui/src/config/section/network.js @@ -1527,7 +1527,7 @@ export default { }, { name: 'dnsserver', - title: 'label.dns.server', + title: 'label.dns.servers', icon: 'cloud-server-outlined', permission: ['listDnsServers'], columns: ['name', 'url', 'provider', 'ispublic', 'port', 'nameservers', 'publicdomainsuffix'],