feat: add feature to use rustls instead of relying on openssl#3870
feat: add feature to use rustls instead of relying on openssl#3870ashort96 wants to merge 1 commit intoAzure:mainfrom
Conversation
bf124bb to
ab7b9db
Compare
|
@microsoft-github-policy-service agree company="Gemini" |
|
Thank you for your contribution @ashort96! We will review the pull request and get back to you soon. |
ab7b9db to
910563c
Compare
There was a problem hiding this comment.
Pull request overview
Adds optional Rustls-based TLS support for the reqwest transport in core crates, enabling OpenSSL-free builds when consumers opt in via feature flags.
Changes:
- Add
reqwest_rustlsfeature totypespec_client_coreandazure_coreto enable reqwest’s rustls TLS mode. - Update docs.rs feature lists to include
reqwest_rustlsfor both crates. - Allow
CDLA-Permissive-2.0indeny.tomlto accommodate transitive licensing from rustls-related crates.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/core/typespec_client_core/Cargo.toml | Introduces reqwest_rustls feature mapping to a reqwest rustls feature. |
| sdk/core/azure_core/Cargo.toml | Re-exports reqwest_rustls feature through azure_core. |
| deny.toml | Adds CDLA-Permissive-2.0 to the allowed license list. |
| Cargo.lock | Captures new transitive dependencies introduced by enabling rustls-related reqwest features. |
| reqwest_deflate = ["reqwest", "reqwest/deflate"] | ||
| reqwest_gzip = ["reqwest", "reqwest/gzip"] | ||
| reqwest_native_tls = ["reqwest", "reqwest/native-tls"] | ||
| reqwest_rustls = ["reqwest", "reqwest/rustls-no-provider"] |
There was a problem hiding this comment.
reqwest_rustls is wired to reqwest/rustls-no-provider. Given the intent is an “it just works” OpenSSL-free TLS option, this feature choice is surprising because it implies reqwest/rustls is being enabled without an associated provider. If this is intentional, please add crate-level documentation for what downstream users must do to make TLS work; otherwise, consider switching to the reqwest rustls feature that includes a provider by default (so enabling reqwest_rustls is sufficient on its own).
| reqwest_rustls = ["reqwest", "reqwest/rustls-no-provider"] | |
| reqwest_rustls = ["reqwest", "reqwest/rustls-tls-native-roots"] |
| @@ -71,6 +71,7 @@ reqwest = ["typespec_client_core/reqwest"] | |||
| reqwest_deflate = ["reqwest", "typespec_client_core/reqwest_deflate"] | |||
| reqwest_gzip = ["reqwest", "typespec_client_core/reqwest_gzip"] | |||
There was a problem hiding this comment.
Because azure_core’s default features still include reqwest_native_tls, users who add features = ["reqwest_rustls"] without also disabling default features will end up enabling both TLS backends and will still pull in native-tls/OpenSSL on some platforms. Consider documenting the intended dependency pattern (default-features = false + explicitly selecting one TLS backend), or adding a guard (e.g., a compile-time error or a dedicated reqwest_default_tls_backend feature) to reduce accidental misconfiguration.
| reqwest_gzip = ["reqwest", "typespec_client_core/reqwest_gzip"] | |
| reqwest_gzip = ["reqwest", "typespec_client_core/reqwest_gzip"] | |
| # TLS backend selection for reqwest: | |
| # - By default, this crate enables `reqwest_native_tls` via the `default` feature set above. | |
| # - To use `reqwest_rustls` instead, depend on `azure_core` with `default-features = false` | |
| # and then enable exactly one of `reqwest_native_tls` or `reqwest_rustls`, e.g.: | |
| # azure_core = { version = "0.33", default-features = false, features = ["reqwest", "reqwest_rustls"] } | |
| # - Enabling both `reqwest_native_tls` and `reqwest_rustls` at the same time will pull in | |
| # both TLS backends (including native-tls/OpenSSL on some platforms). |
|
Thank you for your contribution, but we're not adding parity features. Our features are for the default's we have chosen based on various factors and allow you to disable them if, for example, you don't want to use openssl. If you want to use https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/core/azure_core/README.md#reqwest |
|
The resolver doesn't help if you have a big project and use cargo hakari. The request with the default features will always be built, including all the OpenSSL crates, regardless of any hacks. If this was so common, every single crate would have relied on just the resolver instead of providing the corresponding features. |
|
If you take a dependency on We also specifically cannot author a dependency on See https://github.com/heaths/azsdk-rust-2750/blob/main/reqwest-rustls-webpki/Cargo.toml for an example. No "openssl" anywhere in the |
This is true only if the user fully controls the project and the dependencies, and no one doesn't control all the dependencies. Otherwise, one would have to fork every single crate depending on reqwest and make sure it doesn't get built with the default features. So, in my case, when I use also cargo hakari, I get reqwest with OpenSSL and there is no possible way to change it. I have about 800 dependencies in my workspace_hack crate, and many of my dependencies either directly or transitively depend on reqwest. I have removed all of those depending on the OpenSSL part or forked and added the rustls feature. One way or another, OpenSSL is always there in my dependency tree. That's a shame you guys can't add the rustls backend as a feature. Thank you for your help. I'll check once again what you suggested but I think I have tried it before. |
|
@iddm even if we did add a If you want, feel free to submit a PR to modify or add a new scenario in https://github.com/heaths/azsdk-rust-2750. A demonstration of how this might work and solve the scalability problem you mention would be helpful. |
resolves #3827
Adds
reqwest_rustlsfeature toazure_coreandtypespec_client_core, allowing users to opt into a pure-Rust TLS stack (viarustls) without requiring OpenSSL. The existingreqwest_native_tlsdefault is unchanged, making this a purely additive change.CDLA-Permissive-2.0 is Mozilla's data license for their root certificate bundle (
webpki-root-certs), pulled in transitively byrustls-platform-verifierwhenreqwest_rustlsis enabled. This license was previously only reachable throughureqin dev dependencies (excluded by deny.toml's exclude-dev = true), so it never surfaced. It's a permissive data-only license with no meaningful restrictions on use.