Add support for Step Summary (#1642)

pfleidi · robherley · web-flow · commit 1a0d588d3a5e · 2022-02-04T13:46:30.000-08:00
* First prototype of step summary environment variable

* Fix file contention issue

* Try to simplify cleaning up file references

* use step id as md file name, queue file attachment

* separate logic into attachment summary func

* Fix indentation

* Add (experimental) feature flag support

* reorganize summary upload determination logic

* file i/o exception handling + pr feedback

* Revert changes for now to reintroduce them later

* Add skeleton SetStepSummaryCommand

* Update step summary feature flag name

* Port ShouldUploadAttachment from previous iteration

* Port QueueStepSummaryUpload from previous iteration

* Improve exception handling when uploading attachment

* Add some minor logging improvements

* Refuse to upload files larger than 128k

* Implement secrets scrubbing

* Add TODO comment to remove debugging temp files

* Add first tests

* Add test for secret masking

* Add some naming/style fixes suggested in feedback

* inline check for feature flag

* Inline method for style consistency

* Make sure that scrubbed file doesn't exist before creating it

* Rename SetStepSummaryCommand to CreateStepSummaryCommand

* Fix error handling messages

* Fix file command name when registering extension

* Remove unnecessary file deletion

Co-authored-by: Rob Herley &lt;robherley@github.com&gt;
diff --git a/src/Runner.Common/ExtensionManager.cs b/src/Runner.Common/ExtensionManager.cs
@@ -60,6 +60,7 @@ private List<IExtension> LoadExtensions<T>() where T : class, IExtension
                 case "GitHub.Runner.Worker.IFileCommandExtension":
                     Add<T>(extensions, "GitHub.Runner.Worker.AddPathFileCommand, Runner.Worker");
                     Add<T>(extensions, "GitHub.Runner.Worker.SetEnvFileCommand, Runner.Worker");
+                    Add<T>(extensions, "GitHub.Runner.Worker.CreateStepSummaryCommand, Runner.Worker");
                     break;
                 case "GitHub.Runner.Listener.Check.ICheckExtension":
                     Add<T>(extensions, "GitHub.Runner.Listener.Check.InternetCheck, Runner.Listener");
diff --git a/src/Runner.Worker/FileCommandManager.cs b/src/Runner.Worker/FileCommandManager.cs
@@ -259,4 +259,71 @@ private static string ReadLine(
             return text.Substring(originalIndex, lfIndex - originalIndex);
         }
     }
+
+    public sealed class CreateStepSummaryCommand : RunnerService, IFileCommandExtension
+    {
+        private const int _attachmentSizeLimit = 128 * 1024;
+
+        public string ContextName => "step_summary";
+        public string FilePrefix => "step_summary_";
+
+        public Type ExtensionType => typeof(IFileCommandExtension);
+
+        public void ProcessCommand(IExecutionContext context, string filePath, ContainerInfo container)
+        {
+            if (!context.Global.Variables.GetBoolean("DistributedTask.UploadStepSummary") ?? true)
+            {
+                Trace.Info("Step Summary is disabled; skipping attachment upload");
+                return;
+            }
+
+            if (String.IsNullOrEmpty(filePath) || !File.Exists(filePath))
+            {
+                Trace.Info($"Step Summary file ({filePath}) does not exist; skipping attachment upload");
+                return;
+            }
+
+            try
+            {
+                var fileSize = new FileInfo(filePath).Length;
+                if (fileSize == 0)
+                {
+                    Trace.Info($"Step Summary file ({filePath}) is empty; skipping attachment upload");
+                    return;
+                }
+
+                if (fileSize > _attachmentSizeLimit)
+                {
+                    context.Error($"$GITHUB_STEP_SUMMARY supports content up a size of {_attachmentSizeLimit / 1024}k got {fileSize / 1024}k");
+                    Trace.Info($"Step Summary file ({filePath}) is too large ({fileSize} bytes); skipping attachment upload");
+
+                    return;
+                }
+
+                Trace.Verbose($"Step Summary file exists: {filePath} and has a file size of {fileSize} bytes");
+                var scrubbedFilePath = filePath + "-scrubbed";
+
+                using (var streamReader = new StreamReader(filePath))
+                using (var streamWriter = new StreamWriter(scrubbedFilePath))
+                {
+                    string line;
+                    while ((line = streamReader.ReadLine()) != null)
+                    {
+                        var maskedLine = HostContext.SecretMasker.MaskSecrets(line);
+                        streamWriter.WriteLine(maskedLine);
+                    }
+                }
+
+                var attachmentName = context.Id.ToString();
+
+                Trace.Info($"Queueing file ({filePath}) for attachment upload ({attachmentName})");
+                context.QueueAttachFile(ChecksAttachmentType.StepSummary, attachmentName, scrubbedFilePath);
+            }
+            catch (Exception e)
+            {
+                Trace.Error($"Error while processing file ({filePath}): {e}");
+                context.Error($"Failed to create step summary using 'GITHUB_STEP_SUMMARY': {e.Message}");
+            }
+        }
+    }
 }
