testing: handling non-root users with VNETs in pytests.

[melifaro]

Summary

Currently isolation and resource requirements are handled directly
by the kyua runner, based on the requirements specified by tests.
It works well for simple tests, but may cause discrepancy with tests
doing complex pre-setups. For example, all tests that perform
VNET setups require root access to properly function.

This change adds additional handling of the "require_user" property
within the python testing framework. Specifically, it requests
root access if the test class signals its root requirements and
drops privileges to the desired user after performing the pre-setup.

[melifaro]

Fix skipped tests.

[jlduran]

In my simplistic tests, this works well (on BaseTest, SingleVnetTestTemplate , and VnetTestTemplate):

@pytest.mark.require_user("root")
def test_root(self):
    assert subprocess.getoutput("id -un") == "root"

@pytest.mark.require_user("unprivileged")
def test_unprivileged(self):
    assert subprocess.getoutput("id -un") != "root"

Also, passing marks=[pytest.mark.require_user("unprivileged")] with parametrized tests (using VNETs):

class TestVnetSimple(SingleVnetTestTemplate):
    @pytest.mark.parametrize(
        "user_tuple",
        [
            pytest.param(["kldload -q if_epair", ""], marks=[pytest.mark.require_user("root")], id="root"),
            pytest.param(["kldload if_epair", "Operation not permitted"], marks=[pytest.mark.require_user("unprivileged")], id="unprivileged"),
        ],
    )
    def test_parametrize_require_user(self, user_tuple):
        command, output = user_tuple
        assert output in subprocess.getoutput(command)

[jlduran]

Sorry about the parametrized test. It is wrong.

[asomers]

I find this confusing. It's weird that Kyua handles some of the test case isolation, and Python handles other parts. It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work. It seems like either the vnet stuff needs to be moved up into Kyua itself, or the privilege dropping needs to be moved down out of the runner and into the test class. In the latter case, the test case would still have a "require.user root", but could use some other decorator or just the class setup to drop privileges.

[jlduran]

In D37923#861709, @asomers wrote:

It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work.

I'll retry everything tomorrow, but in this regard, things are working as expected.
Unprivileged users:

• Can run unprivileged test cases
• root test cases are skipped
• VNET test cases are skipped entirely (because an unprivileged user cannot create jails)

What is not working for me, is:

class TestVnetSimple(SingleVnetTestTemplate):
    @pytest.mark.parametrize(
        "user_tuple",
        [
            pytest.param(
                ["kldload if_epair", "module already loaded or in kernel"],
                marks=pytest.mark.require_user("root"),
                id="root",
            ),
            pytest.param(
                ["kldload if_epair", "Operation not permitted"],
                marks=pytest.mark.require_user("unprivileged"),
                id="unprivileged",
            ),
        ],
    )
    def test_parametrize_require_user(self, user_tuple):
        command, output = user_tuple
        assert output in subprocess.getoutput(command)

Specifically the test_parametrize_require_user[root] parameter. It gets an Operation not permitted error, as if unprivileged.
If you s/SingleVnetTestTemplate/BaseTest everything works as expected.

[jlduran]

Here are the basic tests used:
https://github.com/jlduran/freebsd-src/blob/D37923/tests/examples/test_examples.py
And their results, both executed by root and by an unprivileged user named "user":
https://cirrus-ci.com/build/5416736921485312

tests/atf_python/utils.py

67

Is this return annotation for debugging, to print the commented line below (which also looks like a remnant from debugging)?

[melifaro]

In D37923#861709, @asomers wrote:

I find this confusing. It's weird that Kyua handles some of the test case isolation, and Python handles other parts. It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work. It seems like either the vnet stuff needs to be moved up into Kyua itself, or the privilege dropping needs to be moved down out of the runner and into the test class. In the latter case, the test case would still have a "require.user root", but could use some other decorator or just the class setup to drop privileges.

Yep, that's a good observation! I'm happy to work towards making more "native" and less confusing.

The main use case of vnet-derived classes is to avoid the side-effects on the target system. Even without VNETs nearly all of the existing vnet-based python tests (70+) requires elevated privileges to add/delete interfaces/addresses/routes etc, rendering them impossible to run under the unprivilged user.
Logically, there are 2 "parts" in the each test - the first is some infrastructure setup/teardown and the second is the actual test itself. In my view, it maps pretty nicely to the pytest logic - the first part is implemented in the setup_method and generic cleanup wrappers and the second part is the actual test_... method.

I agree that splitting the isolation logic is not ideal. Splitting the ownership one the required_user is not pretty either. I wouldn't go as far as moving the vnet logic into the runner. Infrastructure setup requires notable flexibility - such as different amount of jails, interfaces, IPv4/IPv6 addresses and so on. Kyua is an abstract runner that doesn't know anything about the test specifics.
I like the idea of having a single owner for the privilege dropping.

The harderst part, as usual, is naming :-)
I thought initilly of renaming all kyua-implemented isolations to have a special atf_, kyua_ or other similar prefix, so one can clearly separate atf and pytest isolation layers.
Then I thought if that matters at all for the test writer - If I were the test writer, I only care about the result, but not about the implementation detals.
Additionally, regardless of the naming used, each test in the VNET-derived classes effectively requires root. I'm not sure if writing "require_user=root" decorator for each test in such a class is a good idea.
Maybe a class property, like setup_require_user="root" will work as a good-ebough marker for the user?

In that case, the test class can look like the following:

class TestVnetSimple(SingleVnetTestTemplate):
  SETUP_REQUIRE_USER="root"

  @pytest.mark.require_user("unprivileged")
  def test_one(self):
    pass

What do you think?

[melifaro]

