Should I test for the root user when the script starts and refuse to
run the script otherwise ?

On Thu, Feb 06, 2014 at 10:50:29AM +0100, Michele Tartara wrote:
> On Wed, Feb 5, 2014 at 10:10 AM, Jose A. Lopes <[email protected]> wrote:
> > The script 'tools/kvm-ifup-os' configures TAP network interfaces for
> > for instances, routing, DHCP server, etc.  Note that this script only
> > configures TAP network interfaces that are used by the instance
> > communication, that is, network interfaces named according to the
> > pattern 'gnt.com.%d', where '%d' is a number unique within a given
> > node.
> >
> > Signed-off-by: Jose A. Lopes <[email protected]>
> > ---
> >  Makefile.am          |  11 ++
> >  tools/kvm-ifup-os.in | 296 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/kvm-ifup.in    |  21 +++-
> >  tools/net-common.in  |  17 ++-
> >  4 files changed, 335 insertions(+), 10 deletions(-)
> >  create mode 100644 tools/kvm-ifup-os.in
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index e5d0594..aa631da 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -95,6 +95,7 @@ toolsdir = $(pkglibdir)/tools
> >  iallocatorsdir = $(pkglibdir)/iallocators
> >  pytoolsdir = $(pkgpythondir)/tools
> >  docdir = $(versiondir)$(datadir)/doc/$(PACKAGE)
> > +ifupdir = $(sysconfdir)/ganeti
> >
> >  SYMLINK_TARGET_DIRS = \
> >         $(sysconfdir)/ganeti \
> > @@ -258,6 +259,7 @@ CLEANFILES = \
> >         $(man_MANS) \
> >         $(manhtml) \
> >         tools/kvm-ifup \
> > +       tools/kvm-ifup-os \
> >         tools/vif-ganeti \
> >         tools/net-common \
> >         tools/users-setup \
> > @@ -320,6 +322,9 @@ BUILT_EXAMPLES = \
> >         doc/examples/gnt-config-backup \
> >         doc/examples/hooks/ipsec
> >
> > +dist_ifup_SCRIPTS = \
> > +       tools/kvm-ifup-os
> > +
> >  nodist_pkgpython_PYTHON = \
> >         $(BUILT_PYTHON_SOURCES)
> >
> > @@ -1131,6 +1136,7 @@ pkglib_python_basenames = \
> >  myexeclib_SCRIPTS = \
> >         daemons/daemon-util \
> >         tools/kvm-ifup \
> > +       tools/kvm-ifup-os \
> >         tools/vif-ganeti \
> >         tools/net-common \
> >         $(HS_MYEXECLIB_PROGS)
> > @@ -1169,6 +1175,7 @@ EXTRA_DIST = \
> >         devel/upload \
> >         devel/webserver \
> >         tools/kvm-ifup.in \
> > +       tools/kvm-ifup-os.in \
> >         tools/vif-ganeti.in \
> >         tools/net-common.in \
> >         tools/vcluster-setup.in \
> > @@ -1653,6 +1660,10 @@ tools/kvm-ifup: tools/kvm-ifup.in $(REPLACE_VARS_SED)
> >         sed -f $(REPLACE_VARS_SED) < $< > $@
> >         chmod +x $@
> >
> > +tools/kvm-ifup-os: tools/kvm-ifup-os.in $(REPLACE_VARS_SED)
> > +       sed -f $(REPLACE_VARS_SED) < $< > $@
> > +       chmod +x $@
> > +
> >  tools/vif-ganeti: tools/vif-ganeti.in $(REPLACE_VARS_SED)
> >         sed -f $(REPLACE_VARS_SED) < $< > $@
> >         chmod +x $@
> > diff --git a/tools/kvm-ifup-os.in b/tools/kvm-ifup-os.in
> > new file mode 100644
> > index 0000000..b696da6
> > --- /dev/null
> > +++ b/tools/kvm-ifup-os.in
> > @@ -0,0 +1,296 @@
> > +#!/bin/bash
> > +#
> > +
> > +# Copyright (C) 2014 Google Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, but
> > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +# General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +# 02110-1301, USA.
> > +
> > +# This script is a hook called to configure new TAP network interfaces
> > +# used for instance communication, and it should be called whenever a
> > +# new instance is started.
> > +#
> > +# This script configures the new interface but it also performs
> > +# maintenance on the network interfaces that have been configured
> > +# before, by checking whether those TAP interfaces still exist, etc.
> > +#
> > +# This script also controls the DHCP server that leases IP address for
> > +# instances, i.e., the NICs inside the instances, not the TAP
> > +# interfaces.  The DHCP server is started and restarted (or reHUPed)
> > +# as necessary and always with up-to-date configuration files.
> > +#
> > +# This script expects the following environment variables
> > +#
> > +#   INTERFACE: network interface name to be configured
> > +#   MODE: networking mode for 'INTERFACE' (must be 'routed')
> > +#   MAC: MAC address for 'INTERFACE'
> > +#   IP: IP address for 'INTERFACE'
> > +
> > +source @PKGLIBDIR@/net-common
> > +
> > +readonly NETMASK=255.255.255.255
> > +readonly DNSMASQ_CONF=/var/run/ganeti/dnsmasq.conf
> > +readonly DNSMASQ_HOSTS=/var/run/ganeti/dnsmasq.hosts
> > +readonly DNSMASQ_PID=/var/run/ganeti/dnsmasq.pid
> > +
> > +# join intercalates a sequence of arguments using the given separator
> > +function join {
> > +  local IFS="$1"
> > +  shift
> > +  echo "$*"
> > +}
> > +
> > +# restart_dnsmasq restarts the DHCP server dnsmasq with the (possibly
> > +# up-to-date) configuration file.
> > +#
> > +# If all instances have been terminated, which means there are no more
> > +# TAP network interfaces to monitor or IP addresses to lease, the DHCP
> > +# server is terminated through 'SIGTERM'.
> > +#
> > +# If there are still instances running, then:
> > +#  - if the DHCP server is running, a 'SIGHUP' will be sent to the
> > +#    dnsmasq process which will cause the configuration file to be
> > +#    re-read, while keeping the process running
> > +#  - if the DHCP server is not running, it will be started, and the
> > +#    configuration file will be passed it
> > +function restart_dnsmasq {
> > +  SIGNAL=
> > +
> > +  if [ -z "$ALIVE_INTERFACES" -o -z "$ALIVE_LEASES" ]
> > +  then
> > +      SIGNAL=TERM
> > +  else
> > +      SIGNAL=HUP
> > +  fi
> > +
> > +  RUNNING=
> > +
> > +  if [ -f "$DNSMASQ_PID" ]
> > +  then
> > +      PID=$(cat $DNSMASQ_PID)
> > +      if [ -n "$PID" ] && ps -p "$PID"
> > +      then
> > +         RUNNING=yes
> > +      fi
> > +  fi
> > +
> > +  KILLED=
> > +
> > +  if [ "$RUNNING" = yes ]
> > +  then
> > +      kill -$SIGNAL $PID
> > +
> > +      if [ "$SIGNAL" = TERM ]
> > +      then
> > +         KILLED=yes
> > +      fi
> > +  fi
> > +
> > +  if [ "$KILLED" = yes ]
> > +  then
> > +      rm -f $DNSMASQ_PID
> > +  fi
> > +
> > +  if [ "$RUNNING" != yes -o "$KILLED" == yes ]
> > +  then
> > +      if [ -n "$ALIVE_INTERFACES" -a -n "$ALIVE_LEASES" ]
> > +      then
> > +         dnsmasq -C $DNSMASQ_CONF
> > +      fi
> > +  fi
> > +
> > +  return 0
> > +}
> > +
> > +# Check that environment variable 'INTERFACE' exists.
> > +#
> > +# This environment variable holds the TAP network interface that
> > +# should be configured by this script.  Ganeti always passes it,
> > +# but... :)
> > +if [ -z "$INTERFACE" ]
> > +then
> > +  echo kvm-vif-bridge: Failed to configure communication mechanism \
> > +      interface because the \'INTERFACE\' environment variable was \
> > +      not specified to the \'kvm-vif-bridge\' script
> > +  exit 1
> > +fi
> > +
> > +# Check that environment variable 'MODE' exists.
> > +#
> > +# See comment about environment variable 'INTERFACE'.
> > +if [ -z "$MODE" ]
> > +then
> > +  echo kvm-vif-bridge: Failed to configure communication mechanism \
> > +      interface because the \'MODE\' environment variable was \
> > +      not specified to the \'kvm-vif-bridge\' script
> > +  exit 1
> > +fi
> > +
> > +# Check whether the interface being configured has instance
> > +# communication enabled, otherwise exit this script.
> > +if ! is_instance_communication_tap; then exit 0; fi
> > +
> > +# Check that environment variable 'MAC' exists.
> > +#
> > +# See comment about environment variable 'INTERFACE'.
> > +if [ -z "$MAC" ]
> > +then
> > +  echo kvm-vif-bridge: Failed to configure communication mechanism \
> > +      interface because the \'MAC\' environment variable was \
> > +      not specified to the \'kvm-vif-bridge\' script
> > +  exit 1
> > +fi
> > +
> > +# Check that environment variable 'IP' exists.
> > +#
> > +# See comment about environment variable 'INTERFACE'.
> > +if [ -z "$IP" ]
> > +then
> > +  echo kvm-vif-bridge: Failed to configure communication mechanism \
> > +      interface because the \'IP\' environment variable was \
> > +      not specified to the \'kvm-vif-bridge\' script
> > +  exit 1
> > +fi
> > +
> > +# Configure the TAP interface
> > +#
> > +# Ganeti defers the configuration of instance network interfaces to
> > +# hooks, therefore, we must configure the interface's network address,
> > +# netmask, and IP address.
> > +#
> > +# The TAP network interface, which is used by the instance
> > +# communication, is part of the network 169.254.0.0/16 and has the IP
> > +# 169.254.169.254.  Because all network interfaces used in the
> > +# instance communication have the same IP, the routing table must also
> > +# be configured, and that is done at a later step.
> > +#
> > +# Note the interface must be marked as up before configuring the
> > +# routing table and before starting/restarting the DHCP server.
> > +#
> > +# Note also that we don't have to check whether the interface is
> > +# already configured because reconfiguring the interface with the same
> > +# parameters does not produce an error.
> > +ifconfig $INTERFACE 169.254.169.254 netmask $NETMASK up
> > +
> > +# Configure the routing table
> > +#
> > +# Given that all TAP network interfaces in the instance communication
> > +# have the same IP address, the routing table must be configured in
> > +# order to properly route traffic from the host to the guests.
> > +#
> > +# Note that we must first check if a duplicate routing rule has
> > +# already been added to the routing table, as this operation will fail
> > +# if we try to add a routing rule that already exists.
> > +ACTIVE_IP=$(ip route | grep "dev $INTERFACE" | awk '{ print $1 }')
> > +
> > +if [ -z "$ACTIVE_IP" -o "$ACTIVE_IP" != "$IP" ]
> > +then
> > +  route add -host $IP dev $INTERFACE
> > +fi
> > +
> > +# Ensure the DHCP server configuration files exist
> > +touch $DNSMASQ_CONF
> > +chmod 0644 $DNSMASQ_CONF
> > +
> > +touch $DNSMASQ_HOSTS
> > +chmod 0644 $DNSMASQ_HOSTS
> > +
> > +# Determine dnsmasq operational mode.
> > +#
> > +# The DHCP server dnsmasq can run in different modes.  In this version
> > +# of the script, only the mode 'bind-dynamic' is supported.  Please
> > +# refer to the dnsmasq FAQ for a detailed of each mode.
> > +#
> > +# Note that dnsmasq might already be running, therefore, we don't need
> > +# to determine which modes are supported by this DHCP server.
> > +# Instead, we just read the current mode from the configuration file.
> > +DNSMASQ_MODE=$(head -n 1 $DNSMASQ_CONF)
> > +
> > +if [ -z "$DNSMASQ_MODE" ]
> > +then
> > +  BIND_DYNAMIC=$(dnsmasq --help | grep -e --bind-dynamic)
> > +
> > +  if [ -z "$BIND_DYNAMIC" ]
> > +  then
> > +    echo kvm-vif-bridge: dnsmasq mode \"bind-dynamic\" is not supported
> > +    exit 1
> > +  fi
> > +
> > +  DNSMASQ_MODE=bind-dynamic
> > +fi
> > +
> > +# Determine the interfaces that should go in the configuration file.
> > +#
> > +# The TAP network interfaces used by the instance communication are
> > +# named after the following pattern
> > +#
> > +#  gnt.com.%d
> > +#
> > +# where '%d' is a unique number within the host.  Fortunately, dnsmasq
> > +# supports binding to specific network interfaces via a pattern.
> > +ALIVE_INTERFACES=${GANETI_TAP}.*
> > +
> > +# Determine which of the leases are not duplicated and should go in
> > +# the new configuration file for the DHCP server.
> > +#
> > +# Given that instances come and go, it is possible that we offer more
> > +# leases that necessary and, worse, that we have duplicate leases,
> > +# that is, the same IP address for the same/different MAC addresses.
> > +# Duplicate leases must be eliminated before being written to the
> > +# configuration file.
> > +CONF_LEASES=$(cat $DNSMASQ_HOSTS)
> > +CONF_LEASES=$(join $'\n' $CONF_LEASES | sort -u)
> > +
> > +ALIVE_LEASES=( $MAC,$IP )
> > +
> > +for i in $CONF_LEASES
> > +do
> > +  LEASE_MAC=$(echo $i | cut -d "," -f 1)
> > +  LEASE_IP=$(echo $i | cut -d "," -f 2)
> > +  if [ "$LEASE_MAC" != "$MAC" -a "$LEASE_IP" != "$IP" ]
> > +  then
> > +      ALIVE_LEASES=( ${ALIVE_LEASES[@]} $i )
> > +  fi
> > +done
> > +
> > +ALIVE_LEASES=$(echo ${ALIVE_LEASES[@]} | sort -u)
> > +
> > +# Update dnsmasq configuration.
> > +#
> > +# Write the parameters we have collected before into the new dnsmasq
> > +# configuration file.  Also, write the new leases into the new dnsmasq
> > +# hosts file.  Finally, restart dnsmasq with the new configuration
> > +# files.
> > +cat > $DNSMASQ_CONF <<EOF
> > +$DNSMASQ_MODE
> > +dhcp-authoritative
> > +dhcp-hostsfile=$DNSMASQ_HOSTS
> > +dhcp-range=169.254.0.0,static,255.255.0.0
> > +except-interface=eth*
> > +except-interface=lo
> > +leasefile-ro
> > +no-hosts
> > +no-ping
> > +no-resolv
> > +pid-file=$DNSMASQ_PID
> > +port=0
> > +strict-order
> > +EOF
> > +for i in $ALIVE_INTERFACES; do echo interface=$i >> $DNSMASQ_CONF; done
> > +
> > +echo -n > $DNSMASQ_HOSTS
> > +for i in $ALIVE_LEASES; do echo $i >> $DNSMASQ_HOSTS; done
> > +
> > +restart_dnsmasq
> > diff --git a/tools/kvm-ifup.in b/tools/kvm-ifup.in
> > index 5a53cee..f7ce37d 100644
> > --- a/tools/kvm-ifup.in
> > +++ b/tools/kvm-ifup.in
> > @@ -1,7 +1,7 @@
> >  #!/bin/bash
> >  #
> >
> > -# Copyright (C) 2011, 2012 Google Inc.
> > +# Copyright (C) 2011, 2012, 2014 Google Inc.
> >  #
> >  # This program is free software; you can redistribute it and/or modify
> >  # it under the terms of the GNU General Public License as published by
> > @@ -20,12 +20,23 @@
> >
> >  source @PKGLIBDIR@/net-common
> >
> > +check
> > +
> > +# Execute the user-supplied network script, if applicable
> I'd change this into something like "Execute the script for setting up
> the communication with the instance os, if available"
> No reference to "user-supplied" as it is user-modifiable but supplied by us.
> 
> > +#
> > +# This additional hook is placed here for backwards-compatibility with
> > +# the other hook below.
> 
> Without the whole discussion we had offline, this comment about
> backwards-compatibility does not make much sense, so I'd just remove
> it.
> 
> > +if is_instance_communication_tap && [ -x "$CONF_DIR/kvm-ifup-os" ]; then
> > +  . $CONF_DIR/kvm-ifup-os
> > +fi
> > +
> >  # Execute the user-supplied network script, if applicable
> >  if [ -x "$CONF_DIR/kvm-vif-bridge" ]; then
> >    exec $CONF_DIR/kvm-vif-bridge
> >  fi
> >
> > -check
> > -setup_bridge
> > -setup_ovs
> > -setup_route
> > +if ! is_instance_communication_tap; then
> > +  setup_bridge
> > +  setup_ovs
> > +  setup_route
> > +fi
> > diff --git a/tools/net-common.in b/tools/net-common.in
> > index 7305875..c0d5bdf 100644
> > --- a/tools/net-common.in
> > +++ b/tools/net-common.in
> > @@ -20,8 +20,9 @@
> >
> >  @SHELL_ENV_INIT@
> >
> > -function check {
> > +readonly GANETI_TAP="gnt.com"
> >
> > +function check {
> >    if [ -z "$INTERFACE" ]; then
> >      echo "No network interface specified"
> >      exit 1
> > @@ -31,22 +32,29 @@ function check {
> >      echo "MODE not specified"
> >      exit 1
> >    fi
> > +}
> >
> > +function is_instance_communication_tap {
> > +  COMMUNICATION=$(echo "$INTERFACE" | cut -d "." -f 1-2)
> > +
> > +  if [ "$MODE" = "routed" -a "$COMMUNICATION" = "$GANETI_TAP" ]
> > +  then
> > +    return 0
> > +  else
> > +    return 1
> > +  fi
> >  }
> >
> >  function fix_mac {
> > -
> >    # Fix the autogenerated MAC to have the first octet set to "fe"
> >    # to discourage the bridge from using the TAP dev's MAC
> >    FIXED_MAC=$(ip link show $INTERFACE | \
> >      awk '{if ($1 == "link/ether") printf("fe%s",substr($2,3,15))}')
> >    # in case of a vif (xen_netback device) this action is not allowed
> >    ip link set $INTERFACE address $FIXED_MAC || true
> > -
> >  }
> >
> >  function setup_bridge {
> > -
> >    if [ "$MODE" = "bridged" ]; then
> >      fix_mac
> >      ip link set $INTERFACE up
> > @@ -55,7 +63,6 @@ function setup_bridge {
> >      # Connect the interface to the bridge
> >      brctl addif $LINK $INTERFACE
> >    fi
> > -
> >  }
> >
> >  function setup_ovs {
> > --
> > 1.9.0.rc1.175.g0b1dcb5
> >
> 
> Rest LGTM, no need to resend.
> 
> Thanks,
> Michele
> 
> -- 
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
> 
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores

-- 
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to