search

metio / kube-custom-resources-rs

Kubernetes Custom Resource Bindings for Rust
https://crates.io/crates/kube-custom-resources-rs
BSD Zero Clause License
24 stars 5 forks source link

Possible dependency version issue #289

Open w3irdrobot opened 5 months ago

w3irdrobot commented 5 months ago

I pulled in this library to use the VolumeSnapshot CRD. When trying to use it with an Api, I got compiler errors saying it didn't implement the required Metadata trait. I eventually took a guess that it could be an issue with the version of kube/k8s-openapi that I was using colliding with the version expected by this library. So, I ran kopium against the CRD YAML to have the generated file locally instead, and I no longer have the compiler error about the Metadata trait.

Relevant code snippet:

let api: Api<VolumeSnapshot> = Api::namespaced(client, &namespace);

I'm just guessing, but my understanding is that Cargo.lock files typically aren't suggested to be shipped with libraries. I noticed there is one in the root of the project for the whole workspace. I do not know, however, if cargo publish will ship a workspace's Cargo.lock with the individual crates as they are published.

Versions:

sebhoss commented 5 months ago

Yeah I think you are right & I'm sorry this is broken for you right now. The lockfile should not matter for consumers, but it certainly is an issue that I do specify the exact version for each dependency. I think changing that to a allowed version range should fix this, but I have to verify that this is actually true.

So thanks for the detailed informations - they should help me a lot while testing this!

w3irdrobot commented 4 months ago

@sebhoss no worries. I was able to keep moving forward just by generating it myself using kopium. So I'm not blocked or anything.

sebhoss commented 4 months ago

@w3irdrobot yeah sorry I was traveling and got sick and then lots of work to do :sweat:

So my understanding is that the fundamental issue here is that both kube and k8s-openapi are still using v0 and thus are allowed to introduce breaking changes whenever they feel like it. Would that not be the case, the breaking change to that Metadata API would have caused a bump in their major version and thus highlight that an update from 0.87 to 0.88 does actually cause issues..

Therefore, I do not think that I can really do anything here. I've changed the versions in #294 to use tildes to clarify that I want to support version ranges here, but this does not help in your situation in my local tests. The only solution I could find was to hard select the version of kube to = 0.87.0 which works in your situation but breaks anyone using wanting to use a newer version of kube.

You can try the latest version of this crate tomorrow which uses the tilde versions, but I'm fairly certain that it will still fail with the same build error as long as you are using an older version of kube.

waynr commented 3 months ago

I'm running into the same issue with respect to the Metadata trait not being implemented on the types here. I looked at the lockfile in this repo and see that the k8s-openapi version matches what I'm using in my project so it's not totally clear to me why that trait wouldn't be included :thinking:

waynr commented 3 months ago

I'm about to give using kopium myself a shot, but thought it might be helpful to point out that there appears to be silent failures happening in the Update CRDs CI jobs, eg https://github.com/metio/kube-custom-resources-rs/actions/runs/9905988629/job/27366663681#step:8:242

sebhoss commented 3 months ago

I'm running into the same issue with respect to the Metadata trait not being implemented on the types here. I looked at the lockfile in this repo and see that the k8s-openapi version matches what I'm using in my project so it's not totally clear to me why that trait wouldn't be included 🤔

I think this is happening because the lock file of this repo is not considered in your repo. The FAQ at https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control says: However, this determinism can give a false sense of security because Cargo.lock does not affect the consumers of your package, only Cargo.toml does that

I specified in this repo that it's compatible with every 0.x version of k8s-openapi but I think that's not strictly correct because 0.x versions are allowed to introduce breaking changes whenever they want. Image that k8s-openapi would have published a 1.0 and a 2.0 version - if my repo uses 2.0 but your repo uses 1.0 no one would expect this to work, right? However since they are still using a 0.x version they can break between 0.86 and 0.87 (or whatever the version was that broke this) and it still kinda looks like it should work.

So what I'm going to do is to remove the lock file from this repo entirely. I'm pretty sure that this won't change anything, but at least I did something? :shrug: I have really no idea other than waiting on the upstream libraries to stabilize on a major version.

sebhoss commented 3 months ago

I'm about to give using kopium myself a shot, but thought it might be helpful to point out that there appears to be silent failures happening in the Update CRDs CI jobs, eg https://github.com/metio/kube-custom-resources-rs/actions/runs/9905988629/job/27366663681#step:8:242

Yep thanks thanks ... I'm kinda busy with other stuff at the moment but I'm aware of that and will try to fix it in the next couple of days.

sebhoss commented 3 months ago

err .. so lockfile is removed and the build seems to be fixed.

I just add https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html here just for reference

waynr commented 3 months ago

Cool, thanks for your quick response! For what it's worth, I did try kopium last night and it didn't seem to include an impl of the Metadata trait so maybe it's not actually expected to work :thinking:. My main reason for wanting that trait on these types is so that they can be used with some of the generic methods I am writing that have constraints like T: Metadata<Ty = ObjectMeta>, but I just gave up on that for now and wrote a bespoke method for the CR I'm interested in.