s2n-quic 1.35 is semver breaking due to rustls export

[SergioBenitez]

s2n-quic v1.35 reexports s2n-quic-rustls v0.35.0 which re-exports rustls v0.23. Previously s2n-quic v1.34 reexported s2n-quic-rustls v0.34.0 which reexported rustls v0.21. Since rustls 0.21 to 0.23 is breaking, then so must s2n-quic v1.34 to v1.35 be. As such, a major version bump is required. Minimally, s2n-quic v1.35 should be yanked as soon as possible. (This was detected due to a failing CI job.)

For v2, I would suggest removing the re-export entirely and asking the user to depend on s2n-quic-rustls directly to avoid such changes from being major breaking changes in the future.

[WesleyRosenblum]

Thank you Sergio for opening this issue and highlighting the consequences of breaking changes in the rustls API. We take backwards compatibility and stability seriously, and it’s unfortunate that this has caused your integration with s2n-quic to break.

Given the ongoing instability of the rustls API, it is not feasible for s2n-quic to have a major version bump for each rustls release. And depending on s2n-quic-rustls directly is not recommended, as that crate depends on other internal s2n-quic crates that do not have a stable interface and thus must be upgraded in unison. We will be adding documentation to our re-export of rustls that will highlight the instability of the re-exported API and the risk in using it.

Our suggestion in the near term is to pin s2n-quic to =1.34.0, and prioritize upgrading to rustls v0.23. I believe you were already planning to unify on a single rustls version (see #2055 (comment)), so hopefully this work was already planned/in progress.

Beyond that, the best way to have a stable interface to integrate with is to use the TLS provider builders (client and server) that wrap the rustls implementation, rather than the re-exported rustls. If there are configuration options you need that are not provided in these builders, please open an issue and we will be happy to consider adding the functionality.

[SergioBenitez]

@WesleyRosenblum I want to be clear that this issue isn't on your dependent's side, even if it can be mitigated on that side, but rather with s2n-quic itself. Stated clearly: the release of s2n-quic v1.35 violates semantic version because it includes breaking changes, which are not allowed in a v1 point release. It must be yanked if you wish to adhere to Rust's and Cargo's semantic versioning policies, which are expected (really, required) by the entire ecosystem.

Your suggestions seem to pass the blame to your dependents, and if not blame then at least the onus. This feels contrary to your earlier statement of taking backwards compatibility and stability seriously.

Given the ongoing instability of the rustls API, it is not feasible for s2n-quic to have a major version bump for each rustls release.

This is not a requirement, but you have made it so by reexporting rustls directly. Any time you reexport a crate, then that crate becomes part of your API. As I mentioned earlier, I would suggest that you not reexport rustls or any other crate that you're not okay with making part of your API and handling breaking changes for.

And depending on s2n-quic-rustls directly is not recommended, as that crate depends on other internal s2n-quic crates that do not have a stable interface and thus must be upgraded in unison.

I would suggest making it possible to depend on s2n-quic-rustls directly, or if not that crate then some other crate that provides the integration and that you're willing to make major releases of. Otherwise, it seems the only option you're giving users is to always pin a version of your library. This is especially bad for libraries like Rocket: if Rocket pins a version (as you suggest), then no downstream consumer of Rocket will ever get an updated version of your library unless we manually update it and push a release. In effect, this completely breaks cargo update.

We will be adding documentation to our re-export of rustls that will highlight the instability of the re-exported API and the risk in using it.

This is not sufficient - your crate will still be breaking when it is not allowed by semver or Cargo. Why reexport it at all if it cannot and should not be used?

If you really want to reexport unstable APIs, then the only correct way to do so that I'm aware of is via opt-in cfgs. See, for instance, the way that proc-macro2 does it.

[WesleyRosenblum]

Thank you for clarifying your perspective.

You're correct that re-exporting the rustls crate causes unversioned breaking changes in our interface and compromises our goal of stable backwards compatibility. In hindsight it was a mistake to have the re-export. Our plan is to

• yank version 1.35
• revert the rustls upgrade
• mark the re-exported module as deprecated in our next release

Moving forward, the stable "wrapper" API we provide will be the only way to configure the underlying rustls endpoint and we will likely stop exporting rustls in the near future. We recommend the Rocket integration switch to this interface to ensure it's not impacted by future rustls upgrades. If it would be helpful, we can open a PR to do that work for you.

[WesleyRosenblum]

We've yanked 1.35.0, and published 1.35.1 which reverts the rustls upgrade and marks the re-exported module as deprecated.

I've opened rwf2/Rocket#2768 to perform the migration off of the re-exported rustls