From 50d348db96960cbfcd79e869fb7d3b8a9e62e5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Thu, 27 Jun 2024 19:27:13 +0200 Subject: [PATCH 01/12] Restore and re-apply all VMR patches always (#3690) --- .../DarcLib/LocalGitClient.cs | 21 +- .../VirtualMonoRepo/IVmrPatchHandler.cs | 10 +- .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 27 +- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 112 ++--- .../VirtualMonoRepo/VmrPatchHandler.cs | 80 +++- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 383 +++++++----------- .../VmrPatchAddedTest.cs | 158 ++++++++ .../VmrSyncRepoChangesTest.cs | 12 +- 8 files changed, 469 insertions(+), 334 deletions(-) create mode 100644 test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index b8473702b7..3d3372b666 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -391,15 +391,17 @@ public async Task GetStagedFiles(string repoPath) { var args = new List { - "show", - $"{revision}:{relativeFilePath.TrimStart('/')}" + "show" }; - if (outputPath != null) - { - args.Add("--output"); - args.Add(outputPath); - } + // This somehow stopped working for me + //if (outputPath != null) + //{ + // args.Add("--output"); + // args.Add(outputPath); + //} + + args.Add($"{revision}:{relativeFilePath.TrimStart('/')}"); var result = await _processManager.ExecuteGit(repoPath, args); @@ -408,6 +410,11 @@ public async Task GetStagedFiles(string repoPath) return null; } + if (outputPath != null) + { + _fileSystem.WriteToFile(outputPath, result.StandardOutput); + } + return result.StandardOutput; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs index c0761748f1..6724afc363 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs @@ -39,9 +39,11 @@ Task> CreatePatches( UnixPath? applicationPath, CancellationToken cancellationToken); - IReadOnlyCollection GetVmrPatches(SourceMapping mapping) => GetVmrPatches(mapping.Name); - - IReadOnlyCollection GetVmrPatches(string mappingName); + Task> GetVmrPatches( + string? patchVersion, + CancellationToken cancellationToken); - Task> GetPatchedFiles(string patchPath, CancellationToken cancellationToken); + Task> GetPatchedFiles( + string patchPath, + CancellationToken cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index ef177c9cb3..c44408df65 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -114,8 +113,8 @@ public async Task InitializeRepository( try { IEnumerable updates = initializeDependencies - ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken) - : new[] { rootUpdate }; + ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken) + : new[] { rootUpdate }; foreach (var update in updates) { @@ -195,34 +194,20 @@ private async Task InitializeRepository( string commitMessage = PrepareCommitMessage(InitializationCommitMessage, update.Mapping.Name, update.RemoteUri, newSha: update.TargetRevision); - await UpdateRepoToRevisionAsync( + await StageRepositoryUpdatesAsync( update, clone, additionalRemotes, Constants.EmptyGitObject, - author: null, - commitMessage, - reapplyVmrPatches: true, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); - _logger.LogInformation("Initialization of {name} finished", update.Mapping.Name); - } + await ReapplyVmrPatchesAsync(update.Mapping, cancellationToken); + await CommitAsync(commitMessage); - protected override Task> RestoreVmrPatchedFilesAsync( - SourceMapping mapping, - IReadOnlyCollection patches, - IReadOnlyCollection additionalRemotes, - CancellationToken cancellationToken) - { - // We only need to apply VMR patches that belong to the mapping, nothing to restore from before - IReadOnlyCollection vmrPatchesForMapping = _patchHandler.GetVmrPatches(mapping) - .Select(patch => new VmrIngestionPatch(patch, VmrInfo.GetRelativeRepoSourcesPath(mapping))) - .ToImmutableArray(); - - return Task.FromResult(vmrPatchesForMapping); + _logger.LogInformation("Initialization of {name} finished", update.Mapping.Name); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index cbdfc0104c..53ff5b77ce 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -68,14 +68,11 @@ protected VmrManagerBase( LocalVmr = localGitRepoFactory.Create(_vmrInfo.VmrPath); } - public async Task> UpdateRepoToRevisionAsync( + protected async Task StageRepositoryUpdatesAsync( VmrDependencyUpdate update, ILocalGitRepo repoClone, IReadOnlyCollection additionalRemotes, string fromRevision, - (string Name, string Email)? author, - string commitMessage, - bool reapplyVmrPatches, string? componentTemplatePath, string? tpnTemplatePath, bool generateCodeowners, @@ -92,11 +89,6 @@ public async Task> UpdateRepoToRevisionAs cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - // Get a list of patches that need to be reverted for this update so that repo changes can be applied - // This includes all patches that are also modified by the current change - // (happens when we update repo from which the VMR patches come) - var vmrPatchesToRestore = await RestoreVmrPatchedFilesAsync(update.Mapping, patches, additionalRemotes, cancellationToken); - foreach (var patch in patches) { await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, discardPatches, reverseApply: false, cancellationToken); @@ -125,12 +117,6 @@ public async Task> UpdateRepoToRevisionAs cancellationToken.ThrowIfCancellationRequested(); - if (reapplyVmrPatches) - { - await ReapplyVmrPatchesAsync(vmrPatchesToRestore.DistinctBy(p => p.Path).ToArray(), cancellationToken); - cancellationToken.ThrowIfCancellationRequested(); - } - if (tpnTemplatePath != null) { await UpdateThirdPartyNoticesAsync(tpnTemplatePath, cancellationToken); @@ -140,46 +126,71 @@ public async Task> UpdateRepoToRevisionAs { await _codeownersGenerator.UpdateCodeowners(cancellationToken); } + } - // Commit without adding files as they were added to index directly - await CommitAsync(commitMessage, author); + protected async Task ReapplyVmrPatchesAsync(CancellationToken cancellationToken) + => await ReapplyVmrPatchesAsync(null, cancellationToken); - // TODO: Workaround for cases when we get CRLF problems on Windows - // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes - // https://github.com/dotnet/arcade-services/issues/3277 - if (reapplyVmrPatches && vmrPatchesToRestore.Any() && RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + protected async Task ReapplyVmrPatchesAsync(SourceMapping? mapping, CancellationToken cancellationToken) + { + _logger.LogInformation("Re-applying VMR patches{for}...", mapping != null ? " for " + mapping.Name : null); + + bool patchesApplied = false; + + foreach (var patch in await _patchHandler.GetVmrPatches(patchVersion: null, cancellationToken)) { + if (mapping != null && (!patch.ApplicationPath?.Equals(VmrInfo.SourcesDir / mapping.Name) ?? true)) + { + continue; + } + + await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, removePatchAfter: false, reverseApply: false, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); + patchesApplied = true; } - return vmrPatchesToRestore; - } - - protected async Task ReapplyVmrPatchesAsync( - IReadOnlyCollection patches, - CancellationToken cancellationToken) - { - if (patches.Count == 0) + if (!patchesApplied) { + _logger.LogInformation("No VMR patches to re-apply"); return; } - _logger.LogInformation("Re-applying {count} VMR patch{s}...", - patches.Count, - patches.Count > 1 ? "es" : string.Empty); - - foreach (var patch in patches) + // TODO: Workaround for cases when we get CRLF problems on Windows + // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes + // https://github.com/dotnet/arcade-services/issues/3277 + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - if (!_fileSystem.FileExists(patch.Path)) + cancellationToken.ThrowIfCancellationRequested(); + var result = await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["diff", "--exit-code"], + cancellationToken: cancellationToken); + + if (result.ExitCode != 0) { - // Patch was removed, so it doesn't exist anymore - _logger.LogDebug("Not re-applying {patch} as it was removed", patch.Path); - continue; - } + cancellationToken.ThrowIfCancellationRequested(); + await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); - await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, false, reverseApply: false, cancellationToken); - cancellationToken.ThrowIfCancellationRequested(); + // Sometimes not even checkout helps, so we check again + result = await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["diff", "--exit-code"], + cancellationToken: cancellationToken); + + if (result.ExitCode != 0) + { + cancellationToken.ThrowIfCancellationRequested(); + await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["add", "--u", "."], + cancellationToken: default); + + await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["commit", "--amend", "--no-edit"], + cancellationToken: default); + } + } } _logger.LogInformation("VMR patches re-applied back onto the VMR"); @@ -194,6 +205,14 @@ protected async Task CommitAsync(string commitMessage, (string Name, string Emai await _localGitClient.CommitAsync(_vmrInfo.VmrPath, commitMessage, true, author); _logger.LogInformation("Committed in {duration} seconds", (int) watch.Elapsed.TotalSeconds); + + // TODO: Workaround for cases when we get CRLF problems on Windows + // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes + // https://github.com/dotnet/arcade-services/issues/3277 + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); + } } /// @@ -321,17 +340,12 @@ protected async Task UpdateThirdPartyNoticesAsync(string templatePath, Cancellat } } - protected abstract Task> RestoreVmrPatchedFilesAsync( - SourceMapping mapping, - IReadOnlyCollection patches, - IReadOnlyCollection additionalRemotes, - CancellationToken cancellationToken); - /// /// Takes a given commit message template and populates it with given values, URLs and others. /// /// Template into which the values are filled into - /// Repository mapping + /// Repository name + /// Remote URI of the repository /// SHA we are updating from /// SHA we are updating to /// Additional message inserted in the commit body diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs index ac6dbdee51..d668d51aeb 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs @@ -171,13 +171,13 @@ .. mapping.Exclude.Select(GetExclusionRule), || (destination != null && _fileSystem.FileExists(_vmrInfo.VmrPath / destination / fileName))) { path = new UnixPath(fileName); - + var relativeCloneDir = _fileSystem.GetDirectoryName(relativeClonePath) ?? throw new Exception($"Invalid source path {source} in mapping."); - + contentDir = repoPath / relativeCloneDir; } - else if(!_fileSystem.DirectoryExists(contentDir)) + else if (!_fileSystem.DirectoryExists(contentDir)) { // the source can be a file that doesn't exist, then we skip it continue; @@ -199,7 +199,7 @@ .. mapping.Exclude.Select(GetExclusionRule), { return patches; } - + _logger.LogInformation("Creating diffs for submodules of {repo}..", mapping.Name); foreach (var change in submoduleChanges) @@ -418,7 +418,7 @@ public async Task> CreatePatches( sha2, fileName, // Ignore all files except the one we're currently processing - [.. filters?.Except([GetInclusionRule("**/*")]) ], + [.. filters?.Except([GetInclusionRule("**/*")])], true, workingDir, applicationPath, @@ -561,7 +561,7 @@ private async Task> GetPatchesForSubmoduleChange( CancellationToken cancellationToken) { var checkoutCommit = change.Before == Constants.EmptyGitObject ? change.After : change.Before; - var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, cancellationToken); + var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, cancellationToken); // We are only interested in filters specific to submodule's path ImmutableArray GetSubmoduleFilters(IReadOnlyCollection filters) @@ -580,7 +580,7 @@ static string SanitizeName(string mappingName) { mappingName = mappingName[..^4]; } - + return mappingName; } @@ -608,20 +608,74 @@ static string SanitizeName(string mappingName) cancellationToken); } - public IReadOnlyCollection GetVmrPatches(string mappingName) + /// + /// Returns paths to all VMR patches. + /// + /// Version of the VMR patches to get or null if to take current + /// List of VMR patches belonging to the given mapping + public async Task> GetVmrPatches( + string? patchVersion, + CancellationToken cancellationToken) + => patchVersion == null + ? GetWorkingTreeVmrPatches() + : await GetVmrPatchesFromHistory(patchVersion, cancellationToken); + + private IReadOnlyCollection GetWorkingTreeVmrPatches() + { + if (_vmrInfo.PatchesPath is null) + { + return []; + } + + var patchesDir = _vmrInfo.VmrPath / _vmrInfo.PatchesPath; + + if (!_fileSystem.DirectoryExists(patchesDir)) + { + return []; + } + + return _fileSystem + .GetDirectories(_vmrInfo.VmrPath / _vmrInfo.PatchesPath) + .Select(dir => (Mapping: _fileSystem.GetFileName(dir)!, Patches: _fileSystem.GetFiles(dir))) + .SelectMany(pair => pair.Patches.Select(patch => new VmrIngestionPatch(patch, VmrInfo.SourcesDir / pair.Mapping))) + .ToArray(); + } + + private async Task> GetVmrPatchesFromHistory( + string patchVersion, + CancellationToken cancellationToken) { if (_vmrInfo.PatchesPath is null) { - return Array.Empty(); + return []; + } + + var result = await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["ls-tree", "-r", "--name-only", patchVersion, "--", _vmrInfo.PatchesPath], + cancellationToken); + + if (!result.Succeeded) + { + return []; } - var mappingPatchesPath = _vmrInfo.VmrPath / _vmrInfo.PatchesPath / mappingName; - if (!_fileSystem.DirectoryExists(mappingPatchesPath)) + var patches = result.StandardOutput + .Split(new char[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + + var copiedPatches = new List(); + + foreach (var patch in patches) { - return Array.Empty(); + cancellationToken.ThrowIfCancellationRequested(); + _logger.LogDebug("Found VMR patch {patch}", patch); + var newFileName = _vmrInfo.TmpPath / patch.Replace('/', '_'); + await _localGitClient.GetFileFromGitAsync(_vmrInfo.VmrPath, patch, patchVersion, newFileName); + var mapping = _fileSystem.GetFileName(_fileSystem.GetDirectoryName(patch)!)!; + copiedPatches.Add(new VmrIngestionPatch(newFileName, VmrInfo.SourcesDir / mapping)); } - return _fileSystem.GetFiles(mappingPatchesPath); + return copiedPatches; } public static string GetInclusionRule(string path) => $":(glob,attr:!{VmrInfo.IgnoreAttribute}){path}"; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index d33d305eb7..961fd617a0 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -55,10 +54,12 @@ [Recursive sync] {name} / {oldShaShort}{{Constants.Arrow}}{newShaShort} private readonly ISourceManifest _sourceManifest; private readonly IThirdPartyNoticesGenerator _thirdPartyNoticesGenerator; private readonly IComponentListGenerator _readmeComponentListGenerator; - private readonly ILocalGitClient _localGitClient; private readonly IGitRepoFactory _gitRepoFactory; private readonly IWorkBranchFactory _workBranchFactory; + // The VMR SHA before the synchronization has started + private string? _startingVmrSha; + public VmrUpdater( IVmrDependencyTracker dependencyTracker, IVersionDetailsParser versionDetailsParser, @@ -87,7 +88,6 @@ public VmrUpdater( _fileSystem = fileSystem; _thirdPartyNoticesGenerator = thirdPartyNoticesGenerator; _readmeComponentListGenerator = readmeComponentListGenerator; - _localGitClient = localGitClient; _gitRepoFactory = gitRepoFactory; _workBranchFactory = workBranchFactory; } @@ -106,6 +106,8 @@ public async Task UpdateRepository( { await _dependencyTracker.InitializeSourceMappings(); + _startingVmrSha = await LocalVmr.GetGitCommitAsync(cancellationToken); + var mapping = _dependencyTracker.GetMapping(mappingName); // Reload source-mappings.json if it's getting updated @@ -124,41 +126,44 @@ public async Task UpdateRepository( targetVersion, Parent: null); - if (updateDependencies) - { - return await UpdateRepositoryRecursively( - dependencyUpdate, - additionalRemotes, - componentTemplatePath, - tpnTemplatePath, - generateCodeowners, - discardPatches, - cancellationToken); - } - else + bool hadUpdates; + try { - try + if (updateDependencies) { - var patchesToReapply = await UpdateRepositoryInternal( + hadUpdates = await UpdateRepositoriesRecursively( dependencyUpdate, - reapplyVmrPatches: true, additionalRemotes, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); - return true; } - catch (EmptySyncException e) + else { - _logger.LogInformation(e.Message); - return false; + await RestoreVmrPatchedFilesAsync(cancellationToken); + hadUpdates = await UpdateRepository( + dependencyUpdate, + reapplyVmrPatches: true, + additionalRemotes, + componentTemplatePath, + tpnTemplatePath, + generateCodeowners, + discardPatches, + cancellationToken); } } + catch (EmptySyncException e) + { + _logger.LogInformation(e.Message); + return false; + } + + return hadUpdates; } - private async Task> UpdateRepositoryInternal( + private async Task UpdateRepository( VmrDependencyUpdate update, bool reapplyVmrPatches, IReadOnlyCollection additionalRemotes, @@ -182,10 +187,10 @@ await UpdateTargetVersionOnly( update.Mapping, currentVersion, cancellationToken); - return Array.Empty(); + return true; } - throw new EmptySyncException($"Repository {update.Mapping.Name} is already at {update.TargetRevision}"); + return false; } _logger.LogInformation("Synchronizing {name} from {current} to {repo} / {revision}", @@ -226,33 +231,122 @@ await UpdateTargetVersionOnly( _logger.LogInformation("Updating {repo} from {current} to {next}..", update.Mapping.Name, Commit.GetShortSha(currentVersion.Sha), Commit.GetShortSha(update.TargetRevision)); - var commitMessage = PrepareCommitMessage( - SquashCommitMessage, - update.Mapping.Name, - update.RemoteUri, - currentVersion.Sha, - update.TargetRevision); - - return await UpdateRepoToRevisionAsync( + await StageRepositoryUpdatesAsync( update, clone, additionalRemotes, currentVersion.Sha, - author: null, - commitMessage, - reapplyVmrPatches, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); + + if (reapplyVmrPatches) + { + await ReapplyVmrPatchesAsync(cancellationToken); + } + + var commitMessage = PrepareCommitMessage( + SquashCommitMessage, + update.Mapping.Name, + update.RemoteUri, + currentVersion.Sha, + update.TargetRevision); + + await CommitAsync(commitMessage); + + return true; + } + /// + /// Updates a repository and all of it's dependencies recursively starting with a given mapping. + /// Always updates to the first version found per repository in the dependency tree. + /// + private async Task UpdateRepositoriesRecursively( + VmrDependencyUpdate rootUpdate, + IReadOnlyCollection additionalRemotes, + string? componentTemplatePath, + string? tpnTemplatePath, + bool generateCodeowners, + bool discardPatches, + CancellationToken cancellationToken) + { + // Synchronization creates commits (one per mapping and some extra) + // on a separate branch that is then merged into original one + var originalRootSha = Commit.GetShortSha(GetCurrentVersion(rootUpdate.Mapping)); + var workBranchName = "sync" + + $"/{rootUpdate.Mapping.Name}" + + $"/{originalRootSha}-{rootUpdate.TargetRevision}"; + IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); + + await RestoreVmrPatchedFilesAsync(cancellationToken); + await CommitAsync("[VMR patches] Removed VMR patches"); + + HashSet updatedDependencies; + try + { + updatedDependencies = await UpdateRepositoryRecursively( + rootUpdate, + additionalRemotes, + componentTemplatePath, + tpnTemplatePath, + generateCodeowners, + discardPatches, + cancellationToken); + } + catch (Exception) + { + _logger.LogWarning( + InterruptedSyncExceptionMessage, + workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") + ? "the original" + : workBranch.OriginalBranch); + throw; + } + + await ReapplyVmrPatchesAsync(cancellationToken); + await CommitAsync("[VMR patches] Re-apply VMR patches"); + await CleanUpRemovedRepos(componentTemplatePath, tpnTemplatePath); + + var summaryMessage = new StringBuilder(); + + foreach (var update in updatedDependencies) + { + if (update.TargetRevision == update.TargetVersion) + { + continue; + } + + var fromShort = Commit.GetShortSha(update.TargetVersion); + var toShort = Commit.GetShortSha(update.TargetRevision); + summaryMessage + .AppendLine($" - {update.Mapping.Name} / {fromShort}{Constants.Arrow}{toShort}") + .AppendLine($" {update.RemoteUri}/compare/{update.TargetVersion}..{update.TargetRevision}"); + } + + var commitMessage = PrepareCommitMessage( + MergeCommitMessage, + rootUpdate.Mapping.Name, + rootUpdate.Mapping.DefaultRemote, + originalRootSha, + rootUpdate.TargetRevision, + summaryMessage.ToString()); + + await workBranch.MergeBackAsync(commitMessage); + + _logger.LogInformation("Recursive update for {repo} finished.{newLine}{message}", + rootUpdate.Mapping.Name, + Environment.NewLine, + summaryMessage); + + return updatedDependencies.Any(); } /// /// Updates a repository and all of it's dependencies recursively starting with a given mapping. /// Always updates to the first version found per repository in the dependency tree. /// - private async Task UpdateRepositoryRecursively( + private async Task> UpdateRepositoryRecursively( VmrDependencyUpdate rootUpdate, IReadOnlyCollection additionalRemotes, string? componentTemplatePath, @@ -282,16 +376,6 @@ private async Task UpdateRepositoryRecursively( string.Join(separator, extraneousMappings)); } - // Synchronization creates commits (one per mapping and some extra) on a separate branch that is then merged into original one - - var workBranchName = "sync" + - $"/{rootUpdate.Mapping.Name}" + - $"/{Commit.GetShortSha(GetCurrentVersion(rootUpdate.Mapping))}-{rootUpdate.TargetRevision}"; - IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); - - // Collection of all affected VMR patches we will need to restore after the sync - var vmrPatchesToReapply = new List(); - // Dependencies that were already updated during this run var updatedDependencies = new HashSet(); @@ -326,12 +410,11 @@ private async Task UpdateRepositoryRecursively( update.TargetRevision); } - IReadOnlyCollection patchesToReapply; try { - patchesToReapply = await UpdateRepositoryInternal( + await UpdateRepository( update, - false, + reapplyVmrPatches: false, additionalRemotes, componentTemplatePath, tpnTemplatePath, @@ -357,17 +440,6 @@ private async Task UpdateRepositoryRecursively( _logger.LogWarning(e.Message); continue; } - catch (Exception) - { - _logger.LogWarning( - InterruptedSyncExceptionMessage, - workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") - ? "the original" - : workBranch.OriginalBranch); - throw; - } - - vmrPatchesToReapply.AddRange(patchesToReapply); updatedDependencies.Add(update with { @@ -379,136 +451,31 @@ private async Task UpdateRepositoryRecursively( }); } - string finalRootSha = GetCurrentVersion(rootUpdate.Mapping); - var summaryMessage = new StringBuilder(); - - foreach (var update in updatedDependencies) - { - if (update.TargetRevision == update.TargetVersion) - { - continue; - } - - var fromShort = Commit.GetShortSha(update.TargetVersion); - var toShort = Commit.GetShortSha(update.TargetRevision); - summaryMessage - .AppendLine($" - {update.Mapping.Name} / {fromShort}{Constants.Arrow}{toShort}") - .AppendLine($" {update.RemoteUri}/compare/{update.TargetVersion}..{update.TargetRevision}"); - } - - if (vmrPatchesToReapply.Any()) - { - try - { - await ReapplyVmrPatchesAsync(vmrPatchesToReapply.DistinctBy(p => p.Path).ToArray(), cancellationToken); - } - catch (Exception) - { - _logger.LogWarning( - InterruptedSyncExceptionMessage, - workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") - ? "the original" - : workBranch.OriginalBranch); - throw; - } - - await CommitAsync("[VMR patches] Re-apply VMR patches"); - - // TODO: Workaround for cases when we get CRLF problems on Windows - // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes - // https://github.com/dotnet/arcade-services/issues/3277 - if (vmrPatchesToReapply.Any() && RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - cancellationToken.ThrowIfCancellationRequested(); - var result = await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["diff", "--exit-code"], - cancellationToken: cancellationToken); - - if (result.ExitCode != 0) - { - cancellationToken.ThrowIfCancellationRequested(); - await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); - - // Sometimes not even checkout helps, so we check again - result = await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["diff", "--exit-code"], - cancellationToken: cancellationToken); - - if (result.ExitCode != 0) - { - cancellationToken.ThrowIfCancellationRequested(); - await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["add", "--u", "."], - cancellationToken: default); - - await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["commit", "--amend", "--no-edit"], - cancellationToken: default); - } - } - } - } - - await CleanUpRemovedRepos(componentTemplatePath, tpnTemplatePath); - - var commitMessage = PrepareCommitMessage( - MergeCommitMessage, - rootUpdate.Mapping.Name, - rootUpdate.Mapping.DefaultRemote, - originalRootSha, - finalRootSha, - summaryMessage.ToString()); - - await workBranch.MergeBackAsync(commitMessage); - - _logger.LogInformation("Recursive update for {repo} finished.{newLine}{message}", - rootUpdate.Mapping.Name, - Environment.NewLine, - summaryMessage); - - return updatedDependencies.Any(); + return updatedDependencies; } /// - /// Detects VMR patches affected by a given set of patches and restores files patched by these - /// VMR patches into their original state. - /// Detects whether patched files are coming from a mapped repository or a submodule too. + /// Removes all VMR patches from the working tree so that repository changes can be ingested. /// - /// Mapping that is currently being updated (so we get its patches) - /// Patches with incoming changes to be checked whether they affect some VMR patch - protected override async Task> RestoreVmrPatchedFilesAsync( - SourceMapping updatedMapping, - IReadOnlyCollection patches, - IReadOnlyCollection additionalRemotes, - CancellationToken cancellationToken) + private async Task RestoreVmrPatchedFilesAsync(CancellationToken cancellationToken) { - IReadOnlyCollection vmrPatchesToRestore = await GetVmrPatchesToRestore( - updatedMapping, - patches, - cancellationToken); + _logger.LogInformation("Restoring all VMR patches before we ingest new changes..."); + + IReadOnlyCollection vmrPatchesToRestore = await _patchHandler.GetVmrPatches(_startingVmrSha, cancellationToken); if (vmrPatchesToRestore.Count == 0) { - return vmrPatchesToRestore; + _logger.LogInformation("No VMR patches found"); + return; } foreach (var patch in vmrPatchesToRestore) { - if (!_fileSystem.FileExists(patch.Path)) - { - // Patch is being added, so it doesn't exist yet - _logger.LogDebug("Not restoring {patch} as it will be added during the sync", patch.Path); - continue; - } - + _logger.LogInformation("Restoring VMR patch {patch}", patch.Path); await _patchHandler.ApplyPatch( patch, - _vmrInfo.VmrPath / (patch.ApplicationPath ?? ""), - removePatchAfter: false, + _vmrInfo.VmrPath / (patch.ApplicationPath ?? string.Empty), + removePatchAfter: true, reverseApply: true, cancellationToken); } @@ -517,68 +484,6 @@ await _patchHandler.ApplyPatch( await LocalVmr.ResetWorkingTree(); _logger.LogInformation("Files affected by VMR patches restored"); - - return vmrPatchesToRestore; - } - - /// - /// Gets a list of VMR patches that need to be reverted for a given mapping update so that repo changes can be applied. - /// Usually, this just returns all VMR patches for that given mapping (e.g. for the aspnetcore returns all aspnetcore only patches). - /// - /// One exception is when the updated mapping is the one that the VMR patches come from into the VMR (e.g. dotnet/installer). - /// In this case, we also check which VMR patches are modified by the change and we also returns those. - /// Examples: - /// - An aspnetcore VMR patch is removed from installer - we must remove it from the files it is applied to in the VMR. - /// - A new version of patch is synchronized from installer - we must remove the old version and apply the new. - /// - /// Currently synchronized mapping - /// Patches of currently synchronized changes - private async Task> GetVmrPatchesToRestore( - SourceMapping updatedMapping, - IReadOnlyCollection patches, - CancellationToken cancellationToken) - { - _logger.LogInformation("Getting a list of VMR patches to restore for {repo} before we ingest new changes...", updatedMapping.Name); - - var patchesToRestore = new List(); - - // Always restore all patches belonging to the currently updated mapping - foreach (var vmrPatch in _patchHandler.GetVmrPatches(updatedMapping)) - { - patchesToRestore.Add(new VmrIngestionPatch(vmrPatch, updatedMapping)); - } - - // If we are not updating the mapping that the VMR patches come from, we're done - if (_vmrInfo.PatchesPath == null || !_vmrInfo.PatchesPath.StartsWith(VmrInfo.GetRelativeRepoSourcesPath(updatedMapping))) - { - return patchesToRestore; - } - - _logger.LogInformation("Repo {repo} contains VMR patches, checking which VMR patches have changes...", updatedMapping.Name); - - // Check which files are modified by every of the patches that bring new changes into the VMR - foreach (var patch in patches) - { - cancellationToken.ThrowIfCancellationRequested(); - - IReadOnlyCollection patchedFiles = await _patchHandler.GetPatchedFiles(patch.Path, cancellationToken); - IEnumerable affectedPatches = patchedFiles - .Select(path => VmrInfo.GetRelativeRepoSourcesPath(updatedMapping) / path) - .Where(path => path.Path.StartsWith(_vmrInfo.PatchesPath) && path.Path.EndsWith(".patch")) - .Select(path => _vmrInfo.VmrPath / path); - - foreach (LocalPath affectedPatch in affectedPatches) - { - // patch is in the folder named as the mapping for which it is applied - var affectedRepo = affectedPatch.Path.Split(_fileSystem.DirectorySeparatorChar)[^2]; - var affectedMapping = _dependencyTracker.GetMapping(affectedRepo); - - _logger.LogInformation("Detected a change of a VMR patch {patch} for {repo}", affectedPatch, affectedRepo); - patchesToRestore.Add(new VmrIngestionPatch(affectedPatch, affectedMapping)); - } - } - - return patchesToRestore; } private string GetCurrentVersion(SourceMapping mapping) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs new file mode 100644 index 0000000000..3cb4e7f773 --- /dev/null +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs @@ -0,0 +1,158 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.Helpers; +using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; +using Microsoft.DotNet.DarcLib.VirtualMonoRepo; +using NUnit.Framework; + +namespace Microsoft.DotNet.Darc.Tests.VirtualMonoRepo; + +[TestFixture] +internal class VmrPatchAddedTest : VmrTestsBase +{ + [Test] + public async Task PatchesAreAppliedTest() + { + var newPatchFileName = "new-patch.patch"; + var vmrPatchesDir = VmrPath / VmrInfo.RelativeSourcesDir / Constants.InstallerRepoName / Constants.PatchesFolderName / Constants.ProductRepoName; + var patchPathInVmr = vmrPatchesDir / newPatchFileName; + var installerPatchesDir = InstallerRepoPath / Constants.PatchesFolderName / Constants.ProductRepoName; + var installerFilePathInVmr = VmrPath / VmrInfo.RelativeSourcesDir / Constants.InstallerRepoName / Constants.GetRepoFileName(Constants.InstallerRepoName); + var productRepoFilePathInVmr = VmrPath / VmrInfo.RelativeSourcesDir / Constants.ProductRepoName / Constants.GetRepoFileName(Constants.ProductRepoName); + + await File.WriteAllTextAsync(ProductRepoPath / Constants.GetRepoFileName(Constants.ProductRepoName), + """ + File in the test repo + patches will change the next lines + AAA + CCC + end of changes + """); + await GitOperations.CommitAll(ProductRepoPath, "Change file to CCC"); + + // Update dependent repo to the last commit + var productRepoSha = await GitOperations.GetRepoLastCommit(ProductRepoPath); + var productRepoDependency = string.Format( + Constants.DependencyTemplate, + Constants.ProductRepoName, ProductRepoPath, productRepoSha); + + var versionDetails = string.Format( + Constants.VersionDetailsTemplate, + productRepoDependency); + + await File.WriteAllTextAsync(InstallerRepoPath / VersionFiles.VersionDetailsXml, versionDetails); + await GitOperations.CommitAll(InstallerRepoPath, "Bump product repo to latest"); + + await InitializeRepoAtLastCommit(Constants.InstallerRepoName, InstallerRepoPath); + + CheckFileContents(productRepoFilePathInVmr, "AAA", "CCC"); + + await File.WriteAllTextAsync(ProductRepoPath / Constants.GetRepoFileName(Constants.ProductRepoName), + """ + File in the test repo + patches will change the next lines + AAA + BBB + end of changes + """); + await GitOperations.CommitAll(ProductRepoPath, "Change file to BBB"); + + var expectedFilesFromRepos = new List + { + productRepoFilePathInVmr, + installerFilePathInVmr + }; + + var expectedFiles = GetExpectedFilesInVmr( + VmrPath, + [Constants.ProductRepoName, Constants.InstallerRepoName], + expectedFilesFromRepos + ); + + CheckDirectoryContents(VmrPath, expectedFiles); + + // Add a new patch in installer + File.Copy(VmrTestsOneTimeSetUp.ResourcesPath / newPatchFileName, installerPatchesDir / newPatchFileName); + + // Update dependent repo to the last commit + productRepoSha = await GitOperations.GetRepoLastCommit(ProductRepoPath); + productRepoDependency = string.Format( + Constants.DependencyTemplate, + Constants.ProductRepoName, ProductRepoPath, productRepoSha); + + versionDetails = string.Format( + Constants.VersionDetailsTemplate, + productRepoDependency); + + File.WriteAllText(InstallerRepoPath / VersionFiles.VersionDetailsXml, versionDetails); + + await GitOperations.CommitAll(InstallerRepoPath, "Add a new patch file"); + await UpdateRepoToLastCommit(Constants.InstallerRepoName, InstallerRepoPath); + + // Now we sync installer which means new patch + change in the product repo + // We must check that the patch is detected as being added and won't be restored during repo's sync + // The file will have AAA CCC in the beginning + // The repo change will change it to AAA BBB + // Then the patch will change it to TTT BBB + // If we tried to restore the patch before we sync the repo, the patch fails + // because it will find AAA CCC instead of AAA BBB (which it expects) + await UpdateRepoToLastCommit(Constants.InstallerRepoName, InstallerRepoPath); + + expectedFiles.Add(patchPathInVmr); + CheckDirectoryContents(VmrPath, expectedFiles); + CheckFileContents(productRepoFilePathInVmr, "TTT", "BBB"); + } + + protected override async Task CopyReposForCurrentTest() + { + var dependenciesMap = new Dictionary> + { + { Constants.InstallerRepoName, new List { Constants.ProductRepoName } }, + }; + + await CopyRepoAndCreateVersionFiles(Constants.InstallerRepoName, dependenciesMap); + } + + protected override async Task CopyVmrForCurrentTest() + { + CopyDirectory(VmrTestsOneTimeSetUp.CommonVmrPath, VmrPath); + + var sourceMappings = new SourceMappingFile() + { + Mappings = + [ + new SourceMappingSetting + { + Name = Constants.InstallerRepoName, + DefaultRemote = InstallerRepoPath + }, + new SourceMappingSetting + { + Name = Constants.ProductRepoName, + DefaultRemote = ProductRepoPath + } + ], + PatchesPath = "src/installer/patches/" + }; + + await WriteSourceMappingsInVmr(sourceMappings); + } + + private static void CheckFileContents(NativePath filePath, string line1, string line2) + { + CheckFileContents(filePath, + $""" + File in the test repo + patches will change the next lines + {line1} + {line2} + end of changes + """ + ); + } +} diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs index e20a2d6f2b..eff9b9bbaa 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs @@ -219,8 +219,18 @@ public async Task SubmodulesAreInlinedProperlyTest() // Remove submodule await GitOperations.RemoveSubmodule(ProductRepoPath, submoduleRelativePath); - await File.WriteAllTextAsync(VmrPath / VmrInfo.CodeownersPath, "My new content in the CODEOWNERS\n\n### CONTENT BELOW IS AUTO-GENERATED AND MANUAL CHANGES WILL BE OVERWRITTEN ###\n"); await GitOperations.CommitAll(ProductRepoPath, "Remove the submodule"); + + // Change codeowners + + await File.WriteAllTextAsync(VmrPath / VmrInfo.CodeownersPath, + """ + My new content in the CODEOWNERS + + ### CONTENT BELOW IS AUTO-GENERATED AND MANUAL CHANGES WILL BE OVERWRITTEN ### + """); + await GitOperations.CommitAll(VmrPath, "Updated codeowners"); + await UpdateRepoToLastCommit(Constants.ProductRepoName, ProductRepoPath, generateCodeowners: true); expectedFiles.Remove(submoduleFilePath); From f3171a145ce3bdf15ca303a095e24141e7bcab67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Fri, 28 Jun 2024 12:40:51 +0200 Subject: [PATCH 02/12] Unify AzDO token retrieval across Maestro and darc (#3689) --- .../.config/settings.Development.json | 12 +- .../.config/settings.Production.json | 5 + .../.config/settings.Staging.json | 5 + .../DependencyUpdater/.config/settings.json | 5 - src/Maestro/DependencyUpdater/Program.cs | 2 + .../.config/settings.Development.json | 4 +- .../.config/settings.Production.json | 5 + .../.config/settings.Staging.json | 5 + .../FeedCleanerService/.config/settings.json | 5 - .../FeedCleanerService/FeedCleanerService.cs | 37 ++---- src/Maestro/FeedCleanerService/Program.cs | 13 +- .../AppCredentials/AppCredential.cs | 18 +-- .../AppCredentialResolverOptions.cs | 2 +- .../AppCredentials/ResolvedCredential.cs | 26 ++-- .../AzureDevOpsCredentialResolverOptions.cs | 14 +++ .../AzureDevOpsTokenProvider.cs | 113 +++++++++++++++--- .../AzureDevOpsTokenProviderOptions.cs | 5 +- .../IAzureDevOpsTokenProvider.cs | 22 +--- ...oAzureDevOpsServiceCollectionExtensions.cs | 13 +- .../Maestro.Common/IRemoteTokenProvider.cs | 18 +++ .../DarcRemoteFactory.cs | 33 ++--- .../.config/settings.Development.json | 12 +- .../.config/settings.Production.json | 5 + .../Maestro.Web/.config/settings.Staging.json | 5 + src/Maestro/Maestro.Web/.config/settings.json | 5 - .../Controllers/AzDevController.cs | 2 +- src/Maestro/Maestro.Web/Startup.cs | 2 + .../.config/settings.Development.json | 12 +- .../.config/settings.Production.json | 5 + .../.config/settings.Staging.json | 5 + .../.config/settings.json | 5 - .../DarcRemoteFactory.cs | 23 ++-- .../SubscriptionActorService/Program.cs | 2 + .../Darc/Helpers/RemoteFactory.cs | 8 +- .../Operations/AddBuildToChannelOperation.cs | 9 +- .../Darc/Operations/AddDependencyOperation.cs | 2 +- .../Darc/Operations/CloneOperation.cs | 10 +- .../Operations/GetDependenciesOperation.cs | 2 +- .../Operations/GetDependencyGraphOperation.cs | 6 +- .../Darc/Operations/Operation.cs | 16 ++- .../Operations/UpdateDependenciesOperation.cs | 2 +- .../Darc/Operations/VerifyOperation.cs | 2 +- .../Darc/Options/CommandLineOptions.cs | 20 +++- .../Darc/Options/ICommandLineOptions.cs | 7 +- .../DarcLib/Actions/Local.cs | 5 +- .../DarcLib/AzureDevOpsClient.cs | 43 +++---- .../DarcLib/GitHubClient.cs | 55 ++++++--- .../DarcLib/GitHubTokenProvider.cs | 21 ++++ .../DarcLib/GitNativeRepoCloner.cs | 2 +- .../DarcLib/GitRepoCloner.cs | 9 +- .../DarcLib/GitRepoFactory.cs | 22 ++-- .../DarcLib/ILocalGitClient.cs | 2 +- .../DarcLib/LocalGitClient.cs | 14 +-- .../DarcLib/LocalLibGit2Client.cs | 16 ++- .../DarcLib/Microsoft.DotNet.DarcLib.csproj | 1 + .../DarcLib/Models/Darc/DependencyGraph.cs | 2 +- .../DarcLib/RemoteRepoBase.cs | 39 ++---- .../DarcLib/RemoteTokenProvider.cs | 60 ++++++++-- .../VirtualMonoRepo/VmrRegistrations.cs | 15 ++- .../Configuration/PcsConfiguration.cs | 4 +- .../appsettings.Staging.json | 5 + .../FeedCleanerServiceTests.cs | 13 +- .../MaestroScenarioTestBase.cs | 66 +++++----- test/Maestro.ScenarioTests/TestParameters.cs | 46 +++++-- .../GitHubClientTests.cs | 4 +- 65 files changed, 616 insertions(+), 357 deletions(-) create mode 100644 src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsCredentialResolverOptions.cs create mode 100644 src/Maestro/Maestro.Common/IRemoteTokenProvider.cs create mode 100644 src/Microsoft.DotNet.Darc/DarcLib/GitHubTokenProvider.cs diff --git a/src/Maestro/DependencyUpdater/.config/settings.Development.json b/src/Maestro/DependencyUpdater/.config/settings.Development.json index 887d9725c1..c84178fb69 100644 --- a/src/Maestro/DependencyUpdater/.config/settings.Development.json +++ b/src/Maestro/DependencyUpdater/.config/settings.Development.json @@ -13,10 +13,14 @@ "UseAzCliAuthentication": true }, "AzureDevOps": { - "Tokens": { - "dnceng": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]", - "devdiv": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]", - "domoreexp": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" + "dnceng": { + "Token": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]" + }, + "devdiv": { + "Token": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]" + }, + "domoreexp": { + "Token": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" } } } \ No newline at end of file diff --git a/src/Maestro/DependencyUpdater/.config/settings.Production.json b/src/Maestro/DependencyUpdater/.config/settings.Production.json index 7b67bbc51c..4f1633ebf4 100644 --- a/src/Maestro/DependencyUpdater/.config/settings.Production.json +++ b/src/Maestro/DependencyUpdater/.config/settings.Production.json @@ -10,5 +10,10 @@ "Kusto": { "Database": "engineeringdata", "KustoClusterUri": "https://engsrvprod.westus.kusto.windows.net" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } \ No newline at end of file diff --git a/src/Maestro/DependencyUpdater/.config/settings.Staging.json b/src/Maestro/DependencyUpdater/.config/settings.Staging.json index 270efe35a7..fac1ad31f4 100644 --- a/src/Maestro/DependencyUpdater/.config/settings.Staging.json +++ b/src/Maestro/DependencyUpdater/.config/settings.Staging.json @@ -10,5 +10,10 @@ "Kusto": { "Database": "engineeringdata", "KustoClusterUri": "https://engdata.westus2.kusto.windows.net" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } diff --git a/src/Maestro/DependencyUpdater/.config/settings.json b/src/Maestro/DependencyUpdater/.config/settings.json index 4b7dd5d419..e4b7dddd35 100644 --- a/src/Maestro/DependencyUpdater/.config/settings.json +++ b/src/Maestro/DependencyUpdater/.config/settings.json @@ -2,10 +2,5 @@ "GitHub": { "GitHubAppId": "[vault(github-app-id)]", "PrivateKey": "[vault(github-app-private-key)]" - }, - "AzureDevOps": { - "ManagedIdentities": { - "default": "system" - } } } diff --git a/src/Maestro/DependencyUpdater/Program.cs b/src/Maestro/DependencyUpdater/Program.cs index e26e7956d4..21a7a62d53 100644 --- a/src/Maestro/DependencyUpdater/Program.cs +++ b/src/Maestro/DependencyUpdater/Program.cs @@ -7,6 +7,7 @@ using Maestro.DataProviders; using Microsoft.DncEng.Configuration.Extensions; using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.GitHub.Authentication; using Microsoft.DotNet.Kusto; using Microsoft.DotNet.ServiceFabric.ServiceHost; @@ -57,6 +58,7 @@ public static void Configure(IServiceCollection services) // in such a way that will work with sizing. services.AddSingleton(); + services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); services.AddTransient(); services.AddScoped(); services.AddTransient(); diff --git a/src/Maestro/FeedCleanerService/.config/settings.Development.json b/src/Maestro/FeedCleanerService/.config/settings.Development.json index 4aa6c7834f..b2a3eca14b 100644 --- a/src/Maestro/FeedCleanerService/.config/settings.Development.json +++ b/src/Maestro/FeedCleanerService/.config/settings.Development.json @@ -8,8 +8,8 @@ "ConnectionString": "Data Source=localhost\\SQLEXPRESS;Initial Catalog=BuildAssetRegistry;Integrated Security=true" }, "AzureDevOps": { - "Tokens": { - "dnceng": "[vault(dn-bot-dnceng-packaging-rwm)]" + "dnceng": { + "Token": "[vault(dn-bot-dnceng-packaging-rwm)]" } } } \ No newline at end of file diff --git a/src/Maestro/FeedCleanerService/.config/settings.Production.json b/src/Maestro/FeedCleanerService/.config/settings.Production.json index 68ae60b3d1..8b1b69796e 100644 --- a/src/Maestro/FeedCleanerService/.config/settings.Production.json +++ b/src/Maestro/FeedCleanerService/.config/settings.Production.json @@ -9,5 +9,10 @@ }, "BuildAssetRegistry": { "ConnectionString": "Data Source=tcp:maestro-prod.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Managed Identity; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=30; Encrypt=True; TrustServerCertificate=False;" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } \ No newline at end of file diff --git a/src/Maestro/FeedCleanerService/.config/settings.Staging.json b/src/Maestro/FeedCleanerService/.config/settings.Staging.json index f322dd3dff..7e364edb10 100644 --- a/src/Maestro/FeedCleanerService/.config/settings.Staging.json +++ b/src/Maestro/FeedCleanerService/.config/settings.Staging.json @@ -6,5 +6,10 @@ "KeyVaultUri": "https://maestroint.vault.azure.net/", "BuildAssetRegistry": { "ConnectionString": "Data Source=tcp:maestro-int-server.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Managed Identity; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=30; Encrypt=True; TrustServerCertificate=False;" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } diff --git a/src/Maestro/FeedCleanerService/.config/settings.json b/src/Maestro/FeedCleanerService/.config/settings.json index b87d8cc284..65c05be4e0 100644 --- a/src/Maestro/FeedCleanerService/.config/settings.json +++ b/src/Maestro/FeedCleanerService/.config/settings.json @@ -23,10 +23,5 @@ "Name": "dotnet-tools" } ] - }, - "AzureDevOps": { - "ManagedIdentities": { - "default": "system" - } } } \ No newline at end of file diff --git a/src/Maestro/FeedCleanerService/FeedCleanerService.cs b/src/Maestro/FeedCleanerService/FeedCleanerService.cs index 5496a0c768..3f540f3002 100644 --- a/src/Maestro/FeedCleanerService/FeedCleanerService.cs +++ b/src/Maestro/FeedCleanerService/FeedCleanerService.cs @@ -9,7 +9,6 @@ using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; -using Maestro.Common.AzureDevOpsTokens; using Maestro.Contracts; using Maestro.Data; using Maestro.Data.Models; @@ -17,7 +16,6 @@ using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.ServiceFabric.ServiceHost; using Microsoft.EntityFrameworkCore; -using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -29,19 +27,16 @@ namespace FeedCleanerService; public sealed class FeedCleanerService : IFeedCleanerService, IServiceImplementation { public FeedCleanerService( + IAzureDevOpsClient azureDevOpsClient, ILogger logger, BuildAssetRegistryContext context, IOptions options) { + _azureDevOpsClient = azureDevOpsClient; Logger = logger; Context = context; _httpClient = new HttpClient(new HttpClientHandler() { CheckCertificateRevocationList = true }); _options = options; - AzureDevOpsClients = []; - foreach (string account in _options.Value.AzdoAccounts) - { - AzureDevOpsClients.Add(account, GetAzureDevOpsClientForAccountAsync(account)); - } } public ILogger Logger { get; } @@ -49,9 +44,8 @@ public FeedCleanerService( public FeedCleanerOptions Options => _options.Value; - public Dictionary AzureDevOpsClients { get; set; } - private readonly HttpClient _httpClient; + private readonly IAzureDevOpsClient _azureDevOpsClient; private readonly IOptions _options; public Task RunAsync(CancellationToken cancellationToken) @@ -74,8 +68,7 @@ public async Task CleanManagedFeedsAsync() foreach (var azdoAccount in Options.AzdoAccounts) { - var azdoClient = AzureDevOpsClients[azdoAccount]; - List allFeeds = await azdoClient.GetFeedsAsync(azdoAccount); + List allFeeds = await _azureDevOpsClient.GetFeedsAsync(azdoAccount); IEnumerable managedFeeds = allFeeds.Where(f => Regex.IsMatch(f.Name, FeedConstants.MaestroManagedFeedNamePattern)); foreach (var feed in managedFeeds) @@ -107,20 +100,6 @@ public async Task CleanManagedFeedsAsync() } } - /// - /// Returns an Azure DevOps client for a given account - /// - /// Azure DevOps account that the client will get its token from - /// Azure DevOps client for the given account - private AzureDevOpsClient GetAzureDevOpsClientForAccountAsync(string account) - { - IAzureDevOpsTokenProvider azdoTokenProvider = Context.GetService(); - string accessToken = azdoTokenProvider.GetTokenForAccount(account).GetAwaiter().GetResult(); - - // FeedCleaner does not need Git, or a temporaryRepositoryPath - return new AzureDevOpsClient(null, accessToken, Logger, null); - } - /// /// Get a mapping of feed -> (package, versions) for the release feeds so it /// can be easily queried whether a version of a package is in a feed. @@ -162,9 +141,8 @@ private static string ComputeAzureArtifactsNuGetFeedUrl(string feedName, string /// Dictionary where the key is the package name, and the value is a HashSet of the versions of the package in the feed private async Task>> GetPackageVersionsForFeedAsync(string account, string project, string feedName) { - var azdoClient = AzureDevOpsClients[account]; var packagesWithVersions = new Dictionary>(); - List packagesInFeed = await azdoClient.GetPackagesForFeedAsync(account, project, feedName); + List packagesInFeed = await _azureDevOpsClient.GetPackagesForFeedAsync(account, project, feedName); foreach (AzureDevOpsPackage package in packagesInFeed) { packagesWithVersions.Add(package.Name, new HashSet(StringComparer.OrdinalIgnoreCase)); @@ -266,14 +244,13 @@ private async Task> UpdateReleasedVersionsForPackageAsync( /// private async Task DeletePackageVersionsFromFeedAsync(AzureDevOpsFeed feed, string packageName, HashSet versionsToDelete) { - var azdoClient = AzureDevOpsClients[feed.Account]; foreach (string version in versionsToDelete) { try { Logger.LogInformation($"Deleting package {packageName}.{version} from feed {feed.Name}"); - await azdoClient.DeleteNuGetPackageVersionFromFeedAsync(feed.Account, + await _azureDevOpsClient.DeleteNuGetPackageVersionFromFeedAsync(feed.Account, feed.Project?.Name, feed.Name, packageName, @@ -343,6 +320,6 @@ private async Task IsPackageAvailableInNugetOrgAsync(string name, string v /// private async Task PopulatePackagesForFeedAsync(AzureDevOpsFeed feed) { - feed.Packages = await AzureDevOpsClients[feed.Account].GetPackagesForFeedAsync(feed.Account, feed.Project?.Name, feed.Name); + feed.Packages = await _azureDevOpsClient.GetPackagesForFeedAsync(feed.Account, feed.Project?.Name, feed.Name); } } diff --git a/src/Maestro/FeedCleanerService/Program.cs b/src/Maestro/FeedCleanerService/Program.cs index 22032ade7a..d54eea75dc 100644 --- a/src/Maestro/FeedCleanerService/Program.cs +++ b/src/Maestro/FeedCleanerService/Program.cs @@ -1,11 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Generic; -using System.Linq; using Maestro.Common.AzureDevOpsTokens; using Maestro.Data; using Microsoft.DncEng.Configuration.Extensions; +using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.ServiceFabric.ServiceHost; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -39,12 +39,9 @@ public static void Configure(IServiceCollection services) options.ReleasePackageFeeds.Add((token1.GetValue("Account"), token1.GetValue("Project"), token1.GetValue("Name"))); } - AzureDevOpsTokenProviderOptions azdoConfig = new(); + AzureDevOpsTokenProviderOptions azdoConfig = []; config.GetSection("AzureDevOps").Bind(azdoConfig); - IEnumerable allOrgs = azdoConfig.Tokens.Keys - .Concat(azdoConfig.ManagedIdentities.Keys) - .Distinct(); - options.AzdoAccounts.AddRange(allOrgs); + options.AzdoAccounts.AddRange(azdoConfig.Keys); }); services.AddDefaultJsonConfiguration(); services.AddBuildAssetRegistry((provider, options) => @@ -54,5 +51,7 @@ public static void Configure(IServiceCollection services) }); services.AddAzureDevOpsTokenProvider(); services.Configure("AzureDevOps", (o, s) => s.Bind(o)); + services.AddTransient(); + services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); } } diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs index ad9732d8b5..571e4e759e 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs @@ -20,7 +20,7 @@ public class AppCredential : TokenCredential private readonly TokenRequestContext _requestContext; private readonly TokenCredential _tokenCredential; - private AppCredential(TokenCredential credential, TokenRequestContext requestContext) + public AppCredential(TokenCredential credential, TokenRequestContext requestContext) { _requestContext = requestContext; _tokenCredential = credential; @@ -42,9 +42,13 @@ public override ValueTask GetTokenAsync(TokenRequestContext _, Canc /// Use this for user-based flows. /// public static AppCredential CreateUserCredential(string appId, string userScope = ".default") - { - var requestContext = new TokenRequestContext(new string[] { $"api://{appId}/{userScope}" }); + => CreateUserCredential(appId, new TokenRequestContext([$"api://{appId}/{userScope}"])); + /// + /// Use this for user-based flows. + /// + public static AppCredential CreateUserCredential(string appId, TokenRequestContext requestContext) + { var authRecordPath = Path.Combine(AUTH_CACHE, $"{AUTH_RECORD_PREFIX}-{appId}"); var credential = GetInteractiveCredential(appId, requestContext, authRecordPath); @@ -122,7 +126,7 @@ public static AppCredential CreateFederatedCredential(string appId, string feder appId, token => Task.FromResult(federatedToken)); - var requestContext = new TokenRequestContext(new string[] { $"api://{appId}/.default" }); + var requestContext = new TokenRequestContext([$"api://{appId}/.default"]); return new AppCredential(credential, requestContext); } @@ -139,9 +143,9 @@ public static AppCredential CreateManagedIdentityCredential(string appId, string var appCredential = new ClientAssertionCredential( TENANT_ID, appId, - async (ct) => (await miCredential.GetTokenAsync(new TokenRequestContext(new string[] { "api://AzureADTokenExchange" }), ct)).Token); + async (ct) => (await miCredential.GetTokenAsync(new TokenRequestContext(["api://AzureADTokenExchange"]), ct)).Token); - var requestContext = new TokenRequestContext(new string[] { $"api://{appId}/.default" }); + var requestContext = new TokenRequestContext([$"api://{appId}/.default"]); return new AppCredential(appCredential, requestContext); } @@ -150,7 +154,7 @@ public static AppCredential CreateManagedIdentityCredential(string appId, string /// public static AppCredential CreateNonUserCredential(string appId) { - var requestContext = new TokenRequestContext(new string[] { $"{appId}/.default" }); + var requestContext = new TokenRequestContext([$"{appId}/.default"]); var credential = new AzureCliCredential(); return new AppCredential(credential, requestContext); } diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolverOptions.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolverOptions.cs index dac9841887..56eab349ba 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolverOptions.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolverOptions.cs @@ -8,7 +8,7 @@ public class AppCredentialResolverOptions : CredentialResolverOptions /// /// Client ID of the Azure application to request the token for /// - public string AppId { get; set; } + public string AppId { get; } /// /// User scope to request the token for (in case of user flows). diff --git a/src/Maestro/Maestro.Common/AppCredentials/ResolvedCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/ResolvedCredential.cs index 17bd08e7df..92ef1b01cd 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/ResolvedCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/ResolvedCredential.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Threading; using Azure.Core; namespace Maestro.Common.AppCredentials; @@ -8,22 +9,31 @@ namespace Maestro.Common.AppCredentials; /// /// Credential with a set token. /// -public class ResolvedCredential : TokenCredential +public class ResolvedCredential(string token) : TokenCredential { - public ResolvedCredential(string token) + public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) { - Token = token; + return new AccessToken(token, DateTimeOffset.MaxValue); } - public string Token { get; } + public override ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) + { + return new ValueTask(new AccessToken(token, DateTimeOffset.MaxValue)); + } +} - public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) +/// +/// Credential that resolves the token on each request. +/// +public class ResolvingCredential(Func tokenResolver) : TokenCredential +{ + public override AccessToken GetToken(TokenRequestContext context, CancellationToken cancellationToken) { - return new AccessToken(Token, DateTimeOffset.MaxValue); + return new AccessToken(tokenResolver(context, cancellationToken), DateTimeOffset.UtcNow); } - public override ValueTask GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) + public override ValueTask GetTokenAsync(TokenRequestContext context, CancellationToken cancellationToken) { - return new ValueTask(new AccessToken(Token, DateTimeOffset.MaxValue)); + return new ValueTask(new AccessToken(tokenResolver(context, cancellationToken), DateTimeOffset.UtcNow)); } } diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsCredentialResolverOptions.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsCredentialResolverOptions.cs new file mode 100644 index 0000000000..79646ebb76 --- /dev/null +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsCredentialResolverOptions.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Maestro.Common.AppCredentials; + +namespace Maestro.Common.AzureDevOpsTokens; + +public class AzureDevOpsCredentialResolverOptions : AppCredentialResolverOptions +{ + public AzureDevOpsCredentialResolverOptions() + : base("499b84ac-1321-427f-aa17-267ca6975798") + { + } +} diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs index 37c2de674a..0ada4cacfe 100644 --- a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs @@ -1,8 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Text.RegularExpressions; using Azure.Core; using Azure.Identity; +using Maestro.Common.AppCredentials; using Microsoft.Extensions.Options; namespace Maestro.Common.AzureDevOpsTokens; @@ -26,42 +28,123 @@ namespace Maestro.Common.AzureDevOpsTokens; public class AzureDevOpsTokenProvider : IAzureDevOpsTokenProvider { private const string AzureDevOpsScope = "499b84ac-1321-427f-aa17-267ca6975798/.default"; + private static readonly Regex AccountNameRegex = new(@"^https://dev\.azure\.com/(?[a-zA-Z0-9]+)/"); - private readonly Dictionary _tokenCredentials = []; - private readonly IOptionsMonitor _options; + private readonly Dictionary _accountCredentials; public AzureDevOpsTokenProvider(IOptionsMonitor options) + // We don't mind locking down the current value of the option because non-token settings are not expected to change + // Tokens are always read fresh through the second argument + : this(GetCredentials(options.CurrentValue, (account, _, _) => options.CurrentValue[account].Token!)) { - _options = options; + } + + public static AzureDevOpsTokenProvider FromStaticOptions(AzureDevOpsTokenProviderOptions options) + => new(GetCredentials(options, (account, _, _) => options[account].Token!)); + + private AzureDevOpsTokenProvider(Dictionary accountCredentials) + { + _accountCredentials = accountCredentials; + } + + public string GetTokenForAccount(string account) + { + var credential = GetCredential(account); + return credential.GetToken(new TokenRequestContext([AzureDevOpsScope]), cancellationToken: default).Token; + } + + public async Task GetTokenForAccountAsync(string account) + { + var credential = GetCredential(account); + return (await credential.GetTokenAsync(new TokenRequestContext([AzureDevOpsScope]), cancellationToken: default)).Token; + } - foreach (var credential in options.CurrentValue.ManagedIdentities) + public string GetTokenForRepository(string repositoryUrl) + { + Match m = AccountNameRegex.Match(repositoryUrl); + if (!m.Success) { - _tokenCredentials[credential.Key] = credential.Value == "system" - ? new ManagedIdentityCredential() - : new ManagedIdentityCredential(credential.Value); + throw new ArgumentException($"{repositoryUrl} is not a valid Azure DevOps repository URL"); } + + var account = m.Groups["account"].Value; + return GetTokenForAccount(account); } - public async Task GetTokenForAccount(string account) + public async Task GetTokenForRepositoryAsync(string repositoryUrl) { - if (_options.CurrentValue.Tokens.TryGetValue(account, out var pat) && !string.IsNullOrEmpty(pat)) + Match m = AccountNameRegex.Match(repositoryUrl); + if (!m.Success) { - return pat; + throw new ArgumentException($"{repositoryUrl} is not a valid Azure DevOps repository URL"); } - if (_tokenCredentials.TryGetValue(account, out var credential)) + var account = m.Groups["account"].Value; + return await GetTokenForAccountAsync(account); + } + + private TokenCredential GetCredential(string account) + { + if (_accountCredentials.TryGetValue(account, out var credential)) { - return (await credential.GetTokenAsync(new TokenRequestContext([AzureDevOpsScope]))).Token; + return credential; } - // We can also define just one MI for all accounts - if (_tokenCredentials.TryGetValue("default", out var defaultCredential)) + if (_accountCredentials.TryGetValue("default", out var defaultCredential)) { - return (await defaultCredential.GetTokenAsync(new TokenRequestContext([AzureDevOpsScope]))).Token; + return defaultCredential; } throw new ArgumentOutOfRangeException( $"Azure DevOps account {account} does not have a configured PAT or credential. " + $"Please add the account to the 'AzureDevOps.Tokens' or 'AzureDevOps.ManagedIdentities' configuration section"); } + + private static Dictionary GetCredentials( + AzureDevOpsTokenProviderOptions options, + Func patResolver) + { + Dictionary credentials = []; + + foreach (var pair in options) + { + var account = pair.Key; + var option = pair.Value; + + // 1. AzDO PAT from configuration + if (!string.IsNullOrEmpty(option.Token)) + { + credentials[account] = new ResolvingCredential((context, cancellationToken) => patResolver(account, context, cancellationToken)); + continue; + } + + // 2. Federated token that can be used to fetch an app token (for CI scenarios) + if (!string.IsNullOrEmpty(option.FederatedToken)) + { + credentials[account] = AppCredential.CreateFederatedCredential(option.AppId, option.FederatedToken!); + continue; + } + + // 3. Managed identity (for server-to-AzDO scenarios) + if (!string.IsNullOrEmpty(option.ManagedIdentityId)) + { + credentials[account] = option.ManagedIdentityId == "system" + ? new ManagedIdentityCredential() + : new ManagedIdentityCredential(option.ManagedIdentityId); + continue; + } + + // 4. Azure CLI authentication setup by the caller (for CI scenarios) + if (option.DisableInteractiveAuth) + { + credentials[account] = AppCredential.CreateNonUserCredential(option.AppId); + continue; + } + + // 5. Interactive login (user-based scenario) + credentials[account] = new DefaultAzureCredential(includeInteractiveCredentials: true); + } + + return credentials; + } } diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProviderOptions.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProviderOptions.cs index cd8905ab69..7b2b5c836c 100644 --- a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProviderOptions.cs +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProviderOptions.cs @@ -3,9 +3,6 @@ namespace Maestro.Common.AzureDevOpsTokens; -public class AzureDevOpsTokenProviderOptions +public class AzureDevOpsTokenProviderOptions : Dictionary { - public Dictionary Tokens { get; } = []; - - public Dictionary ManagedIdentities { get; } = []; } diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/IAzureDevOpsTokenProvider.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/IAzureDevOpsTokenProvider.cs index a43042e143..28eeca8929 100644 --- a/src/Maestro/Maestro.Common/AzureDevOpsTokens/IAzureDevOpsTokenProvider.cs +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/IAzureDevOpsTokenProvider.cs @@ -1,27 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Text.RegularExpressions; - namespace Maestro.Common.AzureDevOpsTokens; -public interface IAzureDevOpsTokenProvider -{ - Task GetTokenForAccount(string account); -} - -public static class AzureDevOpsTokenProviderExtensions +public interface IAzureDevOpsTokenProvider : IRemoteTokenProvider { - private static readonly Regex AccountNameRegex = new(@"^https://dev\.azure\.com/(?[a-zA-Z0-9]+)/"); + string GetTokenForAccount(string account); - public static Task GetTokenForRepository(this IAzureDevOpsTokenProvider that, string repositoryUrl) - { - Match m = AccountNameRegex.Match(repositoryUrl); - if (!m.Success) - { - throw new ArgumentException($"{repositoryUrl} is not a valid Azure DevOps repository URL"); - } - var account = m.Groups["account"].Value; - return that.GetTokenForAccount(account); - } + Task GetTokenForAccountAsync(string account); } diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/MaestroAzureDevOpsServiceCollectionExtensions.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/MaestroAzureDevOpsServiceCollectionExtensions.cs index 0eee5b95f7..601659182c 100644 --- a/src/Maestro/Maestro.Common/AzureDevOpsTokens/MaestroAzureDevOpsServiceCollectionExtensions.cs +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/MaestroAzureDevOpsServiceCollectionExtensions.cs @@ -7,8 +7,19 @@ namespace Maestro.Common.AzureDevOpsTokens; public static class MaestroAzureDevOpsServiceCollectionExtensions { - public static IServiceCollection AddAzureDevOpsTokenProvider(this IServiceCollection services) + /// + /// Registers the Azure DevOps token provider. + /// + /// If provided, will initialize these options. Otherwise will try to monitor configuration. + public static IServiceCollection AddAzureDevOpsTokenProvider( + this IServiceCollection services, + AzureDevOpsTokenProviderOptions? staticOptions = null) { + if (staticOptions != null) + { + services.AddSingleton(staticOptions); + } + return services.AddSingleton(); } } diff --git a/src/Maestro/Maestro.Common/IRemoteTokenProvider.cs b/src/Maestro/Maestro.Common/IRemoteTokenProvider.cs new file mode 100644 index 0000000000..9069fcd889 --- /dev/null +++ b/src/Maestro/Maestro.Common/IRemoteTokenProvider.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Maestro.Common; + +public interface IRemoteTokenProvider +{ + string? GetTokenForRepository(string repoUri); + + Task GetTokenForRepositoryAsync(string repoUri); +} + +public class ResolvedTokenProvider(string? token) : IRemoteTokenProvider +{ + public string? GetTokenForRepository(string repoUri) => token; + + public Task GetTokenForRepositoryAsync(string repoUri) => Task.FromResult(token); +} diff --git a/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs b/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs index fd5967a587..7cffd9f56d 100644 --- a/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs +++ b/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs @@ -17,25 +17,27 @@ public class DarcRemoteFactory : IRemoteFactory { private readonly IVersionDetailsParser _versionDetailsParser; private readonly OperationManager _operations; + private readonly IProcessManager _processManager; private readonly BuildAssetRegistryContext _context; private readonly DarcRemoteMemoryCache _cache; private readonly IGitHubTokenProvider _gitHubTokenProvider; - private readonly IAzureDevOpsTokenProvider _azureDevOpsTokenProvider; + private readonly IAzureDevOpsTokenProvider _azdoTokenProvider; public DarcRemoteFactory( BuildAssetRegistryContext context, IGitHubTokenProvider gitHubTokenProvider, - IAzureDevOpsTokenProvider azureDevOpsTokenProvider, + IAzureDevOpsTokenProvider azdoTokenProvider, IVersionDetailsParser versionDetailsParser, DarcRemoteMemoryCache memoryCache, - OperationManager operations) + OperationManager operations, + IProcessManager processManager) { _operations = operations; + _processManager = processManager; _versionDetailsParser = versionDetailsParser; - _context = context; _gitHubTokenProvider = gitHubTokenProvider; - _azureDevOpsTokenProvider = azureDevOpsTokenProvider; + _azdoTokenProvider = azdoTokenProvider; _cache = memoryCache; } @@ -72,32 +74,17 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo throw new GithubApplicationInstallationException($"No installation is available for repository '{normalizedUrl}'"); } - var remoteConfiguration = repoType switch - { - GitRepoType.GitHub => new RemoteTokenProvider( - gitHubToken: await _gitHubTokenProvider.GetTokenForInstallationAsync(installationId)), - GitRepoType.AzureDevOps => new RemoteTokenProvider( - azureDevOpsToken: await _azureDevOpsTokenProvider.GetTokenForRepository(normalizedUrl)), - - _ => throw new NotImplementedException($"Unknown repo url type {normalizedUrl}"), - }; - return repoType switch { GitRepoType.GitHub => installationId == default ? throw new GithubApplicationInstallationException($"No installation is available for repository '{normalizedUrl}'") : new GitHubClient( - gitExecutable: null, - remoteConfiguration.GitHubToken, + new Microsoft.DotNet.DarcLib.GitHubTokenProvider(_gitHubTokenProvider), + _processManager, logger, - temporaryRepositoryPath: null, _cache.Cache), - GitRepoType.AzureDevOps => new AzureDevOpsClient( - gitExecutable: null, - remoteConfiguration.AzureDevOpsToken, - logger, - temporaryRepositoryPath: null), + GitRepoType.AzureDevOps => new AzureDevOpsClient(_azdoTokenProvider, _processManager, logger), _ => throw new NotImplementedException($"Unknown repo url type {normalizedUrl}"), }; diff --git a/src/Maestro/Maestro.Web/.config/settings.Development.json b/src/Maestro/Maestro.Web/.config/settings.Development.json index 70b6f27c98..6f961d63d9 100644 --- a/src/Maestro/Maestro.Web/.config/settings.Development.json +++ b/src/Maestro/Maestro.Web/.config/settings.Development.json @@ -19,10 +19,14 @@ "UseAzCliAuthentication": true }, "AzureDevOps": { - "Tokens": { - "dnceng": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]", - "devdiv": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]", - "domoreexp": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" + "dnceng": { + "Token": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]" + }, + "devdiv": { + "Token": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]" + }, + "domoreexp": { + "Token": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" } } } \ No newline at end of file diff --git a/src/Maestro/Maestro.Web/.config/settings.Production.json b/src/Maestro/Maestro.Web/.config/settings.Production.json index 89ede15ffe..d3e979fcb6 100644 --- a/src/Maestro/Maestro.Web/.config/settings.Production.json +++ b/src/Maestro/Maestro.Web/.config/settings.Production.json @@ -27,5 +27,10 @@ "ClientId": "54c17f3d-7325-4eca-9db7-f090bfc765a8", "UserRole": "User", "RedirectUri": "https://maestro.dot.net/signin-oidc" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } diff --git a/src/Maestro/Maestro.Web/.config/settings.Staging.json b/src/Maestro/Maestro.Web/.config/settings.Staging.json index 2fd1c14a95..796b839997 100644 --- a/src/Maestro/Maestro.Web/.config/settings.Staging.json +++ b/src/Maestro/Maestro.Web/.config/settings.Staging.json @@ -23,5 +23,10 @@ "ClientId": "baf98f1b-374e-487d-af42-aa33807f11e4", "UserRole": "User", "RedirectUri": "https://maestro.int-dot.net/signin-oidc" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } diff --git a/src/Maestro/Maestro.Web/.config/settings.json b/src/Maestro/Maestro.Web/.config/settings.json index 81098fab20..85a503c094 100644 --- a/src/Maestro/Maestro.Web/.config/settings.json +++ b/src/Maestro/Maestro.Web/.config/settings.json @@ -11,11 +11,6 @@ "GitHubAppId": "[vault(github-app-id)]", "PrivateKey": "[vault(github-app-private-key)]" }, - "AzureDevOps": { - "ManagedIdentities": { - "default": "system" - } - }, "WebHooks": { "github": { "SecretKey": { diff --git a/src/Maestro/Maestro.Web/Controllers/AzDevController.cs b/src/Maestro/Maestro.Web/Controllers/AzDevController.cs index 3b1b7c9497..5a4556ecfc 100644 --- a/src/Maestro/Maestro.Web/Controllers/AzDevController.cs +++ b/src/Maestro/Maestro.Web/Controllers/AzDevController.cs @@ -38,7 +38,7 @@ public AzDevController(IAzureDevOpsTokenProvider tokenProvider) [HttpGet("build/status/{account}/{project}/{definitionId}/{*branch}")] public async Task GetBuildStatus(string account, string project, int definitionId, string branch, int count, string status) { - string token = await TokenProvider.GetTokenForAccount(account); + string token = await TokenProvider.GetTokenForAccountAsync(account); return await HttpContext.ProxyRequestAsync( s_lazyClient.Value, diff --git a/src/Maestro/Maestro.Web/Startup.cs b/src/Maestro/Maestro.Web/Startup.cs index 924ad3255d..07dbe7ec2a 100644 --- a/src/Maestro/Maestro.Web/Startup.cs +++ b/src/Maestro/Maestro.Web/Startup.cs @@ -48,6 +48,7 @@ using Newtonsoft.Json; using Swashbuckle.AspNetCore.Swagger; using Maestro.Common.AzureDevOpsTokens; +using Microsoft.DotNet.DarcLib.Helpers; namespace Maestro.Web; @@ -243,6 +244,7 @@ public override void ConfigureServices(IServiceCollection services) // in such a way that will work with sizing. services.AddSingleton(); + services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); services.AddTransient(); services.AddScoped(); services.AddTransient(); diff --git a/src/Maestro/SubscriptionActorService/.config/settings.Development.json b/src/Maestro/SubscriptionActorService/.config/settings.Development.json index 914577e8a6..d43389d25a 100644 --- a/src/Maestro/SubscriptionActorService/.config/settings.Development.json +++ b/src/Maestro/SubscriptionActorService/.config/settings.Development.json @@ -17,10 +17,14 @@ "UseAzCliAuthentication": true }, "AzureDevOps": { - "Tokens": { - "dnceng": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]", - "devdiv": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]", - "domoreexp": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" + "dnceng": { + "Token": "[vault(dn-bot-dnceng-build-rw-code-rw-release-rw)]" + }, + "devdiv": { + "Token": "[vault(dn-bot-devdiv-build-rw-code-rw-release-rw)]" + }, + "domoreexp": { + "Token": "[vault(dn-bot-domoreexp-build-rw-code-rw-release-rw)]" } } } \ No newline at end of file diff --git a/src/Maestro/SubscriptionActorService/.config/settings.Production.json b/src/Maestro/SubscriptionActorService/.config/settings.Production.json index 58d3937003..61284f40b4 100644 --- a/src/Maestro/SubscriptionActorService/.config/settings.Production.json +++ b/src/Maestro/SubscriptionActorService/.config/settings.Production.json @@ -15,5 +15,10 @@ "Kusto": { "Database": "engineeringdata", "KustoClusterUri": "https://engsrvprod.westus.kusto.windows.net" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } \ No newline at end of file diff --git a/src/Maestro/SubscriptionActorService/.config/settings.Staging.json b/src/Maestro/SubscriptionActorService/.config/settings.Staging.json index 0767e7b93f..10f3d23849 100644 --- a/src/Maestro/SubscriptionActorService/.config/settings.Staging.json +++ b/src/Maestro/SubscriptionActorService/.config/settings.Staging.json @@ -14,5 +14,10 @@ "Kusto": { "Database": "engineeringdata", "KustoClusterUri": "https://engdata.westus2.kusto.windows.net" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } \ No newline at end of file diff --git a/src/Maestro/SubscriptionActorService/.config/settings.json b/src/Maestro/SubscriptionActorService/.config/settings.json index 4b7dd5d419..e4b7dddd35 100644 --- a/src/Maestro/SubscriptionActorService/.config/settings.json +++ b/src/Maestro/SubscriptionActorService/.config/settings.json @@ -2,10 +2,5 @@ "GitHub": { "GitHubAppId": "[vault(github-app-id)]", "PrivateKey": "[vault(github-app-private-key)]" - }, - "AzureDevOps": { - "ManagedIdentities": { - "default": "system" - } } } diff --git a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs index fab9ed035a..aebea88b17 100644 --- a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs +++ b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs @@ -3,6 +3,7 @@ using System; using System.Threading.Tasks; +using Maestro.Common; using Maestro.Common.AzureDevOpsTokens; using Maestro.Data; using Microsoft.DotNet.DarcLib; @@ -22,10 +23,10 @@ public class DarcRemoteFactory : IRemoteFactory private readonly IAzureDevOpsTokenProvider _azureDevOpsTokenProvider; private readonly BuildAssetRegistryContext _context; private readonly DarcRemoteMemoryCache _cache; - private readonly TemporaryFiles _tempFiles; private readonly ILocalGit _localGit; private readonly IVersionDetailsParser _versionDetailsParser; + private readonly IProcessManager _processManager; private readonly OperationManager _operations; public DarcRemoteFactory( @@ -37,11 +38,13 @@ public DarcRemoteFactory( TemporaryFiles tempFiles, ILocalGit localGit, IVersionDetailsParser versionDetailsParser, + IProcessManager processManager, OperationManager operations) { _tempFiles = tempFiles; _localGit = localGit; _versionDetailsParser = versionDetailsParser; + _processManager = processManager; _operations = operations; _configuration = configuration; _gitHubTokenProvider = gitHubTokenProvider; @@ -91,16 +94,6 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo throw new GithubApplicationInstallationException($"No installation is available for repository '{normalizedUrl}'"); } - var remoteConfiguration = repoType switch - { - GitRepoType.GitHub => new RemoteTokenProvider( - gitHubToken: await _gitHubTokenProvider.GetTokenForInstallationAsync(installationId)), - GitRepoType.AzureDevOps => new RemoteTokenProvider( - azureDevOpsToken: await _azureDevOpsTokenProvider.GetTokenForRepository(normalizedUrl)), - - _ => throw new NotImplementedException($"Unknown repo url type {normalizedUrl}"), - }; - var gitExe = _localGit.GetPathToLocalGit(); return GitRepoUrlParser.ParseTypeFromUri(normalizedUrl) switch @@ -108,15 +101,15 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo GitRepoType.GitHub => installationId == default ? throw new GithubApplicationInstallationException($"No installation is available for repository '{normalizedUrl}'") : new GitHubClient( - gitExe, - remoteConfiguration.GitHubToken, + new ResolvedTokenProvider(await _gitHubTokenProvider.GetTokenForInstallationAsync(installationId)), + _processManager, logger, temporaryRepositoryRoot, _cache.Cache), GitRepoType.AzureDevOps => new AzureDevOpsClient( - gitExe, - remoteConfiguration.AzureDevOpsToken, + _azureDevOpsTokenProvider, + _processManager, logger, temporaryRepositoryRoot), diff --git a/src/Maestro/SubscriptionActorService/Program.cs b/src/Maestro/SubscriptionActorService/Program.cs index 71363bfb89..e8bff9edfa 100644 --- a/src/Maestro/SubscriptionActorService/Program.cs +++ b/src/Maestro/SubscriptionActorService/Program.cs @@ -8,6 +8,7 @@ using Maestro.MergePolicies; using Microsoft.DncEng.Configuration.Extensions; using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.GitHub.Authentication; using Microsoft.DotNet.Kusto; using Microsoft.DotNet.ServiceFabric.ServiceHost; @@ -39,6 +40,7 @@ private static void Main() public static void Configure(IServiceCollection services) { services.TryAddTransient(sp => sp.GetRequiredService>()); + services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); services.AddSingleton(); services.AddSingleton(); services.AddTransient(); diff --git a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs index 96c081234b..a94452e498 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs @@ -52,8 +52,8 @@ private static IRemoteGitRepo GetRemoteGitClient(ICommandLineOptions options, st { GitRepoType.GitHub => new GitHubClient( - options.GitLocation, - options.GitHubPat, + options.GetGitHubTokenProvider(), + new ProcessManager(logger, options.GitLocation), logger, temporaryRepositoryRoot, // Caching not in use for Darc local client. @@ -61,8 +61,8 @@ private static IRemoteGitRepo GetRemoteGitClient(ICommandLineOptions options, st GitRepoType.AzureDevOps => new AzureDevOpsClient( - options.GitLocation, - options.AzureDevOpsPat, + options.GetAzdoTokenProvider(), + new ProcessManager(logger, options.GitLocation), logger, temporaryRepositoryRoot), diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs index 03d5a4c69b..5f63fff42b 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs @@ -213,13 +213,6 @@ private async Task PromoteBuildAsync(Build build, List targetChann return Constants.SuccessCode; } - if (string.IsNullOrEmpty(_options.AzureDevOpsPat)) - { - Console.WriteLine($"Promoting build {build.Id} with the given parameters would require starting the Build Promotion pipeline, however an AzDO PAT was not found."); - Console.WriteLine("Either specify an AzDO PAT as a parameter or add the --skip-assets-publishing parameter when calling Darc add-build-to-channel."); - return Constants.ErrorCode; - } - var (arcadeSDKSourceBranch, arcadeSDKSourceSHA) = await GetSourceBranchInfoAsync(build).ConfigureAwait(false); // This condition can happen when for some reason we failed to determine the source branch/sha @@ -230,7 +223,7 @@ private async Task PromoteBuildAsync(Build build, List targetChann return Constants.ErrorCode; } - var azdoClient = new AzureDevOpsClient(gitExecutable: null, _options.AzureDevOpsPat, Logger, temporaryRepositoryPath: null); + var azdoClient = Provider.GetRequiredService(); var targetAzdoBuildStatus = await ValidateAzDOBuildAsync(azdoClient, build.AzureDevOpsAccount, build.AzureDevOpsProject, build.AzureDevOpsBuildId.Value) .ConfigureAwait(false); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/AddDependencyOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/AddDependencyOperation.cs index 3b8f88f746..4802fddeaf 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/AddDependencyOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/AddDependencyOperation.cs @@ -24,7 +24,7 @@ public override async Task ExecuteAsync() { DependencyType type = _options.Type.ToLower() == "toolset" ? DependencyType.Toolset : DependencyType.Product; - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); DependencyDetail dependency = new DependencyDetail { diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs index 78eb321223..763df81d8e 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs @@ -79,7 +79,7 @@ public override async Task ExecuteAsync() if (string.IsNullOrWhiteSpace(_options.RepoUri)) { - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); IEnumerable rootDependencies = await local.GetDependenciesAsync(); IEnumerable stripped = rootDependencies.Select(StrippedDependency.GetDependency); foreach (StrippedDependency d in stripped) @@ -231,7 +231,7 @@ private Local HandleRepoAtSpecificHash(string repoPath, string commit, string ma if (Directory.Exists(repoPath)) { Logger.LogDebug($"Repo path {repoPath} already exists, assuming we cloned already and skipping"); - local = new Local(_options.GetRemoteConfiguration(), Logger, repoPath); + local = new Local(_options.GetRemoteTokenProvider(), Logger, repoPath); } else { @@ -239,7 +239,7 @@ private Local HandleRepoAtSpecificHash(string repoPath, string commit, string ma Directory.CreateDirectory(repoPath); File.WriteAllText(Path.Combine(repoPath, ".git"), GetGitDirRedirectString(masterRepoGitDirPath)); Logger.LogInformation($"Checking out {commit} in {repoPath}"); - local = new Local(_options.GetRemoteConfiguration(), Logger, repoPath); + local = new Local(_options.GetRemoteTokenProvider(), Logger, repoPath); local.Checkout(commit, true); } @@ -256,7 +256,7 @@ private async Task HandleMasterCopy(IRemoteFactory remoteFactory, string repoUrl { await HandleMasterCopyWithDefaultGitDir(remoteFactory, repoUrl, masterGitRepoPath, masterRepoGitDirPath); } - var local = new Local(_options.GetRemoteConfiguration(), Logger, masterGitRepoPath); + var local = new Local(_options.GetRemoteTokenProvider(), Logger, masterGitRepoPath); await local.AddRemoteIfMissingAsync(masterGitRepoPath, repoUrl); } @@ -361,7 +361,7 @@ private void HandleMasterCopyWithExistingGitDir(string masterGitRepoPath, string Logger.LogDebug($"Master .gitdir exists and master folder {masterGitRepoPath} does not. Creating master folder."); Directory.CreateDirectory(masterGitRepoPath); File.WriteAllText(Path.Combine(masterGitRepoPath, ".git"), gitDirRedirect); - var masterLocal = new Local(_options.GetRemoteConfiguration(), Logger, masterGitRepoPath); + var masterLocal = new Local(_options.GetRemoteTokenProvider(), Logger, masterGitRepoPath); Logger.LogDebug($"Checking out default commit in {masterGitRepoPath}"); masterLocal.Checkout(null, true); } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependenciesOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependenciesOperation.cs index 6e80501a5b..3045a40a45 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependenciesOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependenciesOperation.cs @@ -23,7 +23,7 @@ public GetDependenciesOperation(GetDependenciesCommandLineOptions options) public override async Task ExecuteAsync() { - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); try { diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs index 3e4c06719d..ac7a35f7d4 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs @@ -27,7 +27,7 @@ public GetDependencyGraphOperation(GetDependencyGraphCommandLineOptions options) { _options = options; _gitClient = new LocalLibGit2Client( - options.GetRemoteConfiguration(), + options.GetRemoteTokenProvider(), new NoTelemetryRecorder(), new ProcessManager(Logger, _options.GitLocation), new FileSystem(), @@ -95,7 +95,7 @@ public override async Task ExecuteAsync() Console.WriteLine($"Getting root dependencies from local repository..."); // Grab root dependency set from local repo - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); rootDependencies = await local.GetDependenciesAsync( _options.AssetName); } @@ -131,7 +131,7 @@ public override async Task ExecuteAsync() { Console.WriteLine($"Getting root dependencies from local repository..."); - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); rootDependencies = await local.GetDependenciesAsync( _options.AssetName); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/Operation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/Operation.cs index 7027f7be39..78008d0a9f 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/Operation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/Operation.cs @@ -3,6 +3,7 @@ using System; using System.Threading.Tasks; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.Arcade.Common; using Microsoft.DotNet.Darc.Helpers; using Microsoft.DotNet.Darc.Options; @@ -47,16 +48,27 @@ protected Operation(ICommandLineOptions options, IServiceCollection? services = .AddConsole(o => o.FormatterName = CompactConsoleLoggerFormatter.FormatterName) .AddConsoleFormatter() .SetMinimumLevel(level)); - + services.AddSingleton(options); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddTransient(sp => ActivatorUtilities.CreateInstance(sp, options.GitLocation)); services.TryAddSingleton(sp => RemoteFactory.GetBarClient(options, sp.GetRequiredService>())); services.TryAddSingleton(sp => sp.GetRequiredService()); - services.TryAddSingleton(options.GetRemoteConfiguration()); services.TryAddTransient(sp => sp.GetRequiredService>()); services.TryAddTransient(); + services.Configure(o => + { + o["default"] = new AzureDevOpsCredentialResolverOptions + { + Token = options.AzureDevOpsPat, + FederatedToken = options.FederatedToken, + DisableInteractiveAuth = options.IsCi, + }; + }); + services.TryAddSingleton(); + services.TryAddSingleton(); + services.TryAddSingleton(); Provider = services.BuildServiceProvider(); Logger = Provider.GetRequiredService>(); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateDependenciesOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateDependenciesOperation.cs index 3688e92862..99140df331 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateDependenciesOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateDependenciesOperation.cs @@ -41,7 +41,7 @@ public override async Task ExecuteAsync() IBarApiClient barClient = Provider.GetRequiredService(); var coherencyUpdateResolver = new CoherencyUpdateResolver(barClient, Logger); - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); List dependenciesToUpdate = []; bool someUpToDate = false; string finalMessage = $"Local dependencies updated from channel '{_options.Channel}'."; diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VerifyOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VerifyOperation.cs index 0a56fe24d1..5f11943d51 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VerifyOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VerifyOperation.cs @@ -26,7 +26,7 @@ public VerifyOperation(VerifyCommandLineOptions options) /// Process exit code. public override async Task ExecuteAsync() { - var local = new Local(_options.GetRemoteConfiguration(), Logger); + var local = new Local(_options.GetRemoteTokenProvider(), Logger); try { diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs index e1b26c132c..cfb1f56b3a 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using CommandLine; +using Maestro.Common; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.DotNet.Darc.Helpers; using Microsoft.DotNet.Darc.Operations; using Microsoft.DotNet.DarcLib; @@ -55,11 +57,25 @@ public abstract class CommandLineOptions : ICommandLineOptions public abstract Operation GetOperation(); - public RemoteTokenProvider GetRemoteConfiguration() + public IRemoteTokenProvider GetRemoteTokenProvider() + => new RemoteTokenProvider(GetAzdoTokenProvider(), GetGitHubTokenProvider()); + + public IAzureDevOpsTokenProvider GetAzdoTokenProvider() { - return new RemoteTokenProvider(GitHubPat, AzureDevOpsPat); + var azdoOptions = new AzureDevOpsTokenProviderOptions + { + ["default"] = new AzureDevOpsCredentialResolverOptions + { + Token = AzureDevOpsPat, + FederatedToken = FederatedToken, + DisableInteractiveAuth = IsCi, + } + }; + return AzureDevOpsTokenProvider.FromStaticOptions(azdoOptions); } + public IRemoteTokenProvider GetGitHubTokenProvider() => new ResolvedTokenProvider(GitHubPat); + public void InitializeFromSettings(ILogger logger) { var localSettings = LocalSettings.GetSettings(this, logger); diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs index 75a3482e3e..697ccd6cc1 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs @@ -1,8 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Maestro.Common; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.DotNet.Darc.Operations; -using Microsoft.DotNet.DarcLib; using Microsoft.Extensions.Logging; namespace Microsoft.DotNet.Darc.Options; @@ -20,7 +21,9 @@ public interface ICommandLineOptions bool IsCi { get; set; } Operation GetOperation(); - RemoteTokenProvider GetRemoteConfiguration(); + IRemoteTokenProvider GetRemoteTokenProvider(); + IAzureDevOpsTokenProvider GetAzdoTokenProvider(); + IRemoteTokenProvider GetGitHubTokenProvider(); /// /// Reads missing options from the local settings. diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Actions/Local.cs b/src/Microsoft.DotNet.Darc/DarcLib/Actions/Local.cs index 14a9151c4f..5f04abefa5 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Actions/Local.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Actions/Local.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Maestro.Common; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models; using Microsoft.Extensions.Logging; @@ -30,11 +31,11 @@ public class Local /// private const string GitExecutable = "git"; - public Local(RemoteTokenProvider remoteConfiguration, ILogger logger, string overrideRootPath = null) + public Local(IRemoteTokenProvider tokenProvider, ILogger logger, string overrideRootPath = null) { _logger = logger; _versionDetailsParser = new VersionDetailsParser(); - _gitClient = new LocalLibGit2Client(remoteConfiguration, new NoTelemetryRecorder(), new ProcessManager(logger, GitExecutable), new FileSystem(), logger); + _gitClient = new LocalLibGit2Client(tokenProvider, new NoTelemetryRecorder(), new ProcessManager(logger, GitExecutable), new FileSystem(), logger); _fileManager = new DependencyFileManager(_gitClient, _versionDetailsParser, logger); _repoRootDir = new(() => overrideRootPath ?? _gitClient.GetRootDirAsync().GetAwaiter().GetResult(), LazyThreadSafetyMode.PublicationOnly); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs index b150ea7196..701e667f04 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs @@ -10,7 +10,9 @@ using System.Text; using System.Text.RegularExpressions; using System.Threading.Tasks; +using Maestro.Common.AzureDevOpsTokens; using Maestro.MergePolicyEvaluation; +using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.AzureDevOps; using Microsoft.DotNet.Services.Utility; using Microsoft.Extensions.Logging; @@ -47,24 +49,25 @@ public class AzureDevOpsClient : RemoteRepoBase, IRemoteGitRepo, IAzureDevOpsCli // Azure DevOps uses this id when creating a new branch as well as when deleting a branch private static readonly string BaseObjectId = "0000000000000000000000000000000000000000"; + + private readonly IAzureDevOpsTokenProvider _tokenProvider; private readonly ILogger _logger; - private readonly string _personalAccessToken; private readonly JsonSerializerSettings _serializerSettings; - /// - /// Create a new azure devops client. - /// - /// - /// PAT for Azure DevOps. This PAT should cover all - /// organizations that may be accessed in a single operation. - /// - /// - /// The AzureDevopsClient currently does not utilize the memory cache - /// - public AzureDevOpsClient(string gitExecutable, string accessToken, ILogger logger, string temporaryRepositoryPath) - : base(gitExecutable, temporaryRepositoryPath, null, logger, new RemoteTokenProvider(azureDevOpsToken: accessToken)) + public AzureDevOpsClient(IAzureDevOpsTokenProvider tokenProvider, IProcessManager processManager, ILogger logger) + : this(tokenProvider, processManager, logger, null) { - _personalAccessToken = accessToken; + } + + public AzureDevOpsClient(IAzureDevOpsTokenProvider tokenProvider, IProcessManager processManager, ILogger logger) + : this(tokenProvider, processManager, (ILogger)logger) + { + } + + public AzureDevOpsClient(IAzureDevOpsTokenProvider tokenProvider, IProcessManager processManager, ILogger logger, string temporaryRepositoryPath) + : base(tokenProvider, processManager, temporaryRepositoryPath, null, logger) + { + _tokenProvider = tokenProvider; _logger = logger; _serializerSettings = new JsonSerializerSettings { @@ -909,7 +912,7 @@ private HttpClient CreateHttpClient(string accountName, string projectName = nul $"application/json;api-version={versionOverride ?? DefaultApiVersion}"); client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue( "Basic", - Convert.ToBase64String(Encoding.ASCII.GetBytes(string.Format("{0}:{1}", "", _personalAccessToken)))); + Convert.ToBase64String(Encoding.ASCII.GetBytes(string.Format("{0}:{1}", "", _tokenProvider.GetTokenForAccount(accountName))))); return client; } @@ -922,7 +925,7 @@ private HttpClient CreateHttpClient(string accountName, string projectName = nul private VssConnection CreateVssConnection(string accountName) { var accountUri = new Uri($"https://dev.azure.com/{accountName}"); - var creds = new VssCredentials(new VssBasicCredential("", _personalAccessToken)); + var creds = new VssCredentials(new VssBasicCredential("", _tokenProvider.GetTokenForAccount(accountName))); return new VssConnection(accountUri, creds); } @@ -1005,18 +1008,16 @@ public static (string accountName, string projectName, string repoName, int id) /// Branch to push to /// Commit message /// - public Task CommitFilesAsync(List filesToCommit, string repoUri, string branch, string commitMessage) - { - return CommitFilesAsync( + public async Task CommitFilesAsync(List filesToCommit, string repoUri, string branch, string commitMessage) + => await CommitFilesAsync( filesToCommit, repoUri, branch, commitMessage, _logger, - _personalAccessToken, + await _tokenProvider.GetTokenForRepositoryAsync(repoUri), "DotNet-Bot", "dn-bot@microsoft.com"); - } /// /// If the release pipeline doesn't have an artifact source a new one is added. diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs index 5b03ad1578..659c427702 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs @@ -20,6 +20,8 @@ using Microsoft.DotNet.Services.Utility; using System.Collections.Immutable; using Maestro.MergePolicyEvaluation; +using Maestro.Common; +using Microsoft.DotNet.DarcLib.Helpers; namespace Microsoft.DotNet.DarcLib; @@ -38,8 +40,8 @@ public class GitHubClient : RemoteRepoBase, IRemoteGitRepo new(@"^/repos/(?[^/]+)/(?[^/]+)/pulls/(?\d+)$"); private readonly Lazy _lazyClient; + private readonly IRemoteTokenProvider _tokenProvider; private readonly ILogger _logger; - private readonly string _personalAccessToken; private readonly JsonSerializerSettings _serializerSettings; private readonly string _userAgent = $"DarcLib-{DarcLibVersion}"; @@ -51,10 +53,24 @@ static GitHubClient() _product = new ProductHeaderValue("DarcLib", version); } - public GitHubClient(string gitExecutable, string accessToken, ILogger logger, string temporaryRepositoryPath, IMemoryCache cache) - : base(gitExecutable, temporaryRepositoryPath, cache, logger, new RemoteTokenProvider(gitHubToken: accessToken, null)) + public GitHubClient( + IRemoteTokenProvider remoteTokenProvider, + IProcessManager processManager, + ILogger logger, + IMemoryCache cache) + : this(remoteTokenProvider, processManager, logger, null, cache) { - _personalAccessToken = accessToken; + } + + public GitHubClient( + IRemoteTokenProvider remoteTokenProvider, + IProcessManager processManager, + ILogger logger, + string temporaryRepositoryPath, + IMemoryCache cache) + : base(remoteTokenProvider, processManager, temporaryRepositoryPath, cache, logger) + { + _tokenProvider = remoteTokenProvider; _logger = logger; _serializerSettings = new JsonSerializerSettings { @@ -773,10 +789,11 @@ private HttpClient CreateHttpClient() { BaseAddress = new Uri(GitHubApiUri) }; - - if (_personalAccessToken != null) + + var token = _tokenProvider.GetTokenForRepository(GitHubApiUri); + if (token != null) { - client.DefaultRequestHeaders.Add("Authorization", $"Token {_personalAccessToken}"); + client.DefaultRequestHeaders.Add("Authorization", $"Token {token}"); } client.DefaultRequestHeaders.Add("User-Agent", _userAgent); @@ -1001,7 +1018,8 @@ private async Task> GetChecksFromChecksApiAsync(string owner, strin private Octokit.GitHubClient CreateGitHubClient() { - if (string.IsNullOrEmpty(_personalAccessToken)) + var token = _tokenProvider.GetTokenForRepository(GitHubApiUri); + if (string.IsNullOrEmpty(token)) { throw new DarcException( "GitHub personal access token is required for this operation. " + @@ -1010,7 +1028,7 @@ private Octokit.GitHubClient CreateGitHubClient() return new Octokit.GitHubClient(_product) { - Credentials = new Credentials(_personalAccessToken) + Credentials = new Credentials(token) }; } @@ -1158,17 +1176,16 @@ public static (string owner, string repo, int id) ParsePullRequestUri(string uri /// Remote repository URI /// Branch to push to /// Commit message - /// - public Task CommitFilesAsync(List filesToCommit, string repoUri, string branch, string commitMessage) + public async Task CommitFilesAsync(List filesToCommit, string repoUri, string branch, string commitMessage) { - return CommitFilesAsync( - filesToCommit, - repoUri, - branch, - commitMessage, - _logger, - _personalAccessToken, - Constants.DarcBotName, + await CommitFilesAsync( + filesToCommit, + repoUri, + branch, + commitMessage, + _logger, + await _tokenProvider.GetTokenForRepositoryAsync(GitHubApiUri), + Constants.DarcBotName, Constants.DarcBotEmail); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitHubTokenProvider.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitHubTokenProvider.cs new file mode 100644 index 0000000000..a4609ce932 --- /dev/null +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitHubTokenProvider.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using Maestro.Common; + +#nullable enable +namespace Microsoft.DotNet.DarcLib; + +public class GitHubTokenProvider(GitHub.Authentication.IGitHubTokenProvider tokenProvider) : IRemoteTokenProvider +{ + public string? GetTokenForRepository(string repoUri) + { + return tokenProvider.GetTokenForRepository(repoUri).GetAwaiter().GetResult(); + } + + public async Task GetTokenForRepositoryAsync(string repoUri) + { + return await tokenProvider.GetTokenForRepository(repoUri); + } +} diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitNativeRepoCloner.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitNativeRepoCloner.cs index c7d2306650..d4bd00b95d 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/GitNativeRepoCloner.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitNativeRepoCloner.cs @@ -52,7 +52,7 @@ private async Task CloneAsync( var args = new List(); var envVars = new Dictionary(); - _localGitClient.AddGitAuthHeader(args, envVars, repoUri); + await _localGitClient.AddGitAuthHeader(args, envVars, repoUri); if (gitDirectory != null) { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitRepoCloner.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitRepoCloner.cs index 76527c5a85..9cd9f468ba 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/GitRepoCloner.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitRepoCloner.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading.Tasks; using LibGit2Sharp; +using Maestro.Common; using Microsoft.Extensions.Logging; #nullable enable @@ -13,13 +14,13 @@ namespace Microsoft.DotNet.DarcLib; public class GitRepoCloner : IGitRepoCloner { - private readonly RemoteTokenProvider _remoteConfiguration; + private readonly IRemoteTokenProvider _remoteTokenProvider; private readonly ILocalLibGit2Client _localGitClient; private readonly ILogger _logger; - public GitRepoCloner(RemoteTokenProvider remoteConfiguration, ILocalLibGit2Client localGitClient, ILogger logger) + public GitRepoCloner(IRemoteTokenProvider remoteTokenProvider, ILocalLibGit2Client localGitClient, ILogger logger) { - _remoteConfiguration = remoteConfiguration; + _remoteTokenProvider = remoteTokenProvider; _localGitClient = localGitClient; _logger = logger; } @@ -60,7 +61,7 @@ private Task CloneAsync(string repoUri, string? commit, string targetDirectory, // The PAT is actually the only thing that matters here, the username // will be ignored. Username = RemoteTokenProvider.GitRemoteUser, - Password = _remoteConfiguration.GetTokenForUri(repoUri), + Password = _remoteTokenProvider.GetTokenForRepository(repoUri), }, }; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitRepoFactory.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitRepoFactory.cs index 58bf97c302..df9c0e4e8f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/GitRepoFactory.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitRepoFactory.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using Maestro.Common; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.Extensions.Logging; @@ -12,9 +14,11 @@ public interface IGitRepoFactory { IGitRepo CreateClient(string repoUri); } + public class GitRepoFactory : IGitRepoFactory { - private readonly RemoteTokenProvider _remoteConfiguration; + private readonly IRemoteTokenProvider _remoteTokenProvider; + private readonly IAzureDevOpsTokenProvider _azdoTokenProvider; private readonly ITelemetryRecorder _telemetryRecorder; private readonly IProcessManager _processManager; private readonly IFileSystem _fileSystem; @@ -22,14 +26,16 @@ public class GitRepoFactory : IGitRepoFactory private readonly string? _temporaryPath = null; public GitRepoFactory( - RemoteTokenProvider remoteConfiguration, + IRemoteTokenProvider remoteTokenProvider, + IAzureDevOpsTokenProvider azdoTokenProvider, ITelemetryRecorder telemetryRecorder, IProcessManager processManager, IFileSystem fileSystem, ILoggerFactory loggerFactory, string temporaryPath) { - _remoteConfiguration = remoteConfiguration; + _remoteTokenProvider = remoteTokenProvider; + _azdoTokenProvider = azdoTokenProvider; _telemetryRecorder = telemetryRecorder; _processManager = processManager; _fileSystem = fileSystem; @@ -40,21 +46,21 @@ public GitRepoFactory( public IGitRepo CreateClient(string repoUri) => GitRepoUrlParser.ParseTypeFromUri(repoUri) switch { GitRepoType.AzureDevOps => new AzureDevOpsClient( - _processManager.GitExecutable, - _remoteConfiguration.AzureDevOpsToken, + _azdoTokenProvider, + _processManager, _loggerFactory.CreateLogger(), _temporaryPath), GitRepoType.GitHub => new GitHubClient( - _processManager.GitExecutable, - _remoteConfiguration.GitHubToken, + _remoteTokenProvider, + _processManager, _loggerFactory.CreateLogger(), _temporaryPath, // Caching not in use for Darc local client. null), GitRepoType.Local => new LocalLibGit2Client( - _remoteConfiguration, + _remoteTokenProvider, _telemetryRecorder, _processManager, _fileSystem, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs index d81e7bd54d..ccda984313 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs @@ -159,7 +159,7 @@ Task StageAsync( /// /// Where to add the new argument into /// Where to add the new variables into - void AddGitAuthHeader(IList args, IDictionary envVars, string repoUri); + Task AddGitAuthHeader(IList args, IDictionary envVars, string repoUri); /// /// Gets a value of a given git configuration setting. diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index 3d3372b666..58bb407cc0 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -9,6 +9,7 @@ using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; +using Maestro.Common; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.Extensions.Logging; @@ -21,14 +22,14 @@ namespace Microsoft.DotNet.DarcLib; /// public class LocalGitClient : ILocalGitClient { - private readonly RemoteTokenProvider _remoteConfiguration; + private readonly IRemoteTokenProvider _remoteConfiguration; private readonly ITelemetryRecorder _telemetryRecorder; private readonly IProcessManager _processManager; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; public LocalGitClient( - RemoteTokenProvider remoteConfiguration, + IRemoteTokenProvider remoteConfiguration, ITelemetryRecorder telemetryRecorder, IProcessManager processManager, IFileSystem fileSystem, @@ -280,14 +281,14 @@ public async Task UpdateRemoteAsync(string repoPath, string remoteName, Cancella List args = [ "remote", "update", remoteName ]; var envVars = new Dictionary(); - AddGitAuthHeader(args, envVars, remoteUri); + await AddGitAuthHeader(args, envVars, remoteUri); result = await _processManager.ExecuteGit(repoPath, args, envVars, cancellationToken: cancellationToken); result.ThrowIfFailed($"Failed to update {repoPath} from remote {remoteName}"); args = [ "fetch", "--tags", "--force", remoteName ]; envVars = []; - AddGitAuthHeader(args, envVars, remoteUri); + await AddGitAuthHeader(args, envVars, remoteUri); result = await _processManager.ExecuteGit(repoPath, args, envVars, cancellationToken: cancellationToken); result.ThrowIfFailed($"Failed to update {repoPath} from remote {remoteName}"); @@ -435,10 +436,9 @@ public async Task BlameLineAsync(string repoPath, string relativeFilePat return result.StandardOutput.Trim().Split(' ').First(); } - public void AddGitAuthHeader(IList args, IDictionary envVars, string repoUri) + public async Task AddGitAuthHeader(IList args, IDictionary envVars, string repoUri) { - var token = _remoteConfiguration.GetTokenForUri(repoUri); - + var token = await _remoteConfiguration.GetTokenForRepositoryAsync(repoUri); if (token == null) { return; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalLibGit2Client.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalLibGit2Client.cs index 22d7cb7081..14d14b6546 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalLibGit2Client.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalLibGit2Client.cs @@ -9,6 +9,7 @@ using System.Text; using System.Threading.Tasks; using LibGit2Sharp; +using Maestro.Common; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.Extensions.Logging; @@ -20,14 +21,19 @@ namespace Microsoft.DotNet.DarcLib; /// public class LocalLibGit2Client : LocalGitClient, ILocalLibGit2Client { - private readonly RemoteTokenProvider _remoteConfiguration; + private readonly IRemoteTokenProvider _remoteTokenProvider; private readonly IProcessManager _processManager; private readonly ILogger _logger; - public LocalLibGit2Client(RemoteTokenProvider remoteConfiguration, ITelemetryRecorder telemetryRecorder, IProcessManager processManager, IFileSystem fileSystem, ILogger logger) - : base(remoteConfiguration, telemetryRecorder, processManager, fileSystem, logger) + public LocalLibGit2Client( + IRemoteTokenProvider remoteTokenProvider, + ITelemetryRecorder telemetryRecorder, + IProcessManager processManager, + IFileSystem fileSystem, + ILogger logger) + : base(remoteTokenProvider, telemetryRecorder, processManager, fileSystem, logger) { - _remoteConfiguration = remoteConfiguration; + _remoteTokenProvider = remoteTokenProvider; _processManager = processManager; _logger = logger; } @@ -350,7 +356,7 @@ public async Task Push( CredentialsProvider = (url, user, cred) => new UsernamePasswordCredentials { - Username = _remoteConfiguration.GetTokenForUri(remoteUrl), + Username = _remoteTokenProvider.GetTokenForRepository(remoteUrl), Password = string.Empty } }; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Microsoft.DotNet.DarcLib.csproj b/src/Microsoft.DotNet.Darc/DarcLib/Microsoft.DotNet.DarcLib.csproj index ffdd86c3b2..d47abaac5b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Microsoft.DotNet.DarcLib.csproj +++ b/src/Microsoft.DotNet.Darc/DarcLib/Microsoft.DotNet.DarcLib.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs index 9b0debd8e6..60ab96ea33 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs @@ -782,7 +782,7 @@ private static async Task GetRepoPathAsync( // If a repo folder or a mapping was not set we use the current parent's // parent folder. var gitClient = new LocalLibGit2Client( - new RemoteTokenProvider(null, null), + new RemoteTokenProvider((string)null, null), new NoTelemetryRecorder(), new ProcessManager(logger, gitExecutable), new FileSystem(), diff --git a/src/Microsoft.DotNet.Darc/DarcLib/RemoteRepoBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/RemoteRepoBase.cs index 72d917f4d7..71f6cf0b82 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/RemoteRepoBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/RemoteRepoBase.cs @@ -1,9 +1,6 @@ // 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.DarcLib.Helpers; -using Microsoft.Extensions.Caching.Memory; -using Microsoft.Extensions.Logging; using System; using System.Collections.Generic; using System.Diagnostics; @@ -11,56 +8,42 @@ using System.Linq; using System.Text; using System.Threading.Tasks; +using Maestro.Common; +using Microsoft.DotNet.DarcLib.Helpers; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; namespace Microsoft.DotNet.DarcLib; public class RemoteRepoBase : GitRepoCloner { private readonly ILogger _logger; - private readonly ProcessManager _processManager; + private readonly IProcessManager _processManager; protected RemoteRepoBase( - string gitExecutable, + IRemoteTokenProvider remoteConfiguration, + IProcessManager processManager, string temporaryRepositoryPath, IMemoryCache cache, - ILogger logger, - RemoteTokenProvider remoteConfiguration) - : this(gitExecutable, temporaryRepositoryPath, cache, logger, new ProcessManager(logger, gitExecutable), remoteConfiguration) - { - } - - private RemoteRepoBase( - string gitExecutable, - string temporaryRepositoryPath, - IMemoryCache cache, - ILogger logger, - ProcessManager processManager, - RemoteTokenProvider remoteConfiguration) + ILogger logger) : base(remoteConfiguration, new LocalLibGit2Client(remoteConfiguration, new NoTelemetryRecorder(), processManager, new FileSystem(), logger), logger) { - TemporaryRepositoryPath = temporaryRepositoryPath; - GitExecutable = gitExecutable; + TemporaryRepositoryPath = temporaryRepositoryPath ?? Path.GetTempPath(); Cache = cache; _logger = logger; _processManager = processManager; } - /// - /// Location of the git executable. Can be "git" or full path to temporary download location - /// used in the Maestro context. - /// - protected string GitExecutable { get; set; } - /// /// Location where repositories should be cloned. /// protected string TemporaryRepositoryPath { get; set; } - + /// /// Generic memory cache that may be supplied by the creator of the /// Remote for the purposes of caching remote responses. /// - protected IMemoryCache Cache { get; set;} + protected IMemoryCache Cache { get; set; } /// /// Cloning big repos takes a considerable amount of time when checking out the files. When diff --git a/src/Microsoft.DotNet.Darc/DarcLib/RemoteTokenProvider.cs b/src/Microsoft.DotNet.Darc/DarcLib/RemoteTokenProvider.cs index f7270300e9..5e586ca3df 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/RemoteTokenProvider.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/RemoteTokenProvider.cs @@ -1,35 +1,73 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -#nullable enable using System; +using System.Threading.Tasks; +using Maestro.Common; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.DotNet.DarcLib.Helpers; +#nullable enable namespace Microsoft.DotNet.DarcLib; -public class RemoteTokenProvider +public class RemoteTokenProvider : IRemoteTokenProvider { - public RemoteTokenProvider(string? gitHubToken = null, string? azureDevOpsToken = null) + private readonly IRemoteTokenProvider _azdoTokenProvider; + private readonly IRemoteTokenProvider _gitHubTokenProvider; + + public RemoteTokenProvider() { - GitHubToken = gitHubToken; - AzureDevOpsToken = azureDevOpsToken; + _azdoTokenProvider = new ResolvedTokenProvider(null); + _gitHubTokenProvider = new ResolvedTokenProvider(null); } - public static string GitRemoteUser => Constants.GitHubBotUserName; + public RemoteTokenProvider( + IRemoteTokenProvider azdoTokenProvider, + IRemoteTokenProvider gitHubTokenProvider) + { + _azdoTokenProvider = azdoTokenProvider; + _gitHubTokenProvider = gitHubTokenProvider; + } - public string? GitHubToken { get; } + public RemoteTokenProvider( + IAzureDevOpsTokenProvider azdoTokenProvider, + string? gitHubToken) + { + _azdoTokenProvider = azdoTokenProvider; + _gitHubTokenProvider = new ResolvedTokenProvider(gitHubToken); + } + public RemoteTokenProvider( + string? azdoToken, + string? gitHubToken) + { + _azdoTokenProvider = new ResolvedTokenProvider(azdoToken); + _gitHubTokenProvider = new ResolvedTokenProvider(gitHubToken); + } - public string? AzureDevOpsToken { get; } + public static string GitRemoteUser => Constants.GitHubBotUserName; + + public async Task GetTokenForRepositoryAsync(string repoUri) + { + var repoType = GitRepoUrlParser.ParseTypeFromUri(repoUri); + + return repoType switch + { + GitRepoType.GitHub => _gitHubTokenProvider.GetTokenForRepository(repoUri), + GitRepoType.AzureDevOps => await _azdoTokenProvider.GetTokenForRepositoryAsync(repoUri), + GitRepoType.Local => null, + _ => throw new NotImplementedException($"Unsupported repository remote {repoUri}"), + }; + } - public string? GetTokenForUri(string repoUri) + public string? GetTokenForRepository(string repoUri) { var repoType = GitRepoUrlParser.ParseTypeFromUri(repoUri); return repoType switch { - GitRepoType.GitHub => GitHubToken, - GitRepoType.AzureDevOps => AzureDevOpsToken, + GitRepoType.GitHub => _gitHubTokenProvider.GetTokenForRepository(repoUri), + GitRepoType.AzureDevOps => _azdoTokenProvider.GetTokenForRepositoryAsync(repoUri).GetAwaiter().GetResult(), GitRepoType.Local => null, _ => throw new NotImplementedException($"Unsupported repository remote {repoUri}"), }; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs index 4e785e2ddb..fa4b0f1d86 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs @@ -5,6 +5,8 @@ using System.IO; using System.Net.Http; using System.Security.Cryptography.X509Certificates; +using Maestro.Common; +using Maestro.Common.AzureDevOpsTokens; using Microsoft.DotNet.Darc.Models.VirtualMonoRepo; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.Extensions.DependencyInjection; @@ -27,7 +29,16 @@ public static IServiceCollection AddVmrManagers( { // Configuration based registrations services.TryAddSingleton(new VmrInfo(Path.GetFullPath(vmrPath), Path.GetFullPath(tmpPath))); - services.TryAddSingleton(new RemoteTokenProvider(gitHubToken, azureDevOpsToken)); + services.TryAddSingleton(sp => + { + if (!string.IsNullOrEmpty(azureDevOpsToken)) + { + return new RemoteTokenProvider(azureDevOpsToken, gitHubToken); + } + + var azdoTokenProvider = sp.GetRequiredService(); + return new RemoteTokenProvider(azdoTokenProvider, gitHubToken); + }); services.TryAddTransient(sp => ActivatorUtilities.CreateInstance(sp, gitLocation)); services.TryAddTransient(sp => sp.GetRequiredService>()); @@ -36,6 +47,7 @@ public static IServiceCollection AddVmrManagers( services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); + services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); @@ -60,6 +72,7 @@ public static IServiceCollection AddVmrManagers( services.TryAddTransient(); services.TryAddScoped(); services.TryAddScoped(); + services.TryAddSingleton(); services.AddHttpClient("GraphQL", httpClient => { diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs index c1fdf1d219..c9ef796199 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs @@ -7,6 +7,7 @@ using ProductConstructionService.Api.Queue; using ProductConstructionService.Api.Controllers; using Microsoft.DotNet.Maestro.Client; +using Maestro.Common.AzureDevOpsTokens; namespace ProductConstructionService.Api.Configuration; @@ -34,9 +35,9 @@ public static string GetRequiredValue(this IConfiguration configuration, string /// Path to the VMR tmp folder /// Uri of the VMR /// Credentials used to authenticate to Azure Resources - /// ConnectionString to the BAR database /// Run service initialization? Currently this just means cloning the VMR /// Add endpoint authentication? + /// Add Swagger UI? /// Uri to used KeyVault public static void ConfigurePcs( this WebApplicationBuilder builder, @@ -68,6 +69,7 @@ public static void ConfigurePcs( }); builder.AddTelemetry(); + builder.Services.Configure("AzureDevOps", (o, s) => s.Bind(o)); builder.AddVmrRegistrations(vmrPath, tmpPath); builder.AddGitHubClientFactory(); builder.AddWorkitemQueues(azureCredential, waitForInitialization: initializeService); diff --git a/src/ProductConstructionService/ProductConstructionService.Api/appsettings.Staging.json b/src/ProductConstructionService/ProductConstructionService.Api/appsettings.Staging.json index 60b13d1ce7..e13aeebd70 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/appsettings.Staging.json +++ b/src/ProductConstructionService/ProductConstructionService.Api/appsettings.Staging.json @@ -21,5 +21,10 @@ "ClientId": "baf98f1b-374e-487d-af42-aa33807f11e4", "UserRole": "User", "RedirectUri": "https://product-construction-int.wittytree-28a89311.westus2.azurecontainerapps.io/signin-oidc" + }, + "AzureDevOps": { + "default": { + "ManagedIdentityId": "system" + } } } diff --git a/test/FeedCleaner.Tests/FeedCleanerServiceTests.cs b/test/FeedCleaner.Tests/FeedCleanerServiceTests.cs index 02bfcdd866..96d19266a8 100644 --- a/test/FeedCleaner.Tests/FeedCleanerServiceTests.cs +++ b/test/FeedCleaner.Tests/FeedCleanerServiceTests.cs @@ -14,6 +14,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Moq; +using NuGet.Common; using NUnit.Framework; namespace FeedCleanerService.Tests; @@ -66,7 +67,10 @@ public void FeedCleanerServiceTests_SetUp() services.Configure( (options) => { - options.Tokens.Add(SomeAccount, "someToken"); + options[SomeAccount] = new() + { + Token = "someToken" + }; }); _provider = services.BuildServiceProvider(); @@ -157,12 +161,7 @@ private Mock SetupAzdoMock() private FeedCleanerService ConfigureFeedCleaner() { - FeedCleanerService cleaner = ActivatorUtilities.CreateInstance(_scope.ServiceProvider); - cleaner.AzureDevOpsClients = new Dictionary - { - { SomeAccount, _azdoMock.Object } - }; - return cleaner; + return ActivatorUtilities.CreateInstance(_scope.ServiceProvider, _azdoMock.Object); } private static void MarkVersionAsDeleted(List packages, string packageName, string version) diff --git a/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs b/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs index da6c7496e3..c8e8c4876a 100644 --- a/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs +++ b/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs @@ -13,6 +13,7 @@ using FluentAssertions; using Maestro.MergePolicyEvaluation; using Maestro.ScenarioTests.ObjectHelpers; +using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.Internal.Testing.Utility; using Microsoft.DotNet.Maestro.Client; using Microsoft.DotNet.Maestro.Client.Models; @@ -20,7 +21,6 @@ using Newtonsoft.Json.Linq; using NuGet.Configuration; using NUnit.Framework; -using Octokit; [assembly: Parallelizable(ParallelScope.Fixtures)] @@ -30,13 +30,13 @@ namespace Maestro.ScenarioTests; internal abstract class MaestroScenarioTestBase { private TestParameters _parameters = null!; - private List _baseDarcRunArgs = new List(); + private List _baseDarcRunArgs = []; protected IMaestroApi MaestroApi => _parameters.MaestroApi; - protected GitHubClient GitHubApi => _parameters.GitHubApi; + protected Octokit.GitHubClient GitHubApi => _parameters.GitHubApi; - protected Microsoft.DotNet.DarcLib.AzureDevOpsClient AzDoClient => _parameters.AzDoClient; + protected AzureDevOpsClient AzDoClient => _parameters.AzDoClient; public void SetTestParameters(TestParameters parameters) { @@ -54,14 +54,14 @@ public void SetTestParameters(TestParameters parameters) } } - protected async Task WaitForPullRequestAsync(string targetRepo, string targetBranch) + protected async Task WaitForPullRequestAsync(string targetRepo, string targetBranch) { - Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); + Octokit.Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); var attempts = 10; while (attempts-- > 0) { - IReadOnlyList prs = await GitHubApi.PullRequest.GetAllForRepository(repo.Id, new PullRequestRequest + IReadOnlyList prs = await GitHubApi.PullRequest.GetAllForRepository(repo.Id, new Octokit.PullRequestRequest { Base = targetBranch, }); @@ -82,10 +82,10 @@ protected async Task WaitForPullRequestAsync(string targetRepo, str throw new MaestroTestException($"No pull request was created in {targetRepo} targeting {targetBranch}"); } - private async Task WaitForUpdatedPullRequestAsync(string targetRepo, string targetBranch, int attempts = 7) + private async Task WaitForUpdatedPullRequestAsync(string targetRepo, string targetBranch, int attempts = 7) { - Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); - PullRequest pr = await WaitForPullRequestAsync(targetRepo, targetBranch); + Octokit.Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); + Octokit.PullRequest pr = await WaitForPullRequestAsync(targetRepo, targetBranch); while (attempts-- > 0) { @@ -104,8 +104,8 @@ private async Task WaitForUpdatedPullRequestAsync(string targetRepo private async Task WaitForMergedPullRequestAsync(string targetRepo, string targetBranch, int attempts = 7) { - Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); - PullRequest pr = await WaitForPullRequestAsync(targetRepo, targetBranch); + Octokit.Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepo); + Octokit.PullRequest pr = await WaitForPullRequestAsync(targetRepo, targetBranch); while (attempts-- > 0) { @@ -160,10 +160,10 @@ private async Task GetAzDoPullRequestIdAsync(string targetRepoName, string private async Task> SearchPullRequestsAsync(string repoUri, string targetPullRequestBranch) { - (var accountName, var projectName, var repoName) = Microsoft.DotNet.DarcLib.AzureDevOpsClient.ParseRepoUri(repoUri); + (var accountName, var projectName, var repoName) = AzureDevOpsClient.ParseRepoUri(repoUri); var query = new StringBuilder(); - Microsoft.DotNet.DarcLib.AzureDevOpsPrStatus prStatus = Microsoft.DotNet.DarcLib.AzureDevOpsPrStatus.Active; + AzureDevOpsPrStatus prStatus = AzureDevOpsPrStatus.Active; query.Append($"searchCriteria.status={prStatus.ToString().ToLower()}"); query.Append($"&searchCriteria.targetRefName=refs/heads/{targetPullRequestBranch}"); @@ -180,10 +180,10 @@ private async Task> SearchPullRequestsAsync(string repoUri, str return prs; } - private async Task> GetAzDoPullRequestAsync(int pullRequestId, string targetRepoName, string targetBranch, bool isUpdated, string? expectedPRTitle = null) + private async Task> GetAzDoPullRequestAsync(int pullRequestId, string targetRepoName, string targetBranch, bool isUpdated, string? expectedPRTitle = null) { var repoUri = GetAzDoRepoUrl(targetRepoName); - (var accountName, var projectName, var repoName) = Microsoft.DotNet.DarcLib.AzureDevOpsClient.ParseRepoUri(repoUri); + (var accountName, var projectName, var repoName) = AzureDevOpsClient.ParseRepoUri(repoUri); var apiBaseUrl = GetAzDoApiRepoUrl(targetRepoName); if (string.IsNullOrEmpty(expectedPRTitle)) @@ -193,7 +193,7 @@ private async Task> SearchPullRequestsAsync(string repoUri, str for (var tries = 10; tries > 0; tries--) { - Microsoft.DotNet.DarcLib.PullRequest pr = await AzDoClient.GetPullRequestAsync($"{apiBaseUrl}/pullRequests/{pullRequestId}"); + PullRequest pr = await AzDoClient.GetPullRequestAsync($"{apiBaseUrl}/pullRequests/{pullRequestId}"); var trimmedTitle = Regex.Replace(pr.Title, @"\s+", " "); if (!isUpdated || trimmedTitle == expectedPRTitle) @@ -227,7 +227,7 @@ private async Task> SearchPullRequestsAsync(string repoUri, str } protected async Task CheckBatchedGitHubPullRequest(string targetBranch, string[] sourceRepoNames, - string targetRepoName, List expectedDependencies, string repoDirectory) + string targetRepoName, List expectedDependencies, string repoDirectory) { var repoNames = sourceRepoNames .Select(name => $"{_parameters.GitHubTestOrg}/{name}") @@ -238,17 +238,17 @@ protected async Task CheckBatchedGitHubPullRequest(string targetBranch, string[] } protected async Task CheckNonBatchedGitHubPullRequest(string sourceRepoName, string targetRepoName, string targetBranch, - List expectedDependencies, string repoDirectory, bool isCompleted = false, bool isUpdated = false) + List expectedDependencies, string repoDirectory, bool isCompleted = false, bool isUpdated = false) { var expectedPRTitle = $"[{targetBranch}] Update dependencies from {_parameters.GitHubTestOrg}/{sourceRepoName}"; await CheckGitHubPullRequest(expectedPRTitle, targetRepoName, targetBranch, expectedDependencies, repoDirectory, isCompleted, isUpdated); } protected async Task CheckGitHubPullRequest(string expectedPRTitle, string targetRepoName, string targetBranch, - List expectedDependencies, string repoDirectory, bool isCompleted, bool isUpdated) + List expectedDependencies, string repoDirectory, bool isCompleted, bool isUpdated) { TestContext.WriteLine($"Checking opened PR in {targetBranch} {targetRepoName}"); - PullRequest pullRequest = isUpdated + Octokit.PullRequest pullRequest = isUpdated ? await WaitForUpdatedPullRequestAsync(targetRepoName, targetBranch) : await WaitForPullRequestAsync(targetRepoName, targetBranch); @@ -271,7 +271,7 @@ protected async Task CheckBatchedAzDoPullRequest( string[] sourceRepoNames, string targetRepoName, string targetBranch, - List expectedDependencies, + List expectedDependencies, string repoDirectory, bool complete = false) { @@ -287,7 +287,7 @@ protected async Task CheckNonBatchedAzDoPullRequest( string sourceRepoName, string targetRepoName, string targetBranch, - List expectedDependencies, + List expectedDependencies, string repoDirectory, bool isCompleted = false, bool isUpdated = false, @@ -303,7 +303,7 @@ protected async Task CheckAzDoPullRequest( string expectedPRTitle, string targetRepoName, string targetBranch, - List expectedDependencies, + List expectedDependencies, string repoDirectory, bool isCompleted, bool isUpdated, @@ -313,12 +313,12 @@ protected async Task CheckAzDoPullRequest( var targetRepoUri = GetAzDoApiRepoUrl(targetRepoName); TestContext.WriteLine($"Checking Opened PR in {targetBranch} {targetRepoUri} ..."); var pullRequestId = await GetAzDoPullRequestIdAsync(targetRepoName, targetBranch); - await using AsyncDisposableValue pullRequest = await GetAzDoPullRequestAsync(pullRequestId, targetRepoName, targetBranch, isUpdated, expectedPRTitle); + await using AsyncDisposableValue pullRequest = await GetAzDoPullRequestAsync(pullRequestId, targetRepoName, targetBranch, isUpdated, expectedPRTitle); var trimmedTitle = Regex.Replace(pullRequest.Value.Title, @"\s+", " "); trimmedTitle.Should().Be(expectedPRTitle); - Microsoft.DotNet.DarcLib.PrStatus expectedPRState = isCompleted ? Microsoft.DotNet.DarcLib.PrStatus.Closed : Microsoft.DotNet.DarcLib.PrStatus.Open; + PrStatus expectedPRState = isCompleted ? PrStatus.Closed : PrStatus.Open; var prStatus = await AzDoClient.GetPullRequestStatusAsync(GetAzDoApiRepoUrl(targetRepoName) + $"/pullRequests/{pullRequestId}"); prStatus.Should().Be(expectedPRState); @@ -340,7 +340,7 @@ protected async Task CheckAzDoPullRequest( } } - private async Task ValidatePullRequestDependencies(string pullRequestBaseBranch, List expectedDependencies, int tries = 1) + private async Task ValidatePullRequestDependencies(string pullRequestBaseBranch, List expectedDependencies, int tries = 1) { var triesRemaining = tries; while (triesRemaining-- > 0) @@ -885,14 +885,14 @@ protected async Task GetRepositoryPolicies(string repoUri, string branch return await RunDarcAsync("get-repository-policies", "--all", "--repo", repoUri, "--branch", branchName); } - protected async Task WaitForMergedPullRequestAsync(string targetRepo, string targetBranch, PullRequest pr, Repository repo, int attempts = 7) + protected async Task WaitForMergedPullRequestAsync(string targetRepo, string targetBranch, Octokit.PullRequest pr, Octokit.Repository repo, int attempts = 7) { while (attempts-- > 0) { TestContext.WriteLine($"Starting check for merge, attempts remaining {attempts}"); pr = await GitHubApi.PullRequest.Get(repo.Id, pr.Number); - if (pr.State == ItemState.Closed) + if (pr.State == Octokit.ItemState.Closed) { return; } @@ -906,18 +906,18 @@ protected async Task WaitForMergedPullRequestAsync(string targetRepo, string tar protected async Task CheckGithubPullRequestChecks(string targetRepoName, string targetBranch) { TestContext.WriteLine($"Checking opened PR in {targetBranch} {targetRepoName}"); - PullRequest pullRequest = await WaitForPullRequestAsync(targetRepoName, targetBranch); - Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepoName); + Octokit.PullRequest pullRequest = await WaitForPullRequestAsync(targetRepoName, targetBranch); + Octokit.Repository repo = await GitHubApi.Repository.Get(_parameters.GitHubTestOrg, targetRepoName); return await ValidateGithubMaestroCheckRunsSuccessful(targetRepoName, targetBranch, pullRequest, repo); } - protected async Task ValidateGithubMaestroCheckRunsSuccessful(string targetRepoName, string targetBranch, PullRequest pullRequest, Repository repo) + protected async Task ValidateGithubMaestroCheckRunsSuccessful(string targetRepoName, string targetBranch, Octokit.PullRequest pullRequest, Octokit.Repository repo) { // Waiting 5 minutes 30 seconds for maestro to add the checks to the PR (it takes 5 minutes for the checks to be added) await Task.Delay(TimeSpan.FromSeconds(5 * 60 + 30)); TestContext.WriteLine($"Checking maestro merge policies check in {targetBranch} {targetRepoName}"); - CheckRunsResponse existingCheckRuns = await GitHubApi.Check.Run.GetAllForReference(repo.Id, pullRequest.Head.Sha); + Octokit.CheckRunsResponse existingCheckRuns = await GitHubApi.Check.Run.GetAllForReference(repo.Id, pullRequest.Head.Sha); var cnt = 0; foreach (var checkRun in existingCheckRuns.CheckRuns) { diff --git a/test/Maestro.ScenarioTests/TestParameters.cs b/test/Maestro.ScenarioTests/TestParameters.cs index cbae51745e..1717cd5f34 100644 --- a/test/Maestro.ScenarioTests/TestParameters.cs +++ b/test/Maestro.ScenarioTests/TestParameters.cs @@ -8,10 +8,12 @@ using System.Reflection; using System.Runtime.InteropServices; using System.Threading.Tasks; +using Maestro.Common.AzureDevOpsTokens; +using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.Internal.Testing.Utility; using Microsoft.DotNet.Maestro.Client; using Microsoft.Extensions.Configuration; -using Octokit; using Octokit.Internal; #nullable enable @@ -67,7 +69,7 @@ public static async Task GetAsync(bool useNonPrimaryEndpoint = f federatedToken: null, disableInteractiveAuth: isCI); - string darcRootDir = darcDir ?? ""; + string? darcRootDir = darcDir; if (string.IsNullOrEmpty(darcRootDir)) { await InstallDarc(maestroApi, testDirSharedWrapper); @@ -75,17 +77,29 @@ public static async Task GetAsync(bool useNonPrimaryEndpoint = f } string darcExe = Path.Join(darcRootDir, RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "darc.exe" : "darc"); + string git = await TestHelpers.Which("git"); + var tokenProvider = AzureDevOpsTokenProvider.FromStaticOptions(new() + { + ["default"] = new() + { + Token = azdoToken + } + }); Assembly assembly = typeof(TestParameters).Assembly; var githubApi = - new GitHubClient( - new ProductHeaderValue(assembly.GetName().Name, assembly.GetCustomAttribute()?.InformationalVersion), - new InMemoryCredentialStore(new Credentials(githubToken))); + new Octokit.GitHubClient( + new Octokit.ProductHeaderValue(assembly.GetName().Name, assembly.GetCustomAttribute()?.InformationalVersion), + new InMemoryCredentialStore(new Octokit.Credentials(githubToken))); var azDoClient = - new Microsoft.DotNet.DarcLib.AzureDevOpsClient(await TestHelpers.Which("git"), azdoToken, new NUnitLogger(), testDirSharedWrapper.TryTake()!.Directory); + new AzureDevOpsClient( + tokenProvider, + new ProcessManager(new NUnitLogger(), git), + new NUnitLogger(), + testDirSharedWrapper.TryTake()!.Directory); return new TestParameters( - darcExe, await TestHelpers.Which("git"), maestroBaseUri, maestroToken, githubToken, maestroApi, githubApi, azDoClient, testDir, azdoToken, isCI); + darcExe, git, maestroBaseUri, maestroToken, githubToken, maestroApi, githubApi, azDoClient, testDir, azdoToken, isCI); } private static async Task InstallDarc(IMaestroApi maestroApi, Shareable toolPath) @@ -110,8 +124,18 @@ private static async Task InstallDarc(IMaestroApi maestroApi, Shareable Date: Fri, 28 Jun 2024 13:27:20 +0200 Subject: [PATCH 03/12] Revert "Restore and re-apply all VMR patches always" (#3691) --- .../DarcLib/LocalGitClient.cs | 21 +- .../VirtualMonoRepo/IVmrPatchHandler.cs | 10 +- .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 27 +- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 112 +++-- .../VirtualMonoRepo/VmrPatchHandler.cs | 80 +--- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 383 +++++++++++------- .../VmrPatchAddedTest.cs | 158 -------- .../VmrSyncRepoChangesTest.cs | 12 +- 8 files changed, 334 insertions(+), 469 deletions(-) delete mode 100644 test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index 58bb407cc0..9a64839ec1 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -392,17 +392,15 @@ public async Task GetStagedFiles(string repoPath) { var args = new List { - "show" + "show", + $"{revision}:{relativeFilePath.TrimStart('/')}" }; - // This somehow stopped working for me - //if (outputPath != null) - //{ - // args.Add("--output"); - // args.Add(outputPath); - //} - - args.Add($"{revision}:{relativeFilePath.TrimStart('/')}"); + if (outputPath != null) + { + args.Add("--output"); + args.Add(outputPath); + } var result = await _processManager.ExecuteGit(repoPath, args); @@ -411,11 +409,6 @@ public async Task GetStagedFiles(string repoPath) return null; } - if (outputPath != null) - { - _fileSystem.WriteToFile(outputPath, result.StandardOutput); - } - return result.StandardOutput; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs index 6724afc363..c0761748f1 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrPatchHandler.cs @@ -39,11 +39,9 @@ Task> CreatePatches( UnixPath? applicationPath, CancellationToken cancellationToken); - Task> GetVmrPatches( - string? patchVersion, - CancellationToken cancellationToken); + IReadOnlyCollection GetVmrPatches(SourceMapping mapping) => GetVmrPatches(mapping.Name); - Task> GetPatchedFiles( - string patchPath, - CancellationToken cancellationToken); + IReadOnlyCollection GetVmrPatches(string mappingName); + + Task> GetPatchedFiles(string patchPath, CancellationToken cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index c44408df65..ef177c9cb3 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -113,8 +114,8 @@ public async Task InitializeRepository( try { IEnumerable updates = initializeDependencies - ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken) - : new[] { rootUpdate }; + ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken) + : new[] { rootUpdate }; foreach (var update in updates) { @@ -194,20 +195,34 @@ private async Task InitializeRepository( string commitMessage = PrepareCommitMessage(InitializationCommitMessage, update.Mapping.Name, update.RemoteUri, newSha: update.TargetRevision); - await StageRepositoryUpdatesAsync( + await UpdateRepoToRevisionAsync( update, clone, additionalRemotes, Constants.EmptyGitObject, + author: null, + commitMessage, + reapplyVmrPatches: true, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); - await ReapplyVmrPatchesAsync(update.Mapping, cancellationToken); - await CommitAsync(commitMessage); - _logger.LogInformation("Initialization of {name} finished", update.Mapping.Name); } + + protected override Task> RestoreVmrPatchedFilesAsync( + SourceMapping mapping, + IReadOnlyCollection patches, + IReadOnlyCollection additionalRemotes, + CancellationToken cancellationToken) + { + // We only need to apply VMR patches that belong to the mapping, nothing to restore from before + IReadOnlyCollection vmrPatchesForMapping = _patchHandler.GetVmrPatches(mapping) + .Select(patch => new VmrIngestionPatch(patch, VmrInfo.GetRelativeRepoSourcesPath(mapping))) + .ToImmutableArray(); + + return Task.FromResult(vmrPatchesForMapping); + } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index 53ff5b77ce..cbdfc0104c 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -68,11 +68,14 @@ protected VmrManagerBase( LocalVmr = localGitRepoFactory.Create(_vmrInfo.VmrPath); } - protected async Task StageRepositoryUpdatesAsync( + public async Task> UpdateRepoToRevisionAsync( VmrDependencyUpdate update, ILocalGitRepo repoClone, IReadOnlyCollection additionalRemotes, string fromRevision, + (string Name, string Email)? author, + string commitMessage, + bool reapplyVmrPatches, string? componentTemplatePath, string? tpnTemplatePath, bool generateCodeowners, @@ -89,6 +92,11 @@ protected async Task StageRepositoryUpdatesAsync( cancellationToken); cancellationToken.ThrowIfCancellationRequested(); + // Get a list of patches that need to be reverted for this update so that repo changes can be applied + // This includes all patches that are also modified by the current change + // (happens when we update repo from which the VMR patches come) + var vmrPatchesToRestore = await RestoreVmrPatchedFilesAsync(update.Mapping, patches, additionalRemotes, cancellationToken); + foreach (var patch in patches) { await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, discardPatches, reverseApply: false, cancellationToken); @@ -117,6 +125,12 @@ protected async Task StageRepositoryUpdatesAsync( cancellationToken.ThrowIfCancellationRequested(); + if (reapplyVmrPatches) + { + await ReapplyVmrPatchesAsync(vmrPatchesToRestore.DistinctBy(p => p.Path).ToArray(), cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); + } + if (tpnTemplatePath != null) { await UpdateThirdPartyNoticesAsync(tpnTemplatePath, cancellationToken); @@ -126,71 +140,46 @@ protected async Task StageRepositoryUpdatesAsync( { await _codeownersGenerator.UpdateCodeowners(cancellationToken); } - } - protected async Task ReapplyVmrPatchesAsync(CancellationToken cancellationToken) - => await ReapplyVmrPatchesAsync(null, cancellationToken); + // Commit without adding files as they were added to index directly + await CommitAsync(commitMessage, author); - protected async Task ReapplyVmrPatchesAsync(SourceMapping? mapping, CancellationToken cancellationToken) - { - _logger.LogInformation("Re-applying VMR patches{for}...", mapping != null ? " for " + mapping.Name : null); - - bool patchesApplied = false; - - foreach (var patch in await _patchHandler.GetVmrPatches(patchVersion: null, cancellationToken)) + // TODO: Workaround for cases when we get CRLF problems on Windows + // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes + // https://github.com/dotnet/arcade-services/issues/3277 + if (reapplyVmrPatches && vmrPatchesToRestore.Any() && RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - if (mapping != null && (!patch.ApplicationPath?.Equals(VmrInfo.SourcesDir / mapping.Name) ?? true)) - { - continue; - } - - await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, removePatchAfter: false, reverseApply: false, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - patchesApplied = true; + await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); } - if (!patchesApplied) + return vmrPatchesToRestore; + } + + protected async Task ReapplyVmrPatchesAsync( + IReadOnlyCollection patches, + CancellationToken cancellationToken) + { + if (patches.Count == 0) { - _logger.LogInformation("No VMR patches to re-apply"); return; } - // TODO: Workaround for cases when we get CRLF problems on Windows - // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes - // https://github.com/dotnet/arcade-services/issues/3277 - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - cancellationToken.ThrowIfCancellationRequested(); - var result = await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["diff", "--exit-code"], - cancellationToken: cancellationToken); + _logger.LogInformation("Re-applying {count} VMR patch{s}...", + patches.Count, + patches.Count > 1 ? "es" : string.Empty); - if (result.ExitCode != 0) + foreach (var patch in patches) + { + if (!_fileSystem.FileExists(patch.Path)) { - cancellationToken.ThrowIfCancellationRequested(); - await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); - - // Sometimes not even checkout helps, so we check again - result = await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["diff", "--exit-code"], - cancellationToken: cancellationToken); - - if (result.ExitCode != 0) - { - cancellationToken.ThrowIfCancellationRequested(); - await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["add", "--u", "."], - cancellationToken: default); - - await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["commit", "--amend", "--no-edit"], - cancellationToken: default); - } + // Patch was removed, so it doesn't exist anymore + _logger.LogDebug("Not re-applying {patch} as it was removed", patch.Path); + continue; } + + await _patchHandler.ApplyPatch(patch, _vmrInfo.VmrPath, false, reverseApply: false, cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); } _logger.LogInformation("VMR patches re-applied back onto the VMR"); @@ -205,14 +194,6 @@ protected async Task CommitAsync(string commitMessage, (string Name, string Emai await _localGitClient.CommitAsync(_vmrInfo.VmrPath, commitMessage, true, author); _logger.LogInformation("Committed in {duration} seconds", (int) watch.Elapsed.TotalSeconds); - - // TODO: Workaround for cases when we get CRLF problems on Windows - // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes - // https://github.com/dotnet/arcade-services/issues/3277 - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); - } } /// @@ -340,12 +321,17 @@ protected async Task UpdateThirdPartyNoticesAsync(string templatePath, Cancellat } } + protected abstract Task> RestoreVmrPatchedFilesAsync( + SourceMapping mapping, + IReadOnlyCollection patches, + IReadOnlyCollection additionalRemotes, + CancellationToken cancellationToken); + /// /// Takes a given commit message template and populates it with given values, URLs and others. /// /// Template into which the values are filled into - /// Repository name - /// Remote URI of the repository + /// Repository mapping /// SHA we are updating from /// SHA we are updating to /// Additional message inserted in the commit body diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs index d668d51aeb..ac6dbdee51 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs @@ -171,13 +171,13 @@ .. mapping.Exclude.Select(GetExclusionRule), || (destination != null && _fileSystem.FileExists(_vmrInfo.VmrPath / destination / fileName))) { path = new UnixPath(fileName); - + var relativeCloneDir = _fileSystem.GetDirectoryName(relativeClonePath) ?? throw new Exception($"Invalid source path {source} in mapping."); - + contentDir = repoPath / relativeCloneDir; } - else if (!_fileSystem.DirectoryExists(contentDir)) + else if(!_fileSystem.DirectoryExists(contentDir)) { // the source can be a file that doesn't exist, then we skip it continue; @@ -199,7 +199,7 @@ .. mapping.Exclude.Select(GetExclusionRule), { return patches; } - + _logger.LogInformation("Creating diffs for submodules of {repo}..", mapping.Name); foreach (var change in submoduleChanges) @@ -418,7 +418,7 @@ public async Task> CreatePatches( sha2, fileName, // Ignore all files except the one we're currently processing - [.. filters?.Except([GetInclusionRule("**/*")])], + [.. filters?.Except([GetInclusionRule("**/*")]) ], true, workingDir, applicationPath, @@ -561,7 +561,7 @@ private async Task> GetPatchesForSubmoduleChange( CancellationToken cancellationToken) { var checkoutCommit = change.Before == Constants.EmptyGitObject ? change.After : change.Before; - var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, cancellationToken); + var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, cancellationToken); // We are only interested in filters specific to submodule's path ImmutableArray GetSubmoduleFilters(IReadOnlyCollection filters) @@ -580,7 +580,7 @@ static string SanitizeName(string mappingName) { mappingName = mappingName[..^4]; } - + return mappingName; } @@ -608,74 +608,20 @@ static string SanitizeName(string mappingName) cancellationToken); } - /// - /// Returns paths to all VMR patches. - /// - /// Version of the VMR patches to get or null if to take current - /// List of VMR patches belonging to the given mapping - public async Task> GetVmrPatches( - string? patchVersion, - CancellationToken cancellationToken) - => patchVersion == null - ? GetWorkingTreeVmrPatches() - : await GetVmrPatchesFromHistory(patchVersion, cancellationToken); - - private IReadOnlyCollection GetWorkingTreeVmrPatches() - { - if (_vmrInfo.PatchesPath is null) - { - return []; - } - - var patchesDir = _vmrInfo.VmrPath / _vmrInfo.PatchesPath; - - if (!_fileSystem.DirectoryExists(patchesDir)) - { - return []; - } - - return _fileSystem - .GetDirectories(_vmrInfo.VmrPath / _vmrInfo.PatchesPath) - .Select(dir => (Mapping: _fileSystem.GetFileName(dir)!, Patches: _fileSystem.GetFiles(dir))) - .SelectMany(pair => pair.Patches.Select(patch => new VmrIngestionPatch(patch, VmrInfo.SourcesDir / pair.Mapping))) - .ToArray(); - } - - private async Task> GetVmrPatchesFromHistory( - string patchVersion, - CancellationToken cancellationToken) + public IReadOnlyCollection GetVmrPatches(string mappingName) { if (_vmrInfo.PatchesPath is null) { - return []; - } - - var result = await _localGitClient.RunGitCommandAsync( - _vmrInfo.VmrPath, - ["ls-tree", "-r", "--name-only", patchVersion, "--", _vmrInfo.PatchesPath], - cancellationToken); - - if (!result.Succeeded) - { - return []; + return Array.Empty(); } - var patches = result.StandardOutput - .Split(new char[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); - - var copiedPatches = new List(); - - foreach (var patch in patches) + var mappingPatchesPath = _vmrInfo.VmrPath / _vmrInfo.PatchesPath / mappingName; + if (!_fileSystem.DirectoryExists(mappingPatchesPath)) { - cancellationToken.ThrowIfCancellationRequested(); - _logger.LogDebug("Found VMR patch {patch}", patch); - var newFileName = _vmrInfo.TmpPath / patch.Replace('/', '_'); - await _localGitClient.GetFileFromGitAsync(_vmrInfo.VmrPath, patch, patchVersion, newFileName); - var mapping = _fileSystem.GetFileName(_fileSystem.GetDirectoryName(patch)!)!; - copiedPatches.Add(new VmrIngestionPatch(newFileName, VmrInfo.SourcesDir / mapping)); + return Array.Empty(); } - return copiedPatches; + return _fileSystem.GetFiles(mappingPatchesPath); } public static string GetInclusionRule(string path) => $":(glob,attr:!{VmrInfo.IgnoreAttribute}){path}"; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index 961fd617a0..d33d305eb7 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -54,12 +55,10 @@ [Recursive sync] {name} / {oldShaShort}{{Constants.Arrow}}{newShaShort} private readonly ISourceManifest _sourceManifest; private readonly IThirdPartyNoticesGenerator _thirdPartyNoticesGenerator; private readonly IComponentListGenerator _readmeComponentListGenerator; + private readonly ILocalGitClient _localGitClient; private readonly IGitRepoFactory _gitRepoFactory; private readonly IWorkBranchFactory _workBranchFactory; - // The VMR SHA before the synchronization has started - private string? _startingVmrSha; - public VmrUpdater( IVmrDependencyTracker dependencyTracker, IVersionDetailsParser versionDetailsParser, @@ -88,6 +87,7 @@ public VmrUpdater( _fileSystem = fileSystem; _thirdPartyNoticesGenerator = thirdPartyNoticesGenerator; _readmeComponentListGenerator = readmeComponentListGenerator; + _localGitClient = localGitClient; _gitRepoFactory = gitRepoFactory; _workBranchFactory = workBranchFactory; } @@ -106,8 +106,6 @@ public async Task UpdateRepository( { await _dependencyTracker.InitializeSourceMappings(); - _startingVmrSha = await LocalVmr.GetGitCommitAsync(cancellationToken); - var mapping = _dependencyTracker.GetMapping(mappingName); // Reload source-mappings.json if it's getting updated @@ -126,44 +124,41 @@ public async Task UpdateRepository( targetVersion, Parent: null); - bool hadUpdates; - try + if (updateDependencies) { - if (updateDependencies) + return await UpdateRepositoryRecursively( + dependencyUpdate, + additionalRemotes, + componentTemplatePath, + tpnTemplatePath, + generateCodeowners, + discardPatches, + cancellationToken); + } + else + { + try { - hadUpdates = await UpdateRepositoriesRecursively( + var patchesToReapply = await UpdateRepositoryInternal( dependencyUpdate, + reapplyVmrPatches: true, additionalRemotes, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); + return true; } - else + catch (EmptySyncException e) { - await RestoreVmrPatchedFilesAsync(cancellationToken); - hadUpdates = await UpdateRepository( - dependencyUpdate, - reapplyVmrPatches: true, - additionalRemotes, - componentTemplatePath, - tpnTemplatePath, - generateCodeowners, - discardPatches, - cancellationToken); + _logger.LogInformation(e.Message); + return false; } } - catch (EmptySyncException e) - { - _logger.LogInformation(e.Message); - return false; - } - - return hadUpdates; } - private async Task UpdateRepository( + private async Task> UpdateRepositoryInternal( VmrDependencyUpdate update, bool reapplyVmrPatches, IReadOnlyCollection additionalRemotes, @@ -187,10 +182,10 @@ await UpdateTargetVersionOnly( update.Mapping, currentVersion, cancellationToken); - return true; + return Array.Empty(); } - return false; + throw new EmptySyncException($"Repository {update.Mapping.Name} is already at {update.TargetRevision}"); } _logger.LogInformation("Synchronizing {name} from {current} to {repo} / {revision}", @@ -231,122 +226,33 @@ await UpdateTargetVersionOnly( _logger.LogInformation("Updating {repo} from {current} to {next}..", update.Mapping.Name, Commit.GetShortSha(currentVersion.Sha), Commit.GetShortSha(update.TargetRevision)); - await StageRepositoryUpdatesAsync( + var commitMessage = PrepareCommitMessage( + SquashCommitMessage, + update.Mapping.Name, + update.RemoteUri, + currentVersion.Sha, + update.TargetRevision); + + return await UpdateRepoToRevisionAsync( update, clone, additionalRemotes, currentVersion.Sha, + author: null, + commitMessage, + reapplyVmrPatches, componentTemplatePath, tpnTemplatePath, generateCodeowners, discardPatches, cancellationToken); - - if (reapplyVmrPatches) - { - await ReapplyVmrPatchesAsync(cancellationToken); - } - - var commitMessage = PrepareCommitMessage( - SquashCommitMessage, - update.Mapping.Name, - update.RemoteUri, - currentVersion.Sha, - update.TargetRevision); - - await CommitAsync(commitMessage); - - return true; - } - /// - /// Updates a repository and all of it's dependencies recursively starting with a given mapping. - /// Always updates to the first version found per repository in the dependency tree. - /// - private async Task UpdateRepositoriesRecursively( - VmrDependencyUpdate rootUpdate, - IReadOnlyCollection additionalRemotes, - string? componentTemplatePath, - string? tpnTemplatePath, - bool generateCodeowners, - bool discardPatches, - CancellationToken cancellationToken) - { - // Synchronization creates commits (one per mapping and some extra) - // on a separate branch that is then merged into original one - var originalRootSha = Commit.GetShortSha(GetCurrentVersion(rootUpdate.Mapping)); - var workBranchName = "sync" + - $"/{rootUpdate.Mapping.Name}" + - $"/{originalRootSha}-{rootUpdate.TargetRevision}"; - IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); - - await RestoreVmrPatchedFilesAsync(cancellationToken); - await CommitAsync("[VMR patches] Removed VMR patches"); - - HashSet updatedDependencies; - try - { - updatedDependencies = await UpdateRepositoryRecursively( - rootUpdate, - additionalRemotes, - componentTemplatePath, - tpnTemplatePath, - generateCodeowners, - discardPatches, - cancellationToken); - } - catch (Exception) - { - _logger.LogWarning( - InterruptedSyncExceptionMessage, - workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") - ? "the original" - : workBranch.OriginalBranch); - throw; - } - - await ReapplyVmrPatchesAsync(cancellationToken); - await CommitAsync("[VMR patches] Re-apply VMR patches"); - await CleanUpRemovedRepos(componentTemplatePath, tpnTemplatePath); - - var summaryMessage = new StringBuilder(); - - foreach (var update in updatedDependencies) - { - if (update.TargetRevision == update.TargetVersion) - { - continue; - } - - var fromShort = Commit.GetShortSha(update.TargetVersion); - var toShort = Commit.GetShortSha(update.TargetRevision); - summaryMessage - .AppendLine($" - {update.Mapping.Name} / {fromShort}{Constants.Arrow}{toShort}") - .AppendLine($" {update.RemoteUri}/compare/{update.TargetVersion}..{update.TargetRevision}"); - } - - var commitMessage = PrepareCommitMessage( - MergeCommitMessage, - rootUpdate.Mapping.Name, - rootUpdate.Mapping.DefaultRemote, - originalRootSha, - rootUpdate.TargetRevision, - summaryMessage.ToString()); - - await workBranch.MergeBackAsync(commitMessage); - - _logger.LogInformation("Recursive update for {repo} finished.{newLine}{message}", - rootUpdate.Mapping.Name, - Environment.NewLine, - summaryMessage); - - return updatedDependencies.Any(); } /// /// Updates a repository and all of it's dependencies recursively starting with a given mapping. /// Always updates to the first version found per repository in the dependency tree. /// - private async Task> UpdateRepositoryRecursively( + private async Task UpdateRepositoryRecursively( VmrDependencyUpdate rootUpdate, IReadOnlyCollection additionalRemotes, string? componentTemplatePath, @@ -376,6 +282,16 @@ private async Task> UpdateRepositoryRecursively( string.Join(separator, extraneousMappings)); } + // Synchronization creates commits (one per mapping and some extra) on a separate branch that is then merged into original one + + var workBranchName = "sync" + + $"/{rootUpdate.Mapping.Name}" + + $"/{Commit.GetShortSha(GetCurrentVersion(rootUpdate.Mapping))}-{rootUpdate.TargetRevision}"; + IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); + + // Collection of all affected VMR patches we will need to restore after the sync + var vmrPatchesToReapply = new List(); + // Dependencies that were already updated during this run var updatedDependencies = new HashSet(); @@ -410,11 +326,12 @@ private async Task> UpdateRepositoryRecursively( update.TargetRevision); } + IReadOnlyCollection patchesToReapply; try { - await UpdateRepository( + patchesToReapply = await UpdateRepositoryInternal( update, - reapplyVmrPatches: false, + false, additionalRemotes, componentTemplatePath, tpnTemplatePath, @@ -440,6 +357,17 @@ await UpdateRepository( _logger.LogWarning(e.Message); continue; } + catch (Exception) + { + _logger.LogWarning( + InterruptedSyncExceptionMessage, + workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") + ? "the original" + : workBranch.OriginalBranch); + throw; + } + + vmrPatchesToReapply.AddRange(patchesToReapply); updatedDependencies.Add(update with { @@ -451,31 +379,136 @@ await UpdateRepository( }); } - return updatedDependencies; + string finalRootSha = GetCurrentVersion(rootUpdate.Mapping); + var summaryMessage = new StringBuilder(); + + foreach (var update in updatedDependencies) + { + if (update.TargetRevision == update.TargetVersion) + { + continue; + } + + var fromShort = Commit.GetShortSha(update.TargetVersion); + var toShort = Commit.GetShortSha(update.TargetRevision); + summaryMessage + .AppendLine($" - {update.Mapping.Name} / {fromShort}{Constants.Arrow}{toShort}") + .AppendLine($" {update.RemoteUri}/compare/{update.TargetVersion}..{update.TargetRevision}"); + } + + if (vmrPatchesToReapply.Any()) + { + try + { + await ReapplyVmrPatchesAsync(vmrPatchesToReapply.DistinctBy(p => p.Path).ToArray(), cancellationToken); + } + catch (Exception) + { + _logger.LogWarning( + InterruptedSyncExceptionMessage, + workBranch.OriginalBranch.StartsWith("sync") || workBranch.OriginalBranch.StartsWith("init") + ? "the original" + : workBranch.OriginalBranch); + throw; + } + + await CommitAsync("[VMR patches] Re-apply VMR patches"); + + // TODO: Workaround for cases when we get CRLF problems on Windows + // We should figure out why restoring and reapplying VMR patches leaves working tree with EOL changes + // https://github.com/dotnet/arcade-services/issues/3277 + if (vmrPatchesToReapply.Any() && RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + cancellationToken.ThrowIfCancellationRequested(); + var result = await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["diff", "--exit-code"], + cancellationToken: cancellationToken); + + if (result.ExitCode != 0) + { + cancellationToken.ThrowIfCancellationRequested(); + await _localGitClient.CheckoutAsync(_vmrInfo.VmrPath, "."); + + // Sometimes not even checkout helps, so we check again + result = await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["diff", "--exit-code"], + cancellationToken: cancellationToken); + + if (result.ExitCode != 0) + { + cancellationToken.ThrowIfCancellationRequested(); + await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["add", "--u", "."], + cancellationToken: default); + + await _localGitClient.RunGitCommandAsync( + _vmrInfo.VmrPath, + ["commit", "--amend", "--no-edit"], + cancellationToken: default); + } + } + } + } + + await CleanUpRemovedRepos(componentTemplatePath, tpnTemplatePath); + + var commitMessage = PrepareCommitMessage( + MergeCommitMessage, + rootUpdate.Mapping.Name, + rootUpdate.Mapping.DefaultRemote, + originalRootSha, + finalRootSha, + summaryMessage.ToString()); + + await workBranch.MergeBackAsync(commitMessage); + + _logger.LogInformation("Recursive update for {repo} finished.{newLine}{message}", + rootUpdate.Mapping.Name, + Environment.NewLine, + summaryMessage); + + return updatedDependencies.Any(); } /// - /// Removes all VMR patches from the working tree so that repository changes can be ingested. + /// Detects VMR patches affected by a given set of patches and restores files patched by these + /// VMR patches into their original state. + /// Detects whether patched files are coming from a mapped repository or a submodule too. /// - private async Task RestoreVmrPatchedFilesAsync(CancellationToken cancellationToken) + /// Mapping that is currently being updated (so we get its patches) + /// Patches with incoming changes to be checked whether they affect some VMR patch + protected override async Task> RestoreVmrPatchedFilesAsync( + SourceMapping updatedMapping, + IReadOnlyCollection patches, + IReadOnlyCollection additionalRemotes, + CancellationToken cancellationToken) { - _logger.LogInformation("Restoring all VMR patches before we ingest new changes..."); - - IReadOnlyCollection vmrPatchesToRestore = await _patchHandler.GetVmrPatches(_startingVmrSha, cancellationToken); + IReadOnlyCollection vmrPatchesToRestore = await GetVmrPatchesToRestore( + updatedMapping, + patches, + cancellationToken); if (vmrPatchesToRestore.Count == 0) { - _logger.LogInformation("No VMR patches found"); - return; + return vmrPatchesToRestore; } foreach (var patch in vmrPatchesToRestore) { - _logger.LogInformation("Restoring VMR patch {patch}", patch.Path); + if (!_fileSystem.FileExists(patch.Path)) + { + // Patch is being added, so it doesn't exist yet + _logger.LogDebug("Not restoring {patch} as it will be added during the sync", patch.Path); + continue; + } + await _patchHandler.ApplyPatch( patch, - _vmrInfo.VmrPath / (patch.ApplicationPath ?? string.Empty), - removePatchAfter: true, + _vmrInfo.VmrPath / (patch.ApplicationPath ?? ""), + removePatchAfter: false, reverseApply: true, cancellationToken); } @@ -484,6 +517,68 @@ await _patchHandler.ApplyPatch( await LocalVmr.ResetWorkingTree(); _logger.LogInformation("Files affected by VMR patches restored"); + + return vmrPatchesToRestore; + } + + /// + /// Gets a list of VMR patches that need to be reverted for a given mapping update so that repo changes can be applied. + /// Usually, this just returns all VMR patches for that given mapping (e.g. for the aspnetcore returns all aspnetcore only patches). + /// + /// One exception is when the updated mapping is the one that the VMR patches come from into the VMR (e.g. dotnet/installer). + /// In this case, we also check which VMR patches are modified by the change and we also returns those. + /// Examples: + /// - An aspnetcore VMR patch is removed from installer - we must remove it from the files it is applied to in the VMR. + /// - A new version of patch is synchronized from installer - we must remove the old version and apply the new. + /// + /// Currently synchronized mapping + /// Patches of currently synchronized changes + private async Task> GetVmrPatchesToRestore( + SourceMapping updatedMapping, + IReadOnlyCollection patches, + CancellationToken cancellationToken) + { + _logger.LogInformation("Getting a list of VMR patches to restore for {repo} before we ingest new changes...", updatedMapping.Name); + + var patchesToRestore = new List(); + + // Always restore all patches belonging to the currently updated mapping + foreach (var vmrPatch in _patchHandler.GetVmrPatches(updatedMapping)) + { + patchesToRestore.Add(new VmrIngestionPatch(vmrPatch, updatedMapping)); + } + + // If we are not updating the mapping that the VMR patches come from, we're done + if (_vmrInfo.PatchesPath == null || !_vmrInfo.PatchesPath.StartsWith(VmrInfo.GetRelativeRepoSourcesPath(updatedMapping))) + { + return patchesToRestore; + } + + _logger.LogInformation("Repo {repo} contains VMR patches, checking which VMR patches have changes...", updatedMapping.Name); + + // Check which files are modified by every of the patches that bring new changes into the VMR + foreach (var patch in patches) + { + cancellationToken.ThrowIfCancellationRequested(); + + IReadOnlyCollection patchedFiles = await _patchHandler.GetPatchedFiles(patch.Path, cancellationToken); + IEnumerable affectedPatches = patchedFiles + .Select(path => VmrInfo.GetRelativeRepoSourcesPath(updatedMapping) / path) + .Where(path => path.Path.StartsWith(_vmrInfo.PatchesPath) && path.Path.EndsWith(".patch")) + .Select(path => _vmrInfo.VmrPath / path); + + foreach (LocalPath affectedPatch in affectedPatches) + { + // patch is in the folder named as the mapping for which it is applied + var affectedRepo = affectedPatch.Path.Split(_fileSystem.DirectorySeparatorChar)[^2]; + var affectedMapping = _dependencyTracker.GetMapping(affectedRepo); + + _logger.LogInformation("Detected a change of a VMR patch {patch} for {repo}", affectedPatch, affectedRepo); + patchesToRestore.Add(new VmrIngestionPatch(affectedPatch, affectedMapping)); + } + } + + return patchesToRestore; } private string GetCurrentVersion(SourceMapping mapping) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs deleted file mode 100644 index 3cb4e7f773..0000000000 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrPatchAddedTest.cs +++ /dev/null @@ -1,158 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.IO; -using System.Threading.Tasks; -using Microsoft.DotNet.DarcLib; -using Microsoft.DotNet.DarcLib.Helpers; -using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; -using Microsoft.DotNet.DarcLib.VirtualMonoRepo; -using NUnit.Framework; - -namespace Microsoft.DotNet.Darc.Tests.VirtualMonoRepo; - -[TestFixture] -internal class VmrPatchAddedTest : VmrTestsBase -{ - [Test] - public async Task PatchesAreAppliedTest() - { - var newPatchFileName = "new-patch.patch"; - var vmrPatchesDir = VmrPath / VmrInfo.RelativeSourcesDir / Constants.InstallerRepoName / Constants.PatchesFolderName / Constants.ProductRepoName; - var patchPathInVmr = vmrPatchesDir / newPatchFileName; - var installerPatchesDir = InstallerRepoPath / Constants.PatchesFolderName / Constants.ProductRepoName; - var installerFilePathInVmr = VmrPath / VmrInfo.RelativeSourcesDir / Constants.InstallerRepoName / Constants.GetRepoFileName(Constants.InstallerRepoName); - var productRepoFilePathInVmr = VmrPath / VmrInfo.RelativeSourcesDir / Constants.ProductRepoName / Constants.GetRepoFileName(Constants.ProductRepoName); - - await File.WriteAllTextAsync(ProductRepoPath / Constants.GetRepoFileName(Constants.ProductRepoName), - """ - File in the test repo - patches will change the next lines - AAA - CCC - end of changes - """); - await GitOperations.CommitAll(ProductRepoPath, "Change file to CCC"); - - // Update dependent repo to the last commit - var productRepoSha = await GitOperations.GetRepoLastCommit(ProductRepoPath); - var productRepoDependency = string.Format( - Constants.DependencyTemplate, - Constants.ProductRepoName, ProductRepoPath, productRepoSha); - - var versionDetails = string.Format( - Constants.VersionDetailsTemplate, - productRepoDependency); - - await File.WriteAllTextAsync(InstallerRepoPath / VersionFiles.VersionDetailsXml, versionDetails); - await GitOperations.CommitAll(InstallerRepoPath, "Bump product repo to latest"); - - await InitializeRepoAtLastCommit(Constants.InstallerRepoName, InstallerRepoPath); - - CheckFileContents(productRepoFilePathInVmr, "AAA", "CCC"); - - await File.WriteAllTextAsync(ProductRepoPath / Constants.GetRepoFileName(Constants.ProductRepoName), - """ - File in the test repo - patches will change the next lines - AAA - BBB - end of changes - """); - await GitOperations.CommitAll(ProductRepoPath, "Change file to BBB"); - - var expectedFilesFromRepos = new List - { - productRepoFilePathInVmr, - installerFilePathInVmr - }; - - var expectedFiles = GetExpectedFilesInVmr( - VmrPath, - [Constants.ProductRepoName, Constants.InstallerRepoName], - expectedFilesFromRepos - ); - - CheckDirectoryContents(VmrPath, expectedFiles); - - // Add a new patch in installer - File.Copy(VmrTestsOneTimeSetUp.ResourcesPath / newPatchFileName, installerPatchesDir / newPatchFileName); - - // Update dependent repo to the last commit - productRepoSha = await GitOperations.GetRepoLastCommit(ProductRepoPath); - productRepoDependency = string.Format( - Constants.DependencyTemplate, - Constants.ProductRepoName, ProductRepoPath, productRepoSha); - - versionDetails = string.Format( - Constants.VersionDetailsTemplate, - productRepoDependency); - - File.WriteAllText(InstallerRepoPath / VersionFiles.VersionDetailsXml, versionDetails); - - await GitOperations.CommitAll(InstallerRepoPath, "Add a new patch file"); - await UpdateRepoToLastCommit(Constants.InstallerRepoName, InstallerRepoPath); - - // Now we sync installer which means new patch + change in the product repo - // We must check that the patch is detected as being added and won't be restored during repo's sync - // The file will have AAA CCC in the beginning - // The repo change will change it to AAA BBB - // Then the patch will change it to TTT BBB - // If we tried to restore the patch before we sync the repo, the patch fails - // because it will find AAA CCC instead of AAA BBB (which it expects) - await UpdateRepoToLastCommit(Constants.InstallerRepoName, InstallerRepoPath); - - expectedFiles.Add(patchPathInVmr); - CheckDirectoryContents(VmrPath, expectedFiles); - CheckFileContents(productRepoFilePathInVmr, "TTT", "BBB"); - } - - protected override async Task CopyReposForCurrentTest() - { - var dependenciesMap = new Dictionary> - { - { Constants.InstallerRepoName, new List { Constants.ProductRepoName } }, - }; - - await CopyRepoAndCreateVersionFiles(Constants.InstallerRepoName, dependenciesMap); - } - - protected override async Task CopyVmrForCurrentTest() - { - CopyDirectory(VmrTestsOneTimeSetUp.CommonVmrPath, VmrPath); - - var sourceMappings = new SourceMappingFile() - { - Mappings = - [ - new SourceMappingSetting - { - Name = Constants.InstallerRepoName, - DefaultRemote = InstallerRepoPath - }, - new SourceMappingSetting - { - Name = Constants.ProductRepoName, - DefaultRemote = ProductRepoPath - } - ], - PatchesPath = "src/installer/patches/" - }; - - await WriteSourceMappingsInVmr(sourceMappings); - } - - private static void CheckFileContents(NativePath filePath, string line1, string line2) - { - CheckFileContents(filePath, - $""" - File in the test repo - patches will change the next lines - {line1} - {line2} - end of changes - """ - ); - } -} diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs index eff9b9bbaa..e20a2d6f2b 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs @@ -219,18 +219,8 @@ public async Task SubmodulesAreInlinedProperlyTest() // Remove submodule await GitOperations.RemoveSubmodule(ProductRepoPath, submoduleRelativePath); + await File.WriteAllTextAsync(VmrPath / VmrInfo.CodeownersPath, "My new content in the CODEOWNERS\n\n### CONTENT BELOW IS AUTO-GENERATED AND MANUAL CHANGES WILL BE OVERWRITTEN ###\n"); await GitOperations.CommitAll(ProductRepoPath, "Remove the submodule"); - - // Change codeowners - - await File.WriteAllTextAsync(VmrPath / VmrInfo.CodeownersPath, - """ - My new content in the CODEOWNERS - - ### CONTENT BELOW IS AUTO-GENERATED AND MANUAL CHANGES WILL BE OVERWRITTEN ### - """); - await GitOperations.CommitAll(VmrPath, "Updated codeowners"); - await UpdateRepoToLastCommit(Constants.ProductRepoName, ProductRepoPath, generateCodeowners: true); expectedFiles.Remove(submoduleFilePath); From 3eed7ecd715c8d6720c297d457c744827306aa20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Fri, 28 Jun 2024 13:52:01 +0200 Subject: [PATCH 04/12] Fix binding of AzDO configuration (#3692) --- src/Maestro/Maestro.Web/Startup.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Maestro/Maestro.Web/Startup.cs b/src/Maestro/Maestro.Web/Startup.cs index 07dbe7ec2a..ea4e4fcf3d 100644 --- a/src/Maestro/Maestro.Web/Startup.cs +++ b/src/Maestro/Maestro.Web/Startup.cs @@ -234,9 +234,10 @@ public override void ConfigureServices(IServiceCollection services) ?.InformationalVersion); }); services.Configure(Configuration.GetSection("GitHub")); + + services.Configure(Configuration.GetSection("AzureDevOps")); services.AddAzureDevOpsTokenProvider(); - services.Configure("AzureDevOps", Configuration); services.AddKustoClientProvider("Kusto"); // We do not use AddMemoryCache here. We use our own cache because we wish to From 719494ffcf616d971ec2597239f772aa08f2efbd Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 17:21:35 +0000 Subject: [PATCH 05/12] [main] Update dependencies from dotnet/dnceng (#3697) [main] Update dependencies from dotnet/dnceng --- .config/dotnet-tools.json | 4 ++-- eng/Version.Details.xml | 8 ++++---- eng/Versions.props | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 5aa9bf38b7..a3f4123928 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -3,7 +3,7 @@ "isRoot": true, "tools": { "microsoft.dnceng.secretmanager": { - "version": "1.1.0-beta.24321.4", + "version": "1.1.0-beta.24325.1", "commands": [ "secret-manager" ] @@ -15,7 +15,7 @@ ] }, "microsoft.dnceng.configuration.bootstrap": { - "version": "1.1.0-beta.24321.4", + "version": "1.1.0-beta.24325.1", "commands": [ "bootstrap-dnceng-configuration" ] diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index f17b8329a5..46a131668f 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -115,13 +115,13 @@ https://github.com/dotnet/arcade 748cd976bf8b0f69b809e569943635ab8be36dc8 - + https://github.com/dotnet/dnceng - 44c25f86ba374d93ba9c22f12552deeed2221588 + b6850078ffe074f4ad7d4952486d4997b1e029b6 - + https://github.com/dotnet/dnceng - 44c25f86ba374d93ba9c22f12552deeed2221588 + b6850078ffe074f4ad7d4952486d4997b1e029b6 diff --git a/eng/Versions.props b/eng/Versions.props index 1e39a902b6..5b71eb6ebe 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -37,8 +37,8 @@ 1.1.0-beta.24319.1 1.1.0-beta.24319.1 1.1.0-beta.24319.1 - 1.1.0-beta.24321.4 - 1.1.0-beta.24321.4 + 1.1.0-beta.24325.1 + 1.1.0-beta.24325.1 From cd47a8efd2225fcd5d06de8d05b84acaa1d1fb5e Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Mon, 1 Jul 2024 19:26:27 +0200 Subject: [PATCH 06/12] [main] Update dependencies from dotnet/arcade (#3696) Co-authored-by: dotnet-maestro[bot] --- eng/Version.Details.xml | 24 ++++++++-------- eng/Versions.props | 10 +++---- eng/common/post-build/publish-using-darc.ps1 | 15 +++++----- .../job/publish-build-assets.yml | 12 ++++---- .../post-build/post-build.yml | 8 ++++-- .../templates/job/publish-build-assets.yml | 12 ++++---- .../templates/post-build/post-build.yml | 8 ++++-- .../post-build/setup-maestro-vars.yml | 28 +++++++++---------- global.json | 2 +- 9 files changed, 63 insertions(+), 56 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 46a131668f..9f204ff1dd 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -91,29 +91,29 @@ - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e - + https://github.com/dotnet/arcade - 748cd976bf8b0f69b809e569943635ab8be36dc8 + 761c516b64fee3941d8909d24205ced835eed83e https://github.com/dotnet/dnceng diff --git a/eng/Versions.props b/eng/Versions.props index 5b71eb6ebe..dc143fd509 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -9,11 +9,11 @@ true 1.0.0-preview.1 - 8.0.0-beta.24324.1 - 8.0.0-beta.24324.1 - 8.0.0-beta.24324.1 - 8.0.0-beta.24324.1 - 8.0.0-beta.24324.1 + 8.0.0-beta.24328.2 + 8.0.0-beta.24328.2 + 8.0.0-beta.24328.2 + 8.0.0-beta.24328.2 + 8.0.0-beta.24328.2 17.4.1 1.1.0-beta.24319.1 1.1.0-beta.24319.1 diff --git a/eng/common/post-build/publish-using-darc.ps1 b/eng/common/post-build/publish-using-darc.ps1 index 5a3a32ea8d..238945cb5a 100644 --- a/eng/common/post-build/publish-using-darc.ps1 +++ b/eng/common/post-build/publish-using-darc.ps1 @@ -2,7 +2,6 @@ param( [Parameter(Mandatory=$true)][int] $BuildId, [Parameter(Mandatory=$true)][int] $PublishingInfraVersion, [Parameter(Mandatory=$true)][string] $AzdoToken, - [Parameter(Mandatory=$true)][string] $MaestroToken, [Parameter(Mandatory=$false)][string] $MaestroApiEndPoint = 'https://maestro.dot.net', [Parameter(Mandatory=$true)][string] $WaitPublishingFinish, [Parameter(Mandatory=$false)][string] $ArtifactsPublishingAdditionalParameters, @@ -31,13 +30,13 @@ try { } & $darc add-build-to-channel ` - --id $buildId ` - --publishing-infra-version $PublishingInfraVersion ` - --default-channels ` - --source-branch main ` - --azdev-pat $AzdoToken ` - --bar-uri $MaestroApiEndPoint ` - --password $MaestroToken ` + --id $buildId ` + --publishing-infra-version $PublishingInfraVersion ` + --default-channels ` + --source-branch main ` + --azdev-pat "$AzdoToken" ` + --bar-uri "$MaestroApiEndPoint" ` + --ci ` @optionalParams if ($LastExitCode -ne 0) { diff --git a/eng/common/templates-official/job/publish-build-assets.yml b/eng/common/templates-official/job/publish-build-assets.yml index 589ac80a18..d01739c128 100644 --- a/eng/common/templates-official/job/publish-build-assets.yml +++ b/eng/common/templates-official/job/publish-build-assets.yml @@ -76,13 +76,16 @@ jobs: - task: NuGetAuthenticate@1 - - task: PowerShell@2 + - task: AzureCLI@2 displayName: Publish Build Assets inputs: - filePath: eng\common\sdk-task.ps1 - arguments: -task PublishBuildAssets -restore -msbuildEngine dotnet + azureSubscription: "Darc: Maestro Production" + scriptType: ps + scriptLocation: scriptPath + scriptPath: $(Build.SourcesDirectory)/eng/common/sdk-task.ps1 + arguments: > + -task PublishBuildAssets -restore -msbuildEngine dotnet /p:ManifestsPath='$(Build.StagingDirectory)/Download/AssetManifests' - /p:BuildAssetRegistryToken=$(MaestroAccessToken) /p:MaestroApiEndpoint=https://maestro-prod.westus2.cloudapp.azure.com /p:PublishUsingPipelines=${{ parameters.publishUsingPipelines }} /p:OfficialBuildId=$(Build.BuildNumber) @@ -144,7 +147,6 @@ jobs: arguments: -BuildId $(BARBuildId) -PublishingInfraVersion 3 -AzdoToken '$(publishing-dnceng-devdiv-code-r-build-re)' - -MaestroToken '$(MaestroApiAccessToken)' -WaitPublishingFinish true -ArtifactsPublishingAdditionalParameters '${{ parameters.artifactsPublishingAdditionalParameters }}' -SymbolPublishingAdditionalParameters '${{ parameters.symbolPublishingAdditionalParameters }}' diff --git a/eng/common/templates-official/post-build/post-build.yml b/eng/common/templates-official/post-build/post-build.yml index da1f40958b..0dfa387e7b 100644 --- a/eng/common/templates-official/post-build/post-build.yml +++ b/eng/common/templates-official/post-build/post-build.yml @@ -272,14 +272,16 @@ stages: - task: NuGetAuthenticate@1 - - task: PowerShell@2 + - task: AzureCLI@2 displayName: Publish Using Darc inputs: - filePath: $(Build.SourcesDirectory)/eng/common/post-build/publish-using-darc.ps1 + azureSubscription: "Darc: Maestro Production" + scriptType: ps + scriptLocation: scriptPath + scriptPath: $(Build.SourcesDirectory)/eng/common/post-build/publish-using-darc.ps1 arguments: -BuildId $(BARBuildId) -PublishingInfraVersion ${{ parameters.publishingInfraVersion }} -AzdoToken '$(publishing-dnceng-devdiv-code-r-build-re)' - -MaestroToken '$(MaestroApiAccessToken)' -WaitPublishingFinish true -ArtifactsPublishingAdditionalParameters '${{ parameters.artifactsPublishingAdditionalParameters }}' -SymbolPublishingAdditionalParameters '${{ parameters.symbolPublishingAdditionalParameters }}' diff --git a/eng/common/templates/job/publish-build-assets.yml b/eng/common/templates/job/publish-build-assets.yml index 8ec0151def..9fd69fa7c9 100644 --- a/eng/common/templates/job/publish-build-assets.yml +++ b/eng/common/templates/job/publish-build-assets.yml @@ -74,13 +74,16 @@ jobs: - task: NuGetAuthenticate@1 - - task: PowerShell@2 + - task: AzureCLI@2 displayName: Publish Build Assets inputs: - filePath: eng\common\sdk-task.ps1 - arguments: -task PublishBuildAssets -restore -msbuildEngine dotnet + azureSubscription: "Darc: Maestro Production" + scriptType: ps + scriptLocation: scriptPath + scriptPath: $(Build.SourcesDirectory)/eng/common/sdk-task.ps1 + arguments: > + -task PublishBuildAssets -restore -msbuildEngine dotnet /p:ManifestsPath='$(Build.StagingDirectory)/Download/AssetManifests' - /p:BuildAssetRegistryToken=$(MaestroAccessToken) /p:MaestroApiEndpoint=https://maestro.dot.net /p:PublishUsingPipelines=${{ parameters.publishUsingPipelines }} /p:OfficialBuildId=$(Build.BuildNumber) @@ -140,7 +143,6 @@ jobs: arguments: -BuildId $(BARBuildId) -PublishingInfraVersion 3 -AzdoToken '$(publishing-dnceng-devdiv-code-r-build-re)' - -MaestroToken '$(MaestroApiAccessToken)' -WaitPublishingFinish true -ArtifactsPublishingAdditionalParameters '${{ parameters.artifactsPublishingAdditionalParameters }}' -SymbolPublishingAdditionalParameters '${{ parameters.symbolPublishingAdditionalParameters }}' diff --git a/eng/common/templates/post-build/post-build.yml b/eng/common/templates/post-build/post-build.yml index aba44a25a3..2db4933468 100644 --- a/eng/common/templates/post-build/post-build.yml +++ b/eng/common/templates/post-build/post-build.yml @@ -268,14 +268,16 @@ stages: - task: NuGetAuthenticate@1 - - task: PowerShell@2 + - task: AzureCLI@2 displayName: Publish Using Darc inputs: - filePath: $(Build.SourcesDirectory)/eng/common/post-build/publish-using-darc.ps1 + azureSubscription: "Darc: Maestro Production" + scriptType: ps + scriptLocation: scriptPath + scriptPath: $(Build.SourcesDirectory)/eng/common/post-build/publish-using-darc.ps1 arguments: -BuildId $(BARBuildId) -PublishingInfraVersion ${{ parameters.publishingInfraVersion }} -AzdoToken '$(publishing-dnceng-devdiv-code-r-build-re)' - -MaestroToken '$(MaestroApiAccessToken)' -WaitPublishingFinish true -ArtifactsPublishingAdditionalParameters '${{ parameters.artifactsPublishingAdditionalParameters }}' -SymbolPublishingAdditionalParameters '${{ parameters.symbolPublishingAdditionalParameters }}' diff --git a/eng/common/templates/post-build/setup-maestro-vars.yml b/eng/common/templates/post-build/setup-maestro-vars.yml index 0c87f149a4..64b9abc685 100644 --- a/eng/common/templates/post-build/setup-maestro-vars.yml +++ b/eng/common/templates/post-build/setup-maestro-vars.yml @@ -11,13 +11,14 @@ steps: artifactName: ReleaseConfigs checkDownloadedFiles: true - - task: PowerShell@2 + - task: AzureCLI@2 name: setReleaseVars displayName: Set Release Configs Vars inputs: - targetType: inline - pwsh: true - script: | + azureSubscription: "Darc: Maestro Production" + scriptType: pscore + scriptLocation: inlineScript + inlineScript: | try { if (!$Env:PromoteToMaestroChannels -or $Env:PromoteToMaestroChannels.Trim() -eq '') { $Content = Get-Content $(Build.StagingDirectory)/ReleaseConfigs/ReleaseConfigs.txt @@ -31,15 +32,16 @@ steps: $AzureDevOpsBuildId = $Env:Build_BuildId } else { - $buildApiEndpoint = "${Env:MaestroApiEndPoint}/api/builds/${Env:BARBuildId}?api-version=${Env:MaestroApiVersion}" + . $(Build.SourcesDirectory)\eng\common\tools.ps1 + $darc = Get-Darc + $buildInfo = & $darc get-build ` + --id ${{ parameters.BARBuildId }} ` + --extended ` + --output-format json ` + --ci ` + | convertFrom-Json - $apiHeaders = New-Object 'System.Collections.Generic.Dictionary[[String],[String]]' - $apiHeaders.Add('Accept', 'application/json') - $apiHeaders.Add('Authorization',"Bearer ${Env:MAESTRO_API_TOKEN}") - - $buildInfo = try { Invoke-WebRequest -Method Get -Uri $buildApiEndpoint -Headers $apiHeaders | ConvertFrom-Json } catch { Write-Host "Error: $_" } - - $BarId = $Env:BARBuildId + $BarId = ${{ parameters.BARBuildId }} $Channels = $Env:PromoteToMaestroChannels -split "," $Channels = $Channels -join "][" $Channels = "[$Channels]" @@ -65,6 +67,4 @@ steps: exit 1 } env: - MAESTRO_API_TOKEN: $(MaestroApiAccessToken) - BARBuildId: ${{ parameters.BARBuildId }} PromoteToMaestroChannels: ${{ parameters.PromoteToChannelIds }} diff --git a/global.json b/global.json index 8136e023fa..45aa063ffb 100644 --- a/global.json +++ b/global.json @@ -15,6 +15,6 @@ } }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "8.0.0-beta.24324.1" + "Microsoft.DotNet.Arcade.Sdk": "8.0.0-beta.24328.2" } } From 2c33dcfa5feea8fb651ef2408efbaf93461f0279 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:36:33 +0200 Subject: [PATCH 07/12] Continue azure cli test on error (#3699) --- eng/templates/stages/deploy.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/templates/stages/deploy.yaml b/eng/templates/stages/deploy.yaml index cc253f8c3b..e34d8ee128 100644 --- a/eng/templates/stages/deploy.yaml +++ b/eng/templates/stages/deploy.yaml @@ -197,6 +197,7 @@ stages: .\darc\darc.exe get-default-channels --source-repo arcade-services --ci --bar-uri "$(GetAuthInfo.BarUri)" --debug displayName: Test Azure CLI authentication + continueOnError: true - powershell: .\darc\darc.exe get-default-channels --source-repo arcade-services --ci -t "$(GetAuthInfo.FederatedToken)" --bar-uri "$(GetAuthInfo.BarUri)" --debug From 139ee32a7411b80eed60efec092f4394beb7ace8 Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Wed, 3 Jul 2024 15:32:55 +0200 Subject: [PATCH 08/12] [main] Update dependencies from dotnet/dnceng-shared (#3703) --- Directory.Packages.props | 4 +- eng/Version.Details.xml | 88 ++++++++++++++++++++-------------------- eng/Versions.props | 44 ++++++++++---------- 3 files changed, 68 insertions(+), 68 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 85ed3abd0d..92a4136c29 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -128,7 +128,7 @@ - + @@ -142,4 +142,4 @@ - \ No newline at end of file + diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 9f204ff1dd..d45cb5bb23 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -1,93 +1,93 @@ - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 - + https://github.com/dotnet/dnceng-shared - 77c5d1699eba47a9fd73a111cabc56a0f4bb7c3f + 5f8639a65b7e5333bb4a4f5c629e5384c6eb7dc2 diff --git a/eng/Versions.props b/eng/Versions.props index dc143fd509..a60051b5ea 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -15,28 +15,28 @@ 8.0.0-beta.24328.2 8.0.0-beta.24328.2 17.4.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 - 1.1.0-beta.24319.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 + 1.1.0-beta.24353.1 1.1.0-beta.24325.1 1.1.0-beta.24325.1 From 91594896fadc1fd3361b7a615cde1138e02eebe1 Mon Sep 17 00:00:00 2001 From: Pavel Purma Date: Tue, 9 Jul 2024 13:55:02 +0200 Subject: [PATCH 09/12] Azure.Extensions.AspNetCore.DataProtection.Keys version update (#3707) Co-authored-by: Pavel Purma --- Directory.Packages.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 92a4136c29..5e768795a5 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -20,7 +20,7 @@ - + @@ -142,4 +142,4 @@ - + \ No newline at end of file From 2946d9c55e63fee48aa260d0bfbec106d0706701 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Tue, 9 Jul 2024 13:55:13 +0200 Subject: [PATCH 10/12] Maestro Logger registration, git location, token retrieval fix (#3706) --- src/Maestro/FeedCleanerService/Program.cs | 6 ++++- src/Maestro/Maestro.Web/Startup.cs | 5 +++- .../DarcRemoteFactory.cs | 2 -- .../SubscriptionActorService/Program.cs | 3 ++- .../DarcLib/GitHubClient.cs | 24 +++++++++++++++---- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Maestro/FeedCleanerService/Program.cs b/src/Maestro/FeedCleanerService/Program.cs index d54eea75dc..7a93562160 100644 --- a/src/Maestro/FeedCleanerService/Program.cs +++ b/src/Maestro/FeedCleanerService/Program.cs @@ -9,6 +9,7 @@ using Microsoft.DotNet.ServiceFabric.ServiceHost; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace FeedCleanerService; @@ -52,6 +53,9 @@ public static void Configure(IServiceCollection services) services.AddAzureDevOpsTokenProvider(); services.Configure("AzureDevOps", (o, s) => s.Bind(o)); services.AddTransient(); - services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); + services.AddTransient(sp => + new ProcessManager( + sp.GetRequiredService>(), + "git")); } } diff --git a/src/Maestro/Maestro.Web/Startup.cs b/src/Maestro/Maestro.Web/Startup.cs index ea4e4fcf3d..b869b446d8 100644 --- a/src/Maestro/Maestro.Web/Startup.cs +++ b/src/Maestro/Maestro.Web/Startup.cs @@ -245,7 +245,10 @@ public override void ConfigureServices(IServiceCollection services) // in such a way that will work with sizing. services.AddSingleton(); - services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); + services.AddTransient(sp => + new ProcessManager( + sp.GetRequiredService>(), + "git")); services.AddTransient(); services.AddScoped(); services.AddTransient(); diff --git a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs index aebea88b17..375273931c 100644 --- a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs +++ b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs @@ -94,8 +94,6 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo throw new GithubApplicationInstallationException($"No installation is available for repository '{normalizedUrl}'"); } - var gitExe = _localGit.GetPathToLocalGit(); - return GitRepoUrlParser.ParseTypeFromUri(normalizedUrl) switch { GitRepoType.GitHub => installationId == default diff --git a/src/Maestro/SubscriptionActorService/Program.cs b/src/Maestro/SubscriptionActorService/Program.cs index e8bff9edfa..6b94205759 100644 --- a/src/Maestro/SubscriptionActorService/Program.cs +++ b/src/Maestro/SubscriptionActorService/Program.cs @@ -40,7 +40,8 @@ private static void Main() public static void Configure(IServiceCollection services) { services.TryAddTransient(sp => sp.GetRequiredService>()); - services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); + services.AddTransient(sp => + ActivatorUtilities.CreateInstance(sp, sp.GetRequiredService().GetPathToLocalGit())); services.AddSingleton(); services.AddSingleton(); services.AddTransient(); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs index 659c427702..36c3118e2f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs @@ -115,6 +115,7 @@ private async Task GetFileContentsAsync(string owner, string repo, strin { using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/contents/{filePath}?ref={branch}", _logger, logFailure: false)) @@ -160,6 +161,7 @@ public async Task CreateBranchAsync(string repoUri, string newBranch, string bas using (await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/branches/{newBranch}", _logger, retryCount: 0)) { } @@ -168,6 +170,7 @@ public async Task CreateBranchAsync(string repoUri, string newBranch, string bas body = JsonConvert.SerializeObject(githubRef, _serializerSettings); using (await ExecuteRemoteGitCommandAsync( new HttpMethod("PATCH"), + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/git/{gitRef}", _logger, body)) { } @@ -181,6 +184,7 @@ public async Task CreateBranchAsync(string repoUri, string newBranch, string bas body = JsonConvert.SerializeObject(githubRef, _serializerSettings); using (await ExecuteRemoteGitCommandAsync( HttpMethod.Post, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/git/refs", _logger, body)) { } @@ -277,6 +281,7 @@ public async Task> SearchPullRequestsAsync( JObject responseContent; using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"search/issues?q={query}", _logger)) { @@ -300,8 +305,11 @@ public async Task GetPullRequestStatusAsync(string pullRequestUrl) (string owner, string repo, int id) = ParsePullRequestUri(pullRequestUrl); JObject responseContent; - using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync(HttpMethod.Get, - $"repos/{owner}/{repo}/pulls/{id}", _logger)) + using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( + HttpMethod.Get, + $"https://github.com/{owner}/{repo}", + $"repos/{owner}/{repo}/pulls/{id}", + _logger)) { responseContent = JObject.Parse(await response.Content.ReadAsStringAsync()); } @@ -748,6 +756,7 @@ private async Task GetGitItemImpl(string path, TreeItem treeItem, strin /// private async Task ExecuteRemoteGitCommandAsync( HttpMethod method, + string repoUri, string requestUri, ILogger logger, string body = null, @@ -759,7 +768,7 @@ private async Task ExecuteRemoteGitCommandAsync( { retryCount = 0; } - using (HttpClient client = CreateHttpClient()) + using (HttpClient client = CreateHttpClient(repoUri)) { var requestManager = new HttpRequestManager(client, method, requestUri, logger, body, versionOverride, logFailure); try @@ -783,14 +792,14 @@ private async Task ExecuteRemoteGitCommandAsync( /// Create a new http client for talking to github. /// /// New http client CheckIfFileExistsAsync(string repoUri, string filePath JObject content; using (response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/contents/{filePath}?ref={branch}", _logger)) { @@ -891,6 +901,7 @@ private async Task GetLastCommitShaAsync(string owner, string repo, stri JObject content; using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/commits/{branch}", _logger)) { @@ -1102,6 +1113,7 @@ private async Task GetCommitMapForPathAsync( using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/contents/{path}?ref={assetsProducedInCommit}", _logger)) { @@ -1207,6 +1219,7 @@ public async Task GitDiffAsync(string repoUri, string baseVersion, stri JObject content; using (HttpResponseMessage response = await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}/compare/{baseVersion}...{targetVersion}", _logger)) { @@ -1241,6 +1254,7 @@ public async Task RepoExistsAsync(string repoUri) { using (await ExecuteRemoteGitCommandAsync( HttpMethod.Get, + $"https://github.com/{owner}/{repo}", $"repos/{owner}/{repo}", _logger, logFailure: false)) { } From a91437319bde5654b41d96fcdba9e7faf635f159 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Wed, 10 Jul 2024 11:11:34 +0200 Subject: [PATCH 11/12] Add required tag to PCS Vnet bicep (#3710) --- .../ProductConstructionService/provision.bicep | 3 +++ 1 file changed, 3 insertions(+) diff --git a/eng/service-templates/ProductConstructionService/provision.bicep b/eng/service-templates/ProductConstructionService/provision.bicep index 58ca8f9b8f..ec98260aa5 100644 --- a/eng/service-templates/ProductConstructionService/provision.bicep +++ b/eng/service-templates/ProductConstructionService/provision.bicep @@ -300,6 +300,9 @@ resource virtualNetwork 'Microsoft.Network/virtualNetworks@2023-04-01' = { ] } } + tags: { + 'ms.inv.v0.networkUsage': 'mixedTraffic' + } } // subnet for the product construction service From ac1a1ead1e465231ac01618f12631e0efba3ffb2 Mon Sep 17 00:00:00 2001 From: Pavel Purma Date: Wed, 10 Jul 2024 11:44:10 +0200 Subject: [PATCH 12/12] Version update for Azure.Security.KeyVault.Keys (#3708) Co-authored-by: Pavel Purma --- Directory.Packages.props | 1 + 1 file changed, 1 insertion(+) diff --git a/Directory.Packages.props b/Directory.Packages.props index 5e768795a5..db97a9b60a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -24,6 +24,7 @@ +