Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move all validation to config #2405

Merged
merged 7 commits into from
Nov 25, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions compose/cli/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from . import verbose_proxy
from .. import config
from ..project import Project
from ..service import ConfigError
from .docker_client import docker_client
from .utils import call_silently
from .utils import get_version_info
Expand Down Expand Up @@ -84,16 +83,12 @@ def get_project(base_dir, config_path=None, project_name=None, verbose=False,
config_details = config.find(base_dir, config_path)

api_version = '1.21' if use_networking else None
try:
return Project.from_dicts(
get_project_name(config_details.working_dir, project_name),
config.load(config_details),
get_client(verbose=verbose, version=api_version),
use_networking=use_networking,
network_driver=network_driver,
)
except ConfigError as e:
raise errors.UserError(six.text_type(e))
return Project.from_dicts(
get_project_name(config_details.working_dir, project_name),
config.load(config_details),
get_client(verbose=verbose, version=api_version),
use_networking=use_networking,
network_driver=network_driver)


def get_project_name(working_dir, project_name=None):
Expand Down
1 change: 0 additions & 1 deletion compose/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from .config import ConfigurationError
from .config import DOCKER_CONFIG_KEYS
from .config import find
from .config import get_service_name_from_net
from .config import load
from .config import merge_environment
from .config import parse_environment
49 changes: 36 additions & 13 deletions compose/config/config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import absolute_import

import codecs
import logging
import operator
Expand All @@ -12,6 +14,12 @@
from .errors import ComposeFileNotFound
from .errors import ConfigurationError
from .interpolation import interpolate_environment_variables
from .sort_services import get_service_name_from_net
from .sort_services import sort_service_dicts
from .types import parse_extra_hosts
from .types import parse_restart_spec
from .types import VolumeFromSpec
from .types import VolumeSpec
from .validation import validate_against_fields_schema
from .validation import validate_against_service_schema
from .validation import validate_extends_file_path
Expand Down Expand Up @@ -198,16 +206,20 @@ def build_service(filename, service_name, service_dict):
service_dict)
resolver = ServiceExtendsResolver(service_config)
service_dict = process_service(resolver.run())

# TODO: move to validate_service()
validate_against_service_schema(service_dict, service_config.name)
validate_paths(service_dict)

service_dict = finalize_service(service_config._replace(config=service_dict))
service_dict['name'] = service_config.name
return service_dict

def build_services(config_file):
return [
return sort_service_dicts([
build_service(config_file.filename, name, service_dict)
for name, service_dict in config_file.config.items()
]
])

def merge_services(base, override):
all_service_names = set(base) | set(override)
Expand Down Expand Up @@ -353,6 +365,7 @@ def validate_ulimits(ulimit_config):
"than 'hard' value".format(ulimit_config))


# TODO: rename to normalize_service
def process_service(service_config):
working_dir = service_config.working_dir
service_dict = dict(service_config.config)
Expand All @@ -370,12 +383,33 @@ def process_service(service_config):
if 'labels' in service_dict:
service_dict['labels'] = parse_labels(service_dict['labels'])

if 'extra_hosts' in service_dict:
service_dict['extra_hosts'] = parse_extra_hosts(service_dict['extra_hosts'])

# TODO: move to a validate_service()
if 'ulimits' in service_dict:
validate_ulimits(service_dict['ulimits'])

return service_dict


def finalize_service(service_config):
service_dict = dict(service_config.config)

