Skip to content

Commit

Permalink
Genericize PartialObjectMeta over the underlying Resource (kube-r…
Browse files Browse the repository at this point in the history
…s#1152)

* Genericize PartialObjectMeta over the underlying Resource

Allows doing static dispatch using the root type, and avoids
having to do stringly typed oob signalling in generic code.

Signed-off-by: clux <[email protected]>

* fmt + clippy fix

Signed-off-by: clux <[email protected]>

* doc tests + consolidate extensions usage for metadata api

Signed-off-by: clux <[email protected]>

* clippy

Signed-off-by: clux <[email protected]>

* add a test converter + a quick unit test

conversion the other way avoids users having to write:

```rust
let pom = PartialObjectMeta::<Pod> {
   types: Some(TypeMeta{ ...constant }),
   metadata: actual_test_data,
   ..Default::default(),
};
```

Signed-off-by: clux <[email protected]>

* check main prop as well

Signed-off-by: clux <[email protected]>

* Remove/split misguided From<ObjectMeta> for PartialObjectMeta

Need to have two different ways to convert from ObjectMeta depending on how we are going to use it.
Have created a sealed trait with converters for it, and updated docs, tests to use it.

Signed-off-by: clux <[email protected]>

* simplify trait bound to just the empty dyn type

avoids unstable associated trait bounds + lints showing that the default really had to be the empty type.

Signed-off-by: clux <[email protected]>

* one clippy lint having a rough one today

bunch of new bugs in https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+let_underscore_untyped
our use seems perfectly fine, what clippy wanted was nonsense:

```
error: non-binding `let` without a type annotation
  --> kube-runtime/src/reflector/store.rs:87:12
   |
87 | pub struct Store<K: 'static + Resource>
   |            ^^^^^
   |
   = help: consider adding a type annotation or removing the `let` keyword
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_untyped

error: non-binding `let` without a type annotation
   --> kube-runtime/src/watcher.rs:123:5
    |
123 |     InitListed { resource_version: String },
    |     ^^^^^^^^^^
    |
    = help: consider adding a type annotation or removing the `let` keyword
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_untyped
```

Signed-off-by: clux <[email protected]>

---------

Signed-off-by: clux <[email protected]>
  • Loading branch information
clux authored Mar 2, 2023
1 parent b156131 commit f4dec45
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 56 deletions.
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ default:
clippy:
#rustup component add clippy --toolchain nightly
cargo +nightly clippy --workspace
cargo +nightly clippy --no-default-features --features=rustls-tls
cargo +nightly clippy --all-features

fmt:
#rustup component add rustfmt --toolchain nightly
Expand Down
45 changes: 23 additions & 22 deletions kube-client/src/api/core_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ where
/// async fn main() -> Result<(), Box<dyn std::error::Error>> {
/// let client = Client::try_default().await?;
/// let pods: Api<Pod> = Api::namespaced(client, "apps");
/// let p: PartialObjectMeta = pods.get_metadata("blog").await?;
/// let p: PartialObjectMeta<Pod> = pods.get_metadata("blog").await?;
/// Ok(())
/// }
/// ```
Expand All @@ -59,10 +59,10 @@ where
///
/// This function assumes that the object is expected to always exist, and returns [`Error`] if it does not.
/// Consider using [`Api::get_metadata_opt`] if you need to handle missing objects.
pub async fn get_metadata(&self, name: &str) -> Result<PartialObjectMeta> {
pub async fn get_metadata(&self, name: &str) -> Result<PartialObjectMeta<K>> {
let mut req = self.request.get_metadata(name).map_err(Error::BuildRequest)?;
req.extensions_mut().insert("get");
self.client.request::<PartialObjectMeta>(req).await
req.extensions_mut().insert("get_metadata");
self.client.request::<PartialObjectMeta<K>>(req).await
}

/// [Get](`Api::get`) a named resource if it exists, returns [`None`] if it doesn't exist
Expand Down Expand Up @@ -111,7 +111,7 @@ where
///
/// Note that [kube_core::metadata::PartialObjectMeta] may be converted to `ObjectMeta`
/// through the usual conversion traits.
pub async fn get_metadata_opt(&self, name: &str) -> Result<Option<PartialObjectMeta>> {
pub async fn get_metadata_opt(&self, name: &str) -> Result<Option<PartialObjectMeta<K>>> {
match self.get_metadata(name).await {
Ok(meta) => Ok(Some(meta)),
Err(Error::Api(ErrorResponse { reason, .. })) if &reason == "NotFound" => Ok(None),
Expand Down Expand Up @@ -156,18 +156,17 @@ where
/// let client = Client::try_default().await?;
/// let pods: Api<Pod> = Api::namespaced(client, "apps");
/// let lp = ListParams::default().labels("app=blog"); // for this app only
/// let list: ObjectList<PartialObjectMeta> = pods.list_metadata(&lp).await?;
/// let list: ObjectList<PartialObjectMeta<Pod>> = pods.list_metadata(&lp).await?;
/// for p in list {
/// let metadata = ObjectMeta::from(p);
/// println!("Found Pod: {}", metadata.name.unwrap());
/// println!("Found Pod: {}", p.name_any());
/// }
/// Ok(())
/// }
/// ```
pub async fn list_metadata(&self, lp: &ListParams) -> Result<ObjectList<PartialObjectMeta>> {
pub async fn list_metadata(&self, lp: &ListParams) -> Result<ObjectList<PartialObjectMeta<K>>> {
let mut req = self.request.list_metadata(lp).map_err(Error::BuildRequest)?;
req.extensions_mut().insert("list");
self.client.request::<ObjectList<PartialObjectMeta>>(req).await
req.extensions_mut().insert("list_metadata");
self.client.request::<ObjectList<PartialObjectMeta<K>>>(req).await
}

/// Create a resource
Expand Down Expand Up @@ -330,9 +329,6 @@ where
/// "labels": {
/// "key": "value"
/// },
/// },
/// "spec": {
/// "activeDeadlineSeconds": 5
/// }
/// });
/// let params = PatchParams::apply("myapp");
Expand All @@ -345,20 +341,25 @@ where
/// [`Patch`]: super::Patch
/// [`PatchParams`]: super::PatchParams
///
/// Note that this method cannot write to the status object (when it exists) of a resource.
/// To set status objects please see [`Api::replace_status`] or [`Api::patch_status`].
/// ### Warnings
///
/// The `TypeMeta` (apiVersion + kind) of a patch request (required for apply patches)
/// must match the underlying type that is being patched (e.g. "v1" + "Pod").
/// The returned `TypeMeta` will always be {"meta.k8s.io/v1", "PartialObjectMetadata"}.
///
/// This method can write to non-metadata fields such as spec if included in the patch.
pub async fn patch_metadata<P: Serialize + Debug>(
&self,
name: &str,
pp: &PatchParams,
patch: &Patch<P>,
) -> Result<PartialObjectMeta> {
) -> Result<PartialObjectMeta<K>> {
let mut req = self
.request
.patch_metadata(name, pp, patch)
.map_err(Error::BuildRequest)?;
req.extensions_mut().insert("patch");
self.client.request::<PartialObjectMeta>(req).await
req.extensions_mut().insert("patch_metadata");
self.client.request::<PartialObjectMeta<K>>(req).await
}

/// Replace a resource entirely with a new one
Expand Down Expand Up @@ -510,12 +511,12 @@ where
&self,
lp: &ListParams,
version: &str,
) -> Result<impl Stream<Item = Result<WatchEvent<PartialObjectMeta>>>> {
) -> Result<impl Stream<Item = Result<WatchEvent<PartialObjectMeta<K>>>>> {
let mut req = self
.request
.watch_metadata(lp, version)
.map_err(Error::BuildRequest)?;
req.extensions_mut().insert("watch");
self.client.request_events::<PartialObjectMeta>(req).await
req.extensions_mut().insert("watch_metadata");
self.client.request_events::<PartialObjectMeta<K>>(req).await
}
}
2 changes: 1 addition & 1 deletion kube-client/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(crate) use kube_core::params;
pub use kube_core::{
dynamic::{ApiResource, DynamicObject},
gvk::{GroupVersionKind, GroupVersionResource},
metadata::{ListMeta, ObjectMeta, TypeMeta},
metadata::{ListMeta, ObjectMeta, PartialObjectMeta, PartialObjectMetaExt, TypeMeta},
object::{NotUsed, Object, ObjectList},
request::Request,
watch::WatchEvent,
Expand Down
22 changes: 10 additions & 12 deletions kube-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ mod test {
api::{DeleteParams, EvictParams, ListParams, Patch, PatchParams, WatchEvent},
core::subresource::LogParams,
};
use kube_core::{ObjectList, ObjectMeta};
use kube_core::{ObjectList, ObjectMeta, PartialObjectMeta, PartialObjectMetaExt};

let client = Client::try_default().await?;
let pods: Api<Pod> = Api::default_namespaced(client);
Expand Down Expand Up @@ -530,22 +530,20 @@ mod test {
Some(&"kube-rs-test".to_string())
);

// Attempt to patch pod
let patch = json!({
"metadata": {
"annotations": {
"test": "123"
},
},
"spec": {
"activeDeadlineSeconds": 5
}
});
// Attempt to patch pod metadata
let patch = ObjectMeta {
annotations: Some([("test".to_string(), "123".to_string())].into()),
..Default::default()
}
.into_request_partial::<Pod>();

let patchparams = PatchParams::default();
let p_patched = pods
.patch_metadata("busybox-kube-meta", &patchparams, &Patch::Merge(&patch))
.await?;
assert_eq!(p_patched.annotations().get("test"), Some(&"123".to_string()));
assert_eq!(p_patched.types.as_ref().unwrap().kind, "PartialObjectMetadata");
assert_eq!(p_patched.types.as_ref().unwrap().api_version, "meta.k8s.io/v1");

// Clean-up
let dp = DeleteParams::default();
Expand Down
2 changes: 1 addition & 1 deletion kube-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub mod gvk;
pub use gvk::{GroupVersion, GroupVersionKind, GroupVersionResource};

pub mod metadata;
pub use metadata::{ListMeta, ObjectMeta, TypeMeta};
pub use metadata::{ListMeta, ObjectMeta, PartialObjectMeta, PartialObjectMetaExt, TypeMeta};

pub mod object;
pub use object::{NotUsed, Object, ObjectList};
Expand Down
111 changes: 94 additions & 17 deletions kube-core/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Metadata structs used in traits, lists, and dynamic objects.
use std::borrow::Cow;
use std::{borrow::Cow, marker::PhantomData};

pub use k8s_openapi::apimachinery::pkg::apis::meta::v1::{ListMeta, ObjectMeta};
use serde::{Deserialize, Serialize};

use crate::{ApiResource, DynamicResourceScope, Resource};
use crate::{DynamicObject, Resource};

/// Type information that is flattened into every kubernetes object
#[derive(Deserialize, Serialize, Clone, Default, Debug, Eq, PartialEq, Hash)]
Expand All @@ -21,41 +21,84 @@ pub struct TypeMeta {
///
/// It allows clients to get access to a particular `ObjectMeta`
/// schema without knowing the details of the version.
///
/// See the [`PartialObjectMetaExt`] trait for how to construct one safely.
#[derive(Deserialize, Serialize, Clone, Default, Debug)]
#[serde(rename_all = "camelCase")]
pub struct PartialObjectMeta {
pub struct PartialObjectMeta<K = DynamicObject> {
/// The type fields, not always present
#[serde(flatten, default)]
pub types: Option<TypeMeta>,
/// Standard object's metadata
#[serde(default)]
pub metadata: ObjectMeta,
/// Type information for static dispatch
#[serde(skip, default)]
pub _phantom: PhantomData<K>,
}

mod private {
pub trait Sealed {}
impl Sealed for super::ObjectMeta {}
}
/// Helper trait for converting `ObjectMeta` into useful `PartialObjectMeta` variants
pub trait PartialObjectMetaExt: private::Sealed {
/// Convert `ObjectMeta` into a Patch-serializable `PartialObjectMeta`
///
/// This object can be passed to `Patch::Apply` and used with `Api::patch_metadata`,
/// for an `Api<K>` using the underlying types `TypeMeta`
fn into_request_partial<K: Resource<DynamicType = ()>>(self) -> PartialObjectMeta<K>;
/// Convert `ObjectMeta` into a response object for a specific `Resource`
///
/// This object emulates a response object and **cannot** be used in request bodies
/// because it contains erased `TypeMeta` (and the apiserver is doing the erasing).
///
/// This method is useful when unit testing local behaviour.
fn into_response_partial<K>(self) -> PartialObjectMeta<K>;
}

impl From<PartialObjectMeta> for ObjectMeta {
fn from(obj: PartialObjectMeta) -> Self {
ObjectMeta { ..obj.metadata }
impl PartialObjectMetaExt for ObjectMeta {
fn into_request_partial<K: Resource<DynamicType = ()>>(self) -> PartialObjectMeta<K> {
PartialObjectMeta {
types: Some(TypeMeta {
api_version: K::api_version(&()).into(),
kind: K::kind(&()).into(),
}),
metadata: self,
_phantom: PhantomData,
}
}

fn into_response_partial<K>(self) -> PartialObjectMeta<K> {
PartialObjectMeta {
types: Some(TypeMeta {
api_version: "meta.k8s.io/v1".to_string(),
kind: "PartialObjectMetadata".to_string(),
}),
metadata: self,
_phantom: PhantomData,
}
}
}

impl Resource for PartialObjectMeta {
type DynamicType = ApiResource;
type Scope = DynamicResourceScope;
impl<K: Resource> Resource for PartialObjectMeta<K> {
type DynamicType = K::DynamicType;
type Scope = K::Scope;

fn kind(dt: &ApiResource) -> Cow<'_, str> {
dt.kind.as_str().into()
fn kind(dt: &Self::DynamicType) -> Cow<'_, str> {
K::kind(dt)
}

fn group(dt: &ApiResource) -> Cow<'_, str> {
dt.group.as_str().into()
fn group(dt: &Self::DynamicType) -> Cow<'_, str> {
K::group(dt)
}

fn version(dt: &ApiResource) -> Cow<'_, str> {
dt.version.as_str().into()
fn version(dt: &Self::DynamicType) -> Cow<'_, str> {
K::version(dt)
}

fn plural(dt: &ApiResource) -> Cow<'_, str> {
dt.plural.as_str().into()
fn plural(dt: &Self::DynamicType) -> Cow<'_, str> {
K::plural(dt)
}

fn meta(&self) -> &ObjectMeta {
Expand All @@ -66,3 +109,37 @@ impl Resource for PartialObjectMeta {
&mut self.metadata
}
}

#[cfg(test)]
mod test {
use super::{ObjectMeta, PartialObjectMeta, PartialObjectMetaExt};
use crate::Resource;
use k8s_openapi::api::core::v1::Pod;

#[test]
fn can_convert_and_derive_partial_metadata() {
// can use generic type for static dispatch;
assert_eq!(PartialObjectMeta::<Pod>::kind(&()), "Pod");
assert_eq!(PartialObjectMeta::<Pod>::api_version(&()), "v1");

// can convert from objectmeta to partials for different use cases:
let meta = ObjectMeta {
name: Some("mypod".into()),
..Default::default()
};
let request_pom = meta.clone().into_request_partial::<Pod>();
let response_pom = meta.into_response_partial::<Pod>();

// they both basically just inline the metadata;
assert_eq!(request_pom.metadata.name, Some("mypod".to_string()));
assert_eq!(response_pom.metadata.name, Some("mypod".to_string()));

// the request_pom will use the TypeMeta from K to support POST/PUT requests
assert_eq!(request_pom.types.as_ref().unwrap().api_version, "v1");
assert_eq!(request_pom.types.as_ref().unwrap().kind, "Pod");

// but the response_pom will use the type-erased kinds from the apiserver
assert_eq!(response_pom.types.as_ref().unwrap().api_version, "meta.k8s.io/v1");
assert_eq!(response_pom.types.as_ref().unwrap().kind, "PartialObjectMetadata");
}
}
2 changes: 2 additions & 0 deletions kube-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#![allow(clippy::type_repetition_in_bounds)]
// Triggered by Tokio macros
#![allow(clippy::semicolon_if_nothing_returned)]
// Triggered by nightly clippy on idiomatic code
#![allow(clippy::let_underscore_untyped)]

pub mod controller;
pub mod events;
Expand Down
4 changes: 2 additions & 2 deletions kube-runtime/src/watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl<K> ApiMode for MetaOnly<'_, K>
where
K: Clone + Debug + DeserializeOwned + Send + 'static,
{
type Value = PartialObjectMeta;
type Value = PartialObjectMeta<K>;

async fn list(&self, lp: &ListParams) -> kube_client::Result<ObjectList<Self::Value>> {
self.api.list_metadata(lp).await
Expand Down Expand Up @@ -439,7 +439,7 @@ pub fn watcher<K: Resource + Clone + DeserializeOwned + Debug + Send + 'static>(
pub fn metadata_watcher<K: Resource + Clone + DeserializeOwned + Debug + Send + 'static>(
api: Api<K>,
list_params: ListParams,
) -> impl Stream<Item = Result<Event<PartialObjectMeta>>> + Send {
) -> impl Stream<Item = Result<Event<PartialObjectMeta<K>>>> + Send {
futures::stream::unfold(
(api, list_params, State::Empty),
|(api, list_params, state)| async {
Expand Down

0 comments on commit f4dec45

Please sign in to comment.