-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Authentication issue fix in AzurePowerShell task #19521
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
Authentication issue fix in AzurePowerShell task #19521
Conversation
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.
Thank you for your PR. Please take a look at my comments and address them.
@@ -11,6 +11,9 @@ function convertToNullIfUndefined<T>(arg: T): T|null { | |||
return arg ? arg : null; | |||
} | |||
|
|||
let input_workingDirectory = tl.getPathInput('workingDirectory', /*required*/ true, /*check*/ true); |
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.
Could you please move this inside the run
function? It is not needed in the global scope. You can then remove second initialization of input_workingDirectory
on line 41.
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.
Done as suggested
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.
Thank you, but could you please do the same with tempDirectory
as well?
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.
Done.
} | ||
|
||
|
||
function Disconnect-AzureAndClearContext { |
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.
Why don't you call the existing Disconnect-AzureAndClearContext
from utility.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.
We can't use that utility as it was written specific to windows and present on common which is not supportable for linux hence we created a new file.
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.
But the code you copied out from there is exactly the same. It would be worth to extract it to separate shared ps1 file and use it on both places.
Task name: AzurePowerShellV5
Description: Pipelines were persisting authentication token while using AzurePowerShellV5 task on Microsoft hosted and Self-Hosted agents.
Documentation changes required: (Y/N) N
Added unit tests: (Y/N) N
Attached related issue: DTS [Pipelines Agents Tasks UI]: pipelines are persisting authentication tokens between runs on self-hosted agents
Checklist: