Skip to content

Fix: Invalid app version label when using hash in custom image #1076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 13, 2025
Merged
7 changes: 7 additions & 0 deletions crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Fixed

- BREAKING: Fixed bug where `app_version_label` could not be used for metadata `Label` when using a hash in custom images.
The `product_image_selection::resolve` now returns a `Result<ResolvedProductImage,Error>` instead of a `ResolvedProductImage` ([#1076]).

[#1076]: https://github.com/stackabletech/operator-rs/pull/1076

## [0.94.0] - 2025-07-10

### Added
Expand Down
89 changes: 60 additions & 29 deletions crates/stackable-operator/src/commons/product_image_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`"))]
AppVersionLabelParseFailed {
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.
Expand Down Expand Up @@ -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: 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,
Expand Down Expand Up @@ -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<ResolvedProductImage, Error> {
let image_pull_policy = self.pull_policy.as_ref().to_string();
let pull_secrets = self.pull_secrets.clone();

Expand All @@ -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 = Self::prepare_app_version_label(&app_version)?;

Ok(ResolvedProductImage {
product_version,
app_version_label,
image: image_selection.custom.clone(),
image_pull_policy,
pull_secrets,
}
})
}
ProductImageSelection::StackableVersion(image_selection) => {
let repo = image_selection
Expand All @@ -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 = Self::prepare_app_version_label(&app_version)?;
Ok(ResolvedProductImage {
product_version,
app_version_label,
image,
image_pull_policy,
pull_secrets,
}
})
}
}
}
Expand All @@ -174,6 +188,21 @@ impl ProductImage {
}) => pv,
}
}

fn prepare_app_version_label(app_version: &str) -> Result<LabelValue, Error> {
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(|_| AppVersionLabelParseFailedSnafu {
app_version: formatted_app_version.to_string(),
})
}
}

#[cfg(test)]
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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,
Expand All @@ -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: "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()}]),
Expand All @@ -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);
}
Expand Down
8 changes: 6 additions & 2 deletions crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: "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,
Expand Down Expand Up @@ -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: "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,
Expand Down
5 changes: 3 additions & 2 deletions crates/stackable-operator/src/kvp/label/value.rs
Original file line number Diff line number Diff line change
@@ -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<Regex> = LazyLock::new(|| {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/stackable-operator/src/product_logging/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: "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,
Expand Down
Loading