diff --git a/docs/notes/2.32.x.md b/docs/notes/2.32.x.md index 7ce50fb838d..62c6173d57e 100644 --- a/docs/notes/2.32.x.md +++ b/docs/notes/2.32.x.md @@ -54,6 +54,9 @@ For `generate-lockfiles`, typos in the name of a resolve now give "Did you mean? ### Backends +The `--pull` flag for `docker_image` targets now supports also podman. When podman is activated the flag can be set to `missing`, `always`,`never` and `newer` as well as False (equal to `missing`) or True (equal to `always`). +The default behavior is now `missing`, which pulls the base image only if it is not already present locally. + #### Docker The option `[docker].push_on_package` can be used to prevent Docker images from being pushed during packaging, i.e. when `--output` contains `push=True` or `type=registry`. diff --git a/src/python/pants/backend/docker/goals/package_image.py b/src/python/pants/backend/docker/goals/package_image.py index 801351d4972..bbb89ca84ac 100644 --- a/src/python/pants/backend/docker/goals/package_image.py +++ b/src/python/pants/backend/docker/goals/package_image.py @@ -28,6 +28,7 @@ DockerBuildOptionFieldValueMixin, DockerBuildOptionFlagFieldMixin, DockerImageBuildImageOutputField, + DockerImageBuildPullOptionField, DockerImageContextRootField, DockerImageRegistriesField, DockerImageRepositoryField, @@ -331,6 +332,7 @@ def get_build_options( global_build_no_cache_option: bool | None, use_buildx_option: bool, target: Target, + docker: DockerBinary | None = None, ) -> Iterator[str]: # Build options from target fields inheriting from DockerBuildOptionFieldMixin for field_type in target.field_types: @@ -354,6 +356,7 @@ def get_build_options( DockerBuildOptionFieldValueMixin, DockerBuildOptionFieldMultiValueMixin, DockerBuildOptionFlagFieldMixin, + DockerImageBuildPullOptionField, ), ): source = InterpolationContext.TextSource( @@ -365,7 +368,7 @@ def get_build_options( error_cls=DockerImageOptionValueError, ) yield from target[field_type].options( - format, global_build_hosts_options=global_build_hosts_options + format, global_build_hosts_options=global_build_hosts_options, docker=docker ) # Target stage @@ -510,6 +513,7 @@ async def get_docker_image_build_process( global_build_no_cache_option=options.build_no_cache, use_buildx_option=options.use_buildx, target=wrapped_target.target, + docker=docker, ) ), ) diff --git a/src/python/pants/backend/docker/goals/package_image_podman_pull_test.py b/src/python/pants/backend/docker/goals/package_image_podman_pull_test.py new file mode 100644 index 00000000000..434a0ce2e2c --- /dev/null +++ b/src/python/pants/backend/docker/goals/package_image_podman_pull_test.py @@ -0,0 +1,215 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +"""Integration tests for Podman-specific pull policy behavior in DockerImageBuildPullOptionField.""" + +from __future__ import annotations + +import pytest + +from pants.backend.docker.goals.package_image import ( + DockerImageBuildProcess, + DockerImageRefs, + DockerPackageFieldSet, + ImageRefRegistry, + ImageRefTag, + get_docker_image_build_process, + rules, +) +from pants.backend.docker.subsystems.docker_options import DockerOptions +from pants.backend.docker.target_types import DockerImageTarget +from pants.backend.docker.util_rules.docker_binary import DockerBinary +from pants.backend.docker.util_rules.docker_build_args import DockerBuildArgs +from pants.backend.docker.util_rules.docker_build_args import rules as build_args_rules +from pants.backend.docker.util_rules.docker_build_context import DockerBuildContext +from pants.backend.docker.util_rules.docker_build_env import DockerBuildEnvironment +from pants.backend.docker.util_rules.docker_build_env import rules as build_env_rules +from pants.engine.addresses import Address +from pants.engine.env_vars import EnvironmentVars +from pants.engine.fs import EMPTY_DIGEST +from pants.engine.target import InvalidFieldException, WrappedTarget +from pants.engine.unions import UnionMembership +from pants.testutil.option_util import create_subsystem +from pants.testutil.rule_runner import QueryRule, RuleRunner, run_rule_with_mocks +from pants.util.value_interpolation import InterpolationContext, InterpolationValue + + +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *rules(), + *build_args_rules(), + *build_env_rules(), + QueryRule(DockerOptions, []), + ], + target_types=[DockerImageTarget], + ) + + +def _make_image_refs(address: Address) -> DockerImageRefs: + repository = address.target_name + return DockerImageRefs( + [ + ImageRefRegistry( + registry=None, + repository=repository, + tags=( + ImageRefTag( + template="latest", + formatted="latest", + full_name=f"{repository}:latest", + uses_local_alias=False, + ), + ), + ) + ] + ) + + +def create_test_context(rule_runner: RuleRunner, pull_value=None): + """Helper to create a mock build context and target with specific pull value.""" + # Create BUILD file with optional pull value + # Python booleans need to be capitalized (True/False) in BUILD files + build_content = "docker_image(name='test'" + if pull_value is not None: + if isinstance(pull_value, str): + build_content += f", pull='{pull_value}'" + else: + # Convert bool to string with proper capitalization + build_content += f", pull={str(pull_value)}" + build_content += ")" + + rule_runner.write_files( + { + "test/BUILD": build_content, + "test/Dockerfile": "FROM alpine:3.16\n", + } + ) + + tgt = rule_runner.get_target(Address("test")) + + # Mock build context + build_context = DockerBuildContext( + build_args=DockerBuildArgs(), + digest=EMPTY_DIGEST, + dockerfile="test/Dockerfile", + build_env=DockerBuildEnvironment(environment=EnvironmentVars()), + interpolation_context=InterpolationContext.from_dict( + { + "tags": InterpolationValue({}), + } + ), + copy_source_vs_context_source=(("test/Dockerfile", ""),), + stages=(), + upstream_image_ids=(), + ) + + return tgt, build_context + + +@pytest.mark.parametrize( + "policy", + ["always", "missing", "never", "newer"], +) +def test_podman_pull_string_policies(rule_runner: RuleRunner, policy: str) -> None: + """Test that Podman accepts all valid string pull policies.""" + tgt, build_context = create_test_context(rule_runner, pull_value=policy) + + docker_options = create_subsystem( + DockerOptions, + registries={}, + default_repository="{name}", + default_context_root="", + build_args=[], + build_target_stage=None, + build_hosts=None, + build_verbose=False, + build_no_cache=False, + use_buildx=False, + env_vars=[], + ) + + # Use Podman binary + podman_binary = DockerBinary( + path="/bin/podman", + fingerprint="test", + extra_env={}, + extra_input_digests=None, + is_podman=True, + ) + + address = Address("test") + image_refs = _make_image_refs(address) + + result: DockerImageBuildProcess = run_rule_with_mocks( + get_docker_image_build_process, + rule_args=[ + DockerPackageFieldSet.create(tgt), + docker_options, + podman_binary, + ], + mock_calls={ + "pants.backend.docker.util_rules.docker_build_context.create_docker_build_context": lambda _req: build_context, + "pants.engine.internals.graph.resolve_target": lambda _: WrappedTarget(tgt), + "pants.backend.docker.goals.package_image.get_image_refs": lambda _: image_refs, + }, + union_membership=UnionMembership.from_rules([]), + show_warnings=False, + ) + + # Verify that the correct policy was used + argv = result.process.argv + expected_flag = f"--pull={policy}" + assert expected_flag in argv, f"Expected '{expected_flag}' in {argv}" + + +def test_docker_pull_string_raises_error(rule_runner: RuleRunner) -> None: + """Test that Docker backend raises error when given a string pull policy.""" + tgt, build_context = create_test_context(rule_runner, pull_value="always") + + docker_options = create_subsystem( + DockerOptions, + registries={}, + default_repository="{name}", + default_context_root="", + build_args=[], + build_target_stage=None, + build_hosts=None, + build_verbose=False, + build_no_cache=False, + use_buildx=False, + env_vars=[], + ) + + # Use Docker binary (not Podman) + docker_binary = DockerBinary( + path="/bin/docker", + fingerprint="test", + extra_env={}, + extra_input_digests=None, + is_podman=False, + ) + + address = Address("test") + image_refs = _make_image_refs(address) + + # Should raise InvalidFieldException + with pytest.raises(InvalidFieldException) as exc_info: + run_rule_with_mocks( + get_docker_image_build_process, + rule_args=[ + DockerPackageFieldSet.create(tgt), + docker_options, + docker_binary, + ], + mock_calls={ + "pants.backend.docker.util_rules.docker_build_context.create_docker_build_context": lambda _req: build_context, + "pants.engine.internals.graph.resolve_target": lambda _: WrappedTarget(tgt), + "pants.backend.docker.goals.package_image.get_image_refs": lambda _: image_refs, + }, + union_membership=UnionMembership.from_rules([]), + show_warnings=False, + ) + + assert "string pull policies are only supported by Podman" in str(exc_info.value) diff --git a/src/python/pants/backend/docker/target_types.py b/src/python/pants/backend/docker/target_types.py index 47a24701821..f44b348987b 100644 --- a/src/python/pants/backend/docker/target_types.py +++ b/src/python/pants/backend/docker/target_types.py @@ -5,6 +5,7 @@ import os import re +import warnings from abc import ABC, abstractmethod from collections.abc import Callable, Iterator from dataclasses import dataclass @@ -32,6 +33,7 @@ ListOfDictStringToStringField, OptionalSingleSourceField, StringField, + StringOrBoolField, StringSequenceField, Target, Targets, @@ -252,7 +254,7 @@ def option_values( @final def options( - self, value_formatter: OptionValueFormatter, global_build_hosts_options + self, value_formatter: OptionValueFormatter, global_build_hosts_options, **kwargs ) -> Iterator[str]: for value in self.option_values( value_formatter=value_formatter, global_build_hosts_options=global_build_hosts_options @@ -523,12 +525,17 @@ def options(self, *args, **kwargs) -> Iterator[str]: yield f"{self.docker_build_option}={','.join(list(self.value))}" -class DockerImageBuildPullOptionField(DockerBuildOptionFieldValueMixin, BoolField): +class DockerImageBuildPullOptionField(StringOrBoolField): alias = "pull" - default = False + default = None + valid_choices = ("always", "missing", "never", "newer") help = help_text( """ - If true, then docker will always attempt to pull a newer version of the image. + Pull policy for the image. + + For Docker: accepts boolean (true to always pull, false to use cached). + For Podman: accepts boolean or string policy ("always", "missing", "never", "newer"). + Default: false for Docker, "missing" for Podman. NOTE: This option cannot be used on images that build off of "transitive" base images referenced by address (i.e. `FROM path/to/your/base/Dockerfile`). @@ -536,6 +543,46 @@ class DockerImageBuildPullOptionField(DockerBuildOptionFieldValueMixin, BoolFiel ) docker_build_option = "--pull" + def options(self, value_formatter, global_build_hosts_options=None, **kwargs): + # Determine backend type from DockerBinary (which is resolved based on + # the [docker].experimental_enable_podman option). When experimental_enable_podman=true, + # the docker_binary will be 'podman' and is_podman will be True. + docker_binary = kwargs.get("docker") or kwargs.get("docker_binary") + is_podman = ( + getattr(docker_binary, "is_podman", False) if docker_binary is not None else False + ) + + val = self.value + if val is None: + # Use defaults based on backend + val = "missing" if is_podman else False + + if isinstance(val, str): + # String policies are only supported by Podman + if not is_podman: + raise InvalidFieldException( + f"The {self.alias!r} field was set to string value {val!r}, " + f"but string pull policies are only supported by Podman, not Docker. " + f"Use a boolean value (true/false) for Docker." + ) + yield f"{self.docker_build_option}={value_formatter(val)}" + else: + # Boolean value + if is_podman: + # Convert boolean to Podman policy string + warnings.warn( + f"Using boolean values for the 'pull' field with Podman is deprecated. " + f"Please use string values instead: 'always', 'missing', 'never', or 'newer'. " + f"Boolean {val} is being converted to 'always' if val else 'missing' policy.", + DeprecationWarning, + stacklevel=2, + ) + policy = "always" if val else "missing" + yield f"{self.docker_build_option}={policy}" + else: + # Docker: emit explicit boolean value with capital first letter + yield f"{self.docker_build_option}={str(val).capitalize()}" + class DockerBuildOptionFlagFieldMixin(BoolField, ABC): """Inherit this mixin class to provide optional flags (i.e. add `--flag` only when the value is diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 452dbf701ee..9bbc11b001b 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -1900,6 +1900,39 @@ def compute_value(cls, raw_value: str | None, address: Address) -> str | None: return value_or_default +class StringOrBoolField(Field): + """A field whose value can be either a string or a boolean. + + This is useful for fields that need to accept both boolean flags and string options. + Subclasses must either set `default: str | bool` or `required = True` so that the value is + always defined. + + If you expect the string to only be one of several values, set the class property + `valid_choices`. + """ + + value: str | bool | None + default: ClassVar[str | bool | None] = None + valid_choices: ClassVar[type[Enum] | tuple[str, ...] | None] = None + + @classmethod + def compute_value( + cls, raw_value: str | bool | None, address: Address + ) -> str | bool | None | Any: + value_or_default = super().compute_value(raw_value, address) + if value_or_default is not None: + if not isinstance(value_or_default, (str, bool)): + raise InvalidFieldTypeException( + address, cls.alias, raw_value, expected_type="a string or boolean" + ) + # Validate string choices if provided + if isinstance(value_or_default, str) and cls.valid_choices is not None: + _validate_choices( + address, cls.alias, [value_or_default], valid_choices=cls.valid_choices + ) + return value_or_default + + class SequenceField(Generic[T], Field): """A field whose value is a homogeneous sequence. diff --git a/src/python/pants/help/help_info_extracter.py b/src/python/pants/help/help_info_extracter.py index 6b9a5b1d69b..85536263dcc 100644 --- a/src/python/pants/help/help_info_extracter.py +++ b/src/python/pants/help/help_info_extracter.py @@ -27,7 +27,14 @@ from pants.engine.goal import GoalSubsystem from pants.engine.internals.parser import BuildFileSymbolInfo, BuildFileSymbolsInfo, Registrar from pants.engine.rules import Rule, TaskRule -from pants.engine.target import Field, RegisteredTargetTypes, StringField, Target, TargetGenerator +from pants.engine.target import ( + Field, + RegisteredTargetTypes, + StringField, + StringOrBoolField, + Target, + TargetGenerator, +) from pants.engine.unions import UnionMembership, UnionRule, is_union from pants.option.native_options import NativeOptionParser, parse_dest from pants.option.option_types import OptionInfo @@ -205,13 +212,16 @@ def create(cls, field: type[Field], *, provider: str) -> TargetFieldHelpInfo: type_hint = pretty_print_type_hint(raw_value_type) # Check if the field only allows for certain choices. - if issubclass(field, StringField) and field.valid_choices is not None: + if issubclass(field, (StringField, StringOrBoolField)) and field.valid_choices is not None: valid_choices = sorted( field.valid_choices if isinstance(field.valid_choices, tuple) else (choice.value for choice in field.valid_choices) ) - type_hint = " | ".join([*(repr(c) for c in valid_choices), "None"]) + if issubclass(field, StringOrBoolField): + type_hint = " | ".join([*(repr(c) for c in valid_choices), "bool", "None"]) + else: + type_hint = " | ".join([*(repr(c) for c in valid_choices), "None"]) if field.required: # We hackily remove `None` as a valid option for the field when it's required. This