Skip to content

Commit 1d1016d

Browse files
committed
Refactor validation module to use typed errors
1 parent 68d0cb2 commit 1d1016d

File tree

2 files changed

+164
-121
lines changed

2 files changed

+164
-121
lines changed

crates/stackable-operator/src/builder/pod/container.rs

Lines changed: 33 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,21 @@ use k8s_openapi::api::core::v1::{
55
LifecycleHandler, ObjectFieldSelector, Probe, ResourceRequirements, SecretKeySelector,
66
SecurityContext, VolumeMount,
77
};
8-
use snafu::Snafu;
8+
use snafu::{ResultExt as _, Snafu};
99

1010
use crate::{
11-
commons::product_image_selection::ResolvedProductImage, validation::is_rfc_1123_label,
11+
commons::product_image_selection::ResolvedProductImage,
12+
validation::{is_rfc_1123_label, ValidationErrors},
1213
};
1314

1415
type Result<T, E = Error> = std::result::Result<T, E>;
1516

16-
#[derive(Debug, PartialEq, Snafu)]
17+
#[derive(Debug, Snafu)]
1718
pub enum Error {
18-
#[snafu(display("container name {container_name:?} is invalid: {violation:?}"))]
19+
#[snafu(display("container name {container_name:?} is invalid"))]
1920
InvalidContainerName {
21+
source: ValidationErrors,
2022
container_name: String,
21-
violation: String,
2223
},
2324
}
2425

@@ -276,16 +277,8 @@ impl ContainerBuilder {
276277

277278
/// Validates a container name is according to the [RFC 1123](https://www.ietf.org/rfc/rfc1123.txt) standard.
278279
/// Returns [Ok] if the name is according to the standard, and [Err] if not.
279-
fn validate_container_name(name: &str) -> Result<()> {
280-
let validation_result = is_rfc_1123_label(name);
281-
282-
match validation_result {
283-
Ok(_) => Ok(()),
284-
Err(err) => Err(Error::InvalidContainerName {
285-
container_name: name.to_owned(),
286-
violation: err.join(", "),
287-
}),
288-
}
280+
fn validate_container_name(container_name: &str) -> Result<()> {
281+
is_rfc_1123_label(container_name).context(InvalidContainerNameSnafu { container_name })
289282
}
290283
}
291284

@@ -497,19 +490,20 @@ mod tests {
497490
"lengthexceededlengthexceededlengthexceededlengthexceededlengthex";
498491
assert_eq!(long_container_name.len(), 64); // 63 characters is the limit for container names
499492
let result = ContainerBuilder::new(long_container_name);
500-
match result {
501-
Ok(_) => {
502-
panic!("Container name exceeding 63 characters should cause an error");
493+
match result
494+
.err()
495+
.expect("Container name exceeding 63 characters should cause an error")
496+
{
497+
Error::InvalidContainerName {
498+
container_name,
499+
source,
500+
} => {
501+
assert_eq!(container_name, long_container_name);
502+
assert_eq!(
503+
source.to_string(),
504+
"input is 64 bytes long but must be no more than 63"
505+
)
503506
}
504-
Err(error) => match error {
505-
Error::InvalidContainerName {
506-
container_name,
507-
violation,
508-
} => {
509-
assert_eq!(container_name.as_str(), long_container_name);
510-
assert_eq!(violation.as_str(), "must be no more than 63 characters")
511-
}
512-
},
513507
}
514508
// One characters shorter name is valid
515509
let max_len_container_name: String = long_container_name.chars().skip(1).collect();
@@ -527,11 +521,11 @@ mod tests {
527521
assert!(ContainerBuilder::new("name-with-hyphen").is_ok());
528522
assert_container_builder_err(
529523
ContainerBuilder::new("ends-with-hyphen-"),
530-
"regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?",
524+
"regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\"",
531525
);
532526
assert_container_builder_err(
533527
ContainerBuilder::new("-starts-with-hyphen"),
534-
"regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?",
528+
"regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\"",
535529
);
536530
}
537531

@@ -545,7 +539,7 @@ mod tests {
545539
assert!(ContainerBuilder::new("name_name").is_err());
546540
assert_container_builder_err(
547541
ContainerBuilder::new("name_name"),
548-
"(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])?)*')",
542+
"(e.g. \"example-label\", or \"1-label-1\", regex used for validation is \"[a-z0-9]([-a-z0-9]*[a-z0-9])?\")",
549543
);
550544
}
551545

@@ -573,19 +567,16 @@ mod tests {
573567
result: Result<ContainerBuilder, Error>,
574568
expected_err_contains: &str,
575569
) {
576-
match result {
577-
Ok(_) => {
578-
panic!("Container name exceeding 63 characters should cause an error");
570+
match result
571+
.err()
572+
.expect("Container name exceeding 63 characters should cause an error")
573+
{
574+
Error::InvalidContainerName {
575+
container_name: _,
576+
source,
577+
} => {
578+
assert!(dbg!(source.to_string()).contains(dbg!(expected_err_contains)));
579579
}
580-
Err(error) => match error {
581-
Error::InvalidContainerName {
582-
container_name: _,
583-
violation,
584-
} => {
585-
println!("{violation}");
586-
assert!(violation.contains(expected_err_contains));
587-
}
588-
},
589580
}
590581
}
591582
}

0 commit comments

Comments
 (0)