nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 322 forks source link

Test with root access in GitHub workflows #1173

Closed arbourd closed 3 months ago

arbourd commented 3 months ago

To enable tests that require privileged root access, this commit tests with sudo. The Java and Python jobs have additional permissions issues, so they are also configured and made with sudo.

A small permissions fix is required before running tests to allow non-root users to execute within the /home/runner directory.

This change also removes the custom directories that were required without root access.

arbourd commented 3 months ago

Re: @ac000

A more general question is do we need to do all the configuring and compiling as root?

I know this is just a throwaway environment, but from a principle of least privilege, is it really only the actual pytests that need to be run as root?

This is actually how I started, but, there are issues with the Java and Python tests: for whatever reasons there are issues with requiring sudo either during the configure or make steps. I am happy to try and make it work though, if we want to try and use sudo as little as possible.

Specifically:

ac000 commented 3 months ago

Even if we can do most of it without sudo and only use sudo for the bits that currently require it...

arbourd commented 3 months ago

@ac000 All done. I will rebase and clean up the commits before merging. 2nd commit is the specific-sudoing.

ac000 commented 3 months ago

I would like to see the final result before merging...

ac000 commented 3 months ago

OK, change looks good.

Commit message just needs re-wrapping. We want to wrap commit messages after 72 chars. Keep in mind that when viewing commit messages in git, it will put 4 chars of padding at the beginning of each line, so in a standard 80 char terminal you will get your message centred with 4 chars of padding each side.

And seeing as you asked, I've added your Signed-off-by and my Reviewed-by to the message. So it looks like

Test with root access in GitHub workflows                                          

To enable tests that require privileged root access, this commit tests          
with `sudo`. The Java and Python jobs have additional permissions               
issues, so they are also configured and made with `sudo`.                       

A small permissions fix is required before running tests to allow               
non-root users to execute within the `/home/runner` directory.                  

This change also removes the custom directories that were required              
without root access.                                                            

Reviewed-by: Andrew Clayton <a.clayton@nginx.com>                               
Signed-off-by: Dylan Arbour <d.arbour@f5.com>

and in git it looks like

commit 639bff661a12ca999c34477c93ad39d105b44e93 (HEAD -> sutest)
Author: Dylan Arbour <d.arbour@f5.com>
Date:   Tue Feb 27 09:41:07 2024 -0500

    Test with root access in GitHub workflows

    To enable tests that require privileged root access, this commit tests
    with `sudo`. The Java and Python jobs have additional permissions
    issues, so they are also configured and made with `sudo`.

    A small permissions fix is required before running tests to allow
    non-root users to execute within the `/home/runner` directory.

    This change also removes the custom directories that were required
    without root access.

    Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
    Signed-off-by: Dylan Arbour <d.arbour@f5.com>

You'll notice I also made a couple of minor tweaks to the message while I was at it...