GitHub Actions automation #600
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the first action we add so it sets an example. All testing actions are triggered by either pushing to important integration branches, or by submitting a pull request touching relevant files, or on schedule at 06:00 UTC every day. All of them are going to have at least "unit-tests" and "examples" jobs, with some more as appropriate. Since Themis Core is the most diverse thing that we have, it has quite a few jobs for checking it. Code examples for Themis Core are self-contained tests, we just need to build and run them. However, there were some minor issues with the code producing warnings that we resolve for a clean build.
The action itself is more or less trivial. Note though how we actually test the examples, making sure they work as expected. The examples themselves were kind of dated and did not work reliably (especially the networked ones). Update them, clean up code style, and make sure that they exit with non-zero code when failing.
This is one of the most straighforward actions -- but not so fast! GoThemis build actually requires GOTHEMIS_IMPORT variable to be set, just be aware of that. "make test_go" relies on it. Take care to test against multiple versions of Go. We had issues with that in the past so this environment is important. Unfortunately, installing multiple versions of Go is not that easy with available actions. But I don't want to write a new one (or some shell scripts) so we simply run them in parallel. Also, touch up code examples: gofmt them and make sure they os.Exit(1) when then fail.
Remember that we have two separate code bases for PHP 5 and PHP 7. Furthermore, PHP is somewhat "old school" with installation so use a convenient PPA to install various versions in parallel. There is some weird interference between modules so we make sure to *not* install php-fpm which seems to break PHP distribution. Anyhow, thank you, Ondrej, for maintaining this repository! Our Makefile expects PHP to be available as "php" in PATH, so we use update-alternatives to fix up the symlinks in the system. We test examples with the latest version only to reduce the matrix. Also note that PHPThemis currently does not support PHP 7.3+.
Well, actually, more like AndroidThemis right now, but the actions are named after code bases, not platforms. There are few nice words that I have for Android on CI. Let's just leave it at that it's abysmally slow. Though, this script seems to do the job. Most of the time. Sometimes we still fail to wait for emulator to boot. If we're lucky, it boots in 3-4 minutes. If we're not lucky it gets stuck for 15 minutes. JavaThemis currently does not have a standlone unit-test suite so we have to test it via instrumentation tests on Android by running the entire emulator. However, we should still do these tests as Android build process is tricky and sometimes it does fail.
iOS build automation is not much easier than Android, but at least iOS Simulator on macOS supports x86. Thankfully, we are developing a library and for tests we do not need code signing. Otherwise we'd dealing with longstanding Apple policy of changing the way code signing works every 18 months. However, most pain and suffering comes from the build systems popular for iOS/macOS development. Note that CocoaPods cache. It shaves off about 4 minutes and 850 MB of crap^W trunk reposistory that CocoaPods pulls. It still takes about three minutes to download and unpack but that's better than nothing. Though, we have to do it for every build. Maybe some day we'll invent a shared cache, but until then let's just ride upon Microsoft's generosity of providing free macOS runners. (Otherwise we would be paying $2.08 per build.) There are also various other issues with dependencies, like not having a decent packaging of OpenSSL, which leads to us using GRKOpenSSL which is not really maintained and causes podspec validation warnings. Eh... I just give up, this is insurmountable, I wonder whether we should be maintaining OpenSSL of our own instead. However, that will hurt dynamic linking if people are using OpenSSL for other pods.
That's the one for Node.js. Well, it's more or less straightforward and without any surprises. We test across multiple versions of Node.js so there is some amount of NVM juggling involved.
WasmThemis is more close to Themis Core than any other wrapper. It also uses Node.js for runtime backend so we test with multiple versions. WasmThemis curretly does not have any code examples. (The ones for JsThemis should work with slight changes, but we don't test that.)
Here come the scripting languages! We need to test with both Python 2
and Python 3 so there is some complexity related to that. Also, some
code examples need a couple of services so make them running.
Right, examples...
- Update them all to be compatible with Python 3
- Avoid hardcoded IP addresses (use localhost)
- Make sure they exit with non-zero code when failing
- Add SO_REUSEADDR to sockets created in networked examaple servers
so that we can run them one after another without waiting for
Linux timeout on listening port reuse
- Also, some "pika" API has changed in the meantime, update that.
Now you see why it's important to test examples automatically?
Well this is easy. The only hard part is installing RVM which for some reason really does not want to play nice, arbitrarily requiring you to relogin into your shell to start working, etc. We can't to that on CI :( There is a nice PPA -- thanks, Rael! -- which helps a bit, but there are still some things that we need to do manually. Also, tweak Secure Comparator example to actually stop once the comparison is complete, not just sit there in an infinite loop.
RustThemis has a couple more native dependencies like pkg-config and clang, make sure to install those. Update the examples to exit cleanly on success and report failures via the exit code. Also, do relay messages to the sender so that we have something in stdout to test against. Rust's Cargo uses CocoaPods-like approach with pulling the entire package index history on clean builds. Rust build times are also quite long, so caching *really* help here, turning 5 minutes into 15 seconds.
This action is for (relatively) quick code style check, mostly of C code right now. It also runs clang-tidy static analysis. We use really recent versions of Clang for that to catch as many issues as we can with static analysis. Unfortunately, GitHub goes wa-a-ay overboard with their prebundled repository lists and they fail to install clang-tidy-8 without dependency issues. Use a clean container.
Run cross-language integration tests. Since anything anywhere can affect these tests, they are running for every build. Refer to individual language workflows for quirks. Note that integration testing does not test *everything*, only those that have tools in "tools" directory.
ilammy
added
docs 📚
ilammy
added
W-JsThemis 🍭
|
we need one more label: |
Adapt recently added code from CircleCI to run AFL fuzzers on GitHub Actions runners too. We use the same approach: build fuzzers and run them for 30 seconds each, looking for some easy wins. Note that while GitHub Actions support submitting directories as artifacts, they choke on colons in filenames, therefore we zip AFL reports manually to avoid stalling the build for 10 minutes.
"supported" function is used to determine whether a flag is supported by the compiler. Use AFL_CC instead of regular CC if available to correctly check for flags when building AFL stuff. Some time ago "supported" supported a second argument to select the compiler to use, but this option does not seem to be used anymore.
Certain versions of afl-clang really don't like incomplete initialization (see a2a5cd1 "Resolve compiler warnings"). Replace those with plain memset() to avoid warnings.
afl-clang does not seem to support detailed UBSan variants so avoid
using them in ed25519 suppressions when compiling for AFL. However, we
still need the suppression to avoid triggering UBSan: use unqualified
no_sanitize("undefined") for that.
Similar to CirclCI (7edf2df "Fuzz passphrase API of Secure Cell").
|
what exactly are we waiting here? let's merge and see what happens? |
Instead of enumerating the schemes, just run "carthage build" to build everything that Carthage will build on user machines. By default it will build only dependencies, pass --no-skip-current so that all projects in the current directory will also get built.
We have CocoaPods-based tests and Carthage-based tests in different Xcode projects. Let's test all of them.
Apparently, it got lost during KDF implementation in PyThemis.
GitHub Actions really want to force their users to install stuff via Actions, so they have broken NVM installation. (I'm kidding, of course, but this *might* be related to recent acquisition of npm, Inc. by GitHub slash Microsoft). Anyways, since NVM in unexplicably broken, use more idiomatic way to install Node.js here. This expands the test matrix enormously, but ship first, ask questions later. We'll optimize build runs later. Both JsThemis and WasmThemis need Node.js so update both of them. Integration tests need it too.
Using "sudo make jsthemis_install" will result in system Node.js being used, not the one installed for testing. Run JsThemis installation as a regular user. Ditto for WasmThemis (though it should not be affected). The reason it needs to be run with sudo sometimes is that some previous installer builds Themis as root so the build directory ends up being owned by root and npm cannot move its stuff there. Apply a quickfix for that, but if we do it properly, we should not be building stuff as root in the first place. I'm really seriously frustrated with this changeset so I don't have mental capacity to debug this tangle at the moment. I'll leave a FIXME there and hope to come back at it later.
|
I've synced up this branch with master:
|
vixentael
reviewed
Mar 21, 2020
| - name: Install WasmThemis | ||
| run: | | ||
| source "$HOME/emsdk/emsdk_env.sh" | ||
| emmake make wasmthemis BUILD_PATH=build-wasm | ||
| sudo make wasmthemis_install BUILD_PATH=build-wasm | ||
| make wasmthemis_install BUILD_PATH=build-wasm |
Lagovas
approved these changes
Mar 23, 2020
| # to be set via shell profile. GitHub Actions do not load profile | ||
| # so we have to tweak the path manually here. | ||
| sudo usermod -a -G rvm $(id -nu) | ||
| echo "::add-path::/usr/share/rvm/bin" |
There was a problem hiding this comment.
This adds rvm command into PATH for all future actions. For some weird reason RVM is really unfriendly to automated installation. I’ve copied this from CircleCI scripts, but I wonder if it can be simplified. I guess we could avoid using RVM entirely and install Ruby via some GitHub Action way. But in this PR I’d avoid tweaking it.
There was a problem hiding this comment.
actually I dont understand how echo "::add-path::/path/" adds path to $PATH. I know export PATH=/new/path:$PATH way. And didn't found anything in google how to add a new path with echo "..." command without append to some profile file
p.s. as I remember, was some bug or we need some feature in ruby which was fixed/added in new minor version of ruby but debian hasn't it and used some own patches. and the simplest method to install supported ruby version was rvm.
There was a problem hiding this comment.
actually I dont understand how
echo "::add-path::/path/"adds path to $PATH
Oh... I'm sorry. This is some GitHub Actions magic, here are the docs. It's one of the special “workflow commands”. Each action executes in a separate shell session so export PATH works only for next command in an action but not for the following actions. echo ::add-path adds the path for next actions (but not for the current one).
as I remember, was some bug or we need some feature in ruby which was fixed/added in new minor version of ruby but debian hasn't it and used some own patches
Currently even CircleCI does not test any particular Ruby, we're testing against the system one only. I'd imagine that BuildBot uses it though.
I've looked through our internal task tracker but that did not reveal anything relevant.
Whatever. I'll look into properly installed Ruby later. We'd better test all supported versions, which currently are 2.4 — 2.7.
There was a problem hiding this comment.
GitHub Actions magic
holy shit, one more tool specific language ))
There was a problem hiding this comment.
How else are you expected to differentiate yourself from other free* CI services? Obviously not by providing better computing power or more stable platform, but by locking your users in with yet another YAML syntax.
For that reason I don’t really like all those special actions to install things. However, they do tend to integrate better.
| sudo apt install --yes software-properties-common | ||
| sudo add-apt-repository ppa:ondrej/php | ||
| sudo apt update | ||
| sudo apt install --yes \ |
|
|
||
| return 0; | ||
| // Initialise Secure Session for the client | ||
| #if __cplusplus >= 201103L |
| ) | ||
|
|
||
| func main() { | ||
| if 2 != len(os.Args) { | ||
| fmt.Printf("usage: %s <password>", os.Args[0]) | ||
| return | ||
| os.Exit(1) |
There was a problem hiding this comment.
heh, when code is under automated tests and need programmatical feedback))
| @@ -100,7 +101,7 @@ def get_record(connection, id): | |||
|
|
|||
| if __name__ == '__main__': | |||
| dsn = ("dbname=scell_token_protect_test user=postgres password=postgres " | |||
| "host=172.17.0.2") | |||
There was a problem hiding this comment.
now it looks like somebody use it and test and it doesn't contain hardcoded values)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds automatic verification of Themis on GitHub Actions. You can find build results here. If you want to read some YAML and Bash, this is a code review that you’ll enjoy.
I have been monitoring GitHub Actions since they were in alpha. This changeset for Themis has been cooking since Actions’ beta. With this we make a pipe dream of automating everything a little bit closer. Well, if anything, I hope to regain some human dignity and no longer have to manually check each and every code sample of ours when we are making a release, like a monkey doing machine’s job.
In order to give you some incentive to merge and support this changeset, here are some goodies it brings:
Feature parity
As far as I can tell, Actions will be more or less on par with what we have running on CircleCI and Bitrise right now.
Click to show a comparison table
1 We have something that calls lcov on CircleCI. It even posts something to Coveralls. But I'm not really sure whether we can trust this measurement. Though it's not bad.
2 We sorta kinda verify the JNI builds. But JavaThemis does not have a standalone unit test suite, thus we do not run it. Example projects exist but on “my pace” of their own, we do not verify them as well.
3 WasmThemis is syntactically compatible with JsThemis (except for the import), but does not have its own code examples and we do not verify existing ones for JsThemis with WasmThemis.
Hopefully, CircleCI and Bitrise could be retired after some time. Let's keep Actions running to race it against the competitors.
From my experience, the stability of the whole thing is not stellar. Sometimes the builds flake, especially the Android one. Furthermore, they tend to hang up occasionally. Logs take a while to pull too while the build is running.
Well, we'll see how it fares in production.
Commit summary
There are way too many of them, I'm not copying them all here. Please read commit messages directly if you're interested in details.
All testing actions are triggered either
All of them are going to have at least
unit-testsandexamplesjobs, with some more as appropriate.Parting notes
Unfortunately, GitHub Actions do not make running Android tests easier. It is an unsatisfactory experience, to speak mildly. I'd say more, but it would be against our Code of Conduct.
*leaves behind a gun with a chambered round*
Builds for Apple platforms are tricky too. But at least the builds themselves work. (Thankfully, we don't have to deal with code signing right now. Otherwise, we'd be in hell.) On the other hand, dependency management is... on par or exceeding Android experience.
*returns and puts down two more rounds on the table*
(Soon we'll see how “amazing” Swift Package Manager is. I guess I should go learn new curse words and stock up on ammunition...)
Also, here is my pet peeve: time to open a dashboard with current build status.
Sapienti sat.
Finally, I'd like to say some thanks:
Yes, even CocoaPods.P.S. This PR is a label fiesta.
Checklist
This PR is expected to fail until the following fixes are merged:
Since this branch is not merged yet, pull request trigger rules do not apply. I have added a temporary branch in my repo which tracks this one and forces the builds to happen. You can look at its build status instead. Actions will trigger in the main repository once this branch is merged.