`rustls/ring` is always active even with `aws-lc-rs` feature

[dsykes16]

The buildkit feature of bollard includes the ssl feature, which includes rustls/ring. This results in ring being brought into dependencies even if the aws-lc-rs feature is selected. A workaround is to call CryptoProvider::install_default(), but that is less than ideal since ring shouldn't be brought into the dependencies at all if aws-lc-rs is chosen.

bollard #docker
An asynchronous Docker daemon API
version: 0.19.1 (latest 0.19.2)
license: Apache-2.0
rust-version: unknown
documentation: https://docs.rs/bollard
homepage: https://github.com/fussybeaver/bollard
repository: https://github.com/fussybeaver/bollard
crates.io: https://crates.io/crates/bollard/0.19.1
features:
  default                = [http, pipe]
...
  aws-lc-rs              = [ssl_providerless, rustls/aws-lc-rs]
  bollard-buildkit-proto = [dep:bollard-buildkit-proto]
  buildkit               = [chrono, num, rand, tokio/fs, tokio-stream, tokio-util/io, tonic, tower-service, ssl, bollard-stubs/buildkit, bollard-buildkit-proto, dep:async-stream, dep:bitflags]
...
  ssh                    = [hyper-util, openssh, tower-service]
  ssl                    = [ssl_providerless, rustls/ring]
...

[DDtKey]

Hi @dsykes16 👋

Thanks for the report!

Looks like this regression introduced in #801 (cc @mbodmer) as previously we have added ability to disable default-features and tls at all #799 (comment)

That's unfortunate, we should have released builder api under a feature and make buildkit enabled only if builder feature enabled..

Since it's been already released, I think we need to make a builder-api feature enabled by default, so if default-features are disabled - ring won't be required. ring enabled anyway with default-features, so this sounds reasonable to me - disable default ones and enable aws-lc-rs is preferred - the main issue is that there won't be a way to enable aws-lc-rs + builder api without ring and that's probably a question to bollard)

However it's not always guaranteed

but that is less than ideal since ring shouldn't be brought into the dependencies at all if aws-lc-rs is chosen.

That's unfortunately not really true even if we fix buildkit feature requirement, cargo features are additive. So any another dependency can enable ring or aws-lc-rs in your dependency tree

[dsykes16]

Thanks. I dug into this and it looks like the buildkit features are pretty integrated in tests (e.g. in therunners::async_runner module). I'll look at opening a PR to resolve this upstream in Bollard.

[DDtKey]

For tests it's possible to have self-dependency in dev-dependencies with the feature enabled (like testcontainers = { path = ".", features = ["buildkit"]) - this won't affect users of testcontainers and would allow to disable default features & any tls libs

But it's better if bollard would have ability to use buildkit with aws-lc-rs only ofc

[mbodmer]

Apology for this issue. I have just also stumbled on it today in my code. However I obviously need the builder API but still wouldn't want the ring dependency pulled in by bollard. It seems like a PR in bollard would be most helpful. A build feature for the builder API however, would benefit all who don't need it.

[mbodmer]

fussybeaver/bollard#590

[dsykes16]

Thanks, @mbodmer!

@DDtKey, I put up a draft PR #845 that breaks out the buildkit feature (included by default to keep it backwards-compatible), but honestly @mbodmer's PR to bollard resolves my issue, so I'll defer to you and the other maintainers as to whether or not you want to breakout the buildkit feature or not.

[DDtKey]

Thanks for the efforts folks!

[mbodmer]

Thanks for your acknowledgements @dsykes16 and @DDtKey.

Obviously we will need to change the bollard buildkit feature to buildkit_providerless, once the PR will be merged and we have a new release of bollard.

Also thanks for #845