[queens-only] Improve volume validation (unclean backport)
Manually backport I9a55698b04dc2c5f01d776c6bdc2f26d47baf803. We need to support both a source filesystem location and container volume for the volume mounts. This change adds the ability to validate that the provided source container is a container volume. Additionally aligns the validation between the docker/podman methods so they operate in the same fashion. Change-Id: Id42eb9d550865682078781a04f961c13bfbcde78 Closes-Bug: #1843734
This commit is contained in:
parent
66487257e7
commit
e13226d4cf
|
@ -235,6 +235,8 @@ class ComposeV1Builder(object):
|
||||||
cmd.append(cconfig.get('image', ''))
|
cmd.append(cconfig.get('image', ''))
|
||||||
cmd.extend(self.command_argument(cconfig.get('command')))
|
cmd.extend(self.command_argument(cconfig.get('command')))
|
||||||
|
|
||||||
|
return self.validate_volumes(cconfig.get('volumes', []))
|
||||||
|
|
||||||
def docker_exec_args(self, cmd, container):
|
def docker_exec_args(self, cmd, container):
|
||||||
cconfig = self.config[container]
|
cconfig = self.config[container]
|
||||||
if 'privileged' in cconfig:
|
if 'privileged' in cconfig:
|
||||||
|
@ -311,6 +313,28 @@ class ComposeV1Builder(object):
|
||||||
return command.split()
|
return command.split()
|
||||||
return command
|
return command
|
||||||
|
|
||||||
|
def validate_volumes(self, volumes):
|
||||||
|
"""Validate volume sources
|
||||||
|
|
||||||
|
Validates that the source volume either exists on the filesystem
|
||||||
|
or is a valid container volume. Since docker will error if the
|
||||||
|
source volume filesystem path doesn't exist, we want to catch the
|
||||||
|
error before docker.
|
||||||
|
|
||||||
|
:param: volumes: list of volume mounts in the format of "src:path"
|
||||||
|
"""
|
||||||
|
valid = True
|
||||||
|
for volume in volumes:
|
||||||
|
if not volume:
|
||||||
|
# ignore when volume is ''
|
||||||
|
continue
|
||||||
|
src_path = volume.split(':', 1)[0]
|
||||||
|
check = self.runner.validate_volume_source(src_path)
|
||||||
|
if not check:
|
||||||
|
self.log.error("%s is not a valid volume source" % src_path)
|
||||||
|
valid = False
|
||||||
|
return valid
|
||||||
|
|
||||||
|
|
||||||
class PullException(Exception):
|
class PullException(Exception):
|
||||||
|
|
||||||
|
|
|
@ -13,6 +13,7 @@
|
||||||
|
|
||||||
import collections
|
import collections
|
||||||
import json
|
import json
|
||||||
|
import os
|
||||||
import random
|
import random
|
||||||
import string
|
import string
|
||||||
import subprocess
|
import subprocess
|
||||||
|
@ -225,3 +226,32 @@ class DockerRunner(object):
|
||||||
for container in self.containers_in_config(conf_id):
|
for container in self.containers_in_config(conf_id):
|
||||||
configs[conf_id].append(self.inspect(container))
|
configs[conf_id].append(self.inspect(container))
|
||||||
return configs
|
return configs
|
||||||
|
|
||||||
|
def validate_volume_source(self, volume):
|
||||||
|
"""Validate that the provided volume
|
||||||
|
|
||||||
|
This checks that the provided volume either exists on the filesystem
|
||||||
|
or is a container volume.
|
||||||
|
|
||||||
|
:param: volume: string containing either a filesystme path or container
|
||||||
|
volume name
|
||||||
|
"""
|
||||||
|
if os.path.exists(volume):
|
||||||
|
return True
|
||||||
|
|
||||||
|
if os.path.sep in volume:
|
||||||
|
# if we get here and have a path seperator, let's skip the
|
||||||
|
# container lookup because container volumes won't have / in them.
|
||||||
|
self.log.debug('Path seperator found in volume (%s), but did not '
|
||||||
|
'exist on the file system' % volume)
|
||||||
|
return False
|
||||||
|
|
||||||
|
self.log.debug('Running volume lookup for "%s"' % volume)
|
||||||
|
filter_opt = '--filter=name={}'.format(volume)
|
||||||
|
cmd = [self.docker_cmd, 'volume', 'ls', '-q', filter_opt]
|
||||||
|
cmd_stdout, cmd_stderr, returncode = self.execute(cmd)
|
||||||
|
if returncode != 0:
|
||||||
|
self.log.error('Error during volume verification')
|
||||||
|
self.log.error(cmd_stderr)
|
||||||
|
return False
|
||||||
|
return (volume in set(cmd_stdout.split()))
|
||||||
|
|
|
@ -477,7 +477,8 @@ three-12345678 three''', '', 0),
|
||||||
)
|
)
|
||||||
|
|
||||||
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
||||||
def test_docker_run_args_lists(self, mock_cpu):
|
@mock.patch('paunch.runner.DockerRunner', autospec=True)
|
||||||
|
def test_docker_run_args_lists(self, mock_runner, mock_cpu):
|
||||||
config = {
|
config = {
|
||||||
'one': {
|
'one': {
|
||||||
'image': 'centos:7',
|
'image': 'centos:7',
|
||||||
|
@ -493,7 +494,8 @@ three-12345678 three''', '', 0),
|
||||||
'volumes_from': ['two', 'three']
|
'volumes_from': ['two', 'three']
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
builder = compose1.ComposeV1Builder('foo', config, None)
|
mock_runner.validate_volume_source.return_value = True
|
||||||
|
builder = compose1.ComposeV1Builder('foo', config, mock_runner)
|
||||||
|
|
||||||
cmd = ['docker', 'run', '--name', 'one']
|
cmd = ['docker', 'run', '--name', 'one']
|
||||||
builder.docker_run_args(cmd, 'one')
|
builder.docker_run_args(cmd, 'one')
|
||||||
|
@ -511,7 +513,8 @@ three-12345678 three''', '', 0),
|
||||||
)
|
)
|
||||||
|
|
||||||
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
||||||
def test_docker_run_args_lists_with_cpu(self, mock_cpu):
|
@mock.patch('paunch.runner.DockerRunner', autospec=True)
|
||||||
|
def test_docker_run_args_lists_with_cpu(self, mock_runner, mock_cpu):
|
||||||
config = {
|
config = {
|
||||||
'one': {
|
'one': {
|
||||||
'image': 'centos:7',
|
'image': 'centos:7',
|
||||||
|
@ -528,7 +531,8 @@ three-12345678 three''', '', 0),
|
||||||
'cpuset_cpus': '0-2',
|
'cpuset_cpus': '0-2',
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
builder = compose1.ComposeV1Builder('foo', config, None)
|
mock_runner.validate_volume_source.return_value = True
|
||||||
|
builder = compose1.ComposeV1Builder('foo', config, mock_runner)
|
||||||
|
|
||||||
cmd = ['docker', 'run', '--name', 'one']
|
cmd = ['docker', 'run', '--name', 'one']
|
||||||
builder.docker_run_args(cmd, 'one')
|
builder.docker_run_args(cmd, 'one')
|
||||||
|
@ -592,3 +596,43 @@ three-12345678 three''', '', 0),
|
||||||
['ls', '-l', '"/foo', 'bar"'],
|
['ls', '-l', '"/foo', 'bar"'],
|
||||||
b.command_argument('ls -l "/foo bar"')
|
b.command_argument('ls -l "/foo bar"')
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@mock.patch('paunch.runner.DockerRunner', autospec=True)
|
||||||
|
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
||||||
|
def test_docker_run_args_validation_true(self, mock_cpu, mock_runner):
|
||||||
|
config = {
|
||||||
|
'one': {
|
||||||
|
'image': 'foo',
|
||||||
|
'volumes': ['/foo:/foo:rw', '/bar:/bar:ro'],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
mock_runner.validate_volume_source.return_value = True
|
||||||
|
builder = compose1.ComposeV1Builder('foo', config, mock_runner)
|
||||||
|
|
||||||
|
cmd = ['docker']
|
||||||
|
self.assertTrue(builder.docker_run_args(cmd, 'one'))
|
||||||
|
self.assertEqual(
|
||||||
|
['docker', '--detach=true', '--volume=/foo:/foo:rw',
|
||||||
|
'--volume=/bar:/bar:ro', '--cpuset-cpus=0,1,2,3', 'foo'],
|
||||||
|
cmd
|
||||||
|
)
|
||||||
|
|
||||||
|
@mock.patch('paunch.runner.DockerRunner', autospec=True)
|
||||||
|
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
|
||||||
|
def test_docker_run_args_validation_false(self, mock_cpu, mock_runner):
|
||||||
|
config = {
|
||||||
|
'one': {
|
||||||
|
'image': 'foo',
|
||||||
|
'volumes': ['/foo:/foo:rw', '/bar:/bar:ro'],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
mock_runner.validate_volume_source.return_value = False
|
||||||
|
builder = compose1.ComposeV1Builder('foo', config, mock_runner)
|
||||||
|
|
||||||
|
cmd = ['docker']
|
||||||
|
self.assertFalse(builder.docker_run_args(cmd, 'one'))
|
||||||
|
self.assertEqual(
|
||||||
|
['docker', '--detach=true', '--volume=/foo:/foo:rw',
|
||||||
|
'--volume=/bar:/bar:ro', '--cpuset-cpus=0,1,2,3', 'foo'],
|
||||||
|
cmd
|
||||||
|
)
|
||||||
|
|
|
@ -284,3 +284,27 @@ four-12345678 four
|
||||||
'two': [{'e': 'f'}, {'e': 'f'}, {'e': 'f'}],
|
'two': [{'e': 'f'}, {'e': 'f'}, {'e': 'f'}],
|
||||||
'three': [{'e': 'f'}, {'e': 'f'}, {'e': 'f'}]
|
'three': [{'e': 'f'}, {'e': 'f'}, {'e': 'f'}]
|
||||||
}, result)
|
}, result)
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', return_value=True)
|
||||||
|
def test_validate_volume_source_file(self, exists_mock):
|
||||||
|
self.assertTrue(self.runner.validate_volume_source('/tmp'))
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', return_value=False)
|
||||||
|
def test_validate_volume_source_file_fail(self, exists_mock):
|
||||||
|
self.assertFalse(self.runner.validate_volume_source('/nope'))
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', return_value=False)
|
||||||
|
@mock.patch('subprocess.Popen')
|
||||||
|
def test_validate_volume_source_container(self, popen, exists_mock):
|
||||||
|
ps_result = '''foo
|
||||||
|
foobar
|
||||||
|
'''
|
||||||
|
self.mock_execute(popen, ps_result, '', 0)
|
||||||
|
self.assertTrue(self.runner.validate_volume_source('foo'))
|
||||||
|
|
||||||
|
@mock.patch('os.path.exists', return_value=False)
|
||||||
|
@mock.patch('subprocess.Popen')
|
||||||
|
def test_validate_volume_source_container_fail(self, popen, exists_mock):
|
||||||
|
ps_result = ''
|
||||||
|
self.mock_execute(popen, ps_result, '', 0)
|
||||||
|
self.assertFalse(self.runner.validate_volume_source('foo'))
|
||||||
|
|
Loading…
Reference in New Issue