dcos-774 support for embedded dict in option

Add support for extracting default from embedded JSON object, and
merging of embedded dict within options.

cli/dcoscli/package/main.py

@@ -52,6 +52,8 @@ import pkg_resources
 from dcos.api import (cmds, config, constants, emitting, errors, marathon,
                       options, package, subcommand, util)
 
+logger = util.get_logger(__name__)
+
 emitter = emitting.FlatEmitter()
 
 
@@ -312,6 +314,7 @@ def _install(package_name, options_path, app_id, cli, app):
             emitter.publish(err)
             return 1
     except Exception as e:
+        logger.exception('Exception while generating options')
         emitter.publish(errors.DefaultError(e.message))
         return 1
 

dcos/api/package.py

@@ -1,6 +1,7 @@
 import abc
 import base64
 import collections
+import copy
 import hashlib
 import json
 import os
@@ -495,20 +496,49 @@ def _search_rank(pkg, query):
 def _extract_default_values(config_schema):
     """
     :param config_schema: A json-schema describing configuration options.
-    :type config_schema: (dict, Error)
+    :type config_schema: dict
+    :returns: a dictionary with the default specified by the schema
+    :rtype: dict
     """
 
-    properties = config_schema.get('properties')
-    schema_type = config_schema.get('type')
+    defaults = {}
+    for key, value in config_schema['properties'].items():
+        if 'default' in value:
+            defaults[key] = value['default']
+        elif value.get('type', '') == 'object':
+            # Generate the default value from the embedded schema
+            defaults[key] = _extract_default_values(value)
 
-    if schema_type != 'object' or properties is None:
-        return ({}, None)
+    return defaults
 
-    defaults = [(p, properties[p]['default'])
-                for p in properties
-                if 'default' in properties[p]]
 
-    return (dict(defaults), None)
+def _merge_options(first, second):
+    """Merges the :code:`second` dictionary into the :code:`first` dictionary.
+    If both dictionaries have the same key and both values are dictionaries
+    then it recursively merges those two dictionaries.
+
+    :param first: first dictionary
+    :type first: dict
+    :param second: second dictionary
+    :type second: dict
+    :returns: merged dictionary
+    :rtype: dict
+    """
+
+    result = copy.deepcopy(first)
+    for key, second_value in second.items():
+        if key in first:
+            first_value = first[key]
+
+            if (isinstance(first_value, collections.Mapping) and
+               isinstance(second_value, collections.Mapping)):
+                result[key] = _merge_options(first_value, second_value)
+            else:
+                result[key] = second_value
+        else:
+            result[key] = second_value
+
+    return result
 
 
 def resolve_package(package_name, config):
@@ -1093,13 +1123,12 @@ class Package():
         if err is not None:
             return (None, err)
 
-        default_options, err = _extract_default_values(config_schema)
-        if err is not None:
-            return (None, err)
+        default_options = _extract_default_values(config_schema)
+        logger.info('Generated default options: %r', default_options)
 
         # Merge option overrides
-        options = dict(list(default_options.items()) +
-                       list(user_options.items()))
+        options = _merge_options(default_options, user_options)
+        logger.info('Merged options: %r', options)
 
         # Validate options with the config schema
         err = util.validate_json(options, config_schema)

tests/test_package.py

@@ -1,33 +1,79 @@
-import json
+import collections
 
 from dcos.api import package
 
+import pytest
+
+MergeData = collections.namedtuple(
+    'MergeData',
+    ['first', 'second', 'expected'])
+
+
+@pytest.fixture(params=[
+    MergeData(
+        first={},
+        second={'a': 1},
+        expected={'a': 1}),
+    MergeData(
+        first={'a': 'a'},
+        second={'a': 1},
+        expected={'a': 1}),
+    MergeData(
+        first={'b': 'b'},
+        second={'a': 1},
+        expected={'b': 'b', 'a': 1}),
+    MergeData(
+        first={'b': 'b'},
+        second={},
+        expected={'b': 'b'}),
+    MergeData(
+        first={'b': {'a': 'a'}},
+        second={'b': {'c': 'c'}},
+        expected={'b': {'c': 'c', 'a': 'a'}}),
+    ])
+def merge_data(request):
+    return request.param
+
 
 def test_extract_default_values():
-
-    config_schema = json.loads("""
-    {
+    config_schema = {
         "type": "object",
         "properties": {
-            "foo.bar": {
-                "type": "string",
-                "description": "A bar name."
+            "foo": {
+                "type": "object",
+                "properties": {
+                    "bar": {
+                        "type": "string",
+                        "description": "A bar name."
+                    },
+                    "baz": {
+                        "type": "integer",
+                        "description": "How many times to do baz.",
+                        "minimum": 0,
+                        "maximum": 16,
+                        "required": False,
+                        "default": 4
+                    }
+                }
             },
-            "foo.baz": {
-                "type": "integer",
-                "description": "How many times to do baz.",
-                "minimum": 0,
-                "maximum": 16,
-                "required": false,
-                "default": 4
+            "fiz": {
+                "type": "boolean",
+                "default": True,
+            },
+            "buz": {
+                "type": "string"
             }
         }
     }
-    """)
 
-    expected = {'foo.baz': 4}
+    expected = {'foo': {'baz': 4}, 'fiz': True}
 
-    result, error = package._extract_default_values(config_schema)
+    result = package._extract_default_values(config_schema)
 
-    assert error is None
     assert result == expected
+
+
+def test_option_merge(merge_data):
+    assert merge_data.expected == package._merge_options(
+        merge_data.first,
+        merge_data.second)