forked from seccomp/libseccomp-golang
-
Couldn't load subscription status.
- Fork 0
[WIP] CI/GHA: test against libseccomp 2.5.2 and HEAD #2
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
Draft
kolyshkin
wants to merge
20
commits into
main
Choose a base branch
from
test-against-2.5.2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or 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
c985b4b to
6df53ab
Compare
GHA CI is available since commit e5b1e63, while travis-ci.org is no more. Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Tom Hromatka <[email protected]> [PM: break up the long line a little] Signed-off-by: Paul Moore <[email protected]>
As generated by https://pkg.go.dev/badge/ While at it, update the old doc URL in the text (currently, godoc.org redirects to pkg.go.dev). Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Tom Hromatka <[email protected]> [PM: break up the long lines] Signed-off-by: Paul Moore <[email protected]>
Those were added based on the discussion in seccomp#2. Nowadays, though, it seems highly unlikely someone will use golang or gcc version released in 2014-2015, so this warning, while still being valid, does not add much value to a user. Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Tom Hromatka <[email protected]> Signed-off-by: Paul Moore <[email protected]>
README.md contains some information about testing the library, also
there are some notes about it in CONTRIBUTING.md. It seems the relevant
text in README better reflects the reality ("make check" performs both
linting and testing, it is sufficient, and it requires golangci-lint),
so let's use it and drop the other.
In other words:
- replace the testing note in CONTRIBUTING with the ones from README;
- in README, add a reference to CONTRIBUTING.
Signed-off-by: Kir Kolyshkin <[email protected]>
Acked-by: Tom Hromatka <[email protected]>
[PM: cleaned up subject line, removed inline changelog, longline fixes]
Signed-off-by: Paul Moore <[email protected]>
688cc3f to
20eb0b5
Compare
All the files in the package (except the recently added seccomp_ver_test.go) have linux build tag. Since seccomp is linux-specific, it does not make much sense to add this build tag, because this is the only GOOS supported, and it won't compile on any other platform anyway. Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Paul Moore <[email protected]> Signed-off-by: Tom Hromatka <[email protected]>
Commit 8b6c34e adds go.sum with checksums of some seemingly random packages. Since libseccomp-golang do not import (use) any of those packages, these checksums are not required (and are probably leftovers of running "go get" after cd to this directory). This commit is a result of "go mod tidy" run. Fixes: 8b6c34e Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Paul Moore <[email protected]> Signed-off-by: Tom Hromatka <[email protected]>
This is a result of running "gofumpt -w *.go" on the source code. gofumpt[1] is a tool similar to (and based on) gofmt, which adds some some extra rules for better code formatting. Here, it removes some extra vertical whitespace, and replaces a var() block with a single element inside with a mere var declaration. Both make sense. Enable gofumpt in golangci-lint config to make sure future commits are gofumpt-ed. [1] https://github.com/mvdan/gofumpt Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Paul Moore <[email protected]> Signed-off-by: Tom Hromatka <[email protected]>
c67a300 to
3e739ba
Compare
43c6868 to
591efe2
Compare
1. Fix a few typos found by codespell: ./seccomp_internal.go:820: reponse ==> response ./seccomp.go:267: notication ==> notification ./seccomp_test.go:19: statment ==> statement ./seccomp_test.go:140: nonexistant ==> nonexistent ./seccomp_test.go:363: nonexistant ==> nonexistent ./seccomp_test.go:366: nonexistant ==> nonexistent ./seccomp_test.go:423: correcly ==> correctly 2. Add a CI job to ensure that future PRs won't add typos (at least those that that are known to codespell). Signed-off-by: Kir Kolyshkin <[email protected]> Acked-by: Paul Moore <[email protected]> Signed-off-by: Tom Hromatka <[email protected]>
126079c to
08d3474
Compare
As the code errors out earlier if seccomp version is less than 2.2.0, there is no need to check for SCMP_VER_MAJOR < 2. Drop it. Signed-off-by: Kir Kolyshkin <[email protected]> Signed-off-by: Paul Moore <[email protected]>
Use CombinedOutput to simplify the code. Signed-off-by: Kir Kolyshkin <[email protected]> [PM: subject line tweak] Signed-off-by: Paul Moore <[email protected]>
1. Only a single per-package #cgo directive is needed to link it against libseccomp with proper flags, and it is provided in seccomp_internal.go. Drop the duplicate. 2. Drop the comments telling how to link against a particular libseccomp version, as those are also available in seccomp_internal.go. Signed-off-by: Kir Kolyshkin <[email protected]> Signed-off-by: Paul Moore <[email protected]>
In Go, one can omit repetitive "type = iota" stance. No functional change. Signed-off-by: Kir Kolyshkin <[email protected]> Signed-off-by: Paul Moore <[email protected]>
Currently this package requires libseccomp < 2.2.0, refusing to build otherwise, and contains a few kludges specifically targeting libseccomp 2.2.0. Let's require version 2.3.1 or greater, and remove the kludges for older versions. While at it, reword the error message to remove the word "supported". Checking for libseccomp versions shipped with various old (but still supported) releases, here is what I found out: * Ubuntu 14.04 "Trusty Tahr": 2.1.1 (unsupported by this pkg), with 2.2.3 available in backports repo [1] * Debian "Stretch" (aka oldoldstable): 2.3.1 [2] * RHEL/CentOS 7: 2.3.1 [3] * SLES 15 SP1: 2.4.3 [4] * openSUSE Leap 15.2: 2.4.1 [4] * Alpine 3.11: 2.4.2 [5] * Arch, Gentoo: 2.5.x [1] https://launchpad.net/ubuntu/+source/libseccomp [2] https://packages.debian.org/search?keywords=libseccomp [3] https://rpmfind.net/linux/rpm2html/search.php?query=libseccomp [4] https://software.opensuse.org/package/libseccomp [5] https://pkgs.alpinelinux.org/packages?name=libseccomp&branch=v3.11 Signed-off-by: Kir Kolyshkin <[email protected]>
Otherwise the LD_PRELOAD, set in a CI job, is not being used, leading to distro libseccomp version being used instead of the one we prepared. Signed-off-by: Kir Kolyshkin <[email protected]>
Instead of testing against whatever version of libseccomp the distro (Ubuntu 20.04 in this case) provides, do compile and test against the latest released version, 2.5.2. While at it, it's relatively easy to enable testing against git tip as well, so implement it as well. All we have to do is to change the version from 0.0.0 to some value that is >= current version. Signed-off-by: Kir Kolyshkin <[email protected]>
Functions seccomp_api_get(3) and seccomp_api_set(3) appeared in libseccomp 2.4.0, not 2.3.3 like the code assumed. This fixes TestGetAPILevel failure with libseccomp 2.3.3. Signed-off-by: Kir Kolyshkin <[email protected]>
While testing this package against a recent kernel and older libseccomp library (2.2.x to 2.4.x) it appears that every time we perform an API level check, libseccomp version check is also needed. Modify checkAPI to also call checkVersion. Unify both functions, as well as VersionError, to use minor, major, micro triad everywhere. While at it: - fix checkAPI and checkVersion docs; - simplify their tests, add more test cases. Signed-off-by: Kir Kolyshkin <[email protected]>
TestNotif should check not only for API level >= 6, but also for libseccomp >= 2.5.0. Use notifSupported() to fix. Same applies to TestNotifUnsupported. Signed-off-by: Kir Kolyshkin <[email protected]>
NOTE that libseccomp < 2.5.x is no longer officially supported upstream, but this library still works with 2.3.x and 2.4.x, so we test against those. Signed-off-by: Kir Kolyshkin <[email protected]>
This is to ensure we're testing against the expected libseccomp version (as prepared by CI, which sets the _EXPECTED_LIBSECCOMP_VERSION var). Signed-off-by: Kir Kolyshkin <[email protected]>
08d3474 to
6f6ba25
Compare
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.
No description provided.