Skip to content

Commit

Permalink
Constrain Resource trait and Api::namespaced by Scope (#956)
Browse files Browse the repository at this point in the history
* Constrain Resource trait and Api::namespaced by Scope

To prevent the namespace type errors we've been seeing for ages that we
have the tools to prevent.

Early sketch.

Signed-off-by: clux <sszynrae@gmail.com>

* Extend kube-core to specify new Scope associated type on Resource

Signed-off-by: clux <sszynrae@gmail.com>

* quick compile tests

Signed-off-by: clux <sszynrae@gmail.com>

* doc for Api ctors and compile_fail doctests on namespaced

Signed-off-by: clux <sszynrae@gmail.com>

* fix cargo deny issue

Signed-off-by: clux <sszynrae@gmail.com>

* better wording on default namespace doc

Signed-off-by: clux <sszynrae@gmail.com>

* use consistent naming of scope structs

Signed-off-by: clux <sszynrae@gmail.com>
  • Loading branch information
clux authored Jul 20, 2022
1 parent fa66405 commit e84cca0
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 15 deletions.
118 changes: 108 additions & 10 deletions kube-client/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub use kube_core::{
watch::WatchEvent,
Resource, ResourceExt,
};
use kube_core::{DynamicResourceScope, NamespaceResourceScope};
pub use params::{
DeleteParams, ListParams, Patch, PatchParams, PostParams, Preconditions, PropagationPolicy,
ValidationDirective,
Expand Down Expand Up @@ -78,7 +79,11 @@ impl<K: Resource> Api<K> {
/// Namespaced resource within a given namespace
///
/// This function accepts `K::DynamicType` so it can be used with dynamic resources.
pub fn namespaced_with(client: Client, ns: &str, dyntype: &K::DynamicType) -> Self {
pub fn namespaced_with(client: Client, ns: &str, dyntype: &K::DynamicType) -> Self
where
K: Resource<Scope = DynamicResourceScope>,
{
// TODO: inspect dyntype scope to verify somehow?
let url = K::url_path(dyntype, Some(ns));
Self {
client,
Expand All @@ -92,9 +97,13 @@ impl<K: Resource> Api<K> {
///
/// This function accepts `K::DynamicType` so it can be used with dynamic resources.
///
/// Unless configured explicitly, the default namespace is either "default"
/// out of cluster, or the service account's namespace in cluster.
pub fn default_namespaced_with(client: Client, dyntype: &K::DynamicType) -> Self {
/// The namespace is either configured on `context` in the kubeconfig
/// or falls back to `default` when running locally, and it's using the service account's
/// namespace when deployed in-cluster.
pub fn default_namespaced_with(client: Client, dyntype: &K::DynamicType) -> Self
where
K: Resource<Scope = DynamicResourceScope>,
{
let ns = client.default_ns().to_string();
Self::namespaced_with(client, &ns, dyntype)
}
Expand All @@ -119,21 +128,88 @@ where
<K as Resource>::DynamicType: Default,
{
/// Cluster level resources, or resources viewed across all namespaces
///
/// Namespace scoped resource allowing querying across all namespaces:
///
/// ```no_run
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Pod;
/// let api: Api<Pod> = Api::all(client);
/// ```
///
/// Cluster scoped resources also use this entrypoint:
///
/// ```no_run
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Node;
/// let api: Api<Node> = Api::all(client);
/// ```
pub fn all(client: Client) -> Self {
Self::all_with(client, &K::DynamicType::default())
}

/// Namespaced resource within a given namespace
pub fn namespaced(client: Client, ns: &str) -> Self {
Self::namespaced_with(client, ns, &K::DynamicType::default())
///
/// ```no_run
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Pod;
/// let api: Api<Pod> = Api::namespaced(client, "default");
/// ```
///
/// This will ONLY work on namespaced resources as set by `Scope`:
///
/// ```compile_fail
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Node;
/// let api: Api<Node> = Api::namespaced(client, "default"); // resource not namespaced!
/// ```
///
/// For dynamic type information, use [`Api::namespaced_with`] variants.
pub fn namespaced(client: Client, ns: &str) -> Self
where
K: Resource<Scope = NamespaceResourceScope>,
{
let dyntype = K::DynamicType::default();
let url = K::url_path(&dyntype, Some(ns));
Self {
client,
request: Request::new(url),
namespace: Some(ns.to_string()),
_phantom: std::iter::empty(),
}
}

/// Namespaced resource within the default namespace
///
/// Unless configured explicitly, the default namespace is either "default"
/// out of cluster, or the service account's namespace in cluster.
pub fn default_namespaced(client: Client) -> Self {
Self::default_namespaced_with(client, &K::DynamicType::default())
/// The namespace is either configured on `context` in the kubeconfig
/// or falls back to `default` when running locally, and it's using the service account's
/// namespace when deployed in-cluster.
///
/// ```no_run
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Pod;
/// let api: Api<Pod> = Api::default_namespaced(client);
/// ```
///
/// This will ONLY work on namespaced resources as set by `Scope`:
///
/// ```compile_fail
/// # use kube::{Api, Client};
/// # let client: Client = todo!();
/// use k8s_openapi::api::core::v1::Node;
/// let api: Api<Node> = Api::default_namespaced(client); // resource not namespaced!
/// ```
pub fn default_namespaced(client: Client) -> Self
where
K: Resource<Scope = NamespaceResourceScope>,
{
let ns = client.default_ns().to_string();
Self::namespaced(client, &ns)
}
}

Expand All @@ -159,3 +235,25 @@ impl<K> Debug for Api<K> {
.finish()
}
}

/// Sanity test on scope restrictions
#[cfg(test)]
mod test {
use crate::{Api, Client};
use k8s_openapi::api::core::v1 as corev1;

use http::{Request, Response};
use hyper::Body;
use tower_test::mock;

#[tokio::test]
async fn scopes_should_allow_correct_interface() {
let (mock_service, _handle) = mock::pair::<Request<Body>, Response<Body>>();
let client = Client::new(mock_service, "default");

let _: Api<corev1::Node> = Api::all(client.clone());
let _: Api<corev1::Pod> = Api::default_namespaced(client.clone());
let _: Api<corev1::PersistentVolume> = Api::all(client.clone());
let _: Api<corev1::ConfigMap> = Api::namespaced(client.clone(), "default");
}
}
6 changes: 5 additions & 1 deletion kube-core/src/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
//! For concrete usage see [examples prefixed with dynamic_](https://github.com/kube-rs/kube-rs/tree/master/examples).

pub use crate::discovery::ApiResource;
use crate::{metadata::TypeMeta, resource::Resource};
use crate::{
metadata::TypeMeta,
resource::{DynamicResourceScope, Resource},
};
use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta;
use std::borrow::Cow;

Expand Down Expand Up @@ -57,6 +60,7 @@ impl DynamicObject {

impl Resource for DynamicObject {
type DynamicType = ApiResource;
type Scope = DynamicResourceScope;

fn group(dt: &ApiResource) -> Cow<'_, str> {
dt.group.as_str().into()
Expand Down
5 changes: 4 additions & 1 deletion kube-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ pub mod request;
pub use request::Request;

mod resource;
pub use resource::{Resource, ResourceExt};
pub use resource::{
ClusterResourceScope, DynamicResourceScope, NamespaceResourceScope, Resource, ResourceExt, ResourceScope,
SubResourceScope,
};

pub mod response;
pub use response::Status;
Expand Down
3 changes: 2 additions & 1 deletion kube-core/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::{
discovery::ApiResource,
metadata::{ListMeta, ObjectMeta, TypeMeta},
resource::Resource,
resource::{DynamicResourceScope, Resource},
};
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
Expand Down Expand Up @@ -216,6 +216,7 @@ where
U: Clone,
{
type DynamicType = ApiResource;
type Scope = DynamicResourceScope;

fn group(dt: &ApiResource) -> Cow<'_, str> {
dt.group.as_str().into()
Expand Down
15 changes: 14 additions & 1 deletion kube-core/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ use k8s_openapi::{

use std::{borrow::Cow, collections::BTreeMap};

pub use k8s_openapi::{ClusterResourceScope, NamespaceResourceScope, ResourceScope, SubResourceScope};

/// Indicates that a [`Resource`] is of an indeterminate dynamic scope.
pub struct DynamicResourceScope {}
impl ResourceScope for DynamicResourceScope {}

/// An accessor trait for a kubernetes Resource.
///
/// This is for a subset of Kubernetes type that do not end in `List`.
Expand All @@ -27,6 +33,11 @@ pub trait Resource {
///
/// See [`DynamicObject`](crate::dynamic::DynamicObject) for a valid implementation of non-k8s-openapi resources.
type DynamicType: Send + Sync + 'static;
/// Type information for the api scope of the resource when known at compile time
///
/// Types from k8s_openapi come with an explicit k8s_openapi::ResourceScope
/// Dynamic types should select `Scope = DynamicResourceScope`
type Scope;

/// Returns kind of this object
fn kind(dt: &Self::DynamicType) -> Cow<'_, str>;
Expand Down Expand Up @@ -105,11 +116,13 @@ pub trait Resource {
}

/// Implement accessor trait for any ObjectMeta-using Kubernetes Resource
impl<K> Resource for K
impl<K, S> Resource for K
where
K: k8s_openapi::Metadata<Ty = ObjectMeta>,
K: k8s_openapi::Resource<Scope = S>,
{
type DynamicType = ();
type Scope = S;

fn kind(_: &()) -> Cow<'_, str> {
K::KIND.into()
Expand Down
7 changes: 6 additions & 1 deletion kube-derive/src/custom_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,17 @@ pub(crate) fn derive(input: proc_macro2::TokenStream) -> proc_macro2::TokenStrea
// 2. Implement Resource trait
let name = singular.unwrap_or_else(|| kind.to_ascii_lowercase());
let plural = plural.unwrap_or_else(|| to_plural(&name));
let scope = if namespaced { "Namespaced" } else { "Cluster" };
let (scope, scope_quote) = if namespaced {
("Namespaced", quote! { #kube_core::NamespaceResourceScope })
} else {
("Cluster", quote! { #kube_core::ClusterResourceScope })
};

let api_ver = format!("{}/{}", group, version);
let impl_resource = quote! {
impl #kube_core::Resource for #rootident {
type DynamicType = ();
type Scope = #scope_quote;

fn group(_: &()) -> std::borrow::Cow<'_, str> {
#group.into()
Expand Down

0 comments on commit e84cca0

Please sign in to comment.