diff mbox

[ovs-dev,7/8] ovs-dev.py: add --user and -u option

Message ID CACzMAJLJVK36dwxPiVDFWKBHaH0F0w31UgYtogJwP++RMd-Zng@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Andy Zhou Oct. 1, 2015, 1:53 a.m. UTC 
As discussed off-line, I decided to drop the "-u" option.  Now the
patch is simpler.

index fab3813..04415f9 100755

On Wed, Sep 30, 2015 at 5:25 PM, Joe Stringer <joestringer@nicira.com> wrote:
> On 22 September 2015 at 17:51, Andy Zhou <azhou@nicira.com> wrote:
>> ovs-dev.py "run" command now accepts the "--user" option for running
>> all ovs daemons as "user". The argument can be specified in
>> "user[:group]" format.
>>
>> '-u' is an short hand option that, if ovs-dev.py is launched as an
>> non-root user, then current user and group will be used as if they
>> are supplied by the --user option.
>>
>> "./ovs-dev.py run -u" can now launch ovsdb-server and ovs-vswitchd as
>> current user and group.
>>
>> Signed-off-by: Andy Zhou <azhou@nicira.com>
>
> I would have expected "-u" to be short-form for "--user", but they're
> subtly different. I'd prefer to see this represented one way, either
> the most common or the most flexible. If you really want to save
> characters, you could always alias it :-)
>
> I think this should also simplify the user handling inside, and make
> it more self-consistent.
>
>> ---
>>  utilities/ovs-dev.py | 36 +++++++++++++++++++++++++++++-------
>>  1 file changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
>> index 68c9a42..1f9dbd7 100755
>> --- a/utilities/ovs-dev.py
>> +++ b/utilities/ovs-dev.py
>> @@ -55,9 +55,16 @@ def uname():
>>      return _sh("uname", "-r", capture=True)[0].strip()
>>
>>
>> -def sudo():
>> +def sudo(run_as_me):
>>      if os.geteuid() != 0:
>> -        _sh(" ".join(["sudo"] + sys.argv), check=True)
>> +        opt = []
>> +        if (run_as_me):
>> +            user = subprocess.check_output(["id", "-un"]).strip()
>> +            group = subprocess.check_output(["id", "-gn"]).strip()
>> +            opt=["--user", user + ":" + group]
>> +            sys.argv.remove("-u")
>> +
>> +        _sh(" ".join(["sudo"] + sys.argv + opt), check=True)
>>          sys.exit(0)
>
> So, if you're running as yourself, you sudo to yourself; if you're
> running as a different user, you sudo to root, and if you're running
> as root you also sudo to root?
>
> <snip>
>
>> @@ -231,6 +238,12 @@ def run():
>>
>>      opts = ["--pidfile", "--log-file"]
>>
>> +    if (options.user == "") or (options.user == "root:root"):
>> +        _sh("chown", "root:root", "-R", RUNDIR)
>> +    else:
>> +        _sh("chown", options.user, "-R", RUNDIR);
>> +        opts = ["--user", options.user] + opts
>> +
>
> So.. if you're running "as_me", then everything will be chowned root:root?

Comments

Joe Stringer Oct. 1, 2015, 1:56 a.m. UTC | #1 
On 30 September 2015 at 18:53, Andy Zhou <azhou@nicira.com> wrote:
> As discussed off-line, I decided to drop the "-u" option.  Now the
> patch is simpler.
>
> index fab3813..04415f9 100755
> --- a/utilities/ovs-dev.py
> +++ b/utilities/ovs-dev.py
> @@ -232,6 +232,13 @@ def run():
>
>      opts = ["--pidfile", "--log-file"]
>
> +    if (options.user == "") or (options.user == "root:root"):
> +        _sh("chown", "root:root", "-R", RUNDIR)
> +        sys.argv.remove("--user")
> +    else:
> +        _sh("chown", options.user, "-R", RUNDIR);
> +        opts = ["--user", options.user] + opts
> +
>      _sh(*(["ovsdb-server",
>             "--remote=punix:%s/run/db.sock" % RUNDIR,
>             "--remote=db:Open_vSwitch,Open_vSwitch,manager_options",
> @@ -316,7 +323,7 @@ Basic Configuration:
>
>      # First install the basic requirements needed to build Open vSwitch.
>      sudo apt-get install git build-essential libtool autoconf pkg-config \\
> -            libssl-dev gdb linux-headers-`uname -r`
> +            libssl-dev gdb libcap-ng linux-headers-`uname -r`
>
>      # Next clone the Open vSwitch source.
>      git clone https://github.com/openvswitch/ovs.git %(ovs)s
> @@ -415,6 +422,8 @@ def main():
>                       help="run ovs-vswitchd with dpdk subopts (ended by --)")
>      group.add_option("--clang", dest="clang", action="store_true",
>                       help="Use binaries built by clang")
> +    group.add_option("--user", dest="user", action="store", default="",
> +                     help="run all daemons as a non root user")
>
>      parser.add_option_group(group)
>

Looks good to me. I assume you've tested it.

Acked-by: Joe Stringer <joestringer@nicira.com>
diff mbox

Patch

--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -232,6 +232,13 @@  def run():

     opts = ["--pidfile", "--log-file"]

+    if (options.user == "") or (options.user == "root:root"):
+        _sh("chown", "root:root", "-R", RUNDIR)
+        sys.argv.remove("--user")
+    else:
+        _sh("chown", options.user, "-R", RUNDIR);
+        opts = ["--user", options.user] + opts
+
     _sh(*(["ovsdb-server",
            "--remote=punix:%s/run/db.sock" % RUNDIR,
            "--remote=db:Open_vSwitch,Open_vSwitch,manager_options",
@@ -316,7 +323,7 @@  Basic Configuration:

     # First install the basic requirements needed to build Open vSwitch.
     sudo apt-get install git build-essential libtool autoconf pkg-config \\
-            libssl-dev gdb linux-headers-`uname -r`
+            libssl-dev gdb libcap-ng linux-headers-`uname -r`

     # Next clone the Open vSwitch source.
     git clone https://github.com/openvswitch/ovs.git %(ovs)s
@@ -415,6 +422,8 @@  def main():
                      help="run ovs-vswitchd with dpdk subopts (ended by --)")
     group.add_option("--clang", dest="clang", action="store_true",
                      help="Use binaries built by clang")
+    group.add_option("--user", dest="user", action="store", default="",
+                     help="run all daemons as a non root user")

     parser.add_option_group(group)