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

Add FreeBSD Jail execution environment support #224

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@

* The FreeBSD Foundation
* Google Inc.
* Igor Ostapenko <pm@igoro.pro>
3 changes: 2 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ include examples/Makefile.am.inc
include integration/Makefile.am.inc
include misc/Makefile.am.inc
include model/Makefile.am.inc
include os/freebsd/Makefile.am.inc
include store/Makefile.am.inc
include utils/Makefile.am.inc

bin_PROGRAMS = kyua
kyua_SOURCES = main.cpp
kyua_CXXFLAGS = $(CLI_CFLAGS) $(ENGINE_CFLAGS) $(UTILS_CFLAGS)
kyua_LDADD = $(CLI_LIBS) $(ENGINE_LIBS) $(UTILS_LIBS)
kyua_LDADD = $(CLI_LIBS) $(ENGINE_LIBS) $(FREEBSD_LIBS) $(UTILS_LIBS)

CHECK_ENVIRONMENT = KYUA_CONFDIR="/non-existent" \
KYUA_DOCDIR="$(abs_top_srcdir)" \
Expand Down
12 changes: 7 additions & 5 deletions cli/cmd_config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ fake_config(void)
{
config::tree user_config = engine::default_config();
user_config.set_string("architecture", "the-architecture");
user_config.set_string("execenvs", "the-env");
user_config.set_string("parallelism", "128");
user_config.set_string("platform", "the-platform");
//user_config.set_string("unprivileged_user", "");
Expand All @@ -83,12 +84,13 @@ ATF_TEST_CASE_BODY(all)
cmdline::ui_mock ui;
ATF_REQUIRE_EQ(EXIT_SUCCESS, cmd.main(&ui, args, fake_config()));

ATF_REQUIRE_EQ(5, ui.out_log().size());
ATF_REQUIRE_EQ(6, ui.out_log().size());
ATF_REQUIRE_EQ("architecture = the-architecture", ui.out_log()[0]);
ATF_REQUIRE_EQ("parallelism = 128", ui.out_log()[1]);
ATF_REQUIRE_EQ("platform = the-platform", ui.out_log()[2]);
ATF_REQUIRE_EQ("test_suites.foo.bar = first", ui.out_log()[3]);
ATF_REQUIRE_EQ("test_suites.foo.baz = second", ui.out_log()[4]);
ATF_REQUIRE_EQ("execenvs = the-env", ui.out_log()[1]);
ATF_REQUIRE_EQ("parallelism = 128", ui.out_log()[2]);
ATF_REQUIRE_EQ("platform = the-platform", ui.out_log()[3]);
ATF_REQUIRE_EQ("test_suites.foo.bar = first", ui.out_log()[4]);
ATF_REQUIRE_EQ("test_suites.foo.baz = second", ui.out_log()[5]);
ATF_REQUIRE(ui.err_log().empty());
}

Expand Down
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -167,5 +167,6 @@ fi
AM_CONDITIONAL(TARGET_SRCDIR_EMPTY, [test -z "${target_srcdir}"])
AC_SUBST([target_srcdir])

AM_CONDITIONAL([FreeBSD], [test "$(uname -o)" = "FreeBSD"])

AC_OUTPUT
13 changes: 11 additions & 2 deletions doc/kyua.conf.5.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.\" Copyright 2012 The Kyua Authors.
.\" Copyright 2012-2024 The Kyua Authors.
.\" All rights reserved.
.\"
.\" Redistribution and use in source and binary forms, with or without
Expand All @@ -25,7 +25,7 @@
.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
.\" OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.Dd February 20, 2015
.Dd March 22, 2024
.Dt KYUA.CONF 5
.Os
.Sh NAME
Expand All @@ -36,6 +36,7 @@
.Pp
Variables:
.Va architecture ,
.Va execenvs ,
.Va platform ,
.Va test_suites ,
.Va unprivileged_user .
Expand Down Expand Up @@ -72,6 +73,14 @@ The following variables are internally recognized by
.Bl -tag -width XX -offset indent
.It Va architecture
Name of the system architecture (aka processor type).
.It Va execenvs
Whitespace-separated list of execution environment names.
.Pp
Only tests which require one of the given execution environments will be run.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioning what execenv's are supported would be a good idea -- otherwise it's unclear (without looking at the source code) what is or isn't supported.
Referencing alternative documentation is a good thing to do in place of hardcoding the values, but I think being explicit avoids the need for the documentation potentially becoming out of sync/less implicit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already covered in kyuafile.5: https://github.com/ihoro/kyua/blob/9721be7abbd48f2ee020e5deaee1e1daa44c0087/doc/kyuafile.5.in#L181
Screenshot 2024-05-07 at 3 13 38 PM

