Skip to content

Commit a21e0e4

Browse files
committed
kola: Add isolation=readonly|dynamicuser
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.
1 parent 6b7a7ba commit a21e0e4

3 files changed

Lines changed: 46 additions & 1 deletion

File tree

docs/kola/external-tests.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,13 @@ is simple and is not expected to conflict with other tests, it should be marked
264264
`exclusive: false`. When the `exclusive` key is not provided, tests are marked
265265
`exclusive: true` by default.
266266

267+
The `isolation` key can take two values: `readonly` and `dynamicuser`. These
268+
are thin wrappers for the equivalent systemd options; `readonly` equals `ProtectSystem=strict`,
269+
and `dynamicuser` means `DynamicUser=yes`. Setting either of these options
270+
also implies `exclusive: false`. Use these for tests that are mostly about
271+
read-only system inspection. If specified, the test *cannot* provide
272+
an Ignition (or butane) config either.
273+
267274
The `conflicts` key takes a list of test names that conflict with this test.
268275
This key can only be specified if `exclusive` is marked `false` since
269276
`exclusive: true` tests are run exclusively in their own VM. At runtime,

mantle/kola/harness.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,7 @@ type externalTestMeta struct {
826826
AppendKernelArgs string `json:"appendKernelArgs,omitempty"`
827827
AppendFirstbootKernelArgs string `json:"appendFirstbootKernelArgs,omitempty"`
828828
Exclusive bool `json:"exclusive"`
829+
Isolation string `json:"isolation"`
829830
TimeoutMin int `json:"timeoutMin"`
830831
Conflicts []string `json:"conflicts"`
831832
AllowConfigWarnings bool `json:"allowConfigWarnings"`
@@ -943,6 +944,24 @@ func registerExternalTest(testname, executable, dependencydir string, userdata *
943944
targetMeta = &metaCopy
944945
}
945946

947+
switch targetMeta.Isolation {
948+
case "readonly":
949+
case "dynamicuser":
950+
// These tests cannot provide their own Ignition
951+
if userdata != nil {
952+
return fmt.Errorf("test %v specifies isolation=%v but includes an Ignition config", testname, targetMeta.Isolation)
953+
}
954+
targetMeta.Exclusive = false
955+
case "":
956+
break
957+
default:
958+
return fmt.Errorf("test %v specifies unknown isolation=%v", testname, targetMeta.Isolation)
959+
}
960+
961+
if userdata == nil {
962+
userdata = conf.EmptyIgnition()
963+
}
964+
946965
warningsAction := conf.FailWarnings
947966
if targetMeta.AllowConfigWarnings {
948967
warningsAction = conf.IgnoreWarnings
@@ -980,6 +999,16 @@ Environment=KOLA_TEST_EXE=%s
980999
Environment=%s=%s
9811000
ExecStart=%s
9821001
`, unitName, testname, base, kolaExtBinDataEnv, destDataDir, remotepath)
1002+
switch targetMeta.Isolation {
1003+
case "readonly":
1004+
unit += "ProtectSystem=strict\nPrivateTmp=yes\n"
1005+
case "dynamicuser":
1006+
unit += "DynamicUser=yes\n"
1007+
case "":
1008+
break
1009+
default:
1010+
return fmt.Errorf("test %v specifies unknown isolation=%v", testname, targetMeta.Isolation)
1011+
}
9831012
if targetMeta.InjectContainer {
9841013
if CosaBuild == nil {
9851014
return fmt.Errorf("test %v uses injectContainer, but no cosa build found", testname)
@@ -1091,7 +1120,7 @@ func registerTestDir(dir, testprefix string, children []os.FileInfo) error {
10911120
var dependencydir string
10921121
var meta externalTestMeta
10931122
var err error
1094-
userdata := conf.EmptyIgnition()
1123+
var userdata *conf.UserData
10951124
executables := []string{}
10961125
for _, c := range children {
10971126
fpath := filepath.Join(dir, c.Name())
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/bin/bash
2+
# This verifies that isolation=dynamicuser maps to systemd DynamicUser=yes and
3+
# (unlike the default kola model) runs as non-root.
4+
# kola: { "isolation": "dynamicuser" }
5+
set -xeuo pipefail
6+
7+
id=$(id -u)
8+
test "$id" '!=' 0
9+
echo "ok dynamicuser"

0 commit comments

Comments
 (0)