tools/run_tests: Run some tests as root

Testing tap functionality requires root privileges. The crosvmdev
user of our dev_container is set up for passwordless sudo, so we can
seamlessly execute these tests via sudo.

For running on the developer workstation directly, this will prompt
for a password, which is disruptive to workflows. The --no-root
option can be used to prevent this and skip the tests in question.

BUG=None
TEST=tools/run_tests [--no-root]

Change-Id: I731887837affceb76152466f0006c4ee51a19234
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4063237
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
This commit is contained in:
Dennis Kempin 2022-11-28 21:22:10 -08:00 committed by crosvm LUCI
parent 354bd50590
commit b157bda3ee
5 changed files with 43 additions and 8 deletions

View file

@ -11,13 +11,11 @@ use net_util::MacAddress;
use net_util::TapTCommon;
#[test]
#[ignore = "Requires root privileges"]
fn tap_create() {
Tap::new(true, false).unwrap();
}
#[test]
#[ignore = "Requires root privileges"]
fn tap_configure() {
let tap = Tap::new(true, false).unwrap();
let ip_addr: net::Ipv4Addr = "100.115.92.5".parse().unwrap();
@ -32,7 +30,6 @@ fn tap_configure() {
}
#[test]
#[ignore = "Requires root privileges"]
fn tap_enable() {
let tap = Tap::new(true, false).unwrap();

View file

@ -42,6 +42,11 @@ class TestOption(enum.Enum):
# This test needs longer than usual to run.
LARGE = "large"
# Integration test that requires root privileges to execute.
# Note that this does not apply to unit tests, which will never be allowed privileged access
# to the system.
REQUIRES_ROOT = "requires_root"
# Configuration to restrict how and where tests of a certain crate can
# be build and run.
@ -95,6 +100,7 @@ CRATE_OPTIONS: Dict[str, List[TestOption]] = {
], # b/181674144
"libvda": [TestOption.DO_NOT_RUN], # b/202293971
"sandbox": [TestOption.DO_NOT_RUN],
"net_util": [TestOption.REQUIRES_ROOT],
}
for name in WIN64_DISABLED_CRATES:

View file

@ -16,7 +16,7 @@ from typing import Dict, Iterable, List, NamedTuple, Optional
from . import test_target, testvm
from .common import all_tracked_files, very_verbose
from .test_config import BUILD_FEATURES, CRATE_OPTIONS, TestOption
from .test_config import CRATE_OPTIONS, TestOption
from .test_target import TestTarget, Triple
USAGE = """\
@ -119,7 +119,9 @@ def get_workspace_excludes(build_triple: Triple):
yield crate
def should_run_executable(executable: Executable, target: TestTarget, test_names: List[str]):
def should_run_executable(
executable: Executable, target: TestTarget, test_names: List[str], execute_as_root: bool
):
arch = target.build_triple.arch
options = CRATE_OPTIONS.get(executable.crate_name, [])
if TestOption.DO_NOT_RUN in options:
@ -132,6 +134,8 @@ def should_run_executable(executable: Executable, target: TestTarget, test_names
return False
if TestOption.DO_NOT_RUN_ON_FOREIGN_KERNEL in options and not target.is_native:
return False
if TestOption.REQUIRES_ROOT in options and not execute_as_root:
return False
if test_names:
for name in test_names:
if fnmatch.fnmatch(executable.name, name):
@ -366,6 +370,8 @@ def execute_integration_test(
"""
args: List[str] = ["--test-threads=1"]
binary_path = executable.binary_path
options = CRATE_OPTIONS.get(executable.crate_name, [])
execute_as_root = TestOption.REQUIRES_ROOT in options
previous_attempts: List[ExecutableResults] = []
for i in range(1, attempts + 1):
@ -380,6 +386,7 @@ def execute_integration_test(
generate_profile=True,
stdout=None if VERBOSE else subprocess.PIPE,
stderr=subprocess.STDOUT,
execute_as_root=execute_as_root,
)
profile_files: List[Path] = []
if collect_coverage:
@ -528,6 +535,13 @@ def generate_lcov(
)
def sudo_is_passwordless():
# Run with --askpass but no askpass set, succeeds only if passwordless sudo
# is available.
(ret, _) = subprocess.getstatusoutput("SUDO_ASKPASS=false sudo --askpass true")
return ret == 0
def main():
parser = argparse.ArgumentParser(usage=USAGE)
parser.add_argument(
@ -581,6 +595,11 @@ def main():
"--crosvm-direct",
action="store_true",
)
parser.add_argument(
"--no-root",
action="store_true",
help="Disables tests that require to be run as root.",
)
parser.add_argument(
"--repeat",
type=int,
@ -649,6 +668,7 @@ def main():
integration_test_target = test_target.TestTarget("vm:aarch64", build_target)
elif str(build_target) == "x86_64-pc-windows-gnu" and os.name == "nt":
integration_test_target = unit_test_target
args.no_root = True
else:
# Do not run integration tests in unrecognized scenarios.
integration_test_target = None
@ -661,6 +681,14 @@ def main():
print("Unit Test target:", unit_test_target or "skip")
print("Integration Test target:", integration_test_target or "skip")
if not args.no_root and integration_test_target and integration_test_target.is_host:
if not sudo_is_passwordless():
print()
print("Prompting for sudo password to execute privileged tests.")
print("If you'd like to prevent this, use the --no-root option.")
subprocess.check_call(["sudo", "true"])
print()
main_target = integration_test_target or unit_test_target
if not main_target:
return
@ -692,7 +720,7 @@ def main():
test_executables = [
e
for e in executables
if e.is_test and should_run_executable(e, main_target, args.test_names)
if e.is_test and should_run_executable(e, main_target, args.test_names, not args.no_root)
]
all_results: List[ExecutableResults] = []

View file

@ -392,6 +392,7 @@ def exec_file_on_target(
args: List[str] = [],
extra_files: List[Path] = [],
generate_profile: bool = False,
execute_as_root: bool = False,
**kwargs: Any,
):
"""Executes a file on the test target.
@ -432,11 +433,14 @@ def exec_file_on_target(
env["LLVM_PROFILE_FILE"] = f"{profile_prefix}.%p"
cmd_line = [*prefix, str(filepath), *args]
if execute_as_root:
cmd_line = ["sudo", *cmd_line]
return subprocess.run(
cmd_line,
env=env,
timeout=timeout,
text=True,
shell=False,
**kwargs,
)
else:

View file

@ -97,7 +97,7 @@ commands=(
if [ "$ALL" == true ]; then
commands+=(
"./tools/run_tests"
"./tools/run_tests --no-root"
"./tools/run_tests --platform=mingw64"
"./tools/clippy --platform=mingw64"
"$(aarch64_wrapper) ./tools/run_tests --platform=aarch64"
@ -106,7 +106,7 @@ if [ "$ALL" == true ]; then
)
elif [ "$QUICK" != true ]; then
commands+=(
"./tools/run_tests"
"./tools/run_tests --no-root"
"$(aarch64_wrapper) ./tools/run_tests --platform=aarch64 --unit-tests"
"./tools/run_tests --platform=mingw64 --unit-tests"
)