| Message ID | 20240325104014.919627-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-ctl: Use the current user for default file permissions. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Thanks Ales, Acked-by: Mark Michelson <mmichels@redhat.com> On 3/25/24 06:40, Ales Musil wrote: > The ovn-ctl utility was assuming that the user/group is always root, > when not specified otherwise by the --ovn-user/--ovn-group options. > This has the consequence of trying to change permissions of OVN > directories to root:root even though the script might be run as > completely different user. > > Take the current user and group instead of the hardcoded root. > At the same time remove the ovs-user option as it was not used for > anything and might be confusing. > > Reported-at: https://issues.redhat.com/browse/FDP-245 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > utilities/ovn-ctl | 5 ++--- > utilities/ovn-ctl.8.xml | 1 - > utilities/ovn-lib.in | 4 ++-- > 3 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > index 700efe35a..dae5e22f4 100755 > --- a/utilities/ovn-ctl > +++ b/utilities/ovn-ctl > @@ -269,8 +269,8 @@ $cluster_remote_port > # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if set. > # This is required because the ovndbs are created with root permission > # if not present when create_cluster/upgrade_db is called. > - INSTALL_USER="root" > - INSTALL_GROUP="root" > + INSTALL_USER="$(id -un)" > + INSTALL_GROUP="$(id -gn)" > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" > > @@ -1088,7 +1088,6 @@ Options: > --ovn-ic-sb-db-ssl-protocols=PROTOCOLS OVN IC Southbound DB SSL protocols > --ovn-ic-sb-db-ssl-ciphers=CIPHERS OVN IC Southbound DB SSL cipher list > --ovn-user="user[:group]" pass the --user flag to the ovn daemons > - --ovs-user="user[:group]" pass the --user flag to ovs daemons > --ovsdb-nb-wrapper=WRAPPER run with a wrapper like valgrind for debugging > --ovsdb-sb-wrapper=WRAPPER run with a wrapper like valgrind for debugging > --ovsdb-disable-file-column-diff=no|yes > diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml > index 57712bfdc..c0fbb0792 100644 > --- a/utilities/ovn-ctl.8.xml > +++ b/utilities/ovn-ctl.8.xml > @@ -70,7 +70,6 @@ > <p><code>--ovsdb-nb-wrapper=<var>WRAPPER</var></code></p> > <p><code>--ovsdb-sb-wrapper=<var>WRAPPER</var></code></p> > <p><code>--ovn-user=<var>USER:GROUP</var></code></p> > - <p><code>--ovs-user=<var>USER:GROUP</var></code></p> > <p><code>-h</code> | <code>--help</code></p> > > <h1>File location options</h1> > diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in > index 1e48ef28c..65cbfbcdc 100644 > --- a/utilities/ovn-lib.in > +++ b/utilities/ovn-lib.in > @@ -48,8 +48,8 @@ LC_ALL=C; export LC_ALL > ovn_install_dir () { > DIR="$1" > INSTALL_MODE="${2:-755}" > - INSTALL_USER="root" > - INSTALL_GROUP="root" > + INSTALL_USER="$(id -un)" > + INSTALL_GROUP="$(id -gn)" > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" >
On Thu, Apr 4, 2024 at 4:01 PM Mark Michelson <mmichels@redhat.com> wrote: > Thanks Ales, > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks. I applied this patch to the main branch. Does this require a backport ? Numan > On 3/25/24 06:40, Ales Musil wrote: > > The ovn-ctl utility was assuming that the user/group is always root, > > when not specified otherwise by the --ovn-user/--ovn-group options. > > This has the consequence of trying to change permissions of OVN > > directories to root:root even though the script might be run as > > completely different user. > > > > Take the current user and group instead of the hardcoded root. > > At the same time remove the ovs-user option as it was not used for > > anything and might be confusing. > > > > Reported-at: https://issues.redhat.com/browse/FDP-245 > > Signed-off-by: Ales Musil <amusil@redhat.com> > > --- > > utilities/ovn-ctl | 5 ++--- > > utilities/ovn-ctl.8.xml | 1 - > > utilities/ovn-lib.in | 4 ++-- > > 3 files changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > > index 700efe35a..dae5e22f4 100755 > > --- a/utilities/ovn-ctl > > +++ b/utilities/ovn-ctl > > @@ -269,8 +269,8 @@ $cluster_remote_port > > # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if > set. > > # This is required because the ovndbs are created with root > permission > > # if not present when create_cluster/upgrade_db is called. > > - INSTALL_USER="root" > > - INSTALL_GROUP="root" > > + INSTALL_USER="$(id -un)" > > + INSTALL_GROUP="$(id -gn)" > > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" > > > > @@ -1088,7 +1088,6 @@ Options: > > --ovn-ic-sb-db-ssl-protocols=PROTOCOLS OVN IC Southbound DB SSL > protocols > > --ovn-ic-sb-db-ssl-ciphers=CIPHERS OVN IC Southbound DB SSL cipher > list > > --ovn-user="user[:group]" pass the --user flag to the ovn > daemons > > - --ovs-user="user[:group]" pass the --user flag to ovs daemons > > --ovsdb-nb-wrapper=WRAPPER run with a wrapper like valgrind for > debugging > > --ovsdb-sb-wrapper=WRAPPER run with a wrapper like valgrind for > debugging > > --ovsdb-disable-file-column-diff=no|yes > > diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml > > index 57712bfdc..c0fbb0792 100644 > > --- a/utilities/ovn-ctl.8.xml > > +++ b/utilities/ovn-ctl.8.xml > > @@ -70,7 +70,6 @@ > > <p><code>--ovsdb-nb-wrapper=<var>WRAPPER</var></code></p> > > <p><code>--ovsdb-sb-wrapper=<var>WRAPPER</var></code></p> > > <p><code>--ovn-user=<var>USER:GROUP</var></code></p> > > - <p><code>--ovs-user=<var>USER:GROUP</var></code></p> > > <p><code>-h</code> | <code>--help</code></p> > > > > <h1>File location options</h1> > > diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in > > index 1e48ef28c..65cbfbcdc 100644 > > --- a/utilities/ovn-lib.in > > +++ b/utilities/ovn-lib.in > > @@ -48,8 +48,8 @@ LC_ALL=C; export LC_ALL > > ovn_install_dir () { > > DIR="$1" > > INSTALL_MODE="${2:-755}" > > - INSTALL_USER="root" > > - INSTALL_GROUP="root" > > + INSTALL_USER="$(id -un)" > > + INSTALL_GROUP="$(id -gn)" > > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Fri, Apr 5, 2024 at 10:09 PM Numan Siddique <numans@ovn.org> wrote: > > > On Thu, Apr 4, 2024 at 4:01 PM Mark Michelson <mmichels@redhat.com> wrote: > >> Thanks Ales, >> >> Acked-by: Mark Michelson <mmichels@redhat.com> >> > > Thanks. I applied this patch to the main branch. Does this require a > backport ? > Hi Numan, yes please, it should be backported to all supported branches if possible. > Numan > > >> On 3/25/24 06:40, Ales Musil wrote: >> > The ovn-ctl utility was assuming that the user/group is always root, >> > when not specified otherwise by the --ovn-user/--ovn-group options. >> > This has the consequence of trying to change permissions of OVN >> > directories to root:root even though the script might be run as >> > completely different user. >> > >> > Take the current user and group instead of the hardcoded root. >> > At the same time remove the ovs-user option as it was not used for >> > anything and might be confusing. >> > >> > Reported-at: https://issues.redhat.com/browse/FDP-245 >> > Signed-off-by: Ales Musil <amusil@redhat.com> >> > --- >> > utilities/ovn-ctl | 5 ++--- >> > utilities/ovn-ctl.8.xml | 1 - >> > utilities/ovn-lib.in | 4 ++-- >> > 3 files changed, 4 insertions(+), 6 deletions(-) >> > >> > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl >> > index 700efe35a..dae5e22f4 100755 >> > --- a/utilities/ovn-ctl >> > +++ b/utilities/ovn-ctl >> > @@ -269,8 +269,8 @@ $cluster_remote_port >> > # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if >> set. >> > # This is required because the ovndbs are created with root >> permission >> > # if not present when create_cluster/upgrade_db is called. >> > - INSTALL_USER="root" >> > - INSTALL_GROUP="root" >> > + INSTALL_USER="$(id -un)" >> > + INSTALL_GROUP="$(id -gn)" >> > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" >> > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" >> > >> > @@ -1088,7 +1088,6 @@ Options: >> > --ovn-ic-sb-db-ssl-protocols=PROTOCOLS OVN IC Southbound DB SSL >> protocols >> > --ovn-ic-sb-db-ssl-ciphers=CIPHERS OVN IC Southbound DB SSL cipher >> list >> > --ovn-user="user[:group]" pass the --user flag to the ovn >> daemons >> > - --ovs-user="user[:group]" pass the --user flag to ovs daemons >> > --ovsdb-nb-wrapper=WRAPPER run with a wrapper like valgrind for >> debugging >> > --ovsdb-sb-wrapper=WRAPPER run with a wrapper like valgrind for >> debugging >> > --ovsdb-disable-file-column-diff=no|yes >> > diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml >> > index 57712bfdc..c0fbb0792 100644 >> > --- a/utilities/ovn-ctl.8.xml >> > +++ b/utilities/ovn-ctl.8.xml >> > @@ -70,7 +70,6 @@ >> > <p><code>--ovsdb-nb-wrapper=<var>WRAPPER</var></code></p> >> > <p><code>--ovsdb-sb-wrapper=<var>WRAPPER</var></code></p> >> > <p><code>--ovn-user=<var>USER:GROUP</var></code></p> >> > - <p><code>--ovs-user=<var>USER:GROUP</var></code></p> >> > <p><code>-h</code> | <code>--help</code></p> >> > >> > <h1>File location options</h1> >> > diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in >> > index 1e48ef28c..65cbfbcdc 100644 >> > --- a/utilities/ovn-lib.in >> > +++ b/utilities/ovn-lib.in >> > @@ -48,8 +48,8 @@ LC_ALL=C; export LC_ALL >> > ovn_install_dir () { >> > DIR="$1" >> > INSTALL_MODE="${2:-755}" >> > - INSTALL_USER="root" >> > - INSTALL_GROUP="root" >> > + INSTALL_USER="$(id -un)" >> > + INSTALL_GROUP="$(id -gn)" >> > [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" >> > [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" >> > >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> Thanks, Ales
diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl index 700efe35a..dae5e22f4 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -269,8 +269,8 @@ $cluster_remote_port # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if set. # This is required because the ovndbs are created with root permission # if not present when create_cluster/upgrade_db is called. - INSTALL_USER="root" - INSTALL_GROUP="root" + INSTALL_USER="$(id -un)" + INSTALL_GROUP="$(id -gn)" [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}" @@ -1088,7 +1088,6 @@ Options: --ovn-ic-sb-db-ssl-protocols=PROTOCOLS OVN IC Southbound DB SSL protocols --ovn-ic-sb-db-ssl-ciphers=CIPHERS OVN IC Southbound DB SSL cipher list --ovn-user="user[:group]" pass the --user flag to the ovn daemons - --ovs-user="user[:group]" pass the --user flag to ovs daemons --ovsdb-nb-wrapper=WRAPPER run with a wrapper like valgrind for debugging --ovsdb-sb-wrapper=WRAPPER run with a wrapper like valgrind for debugging --ovsdb-disable-file-column-diff=no|yes diff --git a/utilities/ovn-ctl.8.xml b/utilities/ovn-ctl.8.xml index 57712bfdc..c0fbb0792 100644 --- a/utilities/ovn-ctl.8.xml +++ b/utilities/ovn-ctl.8.xml @@ -70,7 +70,6 @@ <p><code>--ovsdb-nb-wrapper=<var>WRAPPER</var></code></p> <p><code>--ovsdb-sb-wrapper=<var>WRAPPER</var></code></p> <p><code>--ovn-user=<var>USER:GROUP</var></code></p> - <p><code>--ovs-user=<var>USER:GROUP</var></code></p> <p><code>-h</code> | <code>--help</code></p> <h1>File location options</h1> diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in index 1e48ef28c..65cbfbcdc 100644 --- a/utilities/ovn-lib.in +++ b/utilities/ovn-lib.in @@ -48,8 +48,8 @@ LC_ALL=C; export LC_ALL ovn_install_dir () { DIR="$1" INSTALL_MODE="${2:-755}" - INSTALL_USER="root" - INSTALL_GROUP="root" + INSTALL_USER="$(id -un)" + INSTALL_GROUP="$(id -gn)" [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" [ "${OVN_USER##*:}" != "" ] && INSTALL_GROUP="${OVN_USER##*:}"
The ovn-ctl utility was assuming that the user/group is always root, when not specified otherwise by the --ovn-user/--ovn-group options. This has the consequence of trying to change permissions of OVN directories to root:root even though the script might be run as completely different user. Take the current user and group instead of the hardcoded root. At the same time remove the ovs-user option as it was not used for anything and might be confusing. Reported-at: https://issues.redhat.com/browse/FDP-245 Signed-off-by: Ales Musil <amusil@redhat.com> --- utilities/ovn-ctl | 5 ++--- utilities/ovn-ctl.8.xml | 1 - utilities/ovn-lib.in | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-)