Fix decrypting pack config keys under additionalProperties

CHANGELOG.rst

@@ -42,6 +42,11 @@ Added
 
   Contributed by @Kami.
 
+* Fix a bug in the pack config loader so that objects covered by an additionalProperties schema
+  can use encrypted datastore keys and have their default values applied correctly.
+
+  Contributed by @cognifloyd.
+
 Changed
 ~~~~~~~
 

st2common/st2common/util/config_loader.py

@@ -16,6 +16,8 @@
 from __future__ import absolute_import
 import copy
 
+from collections import defaultdict
+
 import six
 
 from oslo_config import cfg
@@ -98,6 +100,20 @@ def _get_values_for_config(self, config_schema_db, config_db):
         config = self._assign_default_values(schema=schema_values, config=config)
         return config
 
+    @staticmethod
+    def _get_object_property_schema(object_schema, init_additional_properties=None):
+        additional_properties = object_schema.get("additionalProperties", {})
+        if additional_properties and isinstance(additional_properties, dict):
+            property_schema = defaultdict(lambda: additional_properties)
+        else:
+            property_schema = {}
+        if init_additional_properties:
+            # ensure that these keys are present in the object (vs just defaultdict)
+            for key in init_additional_properties:
+                property_schema.__missing__(key)
+        property_schema.update(object_schema.get("properties", {}))
+        return property_schema
+
     def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
         """
         Assign dynamic config value for a particular config item if the ite utilizes a Jinja
@@ -119,7 +135,11 @@ def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
         for config_item_key, config_item_value in iterator:
             if config_is_dict:
                 # different schema for each key/value pair
-                schema_item = schema.get(config_item_key, {})
+                try:
+                    # do not use schema.get() as schema might be a defaultdict
+                    schema_item = schema[config_item_key]
+                except KeyError:
+                    schema_item = {}
             if config_is_list:
                 # same schema is shared between every item in the list
                 schema_item = schema
@@ -130,8 +150,9 @@ def _assign_dynamic_config_values(self, schema, config, parent_keys=None):
             # Inspect nested object properties
             if is_dictionary:
                 parent_keys += [str(config_item_key)]
+                property_schema = self._get_object_property_schema(schema_item)
                 self._assign_dynamic_config_values(
-                    schema=schema_item.get("properties", {}),
+                    schema=property_schema,
                     config=config[config_item_key],
                     parent_keys=parent_keys,
                 )
@@ -182,18 +203,28 @@ def _assign_default_values(self, schema, config):
             default_value = schema_item.get("default", None)
             is_object = schema_item.get("type", None) == "object"
             has_properties = schema_item.get("properties", None)
+            has_additional_properties = schema_item.get("additionalProperties", None)
 
             if has_default_value and not has_config_value:
                 # Config value is not provided, but default value is, use a default value
                 config[schema_item_key] = default_value
 
             # Inspect nested object properties
-            if is_object and has_properties:
+            if is_object and (has_properties or has_additional_properties):
                 if not config.get(schema_item_key, None):
                     config[schema_item_key] = {}
 
+                property_schema = self._get_object_property_schema(
+                    schema_item,
+                    init_additional_properties=(
+                        config[schema_item_key].keys()
+                        if has_additional_properties
+                        else None
+                    ),
+                )
+
                 self._assign_default_values(
-                    schema=schema_item["properties"], config=config[schema_item_key]
+                    schema=property_schema, config=config[schema_item_key]
                 )
 
         return config

st2common/tests/unit/test_config_loader.py

@@ -43,7 +43,7 @@ def test_ensure_local_pack_config_feature_removed(self):
 
     def test_get_config_some_values_overriden_in_datastore(self):
         # Test a scenario where some values are overriden in datastore via pack
-        # flobal config
+        # global config
         kvp_db = set_datastore_value_for_config_key(
             pack_name="dummy_pack_5",
             key_name="api_secret",
@@ -518,6 +518,68 @@ def test_get_config_dynamic_config_item_nested_list(self):
 
         config_db.delete()
 
+    def test_get_config_dynamic_config_item_under_additional_properties(self):
+        pack_name = "dummy_pack_schema_with_additional_properties_1"
+        loader = ContentPackConfigLoader(pack_name=pack_name)
+
+        KeyValuePair.add_or_update(KeyValuePairDB(name="k0", value="v0"))
+        KeyValuePair.add_or_update(
+            KeyValuePairDB(name="k1_encrypted", value="v1_encrypted", secret=True)
+        )
+
+        ####################
+        # values in objects under an object with additionalProperties
+        values = {
+            "profiles": {
+                "dev": {
+                    # no host or port to test default value
+                    "token": "hard-coded-secret",
+                },
+                "stage": {
+                    "host": "127.0.0.1",
+                    "port": 8181,
+                    # unencrypted in datastore
+                    "token": "{{st2kv.system.k0}}",
+                },
+                "prod": {
+                    "host": "127.1.2.7",
+                    "port": 8282,
+                    # encrypted in datastore
+                    "token": "{{st2kv.system.k1_encrypted}}",
+                },
+            }
+        }
+        config_db = ConfigDB(pack=pack_name, values=values)
+        config_db = Config.add_or_update(config_db)
+
+        config_rendered = loader.get_config()
+
+        self.assertEqual(
+            config_rendered,
+            {
+                "region": "us-east-1",
+                "profiles": {
+                    "dev": {
+                        "host": "127.0.0.3",
+                        "port": 8080,
+                        "token": "hard-coded-secret",
+                    },
+                    "stage": {
+                        "host": "127.0.0.1",
+                        "port": 8181,
+                        "token": "v0",
+                    },
+                    "prod": {
+                        "host": "127.1.2.7",
+                        "port": 8282,
+                        "token": "v1_encrypted",
+                    },
+                },
+            },
+        )
+
+        config_db.delete()
+
     def test_empty_config_object_in_the_database(self):
         pack_name = "dummy_pack_empty_config"
 

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_additional_properties_1/config.schema.yaml

@@ -0,0 +1,24 @@
+---
+  region:
+    type: "string"
+    required: false
+    default: "us-east-1"
+  profiles:
+    type: "object"
+    required: false
+    additionalProperties:
+      type: object
+      additionalProperties: false
+      properties:
+        host:
+          type: "string"
+          required: false
+          default: "127.0.0.3"
+        port:
+          type: "integer"
+          required: false
+          default: 8080
+        token:
+          type: "string"
+          required: true
+          secret: true

st2tests/st2tests/fixtures/packs/dummy_pack_schema_with_additional_properties_1/pack.yaml

@@ -0,0 +1,6 @@
+---
+name : dummy_pack_schema_with_additional_properties_1
+description : dummy pack with nested objects under additionalProperties
+version : 0.1.0
+author : st2-dev
+email : [email protected]