diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 0f9410821c6..a34de69391c 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -1319,19 +1319,55 @@ public void PublishStepTelemetry() public void WriteWebhookPayload() { - // Makes directory for event_path data var tempDirectory = HostContext.GetDirectory(WellKnownDirectory.Temp); var workflowDirectory = Path.Combine(tempDirectory, "_github_workflow"); + var workflowFile = Path.Combine(workflowDirectory, "event.json"); + + // Makes directory for event_path data Directory.CreateDirectory(workflowDirectory); - var gitHubEvent = GetGitHubContext("event"); - // adds the GitHub event path/file if the event exists - if (gitHubEvent != null) - { - var workflowFile = Path.Combine(workflowDirectory, "event.json"); - Trace.Info($"Write event payload to {workflowFile}"); - File.WriteAllText(workflowFile, gitHubEvent, new UTF8Encoding(false)); - SetGitHubContext("event_path", workflowFile); + // The event payload is delivered with the job message and never changes for + // the lifetime of the job, so it only needs to be serialized and written once. + // Every step (including each step of a composite action and job hooks) calls + // this method; rewriting the file each time is wasted work and, when steps run + // concurrently (background/parallel steps), a rewrite from one step truncates + // the file while another step's process is reading $GITHUB_EVENT_PATH. + // Still rewrite if the file has gone missing (e.g. removed by a previous step) + // so later steps always have a valid event file. + lock (Global.EventPayloadLock) + { + if (Global.EventPayloadWritten && File.Exists(workflowFile)) + { + SetGitHubContext("event_path", workflowFile); + return; + } + + var gitHubEvent = GetGitHubContext("event"); + + // adds the GitHub event path/file if the event exists + if (gitHubEvent != null) + { + Trace.Info($"Write event payload to {workflowFile}"); + + // Write to a temporary file in the same directory and move it into + // place so a concurrent reader never observes a partially written file. + var tempFile = Path.Combine(workflowDirectory, $".event.json.{Guid.NewGuid():N}.tmp"); + try + { + File.WriteAllText(tempFile, gitHubEvent, new UTF8Encoding(false)); + File.Move(tempFile, workflowFile, overwrite: true); + } + finally + { + if (File.Exists(tempFile)) + { + IOUtil.DeleteFile(tempFile); + } + } + + Global.EventPayloadWritten = true; + SetGitHubContext("event_path", workflowFile); + } } } diff --git a/src/Runner.Worker/GlobalContext.cs b/src/Runner.Worker/GlobalContext.cs index c2db20bd5ac..97e48399b66 100644 --- a/src/Runner.Worker/GlobalContext.cs +++ b/src/Runner.Worker/GlobalContext.cs @@ -15,6 +15,10 @@ public sealed class GlobalContext public ContainerInfo Container { get; set; } public List Endpoints { get; set; } public IDictionary EnvironmentVariables { get; set; } + // Guards the once-per-job write of $GITHUB_EVENT_PATH (event.json), which + // can be reached concurrently from background/parallel steps. + public object EventPayloadLock { get; } = new object(); + public bool EventPayloadWritten { get; set; } public PlanFeatures Features { get; set; } public IList FileTable { get; set; } public IDictionary> JobDefaults { get; set; } diff --git a/src/Test/L0/Worker/ExecutionContextL0.cs b/src/Test/L0/Worker/ExecutionContextL0.cs index 553c8b154ff..d4a39a0d451 100644 --- a/src/Test/L0/Worker/ExecutionContextL0.cs +++ b/src/Test/L0/Worker/ExecutionContextL0.cs @@ -1,10 +1,13 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Runtime.CompilerServices; using System.Threading; +using System.Threading.Tasks; using GitHub.DistributedTask.Pipelines.ContextData; using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Common; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Handlers; @@ -1423,6 +1426,178 @@ public void InitializeJob_WorkflowIdentityNotSet_WhenServerSendsNoData() } } + private Runner.Worker.ExecutionContext CreateJobContextWithEvent(TestHostContext hc, int childCount, out List children) + { + TaskOrchestrationPlanReference plan = new(); + TimelineReference timeline = new(); + Guid jobId = Guid.NewGuid(); + string jobName = "some job name"; + var jobRequest = new Pipelines.AgentJobRequestMessage(plan, timeline, jobId, jobName, jobName, null, null, null, new Dictionary(), new List(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List(), null, null, null, null, null); + jobRequest.Resources.Repositories.Add(new Pipelines.RepositoryResource() + { + Alias = Pipelines.PipelineConstants.SelfAlias, + Id = "github", + Version = "sha1" + }); + var githubEvent = new Pipelines.ContextData.DictionaryContextData(); + githubEvent["ref"] = new Pipelines.ContextData.StringContextData("refs/heads/main"); + var repository = new Pipelines.ContextData.DictionaryContextData(); + repository["full_name"] = new Pipelines.ContextData.StringContextData("actions/runner"); + // Make the payload large enough that a non-atomic rewrite is observable. + repository["description"] = new Pipelines.ContextData.StringContextData(new string('x', 256 * 1024)); + githubEvent["repository"] = repository; + var github = new Pipelines.ContextData.DictionaryContextData(); + github["event"] = githubEvent; + jobRequest.ContextData["github"] = github; + + var jobServerQueue = new Mock(); + jobServerQueue.Setup(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.IsAny())); + hc.SetSingleton(jobServerQueue.Object); + for (var i = 0; i < childCount + 1; i++) + { + hc.EnqueueInstance(new Mock().Object); + } + + var jobContext = new Runner.Worker.ExecutionContext(); + jobContext.Initialize(hc); + jobContext.InitializeJob(jobRequest, CancellationToken.None); + + children = new List(); + for (var i = 0; i < childCount; i++) + { + var child = jobContext.CreateChild(Guid.NewGuid(), $"step{i}", $"step{i}", null, null, ActionRunStage.Main); + // Background and composite steps get their own copy of the github context. + child.ExpressionValues["github"] = (child.ExpressionValues["github"] as GitHubContext).ShallowCopy(); + children.Add(child); + } + + return jobContext; + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void WriteWebhookPayload_WritesEventFileAndSetsEventPath() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobContext = CreateJobContextWithEvent(hc, 1, out var children); + var expectedFile = Path.Combine(hc.GetDirectory(WellKnownDirectory.Temp), "_github_workflow", "event.json"); + + children[0].WriteWebhookPayload(); + + Assert.True(File.Exists(expectedFile)); + Assert.Equal(expectedFile, children[0].GetGitHubContext("event_path")); + var parsed = Newtonsoft.Json.Linq.JObject.Parse(File.ReadAllText(expectedFile)); + Assert.Equal("refs/heads/main", (string)parsed["ref"]); + Assert.Equal("actions/runner", (string)parsed["repository"]["full_name"]); + Assert.Empty(Directory.GetFiles(Path.GetDirectoryName(expectedFile), "*.tmp")); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void WriteWebhookPayload_OnlyWritesOncePerJob() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobContext = CreateJobContextWithEvent(hc, 2, out var children); + var expectedFile = Path.Combine(hc.GetDirectory(WellKnownDirectory.Temp), "_github_workflow", "event.json"); + + children[0].WriteWebhookPayload(); + Assert.True(File.Exists(expectedFile)); + + // A later step must not rewrite a file that is already in place. + File.WriteAllText(expectedFile, "sentinel"); + children[1].WriteWebhookPayload(); + + Assert.Equal("sentinel", File.ReadAllText(expectedFile)); + Assert.Equal(expectedFile, children[1].GetGitHubContext("event_path")); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void WriteWebhookPayload_RewritesIfFileWasRemoved() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobContext = CreateJobContextWithEvent(hc, 2, out var children); + var expectedFile = Path.Combine(hc.GetDirectory(WellKnownDirectory.Temp), "_github_workflow", "event.json"); + + children[0].WriteWebhookPayload(); + Assert.True(File.Exists(expectedFile)); + + File.Delete(expectedFile); + children[1].WriteWebhookPayload(); + + Assert.True(File.Exists(expectedFile)); + var parsed = Newtonsoft.Json.Linq.JObject.Parse(File.ReadAllText(expectedFile)); + Assert.Equal("refs/heads/main", (string)parsed["ref"]); + Assert.Equal(expectedFile, children[1].GetGitHubContext("event_path")); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task WriteWebhookPayload_ConcurrentStepsNeverExposePartialFile() + { + using (TestHostContext hc = CreateTestContext()) + { + const int stepCount = 8; + var jobContext = CreateJobContextWithEvent(hc, stepCount, out var children); + var expectedFile = Path.Combine(hc.GetDirectory(WellKnownDirectory.Temp), "_github_workflow", "event.json"); + + // First step writes the file. + children[0].WriteWebhookPayload(); + var expectedLength = new FileInfo(expectedFile).Length; + + // Simulate a step's process reading $GITHUB_EVENT_PATH while sibling + // background steps (and each step of a composite action) start up. + using var stop = new CancellationTokenSource(); + var badReads = 0; + var reader = Task.Run(() => + { + while (!stop.IsCancellationRequested) + { + try + { + var content = File.ReadAllText(expectedFile); + if (content.Length != expectedLength) + { + Interlocked.Increment(ref badReads); + } + } + catch (IOException) + { + Interlocked.Increment(ref badReads); + } + } + }); + + var writers = children.Select(child => Task.Run(() => + { + for (var i = 0; i < 50; i++) + { + child.WriteWebhookPayload(); + } + })).ToArray(); + await Task.WhenAll(writers); + stop.Cancel(); + await reader; + + Assert.Equal(0, badReads); + Assert.Equal(expectedLength, new FileInfo(expectedFile).Length); + foreach (var child in children) + { + Assert.Equal(expectedFile, child.GetGitHubContext("event_path")); + } + } + } + private bool ExpressionValuesAssertEqual(DictionaryContextData expect, DictionaryContextData actual) { foreach (var key in expect.Keys.ToList())