Move validated Hostname type from secret-operator (#851)

* Refactor validation module to use typed errors * Remove separate regex_str parameter * Upstream Hostname from secret-operator * Add kerberos realm name type This used to use Hostname, but secret-op's Hostname had looser validation constraints * fmt * Changelog * Remove module name from error types * Update crates/stackable-operator/CHANGELOG.md Co-authored-by: Techassi <[email protected]> * Update crates/stackable-operator/src/commons/networking.rs Co-authored-by: Techassi <[email protected]> * Update crates/stackable-operator/src/commons/networking.rs Co-authored-by: Techassi <[email protected]> * Update crates/stackable-operator/src/validation.rs Co-authored-by: Techassi <[email protected]> * Update crates/stackable-operator/src/validation.rs Co-authored-by: Techassi <[email protected]> --------- Co-authored-by: Techassi <[email protected]>
stackabletech · Sep 3, 2024 · 96217cf · 96217cf
1 parent 68d0cb2
commit 96217cf
Show file tree

Hide file tree

Showing 5 changed files with 284 additions and 122 deletions.
diff --git 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
+
+- Add `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
 

diff --git 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::{self, is_rfc_1123_label},
 };
 
 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: validation::Errors,
         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/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;

diff --git a/crates/stackable-operator/src/commons/networking.rs b/crates/stackable-operator/src/commons/networking.rs
@@ -0,0 +1,76 @@
+use std::{fmt::Display, ops::Deref};
+
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
+
+use crate::validation;
+
+/// 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 = validation::Errors;
+
+    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
+    }
+}
+
+/// 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 = validation::Errors;
+
+    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
+    }
+}