Skip to content

Commit aaa7ec9

Browse files
lbussellCopilot
andauthored
Fix CopyBaseImages failure for external source registries (#1975)
Fix the `check-base-image-updates-official` pipeline caused by `CopyImageService.ImportImageAsync` throwing `InvalidOperationException` when the source registry is an external registry like `docker.io`. PR #1945 moved subscription and resource group lookups into `CopyImageService`, but the new code unconditionally calls `GetRegistryResource()` for the source registry, which throws when the registry isn't in the publish configuration. External registries like `docker.io` use username/password credentials for ACR import (via `RegistryAddress` + `Credentials`), not Azure resource IDs — they are intentionally not in the `RegistryAuthentication` list. - Check `FindRegistryAuthentication()` before calling `GetRegistryResource()` in `CopyImageService.ImportImageAsync`, so external source registries correctly get a null `ResourceId` and use the `RegistryAddress` code path - Add test `ImportImageAsync_ExternalSourceRegistry_DoesNotRequireSourceRegistryInPublishConfig` to cover the non-dry-run external registry scenario (existing test only covered the dry-run path) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 22bd4c6 commit aaa7ec9

3 files changed

Lines changed: 73 additions & 5 deletions

File tree

src/ImageBuilder.Tests/CopyImageServiceTests.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Threading.Tasks;
6+
using Azure.Core;
57
using Microsoft.DotNet.ImageBuilder.Configuration;
68
using Microsoft.DotNet.ImageBuilder.Tests.Helpers;
79
using Microsoft.Extensions.Logging;
@@ -35,4 +37,64 @@ await Should.NotThrowAsync(() =>
3537
srcRegistryName: "docker.io",
3638
isDryRun: true));
3739
}
40+
41+
/// <summary>
42+
/// When the source registry is external (e.g., docker.io) and not in the publish configuration,
43+
/// ImportImageAsync should proceed past the registry lookup without throwing. External registries
44+
/// use RegistryAddress + Credentials for ACR import, not ResourceId.
45+
/// </summary>
46+
[Fact]
47+
public async Task ImportImageAsync_ExternalSourceRegistry_DoesNotRequireSourceRegistryInPublishConfig()
48+
{
49+
var publishConfig = new PublishConfiguration
50+
{
51+
RegistryAuthentication =
52+
[
53+
new RegistryAuthentication
54+
{
55+
Server = "myacr.azurecr.io",
56+
ResourceGroup = "my-rg",
57+
Subscription = Guid.NewGuid().ToString(),
58+
ServiceConnection = new ServiceConnection
59+
{
60+
Name = "test",
61+
Id = Guid.NewGuid().ToString(),
62+
TenantId = Guid.NewGuid().ToString(),
63+
ClientId = Guid.NewGuid().ToString()
64+
}
65+
}
66+
]
67+
};
68+
69+
var mockCredProvider = new Mock<IAzureTokenCredentialProvider>();
70+
mockCredProvider
71+
.Setup(x => x.GetCredential(It.IsAny<ServiceConnection>()))
72+
.Returns(Mock.Of<TokenCredential>());
73+
74+
var service = new CopyImageService(
75+
Mock.Of<ILogger<CopyImageService>>(),
76+
mockCredProvider.Object,
77+
ConfigurationHelper.CreateOptionsMock(publishConfig));
78+
79+
try
80+
{
81+
await service.ImportImageAsync(
82+
destTagNames: ["mirror/repo:tag"],
83+
destAcrName: "myacr.azurecr.io",
84+
srcTagName: "repo:tag",
85+
srcRegistryName: "docker.io",
86+
isDryRun: false);
87+
}
88+
catch (InvalidOperationException ex) when (ex.Message.Contains("No authentication details found"))
89+
{
90+
Assert.Fail($"External source registries should not require publish configuration: {ex.Message}");
91+
}
92+
catch
93+
{
94+
// ARM SDK failures are expected with mocked credentials
95+
}
96+
97+
// Verify execution reached the ARM client (past the registry lookup)
98+
mockCredProvider.Verify(x => x.GetCredential(It.IsAny<ServiceConnection>()), Times.Once);
99+
}
38100
}

src/ImageBuilder/Configuration/ConfigurationExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ auth.Server is not null
4747
&& Acr.Parse(auth.Server) == targetAcr);
4848
}
4949

50-
public static ResourceIdentifier GetRegistryResource(this PublishConfiguration publishConfig, string registry)
50+
public static ResourceIdentifier GetAcrResource(this PublishConfiguration publishConfig, string registry)
5151
{
5252
var registryAuthentication = publishConfig.FindRegistryAuthentication(registry)
5353
?? throw new InvalidOperationException(

src/ImageBuilder/CopyImageService.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,16 @@ public async Task ImportImageAsync(
7070
return;
7171
}
7272

73-
var destResourceId = _publishConfig.GetRegistryResource(destAcrName);
74-
var srcResourceId = srcRegistryName is not null
75-
? _publishConfig.GetRegistryResource(srcRegistryName)
76-
: null;
73+
var destResourceId = _publishConfig.GetAcrResource(destAcrName);
74+
75+
// Only look up the source resource ID for registries in the publish config (i.e. ACRs).
76+
// External registries like docker.io use RegistryAddress + Credentials instead.
77+
ResourceIdentifier? srcResourceId =
78+
// TODO: In the future, ACR credentials (user/pw) could be passed via PublishConfiguration
79+
// as well, and resolved here.
80+
srcRegistryName is not null && _publishConfig.FindRegistryAuthentication(srcRegistryName) is not null
81+
? _publishConfig.GetAcrResource(srcRegistryName)
82+
: null;
7783

7884
// Azure ACR import only supports one source identifier. Use ResourceId for ACR-to-ACR
7985
// imports (same tenant), or RegistryAddress for external registries.

0 commit comments

Comments
 (0)