Skip to content

Consider switching to GitHub Actions for automated testing #1195

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

Closed
lukpueh opened this issue Oct 30, 2020 · 14 comments
Closed

Consider switching to GitHub Actions for automated testing #1195

lukpueh opened this issue Oct 30, 2020 · 14 comments
Assignees

Comments

@lukpueh
Copy link
Member

lukpueh commented Oct 30, 2020

Description of issue or feature request:
We switched from automated testing on Travis CI (Linux) and Appveyor (Windows) to GitHub Actions in our sister-project in-toto-golang (see in-toto/in-toto-golang#72), mostly due to availability concerns about Appveyor and because it seems appealing to configure only one service for all platforms.

Let's consider switching here too and testing on more platforms than we currently do.

Current behavior:
Use Travis CI and Appveyor for automated testing on Linux for Windows (no tests on macOS)

Expected behavior:
Use GitHub Actions instead and also test on macOS.

@lukpueh
Copy link
Member Author

lukpueh commented Oct 30, 2020

@joshuagl
Copy link
Member

Being able to test on Windows/Linux/macOS with the same CI make this a no-brainer to me.

@lukpueh
Copy link
Member Author

lukpueh commented Nov 4, 2020

Also consider injection vulnerabilities reported in https://crbug.com/project-zero/2070

@lukpueh
Copy link
Member Author

lukpueh commented Nov 11, 2020

See @jku's #1204 (comment) as backing data point for the "configure only one service for all platforms" argument.

lukpueh added a commit to lukpueh/tuf that referenced this issue Nov 12, 2020
Update badge URL in readme after migrating from travis-ci.org to
travis-ci.com, due to brownout on the former.

Migration was performed via Travis Web UI:
https://docs.travis-ci.com/user/migrate/open-source-repository-migration

NOTE: This is a quick fix to speed up Travis builds until we switch
to GitHub Actions (theupdateframework#1195)

Signed-off-by: Lukas Puehringer <[email protected]>
@jku
Copy link
Member

jku commented Nov 24, 2020

I think the next step is someone testing this to see if it's good enough.

Documenting a potential test matrix:
python: Latest release of all supported cpython 3.x versions, cpython 2.7. Currently that would be [2.7, 3.6, 3.7, 3.8, 3.9]
platform: Latest release of Linux, Windows, Mac: [ubuntu-latest, windows-latest, macos-latest]

Github does offer specific platform versions as well but I'm not sure it makes sense to use those? The matrix size as proposed is 15 test runs. As a bonus it might be nice to add pypy3 to python version list (not sure how much additional benefits it gives -- but it is something pip tests with and currently TUF tests pass with pypy3): that should probably be done separately as the build dependencies might require a bit of tweaking there.

There's also at least two special cases that we want to test (they do not need to be part of the matrix):
special: Securesystemslib master branch on "ubuntu-latest" with python "3.x" (this should not fail a build)
special: Lint on "ubuntu-latest" with python "3.x"

This should be a good starting point: https://docs.github.com/en/free-pro-team@latest/actions/guides/building-and-testing-python
Note this suggestion:

You'll need to invoke tox using the -e py option to choose the version of Python in your PATH, rather than specifying a specific version.

Something like this might work as the matrix setup in workflow yaml:

strategy:
  matrix:
    python-version: [2.7, 3.6, 3.7, 3.8, 3.9]
    os: [ubuntu-latest, windows-latest, macos-latest]
    toxenv: py
    include:
      - python-version: 3.x
        os: ubuntu-latest
        toxenv: with-sslib-master
        experimental: true
      - python-version: 3.x
        os: ubuntu-latest
        toxenv: lint

Things to keep in mind:

lukpueh added a commit to lukpueh/tuf that referenced this issue Dec 11, 2020
We recently enabled support for Python 3.9 and also added
3.9 builds on appveyor, but they don't work out of the box.

Instead of troubleshooting this I suggest we soon switch to
GitHub actions:
theupdateframework#1195.

Signed-off-by: Lukas Puehringer <[email protected]>
@lukpueh
Copy link
Member Author

lukpueh commented Dec 11, 2020

Also, we just ran out of build minutes on Travis. :(

@joshuagl
Copy link
Member

Also, we just ran out of build minutes on Travis. :(

Wow. I didn't even realise we had a build minutes limit. Does that mean we can't do any more Travis builds in December?

@lukpueh
Copy link
Member Author

lukpueh commented Dec 11, 2020

I didn't know either until I noticed that builds didn't finish and came across this message on https://travis-ci.com/organizations/theupdateframework:

Builds have been temporarily disabled for private and public repositories due to a negative credit balance. Please go to the Plan page to replenish your credit balance.

Not exactly sure when credits will reset.

@lukpueh
Copy link
Member Author

lukpueh commented Dec 11, 2020

IIUC, GitHub actions has no limits for open source projects (see first sentence in Billing for GitHub Actions). I am happy to set this up first thing next week.

@lukpueh lukpueh self-assigned this Dec 11, 2020
@joshuagl
Copy link
Member

IIUC, GitHub actions has no limits for open source projects (see first sentence in Billing for GitHub Actions). I am happy to set this up first thing next week.

No limits for open source is my understanding too, thanks for prioritising this @lukpueh!

@jku
Copy link
Member

jku commented Dec 12, 2020

am happy to set this up first thing next week.

I did some experiments but am likely to not concentrate on this at least early this coming week so feel free to copy-paste or just take notes from my branch:

Notes:

@lukpueh
Copy link
Member Author

lukpueh commented Dec 14, 2020

Thanks for the excellent spade-work, @jku! If you don't mind I'll try to PR something tomorrow.

Regarding your notes:

  • I haven't fully understood how, as an actions user, I can protect myself against set-env and add-path vulnerabilities. Do I need to audit the actions I use? Will need to further investigate.
  • Not sure what the best solution for fail-fast is. Your thoughts seem quite reasonable. I can try to create a configuration for that, but maybe in a follow-up PR.
  • Tests on 2.7 worked for me, not sure why they didn't for you. Do you still have the build results somewhere?
  • I also fixed windows builds, they just required a small patch in test_6_download_target.
  • pylint indeed seems to not yet fully support python 3.9 (see e.g. [Python 3.9] Value 'Optional' is unsubscriptable (unsubscriptable-object) (also Union) pylint-dev/pylint#3882). But 3.8 should be just as fine.

Apart from that I still need to finalize coveralls and fossa configuration.

@jku
Copy link
Member

jku commented Dec 14, 2020

  • I haven't fully understood how, as an actions user, I can protect myself against set-env and add-path vulnerabilities. Do I need to audit the actions I use? Will need to further investigate.

We shouldn't really be especially vulnerable I think -- none of our steps look like they could be easily made to print commands without control of the source code -- I'd say only the "Run tox" step is suspect (in the sense that there's so much code running we can't really rule out the possibility of untrusted output).

I think the only thing we can do is something like

echo "::stop-commands::`echo -n ${{ github.token }} | sha256sum | head -c 64`"

we could do that already in "Find pip cache dir" immediately after the ::set-output command: that should prevent any commands from being accepted from the output in the whole build after that.

  • Not sure what the best solution for fail-fast is. Your thoughts seem quite reasonable. I can try to create a configuration for that, but maybe in a follow-up PR.

👍

  • Tests on 2.7 worked for me, not sure why they didn't for you. Do you still have the build results somewhere?

Uh no, I seem to have deleted some builds I didn't intend to... Maybe it was a mistake in an earlier configuration

@lukpueh
Copy link
Member Author

lukpueh commented Dec 17, 2020

#1242 replaced travis/appveyor with GitHub Actions. Remaining issues discussed here are ticketized in #1243, #1244, #1246, #1247 and #1248. Closing...

@lukpueh lukpueh closed this as completed Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants