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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
93 changes: 62 additions & 31 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:?}"))]
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.
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_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,
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_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
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_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,
}
})
}
}
}
Expand All @@ -174,6 +188,21 @@ impl ProductImage {
}) => pv,
}
}

fn prepare_app_version_label_value(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(|_| ParseAppVersionLabelSnafu {
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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,
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_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()}]),
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_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,
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_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,
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/kvp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
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_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,
Expand Down