kube-rs / kube

Rust Kubernetes client and controller runtime
https://kube.rs
Apache License 2.0
3.06k stars 320 forks source link

Implement CEL validation proc macro for generated CRDs #1621

Closed Danil-Grigorev closed 2 weeks ago

Danil-Grigorev commented 1 month ago

Motivation

CRDs allow to declare server-side validation rules using CEL. This functionality is supported via #[schemars(schema_with = "<schemagen-wrapper>")], but requires defining a method with handling logic, which may be error-prone.

Since kube owns CRD generation code, the idea is to simplify this process for added validation rules and achieve more declarative approach, similar to the kubebuilder library. This approach will be compatible with kopium generation based on existing CRD structures, already using CEL expressions.

Solution

Allow for a more native handling of CEL validation rules on the CRDs via a field macro.

Unfortunately due to limitations of derive macros, a new #[proc_macro_attribute] is required to allow adjustments of the annotations on the fields, currently named #[cel_validation] (working prototype name),

Similar approach may also be used to allow for server-side defaults and other custom modifications of the CRD in the future, if decided.

#[cel_validation]
#[derive(…, JsonSchema)]
#[kube(
…
)]
pub struct FooSpec {
    // Field with CEL validation
    #[serde(default)]
    #[validated(rule="self != 'illegal'", message="string cannot be illegal")]
    #[validated(rule="self != 'not legal'")]
    cel_validated: Option<String>,
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 5.55556% with 68 lines in your changes missing coverage. Please review.

Project coverage is 74.6%. Comparing base (179936a) to head (4eec7d8). Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
kube-derive/src/custom_resource.rs 5.8% 66 Missing :warning:
kube-derive/src/lib.rs 0.0% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1621 +/- ## ======================================= - Coverage 75.3% 74.6% -0.6% ======================================= Files 82 82 Lines 7344 7416 +72 ======================================= + Hits 5528 5532 +4 - Misses 1816 1884 +68 ``` | [Files with missing lines](https://app.codecov.io/gh/kube-rs/kube/pull/1621?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs) | Coverage Δ | | |---|---|---| | [kube/src/lib.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1621?src=pr&el=tree&filepath=kube%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS9zcmMvbGliLnJz) | `88.5% <ø> (ø)` | | | [kube-derive/src/lib.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1621?src=pr&el=tree&filepath=kube-derive%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1kZXJpdmUvc3JjL2xpYi5ycw==) | `0.0% <0.0%> (ø)` | | | [kube-derive/src/custom\_resource.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1621?src=pr&el=tree&filepath=kube-derive%2Fsrc%2Fcustom_resource.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1kZXJpdmUvc3JjL2N1c3RvbV9yZXNvdXJjZS5ycw==) | `60.1% <5.8%> (-21.3%)` | :arrow_down: |
clux commented 1 month ago

Hey this is great! Thanks a lot for doing this.

I was wondering if you could elaborate a bit on a couple of things:

Unfortunately due to limitations of derive macros, a new #[proc_macro_attribute] is required to allow adjustments of the annotations on the fields, currently named #[cel_validation] (working prototype name),

is this to ensure precedence of annotations?

is there possibly something we could do to simplify these requirements by perhaps referencing a common rewrite rule in kube-core (like we do in schema.rs for structural rewriting)?

Danil-Grigorev commented 1 month ago

is this to ensure precedence of annotations?

yes, I was trying to replace #[validated] with schemars within the CustomResource derive, but it didn't work (even though there is an implicit derive order and CustomResource comes before JsonSchema). So the macro performs replacement in the first place.

If the keyword comes after derive, no macro will be able to process replaced #[schemars] annotations.

Danil-Grigorev commented 2 weeks ago

I'll close it for now to find more suitable approach.