From b90dd5b1ea0b9d09a5ff06800c0087d4314a2282 Mon Sep 17 00:00:00 2001 From: "Benjamin R. J. Schwedler" Date: Thu, 28 May 2026 14:10:45 -0500 Subject: [PATCH] Encode release stream in image target UID Build metadata is keyed by an image target's UID and matched back onto targets in `bakery ci merge`. The UID combined only the image name, version, variant, and OS, so a development-stream build (daily or preview) and a release build of the same version produced an identical UID. A dev-only build's artifacts then matched the release target and were pushed to the release registries (Docker Hub + GHCR) without the -preview suffix. Append the release stream to the UID for development streams so dev and release builds of the same version no longer collide. Release UIDs are left unsuffixed to keep existing temp-image and cache names stable. Add a guard that fails target generation on any duplicate UID rather than silently merging. Refs: https://github.com/posit-dev/images-shared/issues/553 --- posit-bakery/posit_bakery/config/config.py | 14 +++++++ .../posit_bakery/image/image_target.py | 29 +++++++++++--- posit-bakery/test/config/test_config.py | 38 +++++++++++++++++++ posit-bakery/test/image/test_image_target.py | 37 ++++++++++++++++++ 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/posit-bakery/posit_bakery/config/config.py b/posit-bakery/posit_bakery/config/config.py index a0a85e04..be58e5af 100644 --- a/posit-bakery/posit_bakery/config/config.py +++ b/posit-bakery/posit_bakery/config/config.py @@ -899,6 +899,20 @@ def generate_image_targets(self, settings: BakerySettings = BakerySettings()): ) targets = sorted(targets, key=lambda t: str(t)) + + # Build metadata is matched to targets by UID, so a duplicate would let one + # build's artifacts be pushed as another's. Fail fast. + seen: dict[str, ImageTarget] = {} + for target in targets: + if target.uid in seen: + raise BakeryError( + f"Duplicate image target UID '{target.uid}': two targets resolve to the same " + f"image, version, variant, OS, and release stream ({target.release_stream.value}). " + "Check for a duplicate version definition or multiple development streams " + "resolving to the same version." + ) + seen[target.uid] = target + self.targets = targets def get_image_target_by_uid(self, uid: str) -> ImageTarget | None: diff --git a/posit-bakery/posit_bakery/image/image_target.py b/posit-bakery/posit_bakery/image/image_target.py index 04264f07..289385f4 100644 --- a/posit-bakery/posit_bakery/image/image_target.py +++ b/posit-bakery/posit_bakery/image/image_target.py @@ -14,11 +14,12 @@ from posit_bakery.config.image.build_os import DEFAULT_PLATFORMS from posit_bakery.config.image.build_secret import BuildSecret from posit_bakery.config.image.parsed_version import version_sort_key +from posit_bakery.config.image.posit_product.const import ReleaseStreamEnum from posit_bakery.config.registry import Registry, BaseRegistry from posit_bakery.config.repository import Repository from posit_bakery.config.tag import TagPattern, TagPatternFilter from posit_bakery.const import OCI_LABEL_PREFIX, POSIT_LABEL_PREFIX, REGEX_IMAGE_TAG_SUFFIX_ALLOWED_CHARACTERS_PATTERN -from posit_bakery.error import BakeryToolRuntimeError, BakeryFileError +from posit_bakery.error import BakeryError, BakeryToolRuntimeError, BakeryFileError from posit_bakery.image.image_metadata import MetadataFile, BuildMetadata from posit_bakery.settings import SETTINGS @@ -285,14 +286,34 @@ def __str__(self): @computed_field @property def uid(self) -> str: - """Generate a unique identifier for the target based on its properties.""" + """Generate a unique identifier for the target. + + The stream is appended for development versions so a dev build and a release + build of the same version never share a UID. Release UIDs stay unsuffixed. + """ u = f"{self.image_name}-{self.image_version.name}" if self.image_variant: u += f"-{self.image_variant.name}" if self.image_os: u += f"-{self.image_os.name}" + if self.release_stream != ReleaseStreamEnum.RELEASE: + u += f"-{self.release_stream.value}" return re.sub("[ .+/]", "-", u).lower() + @property + def release_stream(self) -> ReleaseStreamEnum: + """The release stream for this target, defaulting to ``release`` when unset.""" + stream = self.image_version.metadata.get("release_stream") + if stream is None: + return ReleaseStreamEnum.RELEASE + try: + return ReleaseStreamEnum(stream) + except ValueError as e: + raise BakeryError( + f"Invalid release_stream {stream!r} for {self}. " + f"Expected one of: {', '.join(s.value for s in ReleaseStreamEnum)}." + ) from e + @property def image_name(self) -> str: """Return the name of the image.""" @@ -371,9 +392,7 @@ def tag_template_values(self) -> dict[str, str]: "Variant": self.image_variant.tagDisplayName if self.image_variant else "", "OS": self.image_os.tagDisplayName if self.image_os else "", "Name": self.image_name, - "Stream": str(v.value if hasattr(v, "value") else v) - if (v := self.image_version.metadata.get("release_stream")) - else "", + "Stream": self.release_stream.value if self.image_version.metadata.get("release_stream") else "", } @property diff --git a/posit-bakery/test/config/test_config.py b/posit-bakery/test/config/test_config.py index ce4f76ea..19268970 100644 --- a/posit-bakery/test/config/test_config.py +++ b/posit-bakery/test/config/test_config.py @@ -14,6 +14,7 @@ from posit_bakery.config.dependencies import PythonDependencyConstraint, RDependencyVersions from posit_bakery.config.image.posit_product.const import ReleaseStreamEnum from posit_bakery.const import DevVersionInclusionEnum, MatrixVersionInclusionEnum +from posit_bakery.error import BakeryError from posit_bakery.image.image_metadata import BuildMetadata from test.config.conftest import CONFIG_TESTDATA_DIR from test.helpers import ( @@ -1760,6 +1761,43 @@ def test_load_build_metadata_file(self, get_config_obj): assert target.build_metadata[0].image_name is not None assert target.build_metadata[0].container_image_digest is not None + def test_generate_image_targets_rejects_duplicate_uid(self, get_config_obj): + """Two targets sharing a UID fail fast instead of silently colliding. + + Build metadata is keyed by UID, so a duplicate UID would let one build's + metadata match another target and push to the wrong registries. This is a + defense-in-depth guard for the posit-dev/images-shared#553 collision class. + """ + config = get_config_obj("basic") + image = config.model.get_image("test-image") + version = image.get_version("1.0.0") + clone = version.model_copy(deep=True) + clone.parent = version.parent + image.versions.append(clone) + + with pytest.raises(BakeryError, match="Duplicate image target UID"): + config.generate_image_targets() + + def test_dev_and_release_same_version_do_not_collide(self, get_config_obj): + """A dev-stream version and a release version of the same number coexist with + distinct UIDs and generate without error.""" + config = get_config_obj("basic") + image = config.model.get_image("test-image") + release = image.get_version("1.0.0") + + dev = release.model_copy(deep=True) + dev.parent = release.parent + dev.isDevelopmentVersion = True + dev.metadata = {"release_stream": ReleaseStreamEnum.DAILY} + image.versions.append(dev) + + config.generate_image_targets(BakerySettings(dev_versions=DevVersionInclusionEnum.INCLUDE)) + + uids = [t.uid for t in config.targets if t.image_version.name == "1.0.0"] + assert len(uids) == len(set(uids)) + assert any(u.endswith("-daily") for u in uids) + assert any(not u.endswith("-daily") for u in uids) + @pytest.mark.parametrize( "untagged,older_than_days,expected_deletions", [ diff --git a/posit-bakery/test/image/test_image_target.py b/posit-bakery/test/image/test_image_target.py index c7ca026b..1fc0e0e0 100644 --- a/posit-bakery/test/image/test_image_target.py +++ b/posit-bakery/test/image/test_image_target.py @@ -7,6 +7,7 @@ from posit_bakery.config.dependencies import PythonDependencyVersions, RDependencyVersions from posit_bakery.config.image.parsed_version import ParsedVersion +from posit_bakery.config.image.posit_product.const import ReleaseStreamEnum from posit_bakery.config.tag import default_tag_patterns, TagPatternFilter from posit_bakery.const import OCI_LABEL_PREFIX, POSIT_LABEL_PREFIX from posit_bakery.image.image_metadata import BuildMetadata @@ -139,6 +140,42 @@ def test_uid(self, get_config_obj, basic_standard_image_target): expected_uid = re.sub(r"[ .+/]", "-", f"{image.name}-{version.name}-{variant.name}-{os.name}").lower() assert basic_standard_image_target.uid == expected_uid + def test_uid_encodes_release_stream(self, get_config_obj): + """A dev-stream build and a release build of the same version get distinct UIDs. + + Regression test for posit-dev/images-shared#553: the UID omitted the release + stream, so a daily/preview build of version X shared a UID with the release + build of X and `bakery ci merge` pushed the dev image to the release registries. + """ + config = get_config_obj("basic") + image = config.model.get_image("test-image") + version = image.get_version("1.0.0") + variant = image.get_variant("Standard") + os = version.os[0] + + release_target = ImageTarget.new_image_target( + repository=config.model.repository, + image_version=version, + image_variant=variant, + image_os=os, + ) + + dev_version = version.model_copy(deep=True) + dev_version.parent = version.parent + dev_version.metadata = {"release_stream": ReleaseStreamEnum.DAILY} + dev_target = ImageTarget.new_image_target( + repository=config.model.repository, + image_version=dev_version, + image_variant=variant, + image_os=os, + ) + + assert release_target.uid != dev_target.uid + # Release UIDs stay unsuffixed; only dev streams carry a stream suffix. + assert release_target.uid == "test-image-1-0-0-standard-ubuntu-22-04" + assert not release_target.uid.endswith("-release") + assert dev_target.uid.endswith("-daily") + def test_image_name(self, basic_standard_image_target): assert basic_standard_image_target.image_name == "test-image"