Skip to content

Add feature to use reqwest with rustls/webpki_roots #2811

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add feature to use reqwest with rustls/webpki_roots #2811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a734e93
Add feature to use reqwest with rustls/webpki-roots
everest-boehm Jul 23, 2025
44e241e
Fix missing feature in azure_identity
everest-boehm Jul 23, 2025
bd9efc9
Do not auto-enable native-tls accidentally
everest-boehm Jul 24, 2025
880fcad
Fix test
everest-boehm Jul 24, 2025
75fc9b2
Merge remote-tracking branch 'upstream/main' into lb/reqwest_rustls_w…
everest-boehm Jul 26, 2025
ff5b1d8
More test fixes
everest-boehm Jul 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ path = "sdk/typespec/typespec_macros"
[workspace.dependencies.azure_core]
version = "0.27.0"
path = "sdk/core/azure_core"
default-features = false

[workspace.dependencies.azure_core_macros]
version = "0.1.0"
Expand Down
12 changes: 9 additions & 3 deletions sdk/core/azure_core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ sha2 = { workspace = true, optional = true }
tokio = { workspace = true, optional = true }
tracing.workspace = true
typespec = { workspace = true, features = ["http", "json"] }
typespec_client_core = { workspace = true, features = [
typespec_client_core = { workspace = true, default-features = false, features = [
"derive",
"http",
"json",
Expand Down Expand Up @@ -58,10 +58,16 @@ azurite_workaround = []
debug = ["typespec_client_core/debug"]
hmac_openssl = ["dep:openssl"]
hmac_rust = ["dep:sha2", "dep:hmac"]
reqwest = ["typespec_client_core/reqwest"]
reqwest = ["typespec_client_core/reqwest_native_tls"]
reqwest_deflate = ["typespec_client_core/reqwest_deflate"]
reqwest_gzip = ["typespec_client_core/reqwest_gzip"]
reqwest_rustls = ["typespec_client_core/reqwest_rustls"]
reqwest_rustls = ["reqwest_rustls_native_roots"]
reqwest_rustls_native_roots = [
"typespec_client_core/reqwest_rustls_native_roots",
]
reqwest_rustls_webpki_roots = [
"typespec_client_core/reqwest_rustls_webpki_roots",
]
test = ["typespec_client_core/test"]
tokio = ["dep:tokio", "typespec_client_core/tokio"]
xml = ["typespec_client_core/xml"]
Expand Down
3 changes: 2 additions & 1 deletion sdk/core/azure_core/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ We guarantee that all client instance methods are thread-safe and independent of
- `reqwest` (default): enables and sets `reqwest` as the default `HttpClient`. Enables `reqwest`'s `native-tls` feature.
- `reqwest_deflate` (default): enables deflate compression for `reqwest`.
- `reqwest_gzip` (default): enables gzip compression for `reqwest`.
- `reqwest_rustls`: enables `reqwest`'s `rustls-tls-native-roots-no-provider` feature,
- `reqwest_rustls_native_roots`: enables `reqwest`'s `rustls-tls-native-roots-no-provider` feature,
- `reqwest_rustls_webpki_roots`: enables `reqwest`'s `rustls-tls-webpki-roots-no-provider` feature,
- `tokio`: enables and sets `tokio` as the default async runtime.
- `xml`: enables XML support.

Expand Down
8 changes: 6 additions & 2 deletions sdk/identity/azure_identity/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ serde.workspace = true
time.workspace = true
tokio = { workspace = true, optional = true }
tracing.workspace = true
typespec_client_core = { workspace = true, features = ["derive"] }
typespec_client_core = { workspace = true, default-features = false, features = [
"derive",
] }
url.workspace = true

[dev-dependencies]
Expand All @@ -39,7 +41,9 @@ tracing-subscriber.workspace = true
[features]
default = ["reqwest"]
reqwest = ["azure_core/reqwest"]
reqwest_rustls = ["azure_core/reqwest_rustls"]
reqwest_rustls = ["reqwest_rustls_native_roots"]
reqwest_rustls_native_roots = ["azure_core/reqwest_rustls_native_roots"]
reqwest_rustls_webpki_roots = ["azure_core/reqwest_rustls_webpki_roots"]
tokio = ["dep:tokio", "azure_core/tokio", "tokio/process"]
client_certificate = ["openssl"]

Expand Down
4 changes: 3 additions & 1 deletion sdk/keyvault/azure_security_keyvault_certificates/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ categories = ["api-bindings"]

[dependencies]
async-trait = { workspace = true }
azure_core = { workspace = true }
azure_core = { workspace = true, features = [
"default",
] }
futures = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
Expand Down
4 changes: 3 additions & 1 deletion sdk/keyvault/azure_security_keyvault_keys/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ categories = ["api-bindings"]

[dependencies]
async-trait = { workspace = true }
azure_core = { workspace = true }
azure_core = { workspace = true, features = [
"default",
] }
futures = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
Expand Down
4 changes: 3 additions & 1 deletion sdk/keyvault/azure_security_keyvault_secrets/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ categories = ["api-bindings"]

[dependencies]
async-trait = { workspace = true }
azure_core = { workspace = true }
azure_core = { workspace = true, features = [
"default",
] }
futures = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/azure_storage_blob/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ categories = ["api-bindings"]

[dependencies]
async-trait.workspace = true
azure_core = { workspace = true, features = ["xml"] }
azure_core = { workspace = true, features = ["default", "xml"] }
serde.workspace = true
typespec_client_core = { workspace = true, features = ["derive"] }
url.workspace = true
Expand Down
2 changes: 2 additions & 0 deletions sdk/typespec/typespec_client_core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Breaking Changes

- Renamed the `reqwest` feature into `reqwest_native_tls` so that cargo would not auto-enable `reqwest/native-tls` for other TLS provider (like e.g. `reqwest_rustls`).

### Bugs Fixed

### Other Changes
Expand Down
16 changes: 12 additions & 4 deletions sdk/typespec/typespec_client_core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ futures.workspace = true
pin-project.workspace = true
quick-xml = { workspace = true, optional = true }
rand.workspace = true
reqwest = { workspace = true, optional = true }
reqwest = { workspace = true, default-features = false, optional = true }
rust_decimal = { workspace = true, optional = true }
serde.workspace = true
serde_json.workspace = true
Expand All @@ -45,17 +45,25 @@ tracing-subscriber.workspace = true
typespec_macros.path = "../typespec_macros"

[features]
default = ["http", "json", "reqwest", "reqwest_deflate", "reqwest_gzip"]
default = [
"http",
"json",
"reqwest_native_tls",
"reqwest_deflate",
"reqwest_gzip",
]
debug = ["typespec_macros?/debug"]
derive = ["dep:typespec_macros"]
http = ["typespec/http"]
json = ["typespec/json"]
reqwest = ["reqwest/native-tls"]
reqwest_native_tls = ["reqwest/native-tls"]
Copy link
Member

Choose a reason for hiding this comment

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

Similar to #2796, we cannot pull in a dependency on ring which I think this would do. Unfortunately, that's a hard blocker.

Both of these PRs make me wonder if we just need to rethink our dependency on reqwest; rather, how we depend on it. Considering that resolver version >= 2 take a union of features in the dependency tree, why should we even get into the business of which reqwest feature we take? Obviously we need to optionally depend on gzip and deflate for code we have, but the actual TLS features we don't have a hard dependency on. Maybe we should just avoid any such dependencies entirely; unless, of course, that pulls in ring by default. But considering that resolve v2+ doesn't support mutually exclusive features, we could still take a dependency on native-tls-native-roots-no-provider and people can depend on whatever other features they want in reqwest, right? Would that actually work?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for having a look at this :)

I am not sure if you meant to comment on this line specifically or whether you commented on the goal of the PR in general. This line here is only renaming a feature of the typespec_client_core crate. There should not be any change in the pulled dependencies because of it. I think you could also check the Cargo.lock file's diff as (IIUIC) it should always include entries for all possible dependencies. For this PR, it is only extended with an entry for webpki-roots.

I changed this feature's name because of a slightly weird (to me at least) cargo behavior: Cargo auto-enables an optional dependency when a feature from our crate is depending on a feature of the dependency (crate/feature syntax). However, it also will activate any feature of our crate with the name of the dependency while doing so.

Concretely, this meant that when I want to build only with the reqwest_rustls_webpki_roots feature (which is the goal of my PR, i.e. not accidentally pulling in openssl as the native certs provider), I would need to enable reqwest/rustls-tls-webpki-roots-no-provider, hence enabling the reqwest dependency AND our reqwest feature, which was then enabling reqwest/native-tls and thus breaking what I set out to do.

Copy link
Member

Choose a reason for hiding this comment

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

I changed this feature's name because of a slightly weird (to me at least) cargo behavior: Cargo auto-enables an optional dependency when a feature from our crate is depending on a feature of the dependency (crate/feature syntax). However, it also will activate any feature of our crate with the name of the dependency while doing so.

I'm not sure I fully understand what you're saying here. You're saying you have a feature named "reqwest" in your crate as well?

What I'm specifying here in Cargo.toml is that if you want to use reqwest, we default to using the native TLS feature so that there's some TLS provided. I don't want to start getting into 1:1 parity with dependencies' feature names when a consumer can just as easily take a dependency on reqwest themselves and enable whatever other features they want, since cargo's resolver v2 and newer takes a union of features and mutually exclusive features aren't supported.

This is why I'm proposing that instead of adding more features (or even renaming), consumers can add a direct dependency to reqwest themselves and pass in their own HttpClient if they want, which we support fairly easily.

We're not going to take this PR this week as we're about to release core and the other crates, but I want to do a little usability test to see if this is feasible. Our decision to provide a default but not required HTTP stack should not also require us on behalf of consumers to start doing 1:1 parity of dependencies' features. That will be difficult to maintain and difficult for consumers to use.

Copy link
Author

Choose a reason for hiding this comment

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

I changed this feature's name because of a slightly weird (to me at least) cargo behavior: Cargo auto-enables an optional dependency when a feature from our crate is depending on a feature of the dependency (crate/feature syntax). However, it also will activate any feature of our crate with the name of the dependency while doing so.

I'm not sure I fully understand what you're saying here. You're saying you have a feature named "reqwest" in your crate as well?

With "our crate" I meant typespec_client_core. I'm only trying to point out the technical issue that when the feature is named reqwest like the dependency, any usage of reqwest/some-feature will enable the reqwest feature (i.e. not only the dependency). And then - when the reqwest feature is defined as ["reqwest/native-tls"] we pull in openssl on linux which is what I want to avoid.

I don't want to start getting into 1:1 parity with dependencies' feature names

I get this and share the feeling a 100%. I was looking a bit if I can find out how others in the rust world deal with this. This gave me the impression that maybe people solve it over composition rather than dependencies? Might be totally off, though. Actually I was questioning if this wasn't already doable and I just hadn't thought of it. But I am worried about a couple of sprinkled calls to new_http_client from sdk/typespec/typespec_client_core/src/http/clients/mod.rs (especially in impl Default for TransportOptions). But maybe this is the direction to pursue, after all - i.e. only conditionally provide default construction of anything that needs an http client if a backend is explicitly enabled via a feature (which it would be by default)

we default to using the native TLS feature so that there's some TLS provided.

I think that it could be better to express this by having reqwest/native_tls in typespec_client_core's default features rather than in the overall reqwest feature, though. That's what I was doing in this PR.

We're not going to take this PR this week as we're about to release core and the other crates

No worries. I am also fine if we do not end up proceeding with the PR at all. Installing openssl into all our images was more a nuisance rather than a blocker for us. Naively I thought it was going to be a minor thing to fix. But I am starting to realize it very much is not 😄 Overall, I am really happy that I got attention from you for this "problem" and grateful for the time you have spent to give me feedback.

Copy link
Member

Choose a reason for hiding this comment

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

tokio-rs/axum#1032 gave me the impression that maybe people solve it over composition rather than dependencies?

We do support composition - we have to, since 1P will not be using tokio for async, for example (which also rules out reqwest built on hyper) - but we still provide a default for the vast majority of consumers who either won't care, or will likely be using tokio/reqwest as the almost de facto async/HTTP stacks. Therein lies the problem of dependencies. You can "disable" reqwest and use your own stack and we shouldn't pull in anything.

I think that it could be better to express this by having reqwest/native_tls in typespec_client_core's default features rather than in the overall reqwest feature, though. That's what I was doing in this PR.

Given the behavior you noted - I had not noticed that myself but it doesn't surprise me - that indeed may be a good rename but, as noted, we won't take it for this imminent release.

Avoiding the unwanted openssl dependency is certainly justified. It's a pain to get on Windows, though I don't suspect many servers running Rust are probably running on Windows/Windows containers. Still, no reason to make acquisition and deployment harder.

Copy link
Author

Choose a reason for hiding this comment

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

We do support composition

Fair - I suppose TransportOptions can be specified in all relevant places with any HttpClient implementations users could come up with. I should try this out! Within the scope of this SDK I'm worried, however, that new_http_client makes it a bit too easy to accidentally create APIs that just assume one can always be materialized from thin air. Am I mistaken here? Apologies for the silly question, but I actually do not know: In which use cases is falling back to NoopClient desired behavior? Would it maybe be feasible to only make new_http_client (and all implementations of some impl Default using it) available if the reqwest feature is active? I.e. without any NoopClient fallback, thus forcing correct HttpClient provision?

It's a pain to get on Windows, though I don't suspect many servers running Rust are probably running on Windows/Windows containers

I found it a bit surprising that openssl would become a problem on windows - the native_tls crate's docs state

This crate uses SChannel on Windows (via the schannel crate), Secure Transport on OSX (via the security-framework crate), and OpenSSL (via the openssl crate) on all other platforms.

So I would assume on windows it should not be part of the build-time dependencies(?).

reqwest_deflate = ["reqwest/deflate"]
reqwest_gzip = ["reqwest/gzip"]
reqwest_rustls = [
reqwest_rustls = ["reqwest_rustls_native_roots"]
reqwest_rustls_native_roots = [
"reqwest/rustls-tls-native-roots-no-provider",
] # Remove dependency on banned `ring` crate; requires manually configuring crypto provider.
reqwest_rustls_webpki_roots = ["reqwest/rustls-tls-webpki-roots-no-provider"]
test = [] # Enables extra tracing including error bodies that may contain PII.
tokio = ["tokio/fs", "tokio/sync", "tokio/time", "tokio/io-util"]
xml = ["dep:quick-xml"]
Expand Down
3 changes: 2 additions & 1 deletion sdk/typespec/typespec_client_core/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ This is the runtime for [TypeSpec](https://typespec.io)-generated clients.
* `reqwest` (default): enables and sets `reqwest` as the default `HttpClient`. Enables `reqwest`'s `native-tls` feature.
* `reqwest_deflate` (default): enables deflate compression for `reqwest`.
* `reqwest_gzip` (default): enables gzip compression for `reqwest`.
* `reqwest_rustls`: enables `reqwest`'s `rustls-tls-native-roots-no-provider` feature,
* `reqwest_rustls_native_roots`: enables `reqwest`'s `rustls-tls-native-roots-no-provider` feature,
* `reqwest_rustls_webpki_roots`: enables `reqwest`'s `rustls-tls-webpki-roots-no-provider` feature,
which requires manually configuring a cryptography provider since `ring` is a banned dependency.
* `tokio`: enables and sets `tokio` as the default async runtime.
* `xml`: enables XML support.
36 changes: 30 additions & 6 deletions sdk/typespec/typespec_client_core/src/http/clients/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,30 @@

//! Built-in HTTP clients.

#[cfg(not(any(feature = "reqwest", feature = "reqwest_rustls")))]
#[cfg(not(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
)))]
mod noop;
#[cfg(any(feature = "reqwest", feature = "reqwest_rustls"))]
#[cfg(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
))]
mod reqwest;

#[cfg(not(any(feature = "reqwest", feature = "reqwest_rustls")))]
#[cfg(not(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
)))]
use self::noop::new_noop_client;
#[cfg(any(feature = "reqwest", feature = "reqwest_rustls"))]
#[cfg(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
))]
use self::reqwest::new_reqwest_client;

use crate::http::{RawResponse, Request};
Expand All @@ -20,11 +36,19 @@ use typespec::error::Result;

/// Create a new [`HttpClient`].
pub fn new_http_client() -> Arc<dyn HttpClient> {
#[cfg(any(feature = "reqwest", feature = "reqwest_rustls"))]
#[cfg(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
))]
{
new_reqwest_client()
}
#[cfg(not(any(feature = "reqwest", feature = "reqwest_rustls")))]
#[cfg(not(any(
feature = "reqwest",
feature = "reqwest_rustls_native_roots",
feature = "reqwest_rustls_webpki_roots"
)))]
{
new_noop_client()
}
Expand Down
Loading