Add support for Azure Artifact Signing#263
Conversation
| - name: Build bundle | ||
| run: | | ||
| npm run winbundle | ||
| npm run winbundle -- --signtool-path "C:\Program Files (x86)\Windows Kits\10\bin\10.0.26100.0\x64\signtool.exe" --azure-signing-metadata "%RUNNER_TEMP%\metadata.json" |
There was a problem hiding this comment.
This is fine, but note that this path will break very soon. Long term you'll want to find a more reliable way to invoke signtool.
There was a problem hiding this comment.
The Windows SDK path is very consistent, it hasn't changed since the original release of Windows 10 besides the version number increment. Being more smart about this requires bringing a lot more tooling (e.g. VS developer cmd, not really documented registry reads, or installing the SDK at runtime), there isn't an easy environment variable documented unfortunately :(
Thankfully, this should only break when the SDK 26100 is removed from the CI image (which shouldn't happen anytime soon), and fixing it should be merely just updating this path. That's why I made it a parameter, to make that easier.
There was a problem hiding this comment.
One simple idea, instead of an exact path to signtool, pass a path to C:\Program Files (x86)\Windows Kits\10\bin\ and recursively search for signtool.exe. Just a thought.
There was a problem hiding this comment.
The artifact signing dlib requires at least SDK 19041, so a naive search could pick up a version that's too old. It could also pick up the ARM64 or x86 signtool, which we don't want.
OpenDroneMap#263 added granular permissions to grant `id-token: write` (which isn't granted by default). When doing this, I also added `contents: read` because granular permissions overwrite repo-level permissions, and not having it prevented the workflow from cloning its own repo. However, the `svenstaro/upload-release-action` action requires `contents: write`, which the pipeline no longer had. Change `contents` to `write` (which implies `read`) to fix that.
2 participants
This one was tested locally, so it should work from the first go.