feat: Add typed scale argument to derive macro (#1656)

* feat: Add typed scale argument to derive macro This allows cutomizing the scale subresource by providing key-value items instead of a raw JSON string. For backwards-compatibility, it is still supported to provide a JSON string. However, all examples and tests were converted to the new format. Signed-off-by: Techassi <[email protected]> * refactor: Remove k8s_openapi dependency Signed-off-by: Techassi <[email protected]> * chore: Adjust doc comment, fix clippy lint Signed-off-by: Techassi <[email protected]> * chore: Fix clippy lint in kube-runtime Signed-off-by: Techassi <[email protected]> * docs: Adjust doc comment Signed-off-by: Techassi <[email protected]> * chore: Use serde derive feature to enable derive macro Signed-off-by: Techassi <[email protected]> * chore: Use ignore instead of no_run Signed-off-by: Techassi <[email protected]> * test: Add schema test, fix FromMeta implementation Adding this test proved to be very valuable because the FromMeta implemenetation had a few errors and resulted in different panic messages coming from the derive macro. I also added a small note to the #[kube(scale(...))] section stating that the scale subresource can only be used when the status subresource is used as well. I plan to further improve the validation in a future pull request. Signed-off-by: Techassi <[email protected]> --------- Signed-off-by: Techassi <[email protected]>
kube-rs · Feb 19, 2025 · c191439 · c191439
1 parent 07b7891
commit c191439
Show file tree

Hide file tree

Showing 7 changed files with 199 additions and 12 deletions.
diff --git a/examples/crd_api.rs b/examples/crd_api.rs
@@ -19,7 +19,10 @@ use kube::{
 #[derive(CustomResource, Deserialize, Serialize, Clone, Debug, Validate, JsonSchema)]
 #[kube(group = "clux.dev", version = "v1", kind = "Foo", namespaced)]
 #[kube(status = "FooStatus")]
-#[kube(scale = r#"{"specReplicasPath":".spec.replicas", "statusReplicasPath":".status.replicas"}"#)]
+#[kube(scale(
+    spec_replicas_path = ".spec.replicas",
+    status_replicas_path = ".status.replicas"
+))]
 #[kube(printcolumn = r#"{"name":"Team", "jsonPath": ".spec.metadata.team", "type": "string"}"#)]
 pub struct FooSpec {
     #[schemars(length(min = 3))]

diff --git a/examples/crd_derive.rs b/examples/crd_derive.rs
@@ -22,7 +22,10 @@ use serde::{Deserialize, Serialize};
     derive = "PartialEq",
     derive = "Default",
     shortname = "f",
-    scale = r#"{"specReplicasPath":".spec.replicas", "statusReplicasPath":".status.replicas"}"#,
+    scale(
+        spec_replicas_path = ".spec.replicas",
+        status_replicas_path = ".status.replicas"
+    ),
     printcolumn = r#"{"name":"Spec", "type":"string", "description":"name of foo", "jsonPath":".spec.name"}"#,
     selectable = "spec.name"
 )]

diff --git a/kube-derive/Cargo.toml b/kube-derive/Cargo.toml
@@ -18,6 +18,7 @@ workspace = true
 proc-macro2.workspace = true
 quote.workspace = true
 syn = { workspace = true, features = ["extra-traits"] }
+serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
 darling.workspace = true
 

diff --git a/kube-derive/src/custom_resource.rs b/kube-derive/src/custom_resource.rs
@@ -3,6 +3,7 @@
 use darling::{FromDeriveInput, FromMeta};
 use proc_macro2::{Ident, Literal, Span, TokenStream};
 use quote::{ToTokens, TokenStreamExt as _};
+use serde::Deserialize;
 use syn::{parse_quote, Data, DeriveInput, Expr, Path, Visibility};
 
 /// Values we can parse from #[kube(attrs)]
@@ -33,7 +34,12 @@ struct KubeAttrs {
     printcolums: Vec<String>,
     #[darling(multiple)]
     selectable: Vec<String>,
-    scale: Option<String>,
+
+    /// Customize the scale subresource, see [Kubernetes docs][1].
+    ///
+    /// [1]: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#scale-subresource
+    scale: Option<Scale>,
+
     #[darling(default)]
     crates: Crates,
     #[darling(multiple, rename = "annotation")]
@@ -192,6 +198,122 @@ impl FromMeta for SchemaMode {
     }
 }
 
