From 51e7e1cb78f64e935990c6ac49e3b4b451b8d0a0 Mon Sep 17 00:00:00 2001 From: Nick Date: Tue, 2 Apr 2024 10:22:12 +0200 Subject: [PATCH] chore!: remove thiserror (add module-scoped errors with the help of `snafu`) (#761) * remove deprecated exports * refactor: remove thiserror * move error enum to below the Result type-alias * update changelog * use managed_by_labels variable in pdb builder * reorder imports * don't match on metadata * replace map_err with context and remove Box from the error source * remove Ok(..)? * tidy up kube client construction * use proper resource kinds in error * use String Debug impl in error message * rename error variants * upgrade change to breaking * Apply suggestions Co-authored-by: Techassi * replace map_err with context * remove associated error type * simplify error message * non-java error variants * return Snafu fail() * Apply suggestions Co-authored-by: Techassi --------- Co-authored-by: Techassi --- Cargo.toml | 1 - crates/stackable-operator/CHANGELOG.md | 5 + crates/stackable-operator/Cargo.toml | 1 - .../src/builder/configmap.rs | 21 +- crates/stackable-operator/src/builder/meta.rs | 27 +-- crates/stackable-operator/src/builder/mod.rs | 30 --- crates/stackable-operator/src/builder/pdb.rs | 32 ++- .../src/builder/pod/container.rs | 36 ++-- .../stackable-operator/src/builder/pod/mod.rs | 44 ++-- .../src/builder/pod/resources.rs | 9 +- .../src/builder/pod/volume.rs | 8 +- crates/stackable-operator/src/cli.rs | 58 +++--- crates/stackable-operator/src/client.rs | 188 +++++++++++++----- .../src/cluster_resources.rs | 92 ++++++--- .../src/commons/authentication/ldap.rs | 2 +- .../src/commons/authentication/mod.rs | 27 ++- .../src/commons/authentication/tls.rs | 2 +- .../src/commons/listener.rs | 2 +- crates/stackable-operator/src/commons/opa.rs | 40 +++- crates/stackable-operator/src/commons/rbac.rs | 33 ++- .../src/commons/resources.rs | 103 +++++----- crates/stackable-operator/src/commons/s3.rs | 53 +++-- .../src/commons/secret_class.rs | 2 +- crates/stackable-operator/src/cpu.rs | 39 +++- crates/stackable-operator/src/crd.rs | 53 +++-- crates/stackable-operator/src/error.rs | 131 ------------ .../src/kvp/annotation/mod.rs | 2 +- .../src/kvp/label/selector.rs | 130 ++++++------ crates/stackable-operator/src/lib.rs | 1 - .../src/logging/k8s_events.rs | 13 +- crates/stackable-operator/src/memory.rs | 60 ++++-- .../src/product_config_utils.rs | 81 ++++---- .../src/product_logging/framework.rs | 20 +- crates/stackable-operator/src/yaml.rs | 21 +- 34 files changed, 765 insertions(+), 602 deletions(-) delete mode 100644 crates/stackable-operator/src/error.rs diff --git a/Cargo.toml b/Cargo.toml index a1b8f34bc..c8b54676d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,7 +54,6 @@ stackable-operator-derive = { path = "stackable-operator-derive" } strum = { version = "0.26.2", features = ["derive"] } syn = "2.0.55" tempfile = "3.10.1" -thiserror = "1.0.58" time = { version = "0.3.34" } tokio = { version = "1.36.0", features = ["macros", "rt-multi-thread", "fs"] } tokio-rustls = "0.26.0" diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 9d23a4cf5..7559adabc 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -8,6 +8,11 @@ All notable changes to this project will be documented in this file. - Bump kube to 0.89.0 and update all dependencies ([#762]). +### Removed + +- BREAKING: Remove `thiserror` dependency, and deprecated builder exports ([#761]) + +[#761]: https://github.com/stackabletech/operator-rs/pull/761 [#762]: https://github.com/stackabletech/operator-rs/pull/762 ## [0.66.0] - 2024-03-26 diff --git a/crates/stackable-operator/Cargo.toml b/crates/stackable-operator/Cargo.toml index 4ca9f110d..5d8063de9 100644 --- a/crates/stackable-operator/Cargo.toml +++ b/crates/stackable-operator/Cargo.toml @@ -36,7 +36,6 @@ serde_yaml.workspace = true serde.workspace = true snafu.workspace = true strum.workspace = true -thiserror.workspace = true time = { workspace = true, optional = true } tokio.workspace = true tracing-opentelemetry.workspace = true diff --git a/crates/stackable-operator/src/builder/configmap.rs b/crates/stackable-operator/src/builder/configmap.rs index e74c039b1..fb5e67cbc 100644 --- a/crates/stackable-operator/src/builder/configmap.rs +++ b/crates/stackable-operator/src/builder/configmap.rs @@ -1,7 +1,15 @@ -use crate::error::{Error, OperatorResult}; use k8s_openapi::{api::core::v1::ConfigMap, apimachinery::pkg::apis::meta::v1::ObjectMeta}; +use snafu::{OptionExt, Snafu}; use std::collections::BTreeMap; +type Result = std::result::Result; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("object is missing key {key:?}"))] + MissingObjectKey { key: &'static str }, +} + /// A builder to build [`ConfigMap`] objects. #[derive(Clone, Default)] pub struct ConfigMapBuilder { @@ -41,12 +49,13 @@ impl ConfigMapBuilder { self } - pub fn build(&self) -> OperatorResult { + pub fn build(&self) -> Result { + let metadata = self + .metadata + .clone() + .context(MissingObjectKeySnafu { key: "metadata" })?; Ok(ConfigMap { - metadata: match self.metadata { - None => return Err(Error::MissingObjectKey { key: "metadata" }), - Some(ref metadata) => metadata.clone(), - }, + metadata, data: self.data.clone(), ..ConfigMap::default() }) diff --git a/crates/stackable-operator/src/builder/meta.rs b/crates/stackable-operator/src/builder/meta.rs index a3f8b7c0a..fdaeea510 100644 --- a/crates/stackable-operator/src/builder/meta.rs +++ b/crates/stackable-operator/src/builder/meta.rs @@ -3,16 +3,17 @@ use kube::{Resource, ResourceExt}; use snafu::{ResultExt, Snafu}; use tracing::warn; -use crate::{ - error::{Error, OperatorResult}, - kvp::{Annotation, Annotations, Label, LabelError, Labels, ObjectLabels}, -}; +use crate::kvp::{Annotation, Annotations, Label, LabelError, Labels, ObjectLabels}; + +type Result = std::result::Result; -// NOTE (Techassi): Think about that name #[derive(Debug, PartialEq, Snafu)] -pub enum ObjectMetaBuilderError { +pub enum Error { #[snafu(display("failed to set recommended labels"))] RecommendedLabels { source: LabelError }, + + #[snafu(display("object is missing key {key:?}"))] + MissingObjectKey { key: &'static str }, } /// A builder to build [`ObjectMeta`] objects. @@ -89,7 +90,7 @@ impl ObjectMetaBuilder { resource: &T, block_owner_deletion: Option, controller: Option, - ) -> OperatorResult<&mut Self> { + ) -> Result<&mut Self> { self.ownerreference = Some( OwnerReferenceBuilder::new() .initialize_from_resource(resource) @@ -151,7 +152,7 @@ impl ObjectMetaBuilder { pub fn with_recommended_labels( &mut self, object_labels: ObjectLabels, - ) -> Result<&mut Self, ObjectMetaBuilderError> { + ) -> Result<&mut Self> { let recommended_labels = Labels::recommended(object_labels).context(RecommendedLabelsSnafu)?; @@ -284,24 +285,24 @@ impl OwnerReferenceBuilder { self } - pub fn build(&self) -> OperatorResult { + pub fn build(&self) -> Result { Ok(OwnerReference { api_version: match self.api_version { - None => return Err(Error::MissingObjectKey { key: "api_version" }), + None => return MissingObjectKeySnafu { key: "api_version" }.fail(), Some(ref api_version) => api_version.clone(), }, block_owner_deletion: self.block_owner_deletion, controller: self.controller, kind: match self.kind { - None => return Err(Error::MissingObjectKey { key: "kind" }), + None => return MissingObjectKeySnafu { key: "kind" }.fail(), Some(ref kind) => kind.clone(), }, name: match self.name { - None => return Err(Error::MissingObjectKey { key: "name" }), + None => return MissingObjectKeySnafu { key: "name" }.fail(), Some(ref name) => name.clone(), }, uid: match self.uid { - None => return Err(Error::MissingObjectKey { key: "uid" }), + None => return MissingObjectKeySnafu { key: "uid" }.fail(), Some(ref uid) => uid.clone(), }, }) diff --git a/crates/stackable-operator/src/builder/mod.rs b/crates/stackable-operator/src/builder/mod.rs index 1602fbdf7..e4c619c17 100644 --- a/crates/stackable-operator/src/builder/mod.rs +++ b/crates/stackable-operator/src/builder/mod.rs @@ -8,33 +8,3 @@ pub mod event; pub mod meta; pub mod pdb; pub mod pod; - -#[deprecated(since = "0.15.0", note = "Please use `builder::configmap::*` instead.")] -pub use configmap::*; - -#[deprecated(since = "0.15.0", note = "Please use `builder::event::*` instead.")] -pub use event::*; - -#[deprecated(since = "0.15.0", note = "Please use `builder::meta::*` instead.")] -pub use meta::*; - -#[deprecated( - since = "0.15.0", - note = "Please use `builder::pod::container::*` instead." -)] -pub use pod::container::*; - -#[deprecated( - since = "0.15.0", - note = "Please use `builder::pod::security::*` instead." -)] -pub use pod::security::*; - -#[deprecated( - since = "0.15.0", - note = "Please use `builder::pod::volume::*` instead." -)] -pub use pod::volume::*; - -#[deprecated(since = "0.15.0", note = "Please use `builder::pod::*` instead.")] -pub use pod::*; diff --git a/crates/stackable-operator/src/builder/pdb.rs b/crates/stackable-operator/src/builder/pdb.rs index d0c6aba14..4a4465ee1 100644 --- a/crates/stackable-operator/src/builder/pdb.rs +++ b/crates/stackable-operator/src/builder/pdb.rs @@ -6,13 +6,27 @@ use k8s_openapi::{ }, }; use kube::{Resource, ResourceExt}; +use snafu::{ResultExt, Snafu}; use crate::{ - builder::ObjectMetaBuilder, - error::OperatorResult, + builder::meta::ObjectMetaBuilder, kvp::{Label, Labels}, }; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("failed to create role selector labels"))] + RoleSelectorLabels { source: crate::kvp::LabelError }, + + #[snafu(display("failed to set owner reference from resource"))] + OwnerReferenceFromResource { source: crate::builder::meta::Error }, + + #[snafu(display("failed to create app.kubernetes.io/managed-by label"))] + ManagedByLabel { source: crate::kvp::LabelError }, +} + /// This builder is used to construct [`PodDisruptionBudget`]s. /// If you are using this to create [`PodDisruptionBudget`]s according to [ADR 30 on Allowed Pod disruptions][adr], /// the use of [`PodDisruptionBudgetBuilder::new_with_role`] is recommended. @@ -70,14 +84,18 @@ impl PodDisruptionBudgetBuilder<(), (), ()> { role: &str, operator_name: &str, controller_name: &str, - ) -> OperatorResult> { - let role_selector_labels = Labels::role_selector(owner, app_name, role)?; + ) -> Result> { + let role_selector_labels = + Labels::role_selector(owner, app_name, role).context(RoleSelectorLabelsSnafu)?; + let managed_by_label = + Label::managed_by(operator_name, controller_name).context(ManagedByLabelSnafu)?; let metadata = ObjectMetaBuilder::new() .namespace_opt(owner.namespace()) .name(format!("{}-{}", owner.name_any(), role)) - .ownerreference_from_resource(owner, None, Some(true))? + .ownerreference_from_resource(owner, None, Some(true)) + .context(OwnerReferenceFromResourceSnafu)? .with_labels(role_selector_labels.clone()) - .with_label(Label::managed_by(operator_name, controller_name)?) + .with_label(managed_by_label) .build(); Ok(PodDisruptionBudgetBuilder { @@ -191,7 +209,7 @@ mod test { use schemars::JsonSchema; use serde::{Deserialize, Serialize}; - use crate::builder::{ObjectMetaBuilder, OwnerReferenceBuilder}; + use crate::builder::meta::{ObjectMetaBuilder, OwnerReferenceBuilder}; use super::PodDisruptionBudgetBuilder; diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs index fd09115f5..5bde690be 100644 --- a/crates/stackable-operator/src/builder/pod/container.rs +++ b/crates/stackable-operator/src/builder/pod/container.rs @@ -1,15 +1,27 @@ +use std::fmt; + use k8s_openapi::api::core::v1::{ ConfigMapKeySelector, Container, ContainerPort, EnvVar, EnvVarSource, Lifecycle, LifecycleHandler, ObjectFieldSelector, Probe, ResourceRequirements, SecretKeySelector, SecurityContext, VolumeMount, }; -use std::fmt; +use snafu::Snafu; use crate::{ - commons::product_image_selection::ResolvedProductImage, error::Error, - validation::is_rfc_1123_label, + commons::product_image_selection::ResolvedProductImage, validation::is_rfc_1123_label, }; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("container name {container_name:?} is invalid: {violation:?}"))] + InvalidContainerName { + container_name: String, + violation: String, + }, +} + /// A builder to build [`Container`] objects. /// /// This will automatically create the necessary volumes and mounts for each `ConfigMap` which is added. @@ -32,7 +44,7 @@ pub struct ContainerBuilder { } impl ContainerBuilder { - pub fn new(name: &str) -> Result { + pub fn new(name: &str) -> Result { Self::validate_container_name(name)?; Ok(ContainerBuilder { name: name.to_string(), @@ -264,7 +276,7 @@ impl ContainerBuilder { /// Validates a container name is according to the [RFC 1123](https://www.ietf.org/rfc/rfc1123.txt) standard. /// Returns [Ok] if the name is according to the standard, and [Err] if not. - fn validate_container_name(name: &str) -> Result<(), Error> { + fn validate_container_name(name: &str) -> Result<()> { let validation_result = is_rfc_1123_label(name); match validation_result { @@ -355,8 +367,8 @@ mod tests { use super::*; use crate::{ - builder::{ - pod::container::{ContainerBuilder, ContainerPortBuilder, FieldPathEnvVar}, + builder::pod::{ + container::{ContainerBuilder, ContainerPortBuilder, FieldPathEnvVar}, resources::ResourceRequirementsBuilder, }, commons::resources::ResourceRequirementsType, @@ -490,16 +502,13 @@ mod tests { panic!("Container name exceeding 63 characters should cause an error"); } Err(error) => match error { - crate::error::Error::InvalidContainerName { + Error::InvalidContainerName { container_name, violation, } => { assert_eq!(container_name.as_str(), long_container_name); assert_eq!(violation.as_str(), "must be no more than 63 characters") } - _ => { - panic!("InvalidContainerName error expected") - } }, } // One characters shorter name is valid @@ -569,16 +578,13 @@ mod tests { panic!("Container name exceeding 63 characters should cause an error"); } Err(error) => match error { - crate::error::Error::InvalidContainerName { + Error::InvalidContainerName { container_name: _, violation, } => { println!("{violation}"); assert!(violation.contains(expected_err_contains)); } - _ => { - panic!("InvalidContainerName error expected"); - } }, } } diff --git a/crates/stackable-operator/src/builder/pod/mod.rs b/crates/stackable-operator/src/builder/pod/mod.rs index 44fa856ad..49111989f 100644 --- a/crates/stackable-operator/src/builder/pod/mod.rs +++ b/crates/stackable-operator/src/builder/pod/mod.rs @@ -8,14 +8,11 @@ use k8s_openapi::{ }, apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::ObjectMeta}, }; -use snafu::{ResultExt, Snafu}; +use snafu::{OptionExt, ResultExt, Snafu}; use tracing::warn; use crate::{ - builder::{ - meta::ObjectMetaBuilder, ListenerOperatorVolumeSourceBuilder, - ListenerOperatorVolumeSourceBuilderError, ListenerReference, VolumeBuilder, - }, + builder::meta::ObjectMetaBuilder, commons::{ affinity::StackableAffinity, product_image_selection::ResolvedProductImage, @@ -24,15 +21,18 @@ use crate::{ LIMIT_REQUEST_RATIO_CPU, LIMIT_REQUEST_RATIO_MEMORY, }, }, - error::{self, OperatorResult}, time::Duration, }; +use self::volume::{ListenerOperatorVolumeSourceBuilder, ListenerReference, VolumeBuilder}; + pub mod container; pub mod resources; pub mod security; pub mod volume; +type Result = std::result::Result; + #[derive(Debug, PartialEq, Snafu)] pub enum Error { #[snafu(display("termination grace period is too long (got {duration}, maximum allowed is {max})", max = Duration::from_secs(i64::MAX as u64)))] @@ -41,13 +41,15 @@ pub enum Error { duration: Duration, }, - #[snafu(display("failed to add listener volume '{name}' to the pod"))] + #[snafu(display("failed to add listener volume {name:?} to the pod"))] ListenerVolume { - source: ListenerOperatorVolumeSourceBuilderError, + source: volume::ListenerOperatorVolumeSourceBuilderError, name: String, }, + + #[snafu(display("object is missing key {key:?}"))] + MissingObjectKey { key: &'static str }, } -pub type Result = std::result::Result; /// A builder to build [`Pod`] or [`PodTemplateSpec`] objects. #[derive(Clone, Debug, Default, PartialEq)] @@ -280,8 +282,8 @@ impl PodBuilder { /// # Example /// /// ``` - /// # use stackable_operator::builder::PodBuilder; - /// # use stackable_operator::builder::ContainerBuilder; + /// # use stackable_operator::builder::pod::PodBuilder; + /// # use stackable_operator::builder::pod::container::ContainerBuilder; /// # use stackable_operator::builder::pod::resources::ResourceRequirementsBuilder; /// # use k8s_openapi::{ /// api::core::v1::ResourceRequirements, @@ -369,8 +371,8 @@ impl PodBuilder { /// # Example /// /// ``` - /// # use stackable_operator::builder::PodBuilder; - /// # use stackable_operator::builder::ContainerBuilder; + /// # use stackable_operator::builder::pod::PodBuilder; + /// # use stackable_operator::builder::pod::container::ContainerBuilder; /// # use stackable_operator::builder::pod::resources::ResourceRequirementsBuilder; /// # use k8s_openapi::{ /// api::core::v1::ResourceRequirements, @@ -487,8 +489,7 @@ impl PodBuilder { let termination_grace_period_seconds = termination_grace_period .as_secs() .try_into() - .map_err(|err| Error::TerminationGracePeriodTooLong { - source: err, + .context(TerminationGracePeriodTooLongSnafu { duration: *termination_grace_period, })?; @@ -496,13 +497,14 @@ impl PodBuilder { Ok(self) } - /// Consumes the Builder and returns a constructed [`Pod`] - pub fn build(&self) -> OperatorResult { + /// Returns a constructed [`Pod`] + pub fn build(&self) -> Result { + let metadata = self + .metadata + .clone() + .context(MissingObjectKeySnafu { key: "metadata" })?; Ok(Pod { - metadata: match self.metadata { - None => return Err(error::Error::MissingObjectKey { key: "metadata" }), - Some(ref metadata) => metadata.clone(), - }, + metadata, spec: Some(self.build_spec()), status: self.status.clone(), }) diff --git a/crates/stackable-operator/src/builder/pod/resources.rs b/crates/stackable-operator/src/builder/pod/resources.rs index d8dccf267..6efc0d83f 100644 --- a/crates/stackable-operator/src/builder/pod/resources.rs +++ b/crates/stackable-operator/src/builder/pod/resources.rs @@ -6,8 +6,9 @@ use k8s_openapi::{ use tracing::warn; use crate::{ - commons::resources::ResourceRequirementsType, cpu::CpuQuantity, error::OperatorResult, - memory::MemoryQuantity, + commons::resources::ResourceRequirementsType, + cpu::{self, CpuQuantity}, + memory::{self, MemoryQuantity}, }; const RESOURCE_DENYLIST: &[&str] = &["cpu", "memory"]; @@ -53,7 +54,7 @@ impl ResourceRequirementsBuilder<(), CL, MR, ML> { self, request: impl Into, factor: f32, - ) -> OperatorResult> { + ) -> cpu::Result> { let request = CpuQuantity::from_str(&request.into())?; let limit = request * factor; @@ -123,7 +124,7 @@ impl ResourceRequirementsBuilder { self, request: impl Into, factor: f32, - ) -> OperatorResult> { + ) -> memory::Result> { let request = MemoryQuantity::from_str(&request.into())?; let limit = request * factor; diff --git a/crates/stackable-operator/src/builder/pod/volume.rs b/crates/stackable-operator/src/builder/pod/volume.rs index 708d50f91..2716a4e6a 100644 --- a/crates/stackable-operator/src/builder/pod/volume.rs +++ b/crates/stackable-operator/src/builder/pod/volume.rs @@ -12,7 +12,7 @@ use snafu::{ResultExt, Snafu}; use tracing::warn; use crate::{ - builder::ObjectMetaBuilder, + builder::meta::ObjectMetaBuilder, kvp::{Annotation, AnnotationError, Annotations}, }; @@ -426,9 +426,9 @@ pub enum ListenerOperatorVolumeSourceBuilderError { /// /// ``` /// # use k8s_openapi::api::core::v1::Volume; -/// # use stackable_operator::builder::ListenerReference; -/// # use stackable_operator::builder::ListenerOperatorVolumeSourceBuilder; -/// # use stackable_operator::builder::PodBuilder; +/// # use stackable_operator::builder::pod::volume::ListenerReference; +/// # use stackable_operator::builder::pod::volume::ListenerOperatorVolumeSourceBuilder; +/// # use stackable_operator::builder::pod::PodBuilder; /// let mut pod_builder = PodBuilder::new(); /// /// let volume_source = diff --git a/crates/stackable-operator/src/cli.rs b/crates/stackable-operator/src/cli.rs index 127fb5aa9..cc0221de4 100644 --- a/crates/stackable-operator/src/cli.rs +++ b/crates/stackable-operator/src/cli.rs @@ -14,8 +14,7 @@ //! use kube::CustomResource; //! use schemars::JsonSchema; //! use serde::{Deserialize, Serialize}; -//! use stackable_operator::{CustomResourceExt, cli}; -//! use stackable_operator::error::OperatorResult; +//! use stackable_operator::{CustomResourceExt, cli, crd}; //! //! const OPERATOR_VERSION: &str = "23.1.1"; //! @@ -53,7 +52,7 @@ //! command: cli::Command, //! } //! -//! # fn main() -> OperatorResult<()> { +//! # fn main() -> Result<(), crd::Error> { //! let opts = Opts::parse(); //! //! match opts.command { @@ -74,7 +73,6 @@ //! ```no_run //! use clap::{crate_version, Parser}; //! use stackable_operator::cli; -//! use stackable_operator::error::OperatorResult; //! //! #[derive(clap::Parser)] //! #[command( @@ -88,7 +86,7 @@ //! command: cli::Command, //! } //! -//! # fn main() -> OperatorResult<()> { +//! # fn main() -> Result<(), cli::Error> { //! let opts = Opts::parse(); //! //! match opts.command { @@ -108,11 +106,11 @@ //! ``` //! //! -use crate::error::OperatorResult; +use crate::logging::TracingTarget; use crate::namespace::WatchNamespace; -use crate::{error, logging::TracingTarget}; use clap::Args; use product_config::ProductConfigManager; +use snafu::{ResultExt, Snafu}; use std::{ ffi::OsStr, path::{Path, PathBuf}, @@ -120,6 +118,21 @@ use std::{ pub const AUTHOR: &str = "Stackable GmbH - info@stackable.tech"; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("failed to load product config"))] + ProductConfigLoad { + source: product_config::error::Error, + }, + + #[snafu(display( + "failed to locate a required file in any of the following locations: {search_path:?}" + ))] + RequiredFileMissing { search_path: Vec }, +} + /// Framework-standardized commands /// /// If you need operator-specific commands then you can flatten [`Command`] into your own command enum. For example: @@ -218,17 +231,12 @@ impl From<&OsStr> for ProductConfigPath { impl ProductConfigPath { /// Load the [`ProductConfigManager`] from the given path, falling back to the first /// path that exists from `default_search_paths` if none is given by the user. - pub fn load( - &self, - default_search_paths: &[impl AsRef], - ) -> OperatorResult { + pub fn load(&self, default_search_paths: &[impl AsRef]) -> Result { ProductConfigManager::from_yaml_file(resolve_path( self.path.as_deref(), default_search_paths, )?) - .map_err(|source| error::Error::ProductConfigLoadError { - source: source.into(), - }) + .context(ProductConfigLoadSnafu) } } @@ -240,7 +248,7 @@ impl ProductConfigPath { fn resolve_path<'a>( user_provided_path: Option<&'a Path>, default_paths: &'a [impl AsRef + 'a], -) -> OperatorResult<&'a Path> { +) -> Result<&'a Path> { // Use override if specified by the user, otherwise search through defaults given let search_paths = if let Some(path) = user_provided_path { vec![path] @@ -252,9 +260,13 @@ fn resolve_path<'a>( return Ok(path); } } - Err(error::Error::RequiredFileMissing { - search_path: search_paths.into_iter().map(PathBuf::from).collect(), - }) + RequiredFileMissingSnafu { + search_path: search_paths + .into_iter() + .map(PathBuf::from) + .collect::>(), + } + .fail() } #[cfg(test)] @@ -296,8 +308,8 @@ mod tests { #[case] default_locations: Vec<&str>, #[case] path_to_create: &str, #[case] expected: &str, - ) -> OperatorResult<()> { - let temp_dir = tempdir()?; + ) -> Result<()> { + let temp_dir = tempdir().expect("create temporary directory"); let full_path_to_create = temp_dir.path().join(path_to_create); let full_user_provided_path = user_provided_path.map(|p| temp_dir.path().join(p)); let expected_path = temp_dir.path().join(expected); @@ -314,7 +326,7 @@ mod tests { .map(String::as_str) .collect::>(); - let file = File::create(full_path_to_create)?; + let file = File::create(full_path_to_create).expect("create temporary file"); let found_path = resolve_path( full_user_provided_path.as_deref(), @@ -324,7 +336,7 @@ mod tests { assert_eq!(found_path, expected_path); drop(file); - temp_dir.close()?; + temp_dir.close().expect("clean up temporary directory"); Ok(()) } @@ -337,7 +349,7 @@ mod tests { #[test] fn test_resolve_path_nothing_found_errors() { - if let Err(error::Error::RequiredFileMissing { search_path }) = + if let Err(Error::RequiredFileMissing { search_path }) = resolve_path(None, &[DEPLOY_FILE_PATH, DEFAULT_FILE_PATH]) { assert_eq!( diff --git a/crates/stackable-operator/src/client.rs b/crates/stackable-operator/src/client.rs index 7a821baab..cb8a1d9a5 100644 --- a/crates/stackable-operator/src/client.rs +++ b/crates/stackable-operator/src/client.rs @@ -1,4 +1,3 @@ -use crate::error::{Error, OperatorResult}; use crate::kvp::LabelSelectorExt; use either::Either; @@ -13,10 +12,73 @@ use kube::runtime::{watcher, WatchStreamExt}; use kube::{Api, Config}; use serde::de::DeserializeOwned; use serde::Serialize; +use snafu::{OptionExt, ResultExt, Snafu}; use std::convert::TryFrom; use std::fmt::{Debug, Display}; use tracing::trace; +pub type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("unable to get resource {resource_name:?}"))] + GetResource { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to list resources"))] + ListResources { source: kube::Error }, + + #[snafu(display("unable to convert selector to query string"))] + SelectorToQueryString { source: crate::kvp::SelectorError }, + + #[snafu(display("unable to create resource {resource_name:?}"))] + CreateResource { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to patch resource {resource_name:?}"))] + PatchResource { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to patch the status for resource {resource_name:?}"))] + PatchResourceStatus { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to update resource {resource_name:?}"))] + UpdateResource { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to delete resource {resource_name:?}"))] + DeleteResource { + source: kube::Error, + resource_name: String, + }, + + #[snafu(display("unable to delete and finalize resource {resource_name:?}"))] + DeleteAndFinalizeResource { + source: kube::runtime::wait::delete::Error, + resource_name: String, + }, + + #[snafu(display("object is missing key {key:?}"))] + MissingObjectKey { key: &'static str }, + + #[snafu(display("unable to infer kubernetes configuration"))] + InferKubeConfig { source: kube::Error }, + + #[snafu(display("unable to create kubernetes client"))] + CreateKubeClient { source: kube::Error }, +} + /// This `Client` can be used to access Kubernetes. /// It wraps an underlying [kube::client::Client] and provides some common functionality. #[derive(Clone)] @@ -69,12 +131,15 @@ impl Client { } /// Retrieves a single instance of the requested resource type with the given name. - pub async fn get(&self, resource_name: &str, namespace: &T::Namespace) -> OperatorResult + pub async fn get(&self, resource_name: &str, namespace: &T::Namespace) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, { - Ok(self.get_api(namespace).get(resource_name).await?) + self.get_api(namespace) + .get(resource_name) + .await + .context(GetResourceSnafu { resource_name }) } /// Retrieves a single instance of the requested resource type with the given name, if it exists. @@ -82,12 +147,15 @@ impl Client { &self, resource_name: &str, namespace: &T::Namespace, - ) -> OperatorResult> + ) -> Result> where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, { - Ok(self.get_api(namespace).get_opt(resource_name).await?) + self.get_api(namespace) + .get_opt(resource_name) + .await + .context(GetResourceSnafu { resource_name }) } /// Returns Ok(true) if the resource has been registered in Kubernetes, Ok(false) if it could @@ -97,11 +165,7 @@ impl Client { /// exists method as soon as it becomes available (e.g. only returning Ok/Success) to reduce /// network traffic. #[deprecated(since = "0.24.0", note = "Replaced by `get_opt`")] - pub async fn exists( - &self, - resource_name: &str, - namespace: &T::Namespace, - ) -> OperatorResult + pub async fn exists(&self, resource_name: &str, namespace: &T::Namespace) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -118,12 +182,17 @@ impl Client { &self, namespace: &T::Namespace, list_params: &ListParams, - ) -> OperatorResult> + ) -> Result> where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, { - Ok(self.get_api(namespace).list(list_params).await?.items) + let list = self + .get_api(namespace) + .list(list_params) + .await + .context(ListResourcesSnafu)?; + Ok(list.items) } /// Lists resources from the API using a LabelSelector. @@ -138,12 +207,14 @@ impl Client { &self, namespace: &T::Namespace, selector: &LabelSelector, - ) -> OperatorResult> + ) -> Result> where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, { - let selector_string = selector.to_query_string()?; + let selector_string = selector + .to_query_string() + .context(SelectorToQueryStringSnafu)?; trace!("Listing for LabelSelector [{}]", selector_string); let list_params = ListParams { label_selector: Some(selector_string), @@ -153,21 +224,23 @@ impl Client { } /// Creates a new resource. - pub async fn create(&self, resource: &T) -> OperatorResult + pub async fn create(&self, resource: &T) -> Result where T: Clone + Debug + DeserializeOwned + Resource + Serialize + GetApi, ::DynamicType: Default, { - Ok(self - .get_api(resource.get_namespace()) + self.get_api(resource.get_namespace()) .create(&self.post_params, resource) - .await?) + .await + .context(CreateResourceSnafu { + resource_name: resource.name_any(), + }) } /// Patches a resource using the `MERGE` patch strategy described /// in [JSON Merge Patch](https://tools.ietf.org/html/rfc7386) /// This will fail for objects that do not exist yet. - pub async fn merge_patch(&self, resource: &T, patch: P) -> OperatorResult + pub async fn merge_patch(&self, resource: &T, patch: P) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -187,7 +260,7 @@ impl Client { field_manager_scope: &str, resource: &T, patch: P, - ) -> OperatorResult + ) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -202,7 +275,7 @@ impl Client { } /// Patches a resource using the `JSON` patch strategy described in [JavaScript Object Notation (JSON) Patch](https://tools.ietf.org/html/rfc6902). - pub async fn json_patch(&self, resource: &T, patch: json_patch::Patch) -> OperatorResult + pub async fn json_patch(&self, resource: &T, patch: json_patch::Patch) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -221,16 +294,18 @@ impl Client { resource: &T, patch: Patch

, patch_params: &PatchParams, - ) -> OperatorResult + ) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, P: Debug + Serialize, { - Ok(self - .get_api(resource.get_namespace()) + self.get_api(resource.get_namespace()) .patch(&resource.name_any(), patch_params, &patch) - .await?) + .await + .context(PatchResourceSnafu { + resource_name: resource.name_any(), + }) } /// Patches subresource status in a given Resource using apply strategy. @@ -240,7 +315,7 @@ impl Client { field_manager_scope: &str, resource: &T, status: &S, - ) -> OperatorResult + ) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -267,7 +342,7 @@ impl Client { /// Patches subresource status in a given Resource using merge strategy. /// The subresource status must be defined beforehand in the Crd. - pub async fn merge_patch_status(&self, resource: &T, status: &S) -> OperatorResult + pub async fn merge_patch_status(&self, resource: &T, status: &S) -> Result where T: DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -282,11 +357,7 @@ impl Client { /// Patches subresource status in a given Resource using merge strategy. /// The subresource status must be defined beforehand in the Crd. /// Patches a resource using the `JSON` patch strategy described in [JavaScript Object Notation (JSON) Patch](https://tools.ietf.org/html/rfc6902). - pub async fn json_patch_status( - &self, - resource: &T, - patch: json_patch::Patch, - ) -> OperatorResult + pub async fn json_patch_status(&self, resource: &T, patch: json_patch::Patch) -> Result where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, @@ -316,31 +387,35 @@ impl Client { resource: &T, patch: Patch, patch_params: &PatchParams, - ) -> OperatorResult + ) -> Result where T: DeserializeOwned + Resource + GetApi, ::DynamicType: Default, S: Debug + Serialize, { let api = self.get_api(resource.get_namespace()); - Ok(api - .patch_status(&resource.name_any(), patch_params, &patch) - .await?) + api.patch_status(&resource.name_any(), patch_params, &patch) + .await + .context(PatchResourceStatusSnafu { + resource_name: resource.name_any(), + }) } /// This will _update_ an existing resource. /// The operation is called `replace` in the Kubernetes API. /// While a `patch` can just update a partial object /// a `update` will always replace the full object. - pub async fn update(&self, resource: &T) -> OperatorResult + pub async fn update(&self, resource: &T) -> Result where T: Clone + Debug + DeserializeOwned + Resource + Serialize + GetApi, ::DynamicType: Default, { - Ok(self - .get_api(resource.get_namespace()) + self.get_api(resource.get_namespace()) .replace(&resource.name_any(), &self.post_params, resource) - .await?) + .await + .context(UpdateResourceSnafu { + resource_name: resource.name_any(), + }) } /// This deletes a resource _if it is not deleted already_. @@ -350,15 +425,17 @@ impl Client { /// Which of the two are returned depends on the API being called. /// Take a look at the Kubernetes API reference. /// Some `delete` endpoints return the object and others return a `Status` object. - pub async fn delete(&self, resource: &T) -> OperatorResult> + pub async fn delete(&self, resource: &T) -> Result> where T: Clone + Debug + DeserializeOwned + Resource + GetApi, ::DynamicType: Default, { let api: Api = self.get_api(resource.get_namespace()); - Ok(api - .delete(&resource.name_any(), &self.delete_params) - .await?) + api.delete(&resource.name_any(), &self.delete_params) + .await + .context(DeleteResourceSnafu { + resource_name: resource.name_any(), + }) } /// This deletes a resource _if it is not deleted already_ and waits until the deletion is @@ -368,23 +445,26 @@ impl Client { /// /// Afterwards it loops and checks regularly whether the resource has been deleted /// from Kubernetes - pub async fn ensure_deleted(&self, resource: T) -> OperatorResult<()> + pub async fn ensure_deleted(&self, resource: T) -> Result<()> where T: Clone + Debug + DeserializeOwned + Resource + GetApi + Send + 'static, ::DynamicType: Default, { - Ok(delete_and_finalize( + delete_and_finalize( self.get_api::(resource.get_namespace()), resource .meta() .name .as_deref() - .ok_or(Error::MissingObjectKey { + .context(MissingObjectKeySnafu { key: "metadata.name", })?, &self.delete_params, ) - .await?) + .await + .context(DeleteAndFinalizeResourceSnafu { + resource_name: resource.name_any(), + }) } /// Returns an [kube::Api] object which is either namespaced or not depending on whether @@ -542,16 +622,14 @@ where } } -pub async fn create_client(field_manager: Option) -> OperatorResult { +pub async fn create_client(field_manager: Option) -> Result { let kubeconfig: Config = kube::Config::infer() .await - .map_err(kube::Error::InferConfig)?; + .map_err(kube::Error::InferConfig) + .context(InferKubeConfigSnafu)?; let default_namespace = kubeconfig.default_namespace.clone(); - Ok(Client::new( - kube::Client::try_from(kubeconfig)?, - field_manager, - default_namespace, - )) + let client = kube::Client::try_from(kubeconfig).context(CreateKubeClientSnafu)?; + Ok(Client::new(client, field_manager, default_namespace)) } #[cfg(test)] diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index ce85762fc..e39ea1c3e 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -9,7 +9,6 @@ use crate::{ LIMIT_REQUEST_RATIO_CPU, LIMIT_REQUEST_RATIO_MEMORY, }, }, - error::{Error, OperatorResult}, kvp::{ consts::{K8S_APP_INSTANCE_KEY, K8S_APP_MANAGED_BY_KEY, K8S_APP_NAME_KEY}, Label, LabelError, Labels, @@ -32,6 +31,7 @@ use k8s_openapi::{ }; use kube::{core::ErrorResponse, Resource, ResourceExt}; use serde::{de::DeserializeOwned, Serialize}; +use snafu::{OptionExt, ResultExt, Snafu}; use std::{ collections::{BTreeMap, HashSet}, fmt::Debug, @@ -45,6 +45,36 @@ use crate::k8s_openapi::api::{ core::v1::{NodeSelector, Pod}, }; +type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("object is missing key {key:?}"))] + MissingObjectKey { key: &'static str }, + + #[snafu(display("failed to list cluster resources with label selector"))] + ListClusterResources { source: crate::client::Error }, + + #[snafu(display("label {label:?} is missing"))] + MissingLabel { label: &'static str }, + + #[snafu(display("label {label:?} contains unexpected values - expected {expected_content:?}, got {actual_content:?}"))] + UnexpectedLabelContent { + label: &'static str, + expected_content: String, + actual_content: String, + }, + + #[snafu(display("failed to get resource"))] + GetResource { source: crate::client::Error }, + + #[snafu(display("failed to apply patch"))] + ApplyPatch { source: crate::client::Error }, + + #[snafu(display("failed to delete orphaned resource"))] + DeleteOrphanedResource { source: crate::client::Error }, +} + /// A cluster resource handled by [`ClusterResources`]. /// /// This trait is used in the function signatures of [`ClusterResources`] and restricts the @@ -114,7 +144,7 @@ impl ClusterResourceApplyStrategy { manager: &str, resource: &T, client: &Client, - ) -> OperatorResult { + ) -> Result { match self { Self::ReconciliationPaused => { debug!( @@ -128,9 +158,10 @@ impl ClusterResourceApplyStrategy { resource .namespace() .as_deref() - .ok_or(Error::MissingObjectKey { key: "namespace" })?, + .context(MissingObjectKeySnafu { key: "namespace" })?, ) .await + .context(GetResourceSnafu) } Self::Default | Self::ClusterStopped => { debug!( @@ -138,7 +169,10 @@ impl ClusterResourceApplyStrategy { resource.name_any(), self ); - client.apply_patch(manager, resource, resource).await + client + .apply_patch(manager, resource, resource) + .await + .context(ApplyPatchSnafu) } Self::NoApply => { debug!( @@ -259,7 +293,7 @@ impl ClusterResource for DaemonSet { /// use schemars::JsonSchema; /// use serde::{Deserialize, Serialize}; /// use stackable_operator::client::Client; -/// use stackable_operator::cluster_resources::{ClusterResourceApplyStrategy, ClusterResources}; +/// use stackable_operator::cluster_resources::{self, ClusterResourceApplyStrategy, ClusterResources}; /// use stackable_operator::product_config_utils::ValidatedRoleConfigByPropertyKind; /// use stackable_operator::role_utils::Role; /// use std::sync::Arc; @@ -280,13 +314,13 @@ impl ClusterResource for DaemonSet { /// /// enum Error { /// CreateClusterResources { -/// source: stackable_operator::error::Error, +/// source: cluster_resources::Error, /// }, /// AddClusterResource { -/// source: stackable_operator::error::Error, +/// source: cluster_resources::Error, /// }, /// DeleteOrphanedClusterResources { -/// source: stackable_operator::error::Error, +/// source: cluster_resources::Error, /// }, /// }; /// @@ -387,7 +421,7 @@ impl ClusterResources { /// /// # Errors /// - /// If `cluster` does not contain a namespace and a name then an `Error::MissingObjectKey` is + /// If `cluster` does not contain a namespace and a name then an [`Error::MissingObjectKey`] is /// returned. pub fn new( app_name: &str, @@ -395,15 +429,15 @@ impl ClusterResources { controller_name: &str, cluster: &ObjectReference, apply_strategy: ClusterResourceApplyStrategy, - ) -> OperatorResult { + ) -> Result { let namespace = cluster .namespace .to_owned() - .ok_or(Error::MissingObjectKey { key: "namespace" })?; + .context(MissingObjectKeySnafu { key: "namespace" })?; let app_instance = cluster .name .to_owned() - .ok_or(Error::MissingObjectKey { key: "name" })?; + .context(MissingObjectKeySnafu { key: "name" })?; Ok(ClusterResources { namespace, @@ -441,19 +475,20 @@ impl ClusterResources { /// /// # Errors /// - /// If the labels of the given resource are not set properly then an `Error::MissingLabel` or - /// `Error::UnexpectedLabelContent` is returned. The expected labels are: + /// If the labels of the given resource are not set properly then an [`Error::MissingLabel`] or + /// [`Error::UnexpectedLabelContent`] is returned. The expected labels are: + /// /// * `app.kubernetes.io/instance = ` /// * `app.kubernetes.io/managed-by = -operator` /// * `app.kubernetes.io/name = ` /// - /// If the patched resource does not contain a UID then an `Error::MissingObjectKey` is + /// If the patched resource does not contain a UID then an [`Error::MissingObjectKey`] is /// returned. pub async fn add( &mut self, client: &Client, resource: T, - ) -> OperatorResult { + ) -> Result { Self::check_labels( resource.labels(), &[ @@ -489,7 +524,7 @@ impl ClusterResources { .run(&self.manager, &mutated, client) .await?; - let resource_id = patched_resource.uid().ok_or(Error::MissingObjectKey { + let resource_id = patched_resource.uid().context(MissingObjectKeySnafu { key: "metadata/uid", })?; @@ -519,7 +554,7 @@ impl ClusterResources { labels: &BTreeMap, expected_label: &'static str, expected_content: &str, - ) -> OperatorResult<()> { + ) -> Result<()> { if let Some(actual_content) = labels.get(expected_label) { if expected_content == actual_content { Ok(()) @@ -543,7 +578,7 @@ impl ClusterResources { labels: &BTreeMap, expected_labels: &[&'static str], expected_contents: &[&str], - ) -> OperatorResult<()> { + ) -> Result<()> { for (label, content) in expected_labels.iter().zip(expected_contents) { Self::check_label(labels, label, content)?; } @@ -566,7 +601,7 @@ impl ClusterResources { /// /// * `client` - The client which is used to access Kubernetes /// - pub async fn delete_orphaned_resources(self, client: &Client) -> OperatorResult<()> { + pub async fn delete_orphaned_resources(self, client: &Client) -> Result<()> { tokio::try_join!( self.delete_orphaned_resources_of_kind::(client), self.delete_orphaned_resources_of_kind::(client), @@ -595,12 +630,12 @@ impl ClusterResources { /// /// # Errors /// - /// If a deployed resource does not contain a UID then an `Error::MissingObjectKey` is + /// If a deployed resource does not contain a UID then an [`Error::MissingObjectKey`] is /// returned. async fn delete_orphaned_resources_of_kind( &self, client: &Client, - ) -> OperatorResult<()> { + ) -> Result<()> { if !self.apply_strategy.delete_orphans() { debug!( "Skip deleting orphaned resources because of [{}] strategy.", @@ -614,7 +649,7 @@ impl ClusterResources { let mut orphaned_resources = Vec::new(); for resource in deployed_cluster_resources { - let resource_id = resource.uid().ok_or(Error::MissingObjectKey { + let resource_id = resource.uid().context(MissingObjectKeySnafu { key: "metadata/uid", })?; if !self.resource_ids.contains(&resource_id) { @@ -629,13 +664,16 @@ impl ClusterResources { ClusterResources::print_resources(&orphaned_resources), ); for resource in orphaned_resources.iter() { - client.delete(resource).await?; + client + .delete(resource) + .await + .context(DeleteOrphanedResourceSnafu)?; } } Ok(()) } - Err(Error::KubeError { + Err(crate::client::Error::ListResources { source: kube::Error::Api(ErrorResponse { code: 403, .. }), }) => { debug!( @@ -645,7 +683,7 @@ impl ClusterResources { ); Ok(()) } - Err(error) => Err(error), + Err(error) => Err(error).context(ListClusterResourcesSnafu), } } @@ -678,7 +716,7 @@ impl ClusterResources { async fn list_deployed_cluster_resources( &self, client: &Client, - ) -> OperatorResult> { + ) -> crate::client::Result> { let label_selector = LabelSelector { match_expressions: Some(vec![ LabelSelectorRequirement { diff --git a/crates/stackable-operator/src/commons/authentication/ldap.rs b/crates/stackable-operator/src/commons/authentication/ldap.rs index f11724207..35370cce9 100644 --- a/crates/stackable-operator/src/commons/authentication/ldap.rs +++ b/crates/stackable-operator/src/commons/authentication/ldap.rs @@ -5,7 +5,7 @@ use snafu::{ResultExt, Snafu}; use url::{ParseError, Url}; use crate::{ - builder::{ContainerBuilder, PodBuilder, VolumeMountBuilder}, + builder::pod::{container::ContainerBuilder, volume::VolumeMountBuilder, PodBuilder}, commons::{ authentication::{ tls::{TlsClientDetails, TlsClientDetailsError}, diff --git a/crates/stackable-operator/src/commons/authentication/mod.rs b/crates/stackable-operator/src/commons/authentication/mod.rs index 5c7296701..41b9bc1a8 100644 --- a/crates/stackable-operator/src/commons/authentication/mod.rs +++ b/crates/stackable-operator/src/commons/authentication/mod.rs @@ -1,12 +1,10 @@ use kube::CustomResource; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use snafu::{OptionExt, Snafu}; use strum::Display; -use crate::{ - client::Client, - error::{Error, OperatorResult}, -}; +use crate::client::Client; pub mod ldap; pub mod oidc; @@ -15,6 +13,14 @@ pub mod tls; pub(crate) const SECRET_BASE_PATH: &str = "/stackable/secrets"; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("authentication details for OIDC were not specified. The AuthenticationClass {auth_class_name:?} uses an OIDC provider, you need to specify OIDC authentication details (such as client credentials) as well"))] + OidcAuthenticationDetailsNotSpecified { auth_class_name: String }, +} + /// The Stackable Platform uses the AuthenticationClass as a central mechanism to handle user authentication across supported products. /// The authentication mechanism needs to be configured only in the AuthenticationClass which is then referenced in the product. /// Multiple different authentication providers are supported. @@ -77,7 +83,7 @@ impl AuthenticationClass { pub async fn resolve( client: &Client, authentication_class_name: &str, - ) -> OperatorResult { + ) -> crate::client::Result { client .get::(authentication_class_name, &()) // AuthenticationClass has ClusterScope .await @@ -137,8 +143,11 @@ pub struct ClientAuthenticationDetails { impl ClientAuthenticationDetails { /// Resolves this specific [`AuthenticationClass`]. Usually products support - /// a list of authentication classes, which individually need to be resolved. - pub async fn resolve_class(&self, client: &Client) -> OperatorResult { + /// a list of authentication classes, which individually need to be resolved.crate::client + pub async fn resolve_class( + &self, + client: &Client, + ) -> crate::client::Result { AuthenticationClass::resolve(client, &self.authentication_class_ref).await } @@ -152,10 +161,10 @@ impl ClientAuthenticationDetails { pub fn oidc_or_error( &self, auth_class_name: &str, - ) -> OperatorResult<&oidc::ClientAuthenticationOptions> { + ) -> Result<&oidc::ClientAuthenticationOptions> { self.oidc .as_ref() - .ok_or(Error::OidcAuthenticationDetailsNotSpecified { + .with_context(|| OidcAuthenticationDetailsNotSpecifiedSnafu { auth_class_name: auth_class_name.to_string(), }) } diff --git a/crates/stackable-operator/src/commons/authentication/tls.rs b/crates/stackable-operator/src/commons/authentication/tls.rs index c028be9e2..5f2e515f3 100644 --- a/crates/stackable-operator/src/commons/authentication/tls.rs +++ b/crates/stackable-operator/src/commons/authentication/tls.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; use crate::{ - builder::{ContainerBuilder, PodBuilder, VolumeMountBuilder}, + builder::pod::{container::ContainerBuilder, volume::VolumeMountBuilder, PodBuilder}, commons::{ authentication::SECRET_BASE_PATH, secret_class::{SecretClassVolume, SecretClassVolumeError}, diff --git a/crates/stackable-operator/src/commons/listener.rs b/crates/stackable-operator/src/commons/listener.rs index 911b41a06..fca900593 100644 --- a/crates/stackable-operator/src/commons/listener.rs +++ b/crates/stackable-operator/src/commons/listener.rs @@ -37,7 +37,7 @@ use k8s_openapi::api::core::v1::{ }; #[cfg(doc)] -use crate::builder::ListenerOperatorVolumeSourceBuilder; +use crate::builder::pod::volume::ListenerOperatorVolumeSourceBuilder; /// Defines a policy for how [Listeners](DOCS_BASE_URL_PLACEHOLDER/listener-operator/listener) should be exposed. /// Read the [ListenerClass documentation](DOCS_BASE_URL_PLACEHOLDER/listener-operator/listenerclass) diff --git a/crates/stackable-operator/src/commons/opa.rs b/crates/stackable-operator/src/commons/opa.rs index 81f45f3d1..c8d94381e 100644 --- a/crates/stackable-operator/src/commons/opa.rs +++ b/crates/stackable-operator/src/commons/opa.rs @@ -44,14 +44,33 @@ //! assert_eq!(opa_config.full_document_url(&cluster, "http://localhost:8081", None, OpaApiVersion::V1), "http://localhost:8081/v1/data/test".to_string()); //! ``` use crate::client::{Client, GetApi}; -use crate::error; -use crate::error::OperatorResult; use k8s_openapi::{api::core::v1::ConfigMap, NamespaceResourceScope}; use kube::{Resource, ResourceExt}; use lazy_static::lazy_static; use regex::Regex; use schemars::{self, JsonSchema}; use serde::{Deserialize, Serialize}; +use snafu::{OptionExt, ResultExt, Snafu}; + +type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("cannot find OPA ConfigMap {configmap_name:?} in namespace {namespace:?}"))] + GetOpaConfigMap { + source: crate::client::Error, + configmap_name: String, + namespace: String, + }, + + #[snafu(display( + "missing OPA connect string in configmap {configmap_name:?} in namespace {namespace:?}" + ))] + MissingOpaConnectString { + configmap_name: String, + namespace: String, + }, +} lazy_static! { static ref DOT_REGEX: Regex = Regex::new("\\.").unwrap(); @@ -195,7 +214,7 @@ impl OpaConfig { resource: &T, rule: Option<&str>, api_version: OpaApiVersion, - ) -> OperatorResult + ) -> Result where T: Resource, { @@ -212,18 +231,19 @@ impl OpaConfig { /// # Arguments /// * `client` - The kubernetes client. /// * `namespace` - The namespace of the config map. - async fn base_url_from_config_map( - &self, - client: &Client, - namespace: &str, - ) -> OperatorResult { + async fn base_url_from_config_map(&self, client: &Client, namespace: &str) -> Result { client .get::(&self.config_map_name, namespace) - .await? + .await + .with_context(|_| GetOpaConfigMapSnafu { + configmap_name: self.config_map_name.clone(), + namespace, + })? .data .and_then(|mut data| data.remove("OPA")) - .ok_or(error::Error::MissingOpaConnectString { + .with_context(|| MissingOpaConnectStringSnafu { configmap_name: self.config_map_name.clone(), + namespace, }) } diff --git a/crates/stackable-operator/src/commons/rbac.rs b/crates/stackable-operator/src/commons/rbac.rs index 425c6fe49..5907417cb 100644 --- a/crates/stackable-operator/src/commons/rbac.rs +++ b/crates/stackable-operator/src/commons/rbac.rs @@ -1,8 +1,8 @@ use kube::{Resource, ResourceExt}; +use snafu::{ResultExt, Snafu}; use crate::{ - builder::ObjectMetaBuilder, - error::OperatorResult, + builder::meta::ObjectMetaBuilder, k8s_openapi::api::{ core::v1::ServiceAccount, rbac::v1::{RoleBinding, RoleRef, Subject}, @@ -10,6 +10,23 @@ use crate::{ kvp::Labels, }; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("failed to set owner reference from resource for ServiceAccount {name:?}"))] + ServiceAccountOwnerReferenceFromResource { + source: crate::builder::meta::Error, + name: String, + }, + + #[snafu(display("failed to set owner reference from resource Role Binding {name:?}"))] + RoleBindingOwnerReferenceFromResource { + source: crate::builder::meta::Error, + name: String, + }, +} + /// Build RBAC objects for the product workloads. /// The `rbac_prefix` is meant to be the product name, for example: zookeeper, airflow, etc. /// and it is a assumed that a ClusterRole named `{rbac_prefix}-clusterrole` exists. @@ -17,13 +34,16 @@ pub fn build_rbac_resources>( resource: &T, rbac_prefix: &str, labels: Labels, -) -> OperatorResult<(ServiceAccount, RoleBinding)> { +) -> Result<(ServiceAccount, RoleBinding)> { let sa_name = service_account_name(rbac_prefix); let service_account = ServiceAccount { metadata: ObjectMetaBuilder::new() .name_and_namespace(resource) .name(sa_name.clone()) - .ownerreference_from_resource(resource, None, Some(true))? + .ownerreference_from_resource(resource, None, Some(true)) + .with_context(|_| ServiceAccountOwnerReferenceFromResourceSnafu { + name: sa_name.clone(), + })? .with_labels(labels.clone()) .build(), ..ServiceAccount::default() @@ -33,7 +53,10 @@ pub fn build_rbac_resources>( metadata: ObjectMetaBuilder::new() .name_and_namespace(resource) .name(role_binding_name(rbac_prefix)) - .ownerreference_from_resource(resource, None, Some(true))? + .ownerreference_from_resource(resource, None, Some(true)) + .context(RoleBindingOwnerReferenceFromResourceSnafu { + name: resource.name_any(), + })? .with_labels(labels) .build(), role_ref: RoleRef { diff --git a/crates/stackable-operator/src/commons/resources.rs b/crates/stackable-operator/src/commons/resources.rs index 3fea9c0ba..75822c6e3 100644 --- a/crates/stackable-operator/src/commons/resources.rs +++ b/crates/stackable-operator/src/commons/resources.rs @@ -84,12 +84,46 @@ use k8s_openapi::apimachinery::pkg::api::resource::Quantity; use k8s_openapi::apimachinery::pkg::apis::meta::v1::{LabelSelector, ObjectMeta}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use snafu::{ResultExt, Snafu}; use std::{collections::BTreeMap, fmt::Debug}; use strum::Display; pub const LIMIT_REQUEST_RATIO_CPU: f32 = 5.0; pub const LIMIT_REQUEST_RATIO_MEMORY: f32 = 1.0; +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display( + "missing {resource_key:?} resource {resource_type:?} for container {container_name:?}" + ))] + MissingResourceRequirements { + container_name: String, + resource_key: String, + resource_type: ResourceRequirementsType, + }, + + #[snafu(display("{resource_key:?} max limit to request ratio for Container {container_name:?} is {allowed_ration:?}, but ratio was exceeded or request and limit where not set explicitly"))] + LimitToRequestRatioExceeded { + container_name: String, + resource_key: String, + allowed_ration: f32, + }, + + #[snafu(display("failed to convert Quantity to CpuQuantity for cpu limit"))] + CpuLimit { source: crate::cpu::Error }, + + #[snafu(display("failed to convert Quantity to CpuQuantity for cpu request"))] + CpuRequest { source: crate::cpu::Error }, + + #[snafu(display("failed to convert Quantity to MemoryQuantity for memory limit"))] + MemoryLimit { source: crate::memory::Error }, + + #[snafu(display("failed to convert Quantity to MemoryQuantity for memory request"))] + MemoryRequest { source: crate::memory::Error }, +} + /// Resource usage is configured here, this includes CPU usage, memory usage and disk storage /// usage, if this role needs any. #[derive(Clone, Debug, Default, Fragment, PartialEq, JsonSchema)] @@ -336,27 +370,6 @@ impl Into for Resources { } } -#[derive(Debug, thiserror::Error)] -pub enum ResourceRequirementsError { - #[error("failed to parse quantity")] - ParseQuantity { - #[source] - error: crate::error::Error, - }, - #[error("missing {resource_key} resource {resource_type} for container {container_name}")] - MissingResourceRequirements { - resource_type: ResourceRequirementsType, - container_name: String, - resource_key: String, - }, - #[error("{resource_key} max limit to request ratio for Container {container_name} is {allowed_ration}, but ratio was exceeded or request and limit where not set explicitly")] - LimitToRequestRatioExceeded { - container_name: String, - resource_key: String, - allowed_ration: f32, - }, -} - /// [`ResourceRequirementsType`] describes the available resource requirement /// types. The user can set limits, requests and claims. This enum makes it /// possible to check if containers set one or more of these types. @@ -389,7 +402,7 @@ pub trait ResourceRequirementsExt { &self, rr_type: ResourceRequirementsType, resource: &str, - ) -> Result<(), ResourceRequirementsError>; + ) -> Result<()>; /// Returns wether the implementor has a [`ResourceRequirementsType`] set /// for a `resource`. @@ -403,7 +416,7 @@ pub trait ResourceRequirementsExt { &self, rr_types: Vec, resource: &str, - ) -> Result<(), ResourceRequirementsError> { + ) -> Result<()> { for rr_type in rr_types { self.check_resource_requirement(rr_type, resource)?; } @@ -426,7 +439,7 @@ pub trait ResourceRequirementsExt { resource: &ComputeResource, // We did choose a f32 instead of a usize here, as LimitRange ratios can be a floating point (Quantity - e.g. 1500m) ratio: f32, - ) -> Result<(), ResourceRequirementsError>; + ) -> Result<()>; } impl ResourceRequirementsExt for Container { @@ -434,7 +447,7 @@ impl ResourceRequirementsExt for Container { &self, rr_type: ResourceRequirementsType, resource: &str, - ) -> Result<(), ResourceRequirementsError> { + ) -> Result<()> { let resources = match rr_type { ResourceRequirementsType::Limits => { self.resources.as_ref().and_then(|rr| rr.limits.as_ref()) @@ -444,21 +457,18 @@ impl ResourceRequirementsExt for Container { } }; if resources.and_then(|r| r.get(resource)).is_none() { - return Err(ResourceRequirementsError::MissingResourceRequirements { + return MissingResourceRequirementsSnafu { container_name: self.name.clone(), - resource_key: resource.into(), + resource_key: resource, resource_type: rr_type, - }); + } + .fail(); } Ok(()) } - fn check_limit_to_request_ratio( - &self, - resource: &ComputeResource, - ratio: f32, - ) -> Result<(), ResourceRequirementsError> { + fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f32) -> Result<()> { let limit = self .resources .as_ref() @@ -472,19 +482,15 @@ impl ResourceRequirementsExt for Container { if let (Some(limit), Some(request)) = (limit, request) { match resource { ComputeResource::Cpu => { - let limit = CpuQuantity::try_from(limit) - .map_err(|error| ResourceRequirementsError::ParseQuantity { error })?; - let request = CpuQuantity::try_from(request) - .map_err(|error| ResourceRequirementsError::ParseQuantity { error })?; + let limit = CpuQuantity::try_from(limit).context(CpuLimitSnafu)?; + let request = CpuQuantity::try_from(request).context(CpuRequestSnafu)?; if limit / request <= ratio { return Ok(()); } } ComputeResource::Memory => { - let limit = MemoryQuantity::try_from(limit) - .map_err(|error| ResourceRequirementsError::ParseQuantity { error })?; - let request = MemoryQuantity::try_from(request) - .map_err(|error| ResourceRequirementsError::ParseQuantity { error })?; + let limit = MemoryQuantity::try_from(limit).context(MemoryLimitSnafu)?; + let request = MemoryQuantity::try_from(request).context(MemoryRequestSnafu)?; if limit / request <= ratio { return Ok(()); } @@ -492,11 +498,12 @@ impl ResourceRequirementsExt for Container { } } - Err(ResourceRequirementsError::LimitToRequestRatioExceeded { + LimitToRequestRatioExceededSnafu { container_name: self.name.clone(), resource_key: resource.to_string(), allowed_ration: ratio, - }) + } + .fail() } } @@ -505,7 +512,7 @@ impl ResourceRequirementsExt for PodSpec { &self, rr_type: ResourceRequirementsType, resource: &str, - ) -> Result<(), ResourceRequirementsError> { + ) -> Result<()> { for container in &self.containers { container.check_resource_requirement(rr_type, resource)?; } @@ -513,11 +520,7 @@ impl ResourceRequirementsExt for PodSpec { Ok(()) } - fn check_limit_to_request_ratio( - &self, - resource: &ComputeResource, - ratio: f32, - ) -> Result<(), ResourceRequirementsError> { + fn check_limit_to_request_ratio(&self, resource: &ComputeResource, ratio: f32) -> Result<()> { for container in &self.containers { container.check_limit_to_request_ratio(resource, ratio)?; } @@ -528,7 +531,7 @@ impl ResourceRequirementsExt for PodSpec { #[cfg(test)] mod tests { - use crate::builder::resources::ResourceRequirementsBuilder; + use crate::builder::pod::resources::ResourceRequirementsBuilder; use crate::commons::resources::{PvcConfig, PvcConfigFragment, Resources, ResourcesFragment}; use crate::config::{ fragment::{self, Fragment}, diff --git a/crates/stackable-operator/src/commons/s3.rs b/crates/stackable-operator/src/commons/s3.rs index 8a8bc94d6..fd2bb8711 100644 --- a/crates/stackable-operator/src/commons/s3.rs +++ b/crates/stackable-operator/src/commons/s3.rs @@ -7,13 +7,32 @@ use kube::CustomResource; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use snafu::{ResultExt, Snafu}; use crate::{ client::Client, commons::{authentication::tls::Tls, secret_class::SecretClassVolume}, - error::{self, OperatorResult}, }; +type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("missing S3Connection {resource_name:?} in namespace {namespace:?}"))] + MissingS3Connection { + source: crate::client::Error, + resource_name: String, + namespace: String, + }, + + #[snafu(display("missing S3Bucket {resource_name:?} in namespace {namespace:?}"))] + MissingS3Bucket { + source: crate::client::Error, + resource_name: String, + namespace: String, + }, +} + /// S3 bucket specification containing the bucket name and an inlined or referenced connection specification. /// Learn more on the [S3 concept documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/s3). #[derive( @@ -50,22 +69,19 @@ impl S3BucketSpec { resource_name: &str, client: &Client, namespace: &str, - ) -> OperatorResult { + ) -> Result { client .get::(resource_name, namespace) .await .map(|crd| crd.spec) - .map_err(|_source| error::Error::MissingS3Bucket { - name: resource_name.to_string(), + .context(MissingS3BucketSnafu { + resource_name, + namespace, }) } /// Map &self to an [InlinedS3BucketSpec] by obtaining connection spec from the K8S API service if necessary - pub async fn inlined( - &self, - client: &Client, - namespace: &str, - ) -> OperatorResult { + pub async fn inlined(&self, client: &Client, namespace: &str) -> Result { match self.connection.as_ref() { Some(connection_def) => Ok(InlinedS3BucketSpec { connection: Some(connection_def.resolve(client, namespace).await?), @@ -110,11 +126,7 @@ pub enum S3BucketDef { impl S3BucketDef { /// Returns an [InlinedS3BucketSpec]. - pub async fn resolve( - &self, - client: &Client, - namespace: &str, - ) -> OperatorResult { + pub async fn resolve(&self, client: &Client, namespace: &str) -> Result { match self { S3BucketDef::Inline(s3_bucket) => s3_bucket.inlined(client, namespace).await, S3BucketDef::Reference(s3_bucket) => { @@ -139,11 +151,7 @@ pub enum S3ConnectionDef { impl S3ConnectionDef { /// Returns an [S3ConnectionSpec]. - pub async fn resolve( - &self, - client: &Client, - namespace: &str, - ) -> OperatorResult { + pub async fn resolve(&self, client: &Client, namespace: &str) -> Result { match self { S3ConnectionDef::Inline(s3_connection_spec) => Ok(s3_connection_spec.clone()), S3ConnectionDef::Reference(s3_conn_reference) => { @@ -206,13 +214,14 @@ impl S3ConnectionSpec { resource_name: &str, client: &Client, namespace: &str, - ) -> OperatorResult { + ) -> Result { client .get::(resource_name, namespace) .await .map(|conn| conn.spec) - .map_err(|_source| error::Error::MissingS3Connection { - name: resource_name.to_string(), + .context(MissingS3ConnectionSnafu { + resource_name, + namespace, }) } diff --git a/crates/stackable-operator/src/commons/secret_class.rs b/crates/stackable-operator/src/commons/secret_class.rs index 0f19f69ea..a3e4825ec 100644 --- a/crates/stackable-operator/src/commons/secret_class.rs +++ b/crates/stackable-operator/src/commons/secret_class.rs @@ -3,7 +3,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; -use crate::builder::{ +use crate::builder::pod::volume::{ SecretOperatorVolumeSourceBuilder, SecretOperatorVolumeSourceBuilderError, VolumeBuilder, }; diff --git a/crates/stackable-operator/src/cpu.rs b/crates/stackable-operator/src/cpu.rs index e59ab7869..8fae864d2 100644 --- a/crates/stackable-operator/src/cpu.rs +++ b/crates/stackable-operator/src/cpu.rs @@ -7,8 +7,27 @@ use std::{ use k8s_openapi::apimachinery::pkg::api::resource::Quantity; use serde::{de::Visitor, Deserialize, Serialize}; - -use crate::error::{Error, OperatorResult}; +use snafu::{ResultExt, Snafu}; + +pub type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("unsupported precision {value:?}. Kubernetes doesn't allow you to specify CPU resources with a precision finer than 1m. Because of this, it's useful to specify CPU units less than 1.0 or 1000m using the milliCPU form; for example, 5m rather than 0.005"))] + UnsupportedCpuQuantityPrecision { value: String }, + + #[snafu(display("invalid cpu integer quantity {value:?}"))] + InvalidCpuIntQuantity { + source: std::num::ParseIntError, + value: String, + }, + + #[snafu(display("invalid cpu float quantity {value:?}"))] + InvalidCpuFloatQuantity { + source: std::num::ParseFloatError, + value: String, + }, +} /// A representation of CPU quantities with milli precision. /// Supports conversion from [`Quantity`]. @@ -88,28 +107,30 @@ impl FromStr for CpuQuantity { /// For the float, only milli-precision is supported. /// Using more precise values will trigger an error, and using any other /// unit than 'm' or None will also trigger an error. - fn from_str(q: &str) -> OperatorResult { + fn from_str(q: &str) -> Result { let start_of_unit = q.find(|c: char| c != '.' && !c.is_numeric()); if let Some(start_of_unit) = start_of_unit { let (value, unit) = q.split_at(start_of_unit); if unit != "m" { - return Err(Error::UnsupportedCpuQuantityPrecision { + return UnsupportedCpuQuantityPrecisionSnafu { value: q.to_owned(), - }); + } + .fail(); } - let cpu_millis: usize = value.parse().map_err(|_| Error::InvalidCpuQuantity { + let cpu_millis: usize = value.parse().context(InvalidCpuIntQuantitySnafu { value: q.to_owned(), })?; Ok(Self::from_millis(cpu_millis)) } else { - let cpus = q.parse::().map_err(|_| Error::InvalidCpuQuantity { + let cpus = q.parse::().context(InvalidCpuFloatQuantitySnafu { value: q.to_owned(), })?; let millis_float = cpus * 1000.; if millis_float != millis_float.round() { - return Err(Error::UnsupportedCpuQuantityPrecision { + return UnsupportedCpuQuantityPrecisionSnafu { value: q.to_owned(), - }); + } + .fail(); } Ok(Self::from_millis(millis_float as usize)) } diff --git a/crates/stackable-operator/src/crd.rs b/crates/stackable-operator/src/crd.rs index b2638c51e..d0610a478 100644 --- a/crates/stackable-operator/src/crd.rs +++ b/crates/stackable-operator/src/crd.rs @@ -4,8 +4,8 @@ use derivative::Derivative; use schemars::JsonSchema; use semver::Version; use serde::{Deserialize, Serialize}; +use snafu::{ResultExt, Snafu}; -use crate::error::{Error, OperatorResult}; use crate::yaml; use std::fs::File; use std::io::Write; @@ -14,6 +14,29 @@ use std::path::Path; const DOCS_HOME_URL_PLACEHOLDER: &str = "DOCS_BASE_URL_PLACEHOLDER"; const DOCS_HOME_BASE_URL: &str = "https://docs.stackable.tech/home"; +type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("cannot parse version {version:?} as a semantic version"))] + InvalidSemverVersion { + source: semver::Error, + version: String, + }, + + #[snafu(display("error converting CRD byte array to UTF-8"))] + ConvertByteArrayToUtf8 { source: std::string::FromUtf8Error }, + + #[snafu(display("failed to serialize YAML"))] + SerializeYaml { source: yaml::Error }, + + #[snafu(display("failed to write YAML"))] + WriteYamlSchema { source: std::io::Error }, + + #[snafu(display("failed to create YAML file"))] + CreateYamlFile { source: std::io::Error }, +} + /// A reference to a product cluster (for example, a `ZookeeperCluster`) /// /// `namespace`'s defaulting only applies when retrieved via [`ClusterRef::namespace_relative_from`] @@ -74,12 +97,11 @@ pub trait HasApplication { } /// Takes an operator version and returns a docs version -fn docs_version(operator_version: &str) -> OperatorResult { +fn docs_version(operator_version: &str) -> Result { if operator_version == "0.0.0-dev" { Ok("nightly".to_owned()) } else { - let v = Version::parse(operator_version).map_err(|err| Error::InvalidSemverVersion { - source: err, + let v = Version::parse(operator_version).context(InvalidSemverVersionSnafu { version: operator_version.to_owned(), })?; Ok(format!("{}.{}", v.major, v.minor)) @@ -89,7 +111,7 @@ fn docs_version(operator_version: &str) -> OperatorResult { /// Given an operator version like 0.0.0-dev or 23.1.1, generate a docs home /// component base URL like `https://docs.stackable.tech/home/nightly/` or /// `https://docs.stackable.tech/home/23.1/`. -fn docs_home_versioned_base_url(operator_version: &str) -> OperatorResult { +fn docs_home_versioned_base_url(operator_version: &str) -> Result { Ok(format!( "{}/{}", DOCS_HOME_BASE_URL, @@ -103,35 +125,38 @@ 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, operator_version: &str) -> OperatorResult<()> + fn generate_yaml_schema(mut writer: W, operator_version: &str) -> Result<()> where W: Write, { let mut buffer = Vec::new(); - yaml::serialize_to_explicit_document(&mut buffer, &Self::crd())?; + yaml::serialize_to_explicit_document(&mut buffer, &Self::crd()) + .context(SerializeYamlSnafu)?; let yaml_schema = String::from_utf8(buffer) - .map_err(Error::CrdFromUtf8Error)? + .context(ConvertByteArrayToUtf8Snafu)? .replace( DOCS_HOME_URL_PLACEHOLDER, &docs_home_versioned_base_url(operator_version)?, ); - Ok(writer.write_all(yaml_schema.as_bytes())?) + writer + .write_all(yaml_schema.as_bytes()) + .context(WriteYamlSchemaSnafu) } /// 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, operator_version: &str) -> OperatorResult<()> { - let writer = File::create(path)?; + fn write_yaml_schema>(path: P, operator_version: &str) -> Result<()> { + let writer = File::create(path).context(CreateYamlFileSnafu)?; Self::generate_yaml_schema(writer, operator_version) } /// Generates a YAML CustomResourceDefinition and prints it to stdout. /// /// The printed YAML string is an explicit document with leading dashes (`---`). - fn print_yaml_schema(operator_version: &str) -> OperatorResult<()> { + fn print_yaml_schema(operator_version: &str) -> Result<()> { let writer = std::io::stdout(); Self::generate_yaml_schema(writer, operator_version) } @@ -139,10 +164,10 @@ pub trait CustomResourceExt: kube::CustomResourceExt { /// Returns the YAML schema of this CustomResourceDefinition as a string. /// /// The written YAML string is an explicit document with leading dashes (`---`). - fn yaml_schema(operator_version: &str) -> OperatorResult { + fn yaml_schema(operator_version: &str) -> Result { let mut writer = Vec::new(); Self::generate_yaml_schema(&mut writer, operator_version)?; - String::from_utf8(writer).map_err(Error::CrdFromUtf8Error) + String::from_utf8(writer).context(ConvertByteArrayToUtf8Snafu) } } diff --git a/crates/stackable-operator/src/error.rs b/crates/stackable-operator/src/error.rs deleted file mode 100644 index f00a6e4f6..000000000 --- a/crates/stackable-operator/src/error.rs +++ /dev/null @@ -1,131 +0,0 @@ -use crate::{kvp::LabelError, product_config_utils}; -use std::path::PathBuf; - -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error("Failed to serialize YAML: {source}")] - YamlSerializationError { - #[from] - source: serde_yaml::Error, - }, - - #[error("Kubernetes reported error: {source}")] - KubeError { - #[from] - source: kube::Error, - }, - - #[error("Kubernetes failed to delete object: {source}")] - KubeDeleteError { - #[from] - source: kube::runtime::wait::delete::Error, - }, - - #[error("Object is missing key: {key}")] - MissingObjectKey { key: &'static str }, - - #[error("Label is missing: {label}")] - MissingLabel { label: &'static str }, - - #[error( - "Label contains unexpected content. \ - Expected: {label}={expected_content}, \ - actual: {label}={actual_content}" - )] - UnexpectedLabelContent { - label: &'static str, - expected_content: String, - actual_content: String, - }, - - #[error("LabelSelector is invalid: {message}")] - InvalidLabelSelector { message: String }, - - #[error("Role [{role}] is missing. This should not happen. Will requeue.")] - MissingRole { role: String }, - - #[error("RoleGroup [{role_group}] for Role [{role}] is missing. This may happen after custom resource changes. Will requeue.")] - MissingRoleGroup { role: String, role_group: String }, - - #[error( - "A required File is missing. Not found in any of the following locations: {search_path:?}" - )] - RequiredFileMissing { search_path: Vec }, - - #[error("Failed to load ProductConfig: {source}")] - ProductConfigLoadError { - #[source] - source: Box, - }, - - #[error("ProductConfig Framework reported error: {source}")] - ProductConfigError { - #[from] - source: product_config_utils::ConfigError, - }, - - #[error("IO Error: {source}")] - IoError { - #[from] - source: std::io::Error, - }, - - #[error("Error converting CRD byte array to UTF-8")] - CrdFromUtf8Error(#[source] std::string::FromUtf8Error), - - #[error("Missing OPA connect string in configmap [{configmap_name}]")] - MissingOpaConnectString { configmap_name: String }, - - #[error("Missing S3 connection [{name}]")] - MissingS3Connection { name: String }, - - #[error("Missing S3 bucket [{name}]")] - MissingS3Bucket { name: String }, - - #[error("Invalid quantity [{value}]")] - InvalidQuantity { value: String }, - - #[error("Invalid cpu quantity [{value}]")] - InvalidCpuQuantity { value: String }, - - #[error("Invalid quantity unit [{value}]")] - InvalidQuantityUnit { value: String }, - - #[error("No quantity unit provided for [{value}]")] - NoQuantityUnit { value: String }, - - #[error("Unsupported Precision [{value}]. Kubernetes doesn't allow you to specify CPU resources with a precision finer than 1m. Because of this, it's useful to specify CPU units less than 1.0 or 1000m using the milliCPU form; for example, 5m rather than 0.005")] - UnsupportedCpuQuantityPrecision { value: String }, - - #[error("Cannot scale down from kilobytes.")] - CannotScaleDownMemoryUnit, - - #[error("Cannot convert quantity [{value}] to Java heap.")] - CannotConvertToJavaHeap { value: String }, - - #[error("Cannot convert quantity [{value}] to Java heap value with unit [{target_unit}].")] - CannotConvertToJavaHeapValue { value: String, target_unit: String }, - - #[error("container name {container_name:?} is invalid: {violation}")] - InvalidContainerName { - container_name: String, - violation: String, - }, - - #[error("Cannot parse version [{version}] as a semantic version.")] - InvalidSemverVersion { - source: semver::Error, - version: String, - }, - - #[error("OIDC authentication details not specified. The AuthenticationClass {auth_class_name:?} uses an OIDC provider, you need to specify OIDC authentication details (such as client credentials) as well")] - OidcAuthenticationDetailsNotSpecified { auth_class_name: String }, - - #[error("failed to parse label: {source}")] - InvalidLabel { - #[from] - source: LabelError, - }, -} - -pub type OperatorResult = std::result::Result; diff --git a/crates/stackable-operator/src/kvp/annotation/mod.rs b/crates/stackable-operator/src/kvp/annotation/mod.rs index 749495f95..ce8a4e784 100644 --- a/crates/stackable-operator/src/kvp/annotation/mod.rs +++ b/crates/stackable-operator/src/kvp/annotation/mod.rs @@ -18,7 +18,7 @@ use std::{ use delegate::delegate; use crate::{ - builder::SecretOperatorVolumeScope, + builder::pod::volume::SecretOperatorVolumeScope, iter::TryFromIterator, kvp::{Key, KeyValuePair, KeyValuePairError, KeyValuePairs, KeyValuePairsError}, }; diff --git a/crates/stackable-operator/src/kvp/label/selector.rs b/crates/stackable-operator/src/kvp/label/selector.rs index 4656e9fcc..5ab187b97 100644 --- a/crates/stackable-operator/src/kvp/label/selector.rs +++ b/crates/stackable-operator/src/kvp/label/selector.rs @@ -1,23 +1,33 @@ use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector; +use snafu::Snafu; + +type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum SelectorError { + #[snafu(display("label selector with binary operator {operator:?} must have values"))] + LabelSelectorBinaryOperatorWithoutValues { operator: String }, + + #[snafu(display("label selector with unary operator {operator:?} must not have values"))] + LabelSelectorUnaryOperatorWithValues { operator: String }, + + #[snafu(display("labelSelector has an invalid operator {operator:?}"))] + LabelSelectorInvalidOperator { operator: String }, +} /// This trait extends the functionality of [`LabelSelector`]. /// /// Implementing this trait for any other type other than [`LabelSelector`] /// can result in unndefined behaviour. pub trait LabelSelectorExt { - type Error: std::error::Error; - /// Takes a [`LabelSelector`] and converts it to a String that can be used /// in Kubernetes API calls. It will return an error if the LabelSelector /// contains illegal things (e.g. an `Exists` operator with a value). - fn to_query_string(&self) -> Result; + fn to_query_string(&self) -> Result; } impl LabelSelectorExt for LabelSelector { - // NOTE (Techassi): This should be its own error - type Error = crate::error::Error; - - fn to_query_string(&self) -> Result { + fn to_query_string(&self) -> Result { let mut query_string = String::new(); // match_labels are the "old" part of LabelSelectors. @@ -38,62 +48,58 @@ impl LabelSelectorExt for LabelSelector { // Match expressions are more complex than match labels, both can appear in the same API call // They support these operators: "In", "NotIn", "Exists" and "DoesNotExist" let expressions = self.match_expressions.as_ref().map(|requirements| { - // If we had match_labels AND we have match_expressions we need to separate those two - // with a comma. - if !requirements.is_empty() && !query_string.is_empty() { - query_string.push(','); - } - - // Here we map over all requirements (which might be empty) and for each of the requirements - // we create a Result with the Ok variant being the converted match expression - // We then collect those Results into a single Result with the Error being the _first_ error. - // This, unfortunately means, that we'll throw away all but one error. - // TODO: Return all errors in one go: https://github.com/stackabletech/operator-rs/issues/127 - let expression_string: Result, crate::error::Error> = requirements - .iter() - .map(|requirement| match requirement.operator.as_str() { - // In and NotIn can be handled the same, they both map to a simple "key OPERATOR (values)" string - operator @ "In" | operator @ "NotIn" => match &requirement.values { - Some(values) if !values.is_empty() => Ok(format!( - "{} {} ({})", - requirement.key, - operator.to_ascii_lowercase(), - values.join(", ") - )), - _ => Err(crate::error::Error::InvalidLabelSelector { - message: format!( - "LabelSelector has no or empty values for [{operator}] operator" - ), - }), - }, - // "Exists" is just the key and nothing else, if values have been specified it's an error - "Exists" => match &requirement.values { - Some(values) if !values.is_empty() => Err( - crate::error::Error::InvalidLabelSelector { - message: "LabelSelector has [Exists] operator with values, this is not legal".to_string(), - }), - _ => Ok(requirement.key.to_string()), - }, - // "DoesNotExist" is similar to "Exists" but it is preceded by an exclamation mark - "DoesNotExist" => match &requirement.values { - Some(values) if !values.is_empty() => Err( - crate::error::Error::InvalidLabelSelector { - message: "LabelSelector has [DoesNotExist] operator with values, this is not legal".to_string(), + // If we had match_labels AND we have match_expressions we need to separate those two + // with a comma. + if !requirements.is_empty() && !query_string.is_empty() { + query_string.push(','); + } + + // Here we map over all requirements (which might be empty) and for each of the requirements + // we create a Result with the Ok variant being the converted match expression + // We then collect those Results into a single Result with the Error being the _first_ error. + // This, unfortunately means, that we'll throw away all but one error. + // TODO: Return all errors in one go: https://github.com/stackabletech/operator-rs/issues/127 + let expression_string: Result> = requirements + .iter() + .map(|requirement| match requirement.operator.as_str() { + // In and NotIn can be handled the same, they both map to a simple "key OPERATOR (values)" string + operator @ "In" | operator @ "NotIn" => match &requirement.values { + Some(values) if !values.is_empty() => Ok(format!( + "{} {} ({})", + requirement.key, + operator.to_ascii_lowercase(), + values.join(", ") + )), + _ => Err(SelectorError::LabelSelectorBinaryOperatorWithoutValues { + operator: operator.to_owned(), }), - _ => Ok(format!("!{}", requirement.key)) - } - op => { - Err( - crate::error::Error::InvalidLabelSelector { - message: format!("LabelSelector has illegal/unknown operator [{op}]") - }) - } - }) - .collect(); - - expression_string - - }); + }, + // "Exists" is just the key and nothing else, if values have been specified it's an error + operator @ "Exists" => match &requirement.values { + Some(values) if !values.is_empty() => { + Err(SelectorError::LabelSelectorUnaryOperatorWithValues { + operator: operator.to_owned(), + }) + } + _ => Ok(requirement.key.to_string()), + }, + // "DoesNotExist" is similar to "Exists" but it is preceded by an exclamation mark + operator @ "DoesNotExist" => match &requirement.values { + Some(values) if !values.is_empty() => { + Err(SelectorError::LabelSelectorUnaryOperatorWithValues { + operator: operator.to_owned(), + }) + } + _ => Ok(format!("!{key}", key = requirement.key)), + }, + operator => Err(SelectorError::LabelSelectorInvalidOperator { + operator: operator.to_owned(), + }), + }) + .collect(); + + expression_string + }); if let Some(expressions) = expressions.transpose()? { query_string.push_str(&expressions.join(",")); diff --git a/crates/stackable-operator/src/lib.rs b/crates/stackable-operator/src/lib.rs index 86eb247b4..6c748bb6e 100644 --- a/crates/stackable-operator/src/lib.rs +++ b/crates/stackable-operator/src/lib.rs @@ -6,7 +6,6 @@ pub mod commons; pub mod config; pub mod cpu; pub mod crd; -pub mod error; pub mod helm; pub mod iter; pub mod kvp; diff --git a/crates/stackable-operator/src/logging/k8s_events.rs b/crates/stackable-operator/src/logging/k8s_events.rs index 341068e44..82dfdc032 100644 --- a/crates/stackable-operator/src/logging/k8s_events.rs +++ b/crates/stackable-operator/src/logging/k8s_events.rs @@ -154,24 +154,25 @@ mod message { mod tests { use k8s_openapi::api::core::v1::ConfigMap; use kube::runtime::reflector::ObjectRef; + use snafu::Snafu; use strum::EnumDiscriminants; use super::{error_to_event, ReconcilerError}; - #[derive(Debug, thiserror::Error, EnumDiscriminants)] + #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(strum::IntoStaticStr))] enum ErrorFoo { - #[error("bar failed")] + #[snafu(display("bar failed"))] Bar { source: ErrorBar }, } - #[derive(Debug, thiserror::Error)] + #[derive(Snafu, Debug)] enum ErrorBar { - #[error("baz failed")] + #[snafu(display("baz failed"))] Baz { source: ErrorBaz }, } - #[derive(Debug, thiserror::Error)] + #[derive(Snafu, Debug)] enum ErrorBaz { - #[error("couldn't find chocolate")] + #[snafu(display("couldn't find chocolate"))] NoChocolate { descriptor: ObjectRef }, } impl ErrorFoo { diff --git a/crates/stackable-operator/src/memory.rs b/crates/stackable-operator/src/memory.rs index d50c19d32..f62f5cc36 100644 --- a/crates/stackable-operator/src/memory.rs +++ b/crates/stackable-operator/src/memory.rs @@ -10,8 +10,8 @@ use k8s_openapi::apimachinery::pkg::api::resource::Quantity; use serde::{de::Visitor, Deserialize, Serialize}; +use snafu::{OptionExt, ResultExt, Snafu}; -use crate::error::{Error, OperatorResult}; use std::{ fmt::Display, iter::Sum, @@ -19,6 +19,34 @@ use std::{ str::FromStr, }; +pub type Result = std::result::Result; + +#[derive(Debug, PartialEq, Snafu)] +pub enum Error { + #[snafu(display("cannot convert quantity {value:?} to Java heap"))] + CannotConvertToJavaHeap { value: String }, + + #[snafu(display( + "cannot convert quantity {value:?} to Java heap value with unit {target_unit:?}" + ))] + CannotConvertToJavaHeapValue { value: String, target_unit: String }, + + #[snafu(display("cannot scale down from kilobytes"))] + CannotScaleDownMemoryUnit, + + #[snafu(display("invalid quantity {value:?}"))] + InvalidQuantity { + source: std::num::ParseFloatError, + value: String, + }, + + #[snafu(display("invalid quantity unit {value:?}"))] + InvalidQuantityUnit { value: String }, + + #[snafu(display("no quantity unit provided for {value:?}"))] + NoQuantityUnit { value: String }, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)] pub enum BinaryMultiple { Kibi, @@ -62,7 +90,7 @@ impl BinaryMultiple { impl FromStr for BinaryMultiple { type Err = Error; - fn from_str(q: &str) -> OperatorResult { + fn from_str(q: &str) -> Result { match q { "Ki" => Ok(BinaryMultiple::Kibi), "Mi" => Ok(BinaryMultiple::Mebi), @@ -104,7 +132,7 @@ impl Display for BinaryMultiple { since = "0.33.0", note = "use \"-Xmx\" + MemoryQuantity::try_from(quantity).scale_to(unit).format_for_java()" )] -pub fn to_java_heap(q: &Quantity, factor: f32) -> OperatorResult { +pub fn to_java_heap(q: &Quantity, factor: f32) -> Result { let scaled = (q.0.parse::()? * factor).scale_for_java(); if scaled.value < 1.0 { Err(Error::CannotConvertToJavaHeap { @@ -134,11 +162,7 @@ pub fn to_java_heap(q: &Quantity, factor: f32) -> OperatorResult { since = "0.33.0", note = "use (MemoryQuantity::try_from(quantity).scale_to(target_unit) * factor)" )] -pub fn to_java_heap_value( - q: &Quantity, - factor: f32, - target_unit: BinaryMultiple, -) -> OperatorResult { +pub fn to_java_heap_value(q: &Quantity, factor: f32, target_unit: BinaryMultiple) -> Result { let scaled = (q.0.parse::()? * factor) .scale_for_java() .scale_to(target_unit); @@ -146,7 +170,7 @@ pub fn to_java_heap_value( if scaled.value < 1.0 { Err(Error::CannotConvertToJavaHeapValue { value: q.0.to_owned(), - target_unit: format!("{target_unit:?}"), + target_unit: target_unit.to_string(), }) } else { Ok(scaled.value as u32) @@ -189,7 +213,7 @@ impl MemoryQuantity { } /// Scale down the unit by one order of magnitude, i.e. GB to MB. - fn scale_down_unit(&self) -> OperatorResult { + fn scale_down_unit(&self) -> Result { match self.unit { BinaryMultiple::Kibi => Err(Error::CannotScaleDownMemoryUnit), BinaryMultiple::Mebi => Ok(self.scale_to(BinaryMultiple::Kibi)), @@ -219,7 +243,7 @@ impl MemoryQuantity { /// If the MemoryQuantity value is smaller than 1 (starts with a zero), convert it to a smaller /// unit until the non fractional part of the value is not zero anymore. /// This can fail if the quantity is smaller than 1kB. - fn ensure_no_zero(&self) -> OperatorResult { + fn ensure_no_zero(&self) -> Result { if self.value < 1. { self.scale_down_unit()?.ensure_no_zero() } else { @@ -231,7 +255,7 @@ impl MemoryQuantity { /// This is done by picking smaller units until the fractional part is smaller than the tolerated /// rounding loss, and then rounding down. /// This can fail if the tolerated rounding loss is less than 1kB. - fn ensure_integer(&self, tolerated_rounding_loss: MemoryQuantity) -> OperatorResult { + fn ensure_integer(&self, tolerated_rounding_loss: MemoryQuantity) -> Result { let fraction_memory = MemoryQuantity { value: self.value.fract(), unit: self.unit, @@ -249,7 +273,7 @@ impl MemoryQuantity { /// The original quantity may be rounded down to achive a compact, natural number representation. /// This rounding may cause the quantity to shrink by up to 20MB. /// Useful to set memory quantities as JVM paramters. - pub fn format_for_java(&self) -> OperatorResult { + pub fn format_for_java(&self) -> Result { let m = self .scale_to_at_most_gb() // Java Heap only supports specifying kb, mb or gb .ensure_no_zero()? // We don't want 0.9 or 0.2 @@ -340,15 +364,15 @@ impl<'de> Deserialize<'de> for MemoryQuantity { impl FromStr for MemoryQuantity { type Err = Error; - fn from_str(q: &str) -> OperatorResult { + fn from_str(q: &str) -> Result { let start_of_unit = q.find(|c: char| c != '.' && !c.is_numeric()) - .ok_or(Error::NoQuantityUnit { + .context(NoQuantityUnitSnafu { value: q.to_owned(), })?; let (value, unit) = q.split_at(start_of_unit); Ok(MemoryQuantity { - value: value.parse::().map_err(|_| Error::InvalidQuantity { + value: value.parse::().context(InvalidQuantitySnafu { value: q.to_owned(), })?, unit: unit.parse()?, @@ -459,7 +483,7 @@ impl Eq for MemoryQuantity {} impl TryFrom for MemoryQuantity { type Error = Error; - fn try_from(quantity: Quantity) -> OperatorResult { + fn try_from(quantity: Quantity) -> Result { Self::try_from(&quantity) } } @@ -467,7 +491,7 @@ impl TryFrom for MemoryQuantity { impl TryFrom<&Quantity> for MemoryQuantity { type Error = Error; - fn try_from(quantity: &Quantity) -> OperatorResult { + fn try_from(quantity: &Quantity) -> Result { quantity.0.parse() } } diff --git a/crates/stackable-operator/src/product_config_utils.rs b/crates/stackable-operator/src/product_config_utils.rs index e3c86a89b..88aa8833e 100644 --- a/crates/stackable-operator/src/product_config_utils.rs +++ b/crates/stackable-operator/src/product_config_utils.rs @@ -3,26 +3,31 @@ use std::collections::{BTreeMap, HashMap}; use product_config::{types::PropertyNameKind, ProductConfigManager, PropertyValidationResult}; use schemars::JsonSchema; use serde::Serialize; -use snafu::Snafu; +use snafu::{ResultExt, Snafu}; use tracing::{debug, error, warn}; -use crate::{ - error::{Error, OperatorResult}, - role_utils::{CommonConfiguration, Role}, -}; +use crate::role_utils::{CommonConfiguration, Role}; + +type Result = std::result::Result; #[derive(Debug, PartialEq, Snafu)] -pub enum ConfigError { - #[snafu(display("Invalid configuration found: {reason}"))] - InvalidConfiguration { reason: String }, +pub enum Error { + #[snafu(display("invalid configuration found"))] + InvalidConfiguration { + source: product_config::error::Error, + }, - #[snafu(display("Collected product config validation errors: {collected_errors:?}"))] + #[snafu(display("collected product config validation errors: {collected_errors:?}"))] ProductConfigErrors { collected_errors: Vec, }, -} -pub type ConfigResult = std::result::Result; + #[snafu(display("missing role {role:?}. This should not happen. Will requeue."))] + MissingRole { role: String }, + + #[snafu(display("missing roleGroup {role_group:?} for role {role:?}. This might happen after custom resource changes. Will requeue."))] + MissingRoleGroup { role: String, role_group: String }, +} /// This trait is used to compute configuration properties for products. /// @@ -50,20 +55,20 @@ pub trait Configuration { &self, resource: &Self::Configurable, role_name: &str, - ) -> ConfigResult>>; + ) -> Result>>; fn compute_cli( &self, resource: &Self::Configurable, role_name: &str, - ) -> ConfigResult>>; + ) -> Result>>; fn compute_files( &self, resource: &Self::Configurable, role_name: &str, file: &str, - ) -> ConfigResult>>; + ) -> Result>>; } impl Configuration for Box { @@ -73,7 +78,7 @@ impl Configuration for Box { &self, resource: &Self::Configurable, role_name: &str, - ) -> ConfigResult>> { + ) -> Result>> { T::compute_env(self, resource, role_name) } @@ -81,7 +86,7 @@ impl Configuration for Box { &self, resource: &Self::Configurable, role_name: &str, - ) -> ConfigResult>> { + ) -> Result>> { T::compute_cli(self, resource, role_name) } @@ -90,7 +95,7 @@ impl Configuration for Box { resource: &Self::Configurable, role_name: &str, file: &str, - ) -> ConfigResult>> { + ) -> Result>> { T::compute_files(self, resource, role_name, file) } } @@ -120,19 +125,21 @@ pub fn config_for_role_and_group<'a>( role: &str, group: &str, role_config: &'a ValidatedRoleConfigByPropertyKind, -) -> OperatorResult<&'a HashMap>> { +) -> Result<&'a HashMap>> { let result = match role_config.get(role) { None => { - return Err(Error::MissingRole { + return MissingRoleSnafu { role: role.to_string(), - }) + } + .fail() } Some(group_config) => match group_config.get(group) { None => { - return Err(Error::MissingRoleGroup { + return MissingRoleGroupSnafu { role: role.to_string(), role_group: group.to_string(), - }) + } + .fail() } Some(config_by_property_kind) => config_by_property_kind, }, @@ -154,7 +161,7 @@ pub fn config_for_role_and_group<'a>( pub fn transform_all_roles_to_config( resource: &T::Configurable, roles: HashMap, Role)>, -) -> ConfigResult +) -> Result where T: Configuration, U: Default + JsonSchema + Serialize, @@ -189,7 +196,7 @@ pub fn validate_all_roles_and_groups_config( product_config: &ProductConfigManager, ignore_warn: bool, ignore_err: bool, -) -> OperatorResult { +) -> Result { let mut result = HashMap::new(); for (role, role_group) in role_config { result.insert(role.to_string(), HashMap::new()); @@ -234,7 +241,7 @@ fn validate_role_and_group_config( product_config: &ProductConfigManager, ignore_warn: bool, ignore_err: bool, -) -> OperatorResult>> { +) -> Result>> { let mut result = HashMap::new(); for (property_name_kind, config) in properties_by_kind { @@ -245,9 +252,7 @@ fn validate_role_and_group_config( property_name_kind, config.clone().into_iter().collect::>(), ) - .map_err(|err| ConfigError::InvalidConfiguration { - reason: err.to_string(), - })?; + .context(InvalidConfigurationSnafu)?; let validated_config = process_validation_result(&validation_result, ignore_warn, ignore_err)?; @@ -275,7 +280,7 @@ fn process_validation_result( validation_result: &BTreeMap, ignore_warn: bool, ignore_err: bool, -) -> Result, ConfigError> { +) -> Result> { let mut properties = BTreeMap::new(); let mut collected_errors = Vec::new(); @@ -321,7 +326,7 @@ fn process_validation_result( } if !collected_errors.is_empty() { - return Err(ConfigError::ProductConfigErrors { collected_errors }); + return ProductConfigErrorsSnafu { collected_errors }.fail(); } Ok(properties) @@ -350,7 +355,7 @@ fn transform_role_to_config( role_name: &str, role: &Role, property_kinds: &[PropertyNameKind], -) -> ConfigResult +) -> Result where T: Configuration, U: Default + JsonSchema + Serialize, @@ -395,7 +400,7 @@ fn parse_role_config( role_name: &str, config: &CommonConfiguration, property_kinds: &[PropertyNameKind], -) -> ConfigResult>>> +) -> Result>>> where T: Configuration, { @@ -425,7 +430,7 @@ fn parse_cli_properties( resource: &::Configurable, role_name: &str, config: &CommonConfiguration, -) -> ConfigResult>> +) -> Result>> where T: Configuration, { @@ -444,7 +449,7 @@ fn parse_env_properties( resource: &::Configurable, role_name: &str, config: &CommonConfiguration, -) -> ConfigResult>> +) -> Result>> where T: Configuration, { @@ -464,7 +469,7 @@ fn parse_file_properties( role_name: &str, config: &CommonConfiguration, file: &str, -) -> ConfigResult>> +) -> Result>> where T: Configuration, { @@ -537,7 +542,7 @@ mod tests { &self, _resource: &Self::Configurable, _role_name: &str, - ) -> Result>, ConfigError> { + ) -> Result>> { let mut result = BTreeMap::new(); if let Some(env) = &self.env { result.insert("env".to_string(), Some(env.to_string())); @@ -549,7 +554,7 @@ mod tests { &self, _resource: &Self::Configurable, _role_name: &str, - ) -> Result>, ConfigError> { + ) -> Result>> { let mut result = BTreeMap::new(); if let Some(cli) = &self.cli { result.insert("cli".to_string(), Some(cli.to_string())); @@ -562,7 +567,7 @@ mod tests { _resource: &Self::Configurable, _role_name: &str, _file: &str, - ) -> Result>, ConfigError> { + ) -> Result>> { let mut result = BTreeMap::new(); if let Some(conf) = &self.conf { result.insert("file".to_string(), Some(conf.to_string())); diff --git a/crates/stackable-operator/src/product_logging/framework.rs b/crates/stackable-operator/src/product_logging/framework.rs index b3ba66d7c..491fcf53c 100644 --- a/crates/stackable-operator/src/product_logging/framework.rs +++ b/crates/stackable-operator/src/product_logging/framework.rs @@ -3,7 +3,7 @@ use std::{cmp, fmt::Write, ops::Mul}; use crate::{ - builder::ContainerBuilder, + builder::pod::container::ContainerBuilder, commons::product_image_selection::ResolvedProductImage, k8s_openapi::{ api::core::v1::{Container, ResourceRequirements}, @@ -51,7 +51,7 @@ pub const VECTOR_CONFIG_FILE: &str = "vector.yaml"; /// ``` /// use stackable_operator::{ /// builder::{ -/// PodBuilder, +/// pod::PodBuilder, /// meta::ObjectMetaBuilder, /// }, /// memory::{ @@ -106,7 +106,7 @@ pub fn calculate_log_volume_size_limit(max_log_files_size: &[MemoryQuantity]) -> /// /// ``` /// use stackable_operator::{ -/// builder::ContainerBuilder, +/// builder::pod::container::ContainerBuilder, /// config::fragment, /// product_logging, /// product_logging::spec::{ @@ -221,7 +221,7 @@ pub fn capture_shell_output( /// ``` /// use stackable_operator::{ /// builder::{ -/// ConfigMapBuilder, +/// configmap::ConfigMapBuilder, /// meta::ObjectMetaBuilder, /// }, /// config::fragment, @@ -342,7 +342,7 @@ log4j.appender.FILE.layout=org.apache.log4j.xml.XMLLayout /// ``` /// use stackable_operator::{ /// builder::{ -/// ConfigMapBuilder, +/// configmap::ConfigMapBuilder, /// meta::ObjectMetaBuilder, /// }, /// config::fragment, @@ -489,7 +489,7 @@ rootLogger.appenderRef.FILE.ref = FILE"#, /// ``` /// use stackable_operator::{ /// builder::{ -/// ConfigMapBuilder, +/// configmap::ConfigMapBuilder, /// meta::ObjectMetaBuilder, /// }, /// product_logging, @@ -627,7 +627,7 @@ pub fn create_logback_config( /// ``` /// use stackable_operator::{ /// builder::{ -/// ConfigMapBuilder, +/// configmap::ConfigMapBuilder, /// meta::ObjectMetaBuilder, /// }, /// product_logging, @@ -1024,10 +1024,8 @@ sinks: /// ``` /// use stackable_operator::{ /// builder::{ -/// ContainerBuilder, /// meta::ObjectMetaBuilder, -/// PodBuilder, -/// resources::ResourceRequirementsBuilder +/// pod::{container::ContainerBuilder, resources::ResourceRequirementsBuilder, PodBuilder}, /// }, /// product_logging::{self, framework:: {create_vector_shutdown_file_command, remove_vector_shutdown_file_command}}, /// utils::COMMON_BASH_TRAP_FUNCTIONS, @@ -1156,7 +1154,7 @@ kill $vector_pid /// /// ``` /// use stackable_operator::{ -/// builder::ContainerBuilder, +/// builder::pod::container::ContainerBuilder, /// product_logging, /// }; /// diff --git a/crates/stackable-operator/src/yaml.rs b/crates/stackable-operator/src/yaml.rs index 4ccdc0990..bea00be7b 100644 --- a/crates/stackable-operator/src/yaml.rs +++ b/crates/stackable-operator/src/yaml.rs @@ -2,8 +2,18 @@ use std::io::Write; use serde::ser; +use snafu::{ResultExt, Snafu}; -use crate::error::OperatorResult; +type Result = std::result::Result; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("failed to serialize YAML"))] + SerializeYaml { source: serde_yaml::Error }, + + #[snafu(display("failed to write YAML document separator"))] + WriteDocumentSeparator { source: std::io::Error }, +} /// Serializes the given data structure as an explicit YAML document and writes it to a [`Write`]. /// @@ -47,13 +57,16 @@ use crate::error::OperatorResult; /// # Errors /// /// Serialization can fail if `T`'s implementation of `Serialize` decides to return an error. -pub fn serialize_to_explicit_document(mut writer: W, value: &T) -> OperatorResult<()> +pub fn serialize_to_explicit_document(mut writer: W, value: &T) -> Result<()> where T: ser::Serialize, W: Write, { - writer.write_all(b"---\n")?; + writer + .write_all(b"---\n") + .context(WriteDocumentSeparatorSnafu)?; let mut serializer = serde_yaml::Serializer::new(writer); - serde_yaml::with::singleton_map_recursive::serialize(value, &mut serializer)?; + serde_yaml::with::singleton_map_recursive::serialize(value, &mut serializer) + .context(SerializeYamlSnafu)?; Ok(()) }