From bfff439a43e8cab270a85bcb3ef8d587c86964d7 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Mon, 23 Mar 2026 14:48:40 -0700 Subject: [PATCH] Remove unnecessary Docker login from CleanAcrImagesCommand The cleanup command wrapped operations in ExecuteWithCredentialsAsync, which tried to Docker-login using the registry's service connection from publishConfig.RegistryAuthentication. That service connection was not referenced in the pipeline stage, causing Azure DevOps to refuse OIDC token issuance. The Docker login was unnecessary - all ACR operations authenticate directly via cleanServiceConnection through the Azure SDK. Removing the wrapper and the unused IRegistryCredentialsProvider dependency fixes the pipeline failure. Fixes #2028 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CleanAcrImagesCommandTest.cs | 14 ++++---- .../Commands/CleanAcrImagesCommand.cs | 32 +++++++------------ 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/ImageBuilder.Tests/CleanAcrImagesCommandTest.cs b/src/ImageBuilder.Tests/CleanAcrImagesCommandTest.cs index d19719975..47b413590 100644 --- a/src/ImageBuilder.Tests/CleanAcrImagesCommandTest.cs +++ b/src/ImageBuilder.Tests/CleanAcrImagesCommandTest.cs @@ -54,7 +54,7 @@ public async Task StagingRepos() IAcrClientFactory acrClientFactory = CreateAcrClientFactory(AcrName, acrClientMock.Object); CleanAcrImagesCommand command = new( - acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "build-staging/*"; command.Options.Action = CleanAcrImagesAction.Delete; @@ -121,7 +121,7 @@ public async Task PublicNightlyRepos() AcrName, [repo1ContentClient, repo2ContentClient, repo3ContentClient, repo4ContentClient]); CleanAcrImagesCommand command = new( - acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "public/dotnet/*nightly/*"; command.Options.Action = CleanAcrImagesAction.PruneDangling; @@ -166,7 +166,7 @@ public async Task DeleteEmptyTestRepo() IAcrClientFactory acrClientFactory = CreateAcrClientFactory(AcrName, acrClientMock.Object); CleanAcrImagesCommand command = new( - acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "test/*"; command.Options.Action = CleanAcrImagesAction.PruneAll; @@ -205,7 +205,7 @@ public async Task DeleteAllExpiredImagesTestRepo() IAcrClientFactory acrClientFactory = CreateAcrClientFactory(AcrName, acrClientMock.Object); CleanAcrImagesCommand command = new CleanAcrImagesCommand( - acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, Mock.Of(), Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "test/*"; command.Options.Action = CleanAcrImagesAction.PruneAll; @@ -254,7 +254,7 @@ public async Task TestRepos() IAcrContentClientFactory acrContentClientFactory = CreateAcrContentClientFactory(AcrName, [repo1ContentClientMock, repo2ContentClientMock]); CleanAcrImagesCommand command = new CleanAcrImagesCommand( - acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "test/*"; command.Options.Action = CleanAcrImagesAction.PruneAll; @@ -311,7 +311,7 @@ public async Task DeleteEolImages() Mock lifecycleMetadataServiceMock = CreateLifecycleMetadataServiceMock(age, repo1Name); CleanAcrImagesCommand command = new CleanAcrImagesCommand( - acrClientFactory, acrContentClientFactory, Mock.Of>(), lifecycleMetadataServiceMock.Object, Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, acrContentClientFactory, Mock.Of>(), lifecycleMetadataServiceMock.Object, Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "test/*"; command.Options.Action = CleanAcrImagesAction.PruneEol; @@ -361,7 +361,7 @@ public async Task ExcludedImages() AcrName, [repo1ContentClient, repo2ContentClient]); CleanAcrImagesCommand command = new( - acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); + acrClientFactory, acrContentClientFactory, Mock.Of>(), Mock.Of(), Microsoft.Extensions.Options.Options.Create(new PublishConfiguration())); command.Options.RegistryName = AcrName; command.Options.RepoName = "public/dotnet/nightly/*"; command.Options.Action = CleanAcrImagesAction.PruneAll; diff --git a/src/ImageBuilder/Commands/CleanAcrImagesCommand.cs b/src/ImageBuilder/Commands/CleanAcrImagesCommand.cs index 36904992e..7f57845fe 100644 --- a/src/ImageBuilder/Commands/CleanAcrImagesCommand.cs +++ b/src/ImageBuilder/Commands/CleanAcrImagesCommand.cs @@ -25,7 +25,6 @@ public class CleanAcrImagesCommand : Command _logger; private readonly ILifecycleMetadataService _lifecycleMetadataService; - private readonly IRegistryCredentialsProvider _registryCredentialsProvider; private readonly PublishConfiguration _publishConfig; private const int MaxConcurrentDeleteRequestsPerRepo = 5; @@ -35,14 +34,12 @@ public CleanAcrImagesCommand( IAcrContentClientFactory acrContentClientFactory, ILogger logger, ILifecycleMetadataService lifecycleMetadataService, - IRegistryCredentialsProvider registryCredentialsProvider, IOptions publishConfigOptions) { _acrClientFactory = acrClientFactory ?? throw new ArgumentNullException(nameof(acrClientFactory)); _acrContentClientFactory = acrContentClientFactory; _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _lifecycleMetadataService = lifecycleMetadataService ?? throw new ArgumentNullException(nameof(lifecycleMetadataService)); - _registryCredentialsProvider = registryCredentialsProvider ?? throw new ArgumentNullException(nameof(registryCredentialsProvider)); _publishConfig = publishConfigOptions.Value; } @@ -70,25 +67,18 @@ public override async Task ExecuteAsync() List deletedRepos = new List(); List deletedImages = new List(); - await _registryCredentialsProvider.ExecuteWithCredentialsAsync( - isDryRun: false, - async () => + IEnumerable cleanupTasks = await repositoryNames + .Where(repoName => repoNameFilterRegex.IsMatch(repoName)) + .Select(repoName => acrClient.GetRepository(repoName)) + .Select(repo => { - IEnumerable cleanupTasks = await repositoryNames - .Where(repoName => repoNameFilterRegex.IsMatch(repoName)) - .Select(repoName => acrClient.GetRepository(repoName)) - .Select(repo => - { - Acr acr = Acr.Parse(Options.RegistryName); - IAcrContentClient acrContentClient = CreateAcrContentClient(acr, repo.Name); - return ProcessRepoAsync(acrClient, acrContentClient, repo, deletedRepos, deletedImages); - }) - .ToArrayAsync(); - - await Task.WhenAll(cleanupTasks); - }, - Options.CredentialsOptions, - registryName: Options.RegistryName); + Acr acr = Acr.Parse(Options.RegistryName); + IAcrContentClient acrContentClient = CreateAcrContentClient(acr, repo.Name); + return ProcessRepoAsync(acrClient, acrContentClient, repo, deletedRepos, deletedImages); + }) + .ToArrayAsync(); + + await Task.WhenAll(cleanupTasks); await LogSummaryAsync(acrClient, deletedRepos, deletedImages); }