-
Notifications
You must be signed in to change notification settings - Fork 3
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
Create workload set insertion process #302
base: main
Are you sure you want to change the base?
Conversation
…ipt for publishing workload sets. Just downloading the workload assets at the moment.
…e existing one to test the script. # Conflicts: # eng/pipelines/official.yml
# Conflicts: # eng/pipelines/official.yml
…vertFrom-SecureString when passing to DARC.
…ile. # Conflicts: # global.json # workload-versions.sln
…uild properly. But, that's running into issues too.
…on set in the global.json. # Conflicts: # global.json
# Conflicts: # global.json
…ontain upstreams.
# Conflicts: # eng/common/tools.ps1
# Conflicts: # global.json # Conflicts: # NuGet.config # eng/Version.Details.xml # eng/Versions.props # eng/common/templates-official/job/job.yml # global.json
… VS drop creation. # Conflicts: # eng/pipelines/official.yml
…. This will be renamed in a future commit. Made the official.yml finally able to create the VS drops, but needs tested.
<ItemGroup> | ||
<SdkAssetsToPublish Include="$(ArtifactsShippingPackagesDir)*.msi" /> | ||
<SdkAssetsToPublish Include="$(ArtifactsNonShippingPackagesDir)*.wixpack.zip" /> | ||
</ItemGroup> | ||
|
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.
This doesn't do anything. SdkAssetsToPublish
is not an item group in this repo/Arcade.
<ItemGroup> | ||
<ItemsToSign Include="$(ArtifactsPackagesDir)**\*.msi" /> | ||
<ItemsToSign Include="$(ArtifactsPackagesDir)**\*.nupkg" /> | ||
</ItemGroup> | ||
|
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.
These are automatically added via Arcade as part of the publish process. If they remain here, Arcade will actually fail to sign because it'll try to double sign these files.
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.
I didn't approve because I don't feel comfortable really reviewing ps1 files, but the parts I looked at looked good 🙂
…oadIfExists parameter.
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.
I tried to review this as thoroughly as I could. Hopefully my questions at least make sense 🙂
displayName: Primary VS insertion branches | ||
type: object | ||
default: | ||
- rel/d17.12 |
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.
The preview version is 17.12, and the non-preview version is 17.13?
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.
I have no idea. The primary branches insert components and packs. The secondary only insert packs. If that is related to preview versions... I don't know.
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.
@marcpopMSFT What are the VS branches that are appropriate here?
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.
Shouldn't we be using the insertion client JSON configuration to find the relevant branches for insertions?
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.
I think 17.12 and 17.13 are currently primary. I think 18.0 will be secondary in the workloads 9 branches eventually but that branch doesn't exist yet. 17.14 is still in preview on the VS side so when I insert those workloads in, they go into main and I usually copy from the N-1 branch so we could theoretically include main in addition to 12 and 13 until .14 snaps in april
############### ARCADE ############### | ||
# Both this (used in Arcade for the MicroBuildSigningPlugin) and DotNetSignType (used in Arcade in Sign.proj) are necessary to set the sign type. | ||
# https://github.com/dotnet/arcade/blob/ccae251ef033746eb0213329953f5e3c1687693b/Documentation/ArcadeSdk.md#common-steps-in-azure-devops-pipeline | ||
- name: _SignType |
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.
Is this only listed under secondaryVsInsertionBranches? Does that mean the primary one isn't signed?
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.
I don't know what you mean by "listed under". It is the pipeline variable used in the build to set the sign type for the build. You can see the usage when we call build.ps1
inside of workload-build.yml
.
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.
If I understand it correctly, the indentation here means that this applies to secondaryVsInsertionBranches but not primary...if it applies to the whole pipeline (i.e., both), then I was just wrong
src/Microsoft.NET.Workloads.Vsman/Microsoft.NET.Workloads.Vsman.vsmanproj
Show resolved
Hide resolved
eng/VstsDropFolder/plugin.ps1
Outdated
# $SkipUploadIfExists = Get-VstsInput -Name 'SkipUploadIfExists' | ||
# if ($SkipUploadIfExists) { | ||
if ($true) { | ||
# Returns a JSON string. If the drop exists, an array with a single object is returned. If the drop does not exist, an empty array is returned. | ||
$dropJson = $server.Client.List($DropName) | ConvertFrom-Json | ||
if ($dropJson.Count -ne 0) { | ||
Write-Host "Drop '$DropName' has already been published. Skipping VS drop publish..." | ||
Write-Telemetry "DropSkipped" "True" | ||
Write-TelemetryMetricFinishSeconds "Upload Drop" | ||
return | ||
} | ||
} |
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.
Note: Tested logic
… for component insertion.
# Conflicts: # eng/Version.Details.xml # eng/Versions.props # eng/pipelines/official.yml
eng/create-workload-drops.ps1
Outdated
# $vsComponentValue = "$assemblyName.vsman{$workloadVersion}=$dropUrl," | ||
$vsComponentValue = "$assemblyName.vsman=$dropUrl," | ||
$vsComponentValue = "$assemblyName.vsman{$workloadVersion}=$dropUrl," |
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.
Note: Tested logic for VS Drop access
6. Request access to the [.NET Daily Internal Build Access](https://coreidentity.microsoft.com/manage/Entitlement/entitlement/netdailyinte-q2ql) entitlement | ||
- This allows the local AzDO machine auth to gather internal assets from AzDO. | ||
- ***Message Matt Mitchell*** (@mmitche) for approval after requesting access to the entitlement. |
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.
should we publicly list alias and names? Seems like these docs should be internal only. They do not apply to any external user
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.
I would normally just use the public GH handle but I do know he has his full name in his profile currently. That could change in the future.
@@ -56,7 +56,7 @@ | |||
</PropertyGroup> | |||
<PropertyGroup> | |||
<WixPackageVersion>3.14.0-8606.20240208.1</WixPackageVersion> | |||
<SwixPackageVersion>1.1.87-gba258badda</SwixPackageVersion> | |||
<SwixPackageVersion>1.1.392</SwixPackageVersion> |
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.
Is CG going to hit us when the version changes? As I recall, this typically comes from the MSENG feed.
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.
If there's a known CVE, I would expect so. Not sure anything else we're supposed to do ahead of time.
@@ -56,7 +56,7 @@ | |||
</PropertyGroup> | |||
<PropertyGroup> | |||
<WixPackageVersion>3.14.0-8606.20240208.1</WixPackageVersion> |
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.
Should we just use the Arcade version?
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.
yeah, better to use the central one than have to remember to update it every time there's a wix security fix.
$dropInfoRegex = '^Workload\.VSDrop\.(?<full>(?<short>\w*)\..*?(?<type>(pre\.)?components$|packs$))' | ||
$primaryVSComponentJsonValues = '' | ||
$secondaryVSComponentJsonValues = '' | ||
$hashAlgorithm = [System.Security.Cryptography.SHA256]::Create() |
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.
Should this be non crypto hash like xxhash instead?
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.
You would need approval to use SHA256 and SHA1 would get flagged.
<ItemsToSign Include="$(ArtifactsPackagesDir)**\*.msi" /> | ||
<ItemsToSign Include="$(ArtifactsPackagesDir)**\*.nupkg" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> |
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.
This is for signing the workload set MSI, correct?
- When it asks for a subscription to select, just press Enter. The default subscription selection does not affect DARC. | ||
4. [Install DARC](https://github.com/dotnet/arcade/blob/main/Documentation/Darc.md#setting-up-your-darc-client) | ||
5. [Add GitHub auth for DARC](https://github.com/dotnet/arcade/blob/main/Documentation/Darc.md#step-3-set-additional-pats-for-azure-devops-and-github-operations) |
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.
Do you need al of the darc auth details here or just link to arcade (or the details are included when you run darc authenticate)?
@@ -0,0 +1,78 @@ | |||
param ([Parameter(Mandatory=$true)] [string] $workloadPath, [Parameter(Mandatory=$true)] [string] $msBuildToolsPath) |
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.
should there be a comment on what the parameters are for and their format?
@@ -0,0 +1,73 @@ | |||
param ([Parameter(Mandatory=$true)] [string] $workloadPath, [SecureString] $gitHubPat, [SecureString] $azDOPat, [string] $workloadListJson = '', [bool] $usePreComponents = $false) |
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.
same as above. Comment the parameters and their common format
/p:DotNetPublishUsingPipelines=true | ||
/p:OfficialBuildId=$(Build.BuildNumber) | ||
/p:StabilizePackageVersion=${{ parameters.stabilizePackageVersion }} | ||
displayName: 🟣 Build solution |
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.
this is the spot we'd add an additional build if we wanted to do the other bands from the same branch. We'd set CreateVSInsertion to false in the later builds and it might just work even though it's in the same agent. But per offline conversation, we need to determine if we're inserting workload sets into VS as that complicates things.
type: object | ||
default: | ||
- emsdk | ||
- mono |
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.
Did you ever end up getting a drop of maui, android, or ios that worked for this testing?
Related: dotnet/sdk#41607
Summary
The primary goals of this PR are to:
official.yml
build to include a process for:Version.Details.xml
/Versions.props
)public.yml
to actually build and be used in PR builds to this repoIf I've done my job well, the result should be a lot of dense YAML pipeline logic and PowerShell scripts. There is a significant amount of 'magic' involved in making this function. I'll walk through the details below.
The Process
To preface, I attempted to reuse as much existing circuitry as possible. This means Azure Pipeline tasks that already exist and Arcade build 'enchantments' (🧙). In doing so, there were a lot of hurdles and I used PowerShell to try to do the least amount of work possible to overcome those hurdles. Let's review the steps that the
official.yml
build goes through.The Steps
The
official.yml
is configured to only run a single Build stage. I've created a template calledworkload-build.yml
that does the entire build process.workload-build.yml
, there is a static set of values I needed to be defined as a parameter (not a variable) as those values are required at compile time (variables are not evaluated at compile time). Therefore, the only way to have static, 'hidden' parameter is to encase all the logic in a template, which is whyworkload-build.yml
exists. The parameter in question isworkloadDropTypes
declared at the top ofworkload-build.yml
.workload-build.yml
The jobs define in this template are within the
jobs.yml
context from Arcade. This allows the creation of the Publish Assets job automatically, which pushes the build assets to DARC.When doing an insertion, we first run the
download-workloads.ps1
script.README.md
. This script also configured to take aworkloadListJson
, which specifies which workloads to acquire. It does this via a regex sent to the DARC command. The regex also accounts for components versus pre.components, which is designated when running the pipeline using theusePreComponentsForVSInsertion
pipeline parameter.Next, we the "regular build" via the
build.ps1
provided by Arcade. This will build theworkload-versions.sln
. I will not go over the process of the building the workload set JSON via theMicrosoft.NET.Workloads.csproj
, as that process has not changed in this PR. The new projects added areMicrosoft.NET.Workloads.Vsman.csproj
andMicrosoft.NET.Workloads.Vsman.vsmanproj
.BuildVsmanProj
. It only runs this target. The .csproj is a bootstrapper to build the .vsmanproj via thecreate-workload-drops.ps1
script. Before running that script, theBuildVsmanProj
target does the local-build version of using thedownload-workloads.ps1
script (when necessary). After that, it runscreate-workload-drops.ps1
which ends up building the .vsmanproj multiple times, once per workload drop acquired from thedownload-workloads.ps1
script..metadata
file.Microsoft.NET.Workloads.Vsman.vsmanproj
for the drop, which creates the vsman file for that drop.emsdk_components_name
,emsdk_components_dir
, andemsdk_components_url
would be created.After the regular build if the
publishToFeed
pipeline parameter is set, the pipeline will publish the workload set to the designated AzDO feed via thefeedForPublishing
pipeline parameter. Currently, this is only related to the workload set package and does nothing with the workload drops.Now comes the fun part (😈). When the pipeline is doing an insertion, it will utilize compile-time foreach loops over the parameters
workloadDropNames
andworkloadDropTypes
. The types-loop is nested inside of the names-loop. This means for each workload drop, it will run all the following tasks for each workload drop type. For reference, the workload drop types are components, packs, and precomponents. This list of types must be statically defined as a parameter to be used with these compile-time foreach loops.Within this looping mechanism, there are two tasks. The first task is a small, embedded PowerShell script that simply checks to see if the drop exists for the combination of drop name and drop type. This has to be checked as the drop type components and precomponents won't be downloaded at the same time (they are mutually exclusive). Also, there could be a workload designated in the Version.Details that doesn't have a published workload drop yet, so it wouldn't have been downloaded. This script just validates that the drop folder exists on disk.
The second task in the loop is the
1ES.MicroBuildVstsDrop
task. This task uploads the workload drop to the VS drop blob storage for use in creating a VS insertion PR. This task only allows you to upload one drop at a time, which is why this task needs to be looped over in the pipeline. This only runs if the previous PowerShell script set the pipeline variablePublishWorkloadDrop
to true.Outside of those nested loops, the last part of
workload-build.yml
has 2 loops, one per each set of insertion branches designated via the pipeline parametersprimaryVsInsertionBranches
andsecondaryVsInsertionBranches
respectively. These are the branches in the VS repo that the primary (component & packs) and secondary (packs only) will be used for VS insertion PRs. In each loop, theworkload-insertion.yml
template is called.workload-insertion.yml
This template has two tasks. The first is a small, embedded PowerShell script that sets the
InsertionTopicBranch
pipeline variable. This variable is calculated based on the value passed in as thetopicBranch
parameter to this template, which is passed in as thevsTopicBranch
pipeline parameter. By default, this value is|default|
which resolves to a month-based branch name. Someone could also supply|temp|
which creates a branch name using the build number and run attempt for the pipeline (so it always changes). Lastly, if someone supplies any other value, that value is used as the topic branch name.The second task is
MicroBuildInsertVsPayload
which is used to create the VS insertion PR. It required creating our own service connection named MicroBuildInsertionVS in the DncEng org to have the proper permissions for this task. This task primarily relies on the component JSON values (supplied viacomponentJsonValues
) to create the VS insertion PR changes. Other than that, every other parameter to the task is fairly self explanatory.