Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement CEL validation proc macro for generated CRDs #1621

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 27 additions & 13 deletions examples/crd_derive_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use kube::{
runtime::wait::{await_condition, conditions},
Client, CustomResource, CustomResourceExt,
};
use kube_derive::cel_validation;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

Expand All @@ -19,6 +20,7 @@ use serde::{Deserialize, Serialize};
// - https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#defaulting
// - https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#defaulting-and-nullable

#[cel_validation]
#[derive(CustomResource, Serialize, Deserialize, Default, Debug, PartialEq, Eq, Clone, JsonSchema)]
#[kube(
group = "clux.dev",
Expand Down Expand Up @@ -87,7 +89,8 @@ pub struct FooSpec {
set_listable: Vec<u32>,
// Field with CEL validation
#[serde(default)]
#[schemars(schema_with = "cel_validations")]
#[validated(rule = "self != 'illegal'", message = "string cannot be illegal")]
#[validated(rule = "self != 'not legal'")]
cel_validated: Option<String>,
}
// https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy
Expand All @@ -104,18 +107,6 @@ fn set_listable_schema(_: &mut schemars::gen::SchemaGenerator) -> schemars::sche
.unwrap()
}

// https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules
fn cel_validations(_: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
serde_json::from_value(serde_json::json!({
"type": "string",
"x-kubernetes-validations": [{
"rule": "self != 'illegal'",
"message": "string cannot be illegal"
}]
}))
.unwrap()
}

fn default_value() -> String {
"default_value".into()
}
Expand Down Expand Up @@ -248,6 +239,29 @@ async fn main() -> Result<()> {
}
_ => panic!(),
}

// cel validation triggers:
let cel_patch = serde_json::json!({
"apiVersion": "clux.dev/v1",
"kind": "Foo",
"spec": {
"cel_validated": Some("not legal")
}
});
let cel_res = foos.patch("baz", &ssapply, &Patch::Apply(cel_patch)).await;
assert!(cel_res.is_err());
match cel_res.err() {
Some(kube::Error::Api(err)) => {
assert_eq!(err.code, 422);
assert_eq!(err.reason, "Invalid");
assert_eq!(err.status, "Failure");
assert!(err.message.contains("Foo.clux.dev \"baz\" is invalid"));
assert!(err.message.contains("spec.cel_validated: Invalid value"));
assert!(err.message.contains("failed rule: self != 'not legal'"));
clux marked this conversation as resolved.
Show resolved Hide resolved
}
_ => panic!(),
}

// cel validation happy:
let cel_patch_ok = serde_json::json!({
"apiVersion": "clux.dev/v1",
Expand Down
113 changes: 111 additions & 2 deletions kube-derive/src/custom_resource.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Generated by darling macros, out of our control
#![allow(clippy::manual_unwrap_or_default)]

use darling::{FromDeriveInput, FromMeta};
use darling::{FromAttributes, FromDeriveInput, FromMeta};
use proc_macro2::{Ident, Literal, Span, TokenStream};
use quote::ToTokens;
use syn::{parse_quote, Data, DeriveInput, Path, Visibility};
use syn::{parse::Parser, parse_quote, Attribute, Data, DeriveInput, Path, Visibility};

/// Values we can parse from #[kube(attrs)]
#[derive(Debug, FromDeriveInput)]
Expand Down Expand Up @@ -539,6 +539,115 @@
}
}

#[derive(FromAttributes)]
#[darling(attributes(validated))]
struct CELAttr {
rule: String,
message: Option<String>,
}

