CloudStack DNS framework - Integrate PowerDNS as first plugin#12737
CloudStack DNS framework - Integrate PowerDNS as first plugin#12737sudo87 wants to merge 54 commits intoapache:mainfrom
Conversation
2. added relevant changes in dao and vo 3. worked on creatednszone, integration with mgr 4. powerdns create zone api call
1. creatednszone 2. listdnszone 3. updatednszone 4. deletednszone
1. Add dns server 2. create zone 3. add records 4. verify in powerdns 5. verify using dig
1. Registerdnsrecordforvm api 2. removednsrecordforvm api 3. cleanup; fixed license, dao logic
1. refactored client 2. added exceptions 3. enhanced updateZone 4. ownership check for deleteDnsServer
…n svc and handle dot version in client
…elete events 2. add dnsrecordurl in nic_details table 3. add dnsrecordurl in vm response
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12737 +/- ##
============================================
+ Coverage 17.95% 18.28% +0.32%
- Complexity 16522 16978 +456
============================================
Files 6022 6083 +61
Lines 541387 544955 +3568
Branches 66346 66790 +444
============================================
+ Hits 97211 99640 +2429
- Misses 433210 434193 +983
- Partials 10966 11122 +156
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17552 |
|
[SF] Trillian test result (tid-15924)
|
|
[SF] Trillian test result (tid-15933)
|
1. stop and start vm operations wont trigger dns sync 2. ip update for nic will refresh dns records
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17598 |
|
@sudo87 can you update the description to summarize the changes ? |
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17614 |
8ca39da to
e997f4c
Compare
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17616 |
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17625 |
|
@blueorangutan package |
|
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17627 |
| object_storage("object_storage", 15), | ||
| gpu("gpu", 16); | ||
| gpu("gpu", 16), | ||
| dns_zone("dns_zone", 17); |
There was a problem hiding this comment.
resource limitation on dns_zone is not implemented, isn't it ?
| @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Whether the DNS server is publicly accessible by other accounts") | ||
| 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)") |
There was a problem hiding this comment.
@sudo87
can you add more in the description ? for example, `If set, user can only create dns zones with this domain suffix ......"
| private String publicDomainSuffix; | ||
|
|
||
| @Parameter(name = ApiConstants.NAME_SERVERS, type = CommandType.LIST, collectionType = CommandType.STRING, | ||
| required = true, description = "Comma separated list of name servers") |
There was a problem hiding this comment.
add more details in the description like below ?
`NS records will be created for the DNS zone ......"
| private List<String> nameServers; | ||
|
|
||
| @Parameter(name = "externalserverid", type = CommandType.STRING, description = "External server id or hostname for the DNS server, e.g., 'localhost' for PowerDNS") | ||
| private String externalServerId; |
There was a problem hiding this comment.
is it applicable for PowerDNS only ?
I think we'd better avoid provider-specific parameters
| @ACL(accessType = SecurityChecker.AccessType.OperateEntry) | ||
| @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = DnsZoneResponse.class, required = true, | ||
| description = "The ID of the DNS zone") | ||
| private Long id; |
There was a problem hiding this comment.
when delete a DNS zone, is it possible to keep the DNS records on the external provider ?
|
|
||
| @Parameter(name = ApiConstants.PROVIDER_TYPE, type = CommandType.STRING, | ||
| description = "filter by provider type (e.g. PowerDNS, Cloudflare)") | ||
| private String providerType; |
There was a problem hiding this comment.
is this optional or required ?
will line 65 throw an exception if not set ?
| private Boolean isEnabled; | ||
|
|
||
| @SerializedName(ApiConstants.NIC_DNS_NAME) | ||
| @Param(description = "Public IP address associated with this NIC via Static NAT rule") |
There was a problem hiding this comment.
the description seems wrong
| public enum DnsProviderType { | ||
| PowerDNS; | ||
| // Cloudflare | ||
| } |
There was a problem hiding this comment.
there is an API to list all available DNS provider types:
ListDnsProvidersCmd, which calls listProviderNames
this enum may be not needed
| public void removeDetailsForValuesIn(String resourceName, List<String> values) { | ||
| SearchCriteria<NicDetailVO> sc = NameValuesSearch.create(); | ||
| sc.setParameters(ApiConstants.NAME, resourceName); | ||
| sc.setParameters(ApiConstants.VALUE, values.toArray()); |
There was a problem hiding this comment.
to be safer, check if values is null or empty ? do nothing if so.
| } | ||
| HttpPost request = new HttpPost(buildUrl(baseUrl, port, "/servers/" + externalServerId + "/zones")); | ||
| request.setEntity(new StringEntity(json.toString(), StandardCharsets.UTF_8)); | ||
| JsonNode response = execute(request, apiKey, 201); |
There was a problem hiding this comment.
if the DNS zone already exists in powerdns, what will happen ?
There was a problem hiding this comment.
I wonder if DNS zone existence need to be checked before each DNS zone operation (create, update, list, delete), it will lead to 1 more API and cause longer response time. need to consider the trade-offs
Description
This PR introduces initial implementation for a plugin based DNS framework in Apache CloudStack. It enables both admin and end-users to manage DNS zones and record with external authoritative dns providers.
First supported DNS provider: PowerDNS
Github issue: #9958
Cwiki: https://cwiki.apache.org/confluence/display/CLOUDSTACK/DNS+Framework+and+Plugins
Doc PR: apache/cloudstack-documentation#646
Terminology:
API Changes
There are following APIs have been introduced to support DNS management from CloudStack and Instance auto registration:
UI changes




Supported network for Auto Registration: Shared network
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?