feat(ci): move to GitHub Actions #1657

lidel · 2020-09-30T11:20:15Z

TLDR

The main value added here:

secrets stay on github, easier to reason about security
easier QA and release confidence: every commit produces production ready binaries for every supported platform that we can grab and run instead of building PR locally

This PR

Fixes the CI error on Windows during signing – was (not) fun – details in feat(ci): move to GitHub Actions #1657 (comment)
Migrates our CI/CD setup to a single GitHub Action
Removes Travis and Appveyor configuration
This is first time I work with Github Actions – feedback is really appreciated 🙏

Workflow highlights

the same pipeline runs on macOS, Linux and Windows
triggered by push and pull_request events
- we test original commit as-is + check how target branch would behave after PR is merged
secrets
- signing for windows and mac, and notarization for mac
- exposed only when PR was created by Collaborator and disabled for forks
- this means we still run tests and build binaries for PRs from contributors, the only difference is those are not signed/notarized
workflow has two stages:
- test
  - runs lint, unit tests and end-to-end tests with spectron (linux uses xvfb)
- build
  - starts only if tests are green
    - this is important speedup, because build for linux takes time (we have many package types), and this way we immediatelly see failing tests on linux (without waiting 20 minutes)
  - runs electron-build pipeline and produces packages for specific platform
  - produced packages
    - are attached to GithHub Action and can be downloaded for inspection / testing
      - see example here (links to dist...zip archives)
    - if build occurred on a tag (v0.13) then artifacts are also uploaded and attached to release draft on GitHub
      - this means there is no change to the way we do releases and checklist remains the same
caching
- downloads done by npm, electron and electron-builder are cached under key based on OS and hash(package.json, package-lock.json and electron-builder.yml), which saves around a minute per job

cc #1263 #789

https://github.com/electron-userland/electron-builder/releases/tag/v22.8.1 electron-userland/electron-builder#5005 electron-userland/electron-builder#5143 License: MIT Signed-off-by: Marcin Rataj <[email protected]>

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel · 2020-09-30T13:05:59Z

PSA: I am getting flashbacks of the usual MS Windows SNAFU. I am going to test doing windows build on Github Actions to see if its Appveyor-specific issue.

Ref. https://github.com/samuelmeuli/action-electron-builder

this is because CI added next will implicitly run `npm run build` before `electron-builder`, but it also makes sense for dev: if you want to run `npm start`, you don't need to wait for binaries to build. License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel · 2020-09-30T13:43:53Z

~~I'll have to do local tests via nektos/act before this is ready.~~
Nope, it requires windows box. I'll test in this PR instead.

🕳️ 🐇

lidel · 2020-09-30T16:15:36Z

💚 Good news: Github Action works fine, so we can drop Appveyor
💔 Bad news: we have the same error as on Appveyor 🙃

SignTool Error: The file is being used by another process.
SignTool Error: An error occurred while attempting to sign: D:\a\ipfs-desktop\ipfs-desktop\dist\win-unpacked\resources\app.asar.unpacked\node_modules\go-ipfs\bin\ipfs.exe

  stackTrace=
Error: Exit code: 1. Command failed: C:\Users\runneradmin\AppData\Local\electron-builder\Cache\winCodeSign\winCodeSign-2.6.0\windows-10\x64\signtool.exe sign /t http://timestamp.digicert.com /f C:\Users\RUNNER~1\AppData\Local\Temp\t-T5G40O\0.p12 /d IPFS Desktop /du ipfs-shipyard/ipfs-desktop /p fd63d34e656e0ee0396638165d2f35ed4c8aeab428560f4a83cc90b737c6ea71 (sha256 hash) /debug D:\a\ipfs-desktop\ipfs-desktop\dist\win-unpacked\resources\app.asar.unpacked\node_modules\go-ipfs\bin\ipfs.exe

SignTool Error: The file is being used by another process.

Suspects to check:

build step (webui over ipfs for some reason?)
test step (lack of cleanup keeping lock on ipfs.exe?)
node version
electron-builder version
stale binaries
ipfs binary being utilized by ipfs-or-gateway (it says "Fetching from IPFS" instead of "HTTP
-- I don't expect ipfs to be running)
👉 double signing due to node_modules/go-ipfs/bin/ipfs being symlink to node_modules/go-ipfs/go-ipfs/ipfs
- This was the reason. I hate this.

https://github.com/samuelmeuli/action-electron-builder#electron-builder-action License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel · 2020-10-01T00:44:10Z

Fixed Windows signing

Good news: I've found and fixed the problem with signing.

The problem was caused by clash of:

the fact that every .exe bundled with Electron app is signed
our switch from go-ipfs-dep to go-ipfs, which introduced symlink in bin/ directory
how symlinks are represented when unpacked on MS Windows filesystem

That is why signing node_modules\go-ipfs\go-ipfs\ipfs.exe worked
but node_modules\go-ipfs\bin\ipfs.exe caused the problem.

Removing node_modules\go-ipfs\bin before packaging removes file locking issue.

  •   Signing NSIS uninstaller  file=dist\__uninstaller-nsis-ipfs-desktop.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-j1v0tw\0.p12
  • signing         file=dist\IPFS Desktop Setup 0.12.3.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-j1v0tw\0.p12
  • building block map  blockMapFile=dist\IPFS Desktop Setup 0.12.3.exe.blockmap
  • publishing      publisher=Github (owner: ipfs-shipyard, project: ipfs-desktop, version: 0.12.3)
  • uploading       file=IPFS-Desktop-Setup-0.12.3.exe.blockmap provider=GitHub
  • uploading       file=IPFS-Desktop-Setup-0.12.3.exe provider=GitHub
  • overwrite published file  file=IPFS-Desktop-Setup-0.12.3.exe.blockmap reason=already exists on GitHub
  • overwrite published file  file=IPFS-Desktop-Setup-0.12.3.exe reason=already exists on GitHub
  • overwrite published file  file=latest.yml reason=already exists on GitHub

Next: move everything to Github Action

Need todouble check if

While debugging the issue I've created Github Action setup (to confirm its Window issue, not AppVeyor), and it could also replace Travis for macOS and Linux, so I'm removing AppVeyor for building Windows binaries in this PR.

@rafaelramalho19 @jessicaschilling
The Action I created is easy to extend for supporting macOS and Linux, so I'll try to spend some time on Thursday to push this over the finish line, migrate secrets to github, and then do test release with new setup without Travis.

I'll timebox it to one day to avoid 🕳️ 🐇, if this takes too much time, we can keep Travis for macOS and Linux, just to unblock the release.

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

electron and other tools download packages to cache directory, which make install step faster. we use npm ci which always recreates node_modules for more deterministic build. License: MIT Signed-off-by: Marcin Rataj <[email protected]>

We also remove travis, as tests run fine on Github Action now License: MIT Signed-off-by: Marcin Rataj <[email protected]>

if something fails, we still want to see if other platforms are green License: MIT Signed-off-by: Marcin Rataj <[email protected]>

This tests how code will behave after PR is merged. License: MIT Signed-off-by: Marcin Rataj <[email protected]>

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel · 2020-10-01T22:16:53Z

Ok, this is ready for review.

@rafaelramalho19 I migrated secrets and based on the lack of fireworks I believe CI correctly signs Windows and macOS binaries, but would like second set of eyes to confirm by dowloading .zip with package for respective platform and smoke-testing Windows/macOS artifacts attached to this run: https://github.com/ipfs-shipyard/ipfs-desktop/actions/runs/283172385

is .exe for Windows signed? dist-windows-latest.zip
is .dmg for macOS signed and notarized? dist-macos-latest.zip (aka "no security error" on run)

jessicaschilling · 2020-10-01T22:48:06Z

Downloaded from here, unzipped, got this

... and successfully launched upon clicking "Open".

aschmahmann · 2020-10-01T22:57:34Z

@lidel seems to be working fine on Windows and looks signed (SHA1 and SHA256 with Protocol Labs as the signer). Perhaps unrelated to this PR there is a file IPFS Desktop Setup 0.12.3.exe.blockmap inside the zip next to the exe. What's this for?

hacdias

I must say I'm happy we can ditch Appveyor and Travis and put everything together in GitHub actions. Everything looks fine for me! Thanks @lidel

olizilla · 2020-10-02T08:11:49Z

.github/workflows/ci.yml

+          working-directory: ${{ github.workspace }}
+          run: npm run test:e2e
+
+      - name: Lint


Why lint so late? is deliberate? could go before build to fail fast on trivial errors?

I think it may be just my personal preference: I find it more friendly to do linting later,
that way we see that contributed PR passes tests and only need linting fixes (which could be trivial), which makes it easier to evaluate contribution.

olizilla · 2020-10-02T08:17:23Z

test/e2e/launch.e2e.js

@@ -27,7 +27,7 @@ async function getPort () {
 // inside of `daemonReady`. To print them, pass DEBUG=true

 describe('Application launch', function () {
-  this.timeout(process.env.CI ? 180000 : 60000)
+  this.timeout(60000)


Nice! so fast we dont need th special case!

olizilla

it is so clear, i love it! 🏆 ✨

rafaelramalho19

rafaelramalho19 · 2020-10-01T16:58:00Z

.github/workflows/build.yml

+
+      # TODO: run electron builder only on release tag? ${{ startsWith(github.ref, 'refs/tags/v') }}
+      - name: Build binaries with electron-builder
+        uses: samuelmeuli/action-electron-builder@92327c67bc45ff7c38bf55d8aa8c4d75b7ea38e7 # v1.6.0 but safer than a tag that can be changed


Maybe we should keep that version on package.json under dev deps? Would make it easier to maintain in the future perhaps?

This is the version of github action, internally it will use the version of electron-builder we have in package.json. I don't expect the action to be updated unless there is a breaking-change release of electron-builder that we want to switch to.

ps. I've hardened setup and those third party actions are safelisted in https://github.com/ipfs-shipyard/ipfs-desktop/settings/actions, everything else is blocked. So even if someone sneaks different version in a PR, it won't work.

lidel · 2020-10-02T14:01:34Z

Thank you all for help with reviews and testing ✅

I'm going to merge this and start release dance, create release draft, but will wait with publishing Desktop release until Monday, because I'll be AFK this weekend and in my past life I've learned that shipping to production on Friday is not the best idea 🙈 It will also give us weekend and Monday to smoke-test final binaries.

Perhaps unrelated to this PR there is a file IPFS Desktop Setup 0.12.3.exe.blockmap inside the zip next to the exe. What's this for?

@aschmahmann iiuc when published next to binaries it will be used by autoupdate mechanism for partial / differential updates, to shrink the update payload size.

fix(ci): output files locked on MS Windows

6fcaea4

https://github.com/electron-userland/electron-builder/releases/tag/v22.8.1 electron-userland/electron-builder#5005 electron-userland/electron-builder#5143 License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel requested a review from rafaelramalho19 September 30, 2020 11:20

lidel marked this pull request as draft September 30, 2020 11:25

lidel added 3 commits September 30, 2020 14:12

refactor(ci): run only one Windows job at a time

496bba6

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

chore: hide appveyor config

af6fe86

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

chore: enable signing on branch

c88f8df

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch 5 times, most recently from 91e8077 to 5c811e9 Compare September 30, 2020 16:00

lidel force-pushed the fix/windows-singing branch 7 times, most recently from 22647e8 to f1df31b Compare October 1, 2020 00:14

feat(ci): build windows packages via gh action

d57b997

https://github.com/samuelmeuli/action-electron-builder#electron-builder-action License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch from f1df31b to d57b997 Compare October 1, 2020 00:25

refactor(ci): run build before e2e

53c56fa

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch from b48be80 to 53c56fa Compare October 1, 2020 01:01

feat(ci): add cache

fb20c6d

electron and other tools download packages to cache directory, which make install step faster. we use npm ci which always recreates node_modules for more deterministic build. License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch 2 times, most recently from f3d434e to 644f4e9 Compare October 1, 2020 13:27

lidel force-pushed the fix/windows-singing branch from 9cb57a3 to 23944c4 Compare October 1, 2020 18:52

lidel added 2 commits October 1, 2020 21:34

feat(ci): separate test and build

70d7a2b

We also remove travis, as tests run fine on Github Action now License: MIT Signed-off-by: Marcin Rataj <[email protected]>

fix(cid): disable fail fast

889f768

if something fails, we still want to see if other platforms are green License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch from 7f18803 to 98c0b5a Compare October 1, 2020 20:20

feat(ci): run on pull_request event

774f61b

This tests how code will behave after PR is merged. License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch 2 times, most recently from 9b79175 to ca75159 Compare October 1, 2020 21:21

feat(ci): upload pkgs to release draft only on tag

dcb71ac

License: MIT Signed-off-by: Marcin Rataj <[email protected]>

lidel force-pushed the fix/windows-singing branch from ca75159 to dcb71ac Compare October 1, 2020 21:37

lidel requested review from olizilla, Gozala, jessicaschilling and hacdias October 1, 2020 21:59

lidel marked this pull request as ready for review October 1, 2020 22:11

jessicaschilling approved these changes Oct 1, 2020

View reviewed changes

hacdias approved these changes Oct 2, 2020

View reviewed changes

olizilla reviewed Oct 2, 2020

View reviewed changes

olizilla approved these changes Oct 2, 2020

View reviewed changes

rafaelramalho19 approved these changes Oct 2, 2020

View reviewed changes

lidel merged commit f85aabc into master Oct 2, 2020

lidel deleted the fix/windows-singing branch October 2, 2020 14:29

This was referenced Oct 2, 2020

Decrease package sizes #1659

Closed

Smoke-testing IPFS Desktop v0.13.0 #1661

Closed

bertrandfalguiere mentioned this pull request Oct 7, 2020

Error: ENOENT: no such file or directory, open '/opt/IPFS Desktop/resources/app-update.yml #1671

Closed

snyk-bot mentioned this pull request Oct 4, 2021

[Snyk] Security upgrade standard from 14.3.4 to 16.0.4 baby636/ipfs-desktop#21

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ci): move to GitHub Actions #1657

feat(ci): move to GitHub Actions #1657

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Oct 1, 2020 •

edited

Loading

lidel commented Oct 1, 2020 •

edited

Loading

jessicaschilling commented Oct 1, 2020 •

edited

Loading

aschmahmann commented Oct 1, 2020 •

edited

Loading

hacdias left a comment

olizilla Oct 2, 2020

lidel Oct 2, 2020

olizilla Oct 2, 2020

olizilla left a comment

rafaelramalho19 left a comment

rafaelramalho19 Oct 1, 2020

lidel Oct 2, 2020

lidel commented Oct 2, 2020

feat(ci): move to GitHub Actions #1657

feat(ci): move to GitHub Actions #1657

Conversation

lidel commented Sep 30, 2020 • edited Loading

TLDR

This PR

Workflow highlights

lidel commented Sep 30, 2020 • edited Loading

lidel commented Sep 30, 2020 • edited Loading

lidel commented Sep 30, 2020 • edited Loading

lidel commented Oct 1, 2020 • edited Loading

Fixed Windows signing

Next: move everything to Github Action

lidel commented Oct 1, 2020 • edited Loading

jessicaschilling commented Oct 1, 2020 • edited Loading

aschmahmann commented Oct 1, 2020 • edited Loading

hacdias left a comment

Choose a reason for hiding this comment

olizilla Oct 2, 2020

Choose a reason for hiding this comment

lidel Oct 2, 2020

Choose a reason for hiding this comment

olizilla Oct 2, 2020

Choose a reason for hiding this comment

olizilla left a comment

Choose a reason for hiding this comment

rafaelramalho19 left a comment

Choose a reason for hiding this comment

rafaelramalho19 Oct 1, 2020

Choose a reason for hiding this comment

lidel Oct 2, 2020

Choose a reason for hiding this comment

lidel commented Oct 2, 2020

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Sep 30, 2020 •

edited

Loading

lidel commented Oct 1, 2020 •

edited

Loading

lidel commented Oct 1, 2020 •

edited

Loading

jessicaschilling commented Oct 1, 2020 •

edited

Loading

aschmahmann commented Oct 1, 2020 •

edited

Loading