Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

commands translate file path from container action #331

Merged
merged 3 commits into from
Feb 13, 2020
Merged

commands translate file path from container action #331

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
717b2b5
commands translate file path from container action
ericsciple Feb 12, 2020
14e8299
.
ericsciple Feb 12, 2020
8a33711
.
ericsciple Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions src/Runner.Worker/ActionCommandManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using GitHub.DistributedTask.Pipelines;
using GitHub.DistributedTask.WebApi;
using GitHub.Runner.Common.Util;
using GitHub.Runner.Worker.Container;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

despite the diff, the actual change is pretty small. The diff looks bad due to the signature change of IActionCommandExtension.ProcessCommand()

using System;
using System.Collections.Generic;
using System.IO;
Expand All @@ -15,14 +16,14 @@ public interface IActionCommandManager : IRunnerService
{
void EnablePluginInternalCommand();
void DisablePluginInternalCommand();
bool TryProcessCommand(IExecutionContext context, string input);
bool TryProcessCommand(IExecutionContext context, string input, ContainerInfo container);
}

public sealed class ActionCommandManager : RunnerService, IActionCommandManager
{
private const string _stopCommand = "stop-commands";
private readonly Dictionary<string, IActionCommandExtension> _commandExtensions = new Dictionary<string, IActionCommandExtension>(StringComparer.OrdinalIgnoreCase);
private HashSet<string> _registeredCommands = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
private readonly HashSet<string> _registeredCommands = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
private readonly object _commandSerializeLock = new object();
private bool _stopProcessCommand = false;
private string _stopToken = null;
Expand Down Expand Up @@ -58,7 +59,7 @@ public void DisablePluginInternalCommand()
_registeredCommands.Remove("internal-set-repo-path");
}

public bool TryProcessCommand(IExecutionContext context, string input)
public bool TryProcessCommand(IExecutionContext context, string input, ContainerInfo container)
{
if (string.IsNullOrEmpty(input))
{
Expand Down Expand Up @@ -114,7 +115,7 @@ public bool TryProcessCommand(IExecutionContext context, string input)

try
{
extension.ProcessCommand(context, input, actionCommand);
extension.ProcessCommand(context, input, actionCommand, container);
}
catch (Exception ex)
{
Expand All @@ -140,7 +141,7 @@ public interface IActionCommandExtension : IExtension
string Command { get; }
bool OmitEcho { get; }

void ProcessCommand(IExecutionContext context, string line, ActionCommand command);
void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container);
}

public sealed class InternalPluginSetRepoPathCommandExtension : RunnerService, IActionCommandExtension
Expand All @@ -150,7 +151,7 @@ public sealed class InternalPluginSetRepoPathCommandExtension : RunnerService, I

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
if (!command.Properties.TryGetValue(SetRepoPathCommandProperties.repoFullName, out string repoFullName) || string.IsNullOrEmpty(repoFullName))
{
Expand Down Expand Up @@ -180,7 +181,7 @@ public sealed class SetEnvCommandExtension : RunnerService, IActionCommandExtens

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
if (!command.Properties.TryGetValue(SetEnvCommandProperties.Name, out string envName) || string.IsNullOrEmpty(envName))
{
Expand All @@ -205,7 +206,7 @@ public sealed class SetOutputCommandExtension : RunnerService, IActionCommandExt

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
if (!command.Properties.TryGetValue(SetOutputCommandProperties.Name, out string outputName) || string.IsNullOrEmpty(outputName))
{
Expand All @@ -229,7 +230,7 @@ public sealed class SaveStateCommandExtension : RunnerService, IActionCommandExt

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
if (!command.Properties.TryGetValue(SaveStateCommandProperties.Name, out string stateName) || string.IsNullOrEmpty(stateName))
{
Expand All @@ -253,7 +254,7 @@ public sealed class AddMaskCommandExtension : RunnerService, IActionCommandExten

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
if (string.IsNullOrWhiteSpace(command.Data))
{
Expand All @@ -279,7 +280,7 @@ public sealed class AddPathCommandExtension : RunnerService, IActionCommandExten

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
ArgUtil.NotNullOrEmpty(command.Data, "path");
context.PrependPath.RemoveAll(x => string.Equals(x, command.Data, StringComparison.CurrentCulture));
Expand All @@ -294,7 +295,7 @@ public sealed class AddMatcherCommandExtension : RunnerService, IActionCommandEx

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
var file = command.Data;

Expand All @@ -306,9 +307,9 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand
}

// Translate file path back from container path
if (context.Container != null)
if (container != null)
{
file = context.Container.TranslateToHostPath(file);
file = container.TranslateToHostPath(file);
ericsciple marked this conversation as resolved.
Show resolved Hide resolved
}

// Root the path
Expand Down Expand Up @@ -341,7 +342,7 @@ public sealed class RemoveMatcherCommandExtension : RunnerService, IActionComman

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
command.Properties.TryGetValue(RemoveMatcherCommandProperties.Owner, out string owner);
var file = command.Data;
Expand Down Expand Up @@ -369,9 +370,9 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand
else
{
// Translate file path back from container path
if (context.Container != null)
if (container != null)
{
file = context.Container.TranslateToHostPath(file);
file = container.TranslateToHostPath(file);
}

// Root the path
Expand Down Expand Up @@ -409,7 +410,7 @@ public sealed class DebugCommandExtension : RunnerService, IActionCommandExtensi

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string inputLine, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string inputLine, ActionCommand command, ContainerInfo container)
{
context.Debug(command.Data);
}
Expand Down Expand Up @@ -437,7 +438,7 @@ public abstract class IssueCommandExtension : RunnerService, IActionCommandExten

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string inputLine, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string inputLine, ActionCommand command, ContainerInfo container)
{
command.Properties.TryGetValue(IssueCommandProperties.File, out string file);
command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
Expand All @@ -454,10 +455,10 @@ public void ProcessCommand(IExecutionContext context, string inputLine, ActionCo
{
issue.Category = "Code";

if (context.Container != null)
if (container != null)
{
// Translate file path back from container path
file = context.Container.TranslateToHostPath(file);
file = container.TranslateToHostPath(file);
command.Properties[IssueCommandProperties.File] = file;
}

Expand Down Expand Up @@ -517,7 +518,7 @@ public abstract class GroupingCommandExtension : RunnerService, IActionCommandEx

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
var data = this is GroupCommandExtension ? command.Data : string.Empty;
context.Output($"##[{Command}]{data}");
Expand All @@ -531,7 +532,7 @@ public sealed class EchoCommandExtension : RunnerService, IActionCommandExtensio

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command)
public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
ArgUtil.NotNullOrEmpty(command.Data, "value");

Expand Down
2 changes: 1 addition & 1 deletion src/Runner.Worker/Handlers/OutputManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e)
{
// This does not need to be inside of a critical section.
// The logging queues and command handlers are thread-safe.
if (_commandManager.TryProcessCommand(_executionContext, line))
if (_commandManager.TryProcessCommand(_executionContext, line, _container))
Copy link
Collaborator Author

@ericsciple ericsciple Feb 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OutputManager is the only place that calls commandManager.TryProcessCommand

In the OutputManager ctor, _container is set to "step container ?? job container".

The change here is to pass the correct container for the command manager to use when translating paths.

{
return;
}
Expand Down
30 changes: 15 additions & 15 deletions src/Test/L0/Worker/ActionCommandManagerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void EnablePluginInternalCommand()

commandManager.EnablePluginInternalCommand();

Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath", null));

directoryManager.Verify(x => x.UpdateRepositoryDirectory(_ec.Object, "actions/runner", "somepath", true), Times.Once);
}
Expand Down Expand Up @@ -94,11 +94,11 @@ public void DisablePluginInternalCommand()

commandManager.EnablePluginInternalCommand();

Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath", null));

commandManager.DisablePluginInternalCommand();

Assert.False(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath"));
Assert.False(commandManager.TryProcessCommand(_ec.Object, "##[internal-set-repo-path repoFullName=actions/runner;workspaceRepo=true]somepath", null));

directoryManager.Verify(x => x.UpdateRepositoryDirectory(_ec.Object, "actions/runner", "somepath", true), Times.Once);
}
Expand Down Expand Up @@ -141,10 +141,10 @@ public void StopProcessCommand()
ActionCommandManager commandManager = new ActionCommandManager();
commandManager.Initialize(_hc);

Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[stop-commands]stopToken"));
Assert.False(commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[stopToken]"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[stop-commands]stopToken", null));
Assert.False(commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[stopToken]", null));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "##[set-env name=foo]bar", null));
}
}

Expand Down Expand Up @@ -178,16 +178,16 @@ public void EchoProcessCommand()

Assert.False(_ec.Object.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::on"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::on", null));
Assert.True(_ec.Object.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::off"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::off", null));
Assert.False(_ec.Object.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::ON"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::ON", null));
Assert.True(_ec.Object.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::Off "));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::Off ", null));
Assert.False(_ec.Object.EchoOnActionCommand);
}
}
Expand Down Expand Up @@ -249,10 +249,10 @@ public void EchoProcessCommandDebugOn()

