On Thu, Jun 08, 2017 at 03:46:24PM -0400, Aaron Conole wrote: > Flavio Leitner <[email protected]> writes: > > > On Sat, Jun 03, 2017 at 11:10:00AM -0400, Aaron Conole wrote: > >> After this commit, the fedora RPM will create the openvswitch user, from > >> the > >> non-static pool, for use as an Open vSwitch daemon user. This only happens > >> on install - not upgrade. This will be the default user:group > >> combination for the openvswitch daemons. > >> > >> To do this in a way that doesn't impact existing installations, the > >> /etc/openvswitch directory will be created during the installation, > >> rather than being provided as part of the rpm. > > > > In the previous patch you add the user configuration to the sysconfig > > file and here it adds the same info again to another file which the > > user might change, getting us back to state today but now with 2 > > files. > > Sortof. The idea is the openvswitch-pre file is like the default.conf > you detail below. I could remove the comment from the openvswitch > config file, if you think it's causing confusion. More follows.
/etc/sysconfig is meant to be changed by the user and the name implies that it is executed before something, which something? > > Perhaps we could adopt another approach that we have a default > > recommended configuration and then a file where the user can > > customize it? > > > > In this case we would create /etc/openvswitch/default.conf. > > > > If the user wants to change something, it replaces the variable in > > /etc/sysconfig/openvswitch as it works today. Since default.conf is > > owned by the system, we can assume it's not edited by the user. > > That's the assumption on openvswitch-pre, but I might have gotten the > %files section wrong for it (meaning, I assume a user will not edit > it). I can call it /etc/openvswitch/default.conf if you want; that's > probably better, actually - so v2 I will do that, regardless what form > it takes. OK, I assume that since it's /etc/sysconfig, the user could change. > > Then we ship /etc/openvswitch/default.conf with > > OVS_USER_ID="openvswitch:openvswitch" by default, so new installations > > will have the file state correct in the rpmdb. > > Is that not the case for the file I added? I thought it was okay, but I > might misunderstand the way the file flags work. Sort of, because we have the config in three places now: One could edit the systemd services to change the line Environment="OVS_USER_ID=root:root", or the /etc/sysconfig/openvswitch or /etc/sysconfig/openvswitch-pre > > The %post appends to the end of /etc/sysconfig/openvswitch the > > variable replacing the default user id to root. > > Here are the issues I can think of: > > - The rpmdb now thinks that the openvswitch file has been modified by > the user (so the %config part will be flagged even though the user > did nothing). Yup, then on the future upgrades, the config file should not be replaced and that would happen with /etc/sysconfig/openvswitch-pre, right? > - The %post on upgrade will have to detect if the user has an > OVS_USER_ID specified, and ignore it appropriately Right. > - The file permissions have to change again (ie: we change them to in > the %files section to be owned openvswitch:openvswitch, then have to > change them in the %post for upgrade to be the correct > user/group... but only if we know we are editing the ovs_user_id, I > think). You mean /etc/openvswitch or another files? > I'm willing to try and rework this series to go in that direction, but I > prefer doing it this way (setting the uid on new installs in %post) > because it's an easy binary decision. Is it new? Okay, there's no way > we can break something on the user, so change things around. On the > other hand, working from the opposite, we have to detect properly that > the user's system won't be broken by the change. Maybe there's a good > enough regular expression / other test we can design. I'll take any > suggestion :) Wouldn't the rpm change the /etc/openvswitch permissions back to default when the user upgrades from a new installation? > > Then on new installations we have /etc/openvswitch/default.conf with > > the recommended system options, nothing on /etc/sysconfig/openvswitch, > > no need to add root userid to the services. > > > > On upgrades, there will be the default.conf recommending to run as > > user, and /etc/sysconfig/openvswitch changing to root which the > > admin can comment out to move on, and system services are ok. > > > > What do you think? I am sure I missed something. > > In one sense, it's probably more of a six-of-one and > half-dozen-of-the-other thing. Whichever we choose, the requirements, > in my mind, are simple: > > 1. Don't break existing users who are upgrading. > 2. Provide new installs with non-root users, because it's a good > security practice. > > Did I make sense? We share the same requirements. fbl > > > fbl > > > > > >> > >> Signed-off-by: Aaron Conole <[email protected]> > >> --- > >> rhel/openvswitch-fedora.spec.in | 15 ++++++++++++++- > >> rhel/usr_lib_systemd_system_ovs-vswitchd.service | 1 + > >> rhel/usr_lib_systemd_system_ovsdb-server.service | 2 ++ > >> 3 files changed, 17 insertions(+), 1 deletion(-) > >> > >> diff --git a/rhel/openvswitch-fedora.spec.in > >> b/rhel/openvswitch-fedora.spec.in > >> index fe6f15f..f4da735 100644 > >> --- a/rhel/openvswitch-fedora.spec.in > >> +++ b/rhel/openvswitch-fedora.spec.in > >> @@ -92,6 +92,8 @@ Requires: openssl hostname iproute module-init-tools > >> #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3 > >> #Requires: kernel >= 3.15.0-0 > >> > >> +Requires(post): /usr/bin/getent > >> +Requires(post): /usr/sbin/useradd > >> Requires(post): systemd-units > >> Requires(preun): systemd-units > >> Requires(postun): systemd-units > >> @@ -354,6 +356,16 @@ rm -rf $RPM_BUILD_ROOT > >> %endif > >> > >> %post > >> +if [ $1 -eq 1 ]; then > >> + getent passwd openvswitch >/dev/null || \ > >> + useradd -r -d / -s /sbin/nologin -c "Open vSwitch Daemons" > >> openvswitch > >> + echo "OVS_USER_ID=openvswitch:openvswitch" > \ > >> + %{_sysconfdir}/sysconfig/openvswitch-pre > >> + > >> + # In the case of upgrade, this is not needed. > >> + install -d -m 0755 -o openvswitch -g openvswitch /etc/openvswitch > >> +fi > >> + > >> %if 0%{?systemd_post:1} > >> %systemd_post %{name}.service > >> %else > >> @@ -480,7 +492,8 @@ fi > >> %defattr(-,root,root) > >> %{_sysconfdir}/bash_completion.d/ovs-appctl-bashcomp.bash > >> %{_sysconfdir}/bash_completion.d/ovs-vsctl-bashcomp.bash > >> -%dir %{_sysconfdir}/openvswitch > >> +%ghost %{_sysconfdir}/openvswitch > >> +%ghost %{_sysconfdir}/sysconfig/openvswitch-pre > >> %config %ghost %{_sysconfdir}/openvswitch/conf.db > >> %ghost %{_sysconfdir}/openvswitch/.conf.db.~lock~ > >> %config %ghost %{_sysconfdir}/openvswitch/system-id.conf > >> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service > >> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service > >> index d63bf4d..0434d20 100644 > >> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service > >> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service > >> @@ -11,6 +11,7 @@ PartOf=openvswitch.service > >> Type=forking > >> Restart=on-failure > >> Environment="OVS_USER_ID=root:root" > >> +EnvironmentFile=-/etc/sysconfig/openvswitch-pre > >> EnvironmentFile=-/etc/sysconfig/openvswitch > >> ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ > >> --no-ovsdb-server --no-monitor --system-id=random \ > >> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service > >> b/rhel/usr_lib_systemd_system_ovsdb-server.service > >> index 67b50c8..8354087 100644 > >> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service > >> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service > >> @@ -9,7 +9,9 @@ PartOf=openvswitch.service > >> Type=forking > >> Restart=on-failure > >> Environment="OVS_USER_ID=root:root" > >> +EnvironmentFile=-/etc/sysconfig/openvswitch-pre > >> EnvironmentFile=-/etc/sysconfig/openvswitch > >> +ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch > >> ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \ > >> --no-ovs-vswitchd --no-monitor --system-id=random \ > >> --ovs-user=${OVS_USER_ID} \ > >> -- > >> 2.9.4 > >> > >> _______________________________________________ > >> dev mailing list > >> [email protected] > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev -- Flavio _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
