-
Notifications
You must be signed in to change notification settings - Fork 313
ESRP publishing #3065
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
base: main
Are you sure you want to change the base?
ESRP publishing #3065
Conversation
7549c10
to
13de88a
Compare
$orderMatches = $true | ||
for ($i = 0; $i -lt $PackageNames.Count; $i++) { | ||
if ($packages[$i].name -ne $PackageNames[$i]) { | ||
$orderMatches = $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.
This change uses Artifact names coming from the service's sdk/<service>/ci.yml
file to supply the -PackageNames
parameter. The sorting of those artifact names must match the sorted order created by this script for a successful release (while we're releasing one package per-ESRP step).
It's possible to change the sorting by altering the placement of workspace.members
in the root Cargo.tml
file.
If the sorting is incorrect, you'll get an error at release time that the order of packages in -PackageNames
is not correct. To fix, update the order of the service's ci.yml
file to match the order that the script provides and then run the release.
This validation currently only happens during release because the pipeline yaml templates aren't aware of the various libraries' dependencies in the state of their Cargo.toml files (e.g. workspace dependencies vs. released dependencies).
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 should probably be documented somewhere - as a comment in this file, or in some release docs somewhere in the repo.
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.
Heath and I expanded on this a few moments ago:
- Relying on the user to get the order right in
ci.yml
is going to be a source of problems. Instead, we should see if the ordering can be done "dynamically" in the job. This will require some more YAML
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.
Pull Request Overview
This PR implements ESRP (Enterprise Software Signing Release Pipeline) publishing for Azure SDK for Rust packages, replacing the previous cargo publish approach. The changes introduce release package selection parameters and dependency enforcement mechanisms to ensure packages are released in the correct order.
- Adds release intent parameters to each service CI file to allow selective package releases
- Implements dependency enforcement and order validation in the Pack-Crates.ps1 script
- Replaces cargo publish with ESRP release tasks in the pipeline templates
- Updates package artifact handling to work with .crate files instead of Cargo.toml manifests
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
sdk/*/ci.yml | Adds release intent parameters for each package in the service directories |
eng/scripts/Pack-Crates.ps1 | Adds dependency validation and order enforcement for release builds |
eng/scripts/Language-Settings.ps1 | Updates package pattern and artifact processing to handle .crate files |
eng/pipelines/templates/stages/archetype-sdk-client.yml | Filters artifacts based on release intent parameters |
eng/pipelines/templates/stages/archetype-rust-release.yml | Replaces cargo publish with ESRP release workflow |
eng/pipelines/templates/jobs/pack.yml | Adds dependency requirement enforcement for manual builds |
.vscode/cspell.json | Minor formatting cleanup and adds spell check exceptions |
|
||
if ($RequireDependencies) { | ||
$unspecifiedPackages = $packages.name | Where-Object { $_ -notin $PackageNames } | ||
if ($unspecifiedPackages.Count -gt 0) { |
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 seem to allow for packages outside of this pipeline run to be accommodate the current release. e.g. :
azure_core_amqp
depends on azure_core
azure_core
depends on typespec
We're building core
, but typespec
isn't shipped in the core
pipeline
We should be able to build with $PackageNames = ('azure_core', 'azure_core_amqp')
, even though there's that lingering typespec
dependency because we know that, before releasing those packages, we'll be releasing typespec
in a separate pipeline.
This is different from $PackageNames = ('azure_core_amqp')
where we know azure_core_amqp
depends on azure_core
, we know this is the pipeline that should build it and we know we're not building it.
I'm proposing that we call Pack-Crates.ps1 with a list of all the artifacts this pipeline is responsible for, along with a list of packages we intend on releasing. If any dependency is in the list we're responsible for but aren't releasing, then we error.
Allowed
Pack-Crates.ps1 -Packages 'azure_core' -PipelinePackages 'azure_core_amqp', 'azure_core'
Should error
Pack-Crates.ps1 -Packages 'azure_core_amqp' -PipelinePackages 'azure_core_amqp', 'azure_core'
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.
In the scenario where we're releasing azure_core_amqp
and there's a dependency on typespec
how does azure_core_amqp
's Cargo.toml file express the dependency on typespec
?
https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/core/azure_core_amqp/Cargo.toml#L33
As it's set up in this PR, the core
pipeline would fail if it's depending on a release of typespec
that hasn't yet completed. So you need to queue and release typespec
and then you can queue and release your packages in core
that depend on typespec
.
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.
For packing/publishing cargo
will elevate workspace dependencies and remove any path properties from the dependencies, leaving their other properties:
given the workspace dependency in the root toml:
[workspace.dependencies.typespec]
default-features = false
path = "sdk/typespec"
version = "0.9.0"
azure_core_amqp's dependency would process as:
typespec = { workspace = true, features = ["amqp"] } =>
typespec = { path = "sdk/typespec", version = "0.9.0", features = ["amqp"] } =>
typespec = { version = "0.9.0", features = ["amqp"] }
Where typespec 0.9.0 is currently unreleased.
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 we queue and release typespec, we need to uptake the version bump PR to rev the unreleased, path version to 0.10.0.
Doing this would cause azure_core_amqp to depend on the new, unreleased 0.10.0
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.
@hallipr and I went over this on the whiteboard and it works out to mostly YAML changes:
- Check boxes in the pipeline queuing UI signals release intent
- If no boxes are checked (basically, run live tests but don't release anything) and if a pipeline is run manually then treat it as
Pack-Crates.ps1 -OutputPath '...' -PackageNames all,packages,in,build -RequireDependencies:$false
- If boxes are checked, that signals release intent, then use
-RequireDependencies:$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.
@hallipr - Can you have another look? I updated the YAML. I'm debating pulling the inline PowerShell into a file so we do local iteration but it's probably not going to change much as written.
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.
Approving from Cosmos
…s in the service and run live tests"
36c0bf2
to
455288c
Compare
$orderMatches = $true | ||
for ($i = 0; $i -lt $PackageNames.Count; $i++) { | ||
if ($packages[$i].name -ne $PackageNames[$i]) { | ||
$orderMatches = $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.
This should probably be documented somewhere - as a comment in this file, or in some release docs somewhere in the repo.
- pwsh: | | ||
$artifacts = '${{ convertToJson(parameters.Artifacts) }}' | ConvertFrom-Json | ||
$requireDependencies = $true | ||
$artifactsToBuild = $artifacts | Where-Object { $_.releaseInBatch -ne '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.
Why the double negative here?
Do I need to review this as a service owner?
Probably not. This PR is for EngSys. You're probably seeing this review because I updated your
ci.yml
file to use "release intent" which allows you to choose which packages to release at release build time. We in EngSys will merge this after reviewing.Notes
Fixes #3035
Updated runs with release intent:
Sample pipeline run with 2 release crates: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5405078&view=results
Sample pipeline run with no releases and live tests: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5405113&view=results -- In this case note that necessary dependencies were built (e.g.
typespec
) even though it was outside the scope of the service.Sample pipeline run with no releases (also no live tests): https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5405079&view=results
Sample releases:
Open questions
Pack-Crates.ps1
fails in release scenarios when releasing a package that has unreleased dependencies (e.g.dependency_crate = { workspace = true }
. The expectation here is that dependencies will go fromworkspace = true
to{ version = "1.2.3" }
when releasing. Is this a correct expectation?