Remove Build command requirement for service principal

src/Microsoft.DotNet.ImageBuilder/src/AuthHelper.cs

@@ -2,19 +2,49 @@
 // 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 System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using Azure.Core;
 using Azure.Identity;
+using Azure.ResourceManager;
 
 namespace Microsoft.DotNet.ImageBuilder
 {
     public static class AuthHelper
     {
-        public static async Task<string> GetDefaultAccessTokenAsync(string resource)
+        private const string DefaultScope = "https://management.azure.com/.default";
+
+        public static async Task<(string token, Guid tenantId)> GetDefaultAccessTokenAsync(ILoggerService loggerService, string resource = DefaultScope)
         {
             DefaultAzureCredential credential  = new();
             AccessToken token = await credential.GetTokenAsync(new TokenRequestContext([ resource ]));
-            return token.Token;
+            Guid tenantId = GetTenantId(loggerService, credential);
+            return (token.Token, tenantId);
+        }
+
+        public static Guid GetTenantId(ILoggerService loggerService, TokenCredential credential)
+        {
+            ArmClient armClient = new(credential);
+            IEnumerable<Guid> tenants = armClient.GetTenants().ToList()
+                .Select(tenantResource => tenantResource.Data.TenantId)
+                .Where(guid => guid != null)
+                .Select(guid => (Guid)guid);
+
+            if (!tenants.Any())
+            {
+                throw new Exception("Found no tenants for given credential.");
+            }
+
+            if (tenants.Count() > 1)
+            {
+                string allTenantIds = string.Join(' ', tenants.Select(guid => guid.ToString()));
+                loggerService.WriteMessage("Found more than one tenant. Selecting the first one.");
+                loggerService.WriteMessage($"Tenants: {allTenantIds}");
+            }
+
+            return tenants.First();
         }
     }
 }

src/Microsoft.DotNet.ImageBuilder/src/Commands/BuildCommand.cs

@@ -25,6 +25,7 @@ public class BuildCommand : DockerRegistryCommand<BuildOptions, BuildOptionsBuil
         private readonly IGitService _gitService;
         private readonly IProcessService _processService;
         private readonly ICopyImageService _copyImageService;
+        private readonly Lazy<IManifestService> _manifestService;
         private readonly ImageDigestCache _imageDigestCache;
         private readonly List<TagInfo> _processedTags = new List<TagInfo>();
         private readonly HashSet<PlatformData> _builtPlatforms = new();
@@ -41,15 +42,27 @@ public class BuildCommand : DockerRegistryCommand<BuildOptions, BuildOptionsBuil
         private ImageArtifactDetails? _imageArtifactDetails;
 
         [ImportingConstructor]
-        public BuildCommand(IDockerService dockerService, ILoggerService loggerService, IGitService gitService,
-            IProcessService processService, ICopyImageService copyImageService)
+        public BuildCommand(
+            IDockerService dockerService,
+            ILoggerService loggerService,
+            IGitService gitService,
+            IProcessService processService,
+            ICopyImageService copyImageService,
+            IManifestServiceFactory manifestServiceFactory,
+            IRegistryCredentialsProvider registryCredentialsProvider)
+            : base(registryCredentialsProvider)
         {
-            _imageDigestCache = new ImageDigestCache(dockerService);
             _dockerService = new DockerServiceCache(dockerService ?? throw new ArgumentNullException(nameof(dockerService)));
             _loggerService = loggerService ?? throw new ArgumentNullException(nameof(loggerService));
             _gitService = gitService ?? throw new ArgumentNullException(nameof(gitService));
             _processService = processService ?? throw new ArgumentNullException(nameof(processService));
             _copyImageService = copyImageService ?? throw new ArgumentNullException(nameof(copyImageService));
+
+            // Lazily create the Manifest Service so it can have access to options (not available in this constructor)
+            ArgumentNullException.ThrowIfNull(manifestServiceFactory);
+            _manifestService = new Lazy<IManifestService>(() =>
+                manifestServiceFactory.Create(ownedAcr: Options.RegistryOverride, Options.CredentialsOptions));
+            _imageDigestCache = new ImageDigestCache(_manifestService);
         }
 
         protected override string Description => "Builds Dockerfiles";