Assert.True(_ec.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec, "::echo::off"));
Assert.True(commandManager.TryProcessCommand(_ec, "::echo::off", null));
Assert.False(_ec.EchoOnActionCommand);

Assert.True(commandManager.TryProcessCommand(_ec, "::echo::on"));
Assert.True(commandManager.TryProcessCommand(_ec, "::echo::on", null));
Assert.True(_ec.EchoOnActionCommand);
}
}
Expand Down Expand Up @@ -288,12 +288,12 @@ public void EchoProcessCommandInvalid()

// Echo commands below are considered "processed", but are invalid
// 1. Invalid echo value
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::invalid"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::invalid", null));
Assert.Equal(TaskResult.Failed, _ec.Object.CommandResult);
Assert.False(_ec.Object.EchoOnActionCommand);

// 2. No value
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::"));
Assert.True(commandManager.TryProcessCommand(_ec.Object, "::echo::", null));
Assert.Equal(TaskResult.Failed, _ec.Object.CommandResult);
Assert.False(_ec.Object.EchoOnActionCommand);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Test/L0/Worker/OutputManagerL0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,8 +973,8 @@ private TestHostContext Setup(
});

_commandManager = new Mock<IActionCommandManager>();
_commandManager.Setup(x => x.TryProcessCommand(It.IsAny<IExecutionContext>(), It.IsAny<string>()))
.Returns((IExecutionContext executionContext, string line) =>
_commandManager.Setup(x => x.TryProcessCommand(It.IsAny<IExecutionContext>(), It.IsAny<string>(), It.IsAny<ContainerInfo>()))
.Returns((IExecutionContext executionContext, string line, ContainerInfo container) =>
{
if (line.IndexOf("##[some-command]") >= 0)
{
Expand Down