From 394684b8636e3c42da4db10de51fd2c672ae9c72 Mon Sep 17 00:00:00 2001 From: Nikita Ezzhev Date: Tue, 6 Apr 2021 18:51:04 +0300 Subject: [PATCH] Fix 'Self-hosted agent cleaning source folder' issue without affecting env variables (#3318) * Add L0 test * Added tests * Remove extra change * Fix tests * Fix one more case when variable value changed * Fixed test * Use HasMultipleCheckouts method instead of own implementation * Fix nit * Some code style improvements * Added some comments * Changed comments * Code cleanup in tests * Added check that the entered path does not correspond to the default location for this repository * Fixed tests * Added additional test * Fixed nit * Self-hosted agent cleaning source folder (#3237) * Resolved review points * Removed extra braces * Rename method Co-authored-by: Anatoly Bolshakov --- .../Build/BuildDirectoryManager.cs | 22 +- src/Agent.Worker/Build/BuildJobExtension.cs | 57 ++++- .../PluginInternalCommandExtension.cs | 21 +- .../L0/Worker/Build/BuildJobExtensionL0.cs | 236 ++++++++++++++++++ 4 files changed, 326 insertions(+), 10 deletions(-) create mode 100644 src/Test/L0/Worker/Build/BuildJobExtensionL0.cs diff --git a/src/Agent.Worker/Build/BuildDirectoryManager.cs b/src/Agent.Worker/Build/BuildDirectoryManager.cs index 8438007145..3bed8e7987 100644 --- a/src/Agent.Worker/Build/BuildDirectoryManager.cs +++ b/src/Agent.Worker/Build/BuildDirectoryManager.cs @@ -122,7 +122,7 @@ public TrackingConfig PrepareDirectory( // Set the default clone path for each repository (the Checkout task may override this later) foreach (var repository in repositories) { - var repoPath = GetDefaultRepositoryPath(executionContext, repository, newConfig.SourcesDirectory); + string repoPath = GetDefaultRepositoryPath(executionContext, repository, newConfig); if (!string.Equals(repoPath, defaultSourceDirectory, StringComparison.Ordinal)) { @@ -290,18 +290,32 @@ private BuildCleanOption GetBuildDirectoryCleanOption(IExecutionContext executio private string GetDefaultRepositoryPath( IExecutionContext executionContext, RepositoryResource repository, - string defaultSourcesDirectory) + TrackingConfig newConfig + ) { + string repoPath = String.Empty; + string workDirectory = HostContext.GetDirectory(WellKnownDirectory.Work); + if (RepositoryUtil.HasMultipleCheckouts(executionContext.JobSettings)) { // If we have multiple checkouts they should all be rooted to the sources directory (_work/1/s/repo1) - return Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Work), defaultSourcesDirectory, RepositoryUtil.GetCloneDirectory(repository)); + var repoSourceDirectory = newConfig?.RepositoryTrackingInfo.Where(item => string.Equals(item.Identifier, repository.Alias, StringComparison.OrdinalIgnoreCase)).Select(item => item.SourcesDirectory).FirstOrDefault(); + if (repoSourceDirectory != null) + { + repoPath = Path.Combine(workDirectory, repoSourceDirectory); + } + else + { + repoPath = Path.Combine(workDirectory, newConfig.SourcesDirectory, RepositoryUtil.GetCloneDirectory(repository)); + } } else { // For single checkouts, the repository is rooted to the sources folder (_work/1/s) - return Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Work), defaultSourcesDirectory); + repoPath = Path.Combine(workDirectory, newConfig.SourcesDirectory); } + + return repoPath; } } } \ No newline at end of file diff --git a/src/Agent.Worker/Build/BuildJobExtension.cs b/src/Agent.Worker/Build/BuildJobExtension.cs index 663f1e4d12..9d18569222 100644 --- a/src/Agent.Worker/Build/BuildJobExtension.cs +++ b/src/Agent.Worker/Build/BuildJobExtension.cs @@ -166,6 +166,9 @@ public override void InitializeJobExtension(IExecutionContext executionContext, string pipelineWorkspaceDirectory = Path.Combine(_workDirectory, trackingConfig.BuildDirectory); UpdateCheckoutTasksAndVariables(executionContext, steps, pipelineWorkspaceDirectory); + + // Get default value for RepoLocalPath variable + string selfRepoPath = GetDefaultRepoLocalPathValue(executionContext, steps, trackingConfig, repoInfo); // Set the directory variables. executionContext.Output(StringUtil.Loc("SetBuildVars")); @@ -177,7 +180,7 @@ public override void InitializeJobExtension(IExecutionContext executionContext, executionContext.SetVariable(Constants.Variables.Build.SourcesDirectory, Path.Combine(_workDirectory, trackingConfig.SourcesDirectory), isFilePath: true); executionContext.SetVariable(Constants.Variables.Build.StagingDirectory, Path.Combine(_workDirectory, trackingConfig.ArtifactsDirectory), isFilePath: true); executionContext.SetVariable(Constants.Variables.Build.ArtifactStagingDirectory, Path.Combine(_workDirectory, trackingConfig.ArtifactsDirectory), isFilePath: true); - executionContext.SetVariable(Constants.Variables.Build.RepoLocalPath, Path.Combine(_workDirectory, trackingConfig.SourcesDirectory), isFilePath: true); + executionContext.SetVariable(Constants.Variables.Build.RepoLocalPath, Path.Combine(_workDirectory, selfRepoPath), isFilePath: true); executionContext.SetVariable(Constants.Variables.Pipeline.Workspace, pipelineWorkspaceDirectory, isFilePath: true); } @@ -339,6 +342,58 @@ private string ConvertToLegacyRepositoryType(string pipelineRepositoryType) } } + private string GetDefaultRepoLocalPathValue(IExecutionContext executionContext, IList steps, TrackingConfig trackingConfig, RepositoryInfo repoInfo) + { + string selfRepoPath = null; + // For saving backward compatibility with the behavior of the Build.RepoLocalPath that was before this PR https://github.com/microsoft/azure-pipelines-agent/pull/3237 + // We need to change how we set the default value of this variable + // We need to allow the setting of paths from RepositoryTrackingInfo for checkout tasks where path input was provided by the user + // and this input is not point to the default location for this repository + // This is the only case where the value of Build.RepoLocalPath variable is not pointing to the root of sources directory /s. + // The new logic is not affecting single checkout jobs and jobs with multiple checkouts and default paths for Self repository + if (RepositoryUtil.HasMultipleCheckouts(executionContext.JobSettings)) + { + // get checkout task for self repo + var selfCheckoutTask = GetSelfCheckoutTask(steps); + + // Check if the task has path input with custom path, if so we need to set as a value of selfRepoPath the value of SourcesDirectory from RepositoryTrackingInfo + if (IsCheckoutToCustomPath(trackingConfig, repoInfo, selfCheckoutTask)) + { + selfRepoPath = trackingConfig.RepositoryTrackingInfo + .Where(repo => RepositoryUtil.IsPrimaryRepositoryName(repo.Identifier)) + .Select(props => props.SourcesDirectory).FirstOrDefault(); + } + } + // For single checkout jobs and multicheckout jobs with default paths set selfRepoPath to the default sources directory + if (selfRepoPath == null) + { + selfRepoPath = trackingConfig.SourcesDirectory; + } + + return selfRepoPath; + } + + private bool IsCheckoutToCustomPath(TrackingConfig trackingConfig, RepositoryInfo repoInfo, TaskStep selfCheckoutTask) + { + string path; + string selfRepoName = RepositoryUtil.GetCloneDirectory(repoInfo.PrimaryRepository.Properties.Get(Pipelines.RepositoryPropertyNames.Name)); + string defaultRepoCheckoutPath = Path.GetFullPath(Path.Combine(trackingConfig.SourcesDirectory, selfRepoName)); + + return selfCheckoutTask != null + && selfCheckoutTask.Inputs.TryGetValue(PipelineConstants.CheckoutTaskInputs.Path, out path) + && !string.Equals(Path.GetFullPath(Path.Combine(trackingConfig.BuildDirectory, path)), + defaultRepoCheckoutPath, + IOUtil.FilePathStringComparison); + } + + private TaskStep GetSelfCheckoutTask(IList steps) + { + return steps.Select(x => x as TaskStep) + .Where(task => task.IsCheckoutTask() + && task.Inputs.TryGetValue(PipelineConstants.CheckoutTaskInputs.Repository, out string repositoryAlias) + && RepositoryUtil.IsPrimaryRepositoryName(repositoryAlias)).FirstOrDefault(); + } + private class RepositoryInfo { public Pipelines.RepositoryResource PrimaryRepository { set; get; } diff --git a/src/Agent.Worker/PluginInternalCommandExtension.cs b/src/Agent.Worker/PluginInternalCommandExtension.cs index cfadfa1e9c..ab4635306e 100644 --- a/src/Agent.Worker/PluginInternalCommandExtension.cs +++ b/src/Agent.Worker/PluginInternalCommandExtension.cs @@ -60,19 +60,30 @@ public void Execute(IExecutionContext context, Command command) bool isSelfRepo = RepositoryUtil.IsPrimaryRepositoryName(repository.Alias); bool hasMultipleCheckouts = RepositoryUtil.HasMultipleCheckouts(context.JobSettings); + var directoryManager = context.GetHostContext().GetService(); + string _workDirectory = context.GetHostContext().GetDirectory(WellKnownDirectory.Work); + var trackingConfig = directoryManager.UpdateDirectory(context, repository); + if (isSelfRepo || !hasMultipleCheckouts) { - var directoryManager = context.GetHostContext().GetService(); - string _workDirectory = context.GetHostContext().GetDirectory(WellKnownDirectory.Work); - - var trackingConfig = directoryManager.UpdateDirectory(context, repository); if (hasMultipleCheckouts) { // In Multi-checkout, we don't want to reset sources dir or default working dir. // So, we will just reset the repo local path string buildDirectory = context.Variables.Get(Constants.Variables.Pipeline.Workspace); string repoRelativePath = directoryManager.GetRelativeRepositoryPath(buildDirectory, repositoryPath); - context?.SetVariable(Constants.Variables.Build.RepoLocalPath, Path.Combine(_workDirectory, repoRelativePath), isFilePath: true); + + string sourcesDirectory = context.Variables.Get(Constants.Variables.Build.SourcesDirectory); + string repoLocalPath = context.Variables.Get(Constants.Variables.Build.RepoLocalPath); + string newRepoLocation = Path.Combine(_workDirectory, repoRelativePath); + // For saving backward compatibility with the behavior of the Build.RepoLocalPath that was before this PR https://github.com/microsoft/azure-pipelines-agent/pull/3237 + // we need to deny updating of the variable in case the new path is the default location for the repository that is equal to sourcesDirectory/repository.Name + // since the variable already has the right value in this case and pointing to the default sources location + if (repoLocalPath == null + || !string.Equals(newRepoLocation, Path.Combine(sourcesDirectory, repository.Name), IOUtil.FilePathStringComparison)) + { + context?.SetVariable(Constants.Variables.Build.RepoLocalPath, newRepoLocation, isFilePath: true); + } } else { diff --git a/src/Test/L0/Worker/Build/BuildJobExtensionL0.cs b/src/Test/L0/Worker/Build/BuildJobExtensionL0.cs new file mode 100644 index 0000000000..8d3258a977 --- /dev/null +++ b/src/Test/L0/Worker/Build/BuildJobExtensionL0.cs @@ -0,0 +1,236 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.CompilerServices; +using Microsoft.TeamFoundation.Build.WebApi; +using Microsoft.TeamFoundation.DistributedTask.WebApi; +using Microsoft.VisualStudio.Services.Agent.Worker; +using Microsoft.VisualStudio.Services.Agent.Worker.Build; +using Microsoft.VisualStudio.Services.Agent.Worker.Release; +using Pipelines = Microsoft.TeamFoundation.DistributedTask.Pipelines; +using Moq; +using Xunit; +using Agent.Sdk; + +namespace Microsoft.VisualStudio.Services.Agent.Tests.Worker.Build +{ + public sealed class BuildJobExtensionL0 + { + private Mock _ec; + private Mock _extensionManager; + private Mock _sourceProvider; + private Mock _buildDirectoryManager; + private Mock _configurationStore; + private Variables _variables; + private string stubWorkFolder; + private BuildJobExtension buildJobExtension; + private List steps; + private List repositories { get; set; } + private Dictionary jobSettings { get; set; } + + private const string CollectionId = "31ffacb8-b468-4e60-b2f9-c50ce437da92"; + private const string DefinitionId = "1234"; + private Pipelines.WorkspaceOptions _workspaceOptions; + private char directorySeparator = Path.DirectorySeparatorChar; + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckSingleRepoWithoutPathInput() + { + using (TestHostContext tc = Setup(createWorkDirectory: false, checkOutConfig: CheckoutConfigType.SingleCheckoutDefaultPath)) + { + buildJobExtension.InitializeJobExtension(_ec.Object, steps, _workspaceOptions); + var repoLocalPath = _ec.Object.Variables.Get(Constants.Variables.Build.RepoLocalPath); + Assert.NotNull(repoLocalPath); + Assert.Equal(Path.Combine(stubWorkFolder, $"1{directorySeparator}s"), repoLocalPath); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckSingleRepoWithCustomPaths() + { + using (TestHostContext tc = Setup(createWorkDirectory: false, checkOutConfig: CheckoutConfigType.SingleCheckoutCustomPath, pathToSelfRepo: "s/CustomApplicationFolder")) + { + buildJobExtension.InitializeJobExtension(_ec.Object, steps, _workspaceOptions); + var repoLocalPath = _ec.Object.Variables.Get(Constants.Variables.Build.RepoLocalPath); + Assert.NotNull(repoLocalPath); + Assert.Equal(Path.Combine(stubWorkFolder, $"1{directorySeparator}s"), repoLocalPath); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckMultiRepoWithoutPathInput() + { + using (TestHostContext tc = Setup(createWorkDirectory: false, checkOutConfig: CheckoutConfigType.MultiCheckoutDefaultPath)) + { + buildJobExtension.InitializeJobExtension(_ec.Object, steps, _workspaceOptions); + var repoLocalPath = _ec.Object.Variables.Get(Constants.Variables.Build.RepoLocalPath); + Assert.NotNull(repoLocalPath); + Assert.Equal(Path.Combine(stubWorkFolder, $"1{directorySeparator}s"), repoLocalPath); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckMultiRepoWithPathInputToCustomPath() + { + using (TestHostContext tc = Setup(createWorkDirectory: false, checkOutConfig: CheckoutConfigType.MultiCheckoutCustomPath, pathToSelfRepo: "s/CustomApplicationFolder")) + { + buildJobExtension.InitializeJobExtension(_ec.Object, steps, _workspaceOptions); + var repoLocalPath = _ec.Object.Variables.Get(Constants.Variables.Build.RepoLocalPath); + Assert.NotNull(repoLocalPath); + Assert.Equal(Path.Combine(stubWorkFolder, $"1{directorySeparator}s{directorySeparator}App"), repoLocalPath); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckMultiRepoWithPathInputToDefaultPath() + { + using (TestHostContext tc = Setup(createWorkDirectory: false, checkOutConfig: CheckoutConfigType.MultiCheckoutCustomPath, pathToSelfRepo: "s/App")) + { + buildJobExtension.InitializeJobExtension(_ec.Object, steps, _workspaceOptions); + var repoLocalPath = _ec.Object.Variables.Get(Constants.Variables.Build.RepoLocalPath); + Assert.NotNull(repoLocalPath); + Assert.Equal(Path.Combine(stubWorkFolder, $"1{directorySeparator}s"), repoLocalPath); + } + } + + private TestHostContext Setup([CallerMemberName] string name = "", + bool createWorkDirectory = true, + CheckoutConfigType checkOutConfig = CheckoutConfigType.SingleCheckoutDefaultPath, + string pathToSelfRepo = "") + { + bool isMulticheckoutScenario = checkOutConfig == CheckoutConfigType.MultiCheckoutCustomPath || checkOutConfig == CheckoutConfigType.MultiCheckoutDefaultPath; + bool isCustomPathScenario = checkOutConfig == CheckoutConfigType.SingleCheckoutCustomPath || checkOutConfig == CheckoutConfigType.MultiCheckoutCustomPath; + + TestHostContext hc = new TestHostContext(this, name); + this.stubWorkFolder = hc.GetDirectory(WellKnownDirectory.Work); + if (createWorkDirectory) + { + Directory.CreateDirectory(this.stubWorkFolder); + } + + _ec = new Mock(); + + _extensionManager = new Mock(); + _sourceProvider = new Mock(); + _buildDirectoryManager = new Mock(); + _workspaceOptions = new Pipelines.WorkspaceOptions(); + _configurationStore = new Mock(); + _configurationStore.Setup(store => store.GetSettings()).Returns(new AgentSettings { WorkFolder = this.stubWorkFolder }); + + steps = new List(); + var selfCheckoutTask = new Pipelines.TaskStep() + { + Reference = new Pipelines.TaskStepDefinitionReference() + { + Id = Guid.Parse("6d15af64-176c-496d-b583-fd2ae21d4df4"), + Name = "Checkout", + Version = "1.0.0" + } + }; + selfCheckoutTask.Inputs.Add("repository", "self"); + if (isCustomPathScenario) + { + selfCheckoutTask.Inputs.Add("path", pathToSelfRepo); + } + steps.Add(selfCheckoutTask); + + // Setup second checkout only for multicheckout jobs + if (isMulticheckoutScenario) + { + var anotherCheckoutTask = new Pipelines.TaskStep() + { + Reference = new Pipelines.TaskStepDefinitionReference() + { + Id = Guid.Parse("6d15af64-176c-496d-b583-fd2ae21d4df4"), + Name = "Checkout", + Version = "1.0.0" + } + }; + anotherCheckoutTask.Inputs.Add("repository", "BuildRepo"); + anotherCheckoutTask.Inputs.Add("path", "s/BuildRepo"); + steps.Add(anotherCheckoutTask); + } + + hc.SetSingleton(_buildDirectoryManager.Object); + hc.SetSingleton(_extensionManager.Object); + hc.SetSingleton(_configurationStore.Object); + + var buildVariables = GetBuildVariables(); + _variables = new Variables(hc, buildVariables, out _); + _ec.Setup(x => x.Variables).Returns(_variables); + + repositories = new List(); + repositories.Add(GetRepository(hc, "self", "App", "App")); + repositories.Add(GetRepository(hc, "repo2", "BuildRepo", "BuildRepo")); + _ec.Setup(x => x.Repositories).Returns(repositories); + + jobSettings = new Dictionary(); + jobSettings.Add(WellKnownJobSettings.HasMultipleCheckouts, isMulticheckoutScenario.ToString()); + _ec.Setup(x => x.JobSettings).Returns(jobSettings); + + _ec.Setup(x => + x.SetVariable(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((string varName, string varValue, bool isSecret, bool isOutput, bool isFilePath, bool isReadOnly) => { _variables.Set(varName, varValue, false); }); + + _extensionManager.Setup(x => x.GetExtensions()) + .Returns(new List { _sourceProvider.Object }); + + _sourceProvider.Setup(x => x.RepositoryType) + .Returns(Pipelines.RepositoryTypes.ExternalGit); + + _buildDirectoryManager.Setup(x => x.PrepareDirectory(_ec.Object, repositories, _workspaceOptions)) + .Returns(new TrackingConfig(_ec.Object, repositories, 1)); + + buildJobExtension = new BuildJobExtension(); + buildJobExtension.Initialize(hc); + return hc; + } + + private Dictionary GetBuildVariables() + { + var buildVariables = new Dictionary(); + buildVariables.Add(Constants.Variables.Build.SyncSources, Boolean.TrueString); + buildVariables.Add(Constants.Variables.System.CollectionId, CollectionId); + buildVariables.Add(Constants.Variables.System.DefinitionId, DefinitionId); + + return buildVariables; + } + + private Pipelines.RepositoryResource GetRepository(TestHostContext hostContext, String alias, String relativePath, String Name) + { + var workFolder = hostContext.GetDirectory(WellKnownDirectory.Work); + var repo = new Pipelines.RepositoryResource() + { + Alias = alias, + Type = Pipelines.RepositoryTypes.ExternalGit, + Id = alias, + Url = new Uri($"http://contoso.visualstudio.com/{Name}"), + Name = Name, + }; + repo.Properties.Set(Pipelines.RepositoryPropertyNames.Path, Path.Combine(workFolder, "1", relativePath)); + + return repo; + } + + private enum CheckoutConfigType + { + MultiCheckoutDefaultPath = 0, + MultiCheckoutCustomPath = 1, + SingleCheckoutDefaultPath = 2, + SingleCheckoutCustomPath = 3, + } + } +}