I guess we could simply make a reference to the kyuafile.5 for the possible values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the mentioned reference.

.Pp
See
.Xr kyuafile 5
for the list of possible execution environments.
.It Va parallelism
Maximum number of test cases to execute concurrently.
.It Va platform
Expand Down
103 changes: 101 additions & 2 deletions doc/kyuafile.5.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.\" Copyright 2012 The Kyua Authors.
.\" Copyright 2012-2024 The Kyua Authors.
.\" All rights reserved.
.\"
.\" Redistribution and use in source and binary forms, with or without
Expand All @@ -25,7 +25,7 @@
.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
.\" OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.Dd July 3, 2015
.Dd March 23, 2024
.Dt KYUAFILE 5
.Os
.Sh NAME
Expand Down Expand Up @@ -173,6 +173,75 @@ Refer to the
section below for clarification.
.It Va description
Textual description of the test.
.It Va execenv
The name of the execution environment to be used for running the test.
If empty or not defined, the
.Sq host
execution environment is meant.
The possible values are:
.Bl -tag -width xUnnnnnnn
.It host
The default environment which runs the test as a usual child process.
.It jail
The
.Fx
.Xr jail 8
environment.
It creates a temporary jail to run the test and its optional cleanup logic
within.
.Pp
This feature requires
.Xr kyua 1
to be running with superuser privileges.
.Pp
The difference between
.Va security.jail.children.max
and
.Va security.jail.children.cur
sysctl of the jail
.Xr kyua 1
is running within must have a value high enough for the jail based tests
planned to be run.
For instance, the value 1 should be enough for a sequential run of simple
tests.
Otherwise, such aspects as parallel test execution and sub-jails spawned
by specific test cases should be considered.
.Pp
The formula of a temporary jail name is
.Sq kyua
+
.Va test program path
+
.Sq _
+
.Va test case name .
All non-alphanumeric characters are replaced with
.Sq _ .
.Sq kyua_usr_tests_sys_netpfil_pf_pass_block_v4
is an example for /usr/tests/sys/netpfil/pf/pass_block:v4 test case.
.El
.It Va execenv_jail_params
Additional test-specific whitespace-separated parameters of
.Fx
.Xr jail 8
to create a temporary jail within which the test is run.
It makes sense only if execenv is set to
.Sq jail .
.sp
.Xr kyua 1
implicitly passes
.Sq children.max
parameter to
.Xr jail 8
for a temporary jail with the maximum possible value according to
the jail
.Xr kyua 1
itself is running within.
It allows tests to easily spawn their own sub-jails without additional
configuration.
It can be overridden via
.Va execenv_jail_params
if needed.
.It Va is_exclusive
If true, indicates that this test program cannot be executed along any other
programs at the same time.
Expand Down Expand Up @@ -360,6 +429,36 @@ test_suite('FreeBSD')
plain_test_program{name='the_test',
['custom.FreeBSD-Bug-Id']='category/12345'}
.Ed
.Ss FreeBSD jail execution environment
The following example configures the test to be run within a temporary jail
with
.Xr vnet 9
support and the permission to create raw sockets:
.Bd -literal -offset indent
syntax(2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(picking the first instance)
The syntax version should be bumped for an interface change/addition like this.

Suggested change
syntax(2)
syntax(3)

This allows the engine to treat function in a backwards compatible manner with the older Kyuafiles by not dealing with the execenv related syntax.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this topic. I was thinking about this, but I decided not to make a move and postpone its discussion due to lack of strong opinions on my side. In terms of "syntax" term it's not changed, only extra metadata can be defined (if we think about metadata as a free set of key-value), but, probably, the project's philosophy/design means any change should bump the version -- this is where long term maintainer's opinion is expected. Also, I've found no direct support of versioning in the code, i.e. older Kyua binary simply stops a Kyuafile processing and errors about "unknown property". Conversely, if Kyuafile's syntax is set to 3 then Kyua errors about the whole file even if only a single test needs newer metadata like execenv, and other tests in the suite are still fine and could be run by the previous version of Kyua. It's kind of nothing to pick among these scenarios, and probably additional logic should be implemented. For instance, the first thing that comes to my mind is to skip/fail a test with unknown metadata to keep other tests of the same suite still working. But to make it work like this we should follow the design which means that the syntax is not changed and its version still is 2.

I guess we could discuss and conclude here on the desired direction for the project but implement it as a separate PR if it requires extra changes. What do you think?


test_suite('FreeBSD')

atf_test_program{name='network_test',
execenv='jail',
execenv_jail_params='vnet allow.raw_sockets',
required_user='root'}
.Ed
.Pp
A test case itself may have no requirements in superuser privileges,
but required_user='root' metadata property reminds that the jail execution
environment requires
.Xr kyua 1
being running with root privileges, and the test is skipped otherwise with
the respective message. The combination of
.Va execenv
set to
.Sq jail
and
.Va required_user
set to
.Sq unprivileged
does not work respectively.
.Ss Connecting disjoint test suites
Now suppose you had various test suites on your file system and you would
like to connect them together so that they could be executed and treated as
Expand Down
8 changes: 8 additions & 0 deletions drivers/report_junit_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ static const char* const default_metadata =
"allowed_architectures is empty\n"
"allowed_platforms is empty\n"
"description is empty\n"
"execenv is empty\n"
ngie-eign marked this conversation as resolved.
Show resolved Hide resolved
"execenv_jail_params is empty\n"
"has_cleanup = false\n"
"is_exclusive = false\n"
"required_configs is empty\n"
Expand All @@ -80,6 +82,8 @@ static const char* const overriden_metadata =
"allowed_architectures is empty\n"
"allowed_platforms is empty\n"
"description = Textual description\n"
"execenv is empty\n"
"execenv_jail_params is empty\n"
"has_cleanup = false\n"
"is_exclusive = false\n"
"required_configs is empty\n"
Expand Down Expand Up @@ -199,6 +203,8 @@ ATF_TEST_CASE_BODY(junit_metadata__overrides)
.add_allowed_architecture("arch1")
.add_allowed_platform("platform1")
.set_description("This is a test")
.set_execenv("jail")
.set_execenv_jail_params("vnet")
.set_has_cleanup(true)
.set_is_exclusive(true)
.add_required_config("config1")
Expand All @@ -215,6 +221,8 @@ ATF_TEST_CASE_BODY(junit_metadata__overrides)
+ "allowed_architectures = arch1\n"
+ "allowed_platforms = platform1\n"
+ "description = This is a test\n"
+ "execenv = jail\n"
+ "execenv_jail_params = vnet\n"
+ "has_cleanup = true\n"
+ "is_exclusive = true\n"
+ "required_configs = config1\n"
Expand Down
8 changes: 5 additions & 3 deletions engine/Makefile.am.inc
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

ENGINE_CFLAGS = $(STORE_CFLAGS) $(MODEL_CFLAGS) $(UTILS_CFLAGS)
ENGINE_LIBS = libengine.la $(STORE_LIBS) $(MODEL_LIBS) $(UTILS_LIBS)
ENGINE_CFLAGS = $(STORE_CFLAGS) $(MODEL_CFLAGS) $(UTILS_CFLAGS) $(ENGINE_EXECENV_CFLAGS)
ENGINE_LIBS = libengine.la $(STORE_LIBS) $(MODEL_LIBS) $(UTILS_LIBS) $(ENGINE_EXECENV_LIBS)

noinst_LTLIBRARIES += libengine.la
libengine_la_CPPFLAGS = $(STORE_CFLAGS) $(UTILS_CFLAGS)
libengine_la_CPPFLAGS = $(STORE_CFLAGS) $(UTILS_CFLAGS) $(ENGINE_EXECENV_CFLAGS)
libengine_la_SOURCES = engine/atf.cpp
libengine_la_SOURCES += engine/atf.hpp
libengine_la_SOURCES += engine/atf_list.cpp
Expand Down Expand Up @@ -153,3 +153,5 @@ engine_scheduler_test_SOURCES = engine/scheduler_test.cpp
engine_scheduler_test_CXXFLAGS = $(ENGINE_CFLAGS) $(ATF_CXX_CFLAGS)
engine_scheduler_test_LDADD = $(ENGINE_LIBS) $(ATF_CXX_LIBS)
endif

include engine/execenv/Makefile.am.inc
11 changes: 9 additions & 2 deletions engine/atf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extern "C" {
#include "engine/atf_list.hpp"
#include "engine/atf_result.hpp"
#include "engine/exceptions.hpp"
#include "engine/execenv/execenv.hpp"
#include "model/test_case.hpp"
#include "model/test_program.hpp"
#include "model/test_result.hpp"
Expand All @@ -54,6 +55,7 @@ extern "C" {
#include "utils/stream.hpp"

namespace config = utils::config;
namespace execenv = engine::execenv;
namespace fs = utils::fs;
namespace process = utils::process;

Expand Down Expand Up @@ -190,7 +192,10 @@ engine::atf_interface::exec_test(const model::test_program& test_program,

args.push_back(F("-r%s") % (control_directory / result_name));
args.push_back(test_case_name);
process::exec(test_program.absolute_path(), args);

auto e = execenv::get(test_program, test_case_name);
e->init();
e->exec(args);
}


Expand Down Expand Up @@ -219,7 +224,9 @@ engine::atf_interface::exec_cleanup(
}

args.push_back(F("%s:cleanup") % test_case_name);
process::exec(test_program.absolute_path(), args);

auto e = execenv::get(test_program, test_case_name);
e->exec(args);
}


Expand Down
4 changes: 4 additions & 0 deletions engine/atf_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ engine::parse_atf_metadata(const model::properties_map& props)
mdbuilder.set_string("has_cleanup", value);
} else if (name == "require.arch") {
mdbuilder.set_string("allowed_architectures", value);
} else if (name == "execenv") {
mdbuilder.set_string("execenv", value);
} else if (name == "execenv.jail.params") {
mdbuilder.set_string("execenv_jail_params", value);
} else if (name == "require.config") {
mdbuilder.set_string("required_configs", value);
} else if (name == "require.files") {
Expand Down
18 changes: 18 additions & 0 deletions engine/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <stdexcept>

#include "engine/exceptions.hpp"
#include "engine/execenv/execenv.hpp"
#include "utils/config/exceptions.hpp"
#include "utils/config/parser.hpp"
#include "utils/config/tree.ipp"
Expand All @@ -43,6 +44,7 @@
#include "utils/text/operations.ipp"

namespace config = utils::config;
namespace execenv = engine::execenv;
namespace fs = utils::fs;
namespace passwd = utils::passwd;
namespace text = utils::text;
Expand All @@ -59,6 +61,7 @@ static void
init_tree(config::tree& tree)
{
tree.define< config::string_node >("architecture");
tree.define< config::strings_set_node >("execenvs");
tree.define< config::positive_int_node >("parallelism");
tree.define< config::string_node >("platform");
tree.define< engine::user_node >("unprivileged_user");
Expand All @@ -74,6 +77,14 @@ static void
set_defaults(config::tree& tree)
{
tree.set< config::string_node >("architecture", KYUA_ARCHITECTURE);

std::set< std::string > supported;
for (auto em : execenv::execenvs())
if (em->is_supported())
supported.insert(em->name());
supported.insert(execenv::default_execenv_name);
tree.set< config::strings_set_node >("execenvs", supported);

// TODO(jmmv): Automatically derive this from the number of CPUs in the
// machine and forcibly set to a value greater than 1. Still testing
// the new parallel implementation as of 2015-02-27 though.
Expand Down Expand Up @@ -229,6 +240,13 @@ engine::empty_config(void)
{
config::tree tree(false);
init_tree(tree);

// Tests of Kyua itself tend to use an empty config, i.e. default
// execution environment is used. Let's allow it.
std::set< std::string > supported;
supported.insert(engine::execenv::default_execenv_name);
tree.set< config::strings_set_node >("execenvs", supported);

return tree;
}

Expand Down
Loading