Skip to content

Extend service discovery to support Consul-based DNS lookups:#6914

Merged
rzikm merged 5 commits into
dotnet:mainfrom
SteeltoeOSS:service-discovery-consul
Oct 17, 2025
Merged

Extend service discovery to support Consul-based DNS lookups:#6914
rzikm merged 5 commits into
dotnet:mainfrom
SteeltoeOSS:service-discovery-consul

Conversation

@bart-vmware

@bart-vmware bart-vmware commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

This PR provides the building blocks for microsoft/aspire#11740.

Extend service discovery to support HashiCorp Consul-based DNS lookups:

  • Enable specifying how to construct the DNS SRV query
  • Enable adding the Consul DNS server host/port
Microsoft Reviewers: Open in CodeFlow

@github-actions github-actions Bot added the area-ai Microsoft.Extensions.AI libraries label Oct 13, 2025
@bart-vmware bart-vmware marked this pull request as ready for review October 13, 2025 13:25
Copilot AI review requested due to automatic review settings October 13, 2025 13:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR extends service discovery to support HashiCorp Consul-based DNS lookups by implementing new DNS resolver configuration options and query customization capabilities. The changes enable flexible DNS server configuration and customizable DNS SRV query construction to support Consul service discovery patterns.

  • Refactors DNS resolver options to support runtime configuration and custom DNS servers
  • Adds configurable DNS SRV query construction for Consul-style lookups
  • Updates dependency injection patterns to support options-based configuration

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
DnsSrvServiceEndpointProviderOptions.cs Adds ResolverOptions property and GetQueryText delegate for custom DNS query construction
ResolverOptions.cs Converts from internal sealed class to public class with properties and XML documentation
DnsResolver.cs Adds Create factory method and updates constructor to handle empty server lists
DnsSrvServiceEndpointProviderFactory.cs Updates to use custom query text generation when configured
ServiceDiscoveryDnsServiceCollectionExtensions.cs Changes DI registration to use factory method
ResolvConf.cs Refactors to return server list instead of ResolverOptions
NetworkInfo.cs Refactors to return server list instead of ResolverOptions
Test files Updates test instantiations to match new constructor signatures

@davidfowl

Copy link
Copy Markdown
Member

@ReubenBond @rzikm can you take a look?

@rzikm rzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this one up. Comments are mostly about naming since I did not give too much thought to them since they were originally meant to be internal.

@rzikm

rzikm commented Oct 13, 2025

Copy link
Copy Markdown
Member

Also, do we not have to go through https://apireview.net/ since this is public API?

@bart-vmware

Copy link
Copy Markdown
Contributor Author

@rzikm Thanks for the review. I've pushed changes to address them. The link "Details about the pull request review procedure" at https://github.com/dotnet/extensions/blob/main/CONTRIBUTING.md#suggested-workflow is dead, so it's unclear to me what the process is. Can I assume you'll resolve each open conversation that is addressed as desired?

Comment thread src/Libraries/Microsoft.Extensions.ServiceDiscovery.Dns/Resolver/DnsResolver.cs Outdated
@bart-vmware bart-vmware requested a review from rzikm October 14, 2025 15:42

@rzikm rzikm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bart-vmware

Copy link
Copy Markdown
Contributor Author

Also, do we not have to go through https://apireview.net/ since this is public API?

@davidfowl Changes were reviewed and approved. Do we need public API review first, or is this ready to be merged?
And once merged, how does it land in the Aspire codebase?

@davidfowl

Copy link
Copy Markdown
Member

As long as @rzikm and @ReubenBond approve, I'm good!

@bart-vmware

Copy link
Copy Markdown
Contributor Author

@rzikm Can you merge this?

This was referenced Nov 19, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants