From b5c129f9219c50b05e75b84b32db37324d39b2bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 29 Aug 2024 14:46:30 +0200 Subject: [PATCH] Remove module name from error types --- .../src/builder/pod/container.rs | 4 +- .../src/commons/networking.rs | 6 +-- crates/stackable-operator/src/validation.rs | 39 +++++++++++-------- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs index 6933ae38c..1b928d703 100644 --- a/crates/stackable-operator/src/builder/pod/container.rs +++ b/crates/stackable-operator/src/builder/pod/container.rs @@ -9,7 +9,7 @@ use snafu::{ResultExt as _, Snafu}; use crate::{ commons::product_image_selection::ResolvedProductImage, - validation::{is_rfc_1123_label, ValidationErrors}, + validation::{self, is_rfc_1123_label}, }; type Result = std::result::Result; @@ -18,7 +18,7 @@ type Result = std::result::Result; pub enum Error { #[snafu(display("container name {container_name:?} is invalid"))] InvalidContainerName { - source: ValidationErrors, + source: validation::Errors, container_name: String, }, } diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs index 28aa1c83f..9e63c0a19 100644 --- a/crates/stackable-operator/src/commons/networking.rs +++ b/crates/stackable-operator/src/commons/networking.rs @@ -3,7 +3,7 @@ use std::{fmt::Display, ops::Deref}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::validation::{self, ValidationErrors}; +use crate::validation; /// A validated hostname type, for use in CRDs. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)] @@ -11,7 +11,7 @@ use crate::validation::{self, ValidationErrors}; pub struct Hostname(#[validate(regex(path = "validation::RFC_1123_SUBDOMAIN_REGEX"))] String); impl TryFrom for Hostname { - type Error = ValidationErrors; + type Error = validation::Errors; fn try_from(value: String) -> Result { validation::is_rfc_1123_subdomain(&value)?; @@ -44,7 +44,7 @@ pub struct KerberosRealmName( ); impl TryFrom for KerberosRealmName { - type Error = ValidationErrors; + type Error = validation::Errors; fn try_from(value: String) -> Result { validation::is_kerberos_realm_name(&value)?; diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs index c13f5a6ca..e49ca4c26 100644 --- a/crates/stackable-operator/src/validation.rs +++ b/crates/stackable-operator/src/validation.rs @@ -62,10 +62,13 @@ pub(crate) static KERBEROS_REALM_NAME_REGEX: LazyLock = LazyLock::new(|| .expect("failed to compile Kerberos realm name regex") }); +type Result = std::result::Result; + +/// A collection of errors discovered during validation. #[derive(Debug)] -pub struct ValidationErrors(Vec); +pub struct Errors(Vec); -impl Display for ValidationErrors { +impl Display for Errors { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for (i, error) in self.0.iter().enumerate() { let prefix = match i { @@ -77,10 +80,11 @@ impl Display for ValidationErrors { Ok(()) } } -impl std::error::Error for ValidationErrors {} +impl std::error::Error for Errors {} +/// A single validation error. #[derive(Debug, Snafu)] -pub enum ValidationError { +pub enum Error { #[snafu(transparent)] Regex { source: RegexError }, #[snafu(display("input is {length} bytes long but must be no more than {max_length}"))] @@ -121,7 +125,7 @@ impl Display for RegexError { impl std::error::Error for RegexError {} /// Returns [`Ok`] if `value`'s length fits within `max_length`. -fn validate_str_length(value: &str, max_length: usize) -> Result<(), ValidationError> { +fn validate_str_length(value: &str, max_length: usize) -> Result<(), Error> { if value.len() > max_length { TooLongSnafu { length: value.len(), @@ -139,7 +143,7 @@ fn validate_str_regex( regex: &'static Regex, error_msg: &'static str, examples: &'static [&'static str], -) -> Result<(), ValidationError> { +) -> Result<(), Error> { if regex.is_match(value) { Ok(()) } else { @@ -157,9 +161,7 @@ fn validate_str_regex( } /// Returns [`Ok`] if *all* validations are [`Ok`], otherwise returns all errors. -fn validate_all( - validations: impl IntoIterator>, -) -> Result<(), ValidationErrors> { +fn validate_all(validations: impl IntoIterator>) -> Result { let errors = validations .into_iter() .filter_map(|res| res.err()) @@ -167,12 +169,12 @@ fn validate_all( if errors.is_empty() { Ok(()) } else { - Err(ValidationErrors(errors)) + Err(Errors(errors)) } } /// Tests for a string that conforms to the definition of a subdomain in DNS (RFC 1123). -pub fn is_rfc_1123_subdomain(value: &str) -> Result<(), ValidationErrors> { +pub fn is_rfc_1123_subdomain(value: &str) -> Result { validate_all([ validate_str_length(value, RFC_1123_SUBDOMAIN_MAX_LENGTH), validate_str_regex( @@ -186,7 +188,7 @@ pub fn is_rfc_1123_subdomain(value: &str) -> Result<(), ValidationErrors> { /// Tests for a string that conforms to the definition of a label in DNS (RFC 1123). /// Maximum label length supported by k8s is 63 characters (minimum required). -pub fn is_rfc_1123_label(value: &str) -> Result<(), ValidationErrors> { +pub fn is_rfc_1123_label(value: &str) -> Result { validate_all([ validate_str_length(value, RFC_1123_LABEL_MAX_LENGTH), validate_str_regex( @@ -199,7 +201,7 @@ pub fn is_rfc_1123_label(value: &str) -> Result<(), ValidationErrors> { } /// Tests for a string that conforms to the definition of a label in DNS (RFC 1035). -pub fn is_rfc_1035_label(value: &str) -> Result<(), ValidationErrors> { +pub fn is_rfc_1035_label(value: &str) -> Result { validate_all([ validate_str_length(value, RFC_1035_LABEL_MAX_LENGTH), validate_str_regex( @@ -211,7 +213,10 @@ pub fn is_rfc_1035_label(value: &str) -> Result<(), ValidationErrors> { ]) } -pub fn is_kerberos_realm_name(value: &str) -> Result<(), ValidationErrors> { +/// Tests whether a string looks like a reasonable Kerberos realm name. +/// +/// This check is much stricter than krb5's own validation, +pub fn is_kerberos_realm_name(value: &str) -> Result { validate_all([validate_str_regex( value, &KERBEROS_REALM_NAME_REGEX, @@ -238,7 +243,7 @@ fn mask_trailing_dash(mut name: String) -> String { /// /// * `name` - is the name to check for validity /// * `prefix` - indicates whether `name` is just a prefix (ending in a dash, which would otherwise not be legal at the end) -pub fn name_is_dns_subdomain(name: &str, prefix: bool) -> Result<(), ValidationErrors> { +pub fn name_is_dns_subdomain(name: &str, prefix: bool) -> Result { let mut name = name.to_string(); if prefix { name = mask_trailing_dash(name); @@ -254,7 +259,7 @@ pub fn name_is_dns_subdomain(name: &str, prefix: bool) -> Result<(), ValidationE /// /// * `name` - is the name to check for validity /// * `prefix` - indicates whether `name` is just a prefix (ending in a dash, which would otherwise not be legal at the end) -pub fn name_is_dns_label(name: &str, prefix: bool) -> Result<(), ValidationErrors> { +pub fn name_is_dns_label(name: &str, prefix: bool) -> Result { let mut name = name.to_string(); if prefix { name = mask_trailing_dash(name); @@ -266,7 +271,7 @@ pub fn name_is_dns_label(name: &str, prefix: bool) -> Result<(), ValidationError /// Validates a namespace name. /// /// See [`name_is_dns_label`] for more information. -pub fn validate_namespace_name(name: &str, prefix: bool) -> Result<(), ValidationErrors> { +pub fn validate_namespace_name(name: &str, prefix: bool) -> Result { name_is_dns_label(name, prefix) }