pub(crate) fn cel_validation(_: TokenStream, input: TokenStream) -> TokenStream {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the first arg here for? the new proc macro seems take args and input, but you seem to be discarding args?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to be checked, they are currently empty. Could be a good place for name overrides to avoid clashes

let mut ast: DeriveInput = match syn::parse2(input) {
Err(err) => return err.to_compile_error(),
Ok(di) => di,

Check warning on line 552 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L549-L552

Added lines #L549 - L552 were not covered by tests
};

if !ast.attrs.iter().any(|attr| attr.path().is_ident("derive")) {
return syn::Error::new(
ast.ident.span(),

Check warning on line 557 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L555-L557

Added lines #L555 - L557 were not covered by tests
r#"#[cel_validation] macro should be placed before the #[derive(JsonSchema)] macro"#,
)
.to_compile_error();

Check warning on line 560 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L560

Added line #L560 was not covered by tests
}

let struct_name = ast.ident.to_string() + "Validation";
let anchor = Ident::new(&struct_name, Span::call_site());

Check warning on line 564 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L563-L564

Added lines #L563 - L564 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magic names needs a comment here probably


let mut validations: Vec<TokenStream> = vec![];

Check warning on line 566 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L566

Added line #L566 was not covered by tests

let struct_data = match ast.data {
syn::Data::Struct(ref mut struct_data) => struct_data,

Check warning on line 569 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L568-L569

Added lines #L568 - L569 were not covered by tests
_ => {
return syn::Error::new(
ast.ident.span(),

Check warning on line 572 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L571-L572

Added lines #L571 - L572 were not covered by tests
r#"#[cel_validation] has to be used with structs"#,
)
.to_compile_error()
}
};

if let syn::Fields::Named(fields) = &mut struct_data.fields {
for field in &mut fields.named {
let mut rules = vec![];
for attr in field

Check warning on line 582 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L579-L582

Added lines #L579 - L582 were not covered by tests
.attrs
.iter()
.filter(|attr| attr.path().is_ident("validated"))

Check warning on line 585 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L585

Added line #L585 was not covered by tests
{
let CELAttr { rule, message } = match CELAttr::from_attributes(&vec![attr.clone()]) {
Ok(cel) => cel,
Err(e) => return e.with_span(&attr.meta).write_errors(),

Check warning on line 589 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L587-L589

Added lines #L587 - L589 were not covered by tests
};
let message = if let Some(message) = message {
quote! { "message": #message }

Check warning on line 592 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L591-L592

Added lines #L591 - L592 were not covered by tests
} else {
quote! {}

Check warning on line 594 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L594

Added line #L594 was not covered by tests
};
rules.push(quote! {{

Check warning on line 596 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L596

Added line #L596 was not covered by tests
"rule": #rule,
#message
},});
}

if rules.is_empty() {

Check warning on line 602 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L602

Added line #L602 was not covered by tests
continue;
}

let validation_method_name = field.ident.as_ref().map(|i| i.to_string()).unwrap_or_default();
let name = Ident::new(&validation_method_name, Span::call_site());
let field_type = &field.ty;

Check warning on line 608 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L606-L608

Added lines #L606 - L608 were not covered by tests
Comment on lines +678 to +680
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff here could do with some comments. You're implementing a method fith a fixed name. Could this ave clashing issues? Can we parametrise such a function from kube-core instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as there is no structure Validation in the scope, there should not be a clash, but makes sense to add an override for such occasion 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be added now - #[cel_validation(struct_name = "FooSpecValidation")]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess i am still confused about the temporary struct you are making. it seems to me that the #[cel_validation(struct_name = X struct X is there for you to implement a method, but that method feels like something we can define cleanly inside kube-core, and invoke from kube-derive.


validations.push(quote! {

Check warning on line 610 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L610

Added line #L610 was not covered by tests
fn #name(gen: &mut ::schemars::gen::SchemaGenerator) -> ::schemars::schema::Schema {
let s = gen.subschema_for::<#field_type>();
let mut s = serde_json::to_value(s).unwrap();
let v = serde_json::json!([
#(#rules)*
]);
match &mut s {
serde_json::Value::Object(a) => {
a.insert("x-kubernetes-validations".into(), v.clone());
}
_ => (),
}
serde_json::from_value(s).unwrap()
}
});

let validator = struct_name.clone() + "::" + &validation_method_name;
let new_serde_attr = quote! {

Check warning on line 628 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L627-L628

Added lines #L627 - L628 were not covered by tests
#[schemars(schema_with = #validator)]
};

let parser = Attribute::parse_outer;
match parser.parse2(new_serde_attr) {
Ok(ref mut parsed) => field.attrs.append(parsed),
Err(e) => return e.to_compile_error(),

Check warning on line 635 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L633-L635

Added lines #L633 - L635 were not covered by tests
};
}
}

quote! {

Check warning on line 640 in kube-derive/src/custom_resource.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/custom_resource.rs#L640

Added line #L640 was not covered by tests
struct #anchor {}

impl #anchor {
#(#validations)*
}

#ast
}
}

struct StatusInformation {
/// The code to be used for the field in the main struct
field: TokenStream,
Expand Down
37 changes: 36 additions & 1 deletion kube-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,46 @@
/// [`kube::Resource`]: https://docs.rs/kube/*/kube/trait.Resource.html
/// [`kube::core::ApiResource`]: https://docs.rs/kube/*/kube/core/struct.ApiResource.html
/// [`kube::CustomResourceExt`]: https://docs.rs/kube/*/kube/trait.CustomResourceExt.html
#[proc_macro_derive(CustomResource, attributes(kube))]
#[proc_macro_derive(CustomResource, attributes(kube, validated))]
pub fn derive_custom_resource(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
custom_resource::derive(proc_macro2::TokenStream::from(input)).into()
}

/// Generates a JsonSchema patch with a set of CEL expression validation rules applied on the CRD.
///
/// This macro should be placed before the `#[derive(JsonSchema)]` macro on the struct being validated,
/// as it performs addition of the #[schemars(schema_with = "<validation_injector>")] derive macro
/// on the validated field.
///
/// # Example
///
/// ```rust
/// use kube::cel_validation;
/// use kube::CustomResource;
/// use serde::Deserialize;
/// use serde::Serialize;
/// use schemars::JsonSchema;
/// use kube::core::crd::CustomResourceExt;
///
/// #[cel_validation]
/// #[derive(CustomResource, Serialize, Deserialize, Clone, Debug, JsonSchema)]
/// #[kube(group = "kube.rs", version = "v1", kind = "Struct")]
/// struct MyStruct {
/// #[validated(rule = "self != ''")]
/// field: String,
/// }
///
/// assert!(serde_json::to_string(&Struct::crd()).unwrap().contains("x-kubernetes-validations"));
/// assert!(serde_json::to_string(&Struct::crd()).unwrap().contains(r#""rule":"self != ''""#));
/// ```
#[proc_macro_attribute]
pub fn cel_validation(

Check warning on line 346 in kube-derive/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/lib.rs#L346

Added line #L346 was not covered by tests
args: proc_macro::TokenStream,
input: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
custom_resource::cel_validation(args.into(), input.into()).into()

Check warning on line 350 in kube-derive/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

kube-derive/src/lib.rs#L350

Added line #L350 was not covered by tests
}

/// A custom derive for inheriting Resource impl for the type.
///
/// This will generate a [`kube::Resource`] trait implementation,
Expand Down
4 changes: 4 additions & 0 deletions kube/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ pub use kube_derive::CustomResource;
#[cfg_attr(docsrs, doc(cfg(feature = "derive")))]
pub use kube_derive::Resource;

#[cfg(feature = "derive")]
#[cfg_attr(docsrs, doc(cfg(feature = "derive")))]
pub use kube_derive::cel_validation;

#[cfg(feature = "runtime")]
#[cfg_attr(docsrs, doc(cfg(feature = "runtime")))]
#[doc(inline)]
Expand Down