Copilot commented on code in PR #12737:
URL: https://github.com/apache/cloudstack/pull/12737#discussion_r3526383983
##########
server/src/main/java/com/cloud/api/ApiDBUtils.java:
##########
@@ -2297,6 +2310,17 @@ public static Map<String, String>
getResourceDetails(long resourceId, ResourceOb
return details.isEmpty() ? null : details;
}
+ public static Pair<String, String> findDnsZoneByNetworkId(long networkId) {
+ DnsZoneNetworkMapVO dnsNetworkMapVO =
s_dnsZoneNetworkMapDao.findByNetworkId(networkId);
+ if (dnsNetworkMapVO != null) {
+ DnsZoneVO dnsZoneVO =
s_dnsZoneDao.findById(dnsNetworkMapVO.getDnsZoneId());
+ if (Strings.isNotBlank(dnsZoneVO.getName())) {
+ return new Pair<> (dnsZoneVO.getName(),
dnsNetworkMapVO.getSubDomain());
+ }
+ }
Review Comment:
Potential NullPointerException: dnsZoneDao.findById(...) can return null,
but dnsZoneVO.getName() is dereferenced unconditionally. This can break network
response generation when a mapping points to a removed/missing zone.
##########
ui/src/components/view/ListView.vue:
##########
@@ -207,6 +207,9 @@
<template v-if="column.key === 'templatetype'">
<span>{{ text }}</span>
</template>
+ <template v-if="$route.path.startsWith('/dnsserver') && !['name',
'provider'].includes(column.key)">
Review Comment:
This /dnsserver catch-all bodyCell template can render in addition to other
column-specific templates (e.g. state/ispublic), causing duplicated content in
the cell. The current condition matches 'state' and 'ispublic', which are
already rendered later by dedicated templates.
##########
server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.dns.dao;
+
+import java.util.List;
+
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.dns.DnsZone;
+import org.apache.cloudstack.dns.vo.DnsZoneVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.springframework.stereotype.Component;
+
+import com.cloud.utils.Pair;
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+@Component
+public class DnsZoneDaoImpl extends GenericDaoBase<DnsZoneVO, Long> implements
DnsZoneDao {
+ SearchBuilder<DnsZoneVO> DnsServerSearch;
+ SearchBuilder<DnsZoneVO> AccountSearch;
+ SearchBuilder<DnsZoneVO> NameServerTypeSearch;
+
+ public DnsZoneDaoImpl() {
+ super();
+
+ DnsServerSearch = createSearchBuilder();
+ DnsServerSearch.selectFields(DnsServerSearch.entity().getId());
+ DnsServerSearch.and(ApiConstants.DNS_SERVER_ID,
DnsServerSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ);
+ DnsServerSearch.done();
Review Comment:
DnsServerSearch is configured with selectFields(getId()), but
findDnsZonesByServerId returns List<DnsZoneVO>. This means callers receive
partially-populated DnsZoneVOs (only columns selected are hydrated), which is
easy to misuse and can lead to null/default values being read unexpectedly.
##########
ui/src/views/network/NicsTable.vue:
##########
@@ -58,6 +58,9 @@
{{ record.isolationuri }}
</a-descriptions-item>
</template>
+ <a-descriptions-item :label="$t('label.dns.name')"
v-if="record.nicdnsname">
+ {{ record.nicdnsname }}
Review Comment:
The UI translation key label.dns.name appears to be missing from
ui/src/locales (no matches found), so the NIC DNS name label will likely
display as the raw key instead of a human-readable label.
##########
server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneNetworkMapDao.java:
##########
@@ -0,0 +1,29 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.dns.dao;
+
+import org.apache.cloudstack.dns.vo.DnsZoneNetworkMapVO;
+
+import com.cloud.utils.db.GenericDao;
+
+public interface DnsZoneNetworkMapDao extends GenericDao<DnsZoneNetworkMapVO,
Long> {
+ void removeNetworkMappingByZoneId(long dnsZoneId);
+ DnsZoneNetworkMapVO findByNetworkId(long networkId);
+
+ DnsZoneNetworkMapVO findByZoneId(long networkId);
Review Comment:
Method signature uses the wrong parameter name: findByZoneId(long
networkId). This is misleading (it is a zone ID, not a network ID) and makes
call sites harder to read.
##########
engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql:
##########
@@ -475,3 +475,81 @@ ADD UNIQUE KEY `load_balancer_id` (`load_balancer_id`,
`instance_id`, `instance_
ALTER TABLE `cloud`.`global_load_balancer_lb_rule_map`
DROP KEY `gslb_rule_id`,
ADD UNIQUE KEY `gslb_rule_id` (`gslb_rule_id`, `lb_rule_id`, `removed`);
+
+-- ======================================================================
+-- DNS Framework Schema
+-- ======================================================================
+
+-- DNS Server Table (Stores DNS Server Configurations)
+CREATE TABLE IF NOT EXISTS `cloud`.`dns_server` (
+ `id` bigint unsigned NOT NULL AUTO_INCREMENT COMMENT 'id of the dns
server',
+ `uuid` varchar(40) COMMENT 'uuid of the dns server',
+ `name` varchar(255) NOT NULL COMMENT 'display name of the dns server',
+ `provider_type` varchar(255) NOT NULL COMMENT 'Provider type such as
PowerDns',
+ `url` varchar(1024) NOT NULL COMMENT 'dns server url',
+ `dns_username` varchar(255) COMMENT 'username or email for dns server
credentials',
+ `dns_api_key` varchar(255) NOT NULL COMMENT 'api key or token for the dns
server ',
+ `port` int(11) DEFAULT NULL COMMENT 'optional dns server port',
+ `name_servers` varchar(1024) DEFAULT NULL COMMENT 'Comma separated list of
name servers',
+ `is_public` tinyint(1) NOT NULL DEFAULT '0',
+ `public_domain_suffix` VARCHAR(255),
+ `state` ENUM('Enabled', 'Disabled') NOT NULL DEFAULT 'Disabled',
+ `domain_id` bigint unsigned COMMENT 'for domain-specific ownership',
+ `account_id` bigint(20) unsigned NOT NULL,
+ `created` datetime NOT NULL COMMENT 'date created',
+ `removed` datetime DEFAULT NULL COMMENT 'Date removed (soft delete)',
+ PRIMARY KEY (`id`),
+ KEY `i_dns_server__account_id` (`account_id`),
+ CONSTRAINT `fk_dns_server__account_id` FOREIGN KEY (`account_id`)
REFERENCES `account` (`id`) ON DELETE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+
+-- DNS Server Details Table
+CREATE TABLE IF NOT EXISTS `cloud`.`dns_server_details` (
+ `id` bigint unsigned UNIQUE NOT NULL AUTO_INCREMENT COMMENT 'id',
+ `dns_server_id` bigint unsigned NOT NULL COMMENT 'dns_server the detail is
related to',
+ `name` varchar(255) NOT NULL COMMENT 'name of the detail',
+ `value` varchar(255) NOT NULL COMMENT 'value of the detail',
+ `display` tinyint(1) NOT NULL DEFAULT 1 COMMENT 'Should detail be displayed
to the end user',
+ PRIMARY KEY (`id`),
+ CONSTRAINT `fk_dns_server_details__dns_server_id` FOREIGN KEY
(`dns_server_id`) REFERENCES `dns_server`(`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+
+-- DNS Zone Table (Stores DNS Zone Metadata)
+CREATE TABLE IF NOT EXISTS `cloud`.`dns_zone` (
+ `id` bigint unsigned NOT NULL AUTO_INCREMENT COMMENT 'id of the dns zone',
+ `uuid` varchar(40) COMMENT 'uuid of the dns zone',
+ `name` varchar(255) NOT NULL COMMENT 'dns zone name (e.g. example.com)',
+ `dns_server_id` bigint unsigned NOT NULL COMMENT 'fk to dns_server.id',
+ `external_reference` VARCHAR(255) COMMENT 'id of external provider
resource',
+ `domain_id` bigint unsigned COMMENT 'for domain-specific ownership',
+ `account_id` bigint unsigned COMMENT 'account id. foreign key to account
table',
+ `description` varchar(1024) DEFAULT NULL,
+ `type` ENUM('Private', 'Public') NOT NULL DEFAULT 'Public',
+ `state` ENUM('Active', 'Inactive') NOT NULL DEFAULT 'Inactive',
+ `created` datetime NOT NULL COMMENT 'date created',
+ `removed` datetime DEFAULT NULL COMMENT 'Date removed (soft delete)',
+ PRIMARY KEY (`id`),
+ CONSTRAINT `uc_dns_zone__uuid` UNIQUE (`uuid`),
+ KEY `i_dns_zone__dns_server` (`dns_server_id`),
+ KEY `i_dns_zone__account_id` (`account_id`),
+ CONSTRAINT `fk_dns_zone__dns_server_id` FOREIGN KEY (`dns_server_id`)
REFERENCES `dns_server` (`id`) ON DELETE CASCADE,
+ CONSTRAINT `fk_dns_zone__account_id` FOREIGN KEY (`account_id`) REFERENCES
`account` (`id`) ON DELETE CASCADE,
+ CONSTRAINT `fk_dns_zone__domain_id` FOREIGN KEY (`domain_id`) REFERENCES
`domain` (`id`) ON DELETE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+
+-- DNS Zone Network Map (One-to-Many Link)
+CREATE TABLE IF NOT EXISTS `cloud`.`dns_zone_network_map` (
+ `id` bigint unsigned NOT NULL AUTO_INCREMENT COMMENT 'id of the dns zone to
network mapping',
+ `uuid` varchar(40),
+ `dns_zone_id` bigint(20) unsigned NOT NULL,
+ `network_id` bigint(20) unsigned NOT NULL COMMENT 'network to which dns zone
is associated to',
+ `sub_domain` varchar(255) DEFAULT NULL COMMENT 'Subdomain for
auto-registration',
+ `created` datetime NOT NULL COMMENT 'date created',
+ `removed` datetime DEFAULT NULL COMMENT 'Date removed (soft delete)',
+ PRIMARY KEY (`id`),
+ CONSTRAINT `uc_dns_zone__uuid` UNIQUE (`uuid`),
Review Comment:
The UNIQUE constraint name for dns_zone_network_map UUID is copied from
dns_zone (uc_dns_zone__uuid). While it likely works, it is misleading when
inspecting schema/indexes and makes future migrations harder to reason about.
--
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]
