From 1d1016dbb287f97adf8adfc48b5dfa8c568b3be8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Wed, 28 Aug 2024 19:35:26 +0200
Subject: [PATCH 01/12] Refactor validation module to use typed errors

---
 .../src/builder/pod/container.rs              |  75 +++----
 crates/stackable-operator/src/validation.rs   | 210 +++++++++++-------
 2 files changed, 164 insertions(+), 121 deletions(-)

diff --git a/crates/stackable-operator/src/builder/pod/container.rs b/crates/stackable-operator/src/builder/pod/container.rs
index 5bde690b..6933ae38 100644
--- a/crates/stackable-operator/src/builder/pod/container.rs
+++ b/crates/stackable-operator/src/builder/pod/container.rs
@@ -5,20 +5,21 @@ use k8s_openapi::api::core::v1::{
     LifecycleHandler, ObjectFieldSelector, Probe, ResourceRequirements, SecretKeySelector,
     SecurityContext, VolumeMount,
 };
-use snafu::Snafu;
+use snafu::{ResultExt as _, Snafu};
 
 use crate::{
-    commons::product_image_selection::ResolvedProductImage, validation::is_rfc_1123_label,
+    commons::product_image_selection::ResolvedProductImage,
+    validation::{is_rfc_1123_label, ValidationErrors},
 };
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, Snafu)]
 pub enum Error {
-    #[snafu(display("container name {container_name:?} is invalid: {violation:?}"))]
+    #[snafu(display("container name {container_name:?} is invalid"))]
     InvalidContainerName {
+        source: ValidationErrors,
         container_name: String,
-        violation: String,
     },
 }
 
@@ -276,16 +277,8 @@ 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<()> {
-        let validation_result = is_rfc_1123_label(name);
-
-        match validation_result {
-            Ok(_) => Ok(()),
-            Err(err) => Err(Error::InvalidContainerName {
-                container_name: name.to_owned(),
-                violation: err.join(", "),
-            }),
-        }
+    fn validate_container_name(container_name: &str) -> Result<()> {
+        is_rfc_1123_label(container_name).context(InvalidContainerNameSnafu { container_name })
     }
 }
 
@@ -497,19 +490,20 @@ mod tests {
             "lengthexceededlengthexceededlengthexceededlengthexceededlengthex";
         assert_eq!(long_container_name.len(), 64); // 63 characters is the limit for container names
         let result = ContainerBuilder::new(long_container_name);
-        match result {
-            Ok(_) => {
-                panic!("Container name exceeding 63 characters should cause an error");
+        match result
+            .err()
+            .expect("Container name exceeding 63 characters should cause an error")
+        {
+            Error::InvalidContainerName {
+                container_name,
+                source,
+            } => {
+                assert_eq!(container_name, long_container_name);
+                assert_eq!(
+                    source.to_string(),
+                    "input is 64 bytes long but must be no more than 63"
+                )
             }
-            Err(error) => match error {
-                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")
-                }
-            },
         }
         // One characters shorter name is valid
         let max_len_container_name: String = long_container_name.chars().skip(1).collect();
@@ -527,11 +521,11 @@ mod tests {
         assert!(ContainerBuilder::new("name-with-hyphen").is_ok());
         assert_container_builder_err(
             ContainerBuilder::new("ends-with-hyphen-"),
-            "regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?",
+            "regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\"",
         );
         assert_container_builder_err(
             ContainerBuilder::new("-starts-with-hyphen"),
-            "regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?",
+            "regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\"",
         );
     }
 
@@ -545,7 +539,7 @@ mod tests {
         assert!(ContainerBuilder::new("name_name").is_err());
         assert_container_builder_err(
             ContainerBuilder::new("name_name"),
-            "(e.g. 'example-label',  or '1-label-1', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
+            "(e.g. \"example-label\", or \"1-label-1\", regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\")",
         );
     }
 
@@ -573,19 +567,16 @@ mod tests {
         result: Result<ContainerBuilder, Error>,
         expected_err_contains: &str,
     ) {
-        match result {
-            Ok(_) => {
-                panic!("Container name exceeding 63 characters should cause an error");
+        match result
+            .err()
+            .expect("Container name exceeding 63 characters should cause an error")
+        {
+            Error::InvalidContainerName {
+                container_name: _,
+                source,
+            } => {
+                assert!(dbg!(source.to_string()).contains(dbg!(expected_err_contains)));
             }
-            Err(error) => match error {
-                Error::InvalidContainerName {
-                    container_name: _,
-                    violation,
-                } => {
-                    println!("{violation}");
-                    assert!(violation.contains(expected_err_contains));
-                }
-            },
         }
     }
 }
diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index 1c253b15..401dc6eb 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -9,10 +9,11 @@
 // This is adapted from Kubernetes.
 // See apimachinery/pkg/util/validation/validation.go, apimachinery/pkg/api/validation/generic.go and pkg/apis/core/validation/validation.go in the Kubernetes source
 
-use std::sync::LazyLock;
+use std::{fmt::Display, sync::LazyLock};
 
 use const_format::concatcp;
 use regex::Regex;
+use snafu::Snafu;
 
 const RFC_1123_LABEL_FMT: &str = "[a-z0-9]([-a-z0-9]*[a-z0-9])?";
 const RFC_1123_SUBDOMAIN_FMT: &str =
@@ -26,7 +27,7 @@ const RFC_1123_SUBDOMAIN_MAX_LENGTH: usize = 253;
 const RFC_1123_LABEL_MAX_LENGTH: usize = 63;
 
 const RFC_1035_LABEL_FMT: &str = "[a-z]([-a-z0-9]*[a-z0-9])?";
-const RFC_1035_LABEL_ERR_MSG: &str = "a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character";
+const RFC_1035_LABEL_ERROR_MSG: &str = "a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character";
 
 // This is a label's max length in DNS (RFC 1035)
 const RFC_1035_LABEL_MAX_LENGTH: usize = 63;
@@ -37,110 +38,161 @@ static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
         .expect("failed to compile RFC 1123 subdomain regex")
 });
 
+static RFC_1123_LABEL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
+    Regex::new(&format!("^{RFC_1123_LABEL_FMT}$")).expect("failed to compile RFC 1123 label regex")
+});
+
 static RFC_1035_LABEL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new(&format!("^{RFC_1035_LABEL_FMT}$")).expect("failed to compile RFC 1035 label regex")
 });
 
