Skip to content

Commit

Permalink
Fix shell quoting in cloud-init
Browse files Browse the repository at this point in the history
  • Loading branch information
Ivan Shvedunov committed Sep 3, 2018
1 parent 0eb48cc commit a4a3d1d
Show file tree
Hide file tree
Showing 14 changed files with 162 additions and 30 deletions.
4 changes: 2 additions & 2 deletions glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions glide.yaml
Expand Up @@ -84,3 +84,5 @@ import:
version: 2382e3994d48b1d22acc2c86bcad0a2aff028e32
- package: github.com/aykevl/osfs
version: e4b1ff739ec92f420bca98d909fffb71fc68e29c
- package: github.com/kballard/go-shellquote
version: 95032a82bc518f77982ea72343cc1ade730072f0
Expand Up @@ -10,6 +10,6 @@ user-data:
write_files:
- content: |
#!/bin/sh
if ! mountpoint '/opt'; then mkdir -p '/opt' && mount -t 9p -o trans=virtio opt '/opt'; fi
if ! mountpoint /opt; then mkdir -p /opt && mount -t 9p -o trans=virtio opt /opt; fi
path: /etc/cloud/mount-volumes.sh
permissions: "0755"
Expand Up @@ -5,7 +5,7 @@ user-data-str: |
#!/bin/sh
echo hi
#!/bin/sh
if ! mountpoint '/opt'; then mkdir -p '/opt' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 '/opt'; fi
if ! mountpoint /opt; then mkdir -p /opt && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 /opt; fi
#!/bin/sh
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` '/dev/disk-b'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` /dev/disk-b
Expand Up @@ -5,4 +5,4 @@ user-data-str: |
#!/bin/sh
echo hi
#!/bin/sh
if ! mountpoint '/opt'; then mkdir -p '/opt' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 '/opt'; fi
if ! mountpoint /opt; then mkdir -p /opt && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 /opt; fi
Expand Up @@ -4,12 +4,12 @@ meta-data:
user-data:
runcmd:
- /etc/cloud/symlink-devs.sh
- ln -s '/etc/cloud/symlink-devs.sh' /var/lib/cloud/scripts/per-boot/
- ln -s /etc/cloud/symlink-devs.sh /var/lib/cloud/scripts/per-boot/
write_files:
- content: |
#!/bin/sh
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/` '/dev/disk-a'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` '/dev/disk-b'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/` '/dev/disk-c'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/` /dev/disk-a
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` /dev/disk-b
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/` /dev/disk-c
path: /etc/cloud/symlink-devs.sh
permissions: "0755"
Expand Up @@ -13,16 +13,16 @@ user-data:
- /var/lib/foobar
runcmd:
- /etc/cloud/symlink-devs.sh
- ln -s '/etc/cloud/symlink-devs.sh' /var/lib/cloud/scripts/per-boot/
- ln -s /etc/cloud/symlink-devs.sh /var/lib/cloud/scripts/per-boot/
write_files:
- content: |
#!/bin/sh
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/` '/dev/disk-a'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` '/dev/disk-b'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/` /dev/disk-a
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` /dev/disk-b
path: /etc/cloud/symlink-devs.sh
permissions: "0755"
- content: |
#!/bin/sh
if ! mountpoint '/var/lib/foobar'; then mkdir -p '/var/lib/foobar' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/`2 '/var/lib/foobar'; fi
if ! mountpoint /var/lib/foobar; then mkdir -p /var/lib/foobar && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/`2 /var/lib/foobar; fi
path: /etc/cloud/mount-volumes.sh
permissions: "0755"
Expand Up @@ -12,8 +12,8 @@ user-data:
write_files:
- content: |
#!/bin/sh
if ! mountpoint '/opt'; then mkdir -p '/opt' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 '/opt'; fi
if ! mountpoint '/var/lib/whatever'; then mkdir -p '/var/lib/whatever' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` '/var/lib/whatever'; fi
if ! mountpoint '/var/lib/foobar'; then mkdir -p '/var/lib/foobar' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/`2 '/var/lib/foobar'; fi
if ! mountpoint /opt; then mkdir -p /opt && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 /opt; fi
if ! mountpoint /var/lib/whatever; then mkdir -p /var/lib/whatever && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:2/block/` /var/lib/whatever; fi
if ! mountpoint /var/lib/foobar; then mkdir -p /var/lib/foobar && mount /dev/`ls /sys/devices/pci0000:00/0000:00:03.0/virtio*/host*/target*:0:0/*:0:0:3/block/`2 /var/lib/foobar; fi
path: /etc/cloud/mount-volumes.sh
permissions: "0755"
Expand Up @@ -116,7 +116,7 @@
write_files:
- content: |
#!/bin/sh
if ! mountpoint '/var/lib/whatever'; then mkdir -p '/var/lib/whatever' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 '/var/lib/whatever'; fi
if ! mountpoint /var/lib/whatever; then mkdir -p /var/lib/whatever && mount /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 /var/lib/whatever; fi
path: /etc/cloud/mount-volumes.sh
permissions: "0755"
- name: 'domain conn: virtlet-231700d5-c9a6-container1: Destroy'
Expand Down
Expand Up @@ -98,11 +98,11 @@
#cloud-config
runcmd:
- /etc/cloud/symlink-devs.sh
- ln -s '/etc/cloud/symlink-devs.sh' /var/lib/cloud/scripts/per-boot/
- ln -s /etc/cloud/symlink-devs.sh /var/lib/cloud/scripts/per-boot/
write_files:
- content: |
#!/bin/sh
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/` '/dev/tst'
ln -fs /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/` /dev/tst
path: /etc/cloud/symlink-devs.sh
permissions: "0755"
- name: 'domain conn: virtlet-231700d5-c9a6-container1: Destroy'
Expand Down
48 changes: 39 additions & 9 deletions pkg/libvirttools/cloudinit.go
Expand Up @@ -33,6 +33,7 @@ import (
cnicurrent "github.com/containernetworking/cni/pkg/types/current"
"github.com/ghodss/yaml"
"github.com/golang/glog"
"github.com/kballard/go-shellquote"
libvirtxml "github.com/libvirt/libvirt-go-xml"

"github.com/Mirantis/virtlet/pkg/flexvolume"
Expand All @@ -46,8 +47,28 @@ const (
symlinkFileLocation = "/etc/cloud/symlink-devs.sh"
mountFileLocation = "/etc/cloud/mount-volumes.sh"
mountScriptSubst = "@virtlet-mount-script@"
cloudInitPerBootDir = "/var/lib/cloud/scripts/per-boot"
)

// Note that in the templates below, we don't use shellquote (shq) on
// SysfsPath, because it *must* be expanded by the shell to work
// (it contains '*')

var linkStartupScriptTemplate = utils.NewShellTemplate(
"ln -s {{ shq .StartupScript }} /var/lib/cloud/scripts/per-boot/")
var linkBlockDeviceScriptTemplate = utils.NewShellTemplate(
"ln -fs /dev/`ls {{ .SysfsPath }}` {{ shq .DevicePath }}")
var mountDevScriptTemplate = utils.NewShellTemplate(
"if ! mountpoint {{ shq .ContainerPath }}; then " +
"mkdir -p {{ shq .ContainerPath }} && " +
"mount /dev/`ls {{ .SysfsPath }}`{{ .DevSuffix }} {{ .ContainerPath }}; " +
"fi")
var mountFSScriptTemplate = utils.NewShellTemplate(
"if ! mountpoint {{ shq .ContainerPath }}; then " +
"mkdir -p {{ shq .ContainerPath }} && " +
"mount -t 9p -o trans=virtio {{ shq .MountTag }} {{ shq .ContainerPath }}; " +
"fi")

// CloudInitGenerator provides a common part for Cloud Init ISO drive preparation
// for NoCloud and ConfigDrive volume sources.
type CloudInitGenerator struct {
Expand Down Expand Up @@ -129,8 +150,10 @@ func (g *CloudInitGenerator) generateUserData(volumeMap diskPathMap) ([]byte, er
if symlinkScript != "" {
writeFilesUpdater.addSymlinkScript(symlinkScript)
userData["runcmd"] = utils.Merge(userData["runcmd"], []string{
symlinkFileLocation,
fmt.Sprintf("ln -s '%s' /var/lib/cloud/scripts/per-boot/", symlinkFileLocation),
shellquote.Join(symlinkFileLocation),
linkStartupScriptTemplate.MustExecuteToString(map[string]string{
"StartupScript": symlinkFileLocation,
}),
})
}
if mountScript != "" {
Expand Down Expand Up @@ -488,8 +511,11 @@ func (g *CloudInitGenerator) generateSymlinkScript(volumeMap diskPathMap) string
glog.Warningf("Couldn't determine the path for device %q inside the VM (target path inside the VM: %q)", dev.HostPath, dev.DevicePath)
continue
}
// TODO: shell escaping
symlinkLines = append(symlinkLines, fmt.Sprintf("ln -fs /dev/`ls %s` '%s'", dpath.sysfsPath, dev.DevicePath))
line := linkBlockDeviceScriptTemplate.MustExecuteToString(map[string]string{
"SysfsPath": dpath.sysfsPath,
"DevicePath": dev.DevicePath,
})
symlinkLines = append(symlinkLines, line)
}
return makeScript(symlinkLines)
}
Expand Down Expand Up @@ -587,16 +613,20 @@ func generateFlexvolumeMounts(volumeMap diskPathMap, mount types.VMMount) ([]int
devPath += fmt.Sprintf("-part%d", part)
mountDevSuffix += strconv.Itoa(part)
}
// TODO: do better job at escaping mount.ContainerPath
mountScriptLine := fmt.Sprintf("if ! mountpoint '%s'; then mkdir -p '%s' && mount /dev/`ls %s`%s '%s'; fi",
mount.ContainerPath, mount.ContainerPath, dpath.sysfsPath, mountDevSuffix, mount.ContainerPath)
mountScriptLine := mountDevScriptTemplate.MustExecuteToString(map[string]string{
"ContainerPath": mount.ContainerPath,
"SysfsPath": dpath.sysfsPath,
"DevSuffix": mountDevSuffix,
})
return []interface{}{devPath, mount.ContainerPath}, mountScriptLine, nil
}

func generateFsBasedVolumeMounts(mount types.VMMount) ([]interface{}, string, error) {
mountTag := path.Base(mount.ContainerPath)
fsMountScript := fmt.Sprintf("if ! mountpoint '%s'; then mkdir -p '%s' && mount -t 9p -o trans=virtio %s '%s'; fi",
mount.ContainerPath, mount.ContainerPath, mountTag, mount.ContainerPath)
fsMountScript := mountFSScriptTemplate.MustExecuteToString(map[string]string{
"ContainerPath": mount.ContainerPath,
"MountTag": mountTag,
})
r := []interface{}{mountTag, mount.ContainerPath, "9p", "trans=virtio"}
return r, fsMountScript, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/TestCRIMounts.out.yaml
Expand Up @@ -255,7 +255,7 @@
write_files:
- content: |
#!/bin/sh
if ! mountpoint '/mnt'; then mkdir -p '/mnt' && mount /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 '/mnt'; fi
if ! mountpoint /mnt; then mkdir -p /mnt && mount /dev/`ls /sys/devices/pci0000:00/0000:00:01.0/virtio*/host*/target*:0:0/*:0:0:1/block/`1 /mnt; fi
path: /etc/cloud/mount-volumes.sh
permissions: "0755"
- name: 'leave: StartContainer'
Expand Down
66 changes: 66 additions & 0 deletions pkg/utils/shelltemplate.go
@@ -0,0 +1,66 @@
/*
Copyright 2018 Mirantis
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import (
"bytes"
"log"
"text/template"

"github.com/kballard/go-shellquote"
)

// ShellTemplate denotes a simple template used to generate shell
// commands. It adds 'shellquote' function to go-template that can be
// used to quote string values for the shell.
type ShellTemplate struct {
*template.Template
}

// NewShellTemplate returns a new shell template using the specified
// text. It panics if the template doesn't compile.
func NewShellTemplate(text string) *ShellTemplate {
funcs := map[string]interface{}{
"shq": func(text string) string {
return shellquote.Join(text)
},
}
return &ShellTemplate{
template.Must(template.New("script").Funcs(funcs).Parse(text)),
}
}

// ExecuteToString executes the template using the specified data and
// returns the result as a string.
func (t *ShellTemplate) ExecuteToString(data interface{}) (string, error) {
var buf bytes.Buffer
err := t.Template.Execute(&buf, data)
if err != nil {
return "", err
}
return buf.String(), nil
}

// MustExecuteToString executes the template using the specified data
// and returns the result as a string. It panics upon an error
func (t *ShellTemplate) MustExecuteToString(data interface{}) string {
r, err := t.ExecuteToString(data)
if err != nil {
log.Panicf("Error executing template: %v", err)
}
return r
}
34 changes: 34 additions & 0 deletions pkg/utils/shelltemplate_test.go
@@ -0,0 +1,34 @@
/*
Copyright 2018 Mirantis
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package utils

import "testing"

func TestNewShellTemplate(t *testing.T) {
tmpl := NewShellTemplate("mount {{ shq .Foobar }} {{ .Baz }}")
out, err := tmpl.ExecuteToString(map[string]string{
"Foobar": "abc def",
"Baz": "qqq",
})
if err != nil {
t.Fatalf("Execute: %v", err)
}
expectedText := "mount 'abc def' qqq"
if out != expectedText {
t.Errorf("Bad template result. Expected:\n%s\nGot:\n%s", expectedText, out)
}
}

0 comments on commit a4a3d1d

Please sign in to comment.