@@ -63,19 +76,23 @@ public override async Task ExecuteAsync()
                 _imageArtifactDetails = new ImageArtifactDetails();
             }
 
-            await ExecuteWithUserAsync(async () =>
-            {
-                await PullBaseImagesAsync();
+            await ExecuteWithCredentialsAsync(
+                Options.IsDryRun,
+                async () =>
+                {
+                    await PullBaseImagesAsync();
 
-                await BuildImagesAsync();
+                    await BuildImagesAsync();
 
-                if (_processedTags.Any() || _cachedPlatforms.Any())
-                {
-                    PushImages();
-                }
+                    if (_processedTags.Any() || _cachedPlatforms.Any())
+                    {
+                        PushImages();
+                    }
 
-                await PublishImageInfoAsync();
-            });
+                    await PublishImageInfoAsync();
+                },
+                registryName: Manifest.Registry,
+                ownedAcr: Options.RegistryOverride);
 
             WriteBuildSummary();
             WriteBuiltImagesToOutputVar();
@@ -215,14 +232,14 @@ private async Task SetPlatformDataLayersAsync(PlatformData platform, string tag)
         {
             if (platform.Layers == null || !platform.Layers.Any())
             {
-                platform.Layers = (await _dockerService.GetImageManifestLayersAsync(tag, Options.CredentialsOptions, Options.IsDryRun)).ToList();
+                platform.Layers = (await _manifestService.Value.GetImageLayersAsync(tag, Options.IsDryRun)).ToList();
             }
         }
 
         private async Task SetPlatformDataDigestAsync(PlatformData platform, string tag)
         {
             // The digest of an image that is pushed to ACR is guaranteed to be the same when transferred to MCR.
-            string? digest = await _imageDigestCache.GetImageDigestAsync(tag, Options.CredentialsOptions, Options.IsDryRun);
+            string? digest = await _imageDigestCache.GetImageDigestAsync(tag, Options.IsDryRun);
             if (digest is not null && platform.PlatformInfo is not null)
             {
                 digest = DockerHelper.GetDigestString(platform.PlatformInfo.FullRepoModelName, DockerHelper.GetDigestSha(digest));
@@ -302,7 +319,7 @@ private async Task BuildImagesAsync()
                             {
                                 platformData.BaseImageDigest =
                                    await _imageDigestCache.GetImageDigestAsync(
-                                       GetFromImageLocalTag(platform.FinalStageFromImage), Options.CredentialsOptions, Options.IsDryRun);
+                                       GetFromImageLocalTag(platform.FinalStageFromImage), Options.IsDryRun);
                             }
                         }
                     }
@@ -624,7 +641,8 @@ private async Task<bool> IsBaseImageDigestUpToDateAsync(PlatformInfo platform, P
             }
 
             string? currentBaseImageDigest = await _imageDigestCache.GetImageDigestAsync(
-                GetFromImageLocalTag(platform.FinalStageFromImage), Options.CredentialsOptions, Options.IsDryRun);
+                GetFromImageLocalTag(platform.FinalStageFromImage),
+                Options.IsDryRun);
 
             string? baseSha = srcPlatformData.BaseImageDigest is not null ? DockerHelper.GetDigestSha(srcPlatformData.BaseImageDigest) : null;
             string? currentSha = currentBaseImageDigest is not null ? DockerHelper.GetDigestSha(currentBaseImageDigest) : null;
@@ -789,7 +807,7 @@ await Parallel.ForEachAsync(finalStageExternalFromImages, async (fromImage, canc
                         // the DockerServiceCache for later use.  The longer we wait to get the digest after pulling, the
                         // greater chance the tag could be updated resulting in a different digest returned than what was
                         // originally pulled.
-                        await _imageDigestCache.GetImageDigestAsync(fromImage, Options.CredentialsOptions, Options.IsDryRun);
+                        await _imageDigestCache.GetImageDigestAsync(fromImage, Options.IsDryRun);
                     });
 
                     // Tag the images that were pulled from the mirror as they are referenced in the Dockerfiles

src/Microsoft.DotNet.ImageBuilder/src/Commands/BuildOptions.cs

