Switch from jsonpath_lib to jsonpath-rust

[ilya-bobyr]

Motivation

jsonpath_lib is unmaintained, which is not ideal. It is also forcing everyone to enable a preserve_order feature in the serde_json dependency.

preserve_order is not following the proper feature model, where features are only adding functionality. It is actually changing how serde_json works, switching from BTreeMap to IndexMap.

Solution

Switch from jsonpath_lib to jsonpath-rust.

Change has been tested with both unit and integration tests.

---

Based on gregcusack#1

[codecov]

Codecov Report

Merging #1345 (4ab6e54) into main (dde4fc7) will decrease coverage by 0.0%.
The diff coverage is 71.5%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1345     +/-   ##
=======================================
- Coverage   72.1%   72.1%   -0.0%     
=======================================
  Files         75      75             
  Lines       6387    6390      +3     
=======================================
+ Hits        4604    4606      +2     
- Misses      1783    1784      +1     

Files	Coverage Δ
kube-client/src/client/auth/mod.rs	50.7% <71.5%> (+0.3%)	⬆️

[Dav1dde]

I did not test it / look for functional changes

[Dav1dde]

Just some small format! nits other than that LGTM 👍

Out of curiosity have you looked into https://docs.rs/serde_json_path/latest/serde_json_path/ - I have used it before and quite liked it.

[clux]

will it always work to use find_slice here? to me that sounds like we expect the query to return a vector of things, but curious if this will work if we do a query returning a single item like '{.items[3].metadata.name}'

[gregcusack]

ya so find_slice will return an empty vector if the query doesn't match and will return anything else as elements in an array, including if it results in just a single item. The item will just be returned in the vector. See: https://github.com/besok/jsonpath-rust/blob/main/src/lib.rs#L450-L460.

Test here shows find_slice() returning a single item in a vector: https://github.com/besok/jsonpath-rust/blob/main/src/lib.rs#L1194-L1198

[ilya-bobyr]

Existing logic here is a bit suspicious.
If any elements are selected by the JSONPath, it would always use the first element, even if there is more than one result.

I wonder if it would be better to fail with an error, if there are multiple results.
If someone really wants to use the first result, they can always clarify their selector expression.
On the other hand, choosing the first result could hide an error and complicate debugging.

I can send a separate PR to change this behavior.

[clux]

I guess this is ultimately only for a small amount of niche behaviour; local auth where the usage from exec based providers needs to grab one key - so it's not as exposed as something like kubectl jsonpath querying. If there's a clear way to improve it feel free to raise a PR, but I don't see this as a big problem atm.

[ilya-bobyr]

Replaced

    format!("{}{}", $, path.clone())

with

    format!("${path}")

and rebased on top of the latest main.

[ilya-bobyr]

Out of curiosity have you looked into docs.rs/serde_json_path/latest/serde_json_path - I have used it before and quite liked it.

jsonpath-rust seems to be much more popular, compared to serde_json_path, judging just by the download count.
And, similarly, GitHub stars and fork counts for jsonpath-rust are higher, compared to serde_json_path.

It seems that both libraries can work directly with serde_json::Value objects. But jsonpath-rust was developed several years earlier. Not sure what are the advantages of serde_json_path.

I have not used any Rust JSONPath libraries much. And the scope of this PR is that we have a problem with the outdated dependency, that is braking our dependency tree. It is not that I prefer one library over another :)

[clux]

Given how little usage there is of the serde_json_path variant, it is probably too fresh for us to bundle anyway. This takes care of the out-of-date problem for now. Happy to take up the discussion of one-vs-the-other again in the future once the ecosystem stabilises (they have both been releasing new versions quite recently).

Btw, this needs an MSRV bump to pass CI which is perfectly reasonable for us to do at this stage. Have done this in #1353

[Dav1dde]

I think this is the right choice, was just (personally) curious if you explored more alternatives 👍 .