diff --git a/src/Runner.Worker/GitHubContext.cs b/src/Runner.Worker/GitHubContext.cs
@@ -34,6 +34,7 @@ public sealed class GitHubContext : DictionaryContextData, IEnvironmentContextDa
             "run_number",
             "server_url",
             "sha",
+            "step_summary",
             "workflow",
             "workspace",
         };
diff --git a/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs b/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs
@@ -102,4 +102,10 @@ public class CoreAttachmentType
         public static readonly String FileAttachment = "DistributedTask.Core.FileAttachment";
         public static readonly String DiagnosticLog = "DistributedTask.Core.DiagnosticLog";
     }
+
+    [GenerateAllConstants]
+    public class ChecksAttachmentType
+    {
+        public static readonly String StepSummary = "Checks.Step.Summary";
+    }
 }
diff --git a/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs
@@ -0,0 +1,241 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+using System.Runtime.CompilerServices;
+using GitHub.Runner.Common.Util;
+using GitHub.Runner.Sdk;
+using GitHub.Runner.Worker;
+using Moq;
+using Xunit;
+using DTWebApi = GitHub.DistributedTask.WebApi;
+using GitHub.DistributedTask.WebApi;
+
+namespace GitHub.Runner.Common.Tests.Worker
+{
+    public sealed class CreateStepSummaryCommandL0
+    {
+        private Mock<IExecutionContext> _executionContext;
+        private List<Tuple<DTWebApi.Issue, string>> _issues;
+        private Variables _variables;
+        private string _rootDirectory;
+        private CreateStepSummaryCommand _createStepCommand;
+        private ITraceWriter _trace;
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_FeatureDisabled()
+        {
+            using (var hostContext = Setup(featureFlagState: "false"))
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "feature-off");
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_FileNull()
+        {
+            using (var hostContext = Setup())
+            {
+                _createStepCommand.ProcessCommand(_executionContext.Object, null, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_DirectoryNotFound()
+        {
+            using (var hostContext = Setup())
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "directory-not-found", "env");
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_FileNotFound()
+        {
+            using (var hostContext = Setup())
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "file-not-found");
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_EmptyFile()
+        {
+            using (var hostContext = Setup())
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "empty-file");
+                File.Create(stepSummaryFile).Dispose();
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_LargeFile()
+        {
+            using (var hostContext = Setup())
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "empty-file");
+                File.WriteAllBytes(stepSummaryFile, new byte[128 * 1024 + 1]);
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+                Assert.Equal(1, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_Simple()
+        {
+            using (var hostContext = Setup())
+            {
+                var stepSummaryFile = Path.Combine(_rootDirectory, "simple");
+                var content = new List<string>
+                {
+                    "# This is some markdown content",
+                    "",
+                    "## This is more markdown content",
+                };
+                WriteContent(stepSummaryFile, content);
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, _executionContext.Object.Id.ToString(), stepSummaryFile + "-scrubbed"), Times.Once());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void CreateStepSummaryCommand_ScrubSecrets()
+        {
+            using (var hostContext = Setup())
+            {
+                // configure secretmasker to actually mask secrets
+                hostContext.SecretMasker.AddRegex("Password=.*");
+                hostContext.SecretMasker.AddRegex("ghs_.*");
+
+                var stepSummaryFile = Path.Combine(_rootDirectory, "simple");
+                var scrubbedFile = stepSummaryFile + "-scrubbed";
+                var content = new List<string>
+                {
+                    "# Password=ThisIsMySecretPassword!",
+                    "",
+                    "# GITHUB_TOKEN ghs_verysecuretoken",
+                };
+                WriteContent(stepSummaryFile, content);
+
+                _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null);
+
+                var scrubbedFileContents = File.ReadAllText(scrubbedFile);
+                Assert.DoesNotContain("ThisIsMySecretPassword!", scrubbedFileContents);
+                Assert.DoesNotContain("ghs_verysecuretoken", scrubbedFileContents);
+
+                _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, _executionContext.Object.Id.ToString(), scrubbedFile), Times.Once());
+                Assert.Equal(0, _issues.Count);
+            }
+        }
+
+        private void WriteContent(
+            string path,
+            List<string> content,
+            string newline = null)
+        {
+            if (string.IsNullOrEmpty(newline))
+            {
+                newline = Environment.NewLine;
+            }
+
+            var encoding = new UTF8Encoding(true); // Emit BOM
+            var contentStr = string.Join(newline, content);
+            File.WriteAllText(path, contentStr, encoding);
+        }
+
+        private TestHostContext Setup([CallerMemberName] string name = "", string featureFlagState = "true")
+        {
+            _issues = new List<Tuple<DTWebApi.Issue, string>>();
+
+            var hostContext = new TestHostContext(this, name);
+
+            // Trace
+            _trace = hostContext.GetTrace();
+
+            _variables = new Variables(hostContext, new Dictionary<string, VariableValue>
+                {
+                    { "MySecretName", new VariableValue("My secret value", true) },
+                    { "DistributedTask.UploadStepSummary", featureFlagState },
+                });
+
+            // Directory for test data
+            var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work);
+            ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory));
+            Directory.CreateDirectory(workDirectory);
+            _rootDirectory = Path.Combine(workDirectory, nameof(CreateStepSummaryCommandL0));
+            Directory.CreateDirectory(_rootDirectory);
+
+            // Execution context
+            _executionContext = new Mock<IExecutionContext>();
+            _executionContext.Setup(x => x.Global)
+                .Returns(new GlobalContext
+                {
+                    EnvironmentVariables = new Dictionary<string, string>(VarUtil.EnvironmentVariableKeyComparer),
+                    WriteDebug = true,
+                    Variables = _variables,
+                });
+            _executionContext.Setup(x => x.AddIssue(It.IsAny<DTWebApi.Issue>(), It.IsAny<string>()))
+                .Callback((DTWebApi.Issue issue, string logMessage) =>
+                {
+                    _issues.Add(new Tuple<DTWebApi.Issue, string>(issue, logMessage));
+                    var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message;
+                    _trace.Info($"Issue '{issue.Type}': {message}");
+                });
+            _executionContext.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>()))
+                .Callback((string tag, string message) =>
+                {
+                    _trace.Info($"{tag}{message}");
+                });
+
+            //CreateStepSummaryCommand
+            _createStepCommand = new CreateStepSummaryCommand();
+            _createStepCommand.Initialize(hostContext);
+
+            return hostContext;
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -102,4 +102,10 @@ public class CoreAttachmentType`
`102`	`102`	`public static readonly String FileAttachment = "DistributedTask.Core.FileAttachment";`
`103`	`103`	`public static readonly String DiagnosticLog = "DistributedTask.Core.DiagnosticLog";`
`104`	`104`	`}`
	`105`	`+`
	`106`	`+ [GenerateAllConstants]`
	`107`	`+ public class ChecksAttachmentType`
	`108`	`+ {`
	`109`	`+ public static readonly String StepSummary = "Checks.Step.Summary";`
	`110`	`+ }`
`105`	`111`	`}`