From 827247ae7b5cb7b81db8f8d639e2da905fa4ffcd Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Thu, 19 Mar 2026 21:28:09 -0700 Subject: [PATCH] Extract IImageNameResolver interface and unify implementation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../GenerateBuildMatrixCommandTests.cs | 2 +- src/ImageBuilder/Commands/BuildCommand.cs | 7 +- .../Commands/GenerateBuildMatrixCommand.cs | 6 +- src/ImageBuilder/DockerHelper.cs | 2 - src/ImageBuilder/IImageNameResolver.cs | 37 ++++ src/ImageBuilder/ImageCacheService.cs | 8 +- src/ImageBuilder/ImageNameResolver.cs | 165 ++++++++---------- 7 files changed, 121 insertions(+), 106 deletions(-) create mode 100644 src/ImageBuilder/IImageNameResolver.cs diff --git a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs index d017fd0e1..7c70a2185 100644 --- a/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs +++ b/src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs @@ -165,7 +165,7 @@ private static void SetCacheResult(Mock imageCacheServiceMoc It.IsAny(), It.Is(platform => platform.Dockerfile == dockerfilePath), It.IsAny(), - It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 42c0f1ea0..878b70bdf 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -29,7 +29,7 @@ public class BuildCommand : ManifestCommand private readonly ImageDigestCache _imageDigestCache; private readonly List _processedTags = new List(); private readonly HashSet _builtPlatforms = new(); - private readonly Lazy _imageNameResolver; + private readonly Lazy _imageNameResolver; private readonly Lazy _storageAccountToken; /// @@ -67,8 +67,9 @@ public BuildCommand( manifestServiceFactory.Create(Options.CredentialsOptions)); _imageDigestCache = new ImageDigestCache(_manifestService); - _imageNameResolver = new Lazy(() => - new ImageNameResolverForBuild( + _imageNameResolver = new Lazy(() => + new ImageNameResolver( + DigestResolutionMode.Staging, Options.BaseImageOverrideOptions, Manifest, Options.RepoPrefix, diff --git a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs index d90f29988..50e09cfbe 100644 --- a/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs +++ b/src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs @@ -24,7 +24,7 @@ public class GenerateBuildMatrixCommand : ManifestCommand _logger; private readonly ImageDigestCache _imageDigestCache; - private readonly Lazy _imageNameResolver; + private readonly Lazy _imageNameResolver; public GenerateBuildMatrixCommand(IManifestJsonService manifestJsonService, IImageCacheService imageCacheService, IManifestServiceFactory manifestServiceFactory, ILogger logger) : base(manifestJsonService) { @@ -42,8 +42,8 @@ public GenerateBuildMatrixCommand(IManifestJsonService manifestJsonService, IIma _imageDigestCache = new ImageDigestCache( new Lazy( () => manifestServiceFactory.Create(Options.CredentialsOptions))); - _imageNameResolver = new Lazy(() => - new ImageNameResolverForMatrix(Options.BaseImageOverrideOptions, Manifest, Options.RepoPrefix, Options.SourceRepoPrefix)); + _imageNameResolver = new Lazy(() => + new ImageNameResolver(DigestResolutionMode.Public, Options.BaseImageOverrideOptions, Manifest, Options.RepoPrefix, Options.SourceRepoPrefix)); } protected override string Description => "Generate the Azure DevOps build matrix for building the images"; diff --git a/src/ImageBuilder/DockerHelper.cs b/src/ImageBuilder/DockerHelper.cs index fdacefd31..6dfda1d40 100644 --- a/src/ImageBuilder/DockerHelper.cs +++ b/src/ImageBuilder/DockerHelper.cs @@ -168,8 +168,6 @@ public static string NormalizeRepo(string image) public static string TrimRegistry(string tag, string? registry) => tag.TrimStartString($"{registry}/"); - public static bool IsInRegistry(string tag, string registry) => registry is not null && tag.StartsWith(registry); - public static string? GetRegistry(string imageName) { int separatorIndex = imageName.IndexOf("/"); diff --git a/src/ImageBuilder/IImageNameResolver.cs b/src/ImageBuilder/IImageNameResolver.cs new file mode 100644 index 000000000..2897c7247 --- /dev/null +++ b/src/ImageBuilder/IImageNameResolver.cs @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.DotNet.ImageBuilder.ViewModel; + +namespace Microsoft.DotNet.ImageBuilder; + +/// +/// Resolves image names and tags for FROM instructions across different contexts +/// (e.g. local builds, registry pulls, digest queries). +/// +public interface IImageNameResolver +{ + /// + /// Returns the tag to use for interacting with the image of a FROM instruction that has been pulled or built locally. + /// + /// Tag of the FROM image. + string GetFromImageLocalTag(string fromImage); + + /// + /// Returns the tag to use for pulling the image of a FROM instruction. + /// + /// Tag of the FROM image. + string GetFromImagePullTag(string fromImage); + + /// + /// Returns the tag that represents the publicly available tag of a FROM instruction. + /// + /// Tag of the FROM image. + string GetFromImagePublicTag(string fromImage); + + /// + /// Returns the image name to use when querying for the digest of the final stage base image. + /// + /// Platform whose final stage image name is resolved. + string GetFinalStageImageNameForDigestQuery(PlatformInfo platform); +} diff --git a/src/ImageBuilder/ImageCacheService.cs b/src/ImageBuilder/ImageCacheService.cs index 7641d99a8..755c3ce07 100644 --- a/src/ImageBuilder/ImageCacheService.cs +++ b/src/ImageBuilder/ImageCacheService.cs @@ -39,7 +39,7 @@ Task CheckForCachedImageAsync( ImageData? srcImageData, PlatformData platformData, ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, + IImageNameResolver imageNameResolver, string? sourceRepoUrl, bool isLocalBaseImageExpected, bool isDryRun); @@ -82,7 +82,7 @@ public async Task CheckForCachedImageAsync( ImageData? srcImageData, PlatformData platformData, ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, + IImageNameResolver imageNameResolver, string? sourceRepoUrl, bool isLocalBaseImageExpected, bool isDryRun) @@ -158,7 +158,7 @@ private async Task CheckForCachedImageFromImageInfoAsync( PlatformInfo platform, PlatformData srcPlatformData, ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, + IImageNameResolver imageNameResolver, string? sourceRepoUrl, bool isLocalBaseImageExpected, bool isDryRun) @@ -188,7 +188,7 @@ private async Task IsBaseImageDigestUpToDateAsync( PlatformInfo platform, PlatformData srcPlatformData, ImageDigestCache imageDigestCache, - ImageNameResolver imageNameResolver, + IImageNameResolver imageNameResolver, bool isLocalImageExpected, bool isDryRun) { diff --git a/src/ImageBuilder/ImageNameResolver.cs b/src/ImageBuilder/ImageNameResolver.cs index 3fd3a29dd..775425334 100644 --- a/src/ImageBuilder/ImageNameResolver.cs +++ b/src/ImageBuilder/ImageNameResolver.cs @@ -1,54 +1,70 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +using System; using Microsoft.DotNet.ImageBuilder.Commands; using Microsoft.DotNet.ImageBuilder.ViewModel; namespace Microsoft.DotNet.ImageBuilder; -public abstract class ImageNameResolver +/// +/// Controls how internal image names are resolved when querying for digests. +/// +public enum DigestResolutionMode +{ + /// + /// Use the staging ACR registry as-is for internal image digest queries (build scenario). + /// + Staging, + + /// + /// Use the public MCR registry for internal image digest queries (matrix generation scenario). + /// + Public +} + +/// +public class ImageNameResolver : IImageNameResolver { private readonly BaseImageOverrideOptions _baseImageOverrideOptions; private readonly string? _repoPrefix; private readonly string? _sourceRepoPrefix; + private readonly DigestResolutionMode _digestResolutionMode; + private readonly ManifestInfo _manifest; - public ImageNameResolver(BaseImageOverrideOptions baseImageOverrideOptions, ManifestInfo manifest, string? repoPrefix, string? sourceRepoPrefix) + public ImageNameResolver( + DigestResolutionMode digestResolutionMode, + BaseImageOverrideOptions baseImageOverrideOptions, + ManifestInfo manifest, + string? repoPrefix, + string? sourceRepoPrefix) { + _digestResolutionMode = digestResolutionMode; _baseImageOverrideOptions = baseImageOverrideOptions; - Manifest = manifest; + _manifest = manifest; _repoPrefix = repoPrefix; _sourceRepoPrefix = sourceRepoPrefix; } - protected ManifestInfo Manifest { get; } - - /// - /// Returns the tag to use for interacting with the image of a FROM instruction that has been pulled or built locally. - /// - /// Tag of the FROM image. + /// public string GetFromImageLocalTag(string fromImage) => // Provides the overridable value of the registry (e.g. dotnetdocker.azurecr.io) because that is the registry that // would be used for tags that exist locally. - GetFromImageTag(fromImage, Manifest.Registry); + GetFromImageTag(fromImage, _manifest.Registry); - /// - /// Returns the tag to use for pulling the image of a FROM instruction. - /// - /// Tag of the FROM image. + /// public string GetFromImagePullTag(string fromImage) => // Provides the raw registry value from the manifest (e.g. mcr.microsoft.com). This accounts for images that // are classified as external within the model but they are owned internally and not mirrored. An example of // this is sample images. By comparing their base image tag to that raw registry value from the manifest, we // can know that these are owned internally and not to attempt to pull them from the mirror location. - GetFromImageTag(fromImage, Manifest.Model.Registry); + GetFromImageTag(fromImage, _manifest.Model.Registry); - /// - /// Returns the tag that represents the publicly available tag of a FROM instruction. - /// - /// Tag of the FROM image. + /// /// /// This compares the registry of the image tag to determine if it's internally owned. If so, it returns - /// the tag using the raw (non-overriden) registry from the manifest (e.g. mcr.microsoft.com). Otherwise, + /// the tag using the raw (non-overridden) registry from the manifest (e.g. mcr.microsoft.com). Otherwise, /// it returns the image tag unchanged. /// public string GetFromImagePublicTag(string fromImage) @@ -60,11 +76,36 @@ public string GetFromImagePublicTag(string fromImage) } else { - return $"{Manifest.Model.Registry}/{trimmed}"; + return $"{_manifest.Model.Registry}/{trimmed}"; } } - public abstract string GetFinalStageImageNameForDigestQuery(PlatformInfo platform); + /// + public string GetFinalStageImageNameForDigestQuery(PlatformInfo platform) + { + string imageName = platform.FinalStageFromImage ?? string.Empty; + + if (platform.IsInternalFromImage(imageName)) + { + return _digestResolutionMode switch + { + // For build scenarios, the image is already formatted with the staging ACR registry and repo prefix + // (e.g. dotnetdocker.azurecr.io/dotnet-staging/12345/sdk:8.0), so use it as-is. + DigestResolutionMode.Staging => imageName, + + // For matrix generation scenarios, strip the staging prefix and re-prefix with the public MCR + // registry (e.g. mcr.microsoft.com/dotnet/sdk:8.0). + DigestResolutionMode.Public => + $"{_manifest.Model.Registry}/{TrimInternallyOwnedRegistryAndRepoPrefix(DockerHelper.NormalizeRepo(imageName))}", + + _ => throw new NotSupportedException($"Unsupported digest resolution mode: {_digestResolutionMode}") + }; + } + + // External images are formatted to target the mirror location in the ACR + // (e.g. dotnetdocker.azurecr.io/mirror/amd64/alpine:3.XX). + return GetFromImagePullTag(imageName); + } /// /// Gets the tag to use for the image of a FROM instruction. @@ -78,88 +119,26 @@ private string GetFromImageTag(string fromImage, string? registry) { fromImage = _baseImageOverrideOptions.ApplyBaseImageOverride(fromImage); - if ((registry is not null && DockerHelper.IsInRegistry(fromImage, registry)) || - DockerHelper.IsInRegistry(fromImage, Manifest.Model.Registry) + if (IsInRegistry(fromImage, registry) + || IsInRegistry(fromImage, _manifest.Model.Registry) || _sourceRepoPrefix is null) { return fromImage; } string srcImage = TrimInternallyOwnedRegistryAndRepoPrefix(DockerHelper.NormalizeRepo(fromImage)); - return $"{Manifest.Registry}/{_sourceRepoPrefix}{srcImage}"; + return $"{_manifest.Registry}/{_sourceRepoPrefix}{srcImage}"; } - protected string TrimInternallyOwnedRegistryAndRepoPrefix(string imageTag) => + private string TrimInternallyOwnedRegistryAndRepoPrefix(string imageTag) => IsInInternallyOwnedRegistry(imageTag) ? DockerHelper.TrimRegistry(imageTag).TrimStartString(_repoPrefix ?? string.Empty) : imageTag; private bool IsInInternallyOwnedRegistry(string imageTag) => - DockerHelper.IsInRegistry(imageTag, Manifest.Registry) || - DockerHelper.IsInRegistry(imageTag, Manifest.Model.Registry); -} + IsInRegistry(imageTag, _manifest.Registry) || + IsInRegistry(imageTag, _manifest.Model.Registry); -public class ImageNameResolverForBuild : ImageNameResolver -{ - public ImageNameResolverForBuild( - BaseImageOverrideOptions baseImageOverrideOptions, - ManifestInfo manifest, - string? repoPrefix, - string? sourceRepoPrefix) - : base(baseImageOverrideOptions, manifest, repoPrefix, sourceRepoPrefix) - { - } - - public override string GetFinalStageImageNameForDigestQuery(PlatformInfo platform) - { - // For build scenarios, we want to query for the digest of the image according to whether it's internal or not. - // An internal image will already be formatted with the registry and staging repo prefix, so we can use it as is - // (e.g. dotnetdocker.azurecr.io/dotnet-staging/12345/sdk:8.0). An external image should be formatted to target - // the mirror location in the ACR (e.g. dotnetdocker.azurecr.io/mirror/amd64/alpine:3.20). - - string imageName = platform.FinalStageFromImage ?? string.Empty; - - if (platform.IsInternalFromImage(imageName)) - { - return imageName; - } - else - { - return GetFromImagePullTag(imageName); - } - } -} - -public class ImageNameResolverForMatrix : ImageNameResolver -{ - public ImageNameResolverForMatrix( - BaseImageOverrideOptions baseImageOverrideOptions, - ManifestInfo manifest, - string? repoPrefix, - string? sourceRepoPrefix) - : base(baseImageOverrideOptions, manifest, repoPrefix, sourceRepoPrefix) - { - } - - public override string GetFinalStageImageNameForDigestQuery(PlatformInfo platform) - { - // For matrix generation scenarios, we want to query for the digest of the image according - // to whether it's internal or not, just like we do for build. But the target location will - // be different. For internal images, we want to query mcr.microsoft.com (e.g. - // mcr.microsoft.com/dotnet/sdk/8.0). For external images, - // we want to query the mirror location in the ACR (e.g. - // dotnetdockerstaging.azurecr.io/mirror/amd64/alpine:3.20) - - string imageName = platform.FinalStageFromImage ?? string.Empty; - - if (platform.IsInternalFromImage(imageName)) - { - string trimmedImageName = TrimInternallyOwnedRegistryAndRepoPrefix(DockerHelper.NormalizeRepo(imageName)); - return $"{Manifest.Model.Registry}/{trimmedImageName}"; - } - else - { - return GetFromImagePullTag(imageName); - } - } + private static bool IsInRegistry(string imageReference, string? registry) => + !string.IsNullOrEmpty(registry) && imageReference.StartsWith(registry); }