[build] Migrate stable .pkg signing to ESRP - #5700
Conversation
Context: #5529 Commit dc1f7ba enabled ESRP signing for all .NET 6 artifacts, and we've been asked to apply these changes to our stable .pkg installer that is still using a deprecated Apple Developer account and signing logic. The mac build job has been updated to sign and harden all Mach-O files included in both our stable and .NET 6 .pkg files. This was previously happening in two different passes, the first for the stable .pkg content during the build job and the second for the .NET 6 .pkg content during the `dotnet_create_pkg` job. Signing of the stable .pkg has been migrated to use the ESRP signing service via the tasks/targets imported by the new MicroBuild package reference. Notarization of the stable .pkg has been moved to the mac build job. As a result, all legacy macOS automated tests will now run against the final version of the package we ship. This will allow us to catch any potential issues with notarization earlier.
This reverts commit 6e4b54e.
|
Latest "Real signed" test run looks good: https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=4544409&view=results |
This reverts commit 9fed992.
|
Latest "Real signed" test run (plus smoke tests against notarized .pkg) - https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4547971&view=results |
|
I have two concerns here:
Is there any way to keep our existing structure -- Mac > Build doesn't sign, test stages run against the unsigned installer, etc. -- while using the new ESRP signing service? |
|
The .vsix job timeout was caused by an unrelated issue and should now be resolved. Unless we want to unpack, sign and repack the entire .pkg file in our "finalization" stage, we'll have to incur some additional cost of ESRP signing during the build. I think we can get the build time back down to close to what it is today by moving just the .pkg signing and the .pkg notarization calls to a separate job as suggested though, I'll try to rework this. |
0fd2219 to
2f1458b
Compare
2f1458b to
2ae088f
Compare
|
Latest real signed test run: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4553100&view=results |
|
The current 1h30min is much better than the previous 2h30min! :-) Still ~20min longer than current, though. Is this the best we can do? Should we just go with this? |
| <!-- Unpack bundletool.jar and sign embedded aapt2 --> | ||
| <RemoveDir Directories="$(_BundleToolExtractLocation)" /> |
There was a problem hiding this comment.
Just a thought, I didn't know you were having to deal with aapt2 inside bundletool.
We always pass a path to aapt2 to bundletool, so it uses our aapt2, I wonder if we could simply remove it from bundletool.jar? I guess that would invalidate bundletoo.jar's signature.
There was a problem hiding this comment.
I like this idea, we can save some build time (and installation space) by stripping out the nested aapt2 rather than signing it. Let's see if this causes any test failures.
There was a problem hiding this comment.
Looks like bundletool still expects aapt2 to exist even if it may not be using it in our case, I think I'll revert these latest changes:
[BT : 1.4.0] error : Unable to locate aapt2 inside jar. [/Users/runner/work/1/s/tests/Mono.Android-Tests/Runtime-AppBundle/Mono.Android-TestsAppBundle.csproj]
com.android.tools.build.bundletool.model.exceptions.CommandExecutionException: Unable to locate aapt2 inside jar.
at com.android.tools.build.bundletool.model.exceptions.InternalExceptionBuilder.build(InternalExceptionBuilder.java:57)
at com.android.tools.build.bundletool.model.utils.SdkToolsLocator.extractAapt2(SdkToolsLocator.java:109)
at com.android.tools.build.bundletool.commands.CommandUtils.extractAapt2FromJar(CommandUtils.java:64)
at com.android.tools.build.bundletool.commands.BundletoolModule.lambda$provideAapt2Command$0(BundletoolModule.java:35)
at java.base/java.util.Optional.orElseGet(Optional.java:369)
at com.android.tools.build.bundletool.commands.BundletoolModule.provideAapt2Command(BundletoolModule.java:35)
at com.android.tools.build.bundletool.commands.BundletoolModule_ProvideAapt2CommandFactory.provideAapt2Command(BundletoolModule_ProvideAapt2CommandFactory.java:40)
at com.android.tools.build.bundletool.commands.BundletoolModule_ProvideAapt2CommandFactory.get(BundletoolModule_ProvideAapt2CommandFactory.java:31)
at com.android.tools.build.bundletool.commands.BundletoolModule_ProvideAapt2CommandFactory.get(BundletoolModule_ProvideAapt2CommandFactory.java:10)
at dagger.internal.DoubleCheck.get(DoubleCheck.java:47)
There was a problem hiding this comment.
Do you know the command-line that hit this? Just want to check we don't have something wrong.
There was a problem hiding this comment.
This happened in the apk tests job during DeployTestAabs: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4555981&view=logs&j=cd6eb242-2e57-569b-620a-eaaded8e9a51&t=f5527f3e-27f9-5958-e19f-f922328304e1&l=715. My initial changes to strip aapt2 out only ran when doing real signing so the full set of tests didn't run against it. I can kick something off against that commit to see if anything else breaks. - https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4558049&view=results
There was a problem hiding this comment.
Yeah weird we pass --adb and they still extract it:
/Users/runner/Library/Android/jdk-11/bin/java -jar /Library/Frameworks/Xamarin.Android.framework/Libraries/xbuild/Xamarin/Android/bundletool.jar build-apks --connected-device --overwrite --mode default --bundle "../../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.aab" --output "../../../bin/TestRelease/Mono.Android_TestsAppBundle-Signed.aab.apks" --adb /Users/runner/Library/Android/sdk/platform-tools/adb --ks /Users/runner/work/1/s/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/test.keystore --ks-key-alias mykey --key-pass pass:android --ks-pass pass:android
[BT : 1.4.0] error : Unable to locate aapt2 inside jar.
jonathanpeppers
left a comment
There was a problem hiding this comment.
I'm good with the actual changes here, I don't know if we can do much more about the build times. We are probably now seeing the difference between the old signing method and new one.
|
I think the ~20 minute build addition is probably a better price to pay than the added complexity of the alternative -- which would be unpacking and repacking our legacy .pkg installer in a post-build job. We can maybe reduce this added time a little bit more with the latest change that removes the version of aapt2 nested in bundletool.jar rather than signing it. Latest real signed job: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4555981&view=logs&s=1afc3bfe-122c-538b-e9ad-2a86c2efcfef&j=96fd57f5-f69e-53c7-3d47-f67e6cf9b93e |
This reverts commit eb704a2.
|
Latest real signed run is looking good: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4562177&view=logs&j=a62e275d-46d5-5c3b-5a47-c4e00ae7427d. This should be good to go now. |
Context: #5529
Commit dc1f7ba enabled ESRP signing for all .NET 6 artifacts, and we've
been asked to apply these changes to our stable .pkg installer that is
still using a deprecated Apple Developer account and signing logic.
The mac build job has been updated to sign and harden all Mach-O files
included in both our stable and .NET 6 .pkg files. This was previously
happening in two different passes, the first for the stable .pkg content
during the build job and the second for the .NET 6 .pkg content during
the
dotnet_create_pkgjob. Signing of the stable .pkg has been migratedto use the ESRP signing service via the tasks/targets imported by the new
MicroBuild package reference.
Our previous notarization preparation logic required us to unpack
bundletool.jar, and sign the version ofaapt2that is bundled inside.This has been updated to remove all embedded versions of
aapt2instead, as we pass our own path to
aapt2to the tool as needed.