-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.
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
Users/pragupta/azurepwrshelltask #3175
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,17 @@ Trace-VstsEnteringInvocation $MyInvocation | |
Import-VstsLocStrings "$PSScriptRoot\Task.json" | ||
|
||
# Get inputs. | ||
$scriptPath = Get-VstsInput -Name ScriptPath -Require | ||
$scriptType = Get-VstsInput -Name ScriptType -Require | ||
$scriptPath = Get-VstsInput -Name ScriptPath | ||
$scriptInline = Get-VstsInput -Name AzurePowerShellInline | ||
$scriptArguments = Get-VstsInput -Name ScriptArguments | ||
|
||
# Validate the script path and args do not contains new-lines. Otherwise, it will | ||
# break invoking the script via Invoke-Expression. | ||
if ($scriptPath -match '[\r\n]') { | ||
throw (Get-VstsLocString -Key InvalidScriptPath0 -ArgumentList $scriptPath) | ||
if ($scriptType -eq "FilePath") { | ||
if ($scriptPath -match '[\r\n]') { | ||
throw (Get-VstsLocString -Key InvalidScriptPath0 -ArgumentList $scriptPath) | ||
} | ||
} | ||
|
||
if ($scriptArguments -match '[\r\n]') { | ||
|
@@ -20,6 +24,14 @@ Import-Module $PSScriptRoot\ps_modules\VstsAzureHelpers_ | |
Initialize-Azure | ||
|
||
# Trace the expression as it will be invoked. | ||
If ($scriptType -eq "InlineScript") { | ||
$tempFileName = [guid]::NewGuid().ToString() + ".ps1"; | ||
$scriptPath = [System.IO.Path]::Combine([System.IO.Path]::GetTempPath(), $tempFileName); | ||
($scriptInline | Out-File $scriptPath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should try/finally and best-effort cleanup this temp file. here is what the agent does for the powershell task. it creates a warning issue if the file fails to delete for some reason. |
||
|
||
Write-Host "tempFile= $scriptPath" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add localized string here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks like debug messaging to me. consider the write-verbose instead. |
||
} | ||
|
||
$scriptCommand = "& '$($scriptPath.Replace("'", "''"))' $scriptArguments" | ||
Remove-Variable -Name scriptPath | ||
Remove-Variable -Name scriptArguments | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
[CmdletBinding()] | ||
param() | ||
|
||
# Arrange. | ||
. $PSScriptRoot\..\..\..\Tests\lib\Initialize-Test.ps1 | ||
Unregister-Mock Get-VstsInput | ||
Register-Mock Get-VstsInput { "InlineScript" } -- -Name ScriptType -Require | ||
Register-Mock Get-VstsInput { ",@( 'item 1', 'item 2')" } -- -Name AzurePowerShellInline | ||
Register-Mock Initialize-Azure | ||
|
||
# Act. | ||
$actual = @( & $PSScriptRoot\..\AzurePowerShell.ps1 ) | ||
$global:ErrorActionPreference = 'Stop' # Reset to stop. | ||
|
||
# Assert the correct number of elements is returned. | ||
Assert-AreEqual 1 $actual.Length | ||
|
||
# Assert item 1 and 2 are in an array together. | ||
Assert-AreEqual 2 @($actual[0]).Length | ||
Assert-AreEqual 'item 1' $actual[0][0] | ||
Assert-AreEqual 'item 2' $actual[0][1] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
"version": { | ||
"Major": 1, | ||
"Minor": 1, | ||
"Patch": 3 | ||
"Patch": 4 | ||
}, | ||
"demands": [ | ||
"azureps" | ||
|
@@ -54,13 +54,40 @@ | |
"helpMarkDown": "Azure Resource Manager subscription to configure before running PowerShell", | ||
"visibleRule": "ConnectedServiceNameSelector = ConnectedServiceNameARM" | ||
}, | ||
{ | ||
"name": "ScriptType", | ||
"type": "pickList", | ||
"label": "Script Type", | ||
"required": true, | ||
"helpMarkDown": "Type of the script: File Path or Inline Script", | ||
"defaultValue": "FilePath", | ||
"options": { | ||
"FilePath": "AzurePowerShell Script File Path", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, "AzurePowerShell" in the dropdown items seems redundant to me. it's the Azure PowerShell task, so this is implied already. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...nit continued :) also, i would even say having "AzurePowerShell" in the dropdown item text may be confusing. the task initializes the Azure module and then runs the PowerShell script. other than the name of the task, there is no such thing as "Azure PowerShell" |
||
"InlineScript": "Inline AzurePowerShell Script" | ||
} | ||
}, | ||
{ | ||
"name": "ScriptPath", | ||
"type": "filePath", | ||
"label": "Script Path", | ||
"defaultValue": "", | ||
"required": true, | ||
"helpMarkDown": "Path of the script. Should be fully qualified path or relative to the default working directory." | ||
"required": false, | ||
"helpMarkDown": "Path of the script. Should be fully qualified path or relative to the default working directory.", | ||
"visibleRule": "ScriptType = FilePath" | ||
}, | ||
{ | ||
"name": "AzurePowerShellInline", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, "AzurePowerShell" in the input name seems redunant. it's the Azure PowerShell task. |
||
"type": "multiLine", | ||
"label": "Inline AzurePowerShell Script", | ||
"required": false, | ||
"defaultValue": "# You can write your azure powershell scripts inline here. \n# You can also pass predefined and custom variables to this script using arguments", | ||
"helpMarkDown": "Enter the AzurePowerShell script to execute.", | ||
"visibleRule": "ScriptType = InlineScript", | ||
"properties": { | ||
"resizable": "true", | ||
"rows": "10", | ||
"maxLength": "500" | ||
} | ||
}, | ||
{ | ||
"name": "ScriptArguments", | ||
|
@@ -74,7 +101,7 @@ | |
"helpMarkDown": "Additional parameters to pass to PowerShell. Can be either ordinal or named parameters." | ||
} | ||
], | ||
"instanceNameFormat": "Azure PowerShell script: $(ScriptPath)", | ||
"instanceNameFormat": "Azure PowerShell script: $(ScriptType)", | ||
"execution": { | ||
"PowerShell3": { | ||
"target": "AzurePowerShell.ps1" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when script type is filepath, then shouldn't ScriptPath be required? i.e.
Get-VstsInput -Name ScriptPath -Require
similar thing for inline scenario