+/// This struct mirrors the fields of `k8s_openapi::CustomResourceSubresourceScale` to support
+/// parsing from the `#[kube]` attribute.
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct Scale {
+    pub(crate) label_selector_path: Option<String>,
+    pub(crate) spec_replicas_path: String,
+    pub(crate) status_replicas_path: String,
+}
+
+// This custom FromMeta implementation is needed for two reasons:
+//
+// - To enable backwards-compatibility. Up to version 0.97.0 it was only possible to set scale
+//   subresource values as a JSON string.
+// - To be able to declare the scale sub-resource as a list of typed fields. The from_list impl uses
+//   the derived implementation as inspiration.
+impl FromMeta for Scale {
+    /// This is implemented for backwards-compatibility. It allows that the scale subresource can
+    /// be deserialized from a JSON string.
+    fn from_string(value: &str) -> darling::Result<Self> {
+        serde_json::from_str(value).map_err(darling::Error::custom)
+    }
+
+    fn from_list(items: &[darling::ast::NestedMeta]) -> darling::Result<Self> {
+        let mut errors = darling::Error::accumulator();
+
+        let mut label_selector_path: (bool, Option<Option<String>>) = (false, None);
+        let mut spec_replicas_path: (bool, Option<String>) = (false, None);
+        let mut status_replicas_path: (bool, Option<String>) = (false, None);
+
+        for item in items {
+            match item {
+                darling::ast::NestedMeta::Meta(meta) => {
+                    let name = darling::util::path_to_string(meta.path());
+
+                    match name.as_str() {
+                        "label_selector_path" => {
+                            if !label_selector_path.0 {
+                                let path = errors.handle(darling::FromMeta::from_meta(meta));
+                                label_selector_path = (true, Some(path))
+                            } else {
+                                errors.push(
+                                    darling::Error::duplicate_field("label_selector_path").with_span(&meta),
+                                );
+                            }
+                        }
+                        "spec_replicas_path" => {
+                            if !spec_replicas_path.0 {
+                                let path = errors.handle(darling::FromMeta::from_meta(meta));
+                                spec_replicas_path = (true, path)
+                            } else {
+                                errors.push(
+                                    darling::Error::duplicate_field("spec_replicas_path").with_span(&meta),
+                                );
+                            }
+                        }
+                        "status_replicas_path" => {
+                            if !status_replicas_path.0 {
+                                let path = errors.handle(darling::FromMeta::from_meta(meta));
+                                status_replicas_path = (true, path)
+                            } else {
+                                errors.push(
+                                    darling::Error::duplicate_field("status_replicas_path").with_span(&meta),
+                                );
+                            }
+                        }
+                        other => errors.push(darling::Error::unknown_field(other)),
+                    }
+                }
+                darling::ast::NestedMeta::Lit(lit) => {
+                    errors.push(darling::Error::unsupported_format("literal").with_span(&lit.span()))
+                }
+            }
+        }
+
+        if !spec_replicas_path.0 && spec_replicas_path.1.is_none() {
+            errors.push(darling::Error::missing_field("spec_replicas_path"));
+        }
+
+        if !status_replicas_path.0 && status_replicas_path.1.is_none() {
+            errors.push(darling::Error::missing_field("status_replicas_path"));
+        }
+
+        errors.finish()?;
+
+        Ok(Self {
+            label_selector_path: label_selector_path.1.unwrap_or_default(),
+            spec_replicas_path: spec_replicas_path.1.unwrap(),
+            status_replicas_path: status_replicas_path.1.unwrap(),
+        })
+    }
+}
+
+impl Scale {
+    fn to_tokens(&self, k8s_openapi: &Path) -> TokenStream {
+        let apiext = quote! {
+            #k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1
+        };
+
+        let label_selector_path = self
+            .label_selector_path
+            .as_ref()
+            .map_or_else(|| quote! { None }, |p| quote! { Some(#p.into()) });
+        let spec_replicas_path = &self.spec_replicas_path;
+        let status_replicas_path = &self.status_replicas_path;
+
+        quote! {
+            #apiext::CustomResourceSubresourceScale {
+                label_selector_path: #label_selector_path,
+                spec_replicas_path: #spec_replicas_path.into(),
+                status_replicas_path: #status_replicas_path.into()
+            }
+        }
+    }
+}
+
 pub(crate) fn derive(input: proc_macro2::TokenStream) -> proc_macro2::TokenStream {
     let derive_input: DeriveInput = match syn::parse2(input) {
         Err(err) => return err.to_compile_error(),
@@ -452,7 +574,13 @@ pub(crate) fn derive(input: proc_macro2::TokenStream) -> proc_macro2::TokenStrea
         .map(|s| format!(r#"{{ "jsonPath": "{s}" }}"#))
         .collect();
     let fields = format!("[ {} ]", fields.join(","));
-    let scale_code = if let Some(s) = scale { s } else { "".to_string() };
+    let scale = scale.map_or_else(
+        || quote! { None },
+        |s| {
+            let scale = s.to_tokens(&k8s_openapi);
+            quote! { Some(#scale) }
+        },
+    );
 
     // Ensure it generates for the correct CRD version (only v1 supported now)
     let apiext = quote! {
@@ -564,11 +692,7 @@ pub(crate) fn derive(input: proc_macro2::TokenStream) -> proc_macro2::TokenStrea
                 #k8s_openapi::k8s_if_ge_1_30! {
                     let fields : Vec<#apiext::SelectableField> = #serde_json::from_str(#fields).expect("valid selectableField column json");
                 }
-                let scale: Option<#apiext::CustomResourceSubresourceScale> = if #scale_code.is_empty() {
-                    None
-                } else {
-                    #serde_json::from_str(#scale_code).expect("valid scale subresource json")
-                };
+                let scale: Option<#apiext::CustomResourceSubresourceScale> = #scale;
                 let categories: Vec<String> = #serde_json::from_str(#categories_json).expect("valid categories");
                 let shorts : Vec<String> = #serde_json::from_str(#short_json).expect("valid shortnames");
                 let subres = if #has_status {

diff --git a/kube-derive/src/lib.rs b/kube-derive/src/lib.rs
@@ -129,8 +129,22 @@ mod resource;
 /// NOTE: `CustomResourceDefinition`s require a schema. If `schema = "disabled"` then
 /// `Self::crd()` will not be installable into the cluster as-is.
 ///
-/// ## `#[kube(scale = r#"json"#)]`
+/// ## `#[kube(scale(...))]`
+///
 /// Allow customizing the scale struct for the [scale subresource](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#subresources).
+/// It should be noted, that the status subresource must also be enabled to use the scale subresource. This is because
+/// the `statusReplicasPath` only accepts JSONPaths under `.status`.
+///
+/// ```ignore
+/// #[kube(scale(
+///     specReplicasPath = ".spec.replicas",
+///     statusReplicaPath = ".status.replicas",
+///     labelSelectorPath = ".spec.labelSelector"
+/// ))]
+/// ```
+///
+/// The deprecated way of customizing the scale subresource using a raw JSON string is still
+/// support for backwards-compatibility.
 ///
 /// ## `#[kube(printcolumn = r#"json"#)]`
 /// Allows adding straight json to [printcolumns](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#additional-printer-columns).

diff --git a/kube-derive/tests/crd_schema_test.rs b/kube-derive/tests/crd_schema_test.rs
@@ -1,3 +1,4 @@
+#![allow(missing_docs)]
 #![recursion_limit = "256"]
 
 use assert_json_diff::assert_json_eq;
@@ -29,6 +30,12 @@ use std::collections::{HashMap, HashSet};
     label("clux.dev", "cluxingv1"),
     label("clux.dev/persistence", "disabled"),
     rule = Rule::new("self.metadata.name == 'singleton'"),
+    status = "Status",
+    scale(
+        spec_replicas_path = ".spec.replicas",
+        status_replicas_path = ".status.replicas",
+        label_selector_path = ".status.labelSelector"
+    ),
 )]
 #[cel_validate(rule = Rule::new("has(self.nonNullable)"))]
 #[serde(rename_all = "camelCase")]
@@ -62,6 +69,13 @@ struct FooSpec {
     set: HashSet<String>,
 }
 
+#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct Status {
+    replicas: usize,
+    label_selector: String,
+}
+
 fn default_value() -> String {
     "default_value".into()
 }
@@ -231,6 +245,14 @@ fn test_crd_schema_matches_expected() {
                         }, {
                             "jsonPath": ".spec.nullable"
                         }],
+                        "subresources": {
+                            "status": {},
+                            "scale": {
+                                "specReplicasPath": ".spec.replicas",
+                                "labelSelectorPath": ".status.labelSelector",
+                                "statusReplicasPath": ".status.replicas"
+                            }
+                        },
                         "schema": {
                             "openAPIV3Schema": {
                                 "description": "Custom resource representing a Foo",
@@ -358,6 +380,24 @@ fn test_crd_schema_matches_expected() {
                                             "rule": "has(self.nonNullable)",
                                         }],
                                         "type": "object"
+                                    },
+                                    "status": {
+                                        "properties": {
+                                            "replicas": {
+                                                "type": "integer",
+                                                "format": "uint",
+                                                "minimum": 0.0,
+                                            },
+                                            "labelSelector": {
+                                                "type": "string"
+                                            }
+                                        },
+                                        "required": [
+                                            "labelSelector",
+                                            "replicas"
+                                        ],
+                                        "nullable": true,
+                                        "type": "object"
                                     }
                                 },
                                 "required": [
@@ -370,7 +410,6 @@ fn test_crd_schema_matches_expected() {
                                 "type": "object"
                             }
                         },
-                        "subresources": {},
                     }
                 ]
             }

diff --git a/kube/src/lib.rs b/kube/src/lib.rs
@@ -227,7 +227,10 @@ mod test {
     #[derive(CustomResource, Deserialize, Serialize, Clone, Debug, JsonSchema)]
     #[kube(group = "clux.dev", version = "v1", kind = "Foo", namespaced)]
     #[kube(status = "FooStatus")]
-    #[kube(scale = r#"{"specReplicasPath":".spec.replicas", "statusReplicasPath":".status.replicas"}"#)]
+    #[kube(scale(
+        spec_replicas_path = ".spec.replicas",
+        status_replicas_path = ".status.replicas"
+    ))]
     #[kube(crates(kube_core = "crate::core"))] // for dev-dep test structure
     pub struct FooSpec {
         name: String,