-/// Returns a formatted error message for maximum length violations.
-fn max_len_error(length: usize) -> String {
-    format!("must be no more than {length} characters")
+#[derive(Debug)]
+pub struct ValidationErrors(Vec<ValidationError>);
+
+impl Display for ValidationErrors {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        for (i, error) in self.0.iter().enumerate() {
+            let prefix = match i {
+                0 => "",
+                _ => ", ",
+            };
+            write!(f, "{prefix}{error}")?;
+        }
+        Ok(())
+    }
+}
+impl std::error::Error for ValidationErrors {}
+
+#[derive(Debug, Snafu)]
+pub enum ValidationError {
+    #[snafu(transparent)]
+    Regex { source: RegexError },
+    #[snafu(display("input is {length} bytes long but must be no more than {max_length}"))]
+    TooLong { length: usize, max_length: usize },
 }
 
-/// Returns a formatted error message for regex violations.
-///
-/// # Arguments
-///
-/// * `msg` - this is the main error message to return
-/// * `fmt` - this is the regular expression that did not match the input
-/// * `examples` - are optional well, formed examples that would match the regex
-fn regex_error(msg: &str, fmt: &str, examples: &[&str]) -> String {
-    if examples.is_empty() {
-        return format!("{msg} (regex used for validation is '{fmt}')");
-    }
+#[derive(Debug)]
+pub struct RegexError {
+    /// The primary error message.
+    msg: &'static str,
+    /// The regex that the input must match.
+    regex: &'static str,
+    /// Examples of valid inputs (if non-empty).
+    examples: &'static [&'static str],
+}
 
-    let mut msg = msg.to_string();
-    msg.push_str(" (e.g. ");
-    for (i, example) in examples.iter().enumerate() {
-        if i > 0 {
-            msg.push_str(" or ");
+impl Display for RegexError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let Self {
+            msg,
+            regex,
+            examples,
+        } = self;
+        write!(f, "{msg} (")?;
+        if !examples.is_empty() {
+            for (i, example) in examples.iter().enumerate() {
+                let prefix = match i {
+                    0 => "e.g.",
+                    _ => "or",
+                };
+                write!(f, "{prefix} {example:?}, ")?;
+            }
         }
-        msg.push('\'');
-        msg.push_str(example);
-        msg.push_str("', ");
+        write!(f, "regex used for validation is {regex:?})")
     }
-
-    msg.push_str("regex used for validation is '");
-    msg.push_str(fmt);
-    msg.push_str("')");
-    msg
 }
 
