diff --git a/.changelog/27182.txt b/.changelog/27182.txt new file mode 100644 index 00000000000..67549118dad --- /dev/null +++ b/.changelog/27182.txt @@ -0,0 +1,3 @@ +```release-note:improvement +qemu: adds an emulator allowlist to qemu plugin config +``` diff --git a/client/testutil/driver_compatible.go b/client/testutil/driver_compatible.go index f1ef1b8c2a6..36640f545a4 100644 --- a/client/testutil/driver_compatible.go +++ b/client/testutil/driver_compatible.go @@ -112,15 +112,22 @@ func JavaCompatible(t *testing.T) { } } -// QemuCompatible skips tests unless: -// - "qemu-system-x86_64" executable is detected on $PATH (!windows) -// - "qemu-img" executable is detected on on $PATH (windows) -func QemuCompatible(t *testing.T) { +// QemuCompatible_x86_64 skips tests unless: +// - "qemu-system-x86_64" executable is detected on $PATH +func QemuCompatible_x86_64(t *testing.T) { // Check if qemu exists bin := "qemu-system-x86_64" - if runtime.GOOS == "windows" { - bin = "qemu-img" + _, err := exec.Command(bin, "--version").CombinedOutput() + if err != nil { + t.Skipf("Test requires QEMU (%s)", bin) } +} + +// QemuCompatible_aarch64 skips tests unless: +// - "qemu-system-aarch64" executable is detected on $PATH +func QemuCompatible_aarch64(t *testing.T) { + // Check if qemu exists + bin := "qemu-system-aarch64" _, err := exec.Command(bin, "--version").CombinedOutput() if err != nil { t.Skipf("Test requires QEMU (%s)", bin) diff --git a/drivers/qemu/driver.go b/drivers/qemu/driver.go index 4b81c3de2ed..f343843b95c 100644 --- a/drivers/qemu/driver.go +++ b/drivers/qemu/driver.go @@ -38,8 +38,9 @@ const ( fingerprintPeriod = 30 * time.Second // The key populated in Node Attributes to indicate presence of the Qemu driver - driverAttr = "driver.qemu" - driverVersionAttr = "driver.qemu.version" + driverAttr = "driver.qemu" + driverVersionAttr = "driver.qemu.version" + driverEmulatorsAttr = "driver.qemu.emulators" // Represents an ACPI shutdown request to the VM (emulates pressing a physical power button) // Reference: https://en.wikibooks.org/wiki/QEMU/Monitor @@ -83,8 +84,9 @@ var ( // configSpec is the hcl specification returned by the ConfigSchema RPC configSpec = hclspec.NewObject(map[string]*hclspec.Spec{ - "image_paths": hclspec.NewAttr("image_paths", "list(string)", false), - "args_allowlist": hclspec.NewAttr("args_allowlist", "list(string)", false), + "image_paths": hclspec.NewAttr("image_paths", "list(string)", false), + "args_allowlist": hclspec.NewAttr("args_allowlist", "list(string)", false), + "emulators_allowlist": hclspec.NewAttr("emulators_allowlist", "list(string)", false), }) // taskConfigSpec is the hcl specification for the driver config section of @@ -150,9 +152,10 @@ type Config struct { // prevent access to devices ArgsAllowList []string `codec:"args_allowlist"` - // FingerprintEmulator specifies which QEMU binary is used - // for fingerprinting - FingerprintEmulator string `codec:"fingerprint_emulator"` + // EmulatorsAllowList is an allow-list of emulator binaries the + // jobspec and FingerprintEmulator can use, so that cluster + // operators can control which emulators job authors can use. + EmulatorsAllowList []string `codec:"emulators_allowlist"` } // Driver is a driver for running images via Qemu @@ -247,16 +250,20 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { HealthDescription: drivers.DriverHealthy, } - fpEmulator := "qemu-system-x86_64" - if d.config.FingerprintEmulator != "" { - fpEmulator = d.config.FingerprintEmulator + emulators := findEmulators(d.config.EmulatorsAllowList) + + if len(emulators) == 0 { + fingerprint.Health = drivers.HealthStateUndetected + fingerprint.HealthDescription = "" + return fingerprint } - outBytes, err := exec.Command(fpEmulator, "--version").Output() + + // Just fetch the version of the first emulator. If a system has many emulators, it can take a while + // to get the version of each one, and is likely a waste of compute. + outBytes, err := exec.Command(fmt.Sprintf("qemu-system-%s", emulators[0]), "--version").Output() if err != nil { - // return no error, as it isn't an error to not find qemu, it just means we - // can't use it. fingerprint.Health = drivers.HealthStateUndetected - fingerprint.HealthDescription = "" + fingerprint.HealthDescription = fmt.Sprintf("Failed to execute qemu binary: %v", err) return fingerprint } out := strings.TrimSpace(string(outBytes)) @@ -268,8 +275,11 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { return fingerprint } currentQemuVersion := matches[1] - fingerprint.Attributes[driverAttr] = pstructs.NewBoolAttribute(true) + fingerprint.Attributes[driverVersionAttr] = pstructs.NewStringAttribute(currentQemuVersion) + fingerprint.Attributes[driverAttr] = pstructs.NewBoolAttribute(true) + fingerprint.Attributes[driverEmulatorsAttr] = pstructs.NewStringAttribute(strings.Join(emulators, ",")) + return fingerprint } @@ -344,6 +354,43 @@ func (d *Driver) RecoverTask(handle *drivers.TaskHandle) error { return nil } +// findEmulators searches the $PATH for qemu-system binaries until they are found +// and returns a slice of emulators that comply with the emulators allowlist. +func findEmulators(allowList []string) []string { + var ( + glob string = "qemu-system-*" + emulators []string + bins []string + err error + ) + + pathEnv := os.Getenv("PATH") + dirs := filepath.SplitList(pathEnv) + + for _, dir := range dirs { + fullPattern := filepath.Join(dir, glob) + bins, err = filepath.Glob(fullPattern) + if err != nil { + continue + } + + // once the qemu binaries are found, break + if len(bins) > 0 { + break + } + } + + for _, f := range bins { + em := strings.TrimPrefix(filepath.Base(f), "qemu-system-") + if err := validateEmulator(em, allowList); err != nil { + continue + } + emulators = append(emulators, em) + } + + return emulators +} + func isAllowedImagePath(allowedPaths []string, allocDir, imagePath string) bool { if !filepath.IsAbs(imagePath) { imagePath = filepath.Join(allocDir, imagePath) @@ -382,6 +429,16 @@ func isAllowedDriveInterface(driveInterface string) bool { return false } +// validateEmulator validate whether the specified emulator is in allowedEmulators +func validateEmulator(emulator string, allowedEmulators []string) error { + if len(allowedEmulators) > 0 { + if !slices.Contains(allowedEmulators, emulator) { + return fmt.Errorf("'%s' is not an allowed emulator", emulator) + } + } + return nil +} + // validateArgs ensures that all QEMU command line params are in the // allowlist. This function must be called after all interpolation has // taken place. @@ -419,6 +476,10 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive handle := drivers.NewTaskHandle(taskHandleVersion) handle.Config = cfg + if err := validateEmulator(driverConfig.Emulator, d.config.EmulatorsAllowList); err != nil { + return nil, nil, err + } + if err := validateArgs(d.config.ArgsAllowList, driverConfig.Args); err != nil { return nil, nil, err } @@ -436,7 +497,7 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive // Parse configuration arguments // Create the base arguments - emulator := "qemu-system-x86_64" + emulator := "x86_64" if driverConfig.Emulator != "" { emulator = driverConfig.Emulator @@ -456,7 +517,7 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive } mem := fmt.Sprintf("%dM", mb) - absPath, err := GetAbsolutePath(emulator) + absPath, err := GetAbsolutePath(fmt.Sprintf("qemu-system-%s", emulator)) if err != nil { return nil, nil, err } diff --git a/drivers/qemu/driver_test.go b/drivers/qemu/driver_test.go index 936898cfebe..6eed4d119d9 100644 --- a/drivers/qemu/driver_test.go +++ b/drivers/qemu/driver_test.go @@ -5,9 +5,11 @@ package qemu import ( "context" + "errors" "io" "os" "path/filepath" + "strings" "testing" "time" @@ -32,7 +34,7 @@ import ( // Verifies starting a qemu image and stopping it func TestQemuDriver_Start_Wait_Stop(t *testing.T) { ci.Parallel(t) - ctestutil.QemuCompatible(t) + ctestutil.QemuCompatible_x86_64(t) ctestutil.CgroupsCompatible(t) ctx, cancel := context.WithCancel(context.Background()) @@ -110,7 +112,7 @@ func copyFile(src, dst string, t *testing.T) { // Verifies starting a qemu image and stopping it func TestQemuDriver_User(t *testing.T) { ci.Parallel(t) - ctestutil.QemuCompatible(t) + ctestutil.QemuCompatible_x86_64(t) ctestutil.CgroupsCompatible(t) ctx, cancel := context.WithCancel(context.Background()) @@ -155,7 +157,7 @@ func TestQemuDriver_User(t *testing.T) { // TestQemuDriver_Stats verifies we can get resources usage stats func TestQemuDriver_Stats(t *testing.T) { ci.Parallel(t) - ctestutil.QemuCompatible(t) + ctestutil.QemuCompatible_x86_64(t) ctestutil.CgroupsCompatible(t) ctx, cancel := context.WithCancel(context.Background()) @@ -236,24 +238,64 @@ func TestQemuDriver_Stats(t *testing.T) { func TestQemuDriver_Fingerprint(t *testing.T) { ci.Parallel(t) - ctestutil.QemuCompatible(t) + ctestutil.QemuCompatible_x86_64(t) + ctestutil.QemuCompatible_aarch64(t) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + t.Run("fingerpints all emulators", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - d := NewQemuDriver(ctx, testlog.HCLogger(t)) - harness := dtestutil.NewDriverHarness(t, d) + d := NewQemuDriver(ctx, testlog.HCLogger(t)) + harness := dtestutil.NewDriverHarness(t, d) - fingerCh, err := harness.Fingerprint(context.Background()) - must.NoError(t, err) - select { - case finger := <-fingerCh: - must.Eq(t, drivers.HealthStateHealthy, finger.Health) - ok, _ := finger.Attributes["driver.qemu"].GetBool() - must.True(t, ok) - case <-time.After(time.Duration(testutil.TestMultiplier()*5) * time.Second): - t.Fatal("timeout receiving fingerprint") - } + fingerCh, err := harness.Fingerprint(context.Background()) + must.NoError(t, err) + select { + case finger := <-fingerCh: + must.Eq(t, drivers.HealthStateHealthy, finger.Health) + ok, _ := finger.Attributes["driver.qemu"].GetBool() + must.True(t, ok) + + emulators, _ := finger.Attributes[driverEmulatorsAttr].GetString() + must.Greater(t, 1, len(strings.Split(emulators, ","))) + case <-time.After(time.Duration(testutil.TestMultiplier()*5) * time.Second): + t.Fatal("timeout receiving fingerprint") + } + }) + + t.Run("fingerprints only allowed emulators", func(t *testing.T) { + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + allowedEms := []string{"x86_64"} + d := NewQemuDriver(ctx, testlog.HCLogger(t)) + config := &Config{ + EmulatorsAllowList: allowedEms, + } + + var data []byte + must.NoError(t, base.MsgPackEncode(&data, config)) + baseConfig := &base.Config{ + PluginConfig: data, + } + harness := dtestutil.NewDriverHarness(t, d) + harness.SetConfig(baseConfig) + + fingerCh, err := harness.Fingerprint(context.Background()) + must.NoError(t, err) + select { + case finger := <-fingerCh: + must.Eq(t, drivers.HealthStateHealthy, finger.Health) + ok, _ := finger.Attributes[driverAttr].GetBool() + must.True(t, ok) + + emulators, _ := finger.Attributes[driverEmulatorsAttr].GetString() + must.SliceContainsAll(t, allowedEms, strings.Split(emulators, ",")) + case <-time.After(time.Duration(testutil.TestMultiplier()*5) * time.Second): + t.Fatal("timeout receiving fingerprint") + } + }) } func TestConfig_ParseAllHCL(t *testing.T) { @@ -289,6 +331,45 @@ config { must.Eq(t, expected, tc) } +func TestValidateEmulator(t *testing.T) { + testcases := []struct { + name string + validEmulators []string + requestedEmulator string + exp error + }{ + { + name: "empty valid emulators, valid request", + validEmulators: nil, + requestedEmulator: "qemu-system-x86_64", + exp: nil, + }, + { + name: "non-empty valid emulators, valid request", + validEmulators: []string{"qemu-system-x86_64"}, + requestedEmulator: "qemu-system-x86_64", + exp: nil, + }, + { + name: "non-empty valid emulators, invalid request", + validEmulators: []string{"qemu-system-x86_64"}, + requestedEmulator: "qemu-system-aarch64", + exp: errors.New("'qemu-system-aarch64' is not an allowed emulator"), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := validateEmulator(tc.requestedEmulator, tc.validEmulators) + if tc.exp != nil { + must.ErrorContains(t, err, tc.exp.Error()) + } else { + must.NoError(t, err) + } + }) + } +} + func TestIsAllowedDriveInterface(t *testing.T) { validInterfaces := []string{"ide", "scsi", "sd", "mtd", "floppy", "pflash", "virtio", "none"} invalidInterfaces := []string{"foo", "virtio-foo"}