Skip to content

Commit 1364dbf

Browse files
brdeyobrdeyo
authored
Update to better handle path type (absolute, relative) identifications. Handle nested quotation marks in commands for for PowerShell executor. (#627)
1 parent 00f4f5f commit 1364dbf

15 files changed

Lines changed: 485 additions & 216 deletions

File tree

‎src/VirtualClient/VirtualClient.Actions.UnitTests/ScriptExecutor/PowershellExecutorTests.cs‎

Lines changed: 94 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ namespace VirtualClient.Actions
1818

1919
[TestFixture]
2020
[Category("Unit")]
21-
public class PowershellExecutorTests
21+
public class PowerShellExecutorTests
2222
{
23-
private static readonly string ExamplesDirectory = MockFixture.GetDirectory(typeof(PowershellExecutorTests), "Examples", "ScriptExecutor");
23+
private static readonly string ExamplesDirectory = MockFixture.GetDirectory(typeof(PowerShellExecutorTests), "Examples", "ScriptExecutor");
2424

2525
private MockFixture fixture;
2626
private DependencyPath mockPackage;
@@ -35,7 +35,7 @@ public void SetupTest(PlatformID platform = PlatformID.Win32NT)
3535

3636
this.fixture.Dependencies.RemoveAll<IEnumerable<IBlobManager>>();
3737

38-
this.exampleResults = File.ReadAllText(Path.Combine(PowershellExecutorTests.ExamplesDirectory, "validJsonExample.json"));
38+
this.exampleResults = File.ReadAllText(Path.Combine(PowerShellExecutorTests.ExamplesDirectory, "validJsonExample.json"));
3939

4040
this.fixture.FileSystem.Setup(fe => fe.File.Exists(It.IsAny<string>()))
4141
.Returns(true);
@@ -127,46 +127,92 @@ public void SetupTest(PlatformID platform = PlatformID.Win32NT)
127127

128128
this.fixture.Parameters = new Dictionary<string, IConvertible>()
129129
{
130-
{ nameof(PowershellExecutor.PackageName), "workloadPackage" },
131-
{ nameof(PowershellExecutor.Scenario), "GenericScriptWorkload" },
132-
{ nameof(PowershellExecutor.CommandLine), "parameter1 parameter2" },
133-
{ nameof(PowershellExecutor.ScriptPath), "genericScript.ps1" },
134-
{ nameof(PowershellExecutor.LogPaths), "*.log;*.txt;*.json" },
135-
{ nameof(PowershellExecutor.ToolName), "GenericTool" },
136-
{ nameof(PowershellExecutor.UsePwsh), false }
130+
{ nameof(PowerShellExecutor.PackageName), "workloadPackage" },
131+
{ nameof(PowerShellExecutor.Scenario), "GenericScriptWorkload" },
132+
{ nameof(PowerShellExecutor.CommandLine), "parameter1 parameter2" },
133+
{ nameof(PowerShellExecutor.ScriptPath), "genericScript.ps1" },
134+
{ nameof(PowerShellExecutor.LogPaths), "*.log;*.txt;*.json" },
135+
{ nameof(PowerShellExecutor.ToolName), "GenericTool" }
137136
};
138137

139138
this.fixture.ProcessManager.OnCreateProcess = (command, arguments, directory) => this.fixture.Process;
140139
}
141140

142141
[Test]
143-
[TestCase(PlatformID.Win32NT, @"\win-x64", @"genericScript.ps1", true, false, "powershell")]
144-
[TestCase(PlatformID.Win32NT, @"\win-x64", @"genericScript.ps1", false, false, "powershell")]
145-
[TestCase(PlatformID.Win32NT, @"\win-x64", @"genericScript.ps1", true, true, "pwsh")]
146-
[TestCase(PlatformID.Win32NT, @"\win-x64", @"genericScript.ps1", false, true, "pwsh")]
142+
[TestCase(@"genericScript.ps1", "powershell")]
143+
[TestCase(@"genericScript.ps1", "powershell")]
144+
[TestCase(@"genericScript.ps1", "powershell")]
145+
[TestCase(@"genericScript.ps1", "powershell.exe")]
146+
[TestCase(@"genericScript.ps1", "PowerShell.exe")]
147+
[TestCase(@"genericScript.ps1", @"C:\Any\Custom\Location\powershell.exe")]
148+
[TestCase(@"genericScript.ps1", "pwsh")]
149+
[TestCase(@"genericScript.ps1", "pwsh.exe")]
150+
[TestCase(@"genericScript.ps1", @"C:\Any\Custom\Location\pwsh.exe")]
147151
[Platform(Exclude = "Unix,Linux,MacOsX")]
148-
public async Task PowershellExecutorExecutesTheCorrectWorkloadCommands(PlatformID platform, string platformSpecificPath, string command, bool runElevated, bool usePwsh, string executorType)
152+
public async Task PowershellExecutorExecutesTheCorrectWorkloadCommands_Windows_Scenarios(string command, string executable)
149153
{
150-
this.SetupTest(platform);
151-
this.fixture.Parameters[nameof(PowershellExecutor.RunElevated)] = runElevated;
152-
this.fixture.Parameters[nameof(PowershellExecutor.ScriptPath)] = command;
153-
this.fixture.Parameters[nameof(PowershellExecutor.UsePwsh)] = usePwsh;
154+
this.SetupTest(PlatformID.Win32NT);
155+
this.fixture.Parameters[nameof(PowerShellExecutor.ScriptPath)] = command;
156+
this.fixture.Parameters[nameof(PowerShellExecutor.Executable)] = executable;
154157

155-
string fullCommand = $"{this.mockPackage.Path}{platformSpecificPath}\\{command} parameter1 parameter2";
158+
string fullCommand = $"{this.mockPackage.Path}\\win-x64\\{command} parameter1 parameter2";
156159

157-
using (TestPowershellExecutor executor = new TestPowershellExecutor(this.fixture))
160+
using (TestPowerShellExecutor executor = new TestPowerShellExecutor(this.fixture))
158161
{
159162
bool commandExecuted = false;
163+
await executor.InitializeAsync(EventContext.None, CancellationToken.None);
164+
string workingDirectory = executor.ExecutableDirectory;
165+
166+
string expectedCommand = $"{executable} -ExecutionPolicy Bypass -NoProfile -NonInteractive -WindowStyle Hidden -Command \"cd '{workingDirectory}';{fullCommand}\"";
167+
this.fixture.ProcessManager.OnCreateProcess = (exe, arguments, workingDirectory) =>
168+
{
169+
if(expectedCommand == $"{exe} {arguments}")
170+
{
171+
commandExecuted = true;
172+
}
173+
174+
return new InMemoryProcess
175+
{
176+
StartInfo = new ProcessStartInfo
177+
{
178+
FileName = exe,
179+
Arguments = arguments
180+
},
181+
ExitCode = 0,
182+
OnStart = () => true,
183+
OnHasExited = () => true
184+
};
185+
};
160186

161-
await executor.InitializeAsync(EventContext.None, CancellationToken.None)
162-
.ConfigureAwait(false);
187+
await executor.ExecuteAsync(CancellationToken.None);
163188

189+
Assert.DoesNotThrowAsync(() => executor.ExecuteAsync(CancellationToken.None));
190+
Assert.IsTrue(commandExecuted);
191+
}
192+
}
193+
194+
[Test]
195+
[TestCase("genericScript.ps1", "pwsh")]
196+
[TestCase("genericScript.ps1", @"/home/any/custom/location/pwsh")]
197+
[TestCase("genericScript.ps1", "sudo pwsh")]
198+
public async Task PowershellExecutorExecutesTheCorrectWorkloadCommands_Unix_Scenarios(string command, string executable)
199+
{
200+
this.SetupTest(PlatformID.Unix);
201+
this.fixture.Parameters[nameof(PowerShellExecutor.ScriptPath)] = command;
202+
this.fixture.Parameters[nameof(PowerShellExecutor.Executable)] = executable;
203+
204+
string fullCommand = $"{this.mockPackage.Path}/linux-x64/{command} parameter1 parameter2";
205+
206+
using (TestPowerShellExecutor executor = new TestPowerShellExecutor(this.fixture))
207+
{
208+
bool commandExecuted = false;
209+
await executor.InitializeAsync(EventContext.None, CancellationToken.None);
164210
string workingDirectory = executor.ExecutableDirectory;
165211

166-
string expectedCommand = $"{executorType} -ExecutionPolicy Bypass -NoProfile -NonInteractive -WindowStyle Hidden -Command \"cd '{workingDirectory}';{fullCommand}\"";
212+
string expectedCommand = $"{executable} -ExecutionPolicy Bypass -NoProfile -NonInteractive -WindowStyle Hidden -Command \"cd '{workingDirectory}';{fullCommand}\"";
167213
this.fixture.ProcessManager.OnCreateProcess = (exe, arguments, workingDirectory) =>
168214
{
169-
if(expectedCommand == $"{exe} {arguments}")
215+
if (expectedCommand == $"{exe} {arguments}")
170216
{
171217
commandExecuted = true;
172218
}
@@ -184,37 +230,51 @@ await executor.InitializeAsync(EventContext.None, CancellationToken.None)
184230
};
185231
};
186232

187-
await executor.ExecuteAsync(CancellationToken.None)
188-
.ConfigureAwait(false);
233+
await executor.ExecuteAsync(CancellationToken.None);
189234

190235
Assert.DoesNotThrowAsync(() => executor.ExecuteAsync(CancellationToken.None));
191236
Assert.IsTrue(commandExecuted);
192237
}
193238
}
194239

