From e8564aba33ae9fdd0749e22f2da9731afb0d3275 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sun, 4 Sep 2022 00:45:28 +0300 Subject: [PATCH 01/13] Add conversion webhook typings Signed-off-by: Mikail Bagishov --- kube-client/src/lib.rs | 7 +- kube-core/src/conversion/mod.rs | 6 ++ kube-core/src/conversion/types.rs | 165 ++++++++++++++++++++++++++++++ kube-core/src/lib.rs | 2 + kube-core/src/response.rs | 75 ++++++++++++-- 5 files changed, 242 insertions(+), 13 deletions(-) create mode 100644 kube-core/src/conversion/mod.rs create mode 100644 kube-core/src/conversion/types.rs diff --git a/kube-client/src/lib.rs b/kube-client/src/lib.rs index a256f1bb4..4d4aa2658 100644 --- a/kube-client/src/lib.rs +++ b/kube-client/src/lib.rs @@ -137,7 +137,10 @@ mod test { }; use futures::{StreamExt, TryStreamExt}; use k8s_openapi::api::core::v1::Pod; - use kube_core::params::{DeleteParams, Patch}; + use kube_core::{ + params::{DeleteParams, Patch}, + response::StatusSummary, + }; use serde_json::json; use tower::ServiceBuilder; @@ -475,7 +478,7 @@ mod test { let ep = EvictParams::default(); let eres = pods.evict("busybox-kube3", &ep).await?; assert_eq!(eres.code, 201); // created - assert_eq!(eres.status, "Success"); + assert_eq!(eres.status, Some(StatusSummary::Success)); Ok(()) } diff --git a/kube-core/src/conversion/mod.rs b/kube-core/src/conversion/mod.rs new file mode 100644 index 000000000..1561b2604 --- /dev/null +++ b/kube-core/src/conversion/mod.rs @@ -0,0 +1,6 @@ +//! Contains types useful for implementing custom resource conversion webhooks. + +pub use self::types::{ConversionRequest, ConversionResponse, ConversionReview, ConvertConversionReviewError}; + +/// Defines low-level typings. +mod types; diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs new file mode 100644 index 000000000..92fdaab0c --- /dev/null +++ b/kube-core/src/conversion/types.rs @@ -0,0 +1,165 @@ +use crate::{Status, TypeMeta}; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +/// The `kind` field in [`TypeMeta`]. +pub const META_KIND: &str = "ConversionReview"; +/// The `api_version` field in [`TypeMeta`] on the v1 version. +pub const META_API_VERSION_V1: &str = "apiextensions.k8s.io/v1"; + +#[derive(Debug, Error)] +#[error("request missing in ConversionReview")] +/// Failed to convert `AdmissionReview` into `AdmissionRequest`. +pub struct ConvertConversionReviewError; + +/// Struct that describes both request and response. +#[derive(Serialize, Deserialize)] +pub struct ConversionReview { + /// Contains the API version and type of the request. + #[serde(flatten)] + pub types: TypeMeta, + /// Contains conversion request. + #[serde(skip_serializing_if = "Option::is_none")] + pub request: Option, + /// Contains conversion response. + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] + pub response: Option, +} + +/// Part of ConversionReview which is set on input (i.e. generated by apiserver) +#[derive(Serialize, Deserialize)] +pub struct ConversionRequest { + /// Copied from the corresponding consructing [`ConversionReview`]. + /// This field is not part of the Kubernetes API, it's consumed only by `kube`. + #[serde(skip)] + pub types: Option, + /// Random uid uniquely identifying this conversion call + pub uid: String, + /// The API group and version the objects should be converted to + #[serde(rename = "desiredAPIVersion")] + pub desired_api_version: String, + /// The list of objects to convert. + /// May contain one or more objects, in one or more versions. + // This field used raw Value instead of Object/DynamicObject to simplify + // further downcasting. + pub objects: Vec, +} + +impl ConversionRequest { + /// Extracts request from the [`ConversionReview`]. + /// Fails if review has missing request. + pub fn from_review(review: ConversionReview) -> Result { + match review.request { + Some(mut req) => { + req.types = Some(review.types); + Ok(req) + } + None => Err(ConvertConversionReviewError), + } + } +} + +impl TryFrom for ConversionRequest { + type Error = ConvertConversionReviewError; + + fn try_from(review: ConversionReview) -> Result { + ConversionRequest::from_review(review) + } +} + +/// Part of ConversionReview which is set on output (i.e. generated by conversion webhook) +#[derive(Serialize, Deserialize)] +pub struct ConversionResponse { + /// Copied from the corresponding consructing [`ConversionRequest`]. + /// This field is not part of the Kubernetes API, it's consumed only by `kube`. + #[serde(skip)] + pub types: Option, + /// Copy of .request.uid + pub uid: String, + /// Outcome of the conversion operation. + /// Success: all objects were successfully converted + /// Failure: at least one object could not be converted. + /// It is recommended that conversion fails as rare as possible. + pub result: Status, + /// Converted objects in the same order as in the request. Should be empty + /// if conversion failed. + #[serde(rename = "convertedObjects")] + #[serde(skip_serializing_if = "Option::is_none")] + pub converted_objects: Option>, +} + +impl ConversionResponse { + /// Creates a new response, matching provided request. + /// Returned response is empty: you should call `success` or `error` + /// to get a complete `ConversionResponse`. + pub fn for_request(request: ConversionRequest) -> Self { + ConversionResponse { + types: request.types, + uid: request.uid, + result: Status { + status: None, + code: 0, + message: String::new(), + reason: String::new(), + details: None, + }, + converted_objects: None, + } + } + + /// Creates successful conversion response. + /// `converted_objects` must specify objects in the exact same order as on input. + pub fn success(mut self, converted_objects: Vec) -> Self { + self.result = Status::success(); + self.converted_objects = Some(converted_objects); + self + } + + /// Creates failed conversion response (discouraged). + /// `request_uid` must be equal to the `.uid` field in the request. + /// `message` and `reason` will be returned to the apiserver. + pub fn error(mut self, message: &str, reason: &str) -> Self { + self.result = Status::failure(message, reason); + self + } + + /// Creates failed conversion response, not matched with any request. + /// You should only call this function when request couldn't be parsed into [`ConversionRequest`]. + /// Otherwise use `error`. + pub fn unmatched_error(message: &str, reason: &str) -> Self { + ConversionResponse { + types: None, + uid: String::new(), + result: Status::failure(message, reason), + converted_objects: None, + } + } + + /// Converts response into a [`ConversionReview`] value, ready to be sent as a response. + pub fn into_review(mut self) -> ConversionReview { + ConversionReview { + types: self.types.take().unwrap_or_else(|| { + // we don't know which uid, apiVersion and kind to use, let's just use something + TypeMeta { + api_version: META_API_VERSION_V1.to_string(), + kind: META_KIND.to_string(), + } + }), + request: None, + response: Some(self), + } + } +} + +impl From for ConversionResponse { + fn from(request: ConversionRequest) -> Self { + ConversionResponse::for_request(request) + } +} + +impl From for ConversionReview { + fn from(response: ConversionResponse) -> Self { + response.into_review() + } +} diff --git a/kube-core/src/lib.rs b/kube-core/src/lib.rs index f19474697..71b81b917 100644 --- a/kube-core/src/lib.rs +++ b/kube-core/src/lib.rs @@ -14,6 +14,8 @@ #[cfg(feature = "admission")] pub mod admission; +pub mod conversion; + pub mod discovery; pub mod dynamic; diff --git a/kube-core/src/response.rs b/kube-core/src/response.rs index cb55640b6..50f4a3a4f 100644 --- a/kube-core/src/response.rs +++ b/kube-core/src/response.rs @@ -1,20 +1,20 @@ //! Generic api response types -use serde::Deserialize; +use serde::{Deserialize, Serialize}; /// A Kubernetes status object /// /// Equivalent to Status in k8s-openapi except we have have simplified options -#[derive(Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug)] pub struct Status { - /// Suggested HTTP return code (0 if unset) - #[serde(default, skip_serializing_if = "num::Zero::is_zero")] - pub code: u16, - /// Status of the operation /// /// One of: `Success` or `Failure` - [more info](https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status) - #[serde(default, skip_serializing_if = "String::is_empty")] - pub status: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub status: Option, + + /// Suggested HTTP return code (0 if unset) + #[serde(default, skip_serializing_if = "is_u16_zero")] + pub code: u16, /// A human-readable description of the status of this operation #[serde(default, skip_serializing_if = "String::is_empty")] @@ -35,8 +35,53 @@ pub struct Status { pub details: Option, } +impl Status { + /// Returns a successful `Status` + pub fn success() -> Self { + Status { + status: Some(StatusSummary::Success), + code: 0, + message: String::new(), + reason: String::new(), + details: None, + } + } + + /// Returns an unsuccessful `Status` + pub fn failure(message: &str, reason: &str) -> Self { + Status { + status: Some(StatusSummary::Failure), + code: 0, + message: message.to_string(), + reason: reason.to_string(), + details: None, + } + } + + /// Sets an explicit HTTP status code + pub fn with_code(mut self, code: u16) -> Self { + self.code = code; + self + } + + /// Adds details to the `Status` + pub fn with_details(mut self, details: StatusDetails) -> Self { + self.details = Some(details); + self + } +} + +/// Overall status of the operation - whether it succeeded or not +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +pub enum StatusSummary { + /// Operation succeeded + Success, + /// Operation failed + Failure, +} + /// Status details object on the [`Status`] object -#[derive(Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug)] #[serde(rename_all = "camelCase")] pub struct StatusDetails { /// The name attribute of the resource associated with the status StatusReason (when there is a single name which can be described) @@ -69,12 +114,12 @@ pub struct StatusDetails { /// /// Some errors may indicate the client must take an alternate action - /// for those errors this field may indicate how long to wait before taking the alternate action. - #[serde(default, skip_serializing_if = "num::Zero::is_zero")] + #[serde(default, skip_serializing_if = "is_u32_zero")] pub retry_after_seconds: u32, } /// Status cause object on the [`StatusDetails`] object -#[derive(Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug)] pub struct StatusCause { /// A machine-readable description of the cause of the error. If this value is empty there is no information available. #[serde(default, skip_serializing_if = "String::is_empty")] @@ -92,6 +137,14 @@ pub struct StatusCause { pub field: String, } +fn is_u16_zero(&v: &u16) -> bool { + v == 0 +} + +fn is_u32_zero(&v: &u32) -> bool { + v == 0 +} + #[cfg(test)] mod test { use super::Status; From b6d6d482ac00be88b374de855503be6296eb68ec Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sun, 4 Sep 2022 17:34:11 +0300 Subject: [PATCH 02/13] Add simple test Signed-off-by: Mikail Bagishov --- .../src/conversion/test_data/simple.json | 55 +++++++++++++++++++ kube-core/src/conversion/types.rs | 13 +++++ 2 files changed, 68 insertions(+) create mode 100644 kube-core/src/conversion/test_data/simple.json diff --git a/kube-core/src/conversion/test_data/simple.json b/kube-core/src/conversion/test_data/simple.json new file mode 100644 index 000000000..bc15d875b --- /dev/null +++ b/kube-core/src/conversion/test_data/simple.json @@ -0,0 +1,55 @@ +{ + "kind": "ConversionReview", + "apiVersion": "apiextensions.k8s.io/v1", + "request": { + "uid": "f263987e-4d58-465a-9195-bf72a1c83623", + "desiredAPIVersion": "nullable.se/v1", + "objects": [ + { + "apiVersion": "nullable.se/v2", + "kind": "ConfigMapGenerator", + "metadata": { + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"nullable.se/v2\",\"kind\":\"ConfigMapGenerator\",\"metadata\":{\"annotations\":{},\"name\":\"kek\",\"namespace\":\"default\"},\"spec\":{\"content\":\"x\"}}\n" + }, + "creationTimestamp": "2022-09-04T14:21:34Z", + "generation": 1, + "managedFields": [ + { + "apiVersion": "nullable.se/v2", + "fieldsType": "FieldsV1", + "fieldsV1": { + "f:metadata": { + "f:annotations": { + ".": {}, + "f:kubectl.kubernetes.io/last-applied-configuration": {} + } + }, + "f:spec": { + ".": {}, + "f:content": {} + } + }, + "manager": "kubectl-client-side-apply", + "operation": "Update", + "time": "2022-09-04T14:21:34Z" + } + ], + "name": "kek", + "namespace": "default", + "uid": "af7e84e4-573e-4b6e-bb66-0ea578c740da" + }, + "spec": { + "content": "x" + } + } + ] + }, + "response": { + "uid": "", + "convertedObjects": null, + "result": { + "metadata": {} + } + } +} \ No newline at end of file diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index 92fdaab0c..98f12507c 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -163,3 +163,16 @@ impl From for ConversionReview { response.into_review() } } + +#[cfg(test)] +mod tests { + use super::ConversionReview; + + #[test] + fn simple_request_parses() { + // this file contains dump of real request generated by kubernetes v1.22 + let data = include_str!("./test_data/simple.json"); + // check that we can parse this request + let _: ConversionReview = serde_json::from_str(data).unwrap(); + } +} \ No newline at end of file From 6be27e143c293fd8556bb408a23db3ff17927c6d Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sun, 4 Sep 2022 17:39:47 +0300 Subject: [PATCH 03/13] Fix formatting Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/mod.rs | 4 +++- kube-core/src/conversion/types.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/kube-core/src/conversion/mod.rs b/kube-core/src/conversion/mod.rs index 1561b2604..ac0e07aa1 100644 --- a/kube-core/src/conversion/mod.rs +++ b/kube-core/src/conversion/mod.rs @@ -1,6 +1,8 @@ //! Contains types useful for implementing custom resource conversion webhooks. -pub use self::types::{ConversionRequest, ConversionResponse, ConversionReview, ConvertConversionReviewError}; +pub use self::types::{ + ConversionRequest, ConversionResponse, ConversionReview, ConvertConversionReviewError, +}; /// Defines low-level typings. mod types; diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index 98f12507c..c5827ad87 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -175,4 +175,4 @@ mod tests { // check that we can parse this request let _: ConversionReview = serde_json::from_str(data).unwrap(); } -} \ No newline at end of file +} From edad3da58c10b868f9b39f14100dfd228f42498f Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 00:12:13 +0300 Subject: [PATCH 04/13] Improve docs Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 59 ++++++++++++++++++------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index c5827ad87..3faf1c260 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -2,26 +2,26 @@ use crate::{Status, TypeMeta}; use serde::{Deserialize, Serialize}; use thiserror::Error; -/// The `kind` field in [`TypeMeta`]. +/// The `kind` field in [`TypeMeta`] pub const META_KIND: &str = "ConversionReview"; -/// The `api_version` field in [`TypeMeta`] on the v1 version. +/// The `api_version` field in [`TypeMeta`] on the v1 version pub const META_API_VERSION_V1: &str = "apiextensions.k8s.io/v1"; #[derive(Debug, Error)] #[error("request missing in ConversionReview")] -/// Failed to convert `AdmissionReview` into `AdmissionRequest`. +/// Returned when `AdmissionReview` cannot be converted into `AdmissionRequest` pub struct ConvertConversionReviewError; -/// Struct that describes both request and response. +/// Struct that describes both request and response #[derive(Serialize, Deserialize)] pub struct ConversionReview { - /// Contains the API version and type of the request. + /// Contains the API version and type of the request #[serde(flatten)] pub types: TypeMeta, - /// Contains conversion request. + /// Contains conversion request #[serde(skip_serializing_if = "Option::is_none")] pub request: Option, - /// Contains conversion response. + /// Contains conversion response #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] pub response: Option, @@ -30,8 +30,10 @@ pub struct ConversionReview { /// Part of ConversionReview which is set on input (i.e. generated by apiserver) #[derive(Serialize, Deserialize)] pub struct ConversionRequest { - /// Copied from the corresponding consructing [`ConversionReview`]. - /// This field is not part of the Kubernetes API, it's consumed only by `kube`. + /// [`TypeMeta`] of the [`ConversionReview`] this response was created from + /// + /// This field dopied from the corresponding [`ConversionReview`]. + /// It is not part of the Kubernetes API, it's consumed only by `kube`. #[serde(skip)] pub types: Option, /// Random uid uniquely identifying this conversion call @@ -39,16 +41,16 @@ pub struct ConversionRequest { /// The API group and version the objects should be converted to #[serde(rename = "desiredAPIVersion")] pub desired_api_version: String, - /// The list of objects to convert. - /// May contain one or more objects, in one or more versions. - // This field used raw Value instead of Object/DynamicObject to simplify + /// The list of objects to convert + /// + /// Note that list may contain one or more objects, in one or more versions. + // This field uses raw Value instead of Object/DynamicObject to simplify // further downcasting. pub objects: Vec, } impl ConversionRequest { - /// Extracts request from the [`ConversionReview`]. - /// Fails if review has missing request. + /// Extracts request from the [`ConversionReview`] pub fn from_review(review: ConversionReview) -> Result { match review.request { Some(mut req) => { @@ -71,26 +73,32 @@ impl TryFrom for ConversionRequest { /// Part of ConversionReview which is set on output (i.e. generated by conversion webhook) #[derive(Serialize, Deserialize)] pub struct ConversionResponse { - /// Copied from the corresponding consructing [`ConversionRequest`]. - /// This field is not part of the Kubernetes API, it's consumed only by `kube`. + /// [`TypeMeta`] of the [`ConversionReview`] this response was derived from + /// + /// This field is copied from the corresponding [`ConversionRequest`]. + /// It is not part of the Kubernetes API, it's consumed only by `kube`. #[serde(skip)] pub types: Option, /// Copy of .request.uid pub uid: String, - /// Outcome of the conversion operation. + /// Outcome of the conversion operation + /// /// Success: all objects were successfully converted /// Failure: at least one object could not be converted. /// It is recommended that conversion fails as rare as possible. pub result: Status, - /// Converted objects in the same order as in the request. Should be empty - /// if conversion failed. + /// Converted objects + /// + /// This field should contain objects in the same order as in the request + /// Should be empty if conversion failed. #[serde(rename = "convertedObjects")] #[serde(skip_serializing_if = "Option::is_none")] pub converted_objects: Option>, } impl ConversionResponse { - /// Creates a new response, matching provided request. + /// Creates a new response, matching provided request + /// /// Returned response is empty: you should call `success` or `error` /// to get a complete `ConversionResponse`. pub fn for_request(request: ConversionRequest) -> Self { @@ -108,7 +116,8 @@ impl ConversionResponse { } } - /// Creates successful conversion response. + /// Creates successful conversion response + /// /// `converted_objects` must specify objects in the exact same order as on input. pub fn success(mut self, converted_objects: Vec) -> Self { self.result = Status::success(); @@ -116,7 +125,8 @@ impl ConversionResponse { self } - /// Creates failed conversion response (discouraged). + /// Creates failed conversion response (discouraged) + /// /// `request_uid` must be equal to the `.uid` field in the request. /// `message` and `reason` will be returned to the apiserver. pub fn error(mut self, message: &str, reason: &str) -> Self { @@ -124,7 +134,8 @@ impl ConversionResponse { self } - /// Creates failed conversion response, not matched with any request. + /// Creates failed conversion response, not matched with any request + /// /// You should only call this function when request couldn't be parsed into [`ConversionRequest`]. /// Otherwise use `error`. pub fn unmatched_error(message: &str, reason: &str) -> Self { @@ -136,7 +147,7 @@ impl ConversionResponse { } } - /// Converts response into a [`ConversionReview`] value, ready to be sent as a response. + /// Converts response into a [`ConversionReview`] value, ready to be sent as a response pub fn into_review(mut self) -> ConversionReview { ConversionReview { types: self.types.take().unwrap_or_else(|| { From eaa34e49757e40563385acb1813880665393fd2c Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 00:46:58 +0300 Subject: [PATCH 05/13] Add is_success & is_failure methods Signed-off-by: Mikail Bagishov --- kube-client/src/lib.rs | 2 +- kube-core/src/response.rs | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/kube-client/src/lib.rs b/kube-client/src/lib.rs index 4d4aa2658..e79b7009e 100644 --- a/kube-client/src/lib.rs +++ b/kube-client/src/lib.rs @@ -478,7 +478,7 @@ mod test { let ep = EvictParams::default(); let eres = pods.evict("busybox-kube3", &ep).await?; assert_eq!(eres.code, 201); // created - assert_eq!(eres.status, Some(StatusSummary::Success)); + assert!(eres.is_success()); Ok(()) } diff --git a/kube-core/src/response.rs b/kube-core/src/response.rs index 50f4a3a4f..cd67b9114 100644 --- a/kube-core/src/response.rs +++ b/kube-core/src/response.rs @@ -2,8 +2,6 @@ use serde::{Deserialize, Serialize}; /// A Kubernetes status object -/// -/// Equivalent to Status in k8s-openapi except we have have simplified options #[derive(Serialize, Deserialize, Debug)] pub struct Status { /// Status of the operation @@ -69,6 +67,22 @@ impl Status { self.details = Some(details); self } + + /// Checks if this `Status` represents success + /// + /// Note that it is possible for `Status` to be in indeterminate state + /// when both `is_success` and `is_failure` return false. + pub fn is_success(&self) -> bool { + self.status == Some(StatusSummary::Success) + } + + /// Checks if this `Status` represents failure + /// + /// Note that it is possible for `Status` to be in indeterminate state + /// when both `is_success` and `is_failure` return false. + pub fn is_failure(&self) -> bool { + self.status == Some(StatusSummary::Failure) + } } /// Overall status of the operation - whether it succeeded or not From 1012bdc76faa18647c1dbefe42455dd1dfb86a0f Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 01:00:37 +0300 Subject: [PATCH 06/13] Use our Status in admission Signed-off-by: Mikail Bagishov --- kube-core/src/admission.rs | 13 ++++--------- kube-core/src/response.rs | 8 ++++---- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/kube-core/src/admission.rs b/kube-core/src/admission.rs index 7c4b2cbc2..a75371ebb 100644 --- a/kube-core/src/admission.rs +++ b/kube-core/src/admission.rs @@ -10,14 +10,12 @@ use crate::{ gvk::{GroupVersionKind, GroupVersionResource}, metadata::TypeMeta, resource::Resource, + Status, }; use std::collections::HashMap; -use k8s_openapi::{ - api::authentication::v1::UserInfo, - apimachinery::pkg::{apis::meta::v1::Status, runtime::RawExtension}, -}; +use k8s_openapi::{api::authentication::v1::UserInfo, apimachinery::pkg::runtime::RawExtension}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -307,10 +305,7 @@ impl AdmissionResponse { }, uid: Default::default(), allowed: false, - result: Status { - reason: Some(reason.to_string()), - ..Default::default() - }, + result: Status::failure("", &reason.to_string()), patch: None, patch_type: None, audit_annotations: Default::default(), @@ -322,7 +317,7 @@ impl AdmissionResponse { #[must_use] pub fn deny(mut self, reason: T) -> Self { self.allowed = false; - self.result.message = Some(reason.to_string()); + self.result.message = reason.to_string(); self } diff --git a/kube-core/src/response.rs b/kube-core/src/response.rs index cd67b9114..d6fd7b796 100644 --- a/kube-core/src/response.rs +++ b/kube-core/src/response.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; /// A Kubernetes status object -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq)] pub struct Status { /// Status of the operation /// @@ -86,7 +86,7 @@ impl Status { } /// Overall status of the operation - whether it succeeded or not -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)] pub enum StatusSummary { /// Operation succeeded Success, @@ -95,7 +95,7 @@ pub enum StatusSummary { } /// Status details object on the [`Status`] object -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(rename_all = "camelCase")] pub struct StatusDetails { /// The name attribute of the resource associated with the status StatusReason (when there is a single name which can be described) @@ -133,7 +133,7 @@ pub struct StatusDetails { } /// Status cause object on the [`StatusDetails`] object -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] pub struct StatusCause { /// A machine-readable description of the cause of the error. If this value is empty there is no information available. #[serde(default, skip_serializing_if = "String::is_empty")] From a3d36ecf9ad8de6e6638f566575c558d5839a6cc Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 23:09:53 +0300 Subject: [PATCH 07/13] Expand test Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index 3faf1c260..85416cd2c 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -177,13 +177,16 @@ impl From for ConversionReview { #[cfg(test)] mod tests { - use super::ConversionReview; + use super::{ConversionRequest, ConversionResponse}; #[test] fn simple_request_parses() { // this file contains dump of real request generated by kubernetes v1.22 let data = include_str!("./test_data/simple.json"); - // check that we can parse this request - let _: ConversionReview = serde_json::from_str(data).unwrap(); + // check that we can parse this review, and all chain of conversion worls + let review = serde_json::from_str(data).unwrap(); + let req = ConversionRequest::from_review(review).unwrap(); + let res = ConversionResponse::for_request(req); + let _ = res.into_review(); } } From 307f4c8952b8a4d574fb9ae4ab88ce079db2edd5 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 23:16:47 +0300 Subject: [PATCH 08/13] Rename response methods Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index 85416cd2c..ee7c1c332 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -99,8 +99,9 @@ pub struct ConversionResponse { impl ConversionResponse { /// Creates a new response, matching provided request /// - /// Returned response is empty: you should call `success` or `error` - /// to get a complete `ConversionResponse`. + /// This response must be finalized with one of: + /// - [`ConversionResponse::success`] when conversion succeeded + /// - [`ConversionResponse::failure`] when conversion failed pub fn for_request(request: ConversionRequest) -> Self { ConversionResponse { types: request.types, @@ -129,7 +130,7 @@ impl ConversionResponse { /// /// `request_uid` must be equal to the `.uid` field in the request. /// `message` and `reason` will be returned to the apiserver. - pub fn error(mut self, message: &str, reason: &str) -> Self { + pub fn failure(mut self, message: &str, reason: &str) -> Self { self.result = Status::failure(message, reason); self } @@ -138,7 +139,7 @@ impl ConversionResponse { /// /// You should only call this function when request couldn't be parsed into [`ConversionRequest`]. /// Otherwise use `error`. - pub fn unmatched_error(message: &str, reason: &str) -> Self { + pub fn invalid(message: &str, reason: &str) -> Self { ConversionResponse { types: None, uid: String::new(), From 1b2cef4d1bfaee8933b6fb5896ecca373e0a7d06 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 23:37:11 +0300 Subject: [PATCH 09/13] Avoid optionality Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index ee7c1c332..1230a1144 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -1,5 +1,5 @@ use crate::{Status, TypeMeta}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use thiserror::Error; /// The `kind` field in [`TypeMeta`] @@ -92,8 +92,27 @@ pub struct ConversionResponse { /// This field should contain objects in the same order as in the request /// Should be empty if conversion failed. #[serde(rename = "convertedObjects")] - #[serde(skip_serializing_if = "Option::is_none")] - pub converted_objects: Option>, + #[serde(deserialize_with = "parse_converted_objects")] + pub converted_objects: Vec, +} + +fn parse_converted_objects<'de, D>(de: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + #[derive(Deserialize)] + #[serde(untagged)] + enum Helper { + List(Vec), + Null(()), + } + + let h: Helper = Helper::deserialize(de)?; + let res = match h { + Helper::List(l) => l, + Helper::Null(()) => Vec::new(), + }; + Ok(res) } impl ConversionResponse { @@ -113,7 +132,7 @@ impl ConversionResponse { reason: String::new(), details: None, }, - converted_objects: None, + converted_objects: Vec::new(), } } @@ -122,7 +141,7 @@ impl ConversionResponse { /// `converted_objects` must specify objects in the exact same order as on input. pub fn success(mut self, converted_objects: Vec) -> Self { self.result = Status::success(); - self.converted_objects = Some(converted_objects); + self.converted_objects = converted_objects; self } @@ -144,7 +163,7 @@ impl ConversionResponse { types: None, uid: String::new(), result: Status::failure(message, reason), - converted_objects: None, + converted_objects: Vec::new(), } } From ff5077748588221011d4b742749cdded2453ce0e Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Tue, 6 Sep 2022 23:50:20 +0300 Subject: [PATCH 10/13] Move some code around Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 70 +++++++++++++++---------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index 1230a1144..ac401017c 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -52,13 +52,7 @@ pub struct ConversionRequest { impl ConversionRequest { /// Extracts request from the [`ConversionReview`] pub fn from_review(review: ConversionReview) -> Result { - match review.request { - Some(mut req) => { - req.types = Some(review.types); - Ok(req) - } - None => Err(ConvertConversionReviewError), - } + ConversionRequest::try_from(review) } } @@ -66,7 +60,13 @@ impl TryFrom for ConversionRequest { type Error = ConvertConversionReviewError; fn try_from(review: ConversionReview) -> Result { - ConversionRequest::from_review(review) + match review.request { + Some(mut req) => { + req.types = Some(review.types); + Ok(req) + } + None => Err(ConvertConversionReviewError), + } } } @@ -122,18 +122,7 @@ impl ConversionResponse { /// - [`ConversionResponse::success`] when conversion succeeded /// - [`ConversionResponse::failure`] when conversion failed pub fn for_request(request: ConversionRequest) -> Self { - ConversionResponse { - types: request.types, - uid: request.uid, - result: Status { - status: None, - code: 0, - message: String::new(), - reason: String::new(), - details: None, - }, - converted_objects: Vec::new(), - } + ConversionResponse::from(request) } /// Creates successful conversion response @@ -168,30 +157,41 @@ impl ConversionResponse { } /// Converts response into a [`ConversionReview`] value, ready to be sent as a response - pub fn into_review(mut self) -> ConversionReview { - ConversionReview { - types: self.types.take().unwrap_or_else(|| { - // we don't know which uid, apiVersion and kind to use, let's just use something - TypeMeta { - api_version: META_API_VERSION_V1.to_string(), - kind: META_KIND.to_string(), - } - }), - request: None, - response: Some(self), - } + pub fn into_review(self) -> ConversionReview { + self.into() } } impl From for ConversionResponse { fn from(request: ConversionRequest) -> Self { - ConversionResponse::for_request(request) + ConversionResponse { + types: request.types, + uid: request.uid, + result: Status { + status: None, + code: 0, + message: String::new(), + reason: String::new(), + details: None, + }, + converted_objects: Vec::new(), + } } } impl From for ConversionReview { - fn from(response: ConversionResponse) -> Self { - response.into_review() + fn from(mut response: ConversionResponse) -> Self { + ConversionReview { + types: response.types.take().unwrap_or_else(|| { + // we don't know which uid, apiVersion and kind to use, let's just use something + TypeMeta { + api_version: META_API_VERSION_V1.to_string(), + kind: META_KIND.to_string(), + } + }), + request: None, + response: Some(response), + } } } From 68748d70e0b4e1a08a179192d6a6f24faf2edf54 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sat, 10 Sep 2022 23:48:41 +0300 Subject: [PATCH 11/13] Fix copy-paste error Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index ac401017c..fca2fea8b 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -9,7 +9,7 @@ pub const META_API_VERSION_V1: &str = "apiextensions.k8s.io/v1"; #[derive(Debug, Error)] #[error("request missing in ConversionReview")] -/// Returned when `AdmissionReview` cannot be converted into `AdmissionRequest` +/// Returned when `ConversionReview` cannot be converted into `ConversionRequest` pub struct ConvertConversionReviewError; /// Struct that describes both request and response From adf736b0ae03d3a9a81ae1b6169ce7986e54dcd8 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sat, 10 Sep 2022 23:51:53 +0300 Subject: [PATCH 12/13] Improve response signatures Signed-off-by: Mikail Bagishov --- kube-core/src/conversion/types.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kube-core/src/conversion/types.rs b/kube-core/src/conversion/types.rs index fca2fea8b..b26fabb3f 100644 --- a/kube-core/src/conversion/types.rs +++ b/kube-core/src/conversion/types.rs @@ -138,8 +138,8 @@ impl ConversionResponse { /// /// `request_uid` must be equal to the `.uid` field in the request. /// `message` and `reason` will be returned to the apiserver. - pub fn failure(mut self, message: &str, reason: &str) -> Self { - self.result = Status::failure(message, reason); + pub fn failure(mut self, status: Status) -> Self { + self.result = status; self } @@ -147,11 +147,11 @@ impl ConversionResponse { /// /// You should only call this function when request couldn't be parsed into [`ConversionRequest`]. /// Otherwise use `error`. - pub fn invalid(message: &str, reason: &str) -> Self { + pub fn invalid(status: Status) -> Self { ConversionResponse { types: None, uid: String::new(), - result: Status::failure(message, reason), + result: status, converted_objects: Vec::new(), } } From 4d1a80180a7ea12f6909d30cd096138e1078b168 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Fri, 16 Sep 2022 00:48:41 +0300 Subject: [PATCH 13/13] Change status in invalid admission response Signed-off-by: Mikail Bagishov --- kube-core/src/admission.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kube-core/src/admission.rs b/kube-core/src/admission.rs index a75371ebb..b72e27dba 100644 --- a/kube-core/src/admission.rs +++ b/kube-core/src/admission.rs @@ -305,7 +305,7 @@ impl AdmissionResponse { }, uid: Default::default(), allowed: false, - result: Status::failure("", &reason.to_string()), + result: Status::failure(&reason.to_string(), "InvalidRequest"), patch: None, patch_type: None, audit_annotations: Default::default(),