Add repository config option to Docker registries. (Cherry pick of #1…

…5884) (#15952)

docs/markdown/Docker/tagging-docker-images.md

@@ -54,6 +54,14 @@ docker_image(
 )
 ```
 
+You may also provide a registry specific value for the repository, see next section for more details.
+```toml pants.toml
+[docker.registries.company-registry3]
+address = "reg3.company.internal"
+repository = "{parent_directory}/{name}"
+```
+
+
 Setting a repository name
 -------------------------
 
@@ -67,12 +75,18 @@ docker_image(
     repository="example/demo",
 )
 ```
-
 ```shell
 $ ./pants package src/example:demo
 # Will build the image: example/demo:latest
 ```
 
+To use a repository only for a specific registry, provide a `repository` value in the registry configuration, and this can contain placeholders in curly braces that will be interpolated for each image name.
+```toml pants.toml
+[docker.registries.demo]
+address = "reg.company.internal"
+repository = "example/{name}"
+```
+
 You can also specify a default repository name in config, and this name can contain placeholders in curly braces that will be interpolated for each `docker_image`:
 
 ```toml pants.toml
@@ -91,11 +105,17 @@ The default placeholders are:
 - `{parent_directory}`: The parent directory of `{directory}`.
 - `{name}`: The name of the docker_image target.
 - `{build_args.ARG_NAME}`: Each defined Docker build arg is available for interpolation under the `build_args.` prefix.
+- `{default_repository}`: The default repository from configuration.
+- `{target_repository}`: The repository on the `docker_image` if provided, otherwise the default repository.
 
-Since repository names often conform to patterns like these, this can save you on some boilerplate by allowing you to omit the `repository` field on each `docker_image`. But you can always override this field on specific `docker_image` targets, of course. In fact, you can use these placeholders in the `repository` field as well, if you find that helpful.
+Since repository names often conform to patterns like these, this can save you on some boilerplate
+by allowing you to omit the `repository` field on each `docker_image`. But you can always override
+this field on specific `docker_image` targets, of course. In fact, you can use these placeholders in
+the `repository` field as well, if you find that helpful.
 
 See [String interpolation using placeholder values](doc:tagging-docker-images#string-interpolation-using-placeholder-values) for more information.
 
+
 Tagging images
 --------------
 

src/python/pants/backend/docker/goals/package_image.py

@@ -7,13 +7,12 @@
 import re
 from dataclasses import dataclass
 from functools import partial
-from itertools import chain
 from typing import Iterator, cast
 
 # Re-exporting BuiltDockerImage here, as it has its natural home here, but has moved out to resolve
 # a dependency cycle from docker_build_context.
 from pants.backend.docker.package_types import BuiltDockerImage as BuiltDockerImage
-from pants.backend.docker.registries import DockerRegistries
+from pants.backend.docker.registries import DockerRegistries, DockerRegistryOptions
 from pants.backend.docker.subsystems.docker_options import DockerOptions
 from pants.backend.docker.target_types import (
     DockerBuildOptionFieldMixin,
@@ -81,17 +80,27 @@ def format_tag(self, tag: str, interpolation_context: DockerInterpolationContext
         return interpolation_context.format(tag, source=source, error_cls=DockerImageTagValueError)
 
     def format_repository(
-        self, default_repository: str, interpolation_context: DockerInterpolationContext
+        self,
+        default_repository: str,
+        interpolation_context: DockerInterpolationContext,
+        registry: DockerRegistryOptions | None = None,
     ) -> str:
         repository_context = DockerInterpolationContext.from_dict(
             {
                 "directory": os.path.basename(self.address.spec_path),
                 "name": self.address.target_name,
                 "parent_directory": os.path.basename(os.path.dirname(self.address.spec_path)),
+                "default_repository": default_repository,
+                "target_repository": self.repository.value or default_repository,
                 **interpolation_context,
             }
         )
-        if self.repository.value:
+        if registry and registry.repository:
+            repository_text = registry.repository
+            source = DockerInterpolationContext.TextSource(
+                options_scope=f"[docker.registries.{registry.alias or registry.address}].repository"
+            )
+        elif self.repository.value:
             repository_text = self.repository.value
             source = DockerInterpolationContext.TextSource(
                 address=self.address, target_alias="docker_image", field_alias=self.repository.alias
@@ -139,21 +148,20 @@ def image_refs(
 
         This method will always return a non-empty tuple.
         """
-        repository = self.format_repository(default_repository, interpolation_context)
-        image_names = tuple(
-            self.format_names(repository, self.tags.value or (), interpolation_context)
-        )
+        image_tags = self.tags.value or ()
         registries_options = tuple(registries.get(*(self.registries.value or [])))
         if not registries_options:
             # The image name is also valid as image ref without registry.
-            return image_names
+            repository = self.format_repository(default_repository, interpolation_context)
+            return tuple(self.format_names(repository, image_tags, interpolation_context))
 
         return tuple(
             "/".join([registry.address, image_name])
             for registry in registries_options
-            for image_name in chain(
-                image_names,
-                self.format_names(repository, registry.extra_image_tags, interpolation_context),
+            for image_name in self.format_names(
+                self.format_repository(default_repository, interpolation_context, registry),
+                image_tags + registry.extra_image_tags,
+                interpolation_context,
             )
         )
 

src/python/pants/backend/docker/goals/package_image_test.py

@@ -5,6 +5,7 @@
 
 import logging
 import os.path
+from collections import namedtuple
 from textwrap import dedent
 from typing import Callable, ContextManager, cast
 
@@ -38,7 +39,10 @@
     DockerBuildEnvironmentRequest,
 )
 from pants.backend.docker.util_rules.docker_build_env import rules as build_env_rules
-from pants.backend.docker.value_interpolation import DockerInterpolationContext
+from pants.backend.docker.value_interpolation import (
+    DockerInterpolationContext,
+    DockerInterpolationError,
+)
 from pants.engine.addresses import Address
 from pants.engine.fs import EMPTY_DIGEST, EMPTY_FILE_DIGEST, EMPTY_SNAPSHOT, Snapshot
 from pants.engine.platform import Platform
@@ -245,7 +249,8 @@ 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, tags\."
+        r"Try with one of: build_args, default_repository, directory, name, pants, "
+        r"parent_directory, tags, target_repository\."
     )
     with pytest.raises(DockerRepositoryNameError, match=err1):
         assert_build(
@@ -987,26 +992,62 @@ def test_parse_image_id_from_docker_build_output(expected: str, stdout: str, std
     assert expected == parse_image_id_from_docker_build_output(stdout.encode(), stderr.encode())
 
 
+ImageRefTest = namedtuple(
+    "ImageRefTest",
+    "docker_image, registries, default_repository, expect_refs, expect_error",
+    defaults=({}, {}, "{name}", (), None),
+)
+
+
 @pytest.mark.parametrize(
-    "raw_values, expect_raises, image_refs",
+    "test",
     [
-        (dict(name="lowercase"), no_exception(), ("lowercase:latest",)),
-        (dict(name="CamelCase"), no_exception(), ("camelcase:latest",)),
-        (dict(image_tags=["CamelCase"]), no_exception(), ("image:CamelCase",)),
-        (dict(registries=["REG1.example.net"]), no_exception(), ("REG1.example.net/image:latest",)),
+        ImageRefTest(docker_image=dict(name="lowercase"), expect_refs=("lowercase:latest",)),
+        ImageRefTest(docker_image=dict(name="CamelCase"), expect_refs=("camelcase:latest",)),
+        ImageRefTest(docker_image=dict(image_tags=["CamelCase"]), expect_refs=("image:CamelCase",)),
+        ImageRefTest(
+            docker_image=dict(registries=["REG1.example.net"]),
+            expect_refs=("REG1.example.net/image:latest",),
+        ),
+        ImageRefTest(
+            docker_image=dict(registries=["docker.io", "@private"], repository="our-the/pkg"),
+            registries=dict(private={"address": "our.registry", "repository": "the/pkg"}),
+            expect_refs=("docker.io/our-the/pkg:latest", "our.registry/the/pkg:latest"),
+        ),
+        ImageRefTest(
+            docker_image=dict(
+                registries=["docker.io", "@private"],
+                repository="{parent_directory}/{default_repository}",
+            ),
+            registries=dict(
+                private={"address": "our.registry", "repository": "{target_repository}/the/pkg"}
+            ),
+            expect_refs=("docker.io/test/image:latest", "our.registry/test/image/the/pkg:latest"),
+        ),
+        ImageRefTest(
+            docker_image=dict(repository="{default_repository}/a"),
+            default_repository="{target_repository}/b",
+            expect_error=pytest.raises(
+                DockerInterpolationError,
+                match=(
+                    r"Invalid value for the `repository` field of the `docker_image` target at "
+                    r"src/test/docker:image: '\{default_repository\}/a'\.\n\n"
+                    r"The formatted placeholders recurse too deep\.\n"
+                    r"'\{default_repository\}/a' => '\{target_repository\}/b/a' => "
+                    r"'\{default_repository\}/a/b/a'"
+                ),
+            ),
+        ),
     ],
 )
-def test_image_ref_formatting(
-    raw_values: dict, expect_raises: ContextManager, image_refs: tuple[str, ...]
-) -> None:
-    address = Address("test", target_name=raw_values.pop("name", "image"))
-    tgt = DockerImageTarget(raw_values, address)
+def test_image_ref_formatting(test: ImageRefTest) -> None:
+    address = Address("src/test/docker", target_name=test.docker_image.pop("name", "image"))
+    tgt = DockerImageTarget(test.docker_image, address)
     field_set = DockerFieldSet.create(tgt)
-    default_repository = "{name}"
-    registries = DockerRegistries.from_dict({})
+    registries = DockerRegistries.from_dict(test.registries)
     interpolation_context = DockerInterpolationContext.from_dict({})
-    with expect_raises:
+    with test.expect_error or no_exception():
         assert (
-            field_set.image_refs(default_repository, registries, interpolation_context)
-            == image_refs
+            field_set.image_refs(test.default_repository, registries, interpolation_context)
+            == test.expect_refs
         )

src/python/pants/backend/docker/registries.py

@@ -31,6 +31,7 @@ class DockerRegistryOptions:
     default: bool = False
     skip_push: bool = False
     extra_image_tags: tuple[str, ...] = ()
+    repository: str | None = None
 
     @classmethod
     def from_dict(cls, alias: str, d: dict[str, Any]) -> DockerRegistryOptions:
@@ -42,6 +43,7 @@ def from_dict(cls, alias: str, d: dict[str, Any]) -> DockerRegistryOptions:
             extra_image_tags=tuple(
                 d.get("extra_image_tags", DockerRegistryOptions.extra_image_tags)
             ),
+            repository=Parser.to_value_type(d.get("repository"), str, None, None),
         )
 
     def register(self, registries: dict[str, DockerRegistryOptions]) -> None:

src/python/pants/backend/docker/registries_test.py

@@ -88,3 +88,11 @@ def test_extra_image_tags() -> None:
     reg1, reg2 = registries.get("@reg1", "@reg2")
     assert reg1.extra_image_tags == ()
     assert reg2.extra_image_tags == ("latest", "v{build_args.VERSION}")
+
+
+def test_repository() -> None:
+    registries = DockerRegistries.from_dict(
+        {"reg1": {"address": "registry1", "repository": "{name}/foo"}}
+    )
+    (reg1,) = registries.get("@reg1")
+    assert reg1.repository == "{name}/foo"

src/python/pants/backend/docker/subsystems/docker_options.py

@@ -46,6 +46,7 @@ class DockerOptions(Subsystem):
                         "default": bool,
                         "extra_image_tags": [],
                         "skip_push": bool,
+                        "repository": str,
                     },
                     ...
                 }
@@ -67,6 +68,10 @@ class DockerOptions(Subsystem):
             `extra_image_tags` option. The tags may use value formatting the same as for the
             `image_tags` field of the `docker_image` target.
 
+            When a registry provides a `repository` value, it will be used instead of the
+            `docker_image.repository` or the default repository. Using the placeholders
+            `{target_repository}` or `{default_repository}` those overridden values may be
+            incorporated into the registry specific repository value.
             """
         ),
         fromfile=True,
@@ -80,7 +85,7 @@ class DockerOptions(Subsystem):
             The value is formatted and may reference these variables (in addition to the normal
             placeheolders derived from the Dockerfile and build args etc):
 
-            {bullet_list(["name", "directory", "parent_directory"])}
+            {bullet_list(["name", "directory", "parent_directory", "target_repository"])}
 
             Example: `--default-repository="{{directory}}/{{name}}"`.
 
@@ -90,6 +95,8 @@ class DockerOptions(Subsystem):
 
             Use the `repository` field to set this value directly on a `docker_image` target.
 
+            Registries may override the repository value for a specific registry.
+
             Any registries or tags are added to the image name as required, and should
             not be part of the repository name.
             """

src/python/pants/backend/docker/target_types.py

@@ -207,8 +207,10 @@ class DockerImageRepositoryField(StringField):
 
         {_interpolation_help.format(kind="repository")}
 
-        Additional placeholders for the repository field are: `name`, `directory` and
-        `parent_directory`.
+        Additional placeholders for the repository field are: `name`, `directory`,
+        `parent_directory`, and `default_repository`.
+
+        Registries may also configure the repository value for specific registries.
 
         See the documentation for `[docker].default_repository` for more information.
         """

src/python/pants/backend/docker/value_interpolation.py

@@ -77,8 +77,18 @@ def merge(self, other: Mapping[str, str | Mapping[str, str]]) -> DockerInterpola
     def format(
         self, text: str, *, source: TextSource, error_cls: type[ErrorT] | None = None
     ) -> str:
+        stack = [text]
         try:
-            return text.format(**self)
+            while "{" in stack[-1] and "}" in stack[-1]:
+                if len(stack) >= 5:
+                    raise DockerInterpolationError(
+                        "The formatted placeholders recurse too deep.\n"
+                        + " => ".join(map(repr, stack))
+                    )
+                stack.append(stack[-1].format(**self))
+                if stack[-1] == stack[-2]:
+                    break
+            return stack[-1]
         except (KeyError, DockerInterpolationError) as e:
             default_error_cls = DockerInterpolationError
             msg = f"Invalid value for the {source}: {text!r}.\n\n"

src/python/pants/option/parser.py

@@ -501,12 +501,13 @@ def _convert_member_type(member_type, value):
         except ValueError as error:
             raise ParseError(str(error))
 
-    def to_value_type(self, val_str, type_arg, member_type, dest):
+    @classmethod
+    def to_value_type(cls, val_str, type_arg, member_type, dest):
         """Convert a string to a value of the option's type."""
         if val_str is None:
             return None
         if type_arg == bool:
-            return self.ensure_bool(val_str)
+            return cls.ensure_bool(val_str)
         try:
             if type_arg == list:
                 return ListValueComponent.create(val_str, member_type=member_type)