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"