-/// 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<(), Vec<String>> {
-    let mut errors = vec![];
-    if value.len() > RFC_1123_SUBDOMAIN_MAX_LENGTH {
-        errors.push(max_len_error(RFC_1123_SUBDOMAIN_MAX_LENGTH))
+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> {
+    if value.len() > max_length {
+        TooLongSnafu {
+            length: value.len(),
+            max_length,
+        }
+        .fail()
+    } else {
+        Ok(())
     }
+}
 
-    if !RFC_1123_SUBDOMAIN_REGEX.is_match(value) {
-        errors.push(regex_error(
-            RFC_1123_SUBDOMAIN_ERROR_MSG,
-            RFC_1123_SUBDOMAIN_FMT,
-            &["example.com"],
-        ))
+/// Returns [`Ok`] if `value` matches `regex`.
+fn validate_str_regex(
+    value: &str,
+    regex: &'static Regex,
+    regex_str: &'static str,
+    error_msg: &'static str,
+    examples: &'static [&'static str],
+) -> Result<(), ValidationError> {
+    if regex.is_match(value) {
+        Ok(())
+    } else {
+        Err(RegexError {
+            msg: error_msg,
+            regex: regex_str,
+            examples,
+        }
+        .into())
     }
+}
 
+/// Returns [`Ok`] if *all* validations are [`Ok`], otherwise returns all errors.
+fn validate_all(
+    validations: impl IntoIterator<Item = Result<(), ValidationError>>,
+) -> Result<(), ValidationErrors> {
+    let errors = validations
+        .into_iter()
+        .filter_map(|res| res.err())
+        .collect::<Vec<_>>();
     if errors.is_empty() {
         Ok(())
     } else {
-        Err(errors)
+        Err(ValidationErrors(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> {
+    validate_all([
+        validate_str_length(value, RFC_1123_SUBDOMAIN_MAX_LENGTH),
+        validate_str_regex(
+            value,
+            &RFC_1123_SUBDOMAIN_REGEX,
+            RFC_1123_SUBDOMAIN_FMT,
+            RFC_1123_SUBDOMAIN_ERROR_MSG,
+            &["example.com"],
+        ),
+    ])
+}
+
 /// 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<(), Vec<String>> {
-    let mut errors = vec![];
-    if value.len() > RFC_1123_LABEL_MAX_LENGTH {
-        errors.push(max_len_error(RFC_1123_LABEL_MAX_LENGTH))
-    }
-
-    // Regex is identical to RFC 1123 subdomain
-    if !RFC_1123_SUBDOMAIN_REGEX.is_match(value) {
-        errors.push(regex_error(
+pub fn is_rfc_1123_label(value: &str) -> Result<(), ValidationErrors> {
+    validate_all([
+        validate_str_length(value, RFC_1123_LABEL_MAX_LENGTH),
+        validate_str_regex(
+            value,
+            &RFC_1123_LABEL_REGEX,
+            RFC_1123_LABEL_FMT,
             RFC_1123_LABEL_ERROR_MSG,
-            RFC_1123_SUBDOMAIN_FMT,
             &["example-label", "1-label-1"],
-        ))
-    }
-
-    if errors.is_empty() {
-        Ok(())
-    } else {
-        Err(errors)
-    }
+        ),
+    ])
 }
 
 /// 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<(), Vec<String>> {
-    let mut errors = vec![];
-    if value.len() > RFC_1035_LABEL_MAX_LENGTH {
-        errors.push(max_len_error(RFC_1035_LABEL_MAX_LENGTH))
-    }
-
-    if !RFC_1035_LABEL_REGEX.is_match(value) {
-        errors.push(regex_error(
-            RFC_1035_LABEL_ERR_MSG,
+pub fn is_rfc_1035_label(value: &str) -> Result<(), ValidationErrors> {
+    validate_all([
+        validate_str_length(value, RFC_1035_LABEL_MAX_LENGTH),
+        validate_str_regex(
+            value,
+            &RFC_1035_LABEL_REGEX,
             RFC_1035_LABEL_FMT,
+            RFC_1035_LABEL_ERROR_MSG,
             &["my-name", "abc-123"],
-        ))
-    }
-
-    if errors.is_empty() {
-        Ok(())
-    } else {
-        Err(errors)
-    }
+        ),
+    ])
 }
 
 // mask_trailing_dash replaces the final character of a string with a subdomain safe
@@ -161,7 +213,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<(), Vec<String>> {
+pub fn name_is_dns_subdomain(name: &str, prefix: bool) -> Result<(), ValidationErrors> {
     let mut name = name.to_string();
     if prefix {
         name = mask_trailing_dash(name);
@@ -177,7 +229,7 @@ pub fn name_is_dns_subdomain(name: &str, prefix: bool) -> Result<(), Vec<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_label(name: &str, prefix: bool) -> Result<(), Vec<String>> {
+pub fn name_is_dns_label(name: &str, prefix: bool) -> Result<(), ValidationErrors> {
     let mut name = name.to_string();
     if prefix {
         name = mask_trailing_dash(name);
@@ -189,7 +241,7 @@ pub fn name_is_dns_label(name: &str, prefix: bool) -> Result<(), Vec<String>> {
 /// Validates a namespace name.
 ///
 /// See [`name_is_dns_label`] for more information.
-pub fn validate_namespace_name(name: &str, prefix: bool) -> Result<(), Vec<String>> {
+pub fn validate_namespace_name(name: &str, prefix: bool) -> Result<(), ValidationErrors> {
     name_is_dns_label(name, prefix)
 }
 

From 3dcf7fdcfdd4b710ce68cc26daaae94a277c5b65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Wed, 28 Aug 2024 19:43:27 +0200
Subject: [PATCH 02/12] Remove separate regex_str parameter

---
 crates/stackable-operator/src/validation.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index 401dc6eb..43b25913 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -121,7 +121,6 @@ fn validate_str_length(value: &str, max_length: usize) -> Result<(), ValidationE
 fn validate_str_regex(
     value: &str,
     regex: &'static Regex,
-    regex_str: &'static str,
     error_msg: &'static str,
     examples: &'static [&'static str],
 ) -> Result<(), ValidationError> {
@@ -130,7 +129,11 @@ fn validate_str_regex(
     } else {
         Err(RegexError {
             msg: error_msg,
-            regex: regex_str,
+            regex: regex
+                .as_str()
+                // Clean up start/end-of-line markers
+                .trim_start_matches('^')
+                .trim_end_matches('$'),
             examples,
         }
         .into())
@@ -159,7 +162,6 @@ pub fn is_rfc_1123_subdomain(value: &str) -> Result<(), ValidationErrors> {
         validate_str_regex(
             value,
             &RFC_1123_SUBDOMAIN_REGEX,
-            RFC_1123_SUBDOMAIN_FMT,
             RFC_1123_SUBDOMAIN_ERROR_MSG,
             &["example.com"],
         ),
@@ -174,7 +176,6 @@ pub fn is_rfc_1123_label(value: &str) -> Result<(), ValidationErrors> {
         validate_str_regex(
             value,
             &RFC_1123_LABEL_REGEX,
-            RFC_1123_LABEL_FMT,
             RFC_1123_LABEL_ERROR_MSG,
             &["example-label", "1-label-1"],
         ),
@@ -188,7 +189,6 @@ pub fn is_rfc_1035_label(value: &str) -> Result<(), ValidationErrors> {
         validate_str_regex(
             value,
             &RFC_1035_LABEL_REGEX,
-            RFC_1035_LABEL_FMT,
             RFC_1035_LABEL_ERROR_MSG,
             &["my-name", "abc-123"],
         ),

From 9f460dad65f31389b39daf9a044c7b4fa0019575 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Wed, 28 Aug 2024 20:11:37 +0200
Subject: [PATCH 03/12] Upstream Hostname from secret-operator

---
 crates/stackable-operator/src/commons/mod.rs  |  1 +
 .../src/commons/networking.rs                 | 37 +++++++++++++++++++
 crates/stackable-operator/src/validation.rs   |  2 +-
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 crates/stackable-operator/src/commons/networking.rs

diff --git a/crates/stackable-operator/src/commons/mod.rs b/crates/stackable-operator/src/commons/mod.rs
index d85054f4..cc0e14f7 100644
--- a/crates/stackable-operator/src/commons/mod.rs
+++ b/crates/stackable-operator/src/commons/mod.rs
@@ -12,3 +12,4 @@ pub mod resources;
 pub mod s3;
 pub mod secret;
 pub mod secret_class;
+pub mod networking;
diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs
new file mode 100644
index 00000000..7cd8b04f
--- /dev/null
+++ b/crates/stackable-operator/src/commons/networking.rs
@@ -0,0 +1,37 @@
+use std::{fmt::Display, ops::Deref};
+
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
+
+use crate::validation::{self, ValidationErrors};
+
+/// A validated hostname type, for use in CRDs.
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
+#[serde(try_from = "String", into = "String")]
+pub struct Hostname(#[validate(regex(path = "validation::RFC_1123_SUBDOMAIN_REGEX"))] String);
+
+impl TryFrom<String> for Hostname {
+    type Error = ValidationErrors;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        validation::is_rfc_1123_subdomain(&value)?;
+        Ok(Hostname(value))
+    }
+}
+impl From<Hostname> for String {
+    fn from(value: Hostname) -> Self {
+        value.0
+    }
+}
+impl Display for Hostname {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+impl Deref for Hostname {
+    type Target = str;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index 43b25913..da676036 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -33,7 +33,7 @@ const RFC_1035_LABEL_ERROR_MSG: &str = "a DNS-1035 label must consist of lower c
 const RFC_1035_LABEL_MAX_LENGTH: usize = 63;
 
 // Lazily initialized regular expressions
-static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
+pub(crate) static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new(&format!("^{RFC_1123_SUBDOMAIN_FMT}$"))
         .expect("failed to compile RFC 1123 subdomain regex")
 });

From f09a7908cb6bd10344a26b4859cc3e795212bb3e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Thu, 29 Aug 2024 14:34:36 +0200
Subject: [PATCH 04/12] Add kerberos realm name type

This used to use Hostname, but secret-op's Hostname had looser
validation constraints
---
 .../src/commons/networking.rs                 | 33 +++++++++++++++++++
 crates/stackable-operator/src/validation.rs   | 25 ++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs
index 7cd8b04f..28aa1c83 100644
--- a/crates/stackable-operator/src/commons/networking.rs
+++ b/crates/stackable-operator/src/commons/networking.rs
@@ -35,3 +35,36 @@ impl Deref for Hostname {
         &self.0
     }
 }
+
+/// A validated kerberos realm name type, for use in CRDs.
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
+#[serde(try_from = "String", into = "String")]
+pub struct KerberosRealmName(
+    #[validate(regex(path = "validation::KERBEROS_REALM_NAME_REGEX"))] String,
+);
+
+impl TryFrom<String> for KerberosRealmName {
+    type Error = ValidationErrors;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        validation::is_kerberos_realm_name(&value)?;
+        Ok(KerberosRealmName(value))
+    }
+}
+impl From<KerberosRealmName> for String {
+    fn from(value: KerberosRealmName) -> Self {
+        value.0
+    }
+}
+impl Display for KerberosRealmName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+impl Deref for KerberosRealmName {
+    type Target = str;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index da676036..c13f5a6c 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -32,6 +32,17 @@ const RFC_1035_LABEL_ERROR_MSG: &str = "a DNS-1035 label must consist of lower c
 // This is a label's max length in DNS (RFC 1035)
 const RFC_1035_LABEL_MAX_LENGTH: usize = 63;
 
+// Technically Kerberos allows more realm names
+// (https://web.mit.edu/kerberos/krb5-1.21/doc/admin/realm_config.html#realm-name),
+// however, these are embedded in a lot of configuration files and other strings,
+// and will not always be quoted properly.
+//
+// Hence, restrict them to a reasonable subset. The convention is to use upper-case
+// DNS hostnames, so allow all characters used there.
+const KERBEROS_REALM_NAME_FMT: &str = "[-.a-zA-Z0-9]+";
+const KERBEROS_REALM_NAME_ERROR_MSG: &str =
+    "Kerberos realm name must only contain alphanumeric characters, '-', and '.'";
+
 // Lazily initialized regular expressions
 pub(crate) static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new(&format!("^{RFC_1123_SUBDOMAIN_FMT}$"))
@@ -46,6 +57,11 @@ static RFC_1035_LABEL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new(&format!("^{RFC_1035_LABEL_FMT}$")).expect("failed to compile RFC 1035 label regex")
 });
 
+pub(crate) static KERBEROS_REALM_NAME_REGEX: LazyLock<Regex> = LazyLock::new(|| {
+    Regex::new(&format!("^{KERBEROS_REALM_NAME_FMT}$"))
+        .expect("failed to compile Kerberos realm name regex")
+});
+
 #[derive(Debug)]
 pub struct ValidationErrors(Vec<ValidationError>);
 
@@ -195,6 +211,15 @@ pub fn is_rfc_1035_label(value: &str) -> Result<(), ValidationErrors> {
     ])
 }
 
+pub fn is_kerberos_realm_name(value: &str) -> Result<(), ValidationErrors> {
+    validate_all([validate_str_regex(
+        value,
+        &KERBEROS_REALM_NAME_REGEX,
+        KERBEROS_REALM_NAME_ERROR_MSG,
+        &["EXAMPLE.COM"],
+    )])
+}
+
 // mask_trailing_dash replaces the final character of a string with a subdomain safe
 // value if is a dash.
 fn mask_trailing_dash(mut name: String) -> String {

From c8fd2ef0aa49726edc4b04e719c2394a7c567663 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Thu, 29 Aug 2024 14:37:58 +0200
Subject: [PATCH 05/12] fmt

---
 crates/stackable-operator/src/commons/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/stackable-operator/src/commons/mod.rs b/crates/stackable-operator/src/commons/mod.rs
index cc0e14f7..7f145616 100644
--- a/crates/stackable-operator/src/commons/mod.rs
+++ b/crates/stackable-operator/src/commons/mod.rs
@@ -4,6 +4,7 @@ pub mod affinity;
 pub mod authentication;
 pub mod cluster_operation;
 pub mod listener;
+pub mod networking;
 pub mod opa;
 pub mod pdb;
 pub mod product_image_selection;
@@ -12,4 +13,3 @@ pub mod resources;
 pub mod s3;
 pub mod secret;
 pub mod secret_class;
-pub mod networking;

From 8ea2a2ea43d4ae981b2b70d42fb4707e0ebacd33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Thu, 29 Aug 2024 14:40:01 +0200
Subject: [PATCH 06/12] Changelog

---
 crates/stackable-operator/CHANGELOG.md | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md
index 64dbbfc0..b6e06187 100644
--- a/crates/stackable-operator/CHANGELOG.md
+++ b/crates/stackable-operator/CHANGELOG.md
@@ -4,11 +4,20 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Added `Hostname` and `KerberosRealmName` types extracted from secret-operator ([#851]).
+
+### Changed
+
+- BREAKING: `validation` module now uses typed errors ([#851]).
+
 ### Fixed
 
 - Fix the CRD description of `ClientAuthenticationDetails` to not contain internal Rust doc, but a public CRD description ([#846]).
 
 [#846]: https://github.com/stackabletech/operator-rs/pull/846
+[#851]: https://github.com/stackabletech/operator-rs/pull/851
 
 ## [0.74.0] - 2024-08-22
 

From b5c129f9219c50b05e75b84b32db37324d39b2bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?=
 <nat.roijezon@stackable.tech>
Date: Thu, 29 Aug 2024 14:46:30 +0200
Subject: [PATCH 07/12] 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 6933ae38..1b928d70 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<T, E = Error> = std::result::Result<T, E>;
@@ -18,7 +18,7 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 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 28aa1c83..9e63c0a1 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<String> for Hostname {
-    type Error = ValidationErrors;
+    type Error = validation::Errors;
 
     fn try_from(value: String) -> Result<Self, Self::Error> {
         validation::is_rfc_1123_subdomain(&value)?;
@@ -44,7 +44,7 @@ pub struct KerberosRealmName(
 );
 
 impl TryFrom<String> for KerberosRealmName {
-    type Error = ValidationErrors;
+    type Error = validation::Errors;
 
     fn try_from(value: String) -> Result<Self, Self::Error> {
         validation::is_kerberos_realm_name(&value)?;
diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index c13f5a6c..e49ca4c2 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<Regex> = LazyLock::new(||
         .expect("failed to compile Kerberos realm name regex")
 });
 
+type Result<T = (), E = Errors> = std::result::Result<T, E>;
+
+/// A collection of errors discovered during validation.
 #[derive(Debug)]
-pub struct ValidationErrors(Vec<ValidationError>);
+pub struct Errors(Vec<Error>);
 
-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<Item = Result<(), ValidationError>>,
-) -> Result<(), ValidationErrors> {
+fn validate_all(validations: impl IntoIterator<Item = Result<(), Error>>) -> 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)
 }
 

From 41fd8eb2fe8b695f545d5ef1785ee3ba0ddc86f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= <teo@nullable.se>
Date: Fri, 30 Aug 2024 11:56:49 +0200
Subject: [PATCH 08/12] Update crates/stackable-operator/CHANGELOG.md

Co-authored-by: Techassi <git@techassi.dev>
---
 crates/stackable-operator/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md
index b6e06187..24163101 100644
--- a/crates/stackable-operator/CHANGELOG.md
+++ b/crates/stackable-operator/CHANGELOG.md
@@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file.
 
 ### Added
 
-- Added `Hostname` and `KerberosRealmName` types extracted from secret-operator ([#851]).
+- Add `Hostname` and `KerberosRealmName` types extracted from secret-operator ([#851]).
 
 ### Changed
 

From b0207a3521bc32d0001625662a99c991a3625079 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= <teo@nullable.se>
Date: Fri, 30 Aug 2024 11:57:07 +0200
Subject: [PATCH 09/12] Update
 crates/stackable-operator/src/commons/networking.rs

Co-authored-by: Techassi <git@techassi.dev>
---
 crates/stackable-operator/src/commons/networking.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs
index 9e63c0a1..b3ffdbd1 100644
--- a/crates/stackable-operator/src/commons/networking.rs
+++ b/crates/stackable-operator/src/commons/networking.rs
@@ -18,16 +18,19 @@ impl TryFrom<String> for Hostname {
         Ok(Hostname(value))
     }
 }
+
 impl From<Hostname> for String {
     fn from(value: Hostname) -> Self {
         value.0
     }
 }
+
 impl Display for Hostname {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_str(&self.0)
     }
 }
+
 impl Deref for Hostname {
     type Target = str;
 

From 1874aca10588edecd7dedf315e4615bc1c8a4d20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= <teo@nullable.se>
Date: Fri, 30 Aug 2024 11:57:24 +0200
Subject: [PATCH 10/12] Update
 crates/stackable-operator/src/commons/networking.rs

Co-authored-by: Techassi <git@techassi.dev>
---
 crates/stackable-operator/src/commons/networking.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs
index b3ffdbd1..7081823b 100644
--- a/crates/stackable-operator/src/commons/networking.rs
+++ b/crates/stackable-operator/src/commons/networking.rs
@@ -54,16 +54,19 @@ impl TryFrom<String> for KerberosRealmName {
         Ok(KerberosRealmName(value))
     }
 }
+
 impl From<KerberosRealmName> for String {
     fn from(value: KerberosRealmName) -> Self {
         value.0
     }
 }
+
 impl Display for KerberosRealmName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_str(&self.0)
     }
 }
+
 impl Deref for KerberosRealmName {
     type Target = str;
 

From f76649d3d5866bd3003a3f4e5d45d2239703f889 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= <teo@nullable.se>
Date: Fri, 30 Aug 2024 11:57:47 +0200
Subject: [PATCH 11/12] Update crates/stackable-operator/src/validation.rs

Co-authored-by: Techassi <git@techassi.dev>
---
 crates/stackable-operator/src/validation.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index e49ca4c2..25ee0694 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -87,6 +87,7 @@ impl std::error::Error for Errors {}
 pub enum Error {
     #[snafu(transparent)]
     Regex { source: RegexError },
+
     #[snafu(display("input is {length} bytes long but must be no more than {max_length}"))]
     TooLong { length: usize, max_length: usize },
 }

From 4cffca45fb26b1d1bc5dd001f358a4f05b34615c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= <teo@nullable.se>
Date: Fri, 30 Aug 2024 11:58:02 +0200
Subject: [PATCH 12/12] Update crates/stackable-operator/src/validation.rs

Co-authored-by: Techassi <git@techassi.dev>
---
 crates/stackable-operator/src/validation.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/crates/stackable-operator/src/validation.rs b/crates/stackable-operator/src/validation.rs
index 25ee0694..6cf4dbaa 100644
--- a/crates/stackable-operator/src/validation.rs
+++ b/crates/stackable-operator/src/validation.rs
@@ -96,8 +96,10 @@ pub enum Error {
 pub struct RegexError {
     /// The primary error message.
     msg: &'static str,
+
     /// The regex that the input must match.
     regex: &'static str,
+
     /// Examples of valid inputs (if non-empty).
     examples: &'static [&'static str],
 }