@@ -26,7 +26,6 @@ public class BuildOptions : DockerRegistryOptions, IFilterableOptions
         public IDictionary<string, string> BuildArgs { get; set; } = new Dictionary<string, string>();
         public bool SkipPlatformCheck { get; set; }
         public string? OutputVariableName { get; set; }
-        public ServicePrincipalOptions ServicePrincipal { get; set; } = new();
         public string? Subscription { get; set; }
         public string? ResourceGroup { get; set; }
     }
@@ -35,17 +34,11 @@ public class BuildOptionsBuilder : DockerRegistryOptionsBuilder
     {
         private readonly ManifestFilterOptionsBuilder _manifestFilterOptionsBuilder = new();
         private readonly BaseImageOverrideOptionsBuilder _baseImageOverrideOptionsBuilder = new();
-        private readonly ServicePrincipalOptionsBuilder _servicePrincipalOptionsBuilder =
-            ServicePrincipalOptionsBuilder.Build()
-                .WithClientId("acr-client-id", description: "ACR service principal client ID")
-                .WithSecret("acr-password", description: "ACR service principal's password")
-                .WithTenant("acr-tenant", description: "ACR service principal's tenant");
 
         public override IEnumerable<Option> GetCliOptions() =>
             base.GetCliOptions()
                 .Concat(_manifestFilterOptionsBuilder.GetCliOptions())
                 .Concat(_baseImageOverrideOptionsBuilder.GetCliOptions())
-                .Concat(_servicePrincipalOptionsBuilder.GetCliOptions())
                 .Concat(
                     new Option[]
                     {
@@ -80,8 +73,7 @@ public override IEnumerable<Option> GetCliOptions() =>
         public override IEnumerable<Argument> GetCliArguments() =>
             base.GetCliArguments()
                 .Concat(_manifestFilterOptionsBuilder.GetCliArguments())
-                .Concat(_baseImageOverrideOptionsBuilder.GetCliArguments())
-                .Concat(_servicePrincipalOptionsBuilder.GetCliArguments());
+                .Concat(_baseImageOverrideOptionsBuilder.GetCliArguments());
     }
 }
 #nullable disable

src/Microsoft.DotNet.ImageBuilder/src/Commands/DockerRegistryCommand.cs

@@ -12,20 +12,39 @@ public abstract class DockerRegistryCommand<TOptions, TOptionsBuilder> : Manifes
         where TOptions : DockerRegistryOptions, new()
         where TOptionsBuilder : DockerRegistryOptionsBuilder, new()
     {
-        protected void ExecuteWithUser(Action action)
+        protected IRegistryCredentialsProvider RegistryCredentialsProvider { get; init; }
+
+        protected DockerRegistryCommand(IRegistryCredentialsProvider registryCredentialsProvider)
         {
-            ExecuteWithUserAsync(() =>
-            {
-                action();
-                return Task.CompletedTask;
-            });
+            RegistryCredentialsProvider = registryCredentialsProvider;
         }
 
-        protected Task ExecuteWithUserAsync(Func<Task> action)
+        protected async Task ExecuteWithCredentialsAsync(bool isDryRun, Func<Task> action, string registryName, string? ownedAcr)
         {
-            Options.CredentialsOptions.Credentials.TryGetValue(Manifest.Registry ?? "", out RegistryCredentials? credentials);
-            return DockerHelper.ExecuteWithUserAsync(action, credentials?.Username, credentials?.Password, Manifest.Registry, Options.IsDryRun);
+            bool loggedIn = false;
+
+            RegistryCredentials? credentials = await RegistryCredentialsProvider.GetCredentialsAsync(
+                    registryName, ownedAcr, Options.CredentialsOptions);
+
+            if (!string.IsNullOrEmpty(registryName) && credentials is not null)
+            {
+                DockerHelper.Login(credentials, registryName, isDryRun);
+                loggedIn = true;
+            }
+
+            try
+            {
+                await action();
+            }
+            finally
+            {
+                if (loggedIn && !string.IsNullOrEmpty(registryName))
+                {
+                    DockerHelper.Logout(registryName, isDryRun);
+                }
+            }
         }
+
     }
 }
 #nullable disable

src/Microsoft.DotNet.ImageBuilder/src/Commands/GetStaleImagesCommand.cs

@@ -21,22 +21,24 @@ public class GetStaleImagesCommand : Command<GetStaleImagesOptions, GetStaleImag
     {
         private readonly Dictionary<string, string> _imageDigests = new();
         private readonly SemaphoreSlim _imageDigestsLock = new(1);
-        private readonly IManifestService _manifestToolService;
+        private readonly IManifestService _manifestService;
         private readonly ILoggerService _loggerService;
         private readonly IOctokitClientFactory _octokitClientFactory;
         private readonly IGitService _gitService;
 
         [ImportingConstructor]
         public GetStaleImagesCommand(
-            IManifestService manifestToolService,
+            IManifestServiceFactory manifestServiceFactory,
             ILoggerService loggerService,
             IOctokitClientFactory octokitClientFactory,
             IGitService gitService)
         {
-            _manifestToolService = manifestToolService;
             _loggerService = loggerService;
             _octokitClientFactory = octokitClientFactory;
             _gitService = gitService;
+
+            ArgumentNullException.ThrowIfNull(manifestServiceFactory);
+            _manifestService = manifestServiceFactory.Create();
         }
 
         protected override string Description => "Gets paths to images whose base images are out-of-date";
@@ -147,7 +149,7 @@ void processPlatformWithMissingImageInfo(PlatformInfo platform)
                     string currentDigest = await LockHelper.DoubleCheckedLockLookupAsync(_imageDigestsLock, _imageDigests, fromImage,
                         async () =>
                         {
-                            string digest = await _manifestToolService.GetManifestDigestShaAsync(fromImage, Options.CredentialsOptions, Options.IsDryRun);
+                            string digest = await _manifestService.GetManifestDigestShaAsync(fromImage, Options.IsDryRun);
                             return DockerHelper.GetDigestString(DockerHelper.GetRepo(fromImage), digest);
                         });
 

src/Microsoft.DotNet.ImageBuilder/src/Commands/GetStaleImagesOptions.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.CommandLine;
 using System.Linq;
+using static Microsoft.DotNet.ImageBuilder.Commands.CliHelper;
 
 #nullable enable
 namespace Microsoft.DotNet.ImageBuilder.Commands
@@ -19,40 +20,34 @@ public class GetStaleImagesOptions : Options, IFilterableOptions, IGitOptionsHos
 
         public string VariableName { get; set; } = string.Empty;
 
-        public RegistryCredentialsOptions CredentialsOptions { get; set; } = new RegistryCredentialsOptions();
-
         public BaseImageOverrideOptions BaseImageOverrideOptions { get; set; } = new();
-
     }
 
     public class GetStaleImagesOptionsBuilder : CliOptionsBuilder
     {
         private readonly GitOptionsBuilder _gitOptionsBuilder = GitOptionsBuilder.BuildWithDefaults();
         private readonly ManifestFilterOptionsBuilder _manifestFilterOptionsBuilder = new();
         private readonly SubscriptionOptionsBuilder _subscriptionOptionsBuilder = new();
-        private readonly RegistryCredentialsOptionsBuilder _registryCredentialsOptionsBuilder = new();
         private readonly BaseImageOverrideOptionsBuilder _baseImageOverrideOptionsBuilder = new();
 
         public override IEnumerable<Option> GetCliOptions() =>
             base.GetCliOptions()
                 .Concat(_subscriptionOptionsBuilder.GetCliOptions())
                 .Concat(_manifestFilterOptionsBuilder.GetCliOptions())
                 .Concat(_gitOptionsBuilder.GetCliOptions())
-                .Concat(_registryCredentialsOptionsBuilder.GetCliOptions())
                 .Concat(_baseImageOverrideOptionsBuilder.GetCliOptions());
 
         public override IEnumerable<Argument> GetCliArguments() =>
             base.GetCliArguments()
                 .Concat(_subscriptionOptionsBuilder.GetCliArguments())
                 .Concat(_manifestFilterOptionsBuilder.GetCliArguments())
                 .Concat(_gitOptionsBuilder.GetCliArguments())
-                .Concat(_registryCredentialsOptionsBuilder.GetCliArguments()
                 .Concat(
                     new Argument[]
                     {
                         new Argument<string>(nameof(GetStaleImagesOptions.VariableName),
                             "The Azure Pipeline variable name to assign the image paths to")
-                    }));
+                    });
     }
 }
 #nullable disable