Skip to content

Commit

Permalink
kola: Add isolation=readonly|dynamicuser
Browse files Browse the repository at this point in the history
Part of my war against duplicative comments in our kola tests.
We have a few tests that write a comment like this:

```
 # - exclusive: false
 #   - This test doesn't make meaningful changes to the system and
 #     should be able to be combined with other tests.
```
Of course, this comment is already redundant because the
meaning of the `exclusive` tag is defined canonically in coreos-assembler
(here) and copy-pasting that into every test that uses it would
be pointlessly verbose.

But - we can do one better.  Instead of having a test flag which
is mainly an "I promise not to mutate the system in a way which could
interfere with other tests", let's add a field that *enforces* this.
Then it doesn't need to be commented; we have a variety of
tests which are just "system inspection" (e.g. query rpmdb) and
run just fine with `DynamicUser=yes` and hence *cannot* affect
the system, and hence are inherently isolated from other concurrent
tests.
  • Loading branch information
cgwalters committed Sep 1, 2022
1 parent 6b7a7ba commit a21e0e4
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
7 changes: 7 additions & 0 deletions docs/kola/external-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,13 @@ is simple and is not expected to conflict with other tests, it should be marked
`exclusive: false`. When the `exclusive` key is not provided, tests are marked
`exclusive: true` by default.

The `isolation` key can take two values: `readonly` and `dynamicuser`. These
are thin wrappers for the equivalent systemd options; `readonly` equals `ProtectSystem=strict`,
and `dynamicuser` means `DynamicUser=yes`. Setting either of these options
also implies `exclusive: false`. Use these for tests that are mostly about
read-only system inspection. If specified, the test *cannot* provide
an Ignition (or butane) config either.

The `conflicts` key takes a list of test names that conflict with this test.
This key can only be specified if `exclusive` is marked `false` since
`exclusive: true` tests are run exclusively in their own VM. At runtime,
Expand Down
31 changes: 30 additions & 1 deletion mantle/kola/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,7 @@ type externalTestMeta struct {
AppendKernelArgs string `json:"appendKernelArgs,omitempty"`
AppendFirstbootKernelArgs string `json:"appendFirstbootKernelArgs,omitempty"`
Exclusive bool `json:"exclusive"`
Isolation string `json:"isolation"`
TimeoutMin int `json:"timeoutMin"`
Conflicts []string `json:"conflicts"`
AllowConfigWarnings bool `json:"allowConfigWarnings"`
Expand Down Expand Up @@ -943,6 +944,24 @@ func registerExternalTest(testname, executable, dependencydir string, userdata *
targetMeta = &metaCopy
}

switch targetMeta.Isolation {
case "readonly":
case "dynamicuser":
// These tests cannot provide their own Ignition
if userdata != nil {
return fmt.Errorf("test %v specifies isolation=%v but includes an Ignition config", testname, targetMeta.Isolation)
}
targetMeta.Exclusive = false
case "":
break
default:
return fmt.Errorf("test %v specifies unknown isolation=%v", testname, targetMeta.Isolation)
}

if userdata == nil {
userdata = conf.EmptyIgnition()
}

warningsAction := conf.FailWarnings
if targetMeta.AllowConfigWarnings {
warningsAction = conf.IgnoreWarnings
Expand Down Expand Up @@ -980,6 +999,16 @@ Environment=KOLA_TEST_EXE=%s
Environment=%s=%s
ExecStart=%s
`, unitName, testname, base, kolaExtBinDataEnv, destDataDir, remotepath)
switch targetMeta.Isolation {
case "readonly":
unit += "ProtectSystem=strict\nPrivateTmp=yes\n"
case "dynamicuser":
unit += "DynamicUser=yes\n"
case "":
break
default:
return fmt.Errorf("test %v specifies unknown isolation=%v", testname, targetMeta.Isolation)
}
if targetMeta.InjectContainer {
if CosaBuild == nil {
return fmt.Errorf("test %v uses injectContainer, but no cosa build found", testname)
Expand Down Expand Up @@ -1091,7 +1120,7 @@ func registerTestDir(dir, testprefix string, children []os.FileInfo) error {
var dependencydir string
var meta externalTestMeta
var err error
userdata := conf.EmptyIgnition()
var userdata *conf.UserData
executables := []string{}
for _, c := range children {
fpath := filepath.Join(dir, c.Name())
Expand Down
9 changes: 9 additions & 0 deletions tests/kola-ci-self/tests/kola/exclusive-readonly
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash
# This verifies that isolation=dynamicuser maps to systemd DynamicUser=yes and
# (unlike the default kola model) runs as non-root.
# kola: { "isolation": "dynamicuser" }
set -xeuo pipefail

id=$(id -u)
test "$id" '!=' 0
echo "ok dynamicuser"

0 comments on commit a21e0e4

Please sign in to comment.