[Ovirt-devel] [PATCH node] Updated the configuration processing to remove the need for scripts.

[Jim Meyering]

"Darryl L. Pierce" <dpierce at redhat.com> wrote:
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  scripts/ovirt-early          |   12 +++-----
>  scripts/ovirt-process-config |   59 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 8 deletions(-)
>  create mode 100755 scripts/ovirt-process-config
>
> diff --git a/scripts/ovirt-early b/scripts/ovirt-early
> index 4723426..e5f1e9b 100755
> --- a/scripts/ovirt-early
> +++ b/scripts/ovirt-early
> @@ -12,6 +12,8 @@
>
>  # size of the oVirt partition in megabytes
>  OVIRT_SIZE=64
> +BONDING_MODCONF_FILE=/etc/modprobe.d/bonding
> +AUGTOOL_CONFIG=/var/tmp/augtool-config
>
>  get_mac_addresses() {
>      macs=$(ifconfig | awk '/HWaddr/ { print $5"="$1 }' \
> @@ -43,16 +45,10 @@ configure_from_network() {
>                        "http://$SRV_HOST:$SRV_PORT/ovirt/managed_node/config?host=$(hostname)&macs=$macs"
>                      if [ $? -eq 0 ]; then
>                          echo "Remote configuration bundle retrieved to $cfgdb"
> -                        bash $cfgdb
> -                        if [ -f /var/tmp/pre-config-script ]; then
> -                            echo "Loading kernel modules"
> -                            bash /var/tmp/pre-config-script \
> -                                && echo "Kernel modules loaded" \
> -                                || echo "Failed loading kernel modules"
> -                        fi
> +                        ovirt-process-config $cfgdb $BONDING_MODCONF_FILE $AUGTOOL_CONFIG
>                          if [ -f /var/tmp/node-augtool ]; then
>                              echo "Loading remote config"
> -                            augtool < /var/tmp/node-augtool \
> +                            augtool < $AUGTOOL_CONFIG \
>                                  && echo "Remote config applied" \
>                                  || echo "Failed applying remote config"
>                          fi
> diff --git a/scripts/ovirt-process-config b/scripts/ovirt-process-config
> new file mode 100755
> index 0000000..a025489
> --- /dev/null
> +++ b/scripts/ovirt-process-config
> @@ -0,0 +1,59 @@
> +#!/bin/bash
> +
> +CONFIG=$1
> +OVIRT_KERNEL_MODULE_FILE=$2
> +OVIRT_CONFIG_OUTPUT_FILE=$3
> +
> +if [ "$3" == "" ]; then

How about this instead?
(first lines are almost straight out of create-ovirt-appliance,
but for the 3rd, you'd have to add a tiny bit of code to handle -h)

ME=$(basename "$0")
warn() { printf "$ME: $@\n" >&2; }
try_h() { printf "Try \`$ME -h' for more information.\n" >&2; }

case $# in
  0|1|2) warn "too few arguments"; try_help;;
  3) ;;
  *) warn "too man arguments"; try_help;;
esac

With the above doing an exit upon failure, you can un-indent
everything below.

> +    echo "Usage: $(basename $0) [configfile] [output]"

> +else
> +    modconf=$(awk '/bonding=/ {
> +            match($0, "bonding=(.*)", data)
> +            split(data[1], mod, "|")
> +
> +            alias=mod[1]
> +            options=mod[2]
> +
> +            printf("install %s /sbin/modprobe bonding -o %s %s\n",
> +                alias, module, alias, options)

We need to do some sanitization (perl calls this untainting) here.
I.e., we want to reject malicious parameters like "; rm -rf /" or
"& start-stealthy-daemon...".

So, match alias, module, and options for bogus shell meta-characters,
or probably-better, accept only a limited alphabet, maybe just
search for anything matching this /[^[:alnum:]=_ at -]/

> +        }' $CONFIG)

You'll want to diagnose bogosity from within the awk script
and exit nonzero to tell the shell to fail:

           ...
           }' $CONFIG) || exit 1;

> +    echo "$modconf" > $OVIRT_KERNEL_MODULE_FILE
> +
> +    # now build the list of module aliases to load and load them
> +    modules=$(awk '/bonding=/ {
> +            match($0, "bonding=(.*)", data)
> +            split(data[1], mod, "|")
> +
> +            printf("%s ", mod[1])
> +        }' $CONFIG)
> +
> +    /sbin/modprobe $modules

Have you tested this with two or more modules?
Not sure, but you *might* have to do this:

    eval /sbin/modprobe "$modules"

> +    networking=$(awk '/ifcfg=/ {
> +        match($0, "ifcfg=(.*)", data)
> +        split(data[1], ifcfg, "|")
> +
> +        mac = ifcfg[1]
> +        iface = ifcfg[2]
> +
> +        printf("rm /files/etc/sysconfig/network-scripts/ifcfg-%s\n", iface)
> +        printf("set /files/etc/sysconfig/network-scripts/ifcfg-%s/DEVICE %s\n", iface, iface)

If you factor out this long path prefix, the lines will fit in <80, too.

> +
> +        for (line in ifcfg) {
> +            if(line > 2) {
> +                split(ifcfg[line], values, "=")
> +                field = values[1]
> +                value=values[2]
> +
> +                printf("set /files/etc/sysconfig/network-scripts/ifcfg-%s/%s %s\n", iface, field, value)
> +            }
> +        }
> +
> +
> +        printf("save\n")
> +
> +    }' $CONFIG)
> +
> +    echo "$networking" > $OVIRT_CONFIG_OUTPUT_FILE
> +fi
> \ No newline at end of file

Please add one. ^^