Log a warning when boolean is found in `environment`

We're going to warn people that allowing a boolean in the environment is
being deprecated, so in a future release we can disallow it. This is to
ensure boolean variables are quoted in strings to ensure they don't get
mis-parsed by YML.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>

compose/config/fields_schema.json

@@ -40,7 +40,8 @@
               "type": "object",
               "patternProperties": {
                 "^[a-zA-Z0-9_]+$": {
-                  "type": ["string", "number"]
+                  "type": ["string", "number", "boolean"],
+                  "format": "environment"
                 }
               },
               "additionalProperties": false

compose/config/validation.py

@@ -1,4 +1,5 @@
 import json
+import logging
 import os
 from functools import wraps
 
@@ -11,6 +12,9 @@ from jsonschema import ValidationError
 from .errors import ConfigurationError
 
 
+log = logging.getLogger(__name__)
+
+
 DOCKER_CONFIG_HINTS = {
     'cpu_share': 'cpu_shares',
     'add_host': 'extra_hosts',
@@ -44,6 +48,21 @@ def format_ports(instance):
     return True
 
 
+@FormatChecker.cls_checks(format="environment")
+def format_boolean_in_environment(instance):
+    """
+    Check if there is a boolean in the environment and display a warning.
+    Always return True here so the validation won't raise an error.
+    """
+    if isinstance(instance, bool):
+        log.warn(
+            "Warning: There is a boolean value, {0} in the 'environment' key.\n"
+            "Environment variables can only be strings.\nPlease add quotes to any boolean values to make them string "
+            "(eg, '{0}').\nThis warning will become an error in a future release. \r\n".format(instance)
+        )
+    return True
+
+
 def validate_service_names(func):
     @wraps(func)
     def func_wrapper(config):
@@ -259,15 +278,17 @@ def process_errors(errors, service_name=None):
 
 def validate_against_fields_schema(config):
     schema_filename = "fields_schema.json"
-    return _validate_against_schema(config, schema_filename)
+    format_checkers = ["ports", "environment"]
+    return _validate_against_schema(config, schema_filename, format_checkers)
 
 
 def validate_against_service_schema(config, service_name):
     schema_filename = "service_schema.json"
-    return _validate_against_schema(config, schema_filename, service_name)
+    format_checkers = ["ports"]
+    return _validate_against_schema(config, schema_filename, format_checkers, service_name)
 
 
-def _validate_against_schema(config, schema_filename, service_name=None):
+def _validate_against_schema(config, schema_filename, format_checker=[], service_name=None):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
     schema_file = os.path.join(config_source_dir, schema_filename)
 
@@ -275,7 +296,7 @@ def _validate_against_schema(config, schema_filename, service_name=None):
         schema = json.load(schema_fh)
 
     resolver = RefResolver('file://' + config_source_dir + '/', schema)
-    validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(["ports"]))
+    validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(format_checker))
 
     errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
     if errors:

tests/unit/config_test.py

@@ -270,20 +270,22 @@ class ConfigTest(unittest.TestCase):
             )
             self.assertEqual(service[0]['entrypoint'], entrypoint)
 
-    def test_config_environment_contains_boolean_validation_error(self):
+    @mock.patch('compose.config.validation.log')
-        expected_error_msg = "Service 'web' configuration key 'environment' contains an invalid type"
+    def test_logs_warning_for_boolean_in_environment(self, mock_logging):
-
+        expected_warning_msg = "Warning: There is a boolean value, True in the 'environment' key."
-        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+        config.load(
-            config.load(
+            config.ConfigDetails(
-                config.ConfigDetails(
+                {'web': {
-                    {'web': {
+                    'image': 'busybox',
-                        'image': 'busybox',
+                    'environment': {'SHOW_STUFF': True}
-                        'environment': {'SHOW_STUFF': True}
+                }},
-                    }},
+                'working_dir',
-                    'working_dir',
+                'filename.yml'
-                    'filename.yml'
-                )
             )
+        )
+
+        self.assertTrue(mock_logging.warn.called)
+        self.assertTrue(expected_warning_msg in mock_logging.warn.call_args[0][0])
 
 
 class InterpolationTest(unittest.TestCase):