Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow TlsListener::from() to specify a resolver. #2841

Closed
wants to merge 1 commit into from

Conversation

hcldan
Copy link

@hcldan hcldan commented Aug 6, 2024

No description provided.

@hcldan hcldan force-pushed the dyn branch 3 times, most recently from a559c50 to 3138284 Compare August 7, 2024 13:24
@the10thWiz the10thWiz added the declined A declined request or suggestion label Aug 8, 2024
@the10thWiz
Copy link
Collaborator

This specific change doesn't really make much sense, for a number of reasons. First, we would prefer to add support for creating a TlsConfig with a DynResolver directly (which would allow us to avoid using making the internal type public). Second, we are also interested in refactoring this code to support resolver-only configurations.

@hcldan
Copy link
Author

hcldan commented Aug 8, 2024

@the10thWiz I understand the concerns and I agree where you want to go is better.

I'm just trying to use what's there today. As you can see with how I modified the test, while the initial TlsConfig ends up working for the test, it's currently impossible for the test to use the DynResolver.

Using a fairing for this seems like kind of a strange dance, and I would prefer to construct a TlsConfig directly with a resolver, but this is where we currently are. I might be able to hide this more and have it done by rocket directly when you hand it the TlsListener, but I don't know all the code paths here and I don't want to break anything that already works.

I would like to get support for this in the product we are looking to ship. It looks like I'm going to have to deal with a breaking change, regardless, moving forward... I'd appreciate some cooperation in getting something that works for us in the interim, even if it "doesn't really make sense".

@hcldan
Copy link
Author

hcldan commented Aug 8, 2024

I've implemented my own TlsListener for my needs. This isn't necessary.

@hcldan hcldan closed this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
declined A declined request or suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants