From 64345dda6d6ef5119f6448872d181130555ed469 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Tue, 2 Aug 2022 11:56:39 +0200 Subject: [PATCH 1/3] Ensure that explicit YAML documents are generated --- CHANGELOG.md | 6 +++ Cargo.toml | 3 +- src/cli.rs | 13 +++---- src/commons/s3.rs | 12 +++--- src/crd.rs | 11 +++++- src/error.rs | 3 ++ src/lib.rs | 1 + src/yaml.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 128 insertions(+), 16 deletions(-) create mode 100644 src/yaml.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a56fce55..8676d6f07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,13 +8,19 @@ All notable changes to this project will be documented in this file. - Cluster resources can be added to a struct which determines the orphaned resources and deletes them ([#436]). +- YAML module added with a function to serialize a data structure as an + explicit YAML document. The YAML documents generated by the functions in + `crd::CustomResourceExt` are now explicit documents and can be safely + concatenated to produce a YAML stream ([#450]). ### Changed - BREAKING: The `managed_by` label must be passed explicitly to the `ObjectMetaBuilder::with_recommended_labels` function ([#436]). +- serde\_yaml 0.8.26 -> 0.9 ([#450]) [#436]: https://github.com/stackabletech/operator-rs/pull/436 +[#450]: https://github.com/stackabletech/operator-rs/pull/450 ## [0.23.0] - 2022-07-26 diff --git a/Cargo.toml b/Cargo.toml index 46cd1b03b..db5a0748f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ regex = "1.6.0" schemars = "0.8.10" serde = { version = "1.0.140", features = ["derive"] } serde_json = "1.0.82" -serde_yaml = "0.8.26" +serde_yaml = "0.9" strum = { version = "0.24.1", features = ["derive"] } thiserror = "1.0.31" tokio = { version = "1.20.1", features = ["macros", "rt-multi-thread"] } @@ -39,7 +39,6 @@ stackable-operator-derive = { path = "stackable-operator-derive" } [dev-dependencies] rstest = "0.15.0" tempfile = "3.3.0" -serde_yaml = "0.8" [features] default = ["native-tls"] diff --git a/src/cli.rs b/src/cli.rs index 4205cbf93..8106460bb 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -11,10 +11,10 @@ //! ```no_run //! // Handle CLI arguments //! use clap::{crate_version, Parser}; -//! use kube::{CustomResource, CustomResourceExt}; +//! use kube::CustomResource; //! use schemars::JsonSchema; //! use serde::{Deserialize, Serialize}; -//! use stackable_operator::cli; +//! use stackable_operator::{CustomResourceExt, cli}; //! use stackable_operator::error::OperatorResult; //! //! #[derive(Clone, CustomResource, Debug, JsonSchema, Serialize, Deserialize)] @@ -55,11 +55,10 @@ //! let opts = Opts::from_args(); //! //! match opts.command { -//! cli::Command::Crd => println!( -//! "{}{}", -//! serde_yaml::to_string(&FooCluster::crd())?, -//! serde_yaml::to_string(&BarCluster::crd())?, -//! ), +//! cli::Command::Crd => { +//! FooCluster::print_yaml_schema()?; +//! BarCluster::print_yaml_schema()?; +//! }, //! cli::Command::Run { .. } => { //! // Run the operator //! } diff --git a/src/commons/s3.rs b/src/commons/s3.rs index 5e328d5a9..a61fc5789 100644 --- a/src/commons/s3.rs +++ b/src/commons/s3.rs @@ -220,6 +220,7 @@ impl Default for S3AccessStyle { mod test { use crate::commons::s3::{S3AccessStyle, S3ConnectionDef}; use crate::commons::s3::{S3BucketSpec, S3ConnectionSpec}; + use crate::yaml; #[test] fn test_ser_inline() { @@ -235,14 +236,13 @@ mod test { }; assert_eq!( - serde_yaml::to_string(&bucket).unwrap(), + yaml::to_explicit_document_string(&bucket).unwrap(), "--- bucketName: test-bucket-name -connection: - inline: - host: host - port: 8080 - accessStyle: VirtualHosted +connection: !inline + host: host + port: 8080 + accessStyle: VirtualHosted " .to_owned() ) diff --git a/src/crd.rs b/src/crd.rs index c71400369..760268fd2 100644 --- a/src/crd.rs +++ b/src/crd.rs @@ -5,6 +5,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use crate::error::{Error, OperatorResult}; +use crate::yaml; use std::fs::File; use std::io::Write; use std::path::Path; @@ -72,28 +73,36 @@ pub trait HasApplication { /// (e.g. creation) of `CustomResourceDefinition`s in Kubernetes. pub trait CustomResourceExt: kube::CustomResourceExt { /// Generates a YAML CustomResourceDefinition and writes it to a `Write`. + /// + /// The generated YAML string is an explicit document with leading dashes (`---`). fn generate_yaml_schema(mut writer: W) -> OperatorResult<()> where W: Write, { - let schema = serde_yaml::to_string(&Self::crd())?; + let schema = yaml::to_explicit_document_string(&Self::crd())?; writer.write_all(schema.as_bytes())?; Ok(()) } /// Generates a YAML CustomResourceDefinition and writes it to the specified file. + /// + /// The written YAML string is an explicit document with leading dashes (`---`). fn write_yaml_schema>(path: P) -> OperatorResult<()> { let writer = File::create(path)?; Self::generate_yaml_schema(writer) } /// Generates a YAML CustomResourceDefinition and prints it to stdout. + /// + /// The printed YAML string is an explicit document with leading dashes (`---`). fn print_yaml_schema() -> OperatorResult<()> { let writer = std::io::stdout(); Self::generate_yaml_schema(writer) } // Returns the YAML schema of this CustomResourceDefinition as a string. + /// + /// The written YAML string is an explicit document with leading dashes (`---`). fn yaml_schema() -> OperatorResult { let mut writer = Vec::new(); Self::generate_yaml_schema(&mut writer)?; diff --git a/src/error.rs b/src/error.rs index eb4fac511..52206ecbe 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,6 +9,9 @@ pub enum Error { source: serde_yaml::Error, }, + #[error("Failed to process YAML document: {message}")] + UnsupportedYamlDocumentError { message: String }, + #[error("Kubernetes reported error: {source}")] KubeError { #[from] diff --git a/src/lib.rs b/src/lib.rs index b9f9e4632..f993d09b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ pub mod product_config_utils; pub mod role_utils; pub mod utils; pub mod validation; +pub mod yaml; pub use crate::crd::CustomResourceExt; diff --git a/src/yaml.rs b/src/yaml.rs new file mode 100644 index 000000000..004938573 --- /dev/null +++ b/src/yaml.rs @@ -0,0 +1,95 @@ +//! Utility functions for processing data in the YAML file format +use serde::ser; + +use crate::error::{Error, OperatorResult}; + +/// A YAML document type +/// +/// For a detailled description, see the +/// [YAML specification](https://yaml.org/spec/1.2.2/#rule-l-any-document). +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum DocumentType { + DirectiveDocument, + ExplicitDocument, + BareDocument, +} + +/// Serializes the given data structure as an explicit YAML document. +/// +/// # Errors +/// +/// Serialization can fail if `T`'s implementation of `Serialize` decides to return an error. +/// +/// An [`Error::UnsupportedYamlDocumentError`] is returned if the used `serde_yaml` version +/// generates a directive document. +pub fn to_explicit_document_string(value: &T) -> OperatorResult +where + T: ?Sized + ser::Serialize, +{ + // The returned document type depends on the serde_yaml version. + let document = serde_yaml::to_string(value)?; + + match determine_document_type(&document) { + DocumentType::DirectiveDocument => Err(Error::UnsupportedYamlDocumentError { + message: "serde_yaml::to_string generated a directive document which cannot be \ + converted to an explicit document." + .into(), + }), + DocumentType::ExplicitDocument => Ok(document), + DocumentType::BareDocument => Ok(format!("---\n{}", document)), + } +} + +/// Determines the type of the given YAML document. +/// +/// It is assumend that the given string contains a valid YAML document. +pub fn determine_document_type(document: &str) -> DocumentType { + if document.starts_with('%') { + DocumentType::DirectiveDocument + } else if document.starts_with("---") { + DocumentType::ExplicitDocument + } else { + DocumentType::BareDocument + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + #[test] + fn value_can_be_serialized_to_an_explicit_document_string() { + let value: BTreeMap<_, _> = [("key", "value")].into(); + + let actual_yaml = to_explicit_document_string(&value).expect("serializable value"); + + let expected_yaml = "\ + ---\n\ + key: value\n"; + + assert_eq!(expected_yaml, actual_yaml); + } + + #[test] + fn document_type_can_be_determined() { + let directive_document = "\ + %YAML 1.2\n\ + ---\n\ + key: value"; + let explicit_document = "\ + ---\n\ + key: value"; + let bare_document = "\ + key: value"; + + let directive_document_type = determine_document_type(directive_document); + let explicit_document_type = determine_document_type(explicit_document); + let bare_document_type = determine_document_type(bare_document); + + assert_eq!(DocumentType::DirectiveDocument, directive_document_type); + assert_eq!(DocumentType::ExplicitDocument, explicit_document_type); + assert_eq!(DocumentType::BareDocument, bare_document_type); + } +} From 36720e718764995d14a963cd7c8ab4a984eb02e8 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Tue, 2 Aug 2022 15:21:09 +0200 Subject: [PATCH 2/3] Remove check for YAML document type --- src/error.rs | 3 --- src/yaml.rs | 64 ++++------------------------------------------------ 2 files changed, 4 insertions(+), 63 deletions(-) diff --git a/src/error.rs b/src/error.rs index 52206ecbe..eb4fac511 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,9 +9,6 @@ pub enum Error { source: serde_yaml::Error, }, - #[error("Failed to process YAML document: {message}")] - UnsupportedYamlDocumentError { message: String }, - #[error("Kubernetes reported error: {source}")] KubeError { #[from] diff --git a/src/yaml.rs b/src/yaml.rs index 004938573..d45b95eb6 100644 --- a/src/yaml.rs +++ b/src/yaml.rs @@ -1,56 +1,21 @@ //! Utility functions for processing data in the YAML file format use serde::ser; -use crate::error::{Error, OperatorResult}; - -/// A YAML document type -/// -/// For a detailled description, see the -/// [YAML specification](https://yaml.org/spec/1.2.2/#rule-l-any-document). -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum DocumentType { - DirectiveDocument, - ExplicitDocument, - BareDocument, -} +use crate::error::OperatorResult; /// Serializes the given data structure as an explicit YAML document. /// /// # Errors /// /// Serialization can fail if `T`'s implementation of `Serialize` decides to return an error. -/// -/// An [`Error::UnsupportedYamlDocumentError`] is returned if the used `serde_yaml` version -/// generates a directive document. pub fn to_explicit_document_string(value: &T) -> OperatorResult where T: ?Sized + ser::Serialize, { - // The returned document type depends on the serde_yaml version. - let document = serde_yaml::to_string(value)?; + let bare_document = serde_yaml::to_string(value)?; + let explicit_document = format!("---\n{}", bare_document); - match determine_document_type(&document) { - DocumentType::DirectiveDocument => Err(Error::UnsupportedYamlDocumentError { - message: "serde_yaml::to_string generated a directive document which cannot be \ - converted to an explicit document." - .into(), - }), - DocumentType::ExplicitDocument => Ok(document), - DocumentType::BareDocument => Ok(format!("---\n{}", document)), - } -} - -/// Determines the type of the given YAML document. -/// -/// It is assumend that the given string contains a valid YAML document. -pub fn determine_document_type(document: &str) -> DocumentType { - if document.starts_with('%') { - DocumentType::DirectiveDocument - } else if document.starts_with("---") { - DocumentType::ExplicitDocument - } else { - DocumentType::BareDocument - } + Ok(explicit_document) } #[cfg(test)] @@ -71,25 +36,4 @@ mod tests { assert_eq!(expected_yaml, actual_yaml); } - - #[test] - fn document_type_can_be_determined() { - let directive_document = "\ - %YAML 1.2\n\ - ---\n\ - key: value"; - let explicit_document = "\ - ---\n\ - key: value"; - let bare_document = "\ - key: value"; - - let directive_document_type = determine_document_type(directive_document); - let explicit_document_type = determine_document_type(explicit_document); - let bare_document_type = determine_document_type(bare_document); - - assert_eq!(DocumentType::DirectiveDocument, directive_document_type); - assert_eq!(DocumentType::ExplicitDocument, explicit_document_type); - assert_eq!(DocumentType::BareDocument, bare_document_type); - } } From 44edca4fd09fe97a33fb02223aa7e67d3be3dcc1 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 22 Aug 2022 12:03:36 +0200 Subject: [PATCH 3/3] Serialize enums as YAML maps containing one entry in which the key identifies the variant name --- CHANGELOG.md | 2 +- Cargo.toml | 2 +- src/commons/s3.rs | 25 +++++++++------- src/crd.rs | 6 ++-- src/yaml.rs | 74 ++++++++++++++++++++++++++++++----------------- 5 files changed, 66 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd751cda7..fe0ed7c61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to this project will be documented in this file. ### Changed - Objects are now streamed rather than polled when waiting for them to be deleted ([#452]). -- serde\_yaml 0.8.26 -> 0.9 ([#450]) +- serde\_yaml 0.8.26 -> 0.9.9 ([#450]) [#450]: https://github.com/stackabletech/operator-rs/pull/450 [#452]: https://github.com/stackabletech/operator-rs/pull/452 diff --git a/Cargo.toml b/Cargo.toml index 5d5341a1f..6e336ddec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ regex = "1.6.0" schemars = "0.8.10" serde = { version = "1.0.140", features = ["derive"] } serde_json = "1.0.82" -serde_yaml = "0.9" +serde_yaml = "0.9.9" strum = { version = "0.24.1", features = ["derive"] } thiserror = "1.0.31" tokio = { version = "1.20.1", features = ["macros", "rt-multi-thread"] } diff --git a/src/commons/s3.rs b/src/commons/s3.rs index eb60da670..9361466a1 100644 --- a/src/commons/s3.rs +++ b/src/commons/s3.rs @@ -222,6 +222,8 @@ impl Default for S3AccessStyle { #[cfg(test)] mod test { + use std::str; + use crate::commons::s3::{S3AccessStyle, S3ConnectionDef}; use crate::commons::s3::{S3BucketSpec, S3ConnectionSpec}; use crate::yaml; @@ -239,16 +241,19 @@ mod test { })), }; - assert_eq!( - yaml::to_explicit_document_string(&bucket).unwrap(), - "--- + let mut buf = Vec::new(); + yaml::serialize_to_explicit_document(&mut buf, &bucket).expect("serializable value"); + let actual_yaml = str::from_utf8(&buf).expect("UTF-8 encoded document"); + + let expected_yaml = "--- bucketName: test-bucket-name -connection: !inline - host: host - port: 8080 - accessStyle: VirtualHosted -" - .to_owned() - ) +connection: + inline: + host: host + port: 8080 + accessStyle: VirtualHosted +"; + + assert_eq!(expected_yaml, actual_yaml) } } diff --git a/src/crd.rs b/src/crd.rs index 760268fd2..dbdb6cd7d 100644 --- a/src/crd.rs +++ b/src/crd.rs @@ -79,9 +79,7 @@ pub trait CustomResourceExt: kube::CustomResourceExt { where W: Write, { - let schema = yaml::to_explicit_document_string(&Self::crd())?; - writer.write_all(schema.as_bytes())?; - Ok(()) + yaml::serialize_to_explicit_document(&mut writer, &Self::crd()) } /// Generates a YAML CustomResourceDefinition and writes it to the specified file. @@ -100,7 +98,7 @@ pub trait CustomResourceExt: kube::CustomResourceExt { Self::generate_yaml_schema(writer) } - // Returns the YAML schema of this CustomResourceDefinition as a string. + /// Returns the YAML schema of this CustomResourceDefinition as a string. /// /// The written YAML string is an explicit document with leading dashes (`---`). fn yaml_schema() -> OperatorResult { diff --git a/src/yaml.rs b/src/yaml.rs index d45b95eb6..4ccdc0990 100644 --- a/src/yaml.rs +++ b/src/yaml.rs @@ -1,39 +1,59 @@ //! Utility functions for processing data in the YAML file format +use std::io::Write; + use serde::ser; use crate::error::OperatorResult; -/// Serializes the given data structure as an explicit YAML document. +/// Serializes the given data structure as an explicit YAML document and writes it to a [`Write`]. +/// +/// Enums are serialized as a YAML map containing one entry in which the key identifies the variant +/// name. +/// +/// # Example +/// +/// ``` +/// use serde::Serialize; +/// use stackable_operator::yaml; +/// +/// #[derive(Serialize)] +/// #[serde(rename_all = "camelCase")] +/// enum Connection { +/// Inline(String), +/// Reference(String), +/// } +/// +/// #[derive(Serialize)] +/// struct Spec { +/// connection: Connection, +/// } +/// +/// let value = Spec { +/// connection: Connection::Inline("http://localhost".into()), +/// }; +/// +/// let mut buf = Vec::new(); +/// yaml::serialize_to_explicit_document(&mut buf, &value).unwrap(); +/// let actual_yaml = std::str::from_utf8(&buf).unwrap(); +/// +/// let expected_yaml = "--- +/// connection: +/// inline: http://localhost +/// "; +/// +/// assert_eq!(expected_yaml, actual_yaml); +/// ``` /// /// # Errors /// /// Serialization can fail if `T`'s implementation of `Serialize` decides to return an error. -pub fn to_explicit_document_string(value: &T) -> OperatorResult +pub fn serialize_to_explicit_document(mut writer: W, value: &T) -> OperatorResult<()> where - T: ?Sized + ser::Serialize, + T: ser::Serialize, + W: Write, { - let bare_document = serde_yaml::to_string(value)?; - let explicit_document = format!("---\n{}", bare_document); - - Ok(explicit_document) -} - -#[cfg(test)] -mod tests { - use std::collections::BTreeMap; - - use super::*; - - #[test] - fn value_can_be_serialized_to_an_explicit_document_string() { - let value: BTreeMap<_, _> = [("key", "value")].into(); - - let actual_yaml = to_explicit_document_string(&value).expect("serializable value"); - - let expected_yaml = "\ - ---\n\ - key: value\n"; - - assert_eq!(expected_yaml, actual_yaml); - } + writer.write_all(b"---\n")?; + let mut serializer = serde_yaml::Serializer::new(writer); + serde_yaml::with::singleton_map_recursive::serialize(value, &mut serializer)?; + Ok(()) }