From 40b1d870990ebe62b01bbc3a2e4bf59a7d481abc Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Sat, 5 Feb 2022 23:14:46 +0100 Subject: [PATCH 1/2] Migrate the Docker context tags version from `.tag` to `tags.`. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] --- .../docker/goals/package_image_test.py | 2 +- .../docker/util_rules/docker_build_context.py | 24 +++++++- .../util_rules/docker_build_context_test.py | 59 ++++++++++++++----- .../backend/docker/value_interpolation.py | 14 +++++ 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/src/python/pants/backend/docker/goals/package_image_test.py b/src/python/pants/backend/docker/goals/package_image_test.py index b187084fac6..58466f5b1df 100644 --- a/src/python/pants/backend/docker/goals/package_image_test.py +++ b/src/python/pants/backend/docker/goals/package_image_test.py @@ -249,7 +249,7 @@ def test_build_docker_image(rule_runner: RuleRunner) -> None: err1 = ( r"Invalid value for the `repository` field of the `docker_image` target at " r"docker/test:err1: '{bad_template}'\.\n\nThe placeholder 'bad_template' is unknown\. " - r"Try with one of: build_args, directory, name, pants, parent_directory\." + r"Try with one of: build_args, directory, name, pants, parent_directory, tags\." ) with pytest.raises(DockerRepositoryNameError, match=err1): assert_build( diff --git a/src/python/pants/backend/docker/util_rules/docker_build_context.py b/src/python/pants/backend/docker/util_rules/docker_build_context.py index 015d13dfce4..ed1b7d476d0 100644 --- a/src/python/pants/backend/docker/util_rules/docker_build_context.py +++ b/src/python/pants/backend/docker/util_rules/docker_build_context.py @@ -22,6 +22,7 @@ ) from pants.backend.docker.utils import get_hash, suggest_renames from pants.backend.docker.value_interpolation import ( + DeprecatedDockerInterpolationValue, DockerBuildArgsInterpolationValue, DockerInterpolationContext, DockerInterpolationValue, @@ -50,6 +51,14 @@ logger = logging.getLogger(__name__) +class DockerfileImageTagsDeprecation(DeprecatedDockerInterpolationValue): + _removal_version = "2.11.0.dev0" + _hint = ( + "The `.tag` version values are deprecated in favour of `tags.`.\n\n" + "See https://github.com/pantsbuild/pants/issues/14023 for more details." + ) + + class DockerBuildContextError(Exception): pass @@ -118,11 +127,19 @@ def create( # Go over all FROM tags and names for all stages. stage_names: set[str] = set() stage_tags = (tag.split(maxsplit=1) for tag in dockerfile_info.version_tags) + tags_values: dict[str, str] = {} for idx, (stage, tag) in enumerate(stage_tags): if stage != f"stage{idx}": stage_names.add(stage) - value = {"tag": tag} - if not interpolation_context: + if idx == 0: + # Expose the first (stage0) FROM directive as the "baseimage". + tags_values["baseimage"] = tag + tags_values[stage] = tag + + # The remaining part of this for loop is deprecated. + # TODO: remove in 2.11.0.dev0 + value = DockerfileImageTagsDeprecation({"tag": tag}) + if idx == 0: # Expose the first (stage0) FROM directive as the "baseimage". interpolation_context["baseimage"] = value interpolation_context[stage] = value @@ -172,6 +189,9 @@ def create( "hash": get_hash((build_args, build_env, snapshot.digest)).hexdigest(), } + # Base image tags values for all stages (as parsed from the Dockerfile instructions). + interpolation_context["tags"] = tags_values + return cls( build_args=build_args, digest=snapshot.digest, diff --git a/src/python/pants/backend/docker/util_rules/docker_build_context_test.py b/src/python/pants/backend/docker/util_rules/docker_build_context_test.py index 32770d563ae..2f1e7c508c5 100644 --- a/src/python/pants/backend/docker/util_rules/docker_build_context_test.py +++ b/src/python/pants/backend/docker/util_rules/docker_build_context_test.py @@ -24,6 +24,7 @@ from pants.backend.docker.util_rules.docker_build_context import ( DockerBuildContext, DockerBuildContextRequest, + DockerfileImageTagsDeprecation, ) from pants.backend.docker.util_rules.docker_build_env import DockerBuildEnvironment from pants.backend.docker.value_interpolation import ( @@ -140,8 +141,12 @@ def test_pants_hash(rule_runner: RuleRunner) -> None: Address("test"), expected_files=["test/Dockerfile"], expected_interpolation_context={ - "baseimage": {"tag": "latest"}, - "stage0": {"tag": "latest"}, + "tags": { + "baseimage": "latest", + "stage0": "latest", + }, + "baseimage": DockerfileImageTagsDeprecation({"tag": "latest"}), + "stage0": DockerfileImageTagsDeprecation({"tag": "latest"}), "build_args": {}, "pants": {"hash": "fd19488a9b08a0184432762cab85f1370904d09bafd9df1a2f8a94614b2b7eb6"}, }, @@ -238,8 +243,12 @@ def test_from_image_build_arg_dependency(rule_runner: RuleRunner) -> None: expected_files=["src/downstream/Dockerfile"], build_upstream_images=True, expected_interpolation_context={ - "baseimage": {"tag": "latest"}, - "stage0": {"tag": "latest"}, + "tags": { + "baseimage": "latest", + "stage0": "latest", + }, + "baseimage": DockerfileImageTagsDeprecation({"tag": "latest"}), + "stage0": DockerfileImageTagsDeprecation({"tag": "latest"}), "build_args": { "BASE_IMAGE": "upstream/image:latest", }, @@ -319,11 +328,18 @@ def test_interpolation_context_from_dockerfile(rule_runner: RuleRunner) -> None: Address("src/docker"), expected_files=["src/docker/Dockerfile"], expected_interpolation_context={ - "baseimage": {"tag": "3.8"}, - "stage0": {"tag": "3.8"}, - "interim": {"tag": "latest"}, - "stage2": {"tag": "latest"}, - "output": {"tag": "1-1"}, + "tags": { + "baseimage": "3.8", + "stage0": "3.8", + "interim": "latest", + "stage2": "latest", + "output": "1-1", + }, + "baseimage": DockerfileImageTagsDeprecation({"tag": "3.8"}), + "stage0": DockerfileImageTagsDeprecation({"tag": "3.8"}), + "interim": DockerfileImageTagsDeprecation({"tag": "latest"}), + "stage2": DockerfileImageTagsDeprecation({"tag": "latest"}), + "output": DockerfileImageTagsDeprecation({"tag": "1-1"}), "build_args": {}, }, ) @@ -352,11 +368,18 @@ def test_synthetic_dockerfile(rule_runner: RuleRunner) -> None: Address("src/docker"), expected_files=["src/docker/Dockerfile.docker"], expected_interpolation_context={ - "baseimage": {"tag": "3.8"}, - "stage0": {"tag": "3.8"}, - "interim": {"tag": "latest"}, - "stage2": {"tag": "latest"}, - "output": {"tag": "1-1"}, + "tags": { + "baseimage": "3.8", + "stage0": "3.8", + "interim": "latest", + "stage2": "latest", + "output": "1-1", + }, + "baseimage": DockerfileImageTagsDeprecation({"tag": "3.8"}), + "stage0": DockerfileImageTagsDeprecation({"tag": "3.8"}), + "interim": DockerfileImageTagsDeprecation({"tag": "latest"}), + "stage2": DockerfileImageTagsDeprecation({"tag": "latest"}), + "output": DockerfileImageTagsDeprecation({"tag": "1-1"}), "build_args": {}, }, ) @@ -428,8 +451,12 @@ def test_build_arg_defaults_from_dockerfile(rule_runner: RuleRunner) -> None: }, expected_files=["src/docker/Dockerfile"], expected_interpolation_context={ - "baseimage": {"tag": "${base_version}"}, - "stage0": {"tag": "${base_version}"}, + "tags": { + "baseimage": "${base_version}", + "stage0": "${base_version}", + }, + "baseimage": DockerfileImageTagsDeprecation({"tag": "${base_version}"}), + "stage0": DockerfileImageTagsDeprecation({"tag": "${base_version}"}), "build_args": { # `base_name` is not listed here, as it was not an explicitly defined build arg. "base_version": "3.9", diff --git a/src/python/pants/backend/docker/value_interpolation.py b/src/python/pants/backend/docker/value_interpolation.py index 28b0264621b..5f1d8b44ba2 100644 --- a/src/python/pants/backend/docker/value_interpolation.py +++ b/src/python/pants/backend/docker/value_interpolation.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from typing import ClassVar, Mapping, TypeVar, Union +from pants.base.deprecated import warn_or_error from pants.engine.addresses import Address from pants.util.frozendict import FrozenDict @@ -36,6 +37,19 @@ def __getattr__(self, attribute: str) -> str: return self[attribute] +class DeprecatedDockerInterpolationValue(DockerInterpolationValue): + _removal_version: ClassVar[str] + _hint: ClassVar[str | None] + + def __getattr__(self, attribute: str) -> str: + warn_or_error( + self._removal_version, + f"Docker interpolation context type {type(self).__name__!r}", + self._hint, + ) + return super().__getattr__(attribute) + + class DockerBuildArgInterpolationError(DockerInterpolationError): @classmethod def attribute_error( From 380e6328f5d34ea375518aaeedae754eb712d1b1 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Tue, 8 Feb 2022 11:33:29 +0100 Subject: [PATCH 2/2] propagate source of deprecated use. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../docker/util_rules/docker_build_context.py | 24 +++++++++++++++---- .../util_rules/docker_build_context_test.py | 5 ++-- .../backend/docker/value_interpolation.py | 2 +- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/python/pants/backend/docker/util_rules/docker_build_context.py b/src/python/pants/backend/docker/util_rules/docker_build_context.py index ed1b7d476d0..155cd421c40 100644 --- a/src/python/pants/backend/docker/util_rules/docker_build_context.py +++ b/src/python/pants/backend/docker/util_rules/docker_build_context.py @@ -54,10 +54,23 @@ class DockerfileImageTagsDeprecation(DeprecatedDockerInterpolationValue): _removal_version = "2.11.0.dev0" _hint = ( - "The `.tag` version values are deprecated in favour of `tags.`.\n\n" + "The `.tag` version values are deprecated in favour of `tags.`.\n" "See https://github.com/pantsbuild/pants/issues/14023 for more details." ) + @classmethod + def for_tag(cls, tag: str, address: Address) -> type[DockerfileImageTagsDeprecation]: + return type( + f"{cls.__name__}_{tag}", + (cls,), + { + "_hint": ( + f"Interpolation in {address} uses the deprecated placeholder '{tag}.{{key}}', " + f"instead use 'tags.{tag}'.\n\n" + cls._hint + ) + }, + ) + class DockerBuildContextError(Exception): pass @@ -138,11 +151,14 @@ def create( # The remaining part of this for loop is deprecated. # TODO: remove in 2.11.0.dev0 - value = DockerfileImageTagsDeprecation({"tag": tag}) if idx == 0: # Expose the first (stage0) FROM directive as the "baseimage". - interpolation_context["baseimage"] = value - interpolation_context[stage] = value + interpolation_context["baseimage"] = DockerfileImageTagsDeprecation.for_tag( + "baseimage", dockerfile_info.address + )({"tag": tag}) + interpolation_context[stage] = DockerfileImageTagsDeprecation.for_tag( + stage, dockerfile_info.address + )({"tag": tag}) if build_args: # Extract default arg values from the parsed Dockerfile. diff --git a/src/python/pants/backend/docker/util_rules/docker_build_context_test.py b/src/python/pants/backend/docker/util_rules/docker_build_context_test.py index 2f1e7c508c5..66e2291cc67 100644 --- a/src/python/pants/backend/docker/util_rules/docker_build_context_test.py +++ b/src/python/pants/backend/docker/util_rules/docker_build_context_test.py @@ -121,8 +121,9 @@ def assert_build_context( if "pants" not in expected_interpolation_context: expected_interpolation_context["pants"] = context.interpolation_context["pants"] - assert context.interpolation_context == DockerInterpolationContext.from_dict( - expected_interpolation_context + # Converting to `dict` to avoid the fact that FrozenDict is sensitive to the order of the keys. + assert dict(context.interpolation_context) == dict( + DockerInterpolationContext.from_dict(expected_interpolation_context) ) return context diff --git a/src/python/pants/backend/docker/value_interpolation.py b/src/python/pants/backend/docker/value_interpolation.py index 5f1d8b44ba2..221984df049 100644 --- a/src/python/pants/backend/docker/value_interpolation.py +++ b/src/python/pants/backend/docker/value_interpolation.py @@ -45,7 +45,7 @@ def __getattr__(self, attribute: str) -> str: warn_or_error( self._removal_version, f"Docker interpolation context type {type(self).__name__!r}", - self._hint, + self._hint.format(key=attribute) if self._hint else None, ) return super().__getattr__(attribute)