diff --git a/Cargo.lock b/Cargo.lock index 8e909feef..71ad9c779 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2861,9 +2861,9 @@ checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" [[package]] name = "slab" -version = "0.4.10" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04dc19736151f35336d325007ac991178d504a119863a2fcb3758cdb5e52c50d" +checksum = "7a2ae44ef20feb57a68b23d846850f861394c2e02dc425a50098ae8c90267589" [[package]] name = "smallvec" diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index a8d66b5ee..86945a87f 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Changed + +- BREAKING: The `ResolvedProductImage` field `app_version_label` was renamed to `app_version_label_value` to match changes to its type ([#1076]). + +### Fixed + +- BREAKING: Fix bug where `ResolvedProductImage::app_version_label` could not be used as a label value because it can contain invalid characters. + This is the case when referencing custom images via a `@sha256:...` hash. As such, the `product_image_selection::resolve` function is now fallible ([#1076]). + +[#1076]: https://github.com/stackabletech/operator-rs/pull/1076 + ## [0.94.0] - 2025-07-10 ### Added diff --git a/crates/stackable-operator/src/commons/product_image_selection.rs b/crates/stackable-operator/src/commons/product_image_selection.rs index 6fe5ba674..61704b10d 100644 --- a/crates/stackable-operator/src/commons/product_image_selection.rs +++ b/crates/stackable-operator/src/commons/product_image_selection.rs @@ -2,13 +2,22 @@ use dockerfile_parser::ImageRef; use k8s_openapi::api::core::v1::LocalObjectReference; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use snafu::{ResultExt, Snafu}; use strum::AsRefStr; -#[cfg(doc)] -use crate::kvp::Labels; +use crate::kvp::{LABEL_VALUE_MAX_LEN, LabelValue, LabelValueError}; pub const STACKABLE_DOCKER_REPO: &str = "oci.stackable.tech/sdp"; +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("could not parse or create label from app version {app_version:?}"))] + ParseAppVersionLabel { + source: LabelValueError, + app_version: String, + }, +} + /// Specify which image to use, the easiest way is to only configure the `productVersion`. /// You can also configure a custom image registry to pull from, as well as completely custom /// images. @@ -67,8 +76,8 @@ pub struct ResolvedProductImage { /// Version of the product, e.g. `1.4.1`. pub product_version: String, - /// App version as formatted for [`Labels::recommended`] - pub app_version_label: String, + /// App version formatted for Labels + pub app_version_label_value: LabelValue, /// Image to be used for the product image e.g. `oci.stackable.tech/sdp/superset:1.4.1-stackable2.1.0` pub image: String, @@ -101,7 +110,11 @@ impl ProductImage { /// `image_base_name` should be base of the image name in the container image registry, e.g. `trino`. /// `operator_version` needs to be the full operator version and a valid semver string. /// Accepted values are `23.7.0`, `0.0.0-dev` or `0.0.0-pr123`. Other variants are not supported. - pub fn resolve(&self, image_base_name: &str, operator_version: &str) -> ResolvedProductImage { + pub fn resolve( + &self, + image_base_name: &str, + operator_version: &str, + ) -> Result { let image_pull_policy = self.pull_policy.as_ref().to_string(); let pull_secrets = self.pull_secrets.clone(); @@ -111,17 +124,17 @@ impl ProductImage { ProductImageSelection::Custom(image_selection) => { let image = ImageRef::parse(&image_selection.custom); let image_tag_or_hash = image.tag.or(image.hash).unwrap_or("latest".to_string()); - let mut app_version_label = format!("{}-{}", product_version, image_tag_or_hash); - // TODO Use new label mechanism once added - app_version_label.truncate(63); - ResolvedProductImage { + let app_version = format!("{}-{}", product_version, image_tag_or_hash); + let app_version_label_value = Self::prepare_app_version_label_value(&app_version)?; + + Ok(ResolvedProductImage { product_version, - app_version_label, + app_version_label_value, image: image_selection.custom.clone(), image_pull_policy, pull_secrets, - } + }) } ProductImageSelection::StackableVersion(image_selection) => { let repo = image_selection @@ -147,14 +160,15 @@ impl ProductImage { let image = format!( "{repo}/{image_base_name}:{product_version}-stackable{stackable_version}", ); - let app_version_label = format!("{product_version}-stackable{stackable_version}",); - ResolvedProductImage { + let app_version = format!("{product_version}-stackable{stackable_version}"); + let app_version_label_value = Self::prepare_app_version_label_value(&app_version)?; + Ok(ResolvedProductImage { product_version, - app_version_label, + app_version_label_value, image, image_pull_policy, pull_secrets, - } + }) } } } @@ -174,6 +188,21 @@ impl ProductImage { }) => pv, } } + + fn prepare_app_version_label_value(app_version: &str) -> Result { + let mut formatted_app_version = app_version.to_string(); + // Labels cannot have more than `LABEL_VALUE_MAX_LEN` characters. + formatted_app_version.truncate(LABEL_VALUE_MAX_LEN); + // The hash has the format `sha256:85fa483aa99b9997ce476b86893ad5ed81fb7fd2db602977eb` + // As the colon (`:`) is not a valid label value character, we replace it with a valid "-" character. + let formatted_app_version = formatted_app_version.replace(":", "-"); + + formatted_app_version + .parse() + .with_context(|_| ParseAppVersionLabelSnafu { + app_version: formatted_app_version.to_string(), + }) + } } #[cfg(test)] @@ -191,7 +220,7 @@ mod tests { "#, ResolvedProductImage { image: "oci.stackable.tech/sdp/superset:1.4.1-stackable23.7.42".to_string(), - app_version_label: "1.4.1-stackable23.7.42".to_string(), + app_version_label_value: "1.4.1-stackable23.7.42".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -205,7 +234,7 @@ mod tests { "#, ResolvedProductImage { image: "oci.stackable.tech/sdp/superset:1.4.1-stackable0.0.0-dev".to_string(), - app_version_label: "1.4.1-stackable0.0.0-dev".to_string(), + app_version_label_value: "1.4.1-stackable0.0.0-dev".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -219,7 +248,7 @@ mod tests { "#, ResolvedProductImage { image: "oci.stackable.tech/sdp/superset:1.4.1-stackable0.0.0-dev".to_string(), - app_version_label: "1.4.1-stackable0.0.0-dev".to_string(), + app_version_label_value: "1.4.1-stackable0.0.0-dev".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -234,7 +263,7 @@ mod tests { "#, ResolvedProductImage { image: "oci.stackable.tech/sdp/superset:1.4.1-stackable2.1.0".to_string(), - app_version_label: "1.4.1-stackable2.1.0".to_string(), + app_version_label_value: "1.4.1-stackable2.1.0".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -250,7 +279,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/trino:1.4.1-stackable2.1.0".to_string(), - app_version_label: "1.4.1-stackable2.1.0".to_string(), + app_version_label_value: "1.4.1-stackable2.1.0".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -265,7 +294,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset".to_string(), - app_version_label: "1.4.1-latest".to_string(), + app_version_label_value: "1.4.1-latest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -280,7 +309,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -295,7 +324,7 @@ mod tests { "#, ResolvedProductImage { image: "127.0.0.1:8080/myteam/stackable/superset".to_string(), - app_version_label: "1.4.1-latest".to_string(), + app_version_label_value: "1.4.1-latest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -310,7 +339,7 @@ mod tests { "#, ResolvedProductImage { image: "127.0.0.1:8080/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -325,7 +354,7 @@ mod tests { "#, ResolvedProductImage { image: "oci.stackable.tech/sdp/superset@sha256:85fa483aa99b9997ce476b86893ad5ed81fb7fd2db602977eb8c42f76efc1098".to_string(), - app_version_label: "1.4.1-sha256:85fa483aa99b9997ce476b86893ad5ed81fb7fd2db602977eb".to_string(), + app_version_label_value: "1.4.1-sha256-85fa483aa99b9997ce476b86893ad5ed81fb7fd2db602977eb".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -341,7 +370,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -357,7 +386,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "IfNotPresent".to_string(), pull_secrets: None, @@ -373,7 +402,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -389,7 +418,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Never".to_string(), pull_secrets: None, @@ -408,7 +437,7 @@ mod tests { "#, ResolvedProductImage { image: "my.corp/myteam/stackable/superset:latest-and-greatest".to_string(), - app_version_label: "1.4.1-latest-and-greatest".to_string(), + app_version_label_value: "1.4.1-latest-and-greatest".parse().expect("static app version label is always valid"), product_version: "1.4.1".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: Some(vec![LocalObjectReference{name: "myPullSecrets1".to_string()}, LocalObjectReference{name: "myPullSecrets2".to_string()}]), @@ -421,7 +450,9 @@ mod tests { #[case] expected: ResolvedProductImage, ) { let product_image: ProductImage = serde_yaml::from_str(&input).expect("Illegal test input"); - let resolved_product_image = product_image.resolve(&image_base_name, &operator_version); + let resolved_product_image = product_image + .resolve(&image_base_name, &operator_version) + .expect("Illegal test input"); assert_eq!(resolved_product_image, expected); } diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 8193123ec..934204dc7 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -374,7 +374,9 @@ mod tests { let resolved_product_image = ResolvedProductImage { image: "oci.stackable.tech/sdp/product:latest".to_string(), - app_version_label: "1.0.0-latest".to_string(), + app_version_label_value: "1.0.0-latest" + .parse() + .expect("static app version label is always valid"), product_version: "1.0.0".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, @@ -439,7 +441,9 @@ mod tests { let resolved_product_image = ResolvedProductImage { image: "oci.stackable.tech/sdp/product:latest".to_string(), - app_version_label: "1.0.0-latest".to_string(), + app_version_label_value: "1.0.0-latest" + .parse() + .expect("static app version label is always valid"), product_version: "1.0.0".to_string(), image_pull_policy: "Always".to_string(), pull_secrets: None, diff --git a/crates/stackable-operator/src/kvp/label/value.rs b/crates/stackable-operator/src/kvp/label/value.rs index 7135ceef7..510887e65 100644 --- a/crates/stackable-operator/src/kvp/label/value.rs +++ b/crates/stackable-operator/src/kvp/label/value.rs @@ -1,11 +1,12 @@ use std::{fmt::Display, ops::Deref, str::FromStr, sync::LazyLock}; use regex::Regex; +use schemars::JsonSchema; use snafu::{Snafu, ensure}; use crate::kvp::Value; -const LABEL_VALUE_MAX_LEN: usize = 63; +pub const LABEL_VALUE_MAX_LEN: usize = 63; // Lazily initialized regular expressions static LABEL_VALUE_REGEX: LazyLock = LazyLock::new(|| { @@ -43,7 +44,7 @@ pub enum LabelValueError { /// unvalidated mutable access to inner values. /// /// [k8s-labels]: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ -#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, JsonSchema)] pub struct LabelValue(String); impl Value for LabelValue { diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index f5aab1b83..8aaee66be 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -380,7 +380,7 @@ pub struct ObjectLabels<'a, T> { /// /// However, this is pure documentation and should not be parsed. /// - /// [avl]: crate::commons::product_image_selection::ResolvedProductImage::app_version_label + /// [avl]: crate::commons::product_image_selection::ResolvedProductImage::app_version_label_value pub app_version: &'a str, /// The DNS-style name of the operator managing the object (such as `zookeeper.stackable.tech`) diff --git a/crates/stackable-operator/src/product_logging/framework.rs b/crates/stackable-operator/src/product_logging/framework.rs index d62a6445c..0279a23a8 100644 --- a/crates/stackable-operator/src/product_logging/framework.rs +++ b/crates/stackable-operator/src/product_logging/framework.rs @@ -1382,7 +1382,7 @@ sinks: /// /// # let resolved_product_image = ResolvedProductImage { /// # product_version: "1.0.0".into(), -/// # app_version_label: "1.0.0".into(), +/// # app_version_label_value: "1.0.0".parse().expect("static app version label is always valid"), /// # image: "oci.stackable.tech/sdp/my-product:1.0.0-stackable1.0.0".into(), /// # image_pull_policy: "Always".into(), /// # pull_secrets: None,