if 'volumes_from' in service_dict:
service_dict['volumes_from'] = [
VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']]

if 'volumes' in service_dict:
service_dict['volumes'] = [
VolumeSpec.parse(v) for v in service_dict['volumes']]

if 'restart' in service_dict:
service_dict['restart'] = parse_restart_spec(service_dict['restart'])

return service_dict


def merge_service_dicts_from_files(base, override):
"""When merging services from multiple files we need to merge the `extends`
field. This is not handled by `merge_service_dicts()` which is used to
Expand Down Expand Up @@ -606,17 +640,6 @@ def to_list(value):
return value


def get_service_name_from_net(net_config):
if not net_config:
return

if not net_config.startswith('container:'):
return

_, net_name = net_config.split(':', 1)
return net_name


def load_yaml(filename):
try:
with open(filename, 'r') as fh:
Expand Down
4 changes: 4 additions & 0 deletions compose/config/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ def __str__(self):
return self.msg


class DependencyError(ConfigurationError):
pass


class CircularReference(ConfigurationError):
def __init__(self, trail):
self.trail = trail
Expand Down
31 changes: 12 additions & 19 deletions compose/config/fields_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,7 @@
"domainname": {"type": "string"},
"entrypoint": {"$ref": "#/definitions/string_or_list"},
"env_file": {"$ref": "#/definitions/string_or_list"},

"environment": {
"oneOf": [
{
"type": "object",
"patternProperties": {
".+": {
"type": ["string", "number", "boolean", "null"],
"format": "environment"
}
},
"additionalProperties": false
},
{"type": "array", "items": {"type": "string"}, "uniqueItems": true}
]
},
"environment": {"$ref": "#/definitions/list_or_dict"},

"expose": {
"type": "array",
Expand Down Expand Up @@ -165,10 +150,18 @@

"list_or_dict": {
"oneOf": [
{"type": "array", "items": {"type": "string"}, "uniqueItems": true},
{"type": "object"}
{
"type": "object",
"patternProperties": {
".+": {
"type": ["string", "number", "boolean", "null"],
"format": "bool-value-in-mapping"
}
},
"additionalProperties": false
},
{"type": "array", "items": {"type": "string"}, "uniqueItems": true}
]
}

}
}
55 changes: 55 additions & 0 deletions compose/config/sort_services.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
from compose.config.errors import DependencyError


def get_service_name_from_net(net_config):
if not net_config:
return

if not net_config.startswith('container:'):
return

_, net_name = net_config.split(':', 1)
return net_name


def sort_service_dicts(services):
# Topological sort (Cormen/Tarjan algorithm).
unmarked = services[:]
temporary_marked = set()
sorted_services = []

def get_service_names(links):
return [link.split(':')[0] for link in links]

def get_service_names_from_volumes_from(volumes_from):
return [volume_from.source for volume_from in volumes_from]

def get_service_dependents(service_dict, services):
name = service_dict['name']
return [
service for service in services
if (name in get_service_names(service.get('links', [])) or
name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or
name == get_service_name_from_net(service.get('net')))
]

def visit(n):
if n['name'] in temporary_marked:
if n['name'] in get_service_names(n.get('links', [])):
raise DependencyError('A service can not link to itself: %s' % n['name'])
if n['name'] in n.get('volumes_from', []):
raise DependencyError('A service can not mount itself as volume: %s' % n['name'])
else:
raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked))
if n in unmarked:
temporary_marked.add(n['name'])
for m in get_service_dependents(n, services):
visit(m)
temporary_marked.remove(n['name'])
unmarked.remove(n)
sorted_services.insert(0, n)

while unmarked:
visit(unmarked[-1])

return sorted_services
120 changes: 120 additions & 0 deletions compose/config/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
"""
Types for objects parsed from the configuration.
"""
from __future__ import absolute_import
from __future__ import unicode_literals

import os
from collections import namedtuple

from compose.config.errors import ConfigurationError
from compose.const import IS_WINDOWS_PLATFORM


class VolumeFromSpec(namedtuple('_VolumeFromSpec', 'source mode')):

@classmethod
def parse(cls, volume_from_config):
parts = volume_from_config.split(':')
if len(parts) > 2:
raise ConfigurationError(
"volume_from {} has incorrect format, should be "
"service[:mode]".format(volume_from_config))

if len(parts) == 1:
source = parts[0]
mode = 'rw'
else:
source, mode = parts

return cls(source, mode)


def parse_restart_spec(restart_config):
if not restart_config:
return None
parts = restart_config.split(':')
if len(parts) > 2:
raise ConfigurationError(
"Restart %s has incorrect format, should be "
"mode[:max_retry]" % restart_config)
if len(parts) == 2:
name, max_retry_count = parts
else:
name, = parts
max_retry_count = 0

return {'Name': name, 'MaximumRetryCount': int(max_retry_count)}


def parse_extra_hosts(extra_hosts_config):
if not extra_hosts_config:
return {}

if isinstance(extra_hosts_config, dict):
return dict(extra_hosts_config)

if isinstance(extra_hosts_config, list):
extra_hosts_dict = {}
for extra_hosts_line in extra_hosts_config:
# TODO: validate string contains ':' ?
host, ip = extra_hosts_line.split(':')
extra_hosts_dict[host.strip()] = ip.strip()
return extra_hosts_dict


def normalize_paths_for_engine(external_path, internal_path):
"""Windows paths, c:\my\path\shiny, need to be changed to be compatible with
the Engine. Volume paths are expected to be linux style /c/my/path/shiny/
"""
if not IS_WINDOWS_PLATFORM:
return external_path, internal_path

if external_path:
drive, tail = os.path.splitdrive(external_path)

if drive:
external_path = '/' + drive.lower().rstrip(':') + tail

external_path = external_path.replace('\\', '/')

return external_path, internal_path.replace('\\', '/')


class VolumeSpec(namedtuple('_VolumeSpec', 'external internal mode')):

@classmethod
def parse(cls, volume_config):
"""Parse a volume_config path and split it into external:internal[:mode]
parts to be returned as a valid VolumeSpec.
"""
if IS_WINDOWS_PLATFORM:
# relative paths in windows expand to include the drive, eg C:\
# so we join the first 2 parts back together to count as one
drive, tail = os.path.splitdrive(volume_config)
parts = tail.split(":")

if drive:
parts[0] = drive + parts[0]
else:
parts = volume_config.split(':')

if len(parts) > 3:
raise ConfigurationError(
"Volume %s has incorrect format, should be "
"external:internal[:mode]" % volume_config)

if len(parts) == 1:
external, internal = normalize_paths_for_engine(
None,
os.path.normpath(parts[0]))
else:
external, internal = normalize_paths_for_engine(
os.path.normpath(parts[0]),
os.path.normpath(parts[1]))

mode = 'rw'
if len(parts) == 3:
mode = parts[2]

return cls(external, internal, mode)
4 changes: 2 additions & 2 deletions compose/config/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def format_ports(instance):
return True


@FormatChecker.cls_checks(format="environment")
@FormatChecker.cls_checks(format="bool-value-in-mapping")
def format_boolean_in_environment(instance):
"""
Check if there is a boolean in the environment and display a warning.
Expand Down Expand Up @@ -273,7 +273,7 @@ def validate_against_fields_schema(config, filename):
_validate_against_schema(
config,
"fields_schema.json",
format_checker=["ports", "environment"],
format_checker=["ports", "bool-value-in-mapping"],
filename=filename)


Expand Down
Loading