Split validation into fields and service

We want to give feedback to the user as soon as possible about the
validity of the config supplied for the services.

When extending a service, we can validate that the fields are
correct against our schema but we must wait until the *end* of
the extends cycle once all of the extended dicts have been merged
into the service dict, to perform the final validation check on the
config to ensure it is a complete valid service.

Doing this before that had happened resulted in false reports of
invalid config, as common config when split out, by itself, is not
a valid service but *is* valid config to be included.

Signed-off-by: Mazz Mosley <[email protected]>

compose/config/config.py

@@ -10,7 +10,8 @@ from .errors import CircularReference
 from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
-from .validation import validate_against_schema
+from .validation import validate_against_fields_schema
+from .validation import validate_against_service_schema
 from .validation import validate_extended_service_exists
 from .validation import validate_extends_file_path
 from .validation import validate_service_names
@@ -139,7 +140,7 @@ def load(config_details):
     config, working_dir, filename = config_details
 
     processed_config = pre_process_config(config)
-    validate_against_schema(processed_config)
+    validate_against_fields_schema(processed_config)
 
     service_dicts = []
 
@@ -193,7 +194,7 @@ class ServiceLoader(object):
                 full_extended_config,
                 self.extended_config_path
             )
-            validate_against_schema(full_extended_config)
+            validate_against_fields_schema(full_extended_config)
 
             self.extended_config = full_extended_config[self.extended_service_name]
         else:
@@ -205,6 +206,10 @@ class ServiceLoader(object):
 
     def make_service_dict(self):
         self.service_dict = self.resolve_extends()
+
+        if not self.already_seen:
+            validate_against_service_schema(self.service_dict)
+
         return process_container_options(self.service_dict, working_dir=self.working_dir)
 
     def resolve_environment(self):

compose/config/schema.json → compose/config/fields_schema.json

@@ -106,24 +106,6 @@
         "working_dir": {"type": "string"}
       },
 
-      "anyOf": [
-        {
-          "required": ["build"],
-          "not": {"required": ["image"]}
-        },
-        {
-          "required": ["image"],
-          "not": {"anyOf": [
-            {"required": ["build"]},
-            {"required": ["dockerfile"]}
-          ]}
-        },
-        {
-          "required": ["extends"],
-          "not": {"required": ["build", "image"]}
-        }
-      ],
-
       "dependencies": {
         "memswap_limit": ["mem_limit"]
       },

compose/config/service_schema.json

@@ -0,0 +1,39 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+
+  "type": "object",
+
+  "properties": {
+      "name": {"type": "string"}
+  },
+
+  "required": ["name"],
+
+  "allOf": [
+    {"$ref": "fields_schema.json#/definitions/service"},
+    {"$ref": "#/definitions/service_constraints"}
+  ],
+
+  "definitions": {
+    "service_constraints": {
+      "anyOf": [
+        {
+          "required": ["build"],
+          "not": {"required": ["image"]}
+        },
+        {
+          "required": ["image"],
+          "not": {"anyOf": [
+            {"required": ["build"]},
+            {"required": ["dockerfile"]}
+          ]}
+        },
+        {
+          "required": ["extends"],
+          "not": {"required": ["build", "image"]}
+        }
+      ]
+    }
+  }
+
+}

compose/config/validation.py

@@ -5,6 +5,7 @@ from functools import wraps
 from docker.utils.ports import split_port
 from jsonschema import Draft4Validator
 from jsonschema import FormatChecker
+from jsonschema import RefResolver
 from jsonschema import ValidationError
 
 from .errors import ConfigurationError
@@ -210,14 +211,25 @@ def process_errors(errors):
     return "\n".join(root_msgs + invalid_keys + required + type_errors + other_errors)
 
 
-def validate_against_schema(config):
+def validate_against_fields_schema(config):
+    schema_filename = "fields_schema.json"
+    return _validate_against_schema(config, schema_filename)
+
+
+def validate_against_service_schema(config):
+    schema_filename = "service_schema.json"
+    return _validate_against_schema(config, schema_filename)
+
+
+def _validate_against_schema(config, schema_filename):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
-    schema_file = os.path.join(config_source_dir, "schema.json")
+    schema_file = os.path.join(config_source_dir, schema_filename)
 
     with open(schema_file, "r") as schema_fh:
         schema = json.load(schema_fh)
 
-    validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"]))
+    resolver = RefResolver('file://' + config_source_dir + '/', schema)
+    validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(["ports"]))
 
     errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
     if errors:

tests/fixtures/extends/service-with-invalid-schema.yml

@@ -1,5 +1,4 @@
 myweb:
   extends:
+    file: valid-composite-extends.yml
     service: web
-web:
-  command: top

tests/fixtures/extends/service-with-valid-composite-extends.yml

@@ -0,0 +1,5 @@
+myweb:
+  build: '.'
+  extends:
+    file: 'valid-composite-extends.yml'
+    service: web

tests/fixtures/extends/valid-common-config.yml

@@ -0,0 +1,6 @@
+myweb:
+  build: '.'
+  extends:
+    file: valid-common.yml
+    service: common-config
+  command: top

tests/fixtures/extends/valid-common.yml

@@ -0,0 +1,3 @@
+common-config:
+  environment:
+    - FOO=1

tests/fixtures/extends/valid-composite-extends.yml

@@ -0,0 +1,2 @@
+web:
+  command: top

tests/unit/config_test.py

@@ -866,6 +866,7 @@ class ExtendsTest(unittest.TestCase):
 
         self.assertEquals(len(service), 1)
         self.assertIsInstance(service[0], dict)
+        self.assertEquals(service[0]['command'], "/bin/true")
 
     def test_extended_service_with_invalid_config(self):
         expected_error_msg = "Service 'myweb' has neither an image nor a build path specified"
@@ -873,6 +874,10 @@ class ExtendsTest(unittest.TestCase):
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             load_from_filename('tests/fixtures/extends/service-with-invalid-schema.yml')
 
+    def test_extended_service_with_valid_config(self):
+        service = load_from_filename('tests/fixtures/extends/service-with-valid-composite-extends.yml')
+        self.assertEquals(service[0]['command'], "top")
+
     def test_extends_file_defaults_to_self(self):
         """
         Test not specifying a file in our extends options that the
@@ -955,6 +960,10 @@ class ExtendsTest(unittest.TestCase):
         with self.assertRaisesRegexp(ConfigurationError, err_msg):
             load_from_filename('tests/fixtures/extends/nonexistent-service.yml')
 
+    def test_partial_service_config_in_extends_is_still_valid(self):
+        dicts = load_from_filename('tests/fixtures/extends/valid-common-config.yml')
+        self.assertEqual(dicts[0]['environment'], {'FOO': '1'})
+
 
 class BuildPathTest(unittest.TestCase):
     def setUp(self):