diff --git a/src/Runner.Worker/Handlers/OutputManager.cs b/src/Runner.Worker/Handlers/OutputManager.cs index a53a0436f..369042ddf 100644 --- a/src/Runner.Worker/Handlers/OutputManager.cs +++ b/src/Runner.Worker/Handlers/OutputManager.cs @@ -18,44 +18,46 @@ namespace GitHub.Runner.Worker.Handlers private static readonly Regex _colorCodeRegex = new Regex(@"\x0033\[[0-9;]*m?", RegexOptions.Compiled | RegexOptions.CultureInvariant); private readonly IActionCommandManager _commandManager; private readonly IExecutionContext _executionContext; + private readonly int _failsafe = 50; private readonly object _matchersLock = new object(); private readonly TimeSpan _timeout; private IssueMatcher[] _matchers = Array.Empty(); + // Mapping that indicates whether a directory belongs to the workflow repository + private readonly Dictionary _directoryMap = new Dictionary(); public OutputManager(IExecutionContext executionContext, IActionCommandManager commandManager) { - //executionContext.Debug("ENTERING OutputManager ctor"); _executionContext = executionContext; _commandManager = commandManager; - //_executionContext.Debug("OutputManager ctor - determine timeout from variable"); + // Recursion failsafe (test override) + var failsafeString = Environment.GetEnvironmentVariable("RUNNER_TEST_GET_REPOSITORY_PATH_FAILSAFE"); + if (!string.IsNullOrEmpty(failsafeString)) + { + _failsafe = int.Parse(failsafeString, NumberStyles.None); + } + // Determine the timeout var timeoutStr = _executionContext.Variables.Get(_timeoutKey); if (string.IsNullOrEmpty(timeoutStr) || !TimeSpan.TryParse(timeoutStr, CultureInfo.InvariantCulture, out _timeout) || _timeout <= TimeSpan.Zero) { - //_executionContext.Debug("OutputManager ctor - determine timeout from env var"); timeoutStr = Environment.GetEnvironmentVariable(_timeoutKey); if (string.IsNullOrEmpty(timeoutStr) || !TimeSpan.TryParse(timeoutStr, CultureInfo.InvariantCulture, out _timeout) || _timeout <= TimeSpan.Zero) { - //_executionContext.Debug("OutputManager ctor - set timeout to default"); _timeout = TimeSpan.FromSeconds(1); } } - //_executionContext.Debug("OutputManager ctor - adding matchers"); // Lock lock (_matchersLock) { - //_executionContext.Debug("OutputManager ctor - adding OnMatcherChanged"); _executionContext.Add(OnMatcherChanged); - //_executionContext.Debug("OutputManager ctor - getting matchers"); _matchers = _executionContext.GetMatchers().Select(x => new IssueMatcher(x, _timeout)).ToArray(); } - //_executionContext.Debug("LEAVING OutputManager ctor"); } public void Dispose() @@ -71,7 +73,6 @@ namespace GitHub.Runner.Worker.Handlers public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e) { - //_executionContext.Debug("ENTERING OutputManager OnDataReceived"); var line = e.Data; // ## commands @@ -82,7 +83,6 @@ namespace GitHub.Runner.Worker.Handlers // The logging queues and command handlers are thread-safe. if (_commandManager.TryProcessCommand(_executionContext, line)) { - //_executionContext.Debug("LEAVING OutputManager OnDataReceived - command processed"); return; } } @@ -142,7 +142,6 @@ namespace GitHub.Runner.Worker.Handlers // Log issue _executionContext.AddIssue(issue, stripped); - //_executionContext.Debug("LEAVING OutputManager OnDataReceived - issue logged"); return; } } @@ -151,7 +150,6 @@ namespace GitHub.Runner.Worker.Handlers // Regular output _executionContext.Output(line); - //_executionContext.Debug("LEAVING OutputManager OnDataReceived"); } private void OnMatcherChanged(object sender, MatcherChangedEventArgs e) @@ -261,7 +259,7 @@ namespace GitHub.Runner.Worker.Handlers var file = match.File; // Root using fromPath - if (!string.IsNullOrWhiteSpace(match.FromPath) && !Path.IsPathRooted(file)) + if (!string.IsNullOrWhiteSpace(match.FromPath) && !Path.IsPathFullyQualified(file)) { var fromDirectory = Path.GetDirectoryName(match.FromPath); if (!string.IsNullOrWhiteSpace(fromDirectory)) @@ -271,7 +269,7 @@ namespace GitHub.Runner.Worker.Handlers } // Root using workspace - if (!Path.IsPathRooted(file)) + if (!Path.IsPathFullyQualified(file)) { var workspace = _executionContext.GetGitHubContext("workspace"); ArgUtil.NotNullOrEmpty(workspace, "workspace"); @@ -279,31 +277,27 @@ namespace GitHub.Runner.Worker.Handlers file = Path.Combine(workspace, file); } - // Normalize slashes - file = file.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); + // Remove relative pathing and normalize slashes + file = Path.GetFullPath(file); - // File exists + // Check whether the file exists if (File.Exists(file)) { - // Repository path - var repositoryPath = _executionContext.GetGitHubContext("workspace"); - ArgUtil.NotNullOrEmpty(repositoryPath, nameof(repositoryPath)); - - // Normalize slashes - repositoryPath = repositoryPath.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; - - if (!file.StartsWith(repositoryPath, IOUtil.FilePathStringComparison)) + // Check whether the file is under the workflow repository + var repositoryPath = GetRepositoryPath(file); + if (!string.IsNullOrEmpty(repositoryPath)) { - // File is not under repo - _executionContext.Debug($"Dropping file value '{file}'. Path is not under the repo."); + // Get the relative file path + var relativePath = file.Substring(repositoryPath.Length).TrimStart(Path.DirectorySeparatorChar); + + // Prefer `/` on all platforms + issue.Data["file"] = relativePath.Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } else { - // Prefer `/` on all platforms - issue.Data["file"] = file.Substring(repositoryPath.Length).TrimStart(Path.DirectorySeparatorChar).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + _executionContext.Debug($"Dropping file value '{file}'. Path is not under the workflow repo."); } } - // File does not exist else { _executionContext.Debug($"Dropping file value '{file}'. Path does not exist"); @@ -317,5 +311,60 @@ namespace GitHub.Runner.Worker.Handlers return issue; } + + private string GetRepositoryPath(string filePath, int recursion = 0) + { + // Prevent the cache from growing too much + if (_directoryMap.Count > 100) + { + _directoryMap.Clear(); + } + + // Empty directory means we hit the root of the drive + var directoryPath = Path.GetDirectoryName(filePath); + if (string.IsNullOrEmpty(directoryPath) || recursion > _failsafe) + { + return null; + } + + // Check the cache + if (_directoryMap.TryGetValue(directoryPath, out string repositoryPath)) + { + return repositoryPath; + } + + try + { + // Check if .git/config exists + var gitConfigPath = Path.Combine(directoryPath, ".git", "config"); + if (File.Exists(gitConfigPath)) + { + // Check if the config contains the workflow repository url + var qualifiedRepository = _executionContext.GetGitHubContext("repository"); + var configMatch = $"url = https://github.com/{qualifiedRepository}"; + var content = File.ReadAllText(gitConfigPath); + foreach (var line in content.Split("\n").Select(x => x.Trim())) + { + if (String.Equals(line, configMatch, StringComparison.OrdinalIgnoreCase)) + { + repositoryPath = directoryPath; + break; + } + } + } + else + { + // Recursive call + repositoryPath = GetRepositoryPath(directoryPath, recursion + 1); + } + } + catch (Exception ex) + { + _executionContext.Debug($"Error when attempting to determine whether the path '{filePath}' is under the workflow repository: {ex.Message}"); + } + + _directoryMap[directoryPath] = repositoryPath; + return repositoryPath; + } } } diff --git a/src/Test/L0/Worker/OutputManagerL0.cs b/src/Test/L0/Worker/OutputManagerL0.cs index cb0eefebd..ff8b18704 100644 --- a/src/Test/L0/Worker/OutputManagerL0.cs +++ b/src/Test/L0/Worker/OutputManagerL0.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using System.Runtime.CompilerServices; using GitHub.Runner.Sdk; using GitHub.Runner.Worker; @@ -644,8 +646,9 @@ namespace GitHub.Runner.Common.Tests.Worker [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void MatcherFile() + public async void MatcherFile() { + Environment.SetEnvironmentVariable("RUNNER_TEST_GET_REPOSITORY_PATH_FAILSAFE", "2"); var matchers = new IssueMatchersConfig { Matchers = @@ -668,142 +671,87 @@ namespace GitHub.Runner.Common.Tests.Worker using (var hostContext = Setup(matchers: matchers)) using (_outputManager) { - // Setup github.workspace + // Setup github.workspace, github.repository var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work); ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory)); Directory.CreateDirectory(workDirectory); var workspaceDirectory = Path.Combine(workDirectory, "workspace"); Directory.CreateDirectory(workspaceDirectory); _executionContext.Setup(x => x.GetGitHubContext("workspace")).Returns(workspaceDirectory); + _executionContext.Setup(x => x.GetGitHubContext("repository")).Returns("my-org/workflow-repo"); - // Create a test file - Directory.CreateDirectory(Path.Combine(workspaceDirectory, "some-directory")); - var filePath = Path.Combine(workspaceDirectory, "some-directory", "some-file.txt"); - File.WriteAllText(filePath, ""); - - // Process - Process("some-directory/some-file.txt: some error"); - Assert.Equal(1, _issues.Count); - Assert.Equal("some error", _issues[0].Item1.Message); - Assert.Equal("some-directory/some-file.txt", _issues[0].Item1.Data["file"]); - Assert.Equal(0, _commands.Count); - Assert.Equal(0, _messages.Count); - } - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void MatcherFileExists() - { - var matchers = new IssueMatchersConfig - { - Matchers = - { - new IssueMatcherConfig - { - Owner = "my-matcher-1", - Patterns = new[] - { - new IssuePatternConfig - { - Pattern = @"(.+): (.+)", - File = 1, - Message = 2, - }, - }, - }, - }, - }; - using (var hostContext = Setup(matchers: matchers)) - using (_outputManager) - { - // Setup github.workspace - var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work); - ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory)); - Directory.CreateDirectory(workDirectory); - var workspaceDirectory = Path.Combine(workDirectory, "workspace"); - Directory.CreateDirectory(workspaceDirectory); - _executionContext.Setup(x => x.GetGitHubContext("workspace")).Returns(workspaceDirectory); - - // Create a test file - File.WriteAllText(Path.Combine(workspaceDirectory, "some-file-1.txt"), ""); - - // Process - Process("some-file-1.txt: some error 1"); // file exists - Process("some-file-2.txt: some error 2"); // file does not exist - - Assert.Equal(2, _issues.Count); - Assert.Equal("some error 1", _issues[0].Item1.Message); - Assert.Equal("some-file-1.txt", _issues[0].Item1.Data["file"]); - Assert.Equal("some error 2", _issues[1].Item1.Message); - Assert.False(_issues[1].Item1.Data.ContainsKey("file")); // does not contain file key - Assert.Equal(0, _commands.Count); - Assert.Equal(0, _messages.Where(x => !x.StartsWith("##[debug]")).Count()); - } - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void MatcherFileOutsideRepository() - { - var matchers = new IssueMatchersConfig - { - Matchers = - { - new IssueMatcherConfig - { - Owner = "my-matcher-1", - Patterns = new[] - { - new IssuePatternConfig - { - Pattern = @"(.+): (.+)", - File = 1, - Message = 2, - }, - }, - }, - }, - }; - using (var hostContext = Setup(matchers: matchers)) - using (_outputManager) - { - // Setup github.workspace - var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work); - ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory)); - Directory.CreateDirectory(workDirectory); - var workspaceDirectory = Path.Combine(workDirectory, "workspace"); - Directory.CreateDirectory(workspaceDirectory); - _executionContext.Setup(x => x.GetGitHubContext("workspace")).Returns(workspaceDirectory); + // Setup some git repositories + // /workflow-repo + // /workflow-repo/nested-other-repo + // /other-repo + // /other-repo/nested-workflow-repo + var workflowRepository = Path.Combine(workspaceDirectory, "workflow-repo"); + var nestedOtherRepository = Path.Combine(workspaceDirectory, "workflow-repo", "nested-other-repo"); + var otherRepository = Path.Combine(workspaceDirectory, workflowRepository, "nested-other-repo"); + var nestedWorkflowRepository = Path.Combine(workspaceDirectory, "other-repo", "nested-workflow-repo"); + await CreateRepository(hostContext, workflowRepository, "https://github.com/my-org/workflow-repo"); + await CreateRepository(hostContext, nestedOtherRepository, "https://github.com/my-org/other-repo"); + await CreateRepository(hostContext, otherRepository, "https://github.com/my-org/other-repo"); + await CreateRepository(hostContext, nestedWorkflowRepository, "https://github.com/my-org/workflow-repo"); // Create test files - var filePath1 = Path.Combine(workspaceDirectory, "some-file-1.txt"); - File.WriteAllText(filePath1, ""); - var workspaceSiblingDirectory = Path.Combine(Path.GetDirectoryName(workspaceDirectory), "workspace-sibling"); - Directory.CreateDirectory(workspaceSiblingDirectory); - var filePath2 = Path.Combine(workspaceSiblingDirectory, "some-file-2.txt"); - File.WriteAllText(filePath2, ""); + var file_noRepository = Path.Combine(workspaceDirectory, "no-repo.txt"); + var file_workflowRepository = Path.Combine(workflowRepository, "workflow-repo.txt"); + var file_workflowRepository_nestedDirectory = Path.Combine(workflowRepository, "subdir", "subdir2", "workflow-repo-nested-dir.txt"); + var file_workflowRepository_failsafe = Path.Combine(workflowRepository, "failsafe-subdir", "failsafe-subdir2", "failsafe-subdir3", "workflow-repo-failsafe.txt"); + var file_nestedOtherRepository = Path.Combine(nestedOtherRepository, "nested-other-repo"); + var file_otherRepository = Path.Combine(otherRepository, "other-repo.txt"); + var file_nestedWorkflowRepository = Path.Combine(nestedWorkflowRepository, "nested-workflow-repo.txt"); + foreach (var file in new[] { file_noRepository, file_workflowRepository, file_workflowRepository_nestedDirectory, file_workflowRepository_failsafe, file_nestedOtherRepository, file_otherRepository, file_nestedWorkflowRepository }) + { + Directory.CreateDirectory(Path.GetDirectoryName(file)); + File.WriteAllText(file, ""); + } // Process - Process($"{filePath1}: some error 1"); // file exists inside workspace - Process($"{filePath2}: some error 2"); // file exists outside workspace + Process($"{file_noRepository}: some error 1"); + Process($"{file_workflowRepository}: some error 2"); + Process($"{file_workflowRepository.Substring(workspaceDirectory.Length + 1)}: some error 3"); // Relative path from workspace dir + Process($"{file_workflowRepository_nestedDirectory}: some error 4"); + Process($"{file_workflowRepository_failsafe}: some error 5"); + Process($"{file_nestedOtherRepository}: some error 6"); + Process($"{file_otherRepository}: some error 7"); + Process($"{file_nestedWorkflowRepository}: some error 8"); + + Assert.Equal(8, _issues.Count); - Assert.Equal(2, _issues.Count); Assert.Equal("some error 1", _issues[0].Item1.Message); - Assert.Equal("some-file-1.txt", _issues[0].Item1.Data["file"]); + Assert.False(_issues[0].Item1.Data.ContainsKey("file")); + Assert.Equal("some error 2", _issues[1].Item1.Message); - Assert.False(_issues[1].Item1.Data.ContainsKey("file")); // does not contain file key - Assert.Equal(0, _commands.Count); - Assert.Equal(0, _messages.Where(x => !x.StartsWith("##[debug]")).Count()); + Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[1].Item1.Data["file"]); + + Assert.Equal("some error 3", _issues[2].Item1.Message); + Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[2].Item1.Data["file"]); + + Assert.Equal("some error 4", _issues[3].Item1.Message); + Assert.Equal(file_workflowRepository_nestedDirectory.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[3].Item1.Data["file"]); + + Assert.Equal("some error 5", _issues[4].Item1.Message); + Assert.False(_issues[4].Item1.Data.ContainsKey("file")); + + Assert.Equal("some error 6", _issues[5].Item1.Message); + Assert.False(_issues[5].Item1.Data.ContainsKey("file")); + + Assert.Equal("some error 7", _issues[6].Item1.Message); + Assert.False(_issues[6].Item1.Data.ContainsKey("file")); + + Assert.Equal("some error 8", _issues[7].Item1.Message); + Assert.Equal(file_nestedWorkflowRepository.Substring(nestedWorkflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[7].Item1.Data["file"]); } + + Environment.SetEnvironmentVariable("RUNNER_TEST_GET_REPOSITORY_PATH_FAILSAFE", ""); } [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void MatcherFromPath() + public async void MatcherFromPath() { var matchers = new IssueMatchersConfig { @@ -828,22 +776,26 @@ namespace GitHub.Runner.Common.Tests.Worker using (var hostContext = Setup(matchers: matchers)) using (_outputManager) { - // Setup github.workspace + // Setup github.workspace, github.repository var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work); ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory)); Directory.CreateDirectory(workDirectory); var workspaceDirectory = Path.Combine(workDirectory, "workspace"); Directory.CreateDirectory(workspaceDirectory); _executionContext.Setup(x => x.GetGitHubContext("workspace")).Returns(workspaceDirectory); + _executionContext.Setup(x => x.GetGitHubContext("repository")).Returns("my-org/workflow-repo"); + + // Setup a git repository + var repositoryPath = Path.Combine(workspaceDirectory, "workflow-repo"); + await CreateRepository(hostContext, repositoryPath, "https://github.com/my-org/workflow-repo"); // Create a test file - Directory.CreateDirectory(Path.Combine(workspaceDirectory, "some-directory")); - Directory.CreateDirectory(Path.Combine(workspaceDirectory, "some-project", "some-directory")); - var filePath = Path.Combine(workspaceDirectory, "some-project", "some-directory", "some-file.txt"); + var filePath = Path.Combine(repositoryPath, "some-project", "some-directory", "some-file.txt"); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)); File.WriteAllText(filePath, ""); // Process - Process("some-directory/some-file.txt: some error [some-project/some-project.proj]"); + Process("some-directory/some-file.txt: some error [workflow-repo/some-project/some-project.proj]"); Assert.Equal(1, _issues.Count); Assert.Equal("some error", _issues[0].Item1.Message); Assert.Equal("some-project/some-directory/some-file.txt", _issues[0].Item1.Data["file"]); @@ -931,5 +883,22 @@ namespace GitHub.Runner.Common.Tests.Worker { _outputManager.OnDataReceived(null, new ProcessDataReceivedEventArgs(line)); } + + private async Task CreateRepository(TestHostContext hostConetxt, string path, string url) + { + Directory.CreateDirectory(path); + var gitPath = WhichUtil.Which("git", true); + var environment = new Dictionary(); + + using (var processInvoker = new ProcessInvoker(hostConetxt.GetTrace())) + { + await processInvoker.ExecuteAsync(path, gitPath, "init", environment, CancellationToken.None); + } + + using (var processInvoker = new ProcessInvoker(hostConetxt.GetTrace())) + { + await processInvoker.ExecuteAsync(path, gitPath, $"remote add origin {url}", environment, CancellationToken.None); + } + } } }