refactor: add service package and start splitting HTTP handling #1427
Conversation
This file contains 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
Package service exposes types to abstract services from the networking. The idea is that we build a set of services and a set of network endpoints (Listener). The services are then assigned to endpoints based on the address(es) they were configured for. Actual service to endpoint binding is not handled by the abstractions in this package as it is protocol specific. The general pattern is to make a "server" that wraps a service, and can then be started on an endpoint using a `Serve` method, similar to `http.Server`. To support exposing multiple compatible services on a single endpoint (example: DoH + metrics on a single port), services can implement `Merger`.
|
I still need to write tests for the new package, and the various new "services", but besides that it's in a reviewable state :) |
|
Skimmed over your PR and it looks very good! 👍 |
kwitsch
approved these changes
Apr 12, 2024
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.
Progress for #1206.
No user visible changes in this PR: it's just refactoring and the new config cannot be set from the YAML file.
Instead the new config structs are filled with the info from
ports:until we're ready to deprecateportsfully.The first part of the commits refactor how we setup the HTTP listeners.
The second part adds a new
servicepackage with interfaces used to split upserver.Server.The commits after adding that package refactor the DoH, metrics, and API to be services.
Here's the docs that package as an overview of the design:
This doesn't touch DNS listener/server code (besides moving the TLS config to a single spot), but I think we'll be able to re-use the same code to setup DNS listeners, and split the DNS server logic into a bunch of services + one server.
The follow-up work is to continue removing things from
configureHTTPRouteruntil it's empty and we can remove the transitionalhttpMiscService.At that point we'll be ready to deprecate
portsand switch to the newservicesconfig format under discussion in #1206.