Merge pull request #2000 from mnowster/do-not-allow-booleans-in-environment

Disallow booleans in environment

Author: aanand

compose/config/fields_schema.json

@@ -36,7 +36,16 @@
 
         "environment": {
           "oneOf": [
-            {"type": "object"},
+            {
+              "type": "object",
+              "patternProperties": {
+                "^[^-]+$": {
+                  "type": ["string", "number", "boolean"],
+                  "format": "environment"
+                }
+              },
+              "additionalProperties": false
+            },
             {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
           ]
         },

compose/config/validation.py

@@ -1,4 +1,5 @@
 import json
+import logging
 import os
 from functools import wraps
 
@@ -11,6 +12,9 @@
 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,23 +278,25 @@ def _parse_oneof_validator(error):
 
 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)
 
     with open(schema_file, "r") as schema_fh:
         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:

docs/yml.md

@@ -184,17 +184,21 @@ Mount all of the volumes from another service or container.
 
 ### environment
 
-Add environment variables. You can use either an array or a dictionary.
+Add environment variables. You can use either an array or a dictionary. Any
+boolean values; true, false, yes no, need to be enclosed in quotes to ensure
+they are not converted to True or False by the YML parser.
 
 Environment variables with only a key are resolved to their values on the
 machine Compose is running on, which can be helpful for secret or host-specific values.
 
     environment:
       RACK_ENV: development
+      SHOW: 'true'
       SESSION_SECRET:
 
     environment:
       - RACK_ENV=development
+      - SHOW=true
       - SESSION_SECRET
 
 ### env_file

tests/unit/config_test.py

@@ -270,15 +270,32 @@ def test_valid_config_oneof_string_or_list(self):
             )
             self.assertEqual(service[0]['entrypoint'], entrypoint)
 
-    def test_validation_message_for_invalid_type_when_multiple_types_allowed(self):
-        expected_error_msg = "Service 'web' configuration key 'mem_limit' contains an invalid type, it should be a number or a string"
+    @mock.patch('compose.config.validation.log')
+    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."
+        config.load(
+            config.ConfigDetails(
+                {'web': {
+                    'image': 'busybox',
+                    'environment': {'SHOW_STUFF': True}
+                }},
+                'working_dir',
+                'filename.yml'
+            )
+        )
+
+        self.assertTrue(mock_logging.warn.called)
+        self.assertTrue(expected_warning_msg in mock_logging.warn.call_args[0][0])
+
+    def test_config_invalid_environment_dict_key_raises_validation_error(self):
+        expected_error_msg = "Service 'web' configuration key 'environment' contains an invalid type"
 
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             config.load(
                 config.ConfigDetails(
                     {'web': {
                         'image': 'busybox',
-                        'mem_limit': ['incorrect']
+                        'environment': {'---': 'nope'}
                     }},
                     'working_dir',
                     'filename.yml'