-
Notifications
You must be signed in to change notification settings - Fork 48
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
Migration to ESRP Code Signing from PackageES Code Signing #93
Conversation
Note: After this PR is complete, I plan to bump the version number and update the changelog. |
Do we need to permit the pipeline to access the code signing service every run? |
When I setup the code signing service, I disallowed it access to all pipelines, except the manually triggered one. So the only pipeline that can use it is the Release-Nuget pipeline. |
Does that mean the CI-Nuget on pull requests will never complete? It shows as waiting for permission before continuing with the pipeline, since it's trying to use the code signing service. |
Hmm, good point. It looks like it's stuck waiting for permissions. However, I really don't want to allow the "CI-Nuget" pipeline access to the signing service though. I'll test this out in another branch, and then update this PR once I've been able to test things. |
Good point. Based on looking at the ProjectReunion build infra, I don't think they are needed anymore. |
Update: I tested it out, and it looks like the PackageES tasks can be removed without issue. I also removed the I've refactored the pipeline to move the Nuget signing into a separate stage from the Nuget package creation. This lets us completely skip the Nuget package signing for the This means that we can enable the code signing service for only the |
build/azure-nuget.yml
Outdated
@@ -248,10 +248,15 @@ stages: | |||
name: Package ES CodeHub Lab E |
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.
Package ES CodeHub Lab E [](start = 10, length = 24)
there is no need to use PackageES lab, which has few build lines.
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.
Can you code sign on non-PackageES lab agents?
FWIW, it looked like ProjectReunion was still using PackageES lab for the code signing part of the pipeline, which is why I kept this.
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'll try using a non-PackageES lab and see if it works....
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.
Hm, I tried using the regular Azure Pipelines pool here.
However, the CodeSign stage of the build appears to be cancelled with the following error message:
##[error]The remote provider was unable to process the request.
I wonder if maybe it doesn't support the ESRP task? Perhaps that is why ProjectReunion still uses PackageES lab?
Anyways, I think we might need to keep it using PackageES lab for now.
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 a few more builds, and they had the same odd error, so leaving this as PackageES for now.
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.
not sure whether "Azure Pipelines" is a valid name. Reunion uses "pool: vmImage: 'windows-2019'".
In reply to: 612874376 [](ancestors = 612874376)
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.
Hmm, interesting...
In the builds that failed I just used pool: name: Azure Pipelines
as that is what I have setup for the initial build step here:
https://github.com/microsoft/icu/blob/master/build/azure-nuget.yml#L19
However, it also sets the vmImage
as well. In the builds that failed, I did not set vmImage
as well.
I wonder if both are perhaps needed?
I'll try out another build with both and see if that is it. Thanks Hui.
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.
Ah hah! That was it! You need to set both the pool name, and the vmImage.
By setting, the code signing now works on Azure Pipelines!
This means we can completely move off the PackageES lab entirely.
Thanks Hui for commenting on this and making me try out all the combinations. :)
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.
nice! btw, name is not required.
…the pool name and vmImage.
Update 2: |
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.
Good point, we can likely remove the
Well the code signing still only runs on the pipeline in MSCodeHub, so this comment is still correct. However, I can update it a bit to note that the pipeline is manually triggered. |
Since the Nuget creation and Nuget code signing are now done as two separate stages in the build pipeline, the Windows symbols publishing task fails as it relied on the DownloadBuildArtifacts tasks in the Nuget creation step. (This was missed in the refactoring of the pipeline in PR #93.) However, since the Nuget doesn't contain any symbols at all, we can move the DownloadBuildArtifacts tasks entirely to the Nuget code signing stage instead. Also, it turns out that the PublishSymbols task cannot run on the public Azure Pipelines pool and must be run on the PackageES pool. (Note: I skipped the actual publish step when testing PR #93, as you can't unpublish things once published. However, when running the Release Pipeline to publish for real, it errors out.) Note: FWIW, the ProjectReunion pipeline also uses the PackageES lab for publishing symbols as well here: https://github.com/microsoft/ProjectReunion/blob/1714557cad5e740c0153e451fbf0311c8e5bdfc1/build/ProjectReunion-BuildFoundation.yml#L172-L188
Summary
This change modifies the MS-ICU Nuget build pipeline to use the ESRP Code Signing instead of the PackageES Code Signing.
PR Checklist
Detailed Description
Finally, after various delays and setbacks, we are now able to migrate to using ESRP Code Signing for the MS-ICU Nuget. This PR changes the Nuget build pipeline to use the ESRP Code Signing instead of the deprecated PackageES Code Signing. The end result in terms of the output signed binaries and packages is the same, but the mechanism is now different.
I did a test build of the pipeline and doubled-checked the output.
The DLLs are signed with the same certificate:
CN = Microsoft Code Signing PCA 2011
The Nuget packages are signed with the same certificate as well:
For reference, here is the new build, using the ESRP Code Signing:
https://mscodehub.visualstudio.com/OSS_github.com_Unicode-org/_build/results?buildId=180999&view=results
Here is a previous build, using PackageES Code Signing (for comparison):
https://mscodehub.visualstudio.com/OSS_github.com_Unicode-org/_build/results?buildId=168347&view=artifacts&pathAsName=false&type=publishedArtifacts
The ESRP process signs the DLLs and packages in-place, rather than outputting them to a separate folder, so I also had to slightly modify the Nuget build script in order to not look for the "signed" folder anymore.
Note: This change also migrates us off the PackageES lab and uses the public Azure Pipelines for all stages now.