core: Look up live user from PKEXEC_UID

[halfline]

anaconda can be run as gnome-initial-setup or liveuser on live images, depending on how its started by the use.

It currently assumes it will be started as liveuser always.

This commit makes it look up the username and home directory by querying PKEXEC_UID environment variable.

[halfline]

note this is untested. it's just fixing a problem I saw when looking at recent commit history.

[halfline]

cc @jkonecny12

[jkonecny12]

Thanks, I'll take a look on this.

[jkonecny12]

Tested and seems to be fix for https://bugzilla.redhat.com/show_bug.cgi?id=2235728 .

[jkonecny12]

@halfline thanks for the PR! Do you agree that I'll take over this PR and finish it?

[halfline]

@halfline thanks for the PR! Do you agree that I'll take over this PR and finish it?

yes please

[jkonecny12]

Verified even in the Live environment and it seems to be working correctly everywhere, great work!

We just need to fix the tests and link the bug.

[jkonecny12]

@VladimirSlavik, @M4rtinK, @rvykydal could you please tell me if you agree with the second commit (should be squashed if you agree). I tried to do this in way of pytest because they have great monkeypatching for environment vars, however, it was a bit bigger overhaul.

What I wonder is if we want to go this way or rather stick with the current UnitTests approach. I definitely can change the tests to the old solution if we prefer that.

[VladimirSlavik]

Please test also really possible values for the environment variable:

• negative number
• zero
• huge number out of range
• non-numeric string such as "hello world"
• empty string
• etc...

The actual code as written now does not try to do anything beyond surviving missing variable.

[jkonecny12]

UPDATED:

• Remove pytest migration - this is not the best PR for that test
• Updated the tests to have fixed tests

[jkonecny12]

I definitely can add that but I wonder what we want to cover here? If the PKEXEC_UID will be wrong then policy kit doesn't work as expected and we have much more problems then broken installation. Do we want to be that paranoid?

[jkonecny12]

I guess you can't be paranoid enough 😆

Anyway, updated to add more tests for the input environment variable.

[jkonecny12]

/kickstart-test --testtype smoke

[VladimirSlavik]

I guess you can't be paranoid enough 😆

Ah, sorry. The point was that there are values that will clearly make the code crash, and that the code is always executed, so you want to make it not-crashy.

[VladimirSlavik]

Thank you, looks good to me in terms of ensuring it works.

Please consider merging the "fail" tests and adding doc texts.

[VladimirSlavik]

Ah, now I understand why you thought this is verbose compared to pytest! You can actually collapse all of the "fail" tests into one by using patch.dict within a loop:

    @patch("pyanaconda.core.live_user.conf")
    @patch("pyanaconda.core.live_user.getpwuid")
    def test_get_live_user_with_wrong_pkexec_env(self, getpwuid_mock, conf_mock):
        """Test getting live user with wrong environment values"""
        # supposedly live but missing user
        conf_mock.system.provides_liveuser = True
        
        # no value
        with patch.dict("pyanaconda.core.live_user.os.environ", dict()):
            assert get_live_user() is None
            getpwuid_mock.assert_not_called()
        
        # all the rest
        for value in ("", "0", "-1", "999999999999999999999999", "hello world"):
            with patch.dict("pyanaconda.core.live_user.os.environ", {"PKEXEC_ENV": value}):
                assert get_live_user() == None
                getpwuid_mock.assert_not_called()

[jkonecny12]

Yeah, your suggested solution is probably better one. I was balancing between cleanliness and correctness.

Having each of these tests as separate test seems to me like more correct approach - one test / one test-case. However, yeah it's pretty verbose.

[jkonecny12]

UPDATED

• Implemented improvements pointed out by @VladimirSlavik

[jkonecny12]

/kickstart-test --testtype smoke