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

Add support for rustls #126

Merged
merged 11 commits into from
Sep 15, 2023
Merged

Conversation

bswinnerton
Copy link
Contributor

@bswinnerton bswinnerton commented Sep 13, 2023

This is the final pull request in a series to add support for using rustls as the underlying TLS library for bgpkit-parser's dependencies. The two other related pull requests were:

This pull request introduces a new feature flag known as parser-rustls that enables oneio's rustls feature flag. I'm not sure this is the best option, and would love feedback on this approach. Alternatively, we can also explore adding two new feature flags: native-tls and rustls, similar to what was done in bgpkit/oneio#21, but that feels like more of a breaking change, and this is a far more popular crate and would require users to explicitly opt into both flags when they want to use parser: features = ["parser", "native-tls"] or features = ["parser", "rustls"].

Happy to make updates based on the preferred approach, and thank you for considering this PR!

Cargo.toml Outdated Show resolved Hide resolved
@bswinnerton
Copy link
Contributor Author

bswinnerton commented Sep 13, 2023

Ah, actually on second thought the parser-rustls based approach won't work without further changes:

bgpkit-parser/src/lib.rs

Lines 375 to 376 in 0554fff

#[cfg(feature = "parser")]
pub mod parser;

I'll explore a few other options.

@bswinnerton
Copy link
Contributor Author

Alright, I've switched the strategy around a bit and introduced two new feature flags: native-tls and rustls, which must be passed alongside parser.

Would love feedback on this approach as opposed to the one written in the OP.

@bswinnerton bswinnerton changed the title Add new parser-rustls feature flag Add support for rustls Sep 13, 2023
Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@digizeph digizeph left a comment

Choose a reason for hiding this comment

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

Just released oneio=0.13.1 that adds lib-native-tls and lib-rustls flags.
https://github.com/bgpkit/oneio/releases/tag/v0.13.1

This should make the dependencies cleaner.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
bswinnerton and others added 3 commits September 14, 2023 14:59
Co-authored-by: Mingwei Zhang <[email protected]>
Co-authored-by: Mingwei Zhang <[email protected]>
Co-authored-by: Mingwei Zhang <[email protected]>
@bswinnerton
Copy link
Contributor Author

Looks great! Thanks @digizeph.

@digizeph digizeph merged commit 99af99b into bgpkit:main Sep 15, 2023
1 check passed
@bswinnerton bswinnerton deleted the add-rustls-feature-flag branch September 19, 2023 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants