From c367c70f59d677d485d57ae8b9fd8a9a8a08f1e2 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Fri, 4 Sep 2026 12:00:37 -0700 Subject: [PATCH 1/2] Use docker secret mount support for Linux/BuildKit builds --- src/ImageBuilder/BuildSecretMode.cs | 10 +++ src/ImageBuilder/Commands/BuildCommand.cs | 30 ++++----- src/ImageBuilder/DockerService.cs | 78 +++++++++++++++++++---- src/ImageBuilder/DockerServiceCache.cs | 28 ++++++-- src/ImageBuilder/IDockerService.cs | 2 + 5 files changed, 115 insertions(+), 33 deletions(-) create mode 100644 src/ImageBuilder/BuildSecretMode.cs diff --git a/src/ImageBuilder/BuildSecretMode.cs b/src/ImageBuilder/BuildSecretMode.cs new file mode 100644 index 000000000..3a60379f4 --- /dev/null +++ b/src/ImageBuilder/BuildSecretMode.cs @@ -0,0 +1,10 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.DotNet.ImageBuilder; + +public enum BuildSecretMode +{ + SecretMounts, + BuildArgs, +} diff --git a/src/ImageBuilder/Commands/BuildCommand.cs b/src/ImageBuilder/Commands/BuildCommand.cs index 5ae85c30c..002a0ccc6 100644 --- a/src/ImageBuilder/Commands/BuildCommand.cs +++ b/src/ImageBuilder/Commands/BuildCommand.cs @@ -30,7 +30,7 @@ public class BuildCommand : ManifestCommand private readonly List _processedTags = new List(); private readonly HashSet _builtPlatforms = new(); private readonly Lazy _imageNameResolver; - private readonly Lazy _storageAccountToken; + private readonly Lazy _storageAccountToken; /// /// Maps a source digest from the image info file to the corresponding digest in the copied location for image caching. @@ -74,13 +74,8 @@ public BuildCommand( Options.RepoPrefix, Options.SourceRepoPrefix)); - _storageAccountToken = new Lazy(() => + _storageAccountToken = new Lazy(() => { - if (!Options.Internal) - { - return null; - } - var tokenObject = _tokenCredentialProvider.GetToken( Options.StorageServiceConnection, AzureScopes.StorageAccount); @@ -487,12 +482,18 @@ private void BuildImage(PlatformInfo platform, IEnumerable allTags) try { + BuildSecretMode buildSecretMode = platform.IsWindows + ? BuildSecretMode.BuildArgs + : BuildSecretMode.SecretMounts; + string? buildOutput = _dockerService.BuildImage( dockerfilePath, platform.BuildContextPath, platform.PlatformLabel, allTags, GetBuildArgs(platform), + GetBuildSecrets(), + buildSecretMode, GetDockerBuildOptions(), Options.IsRetryEnabled, Options.IsDryRun); @@ -523,10 +524,7 @@ private void BuildImage(PlatformInfo platform, IEnumerable allTags) } /// - /// Gets all the necessary Docker build-args for the specified platform. When building internal images, this - /// also includes the access token for the storage account service connection's access token. This refers to - /// arguments passed via the --build-arg option to the docker build command, not the arguments - /// passed directly to docker build. + /// Gets all the necessary Docker build arguments for the specified platform. /// /// /// Platform build args (from the manifest) take precedence over any build args specified via the command line. @@ -537,11 +535,6 @@ private void BuildImage(PlatformInfo platform, IEnumerable allTags) { Dictionary buildArgs = []; - if (Options.Internal) - { - buildArgs["ACCESSTOKEN"] = _storageAccountToken.Value; - } - foreach (var kvp in Options.BuildArgs) { buildArgs[kvp.Key] = kvp.Value; @@ -555,6 +548,11 @@ private void BuildImage(PlatformInfo platform, IEnumerable allTags) return buildArgs; } + private IReadOnlyDictionary GetBuildSecrets() => + Options.Internal + ? new Dictionary { ["ACCESSTOKEN"] = _storageAccountToken.Value } + : new Dictionary(); + private IEnumerable GetDockerBuildOptions() => Options.DockerBuildOptions.Where(option => !string.IsNullOrWhiteSpace(option)); diff --git a/src/ImageBuilder/DockerService.cs b/src/ImageBuilder/DockerService.cs index d24dbe9a3..df4142043 100644 --- a/src/ImageBuilder/DockerService.cs +++ b/src/ImageBuilder/DockerService.cs @@ -3,8 +3,8 @@ // See the LICENSE file in the project root for more information. using System; -using System.IO; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using Microsoft.DotNet.ImageBuilder.Models.Manifest; @@ -12,6 +12,8 @@ namespace Microsoft.DotNet.ImageBuilder { public class DockerService : IDockerService { + private const string BuildSecretEnvironmentVariablePrefix = "IMAGEBUILDER_BUILD_SECRET_"; + public Architecture Architecture => DockerHelper.Architecture; public void PullImage(string image, string? platform, bool isDryRun) => DockerHelper.PullImage(image, platform, isDryRun); @@ -34,33 +36,85 @@ public void CreateManifestList(string manifestListTag, IEnumerable image string platform, IEnumerable tags, IDictionary buildArgs, + IReadOnlyDictionary buildSecrets, + BuildSecretMode buildSecretMode, IEnumerable dockerBuildOptions, bool isRetryEnabled, bool isDryRun) { - string tagArgs = $"-t {string.Join(" -t ", tags)}"; + List dockerArgs = ["build", "--platform", platform]; + ProcessStartInfo processStartInfo = new("docker"); - IEnumerable buildArgList = buildArgs - .Select(buildArg => $" --build-arg {buildArg.Key}={buildArg.Value}"); - string buildArgsString = string.Join(string.Empty, buildArgList); + foreach (string tag in tags) + { + dockerArgs.Add("-t"); + dockerArgs.Add(tag); + } - IEnumerable dockerBuildOptionList = dockerBuildOptions - .Where(option => !string.IsNullOrWhiteSpace(option)) - .Select(option => $" {option}"); - string dockerBuildOptionsString = string.Join(string.Empty, dockerBuildOptionList); + dockerArgs.Add("-f"); + dockerArgs.Add(dockerfilePath); - string dockerArgs = $"build --platform {platform} {tagArgs} -f {dockerfilePath}{buildArgsString}{dockerBuildOptionsString} {buildContextPath}"; + List buildSecretArgs = buildSecretMode switch + { + BuildSecretMode.SecretMounts => GetSecretMountArgs(processStartInfo, buildSecrets), + BuildSecretMode.BuildArgs => GetSecretBuildArgs(buildSecrets), + _ => throw new ArgumentOutOfRangeException(nameof(buildSecretMode), buildSecretMode, null), + }; + dockerArgs.AddRange(buildSecretArgs); + + foreach (KeyValuePair buildArg in buildArgs) + { + dockerArgs.Add("--build-arg"); + dockerArgs.Add($"{buildArg.Key}={buildArg.Value}"); + } + + dockerBuildOptions = dockerBuildOptions.Where(option => !string.IsNullOrWhiteSpace(option)); + dockerArgs.AddRange(dockerBuildOptions); + dockerArgs.Add(buildContextPath); + + processStartInfo.Arguments = string.Join(' ', dockerArgs); if (isRetryEnabled) { - return ExecuteHelper.ExecuteWithRetry("docker", dockerArgs, isDryRun); + return ExecuteHelper.ExecuteWithRetry(processStartInfo, isDryRun: isDryRun); } else { - return ExecuteHelper.Execute("docker", dockerArgs, isDryRun); + return ExecuteHelper.Execute(processStartInfo, isDryRun); } } + private static List GetSecretMountArgs( + ProcessStartInfo processStartInfo, + IReadOnlyDictionary buildSecrets) + { + List buildSecretArgs = []; + int secretNumber = 0; + foreach (KeyValuePair buildSecret in buildSecrets) + { + // https://docs.docker.com/build/building/secrets/ + string environmentVariableName = $"{BuildSecretEnvironmentVariablePrefix}{secretNumber}"; + buildSecretArgs.Add("--secret"); + buildSecretArgs.Add($"id={buildSecret.Key},env={environmentVariableName}"); + processStartInfo.Environment[environmentVariableName] = buildSecret.Value; + secretNumber++; + } + + return buildSecretArgs; + } + + private static List GetSecretBuildArgs(IReadOnlyDictionary buildSecrets) + { + List buildSecretArgs = []; + foreach (KeyValuePair buildSecret in buildSecrets) + { + buildSecretArgs.Add("--build-arg"); + buildSecretArgs.Add($"{buildSecret.Key}={buildSecret.Value}"); + } + + return buildSecretArgs; + } + public (Architecture Arch, string? Variant) GetImageArch(string image, bool isDryRun) { string archAndVariant = DockerHelper.ExecuteCommand( diff --git a/src/ImageBuilder/DockerServiceCache.cs b/src/ImageBuilder/DockerServiceCache.cs index 7878c025b..bd4604421 100644 --- a/src/ImageBuilder/DockerServiceCache.cs +++ b/src/ImageBuilder/DockerServiceCache.cs @@ -31,9 +31,27 @@ public DockerServiceCache(IDockerService inner) public Architecture Architecture => _inner.Architecture; public string? BuildImage( - string dockerfilePath, string buildContextPath, string platform, IEnumerable tags, - IDictionary buildArgs, IEnumerable dockerBuildOptions, bool isRetryEnabled, bool isDryRun) => - _inner.BuildImage(dockerfilePath, buildContextPath, platform, tags, buildArgs, dockerBuildOptions, isRetryEnabled, isDryRun); + string dockerfilePath, + string buildContextPath, + string platform, + IEnumerable tags, + IDictionary buildArgs, + IReadOnlyDictionary buildSecrets, + BuildSecretMode buildSecretMode, + IEnumerable dockerBuildOptions, + bool isRetryEnabled, + bool isDryRun) => + _inner.BuildImage( + dockerfilePath, + buildContextPath, + platform, + tags, + buildArgs, + buildSecrets, + buildSecretMode, + dockerBuildOptions, + isRetryEnabled, + isDryRun); public (Architecture Arch, string? Variant) GetImageArch(string image, bool isDryRun) => _architectureCache.GetOrAdd(image, _ =>_inner.GetImageArch(image, isDryRun)); @@ -49,10 +67,10 @@ public DateTime GetCreatedDate(string image, bool isDryRun) => public long GetImageSize(string image, bool isDryRun) => _imageSizeCache.GetOrAdd(image, _ => _inner.GetImageSize(image, isDryRun)); - + public bool LocalImageExists(string tag, bool isDryRun) => _localImageExistsCache.GetOrAdd(tag, _ => _inner.LocalImageExists(tag, isDryRun)); - + public void PullImage(string image, string? platform, bool isDryRun) { _pulledImages.GetOrAdd(image, _ => diff --git a/src/ImageBuilder/IDockerService.cs b/src/ImageBuilder/IDockerService.cs index 8eb379f71..b75fbf4a8 100644 --- a/src/ImageBuilder/IDockerService.cs +++ b/src/ImageBuilder/IDockerService.cs @@ -29,6 +29,8 @@ public interface IDockerService string platform, IEnumerable tags, IDictionary buildArgs, + IReadOnlyDictionary buildSecrets, + BuildSecretMode buildSecretMode, IEnumerable dockerBuildOptions, bool isRetryEnabled, bool isDryRun); From 98f0b35d0623ee033108c77258dc762a326f6643 Mon Sep 17 00:00:00 2001 From: Logan Bussell Date: Fri, 11 Sep 2026 15:56:59 -0700 Subject: [PATCH 2/2] Fix build secret test mocks and coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/ImageBuilder.Tests/BuildCommandTests.cs | 68 +++++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/src/ImageBuilder.Tests/BuildCommandTests.cs b/src/ImageBuilder.Tests/BuildCommandTests.cs index c34c17119..31ef71d05 100644 --- a/src/ImageBuilder.Tests/BuildCommandTests.cs +++ b/src/ImageBuilder.Tests/BuildCommandTests.cs @@ -7,7 +7,9 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; +using Azure.Core; using Azure.ResourceManager.ContainerRegistry.Models; using FluentAssertions; using Microsoft.DotNet.ImageBuilder.Commands; @@ -521,6 +523,8 @@ public async Task BuildCommand_Publish() TagInfo.GetFullyQualifiedName(repoName, sharedTag) }, It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -637,31 +641,51 @@ public async Task BuildCommand_ArmVariantCompatibility(string manifestVariant, s } /// - /// Verifies that manifest-defined and globally-defined build args can be used. + /// Verifies build argument precedence and platform-specific handling of internal build secrets. /// [TestMethod] - public async Task BuildCommand_BuildArgs() + [DataRow(OS.Linux, false, BuildSecretMode.SecretMounts)] + [DataRow(OS.Linux, true, BuildSecretMode.SecretMounts)] + [DataRow(OS.Windows, false, BuildSecretMode.BuildArgs)] + [DataRow(OS.Windows, true, BuildSecretMode.BuildArgs)] + public async Task BuildCommand_BuildArgs(OS os, bool isInternal, BuildSecretMode expectedSecretMode) { const string repoName = "runtime"; const string tag = "tag"; const string baseImageRepo = "baserepo"; string baseImageTag = $"{baseImageRepo}:basetag"; + const string accessToken = "test-storage-token"; + const string storageScope = "https://storage.azure.com/.default"; using TempFolderContext tempFolderContext = TestHelper.UseTempFolder(); Mock dockerServiceMock = CreateDockerServiceMock(); + Mock credentialMock = new(); + credentialMock + .Setup(credential => credential.GetToken( + It.Is(context => context.Scopes.SequenceEqual(new[] { storageScope })), + It.IsAny())) + .Returns(new AccessToken(accessToken, DateTimeOffset.UtcNow.AddHours(1))); + Mock credentialProviderMock = new(); + credentialProviderMock + .Setup(provider => provider.GetCredential(It.IsAny())) + .Returns(credentialMock.Object); BuildCommand command = CreateBuildCommand( dockerService: dockerServiceMock.Object, copyImageService: Mock.Of(), manifestServiceFactory: CreateManifestServiceFactoryMock().Object, + azureTokenCredentialProvider: credentialProviderMock.Object, imageCacheService: new ImageCacheService(Mock.Of>(), Mock.Of())); command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); + command.Options.Internal = isInternal; command.Options.BuildArgs.Add("arg1", "val1"); command.Options.BuildArgs.Add("arg2", "val2a"); Platform platform = CreatePlatform( DockerfileHelper.CreateDockerfile("1.0/runtime/os", tempFolderContext, baseImageTag), - new string[] { tag }); + new string[] { tag }, + os: os, + osVersion: os == OS.Windows ? "nanoserver-ltsc2022" : "noble"); platform.BuildArgs.Add("arg2", "val2b"); platform.BuildArgs.Add("arg3", "val3"); @@ -687,11 +711,23 @@ public async Task BuildCommand_BuildArgs() It.IsAny>(), It.Is>( args => args.Count == 3 && args["arg1"] == "val1" && args["arg2"] == "val2b" && args["arg3"] == "val3"), + It.Is>(secrets => isInternal + ? secrets.Count == 1 && secrets["ACCESSTOKEN"] == accessToken + : secrets.Count == 0), + expectedSecretMode, It.IsAny>(), It.IsAny(), It.IsAny())); dockerServiceMock.Verify( o => o.GetImageSize(It.IsAny(), false)); + credentialProviderMock.Verify( + provider => provider.GetCredential(command.Options.StorageServiceConnection), + isInternal ? Times.Once() : Times.Never()); + credentialMock.Verify( + credential => credential.GetToken( + It.Is(context => context.Scopes.SequenceEqual(new[] { storageScope })), + It.IsAny()), + isInternal ? Times.Once() : Times.Never()); } /// @@ -741,6 +777,8 @@ public async Task BuildCommand_DockerBuildOptions() It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.Is>(args => args.SequenceEqual(command.Options.DockerBuildOptions)), It.IsAny(), It.IsAny())); @@ -805,6 +843,8 @@ public async Task BuildCommand_NoBaseImage_Build() TagInfo.GetFullyQualifiedName(repoName, sharedTag) }, It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -998,6 +1038,7 @@ public async Task BuildCommand_NoBaseImage_Cached() o.BuildImage( PathHelper.NormalizePath(Path.Combine(tempFolderContext.Path, runtimeDepsLinuxDockerfileRelativePath)), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny()), Times.Never); dockerServiceMock.Verify( @@ -1726,7 +1767,8 @@ public async Task BuildCommand_Caching_SharedDockerfile_MissingSourceImageInfoEn dockerServiceMock.Verify(o => o.BuildImage( It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), - It.IsAny>(), It.IsAny>(), It.IsAny(), It.IsAny()), + It.IsAny>(), It.IsAny>(), It.IsAny(), + It.IsAny>(), It.IsAny(), It.IsAny()), Times.Never); dockerServiceMock.VerifyNoOtherCalls(); @@ -2034,6 +2076,8 @@ public async Task BuildCommand_Caching_SharedDockerfile_MissingSourceImageInfoEn It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -2241,6 +2285,8 @@ public async Task BuildCommand_Caching_SharedDockerfile_NoExistingImageInfoEntri It.IsAny(), new string[] { expectedTag }, It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny()), @@ -2485,6 +2531,8 @@ public async Task BuildCommand_SharedDockerfile() It.IsAny(), new string[] { expectedTag }, It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny()), @@ -2703,6 +2751,7 @@ public async Task BuildCommand_Caching_TagUpdate() o.BuildImage( PathHelper.NormalizePath(Path.Combine(tempFolderContext.Path, runtimeDepsLinuxDockerfileRelativePath)), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny()), Times.Never); dockerServiceMock.Verify( @@ -2962,7 +3011,8 @@ public async Task BuildCommand_Caching_SharedDockerfile_TagUpdate() dockerServiceMock.Verify(o => o.BuildImage( It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), - It.IsAny>(), It.IsAny>(), It.IsAny(), It.IsAny()), + It.IsAny>(), It.IsAny>(), It.IsAny(), + It.IsAny>(), It.IsAny(), It.IsAny()), Times.Never); dockerServiceMock.Verify(o => o.GetCreatedDate(It.IsAny(), false)); @@ -3322,6 +3372,8 @@ public async Task BuildCommand_MirroredImages(bool hasCachedImage, string srcBas It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -3468,6 +3520,8 @@ public async Task BuildCommand_MirroredImages_External(string baseImageRegistry, It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -3610,6 +3664,8 @@ public async Task BuildCommand_MirroredImages_BaseImageTagOverride() It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny())); @@ -3674,6 +3730,8 @@ private static Mock CreateDockerServiceMock(string buildOutput = It.IsAny(), It.IsAny>(), It.IsAny>(), + It.IsAny>(), + It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny()))