195240
[Test]
196-
[TestCase(PlatformID.Win32NT, @"\win-x64\")]
197-
public void PowershellExecutorDoesNotThrowWhenTheWorkloadDoesNotProduceValidMetricsFile(PlatformID platform, string platformSpecificPath)
241+
public void PowershellExecutorDoesNotThrowWhenTheWorkloadDoesNotProduceValidMetricsFile()
198242
{
199-
this.SetupTest(platform);
200-
this.fixture.File.Setup(fe => fe.Exists($"{this.mockPackage.Path}{platformSpecificPath}test-metrics.json"))
243+
this.SetupTest(PlatformID.Win32NT);
244+
this.fixture.File.Setup(fe => fe.Exists($@"{this.mockPackage.Path}\win-x64\test-metrics.json"))
201245
.Returns(false);
202246

203-
using (TestPowershellExecutor executor = new TestPowershellExecutor(this.fixture))
247+
using (TestPowerShellExecutor executor = new TestPowerShellExecutor(this.fixture))
204248
{
205249
this.fixture.ProcessManager.OnCreateProcess = (command, arguments, directory) => this.fixture.Process;
206250

207251
Assert.DoesNotThrowAsync(() => executor.ExecuteAsync(CancellationToken.None));
208252
}
209253
}
210254

211-
private class TestPowershellExecutor : PowershellExecutor
255+
private class TestPowerShellExecutor : PowerShellExecutor
212256
{
213-
public TestPowershellExecutor(MockFixture fixture)
257+
public TestPowerShellExecutor(MockFixture fixture)
214258
: base(fixture.Dependencies, fixture.Parameters)
215259
{
216260
}
217261

262+
public new string ExecutablePath
263+
{
264+
get
265+
{
266+
return base.ExecutablePath;
267+
}
268+
}
269+
270+
public new string ExecutableDirectory
271+
{
272+
get
273+
{
274+
return base.ExecutableDirectory;
275+
}
276+
}
277+
218278
public new Task InitializeAsync(EventContext telemetryContext, CancellationToken cancellationToken)
219279
{
220280
return base.InitializeAsync(telemetryContext, cancellationToken);

‎src/VirtualClient/VirtualClient.Actions.UnitTests/ScriptExecutor/ScriptExecutorTests.cs‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,9 @@ public void ScriptExecutorMovesTheLogFilesToCorrectDirectory_Win(PlatformID plat
327327
this.mockFixture.File.Setup(fe => fe.Move(It.IsAny<string>(), It.IsAny<string>(), true))
328328
.Callback<string, string, bool>((sourcePath, destinitionPath, overwrite) =>
329329
{
330-
destinitionPathCorrect = Regex.IsMatch(
331-
destinitionPath,
332-
$"{logsDir}.{this.mockFixture.Parameters["ToolName"].ToString().ToLower()}.{this.mockFixture.Parameters["Scenario"].ToString().ToLower()}");
330+
destinitionPathCorrect = destinitionPath.StartsWith(this.mockFixture.Combine(
331+
logsDir,
332+
this.mockFixture.Parameters["ToolName"].ToString().ToLower()));
333333

334334
sourcePathCorrect = Regex.IsMatch(
335335
sourcePath,
@@ -363,9 +363,9 @@ public void ScriptExecutorMovesTheLogFilesToCorrectDirectory_Unix(PlatformID pla
363363
this.mockFixture.File.Setup(fe => fe.Move(It.IsAny<string>(), It.IsAny<string>(), true))
364364
.Callback<string, string, bool>((sourcePath, destinitionPath, overwrite) =>
365365
{
366-
destinitionPathCorrect = Regex.IsMatch(
367-
destinitionPath,
368-
$"{logsDir}.{this.mockFixture.Parameters["ToolName"].ToString().ToLower()}.{this.mockFixture.Parameters["Scenario"].ToString().ToLower()}");
366+
destinitionPathCorrect = destinitionPath.StartsWith(this.mockFixture.Combine(
367+
logsDir,
368+
this.mockFixture.Parameters["ToolName"].ToString().ToLower()));
369369

370370
sourcePathCorrect = Regex.IsMatch(
371371
sourcePath,
@@ -387,6 +387,22 @@ public TestScriptExecutor(MockFixture fixture)
387387
{
388388
}
389389

390+
public new string ExecutablePath
391+
{
392+
get
393+
{
394+
return base.ExecutablePath;
395+
}
396+
}
397+
398+
public new string ExecutableDirectory
399+
{
400+
get
401+
{
402+
return base.ExecutableDirectory;
403+
}
404+
}
405+
390406
public new Task InitializeAsync(EventContext telemetryContext, CancellationToken cancellationToken)
391407
{
392408
return base.InitializeAsync(telemetryContext, cancellationToken);

‎src/VirtualClient/VirtualClient.Actions/ScriptExecutor/PowershellExecutor.cs‎

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace VirtualClient.Actions
55
{
66
using System;
77
using System.Collections.Generic;
8+
using System.Globalization;
89
using System.Threading;
910
using System.Threading.Tasks;
1011
using Microsoft.Extensions.DependencyInjection;
@@ -15,26 +16,49 @@ namespace VirtualClient.Actions
1516
/// <summary>
1617
/// The Generic Script executor for Powershell
1718
/// </summary>
18-
public class PowershellExecutor : ScriptExecutor
19+
public class PowerShellExecutor : ScriptExecutor
1920
{
21+
private const string PowerShellExecutableName = "powershell";
22+
private const string PowerShell7ExecutableName = "pwsh";
23+
2024
/// <summary>
21-
/// Constructor for <see cref="PowershellExecutor"/>
25+
/// Constructor for <see cref="PowerShellExecutor"/>
2226
/// </summary>
2327
/// <param name="dependencies">Provides required dependencies to the component.</param>
2428
/// <param name="parameters">Parameters defined in the profile or supplied on the command line.</param>
25-
public PowershellExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
29+
public PowerShellExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
2630
: base(dependencies, parameters)
2731
{
32+
this.ApplyBackwardsCompatibility();
33+
}
34+
35+
/// <summary>
36+
/// The name (or path) of the PowerShell executable to use (e.g. pwsh, PowerShell.exe)
37+
/// </summary>
38+
public string Executable
39+
{
40+
get
41+
{
42+
return this.Parameters.GetValue<string>(nameof(this.Executable), PowerShellExecutor.PowerShellExecutableName);
43+
}
2844
}
2945

3046
/// <summary>
3147
/// The parameter specifies whether to use pwsh, by default it is false
3248
/// </summary>
33-
public bool UsePwsh
49+
private bool UsePwsh
3450
{
3551
get
3652
{
37-
return this.Parameters.GetValue<bool>(nameof(this.UsePwsh), false);
53+
// TODO:
54+
// Remove this property entirely.
55+
//
56+
// This is an indirect way to get to the name of the PowerShell executable (pwsh vs. PowerShell.exe).
57+
// There is no reason to do so indirectly. There are additional benefits to allowing the user to simply
58+
// specify the executable name:
59+
// 1) The usage is just as easy and the outcome is the same.
60+
// 2) A full path to a specific version of PowerShell can be supplied (i.e. flexibility for other use cases).
61+
throw new NotSupportedException("Design Correction. The PowerShell executable name or path should be specified explicitly (e.g. pwsh, PowerShell.exe).");
3862
}
3963
}
4064

@@ -45,26 +69,45 @@ protected override async Task ExecuteAsync(EventContext telemetryContext, Cancel
4569
{
4670
using (BackgroundOperations profiling = BackgroundOperations.BeginProfiling(this, cancellationToken))
4771
{
48-
string command = this.UsePwsh ? "pwsh" : "powershell";
49-
string commandArguments = SensitiveData.ObscureSecrets(
50-
$"-ExecutionPolicy Bypass -NoProfile -NonInteractive -WindowStyle Hidden -Command \"cd '{this.ExecutableDirectory}';{this.ExecutablePath} {this.CommandLine}\"");
72+
string command = this.Executable;
73+
74+
// Handle nested quotation mark parsing pitfalls.
75+
string commandLine = this.CommandLine.Replace("\"", "\\\"");
76+
string executablePath = this.ExecutablePath.Replace("\"", "\\\"");
77+
string commandArguments = $"-ExecutionPolicy Bypass -NoProfile -NonInteractive -WindowStyle Hidden -Command \"cd '{this.ExecutableDirectory}';{executablePath} {commandLine}\"";
5178

5279
telemetryContext
5380
.AddContext(nameof(command), command)
54-
.AddContext(nameof(commandArguments), commandArguments);
81+
.AddContext(nameof(commandArguments), SensitiveData.ObscureSecrets(commandArguments));
5582

5683
using (IProcessProxy process = await this.ExecuteCommandAsync(command, commandArguments, this.ExecutableDirectory, telemetryContext, cancellationToken, this.RunElevated))
5784
{
5885
if (!cancellationToken.IsCancellationRequested)
5986
{
60-
await this.LogProcessDetailsAsync(process, telemetryContext, this.ToolName);
61-
process.ThrowIfWorkloadFailed();
62-
63-
await this.CaptureMetricsAsync(process, telemetryContext, cancellationToken);
64-
await this.CaptureLogsAsync(cancellationToken);
87+
try
88+
{
89+
await this.LogProcessDetailsAsync(process, telemetryContext, this.ToolName);
90+
process.ThrowIfWorkloadFailed();
91+
}
92+
finally
93+
{
94+
await this.CaptureMetricsAsync(process, telemetryContext, cancellationToken);
95+
await this.CaptureLogsAsync(cancellationToken);
96+
}
6597
}
6698
}
6799
}
68100
}
101+
102+
private void ApplyBackwardsCompatibility()
103+
{
104+
if (this.Parameters.TryGetValue(nameof(this.UsePwsh), out IConvertible usePwsh))
105+
{
106+
bool usePowerShell7 = usePwsh.ToBoolean(CultureInfo.InvariantCulture);
107+
this.Parameters[nameof(this.Executable)] = usePowerShell7
108+
? PowerShellExecutor.PowerShell7ExecutableName
109+
: PowerShellExecutor.PowerShellExecutableName;
110+
}
111+
}
69112
}
70113
}

0 commit comments

Comments
 (0)