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

feat: Initial version of Save-AzDevOpsBuild #183

Merged
merged 20 commits into from
Jul 14, 2021
Merged

feat: Initial version of Save-AzDevOpsBuild #183

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1906040
Initial version of Set-RetainBuildFlag
fgheysels Jun 23, 2021
4639917
Required changes to make the RetainBuildFlag function available
fgheysels Jun 23, 2021
9239349
Code layout improvements
fgheysels Jun 23, 2021
66161c1
Include script in project file
fgheysels Jun 23, 2021
447efee
Update src/Arcus.Scripting.DevOps/Scripts/Set-RetainBuildFlag.ps1
fgheysels Jun 24, 2021
d24dfd3
Rename Function to Save-AzDevOpsBuild
fgheysels Jun 25, 2021
2133af3
Throw an exception when request didnt finish successfully
fgheysels Jun 25, 2021
cb3d5a9
Add some tracing
fgheysels Jun 25, 2021
b491c51
Merge branch 'frgh/feature/retain_build' of https://github.com/fgheys…
fgheysels Jun 25, 2021
862bcbf
Merge branch 'master' into frgh/feature/retain_build
fgheysels Jul 6, 2021
2932f0f
First unit-test for Save-AzDevOpsBuild
fgheysels Jul 6, 2021
2bd2bd3
Additional unit-test for positive scenario
fgheysels Jul 6, 2021
771f1da
Alternative way of mocking Invoke-WebRequest
fgheysels Jul 6, 2021
1a08fbe
Fixed typo in test definition
fgheysels Jul 7, 2021
c43aa70
Clean up test definition
fgheysels Jul 7, 2021
7b901ec
FIX: refer to correct file in psproj file
fgheysels Jul 9, 2021
7322ba6
Use DevOps pipeline variables
fgheysels Jul 12, 2021
dab2f2d
Align psm1 file with actual script
fgheysels Jul 12, 2021
36717ea
should -invoke syntax
fgheysels Jul 12, 2021
5a21069
Add docs on Save-AzDevOpsBuild
fgheysels Jul 14, 2021
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
23 changes: 23 additions & 0 deletions src/Arcus.Scripting.DevOps/Scripts/Set-RetainBuildFlag.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
param(
[Parameter(Mandatory = $true)][string] $OrganizationName = $(throw "The name of the organization is required"),
[Parameter(Mandatory = $true)][string] $ProjectId = $(throw "ProjectId is required"),
[Parameter(Mandatory = $true)][string] $BuildId = $(throw "BuildId is required"),
[Parameter(Mandatory = $true)][string] $AccessToken = $(throw "An access Token for the DevOps API is required")
)

$retentionPayload = @{
keepforever='true'
}

$requestBody = $retentionPayload | ConvertTo-Json -Depth 100
stijnmoreels marked this conversation as resolved.
Show resolved Hide resolved

$requestUri = "https://dev.azure.com/$OrganizationName/$ProjectId/_apis/build/builds/" + $BuildId + "?api-version=6.0"
Copy link
Member

Choose a reason for hiding this comment

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

It may be important to also make sure that legacy Azure DevOps systems can benefit from this (legacy = {organizationName}.visualstudio.com).
As I see it, you can benefit from the environment variable $env:SYSTEM_TEAMFOUNDATIONCOLLECTIONURI to retrieve the root URL where all Azure DevOps API calls can be made.

Context: https://docs.microsoft.com/en-us/azure/devops/extend/develop/work-with-urls?view=azure-devops&tabs=http

On the other hand, we may consider using a client package for this. But that may be taking things to far.
https://docs.microsoft.com/en-us/azure/devops/integrate/concepts/dotnet-client-libraries?view=azure-devops

Copy link
Member

Choose a reason for hiding this comment

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

Is the API version also something we should parameterize? Default can be 6.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to parameterize the complete 'base' uri, as I thought it would make this more complex to pass this in as a parameter.
Unless you rely on this environment variable inside the script and do not pass it via a param.

Copy link
Member Author

Choose a reason for hiding this comment

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

To be honest, I don't see a point right now in parametrizing the API version ? @mbraekman ?

Copy link
Contributor

Choose a reason for hiding this comment

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

What we're typically doing, in case it's applicable, is set it as a parameter but default it to a specific version (as Stijn indicates). (for example in Enable-AzLogicApp)
The reason for this is that based on this approach the same script can easily be used to use the latest/another specific version of the API, without requiring any changes in the script.

However, while API versions change very often in the case of ARM templates, for instance, this doesn't seem to be the case for DevOps-related APIs. So, not quite sure if in this case, it would be very useful.
--> So not really required in this case.

Regarding the base-URI, I would rely on the built-in variables as much as possible, as this requires less input from the client using the script, increasing user-friendliness. But, even so, we should still offer users the possibility to provide their own custom values, just in case.
--> use built-in variable.

Copy link
Member Author

@fgheysels fgheysels Jun 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

But then I'd suggest that we do not pass in the organizationname parameter, but put the DevOps variable which contains the Uri inside the script ? This has the side effect that this script is only usable from within an Azure DevOps pipeline; that's why I opted for passing in the organisationName as a parameter, and not to rely on the System.CollectionUri variable which defines this base-url.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, exactly. So, we could opt for using the built-in base URL but that this is the default value for a custom provided URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this would make it more useable. You'll have to pass in an url, consumer needs to know if it requires a trailing slash or not (ok, you can handle that in your script), but imho, it makes the whole thing a bit less user-friendly

Copy link
Member

Choose a reason for hiding this comment

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

This one is a good example how we did it for another DevOps script:
https://github.com/arcus-azure/arcus.scripting/blob/master/src/Arcus.Scripting.DevOps/Scripts/Set-AzDevOpsArmOutputs.ps1


$response = Invoke-WebRequest -Uri $requestUri -Method Patch -Body $requestBody -ContentType "application/json" -Headers @{ Authorization = "Bearer $accessToken" }
stijnmoreels marked this conversation as resolved.
Show resolved Hide resolved

if( $response.StatusCode -ne 200 ) {
fgheysels marked this conversation as resolved.
Show resolved Hide resolved
Write-Error "Unable to retain build indefinetely. API request returned statuscode $($response.StatusCode)"
exit 1
stijnmoreels marked this conversation as resolved.
Show resolved Hide resolved
}

exit 0
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm absolutely no Powershell expert, so if you have any improvements, recommendations, .... please let me know :)