-
Notifications
You must be signed in to change notification settings - Fork 49
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
Added support for Cargo http.cainfo
#615
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we want to add a hard dependency on the full cargo
crate. That's a pretty heavy dependency to pull in just to read the cargo config file.
The API of the cargo
crate is also quite unstable (as noted in the documentation, in fact the latest version appears to have a different API than the 0.65.0 which you're depending on here).
I'm open to us using cargo HTTP configuration options to configure our reqwest client. Ideally we'd shell out to the cargo
subcommand with cargo config get
, though that feature is unfortunately still unstable (rust-lang/cargo#9301).
For now, to support this unfortunately I think we'd need to parse the cargo config options independently, rather than relying on the cargo
crate to do so. Based on the cargo configuration documentation, the process would roughly be to try to parse TOML from each $ANCESTOR_DIR/.cargo/config
or $ANCESTOR_DIR/.cargo/config.toml
, then try the next ancestor up, followed by finally checking $CARGO_HOME/config[.toml]
. There's also a CARGO_HTTP_CAINFO
environment variable which would need to be checked.
…better-ca-support
…rgo-vet into better-ca-support
Hey there @mystor 👋 I updated this PR to use cargo-config2 instead of Cargo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied some clean-ups and audited the new dependencies. Approving with those changes.
Thanks for the patch!
This PR adds support for using a custom CA via the Cargo http.cainfo configuration.
See #612 for more info