kube-runtime pulls in two versions of pin-project

[olix0r]

Current and expected behavior

kube-runtime pulls in two distinct versions of pin-project & pin-project-lite:

:; cargo tree -d -i kube-runtime
pin-project v0.4.28
└── snafu v0.6.10
    └── kube-runtime v0.63.2

pin-project v1.0.8
├── kube-client v0.63.2
│   └── kube-runtime v0.63.2
├── kube-runtime v0.63.2
├── tower v0.4.10
│   └── kube-client v0.63.2 (*)
└── tower-http v0.1.2
    └── kube-client v0.63.2 (*)

pin-project-internal v0.4.28 (proc-macro)
└── pin-project v0.4.28 (*)

pin-project-internal v1.0.8 (proc-macro)
└── pin-project v1.0.8 (*)

When using tools like cargo-deny to audit dependencies, this requires an exemption like:

[bans]
multiple-versions = "deny"
# Wildcard dependencies are used for all workspace-local crates.
wildcards = "allow"
highlight = "all"
deny = []
skip-tree = []
skip = [
    # kube-runtime pulls in snafu v0.6, which pulls in an older version of pin-project
    { name = "pin-project", version = "0.4.28" },
    { name = "pin-project-internal", version = "0.4.28" },
]

I imagine this bloats compile times for packages that use kube-runtime, though I don't have hard numbers to back up this assumption.

Possible solution

I'd love it if we could drop the snafu dependency entirely, just to avoid the dependency. I know it offers some conveniences, but these seem surmountable, especially if we use thiserror for error definitions. thiserror is already pulled in by kube-client and kube-core.

Otherwise, it looks like there's a snafu v0.7 release in the works that hopefully resolves this?

Additionally, it may be valuable to use something like cargo-deny-action to audit duplicate/mismatched dependencies, license compliance, etc.

Additional context

No response

Environment

n/a

Configuration and features

:; cargo tree -p kube --depth=1
kube v0.63.2
├── k8s-openapi v0.13.1
├── kube-client v0.63.2
├── kube-core v0.63.2
├── kube-derive v0.63.2 (proc-macro)
└── kube-runtime v0.63.2

Affected crates

kube-runtime

Would you like to work on fixing this bug?

maybe

[kazk]

snafu was replaced by thiserror (#686), so this should be fixed in the next release.

[kazk]

Additionally, it may be valuable to use something like cargo-deny-action to audit duplicate/mismatched dependencies, license compliance, etc.

We already have cargo-deny-action.

https://github.com/kube-rs/kube-rs/blob/519cc7b6332b059cf0c487d81f6d32e5579272a7/.github/workflows/ci.yml#L126

So we can configure to deny duplicate dependencies. PR to do that is welcomed.

[clux]

snafu was replaced by thiserror (#686), so this should be fixed in the next release.

Do you think we are at a reasonable point to cut a new release at this time? We have a quite sizable CHANGELOG, but there are a couple of outstanding things that are possibly very close (hyper-rustls), and possibly something we might want to not split too much across releases (like error refinement).

duplicate dependencies check

I can play around with that.
EDIT: Done. in #711 - shouldn't get this type of issue again.

[nightkr]

I'd say the error stuff is relatively fine to split up. Especially if we ensure that any specific error types that we end up exposing to the public API are Into<kube::Error> (so that users don't need to opt into the more specific types if they don't want them).

[kazk]

Yeah, I've been trying to refine errors gradually per module (starting from lower level ones), so splitting across releases shouldn't be an issue. Any future refinements shouldn't be related to them.

All the higher level modules users use still returns kube::Error. Many variants were removed, and some were renamed/added. Because of how kube::Error has been, I think most users have catchall for the errors that have been changed.

The changelog is overwhelming, so we can add an overview to make the change easier to understand.

[kazk]

I think hyper-rustls will be released soon. They merged the PR we were waiting for, and opened a PR to release (rustls/hyper-rustls#159).

[kazk]

Summary of changes to kube::Error:

• Removed impl From<T> for Error
• Removed Error::Connection (unused)
• Removed Error::RequestBuild (unused)
• Removed Error::RequestSend (unused)
• Removed Error::RequestParse (unused)
• Removed Error::InvalidUri (replaced by variants of errors in kube::config)
• Removed Error::RequestValidation (replaced by variant of Error::BuildRequest)
• Removed Error::Kubeconfig (replaced by Error::InferConfig, and Error::Auth)
• Removed Error::ProtocolSwitch (ws only, replaced by Error::UpgradeConnection)
• Removed Error::MissingUpgradeWebSocketHeader (ws only, replaced by Error::UpgradeConnection)
• Removed Error::MissingConnectionUpgradeHeader (ws only, replaced by Error::UpgradeConnection)
• Removed Error::SecWebSocketAcceptKeyMismatch (ws only, replaced by Error::UpgradeConnection)
• Removed Error::SecWebSocketProtocolMismatch (ws only, replaced by Error::UpgradeConnection)
• Added Error::Auth (errors related to client auth, some of them were previously under Error::Kubeconfig, confusingly)
• Added Error::BuildRequest (errors building request from kube::core)
• Added Error::InferConfig (for Client::try_default)
• Added Error::OpensslTls (new openssl-tls feature)
• Added Error::RustlsTls (rustls-tls feature)
• Added Error::UpgradeConnection (ws feature, upgrade errors)

[clux]

Fixed in 0.64.0.

[olix0r]

@clux @kazk thanks!