ovn-ctl and ovn-lib currently chown the OVN log directory recursively when services start. That can undo ownership set by distribution tooling such as tmpfiles.d and logrotate. In particular, Debian and Ubuntu need ovn-controller.log to remain root:adm so rsyslog can read it, but a later OVN service restart may change it back to the OVN daemon user/group.
Add an OVN tmpfiles.d template for the log directory and ovn-controller.log, with configurable log user, log group and tmpfiles.d installation directory. Debian configures the log owner as root:adm, while Fedora/RHEL keeps openvswitch:openvswitch. Stop recursively changing the OVN log directory ownership from ovn-ctl and ovn-lib at service startup. Runtime, database and configuration paths are still chowned according to --ovn-user, preserving the existing privilege-drop behavior for non-log state. Reported-at: https://github.com/ovn-org/ovn/issues/310 Signed-off-by: Zhang Hua <[email protected]> --- Submitted-at: https://github.com/ovn-org/ovn/pull/311 v2: - Added the trailing dot required by checkpatch to the subject. Testing: - Ran utilities/checkpatch.py -1. - Ran make check TESTSUITEFLAGS="-j$(nproc)". Makefile.am | 4 ++++ configure.ac | 3 +++ debian/ovn-common.install | 1 + debian/rules | 2 +- m4/ovn.m4 | 31 +++++++++++++++++++++++++++++++ rhel/ovn-fedora.spec.in | 3 +++ utilities/automake.mk | 4 ++++ utilities/ovn-ctl | 6 ++---- utilities/ovn-lib.in | 1 - utilities/ovn-tmpfiles.conf.in | 2 ++ 10 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 utilities/ovn-tmpfiles.conf.in diff --git a/Makefile.am b/Makefile.am index 0f2389b25..79929ee6c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -131,6 +131,7 @@ noinst_PROGRAMS = noinst_SCRIPTS = OVSIDL_BUILT = pkgdata_DATA = +tmpfiles_DATA = sbin_SCRIPTS = scripts_SCRIPTS = completion_SCRIPTS = @@ -148,6 +149,7 @@ endif scriptsdir = $(pkgdatadir)/scripts completiondir = $(sysconfdir)/bash_completion.d pkgconfigdir = $(libdir)/pkgconfig +tmpfilesdir = @TMPFILESDIR@ # This ensures that files added to EXTRA_DIST are always distributed, # even if they are inside an Automake if...endif conditional block that is @@ -166,6 +168,8 @@ SUFFIXES += .in sed \ -e 's,[@]PKIDIR[@],$(PKIDIR),g' \ -e 's,[@]LOGDIR[@],$(LOGDIR),g' \ + -e 's,[@]LOGUSER[@],$(LOGUSER),g' \ + -e 's,[@]LOGGROUP[@],$(LOGGROUP),g' \ -e 's,[@]DBDIR[@],$(DBDIR),g' \ -e 's,[@]PYTHON3[@],$(PYTHON3),g' \ -e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \ diff --git a/configure.ac b/configure.ac index cfa4cc386..d19e26612 100644 --- a/configure.ac +++ b/configure.ac @@ -88,6 +88,9 @@ OVS_CHECK_NETLINK OVS_CHECK_LINUX_NETLINK OVS_CHECK_OPENSSL OVN_CHECK_LOGDIR +OVN_CHECK_LOGUSER +OVN_CHECK_LOGGROUP +OVN_CHECK_TMPFILESDIR OVN_CHECK_PYTHON3 OVN_CHECK_FLAKE8 OVN_CHECK_SPHINX diff --git a/debian/ovn-common.install b/debian/ovn-common.install index fc48f07e4..b890c6797 100644 --- a/debian/ovn-common.install +++ b/debian/ovn-common.install @@ -12,4 +12,5 @@ usr/share/ovn/scripts/ovn-lib usr/share/ovn/scripts/ovn-bugtool-nbctl-show usr/share/ovn/scripts/ovn-bugtool-sbctl-lflow-list usr/share/ovn/scripts/ovn-bugtool-sbctl-show +usr/lib/tmpfiles.d/ovn-tmpfiles.conf usr/lib/*/libovn*.so.* diff --git a/debian/rules b/debian/rules index b25a0b48e..0d5da5b26 100755 --- a/debian/rules +++ b/debian/rules @@ -30,7 +30,7 @@ override_dh_autoreconf: dh_autoreconf $(DH_AS_NEEDED) override_dh_auto_configure: - dh_auto_configure -- --enable-ssl --enable-shared --with-ovs-source=${OVSDIR} $(EXTRA_CONFIGURE_OPTS) + dh_auto_configure -- --enable-ssl --enable-shared --with-ovs-source=${OVSDIR} --with-log-user=root --with-log-group=adm $(EXTRA_CONFIGURE_OPTS) override_dh_auto_test: ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS))) diff --git a/m4/ovn.m4 b/m4/ovn.m4 index 22ad1a27f..f022a0152 100644 --- a/m4/ovn.m4 +++ b/m4/ovn.m4 @@ -127,6 +127,37 @@ AC_DEFUN([OVN_CHECK_LOGDIR], [LOGDIR='${localstatedir}/log/${PACKAGE}']) AC_SUBST([LOGDIR])]) +dnl Checks for the user that should own log files. +AC_DEFUN([OVN_CHECK_LOGUSER], + [AC_ARG_WITH( + [log-user], + AS_HELP_STRING([--with-log-user=USER], + [user used for log files [[root]]]), + [LOGUSER=$withval], + [LOGUSER=root]) + AC_SUBST([LOGUSER])]) + +dnl Checks for the group that should own log files. +AC_DEFUN([OVN_CHECK_LOGGROUP], + [AC_ARG_WITH( + [log-group], + AS_HELP_STRING([--with-log-group=GROUP], + [group used for log files [[root]]]), + [LOGGROUP=$withval], + [LOGGROUP=root]) + AC_SUBST([LOGGROUP])]) + +dnl Checks for the directory in which to install tmpfiles.d configuration. +AC_DEFUN([OVN_CHECK_TMPFILESDIR], + [AC_ARG_WITH( + [tmpfilesdir], + AS_HELP_STRING([--with-tmpfilesdir=DIR], + [directory used for tmpfiles.d configuration + [[PREFIX/lib/tmpfiles.d]]]), + [TMPFILESDIR=$withval], + [TMPFILESDIR='${prefix}/lib/tmpfiles.d']) + AC_SUBST([TMPFILESDIR])]) + dnl Checks for the directory in which to store the OVN database. AC_DEFUN([OVN_CHECK_DBDIR], [AC_ARG_WITH( diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in index 131b3eaab..1fdaad706 100644 --- a/rhel/ovn-fedora.spec.in +++ b/rhel/ovn-fedora.spec.in @@ -159,6 +159,8 @@ cd - --with-ovs-source=$PWD/openvswitch-%{ovsver} \ %if %{with libcapng} --enable-libcapng \ + --with-log-user=openvswitch \ + --with-log-group=openvswitch \ %else --disable-libcapng \ %endif @@ -531,6 +533,7 @@ fi %{_mandir}/man8/ovn-debug.8* %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers %config(noreplace) %{_sysconfdir}/logrotate.d/ovn +%{_tmpfilesdir}/ovn-tmpfiles.conf %{_unitdir}/[email protected] %files docker diff --git a/utilities/automake.mk b/utilities/automake.mk index b620038d0..c22b5d8c3 100644 --- a/utilities/automake.mk +++ b/utilities/automake.mk @@ -26,6 +26,7 @@ EXTRA_DIST += \ utilities/ovn-ctl \ utilities/ovn-lib.in \ utilities/ovn-ctl.8.xml \ + utilities/ovn-tmpfiles.conf.in \ utilities/ovn-docker-overlay-driver.in \ utilities/ovn-docker-underlay-driver.in \ utilities/ovn-nbctl.8.xml \ @@ -48,6 +49,7 @@ EXTRA_DIST += \ CLEANFILES += \ utilities/ovn-ctl.8 \ utilities/ovn-lib \ + utilities/ovn-tmpfiles.conf \ utilities/ovn-docker-overlay-driver \ utilities/ovn-docker-underlay-driver \ utilities/ovn-nbctl.8 \ @@ -66,7 +68,9 @@ CLEANFILES += \ EXTRA_DIST += utilities/ovn-sim.in noinst_SCRIPTS += utilities/ovn-sim +tmpfiles_DATA += utilities/ovn-tmpfiles.conf utilities/ovn-lib: $(top_builddir)/config.status +utilities/ovn-tmpfiles.conf: $(top_builddir)/config.status # ovn-nbctl bin_PROGRAMS += utilities/ovn-nbctl diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl index 3b62ca9b7..58effd5e5 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -276,9 +276,8 @@ $cluster_remote_port upgrade_db "$file" "$schema" fi - # 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. + # Database files may be created as root before ovsdb-server drops + # privileges, so keep ownership aligned with OVN_USER when configured. INSTALL_USER="$(id -un)" INSTALL_GROUP="$(id -gn)" [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" @@ -286,7 +285,6 @@ $cluster_remote_port chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_dbdir chown -R $INSTALL_USER:$INSTALL_GROUP $OVN_RUNDIR - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_etcdir set ovsdb-server diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in index 5a0766816..0b19149da 100644 --- a/utilities/ovn-lib.in +++ b/utilities/ovn-lib.in @@ -133,7 +133,6 @@ start_ovn_daemon () { set "$@" --detach test X"$MONITOR" = Xno || set "$@" --monitor - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_rundir start_wrapped_daemon "$wrapper" $daemon "$priority" "$@" diff --git a/utilities/ovn-tmpfiles.conf.in b/utilities/ovn-tmpfiles.conf.in new file mode 100644 index 000000000..d37391f88 --- /dev/null +++ b/utilities/ovn-tmpfiles.conf.in @@ -0,0 +1,2 @@ +d @LOGDIR@ 0750 @LOGUSER@ @LOGGROUP@ - +f @LOGDIR@/ovn-controller.log 0640 @LOGUSER@ @LOGGROUP@ - \ No newline at end of file -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
