Skip to content
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

[test] test in actuated runners #219

Closed
wants to merge 12 commits into from

Conversation

kolyshkin
Copy link
Collaborator

(currently includes #218 but it's not really required here).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix the following gosec warnings in tests by using uint32 everywhere, so
we don't have to do a single cast:

pkg/pwalk/pwalk_test.go:29:20: G115: integer overflow conversion int -> uint32 (gosec)
        if count != uint32(total) {
                          ^
pkg/pwalk/pwalk_test.go:73:15: G115: integer overflow conversion int -> uint32 (gosec)
        max := uint32(total / 2)
                     ^
pkg/pwalk/pwalk_test.go:86:21: G115: integer overflow conversion int -> uint32 (gosec)
                if count != uint32(total) {
                                  ^
pkg/pwalkdir/pwalkdir_test.go:32:20: G115: integer overflow conversion int -> uint32 (gosec)
        if count != uint32(total) {
                          ^
pkg/pwalkdir/pwalkdir_test.go:76:15: G115: integer overflow conversion int -> uint32 (gosec)
        max := uint32(total / 2)
                     ^
pkg/pwalkdir/pwalkdir_test.go:89:21: G115: integer overflow conversion int -> uint32 (gosec)
                if count != uint32(total) {
                                  ^

While at it,
 - switch from atomic op (atomic.AddUint32) to atomic type
   (atomic.Int32) with methods, which is more error-prone;
 - rename max to maxFiles as the former is now a built-in function.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently supported go versions are 1.22 and 1.23.

Drop min and max functions now, as Go 1.21 has built-in ones.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Most of parseLevelItem users will cast its result to int. On a 32-bit
platform this means we may end up with a negative number.

So, let's limit bitSize to 31 in a call to ParseUint, and return int so
there are less typecasts in the code.

Also, change MLS level to use int, for the same reason (less
typecasts).

This fixes the following gosec warnings:

go-selinux/selinux_linux.go:505:30: G115: integer overflow conversion uint -> int (gosec)
				bitset.SetBit(bitset, int(i), 1)
				                         ^
go-selinux/selinux_linux.go:512:29: G115: integer overflow conversion uint -> int (gosec)
			bitset.SetBit(bitset, int(cat), 1)
			                         ^
go-selinux/selinux_linux.go:626:31: G115: integer overflow conversion uint -> int (gosec)
	low := "s" + strconv.Itoa(int(m.low.sens))
	                             ^
go-selinux/selinux_linux.go:635:32: G115: integer overflow conversion uint -> int (gosec)
	high := "s" + strconv.Itoa(int(m.high.sens))
	                              ^

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Gosec doesn't like this code:

go-selinux/selinux_linux.go:141:11: G115: integer overflow conversion int64 -> uint32 (gosec)
	if uint32(buf.Type) != uint32(unix.SELINUX_MAGIC) {
	         ^

But it is correct because
 - buf.Type is int64 or int32, depending on the platform;
 - unix.SELINUX_MAGIC is untyped int which overflows int32
   (i.e. it becomes negative).

So the best type to use here is uint32.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Gosec complains:

go-selinux/selinux_linux.go:587:14: G115: integer overflow conversion uint -> int (gosec)
	for i := int(c.TrailingZeroBits()); i < c.BitLen(); i++ {
	            ^

This is indeed a valid concern in case TrailingZeroBits returns a value
which uses a highest bit (i.e. more than MaxInt32 or MaxInt64, depending
on the platform).

But I think this is highly unlikely.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The new version produces the following warnings:

WARN [config_reader] The configuration option `linters.govet.check-shadowing` is deprecated. Please enable `shadow` instead, if you are not using `enable-all`.
WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.

so fix the configuration accordingly.

Note we do not enable copyloopvar since it requires Go 1.22 and we're
currently have it set to Go 1.21.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since v5, golangci-lint-action relies on actions/setup-go for
caching, so remove "cache: false" from actions/setup-go to re-enable
caching.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The sole reason is to simplify branch protection rules,
requiring just this one to be passed.

I tried but could not find a way to list all other jobs, so had to add
all of them manually.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin force-pushed the actuated branch 6 times, most recently from 401fac8 to e2e6498 Compare October 17, 2024 02:44
@kolyshkin
Copy link
Collaborator Author

Managed to make it work, with a handful of failures:

=== RUN   TestComputeCreateContext
    selinux_linux_test.go:331: ComputeCreateContext(system_u:system_r:init_t:s0, system_u:object_r:tmp_t:s0, file)
    selinux_linux_test.go:337: ComputeCreateContext unexpected answer system_u:object_r:tmp_t:s0, possibly not reference policy
    selinux_linux_test.go:343: ComputeCreateContext(badcon, system_u:object_r:tmp_t:s0, process)
--- FAIL: TestComputeCreateContext (0.00s)

....

=== RUN   TestInit
    label_linux_test.go:40: InitLabels Disabled mlabel Failed, system_u:object_r:svirt_lxc_file_t:s0:c1022,c1023
--- FAIL: TestInit (0.00s)

...

=== RUN   TestRelabel
    label_linux_test.go:106: Relabel shared failed: lsetxattr(label=system_u:object_r:container_file_t:s0) /tmp/TestRelabel1299838602/001: invalid argument
--- FAIL: TestRelabel (0.00s)

...

=== RUN   TestSocketLabel
    label_linux_test.go:186: write /proc/thread-self/attr/sockcreate: invalid argument
--- FAIL: TestSocketLabel (0.00s)
=== RUN   TestKeyLabel
    label_linux_test.go:202: write /proc/self/attr/keycreate: permission denied
--- FAIL: TestKeyLabel (0.00s)

@rhatdan can you take a look? (I've seen occasional errors from TestKeyLabel locally, others look new to me).

Actuated runners have Ubuntu with SELinux configured, so we can actually
run the test cases here (most of which require SELinux enabled).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Collaborator Author

@rhatdan can you take a look? (I've seen occasional errors from TestKeyLabel locally, others look new to me).

Nevermind, [most of] the failures are caused by missing container-selinux (and thus things like container_t and container_file_t), which is not available on Ubuntu. Can try to install it manually, but overall I guess it is easier to test on an RHEL clone instead.

@kolyshkin kolyshkin closed this Oct 18, 2024
@kolyshkin
Copy link
Collaborator Author

Going with Cirrus CI instead, but I'm stuck as I don't have rights.
#220

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

Successfully merging this pull request may close these issues.

1 participant