Refactor DNS provider implementations to use CancellationToken for async methods#989
Merged
Refactor DNS provider implementations to use CancellationToken for async methods#989
Conversation
…ync methods - Updated all IDnsProvider implementations to include CancellationToken in ListZonesAsync, CreateTxtRecordAsync, and DeleteTxtRecordAsync methods. - Changed PropagationSeconds property to PropagationDelay of type TimeSpan for better clarity. - Refactored HTTP requests to use new async methods with CancellationToken support. - Improved error handling in DeleteTxtRecordAsync methods across providers to ignore NotFound exceptions. - Introduced new internal classes for TXT record parameters and domain pagination handling. - Enhanced code readability and consistency across different DNS provider implementations.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors DNS provider implementations and orchestration code to support cancellation-aware async operations, while tightening nullability/type-safety across models and options (and addressing renewal/DNS-challenge robustness for #913).
Changes:
- Update
IDnsProviderto useTimeSpan PropagationDelayand addCancellationTokenparameters across DNS provider async APIs. - Enforce stricter nullability/type-safety via
requiredproperties andWarningsAsErrorsfor nullable warnings. - Improve DNS challenge orchestration and provider implementations (including “record not found” handling and enum-based ACME status checks).
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Acmebot.App/Providers/IDnsProvider.cs | Add CancellationToken + PropagationDelay API. |
| src/Acmebot.App/Providers/AkamaiEdgeDnsProvider.cs | Implement new provider interface + cancellation. |
| src/Acmebot.App/Providers/AzureDnsProvider.cs | Implement new provider interface + cancellation. |
| src/Acmebot.App/Providers/AzurePrivateDnsProvider.cs | Implement new provider interface + cancellation; adjust visibility. |
| src/Acmebot.App/Providers/CloudflareProvider.cs | Refactor Cloudflare client, paging, cancellation, and record ops. |
| src/Acmebot.App/Providers/CustomDnsProvider.cs | Switch to System.Net.Http.Json + cancellation + 404 ignore. |
| src/Acmebot.App/Providers/DnsMadeEasyProvider.cs | Refactor API client to Json helpers + cancellation + new models. |
| src/Acmebot.App/Providers/DnsZone.cs | Strengthen equality/hash semantics for zones. |
| src/Acmebot.App/Providers/GandiLiveDnsProvider.cs | Add cancellation + improved null handling in API responses. |
| src/Acmebot.App/Providers/GoDaddyProvider.cs | Add cancellation + async paging for domains; ignore 404 deletes. |
| src/Acmebot.App/Providers/GoogleDnsProvider.cs | Add cancellation support to Google DNS calls. |
| src/Acmebot.App/Providers/Route53Provider.cs | Add cancellation support; safer delete behavior. |
| src/Acmebot.App/Providers/TransIpProvider.cs | Add cancellation support; move to System.Text.Json + token handling tweaks. |
| src/Acmebot.App/Options/CloudflareOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/DnsMadeEasyOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/GandiLiveDnsOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/GoDaddyOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/GoogleDnsOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/Route53Options.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Options/TransIpOptions.cs | Make option properties required (non-nullable). |
| src/Acmebot.App/Models/AcmeChallengeResult.cs | Tighten types (Uri, required) for challenge payloads. |
| src/Acmebot.App/Models/CertificateItem.cs | Tighten types (required) for certificate model. |
| src/Acmebot.App/Functions/Orchestration/DnsChallengeActivities.cs | Pass cancellation through DNS challenge activities; handle null zone lists. |
| src/Acmebot.App/Functions/Orchestration/CertificateIssuanceOrchestrator.cs | Use typed ACME statuses + extra input validation. |
| src/Acmebot.App/Functions/Orchestration/AcmeOrderActivities.cs | Use Uri directly for challenge operations. |
| src/Acmebot.App/Functions/Orchestration/RenewCertificates.cs | Null-safety simplification for logging. |
| src/Acmebot.App/Extensions/DnsProvidersExtensions.cs | Add cancellation + make zone list nullable on provider failures. |
| src/Acmebot.App/Extensions/HttpClientExtensions.cs | Add CancellationToken plumbing to extension methods. |
| src/Acmebot.App/Acme/AcmeClientFactory.cs | Minor refactors + safer directory creation in state persistence. |
| src/Acmebot.App/Acmebot.App.csproj | Treat nullable warnings as errors. |
| src/Acmebot.Acme/Acmebot.Acme.csproj | Treat nullable warnings as errors. |
| src/Acmebot.Acme.Tests/Acmebot.Acme.Tests.csproj | Treat nullable warnings as errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…and improve cancellation token usage
…roved functionality
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and refactorings across the codebase, focusing on stricter nullability enforcement, improved cancellation support, and enhanced type safety. The most significant changes are the addition of cancellation token support to asynchronous methods, stricter nullability requirements for key models and options, and minor bug fixes and code simplifications.
Nullability and Type Safety Improvements:
<WarningsAsErrors>nullable</WarningsAsErrors>to all project files to enforce strict nullability checks during compilation. [1] [2] [3]AcmeChallengeResult,CertificateItem, and DNS provider options to userequiredproperties and non-nullable types, improving type safety and reducing the risk of null reference errors. [1] [2] [3] [4] [5]Cancellation Token Support:
CancellationTokenparameters to many async extension methods and function activities, ensuring that long-running operations can be cancelled gracefully. This includes DNS provider zone listing, HTTP client methods, and DNS challenge orchestration activities. [1] [2] [3] [4] [5] [6] [7] [8] [9]Functional and Logical Updates:
AcmeOrderStatuses.Readyinstead of string literals), and improved input validation for certificate issuance. [1] [2] [3] [4]PropagationDelay.TotalSeconds. [1] [2] [3]Minor Bug Fixes and Code Simplification:
SaveStateby ensuring directory existence checks handle null or empty paths.ContactsEqualand other minor refactorings for clarity and correctness. [1] [2] [3] [4]These changes collectively improve code robustness, maintainability, and reliability, especially in asynchronous and distributed scenarios.
Close #913