In D37923#861728, @jlduran_gmail.com wrote:

In D37923#861709, @asomers wrote:

It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work.

I'll retry everything tomorrow, but in this regard, things are working as expected.
Unprivileged users:

• Can run unprivileged test cases
• root test cases are skipped
• VNET test cases are skipped entirely (because an unprivileged user cannot create jails)

What is not working for me, is:

class TestVnetSimple(SingleVnetTestTemplate):
    @pytest.mark.parametrize(
        "user_tuple",
        [
            pytest.param(
                ["kldload if_epair", "module already loaded or in kernel"],
                marks=pytest.mark.require_user("root"),
                id="root",
            ),
            pytest.param(
                ["kldload if_epair", "Operation not permitted"],
                marks=pytest.mark.require_user("unprivileged"),
                id="unprivileged",
            ),
        ],
    )
    def test_parametrize_require_user(self, user_tuple):
        command, output = user_tuple
        assert output in subprocess.getoutput(command)

Specifically the test_parametrize_require_user[root] parameter. It gets an Operation not permitted error, as if unprivileged.
If you s/SingleVnetTestTemplate/BaseTest everything works as expected.

Thank you for testing & catching the issue!
I'll check this case and update the diff.

[jlduran]

Sorry, the tests are failing because of a silly mistake from my part: (kldload) kernel modules cannot be loaded from inside the jail, even if the user is root.
Besides the trivial comments that looks like debugging leftovers, this revision works for me.

Thank you!

[melifaro]

In D37923#861728, @jlduran_gmail.com wrote:

In D37923#861709, @asomers wrote:

It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work.

I'll retry everything tomorrow, but in this regard, things are working as expected.
Unprivileged users:

• Can run unprivileged test cases
• root test cases are skipped
• VNET test cases are skipped entirely (because an unprivileged user cannot create jails)

What is not working for me, is:

class TestVnetSimple(SingleVnetTestTemplate):
    @pytest.mark.parametrize(
        "user_tuple",
        [
            pytest.param(
                ["kldload if_epair", "module already loaded or in kernel"],
                marks=pytest.mark.require_user("root"),
                id="root",
            ),
            pytest.param(
                ["kldload if_epair", "Operation not permitted"],
                marks=pytest.mark.require_user("unprivileged"),
                id="unprivileged",
            ),
        ],
    )
    def test_parametrize_require_user(self, user_tuple):
        command, output = user_tuple
        assert output in subprocess.getoutput(command)

Specifically the test_parametrize_require_user[root] parameter. It gets an Operation not permitted error, as if unprivileged.
If you s/SingleVnetTestTemplate/BaseTest everything works as expected.

Thank you for testing & catching the issue!
I'll check this case and update the diff.

In D37923#861709, @asomers wrote:

I find this confusing. It's weird that Kyua handles some of the test case isolation, and Python handles other parts. It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work. It seems like either the vnet stuff needs to be moved up into Kyua itself, or the privilege dropping needs to be moved down out of the runner and into the test class. In the latter case, the test case would still have a "require.user root", but could use some other decorator or just the class setup to drop privileges.

In D37923#861954, @melifaro wrote:

In D37923#861709, @asomers wrote:

I find this confusing. It's weird that Kyua handles some of the test case isolation, and Python handles other parts. It also means that an unprivileged user won't be able to simply type "kyua test ..." and expect the unprivileged test cases to work. It seems like either the vnet stuff needs to be moved up into Kyua itself, or the privilege dropping needs to be moved down out of the runner and into the test class. In the latter case, the test case would still have a "require.user root", but could use some other decorator or just the class setup to drop privileges.

Yep, that's a good observation! I'm happy to work towards making more "native" and less confusing.

The main use case of vnet-derived classes is to avoid the side-effects on the target system. Even without VNETs nearly all of the existing vnet-based python tests (70+) requires elevated privileges to add/delete interfaces/addresses/routes etc, rendering them impossible to run under the unprivilged user.
Logically, there are 2 "parts" in the each test - the first is some infrastructure setup/teardown and the second is the actual test itself. In my view, it maps pretty nicely to the pytest logic - the first part is implemented in the setup_method and generic cleanup wrappers and the second part is the actual test_... method.

I agree that splitting the isolation logic is not ideal. Splitting the ownership one the required_user is not pretty either. I wouldn't go as far as moving the vnet logic into the runner. Infrastructure setup requires notable flexibility - such as different amount of jails, interfaces, IPv4/IPv6 addresses and so on. Kyua is an abstract runner that doesn't know anything about the test specifics.
I like the idea of having a single owner for the privilege dropping.

The harderst part, as usual, is naming :-)
I thought initilly of renaming all kyua-implemented isolations to have a special atf_, kyua_ or other similar prefix, so one can clearly separate atf and pytest isolation layers.
Then I thought if that matters at all for the test writer - If I were the test writer, I only care about the result, but not about the implementation detals.
Additionally, regardless of the naming used, each test in the VNET-derived classes effectively requires root. I'm not sure if writing "require_user=root" decorator for each test in such a class is a good idea.
Maybe a class property, like setup_require_user="root" will work as a good-ebough marker for the user?

In that case, the test class can look like the following:

class TestVnetSimple(SingleVnetTestTemplate):
  SETUP_REQUIRE_USER="root"

  @pytest.mark.require_user("unprivileged")
  def test_one(self):
    pass

What do you think?

@asomers: friendly ping :-)
I'd love to have some code in the base addressing the isolation problem in the next week or so. If you could find the time and comment, that would unblock me to proceed towards the solution :-)

[melifaro]

I'm going to commit this change tomorrow, February 7.
It's always possible to change the implementation later if so desired.