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

build: allow to use rustls instead of native-tls #288

Merged
merged 3 commits into from
Aug 2, 2023

Conversation

FlorentinDUBOIS
Copy link
Collaborator

  • This is used in an effort to remove all dependencies to openssl.
    Which could be interesting in embedded system or on environment
    which is difficult to know on which openssl version the software
    will run it and breaks deployments.
  • It introduces two compiler feature flags which are tokio-rustls-runtime
    and async-std-rustls-runtime that have the same meaning as
    tokio-runtime and async-std-runtime except that they use rustls.
  • There is a safe guard, if we enable both runtimes, this is the
    native-tls ones that are used to keep consistent with the current
    behaviour.

* Bump bytes to 1.4.0
* Bump crc to 3.0.1
* Bump nom to 7.1.3
* Bump prost to 0.11.9
* Bump prost-derive to 0.11.9
* Bump chrono to 0.4.26
* Bump log to 0.4.19
* Bump url to 2.4.0
* Bump regex to 1.9.1
* Bump futures to 0.3.28
* Bump futures-io to 0.3.28
* Bump native-tls to 0.2.11
* Bump pem to 3.0.0
* Bump tokio to 1.29.1
* Bump tokio-util to 0.7.8
* Bump tokio-native-tls to 0.3.1
* Bump asynchronous-codec to 0.6.2
* Bump async-native-tls to 0.5.0
* Bump flate2 to 1.0.26
* Bump zstd to 0.12.4
* Bump snap to 1.1.0
* Bump openidconnect to 3.3.0
* Bump oauth2 to 4.4.1
* Bump serde to 1.0.175
* Bump serde_json to 1.0.103
* Bump async-trait to 0.1.72
* Bump data-url to 0.3.0
* Bump uuid to 1.4.1

Signed-off-by: Florentin Dubois <[email protected]>
@FlorentinDUBOIS
Copy link
Collaborator Author

Hello @tisonkun 👋, long time no see !
I made some changes to allow to use rustls instead of native-tls using compiler feature flags.
Could you give me your thought of this pull request ?

To give you more context about this pull request, at Clever Cloud, we are doing cross-compiling stuffs and some times build statically binaries and it will helps us, if we could remove the dependency to openssl to do so.

@FlorentinDUBOIS
Copy link
Collaborator Author

By the way, I have updated all dependencies !

@FlorentinDUBOIS FlorentinDUBOIS force-pushed the devel/fdubois/build/rustls-runtime branch from ba163f9 to f9f5e82 Compare July 26, 2023 12:18
@FlorentinDUBOIS FlorentinDUBOIS self-assigned this Jul 26, 2023
@FlorentinDUBOIS FlorentinDUBOIS force-pushed the devel/fdubois/build/rustls-runtime branch 2 times, most recently from dd0ab59 to b2c658b Compare July 26, 2023 12:35
@FlorentinDUBOIS
Copy link
Collaborator Author

Hello @tisonkun and @CleverAkanoa, I have deployed this patch in production and it works pretty well.

Copy link
Collaborator

@CleverAkanoa CleverAkanoa left a comment

Choose a reason for hiding this comment

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

Looks good :)

@tisonkun
Copy link
Contributor

Thank you! I'll review this patch in days.

src/error.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @FlorentinDUBOIS!

The direction looks good to me. One minor comment inline.

Also, I'm thinking of introduce a compile error guard to assert users not enable rustls and native-tls at the same time. This can simplify the cfg guard and give users a clear expectation. Existing configs won't be affected also.

See https://doc.rust-lang.org/std/macro.compile_error.html for reference.

But I can accept if we do this refactor later after this PR merged and before the next release out.

@FlorentinDUBOIS
Copy link
Collaborator Author

I am glad that you are ok with those modification, I will do the modification above about the hyphen and I will add the compile time error as well.
By the way, once this will be merged I will rebase the pull request #267 to integrate those changes.

@FlorentinDUBOIS FlorentinDUBOIS force-pushed the devel/fdubois/build/rustls-runtime branch 3 times, most recently from 0cf2fdc to 6734d6c Compare July 31, 2023 13:02
@FlorentinDUBOIS
Copy link
Collaborator Author

Hey @tisonkun, I did the modification, could you check that everything is ok for you ?

* This is used in an effort to remove all dependencies to openssl.
  Which could be interesting in embedded system or on environment
  which is difficult to know on which openssl version the software
  will run it and breaks deployments.
* It introduces two compiler feature flags which are `tokio-rustls-runtime`
  and `async-std-rustls-runtime` that have the same meaning as
  `tokio-runtime` and `async-std-runtime` except that they use rustls.
* There is a safe guard, if we enable both runtimes, this is the
  native-tls ones that are used to keep consistent with the current
  behaviour.

Signed-off-by: Florentin Dubois <[email protected]>
@FlorentinDUBOIS FlorentinDUBOIS force-pushed the devel/fdubois/build/rustls-runtime branch from 6734d6c to ef3ca92 Compare July 31, 2023 17:29
Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Thank you!

@FlorentinDUBOIS
Copy link
Collaborator Author

As @CleverAkanoa and @tisonkun approve this pull request, I merge it.

@FlorentinDUBOIS FlorentinDUBOIS merged commit 597e57b into master Aug 2, 2023
@delete-merged-branch delete-merged-branch bot deleted the devel/fdubois/build/rustls-runtime branch August 2, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants